[Cake] Long-RTT broken again

[Sebastian Moeller]

Hi Kevin,

On Nov 3, 2015, at 11:29 , Kevin Darbyshire-Bryant <kevin at darbyshire-bryant.me.uk> wrote:

> 
> 
> On 03/11/15 08:34, Sebastian Moeller wrote:
> 
>> Hi Jonathan,
>> 
>> On Nov 3, 2015, at 09:25 , Jonathan Morton <chromatix99 at gmail.com> wrote:
>> 
>>>> On 3 Nov, 2015, at 10:20, Sebastian Moeller <moeller0 at gmx.de> wrote:
>>>> 
>>>> Also what about the BUG_ON I managed to trigger during my testing?
>>> That turned out to be a flawed condition, which I’ve already fixed.
>> 
> If my git-blame-fu is working right this morning, then Dave is the one
> who with the aid of commit c4f75d38 turned "BUG_ON(CAKE_MAX_BINS <
> q->bin_cnt);" into "BUG_ON(q->bin_cnt >= CAKE_MAX_BINS);" and introduced
> a classic 'off-by-one/fencepost' error on 2015-10-05.  A reconfigure
> using 'diffserv8' should have blown up quite nicely after that date and
> suggests little testing has been done with diffserv8 since that time.
> 
> Well done Seb for clearly treading rarely visited areas of the code!

	Fair enough so the initial BUG_ON seems to have been safe.

> 
> BUG_ON I put in the category of "Never test for a condition you don't
> know how to handle" ;-)  Now that the off-by-one has been stomped on I'm
> going to stick my neck out and say that bin_cnt > 8 implies something
> has stomped on our config structure and we really shouldn't be trusting
> anything in it.  Goodness knows what our skb queues are like...and err,
> help!  Also note that the correctly written BUG_ON didn't explode for
> many months.

	Well, not my call, but I have seen discussions on LKML about this topic with the general gist that a BUG_ON needs to be justified, and only be there is the situation is not salvageable. After all we take down a whole machine that might be doing something really really important...

> 
> I'm happy with it being there.

	As long as it can never trigger I guess I do not care too much about this either. But the fact that we managed to get it wrong does not fill me with confidence (I seem to suffer from a lack of confidence lately...)

Best Regards
	Sebastian

> 
> _______________________________________________
> Cake mailing list
> Cake at lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/cake