[Cake] Long-RTT broken again

[Cake] Long-RTT broken again

[Toke Høiland-Jørgensen]

[-- Attachment #1: Type: text/plain, Size: 874 bytes --]

Hi Jonathan

I see you reverted the in-kernel target scaling and reinstated the limit
on max queue length. Well, TCP now once again can't fill the pipe at
long RTTs when running through cake; see attached plot. And yes, the
userspace code did set target to 62.5ms here.

Since you obviously didn't like my previous fix, could you please do
something else about it? The current state is quite obviously broken.

kthxbye,

-Toke

P.S. I still think the scaling of the target should be in the kernel,
and that target should not be exposed to userspace at all.

P.P.S. If the above seems a bit blunt, it's because I'm somewhat annoyed
at you arbitrarily reverting a commit without (a) saying anything about
it and (b) adding another fix for the issue it was supposed to address.
Was going to start a long series of tests today and ended up having to
debug this instead. Grump.


[-- Attachment #2: cake-longrtt-fail.pdf --]
[-- Type: application/pdf, Size: 188560 bytes --]

Re: [Cake] Long-RTT broken again

[Sebastian Moeller]

[-- Attachment #1: Type: text/plain, Size: 2458 bytes --]

Hi Toke,

On Nov 2, 2015, at 17:53 , Toke Høiland-Jørgensen <toke@toke.dk> wrote:

> Hi Jonathan
> 
> I see you reverted the in-kernel target scaling and reinstated the limit
> on max queue length.

	Not that I have much leverage here, but a) not exposing the limit control to user space and b) not honoring as a hard maximum limit is not a road to success; unless one defines success as "causing un-enforced OOMs”.

> Well, TCP now once again can't fill the pipe at
> long RTTs when running through cake; see attached plot. And yes, the
> userspace code did set target to 62.5ms here.
> 
> Since you obviously didn't like my previous fix, could you please do
> something else about it? The current state is quite obviously broken.
> 
> kthxbye,
> 
> -Toke
> 
> P.S. I still think the scaling of the target should be in the kernel,
> and that target should not be exposed to userspace at all.

	I beg to differ; this is rather another argument for exposing target to user space, if even we, arguable one of the groups on this planet most sensitive to the nuances of codel behavior, consistently get this wrong, maybe exposing target as a mechanism around failed auto-tuning strategies has some merit. I want to add I do not buy the argument that this will disincentivate “researchers” to play with target (after all this is just a small source change further away than if exposed via tc); all it will do is make it harder for common people to opt out of any automatic process we come up with, hopefully allowing better behavior in corner cases. I also realize that I am most likely not wining this battle ;) (as long as I convince you/us to expose limit I am fine).

> 
> P.P.S. If the above seems a bit blunt, it's because I'm somewhat annoyed
> at you arbitrarily reverting a commit without (a) saying anything about
> it and (b) adding another fix for the issue it was supposed to address.
> Was going to start a long series of tests today and ended up having to
> debug this instead. Grump.

	I believe I understand your emotions, I also understand that the initial commit was not as well tested as it should have been… and if this is about voicing raw feelings, I am less than excited about the report I got for triggering a BUG_ON with simple tc operations. My cursory reading of linux-netdev tells me this will not makes us many fans over there (also not with our potential users).

Best Regards
	Sebastian

> 

[-- Attachment #2: cake-longrtt-fail.pdf --]
[-- Type: application/pdf, Size: 188560 bytes --]

[-- Attachment #3: Type: text/plain, Size: 146 bytes --]

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

Re: [Cake] Long-RTT broken again

[Jonathan Morton]

> On 2 Nov, 2015, at 20:29, Sebastian Moeller <moeller0@gmx.de> wrote:
> 
> the initial commit was not as well tested as it should have been…

Indeed, and the obvious problems with it were why I reverted it.  I was able to put in a partial implementation by other means at the same time, but haven’t yet had time to polish off the rough edges.

The question remains why a 15MB buffer (which comfortably exceeds the traditional FIFO rule of thumb for 1 second * 100Mbps) is apparently insufficient according to Toke’s tests, even with the target increased as requested.

 - Jonathan Morton

Re: [Cake] Long-RTT broken again

[Sebastian Moeller]

Hi Jonathan,

On Nov 3, 2015, at 02:39 , Jonathan Morton <chromatix99@gmail.com> wrote:

> 
>> On 2 Nov, 2015, at 20:29, Sebastian Moeller <moeller0@gmx.de> wrote:
>> 
>> the initial commit was not as well tested as it should have been…
> 
> Indeed, and the obvious problems with it were why I reverted it.  I was able to put in a partial implementation by other means at the same time, but haven’t yet had time to polish off the rough edges.

	Well, I believe one of the next steps needs to be to expose limit to user-space, which would have Toke allowed his measurements and would have followed the example of most/all other leaf qdiscs and put policy into user space where it arguably belongs… if no one beats me to it I might go and try implement this, but behold it is not going to be pretty...

> 
> The question remains why a 15MB buffer (which comfortably exceeds the traditional FIFO rule of thumb for 1 second * 100Mbps) is apparently insufficient according to Toke’s tests, even with the target increased as requested.

	That also is a valid question that should be answered, but it would be great if cake had just enough knobs to allow working around this issue, to sort of compartmentalize different problem spheres and allow concurrent progress on different issues, no?

	Also what about the BUG_ON I managed to trigger during my testing? Could we just change this to a WARN_ON and “gracefully” fail the initialization of the qdisc with an appropriate message to the user? I am not sure that cake’s unhappiness should wedge a whole system...


Best Regards
	Sebastian

> 
> - Jonathan Morton
> 

Re: [Cake] Long-RTT broken again

[Jonathan Morton]

> On 3 Nov, 2015, at 10:20, Sebastian Moeller <moeller0@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.  It’s necessary for something like that to be there, because there is a fixed, static allocation of tins; exceeding that number would result in a buffer overflow.  The only way to do so is to add a new Diffserv scheme with more than eight tins, which isn’t outside the bounds of possibility in the future, though unlikely in the short term.

 - Jonathan Morton

Re: [Cake] Long-RTT broken again

[Sebastian Moeller]

Hi Jonathan,

On Nov 3, 2015, at 09:25 , Jonathan Morton <chromatix99@gmail.com> wrote:

> 
>> On 3 Nov, 2015, at 10:20, Sebastian Moeller <moeller0@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.

	Excellent, thanks a lot. I would not have repeatedly brought this up, if I had known it fixed, Call me weird, but I am not too keen to look at the source to see whether “reported” issues are fixed. I also note it still is a BUG_ON ()actually there are two in sch_cake) and that still has the issue whether cake is important enough to hose a computer on failure… My understanding from luring on LKML is that under-justified BUG_ONs are frowned upon… Why is that an issue? In my case I needed physical access to reboot the machine, which for remote servers gets difficult...

>  It’s necessary for something like that to be there, because there is a fixed, static allocation of tins; exceeding that number would result in a buffer overflow.  The only way to do so is to add a new Diffserv scheme with more than eight tins, which isn’t outside the bounds of possibility in the future, though unlikely in the short term.

	Sure, but BUG_ON is the big gun that will most likely make the machine unusable (at least it wedged my opensuse 13.2 machine hard enough that a reboot was required); which I believe is too drastic for a failed qdisc, but I could be wrong…

Best Regards
	Sebastian

> 
> - Jonathan Morton
> 

Re: [Cake] Long-RTT broken again

[Kevin Darbyshire-Bryant]

[-- Attachment #1: Type: text/plain, Size: 1361 bytes --]



On 03/11/15 08:34, Sebastian Moeller wrote:

> Hi Jonathan,
>
> On Nov 3, 2015, at 09:25 , Jonathan Morton <chromatix99@gmail.com> wrote:
>
>>> On 3 Nov, 2015, at 10:20, Sebastian Moeller <moeller0@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!

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.

I'm happy with it being there.


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4816 bytes --]

Re: [Cake] Long-RTT broken again

[Sebastian Moeller]

Hi Kevin,

On Nov 3, 2015, at 11:29 , Kevin Darbyshire-Bryant <kevin@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@gmail.com> wrote:
>> 
>>>> On 3 Nov, 2015, at 10:20, Sebastian Moeller <moeller0@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@lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/cake

Re: [Cake] Long-RTT broken again

[Toke Høiland-Jørgensen]

Sebastian Moeller <moeller0@gmx.de> writes:

>> 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...

The right thing to do would be to reject the netlink request and not
change anything.

-Toke

Re: [Cake] Long-RTT broken again

[Toke Høiland-Jørgensen]

Sebastian Moeller <moeller0@gmx.de> writes:

> Well, I believe one of the next steps needs to be to expose limit to
> user-space,

No, just throwing the problem to user space is not a solution: it's a
cop-out. We need to fix the issues so things actually work, and then
*maybe* expose the value to userspace if there is a real need for it.
Not just go "meh, let userspace sort it out"; that is horrible design.

> which would have Toke allowed his measurements

No, it would not: I'm not trying to test whether we have a qdisc that,
through arcane configuration options, can be made to behave properly.
That already exists in fq_codel+HTB. What we're doing here is trying to
build a no-knobs qdisc here that works well in all the scenarios we can
think of. So let's make sure it does, and then talk about whether a
variable should be exposed *after* we've done proper auto-tuning.

> and would have followed the example of most/all other leaf qdiscs and
> put policy into user space where it arguably belongs…

Packet limit is not policy, it's an implementation detail. If you don't
have the memory to run at 100Mbps / 1 second, then *set those values
lower*. You're not going to achieve it anyway if you don't have the
buffer space.

Same thing with the target parameter, BTW: The fact that we still
haven't got it right is not an argument for exposing it to userspace,
quite the contrary: If we, the experts, can't even get it right, why on
earth would be expect users to?

-Toke

Re: [Cake] Long-RTT broken again

[Sebastian Moeller]

Hi Toke,

On Nov 3, 2015, at 12:57 , Toke Høiland-Jørgensen <toke@toke.dk> wrote:

> Sebastian Moeller <moeller0@gmx.de> writes:
> 
>> Well, I believe one of the next steps needs to be to expose limit to
>> user-space,
> 
> No, just throwing the problem to user space is not a solution: it's a
> cop-out. We need to fix the issues so things actually work, and then
> *maybe* expose the value to userspace if there is a real need for it.
> Not just go "meh, let userspace sort it out"; that is horrible design.

	I guess all we can do is to agree to disagree, the amount of memory a user is willing to dedicate to a specific functionality is a policy question in my book. It is not a cop out, as the amount of memory the user might require for other perhaps more important functionality can not be deduced without the users input, short of a perfect oracle. 

> 
>> which would have Toke allowed his measurements
> 
> No, it would not: I'm not trying to test whether we have a qdisc that,
> through arcane configuration options, can be made to behave properly.

	Sorry, I misunderstood then, I thought the experiment was about target at long intervals/rtts, I am genuinely sorry.

> That already exists in fq_codel+HTB. What we're doing here is trying to
> build a no-knobs qdisc here that works well in all the scenarios we can
> think of. So let's make sure it does, and then talk about whether a
> variable should be exposed *after* we've done proper auto-tuning.

	I am a biologist by training and profession, in my world there is no perfect auto-tuning, there is just better or worse adaptation to the current environment, the optimality of which changes with changes in said environments that at least partly are driven by the attempt of the environments inhabitants to improve their well being. So in other words I believe "good-enough” is a reasonably achievable optimization goal, while perfect is not; and one implication of that is that I would appreciate to be able to do away with automatic optimization if I feel that it gets in the way. But I realize that this is a very personal opinion not shared by others. Given that most of your have a much stronger CS background, I will not be too sad if I can not convince all of you; I would be unhappy with myself if I did not at least try, though. It is after all not the first time this isse came up, I seem to recall that we had this limit discussion already during the codel or fq_codel development.

> 
>> and would have followed the example of most/all other leaf qdiscs and
>> put policy into user space where it arguably belongs…
> 
> Packet limit is not policy, it's an implementation detail. If you don't
> have the memory to run at 100Mbps / 1 second, then *set those values
> lower*. You're not going to achieve it anyway if you don't have the
> buffer space.

	So much for no-knob… Now the user on such a high-bandwidth high delay link will need to actively “lie" to cake about the link specific parameters to avoid giving remote parties the possibility to easily OOM his/her router, this looks a bit like DDOS on steroids to me. This is not improved by the fact that the increased worst-case memory demand is a (so far un documented) side effect of setting unrelated parameters that describe parameters a user might know a priori about her/his link. Seriously, is this as robust as we can make it? Also we do not even report the worst case memory consumption (or a number that strongly correlates with it) so this is not as user-friendly and obvious as it should be.

> 
> Same thing with the target parameter, BTW: The fact that we still
> haven't got it right is not an argument for exposing it to userspace,
> quite the contrary: If we, the experts, can't even get it right, why on
> earth would be expect users to?

	Because the fact that even we are struggling might indicate that there is no real one-size-fits-all value for target? Now, I agree that the issues we had are not really big conceptual ones but rather small implementation issues, but still.. Anyway, exposing limit is the white whale I am chasing, I am fine with target somewhere between 5% to 10% of interval, once we figured out whether at low bandwidths target is supposed to grow to a larger fraction or not, that is...

Best Regards
	Sebastian

> 
> -Toke

Re: [Cake] Long-RTT broken again

[Toke Høiland-Jørgensen]

Jonathan Morton <chromatix99@gmail.com> writes:

>> On 2 Nov, 2015, at 20:29, Sebastian Moeller <moeller0@gmx.de> wrote:
>> 
>> the initial commit was not as well tested as it should have been…
>
> Indeed, and the obvious problems with it were why I reverted it.  I
> was able to put in a partial implementation by other means at the same
> time, but haven’t yet had time to polish off the rough edges.

Well I would have preferred that you kept the part that fixed the (real,
demonstrated) bug, and then went back and fixed the (theoretical) memory
limit issues later; which I was well aware were there.

And BTW, I'm not entirely convinced that this is anything but a
theoretical problem; I challenge anyone to actually demonstrate a router
crashing from OOM from this. :)

> The question remains why a 15MB buffer (which comfortably exceeds the
> traditional FIFO rule of thumb for 1 second * 100Mbps) is apparently
> insufficient according to Toke’s tests, even with the target increased
> as requested.

Because it's not a 15MB buffer; it's a 10240 packet buffer. So anything
from ~.5 to ~15MB. In this case, it's a bidirectional test, so about
half the packets will be tiny ACKs. I guess doing byte accounting would
actually be better here, regardless of what happens to the overall limit... :)

-Toke

Re: [Cake] Long-RTT broken again

[Jonathan Morton]

> On 3 Nov, 2015, at 13:50, Toke Høiland-Jørgensen <toke@toke.dk> wrote:
> 
>> The question remains why a 15MB buffer (which comfortably exceeds the
>> traditional FIFO rule of thumb for 1 second * 100Mbps) is apparently
>> insufficient according to Toke’s tests, even with the target increased
>> as requested.
> 
> Because it's not a 15MB buffer; it's a 10240 packet buffer. So anything
> from ~.5 to ~15MB. In this case, it's a bidirectional test, so about
> half the packets will be tiny ACKs. I guess doing byte accounting would
> actually be better here, regardless of what happens to the overall limit... :)

Cake does the queue accounting in bytes, and calculates 15MB (by default) as the upper limit.  It’s *not* meant to be a packet buffer.

However, the bytes counted are those allocated, not the on-wire packet sizes, because this limit is meant to avoid consuming all of a small router’s RAM for one queue.  This isn’t just about hard OOM, which the kernel probably has handling for already, but sharing RAM between different queues on the same device; note the different behaviour of the upload and download streams in the results given.

The only way this could behave like a “packet buffer” instead of a byte-accounted queue is if there is a fixed size allocation per packet, regardless of the size of said packet.  There are hints that this might actually be the case, and that the allocation is a hugely wasteful (for an ack) 2KB.  (This also means that it’s not a 10240 packet buffer, but about 7500.)

But in a bidirectional TCP scenario with ECN, only about a third of the packets should be acks (ignoring the relatively low number of ICMP and UDP probes); ECN causes an ack to be sent immediately, but then normal delayed-ack processing should resume.  This makes 6KB allocated per ~3KB transmitted.  The effective buffer size is thus 7.5MB, which is still compliant with the traditional rule of thumb (BDP / sqrt(flows)), given that there are four bulk flows each way.

This effect is therefore not enough to explain the huge deficit Toke measured.  The calculus also changes by only a small factor if we ignore delayed acks, making 8KB allocated per 3KB transmitted.

So, again - what’s going on?  Are there any clues in packet traces with sequence analysis?

I’ll put in a configurable memory limit anyway, but I really do want to understand why this is happening.

 - Jonathan Morton

Re: [Cake] Long-RTT broken again

[Toke Høiland-Jørgensen]

Jonathan Morton <chromatix99@gmail.com> writes:

> Cake does the queue accounting in bytes, and calculates 15MB (by
> default) as the upper limit.  It’s *not* meant to be a packet buffer.

Ah, good.

> note the different behaviour of the upload and download streams in the
> results given.

This is not a result of ingress/egress shaping, though; the upstream and
downstream shaping is done on each side of the bottleneck on separate
boxes.

> The only way this could behave like a “packet buffer” instead of a
> byte-accounted queue is if there is a fixed size allocation per
> packet, regardless of the size of said packet. There are hints that
> this might actually be the case, and that the allocation is a hugely
> wasteful (for an ack) 2KB. (This also means that it’s not a 10240
> packet buffer, but about 7500.)

Right, well, in that case fixing the calculation to use the actual
packet size would make sense in any case?

> But in a bidirectional TCP scenario with ECN, only about a third of
> the packets should be acks (ignoring the relatively low number of ICMP
> and UDP probes); ECN causes an ack to be sent immediately, but then
> normal delayed-ack processing should resume. This makes 6KB allocated
> per ~3KB transmitted. The effective buffer size is thus 7.5MB, which
> is still compliant with the traditional rule of thumb (BDP /
> sqrt(flows)), given that there are four bulk flows each way.
>
> This effect is therefore not enough to explain the huge deficit Toke
> measured. The calculus also changes by only a small factor if we
> ignore delayed acks, making 8KB allocated per 3KB transmitted.

Well, there are also several UDP measurement flows and a ping, which all
send really small packets; so it's not just the acks.

> So, again - what’s going on?  Are there any clues in packet traces
> with sequence analysis?

Not really; the qdisc stats show ~6000 packets dropped over the 300
second test; and lots and lots of overlimits. So my guess is that the
queue is in fact overflowing.

Will post a trace when I get a chance tomorrow.

> I’ll put in a configurable memory limit anyway, but I really do want
> to understand why this is happening.

As I said before: a configurable limit is not a fix for this; we need
the default behaviour to be sane.

-Toke

Re: [Cake] Long-RTT broken again

[Sebastian Moeller]

Hi Toke,

On Nov 3, 2015, at 18:05 , Toke Høiland-Jørgensen <toke@toke.dk> wrote:

> Jonathan Morton <chromatix99@gmail.com> writes:
> 
>> Cake does the queue accounting in bytes, and calculates 15MB (by
>> default) as the upper limit.  It’s *not* meant to be a packet buffer.
> 
> Ah, good.
> 
>> note the different behaviour of the upload and download streams in the
>> results given.
> 
> This is not a result of ingress/egress shaping, though; the upstream and
> downstream shaping is done on each side of the bottleneck on separate
> boxes.
> 
>> The only way this could behave like a “packet buffer” instead of a
>> byte-accounted queue is if there is a fixed size allocation per
>> packet, regardless of the size of said packet. There are hints that
>> this might actually be the case, and that the allocation is a hugely
>> wasteful (for an ack) 2KB. (This also means that it’s not a 10240
>> packet buffer, but about 7500.)
> 
> Right, well, in that case fixing the calculation to use the actual
> packet size would make sense in any case?

	Would it? I thought actually using the amount of “pinned” kernel memory would be more relevant, it a ACK packet pins 2KB then it should be accounted at 2KB, IF the goal of the accounting is to avoid un-scheduled OOM, no? And if something like Dave’s patch kicks in that copies larger mostly empty skbs to smaller sizes, these packets then should be accounted with that smaller size. In anyway I believe with default kernels there is a strong correlation between a packet count limit and a byte count limit…

> 
>> But in a bidirectional TCP scenario with ECN, only about a third of
>> the packets should be acks (ignoring the relatively low number of ICMP
>> and UDP probes); ECN causes an ack to be sent immediately, but then
>> normal delayed-ack processing should resume. This makes 6KB allocated
>> per ~3KB transmitted. The effective buffer size is thus 7.5MB, which
>> is still compliant with the traditional rule of thumb (BDP /
>> sqrt(flows)), given that there are four bulk flows each way.
>> 
>> This effect is therefore not enough to explain the huge deficit Toke
>> measured. The calculus also changes by only a small factor if we
>> ignore delayed acks, making 8KB allocated per 3KB transmitted.
> 
> Well, there are also several UDP measurement flows and a ping, which all
> send really small packets; so it's not just the acks.
> 
>> So, again - what’s going on?  Are there any clues in packet traces
>> with sequence analysis?
> 
> Not really; the qdisc stats show ~6000 packets dropped over the 300
> second test; and lots and lots of overlimits. So my guess is that the
> queue is in fact overflowing.
> 
> Will post a trace when I get a chance tomorrow.
> 
>> I’ll put in a configurable memory limit anyway, but I really do want
>> to understand why this is happening.
> 
> As I said before: a configurable limit is not a fix for this; we need
> the default behaviour to be sane.

	As much as I push for a configurable limit, I fully agree that cake’s auto-tuning smarts should actually be smart and do the right thing.

Best Regards
	Sebastian

> 
> -Toke

Re: [Cake] Long-RTT broken again

[Toke Høiland-Jørgensen]

Sebastian Moeller <moeller0@gmx.de> writes:

>> Right, well, in that case fixing the calculation to use the actual
>> packet size would make sense in any case?
>
> Would it? I thought actually using the amount of “pinned” kernel
> memory would be more relevant, it a ACK packet pins 2KB then it should
> be accounted at 2KB, IF the goal of the accounting is to avoid
> un-scheduled OOM, no? And if something like Dave’s patch kicks in that
> copies larger mostly empty skbs to smaller sizes, these packets then
> should be accounted with that smaller size. In anyway I believe with
> default kernels there is a strong correlation between a packet count
> limit and a byte count limit…

No, a limit on a qdisc is in terms of the packets we are transmitted. We
can't expect the user to know how much memory the kernel actually
allocates to an skb in order to configure their package queue. If
someone configures a limit, they are going to do a BDP calculation,
input that, and complain when that doesn't work.

FWIW, this is what the in-kernel FIFO qdisc does when configured in byte
mode:

		if (is_bfifo)
			limit *= psched_mtu(qdisc_dev(sch));
on init

and

	if (likely(sch->qstats.backlog + qdisc_pkt_len(skb) <= sch->limit))
		return qdisc_enqueue_tail(skb, sch);

on enqueue.

Which is the same data that Cake uses. Hmm, weird...

-Toke

Re: [Cake] Long-RTT broken again

[Jonathan Morton]

> On 3 Nov, 2015, at 19:25, Toke Høiland-Jørgensen <toke@toke.dk> wrote:
> 
> 	if (likely(sch->qstats.backlog + qdisc_pkt_len(skb) <= sch->limit))
> 		return qdisc_enqueue_tail(skb, sch);
> 
> on enqueue.
> 
> Which is the same data that Cake uses. Hmm, weird…

No, Cake uses skb->truesize for this particular purpose.  It uses qdisc_pkt_len(skb) only for timing purposes.

 - Jonathan Morton

Re: [Cake] Long-RTT broken again

[Toke Høiland-Jørgensen]

Jonathan Morton <chromatix99@gmail.com> writes:

>> Which is the same data that Cake uses. Hmm, weird…
>
> No, Cake uses skb->truesize for this particular purpose.  It uses
> qdisc_pkt_len(skb) only for timing purposes.

Ah, right. Well, I would consider that a bug. If we're doing a "max
queue size" it should (conceptually) be in packet data units. A hard
memory limit may make sense *in addition*, but then that should be a
separate safeguard IMO.

-Toke

Re: [Cake] Long-RTT broken again

[Sebastian Moeller]

Hi Toke,

On Nov 3, 2015, at 18:33 , Toke Høiland-Jørgensen <toke@toke.dk> wrote:

> Jonathan Morton <chromatix99@gmail.com> writes:
> 
>>> Which is the same data that Cake uses. Hmm, weird…
>> 
>> No, Cake uses skb->truesize for this particular purpose.  It uses
>> qdisc_pkt_len(skb) only for timing purposes.
> 
> Ah, right. Well, I would consider that a bug. If we're doing a "max
> queue size" it should (conceptually) be in packet data units. A hard
> memory limit may make sense *in addition*, but then that should be a
> separate safeguard IMO.

	I guess I have constantly been talking about a different limit than you, sorry. I was and still am advocating for the hard memory limit. But I maintain that as the kernel typically uses 2KB skis per packet (independent of packet size, as long as MTU = 1500 or so) the packet limit will also do double duty as memory limit. Then again the max queue size in packets is more an implementation thing in my mind ;) I believe the only people changing limit will not be people thinking about how many packets they want queued maximally, but rather those that want to avoid OOM; from that view a memory limit seems quite natural. So maybe we need a min(max_packets, consumed_queue_memory) as true limit? Potentially with a notification if there is a large effective difference between the two?

Best Regards
	Sebastian

> 
> -Toke

Re: [Cake] Long-RTT broken again

[Toke Høiland-Jørgensen]

Sebastian Moeller <moeller0@gmx.de> writes:

> I guess I have constantly been talking about a different limit than you,
> sorry. I was and still am advocating for the hard memory limit.

Well, if we add a 'limit' parameter that limits memory consumption
rather than queue space measured in packet size, then it will function
different than the same parameter of all other qdiscs. Hardly a good
idea, I'd say?

-Toke

Re: [Cake] Long-RTT broken again

[Sebastian Moeller]

Hi Toke,

On Nov 3, 2015, at 18:49 , Toke Høiland-Jørgensen <toke@toke.dk> wrote:

> Sebastian Moeller <moeller0@gmx.de> writes:
> 
>> I guess I have constantly been talking about a different limit than you,
>> sorry. I was and still am advocating for the hard memory limit.
> 
> Well, if we add a 'limit' parameter that limits memory consumption
> rather than queue space measured in packet size, then it will function
> different than the same parameter of all other qdiscs. Hardly a good
> idea, I'd say?

	Except, both are strongly correlated in practise, by virtue of each packet pinning a 2KB skb, I do not care too much if there is a constant factor required to get from packet limit to memory or not. What I really want is that halving the limit halves the memory demand ;). 

Best Regards
	Sebastian

> 
> -Toke

Re: [Cake] Long-RTT broken again

[Toke Høiland-Jørgensen]

Sebastian Moeller <moeller0@gmx.de> writes:

> Except, both are strongly correlated in practise, by virtue of each
> packet pinning a 2KB skb, I do not care too much if there is a
> constant factor required to get from packet limit to memory or not.
> What I really want is that halving the limit halves the memory demand
> ;).

Well, all other things being equal it would either way... :)

-Toke

Re: [Cake] Long-RTT broken again

[Sebastian Moeller]

Hi Toke,

On Nov 3, 2015, at 18:54 , Toke Høiland-Jørgensen <toke@toke.dk> wrote:

> Sebastian Moeller <moeller0@gmx.de> writes:
> 
>> Except, both are strongly correlated in practise, by virtue of each
>> packet pinning a 2KB skb, I do not care too much if there is a
>> constant factor required to get from packet limit to memory or not.
>> What I really want is that halving the limit halves the memory demand
>> ;).
> 
> Well, all other things being equal it would either way... :)

	Touche, all I want is some control over the required memory then ;) I would also be really impressed if cake could report the amount of currently actually pinned down kernel memory… But incase of a byte limit I think using sib->true size is the right thing...

Best Regards
	Sebastian

> 
> -Toke

Re: [Cake] Long-RTT broken again

[Toke Høiland-Jørgensen]

Sebastian Moeller <moeller0@gmx.de> writes:

> But incase of a byte limit I think using sib->true size is the right
> thing...

No, the opposite: The byte limit expresses the queue size measured in
actual packet size, not consumed memory; aren't we going somewhat in
circles here? :P

-Toke

Re: [Cake] Long-RTT broken again

[Sebastian Moeller]

On Nov 3, 2015, at 18:59 , Toke Høiland-Jørgensen <toke@toke.dk> wrote:

> Sebastian Moeller <moeller0@gmx.de> writes:
> 
>> But incase of a byte limit I think using sib->true size is the right
>> thing...
> 
> No, the opposite: The byte limit expresses the queue size measured in
> actual packet size, not consumed memory; aren't we going somewhat in
> circles here? :P

	Could be, I am easily distracted ;) . I think I already agreed that for people wanting to control BDP actual packet size matters more, but forgot about that when thinking how clever the usage of skb-> truesize is. So the rephrase, skb->truesize is great if one wants to control the routers memory consumption, which is the angle I have been mainly arguing from. It seems all three ways of limiting the queue have some merit, with n_packets and consumed_memory being sort of equivalent, and byte_limit being the odd one out…

Best Regards
	Sebastian

> 
> -Toke

Re: [Cake] Long-RTT broken again

[Jonathan Morton]

[-- Attachment #1: Type: text/plain, Size: 686 bytes --]

Conceptually, I think limiting actual memory usage is the right thing to
do.  Ideally, the allocations would turn out closer to the actual packet
sizes, but by watching the allocated size, we will automagically take
advantage of improvements in this area - which must happen elsewhere in the
kernel.

Limiting total on-wire packet bytes is nice from a theoretical point of
view, but we don't live in a theoretical world.  The performance
discrepancy shouldn't be as big as it is.

Limiting packet count is wrong and awful and we shouldn't do that, even if
everyone else does.  The fact that counting allocated bytes behaves the
same way at the moment is unfortunate.

- Jonathan Morton

[-- Attachment #2: Type: text/html, Size: 777 bytes --]

Re: [Cake] Long-RTT broken again

[Sebastian Moeller]

Hi Jonathan,

On Nov 3, 2015, at 20:17 , Jonathan Morton <chromatix99@gmail.com> wrote:

> Conceptually, I think limiting actual memory usage is the right thing to do.  

	I agree ;) but I am coming at this from the avoid OOM direction anyway; I believe Toke’s point for people coming from the BDP angle is equally valid (if less important to me personally).

> Ideally, the allocations would turn out closer to the actual packet sizes, but by watching the allocated size, we will automagically take advantage of improvements in this area - which must happen elsewhere in the kernel.

	But this is acknowledging there is a discrepancy and the punting, turning it into someone else’s problem, no?

> 
> Limiting total on-wire packet bytes is nice from a theoretical point of view, but we don't live in a theoretical world.  The performance discrepancy shouldn't be as big as it is.

	But this is about honoring BDP calculations for worst case buffering scenarios where the sender dumps all packets for a whole transfer period in one instantaneous burst (well almost); this seems well documented in the literature and hence might be something users might actually want to do...

> 
> Limiting packet count is wrong and awful and we shouldn't do that, even if everyone else does.  The fact that counting allocated bytes behaves the same way at the moment is unfortunate.

	Why? The fact that they are correlated also allows to limit by allocated size, since the correlation is directionally insensitive, so to speak, we can always argue that the packet fans just need a proportionality constant and be done with ;). I guess this needs a clear name than simply limit to avoid confusing people who already know other qdiscs and their behavior.

Best Regards
	Sebastian

> 
> - Jonathan Morton

Re: [Cake] Long-RTT broken again

[Toke Høiland-Jørgensen]

Jonathan Morton <chromatix99@gmail.com> writes:

> So, again - what’s going on?  Are there any clues in packet traces
> with sequence analysis?

So, reran and took traces. Data and packet dumps at:
https://kau.toke.dk/experiments/cake/longrtt/

There's two runs: the first one is what's in git now, the second one
patches the byte accounting to use the packet length rather than the
truesize, which helps a bit but is still inadequate.

The packet dumps and cake stats output agree: cake is running out of
buffer size. ECN is enabled on this test, and there's not a single mark
performed, but several thousand drops.

So something is definitely wrong with the buffer sizing.

-Toke

Re: [Cake] Long-RTT broken again

[Jonathan Morton]

> On 5 Nov, 2015, at 16:36, Toke Høiland-Jørgensen <toke@toke.dk> wrote:
> 
> Jonathan Morton <chromatix99@gmail.com> writes:
> 
>> So, again - what’s going on?  Are there any clues in packet traces
>> with sequence analysis?
> 
> So, reran and took traces. Data and packet dumps at:
> https://kau.toke.dk/experiments/cake/longrtt/
> 
> There's two runs: the first one is what's in git now, the second one
> patches the byte accounting to use the packet length rather than the
> truesize, which helps a bit but is still inadequate.
> 
> The packet dumps and cake stats output agree: cake is running out of
> buffer size. ECN is enabled on this test, and there's not a single mark
> performed, but several thousand drops.
> 
> So something is definitely wrong with the buffer sizing.

Firstly: I can’t read the flent output with the version of flent currently available to pip.  It complains about “version 3” files being unsupported as yet.

Running the captures through tcptrace shows that the traffic on each flow is very bursty, and that the bursts are roughly synchronised, leading to synchronised drop bursts.  It reads like a massive advertisement for TCP Pacing, and this time cake’s shaper isn’t enough to emulate it.  (Perhaps a bigger buffer would sort that out.)

Taking one such synchronised drop burst as representative, the sum of outstanding data on all four flows in that direction is less than 6MB at that moment.  This is on the version which I think you’ve patched, and I assume the figure also includes data “in flight” in the delay line, not just in cake’s buffer.  It’s difficult to reconcile that with the nominal 15MB limit.  The absolute peak within-flow induced delay is 150ms, which corresponds to 1.5MB, not 15MB - that’s suspicious.

I also get the impression that CUBIC’s RTT-insensitive behaviour isn’t helping matters.  I’d be interested to see how more traditional algorithms (Reno and Westwood+ in particular) cope with this situation.

I’ll try to reproduce the problem locally, then see what I can do to fix it.

 - Jonathan Morton

Re: [Cake] Long-RTT broken again

[Toke Høiland-Jørgensen]

Jonathan Morton <chromatix99@gmail.com> writes:

> Firstly: I can’t read the flent output with the version of flent
> currently available to pip.  It complains about “version 3” files
> being unsupported as yet.

Ah, right, sorry about that. Will do a new release this afternoon :)

> Running the captures through tcptrace shows that the traffic on each
> flow is very bursty, and that the bursts are roughly synchronised,
> leading to synchronised drop bursts. It reads like a massive
> advertisement for TCP Pacing, and this time cake’s shaper isn’t enough
> to emulate it. (Perhaps a bigger buffer would sort that out.)

Yup, that sounds like a reasonable explanation (and solution, given that
we've seen that a bigger buffer helps).

> Taking one such synchronised drop burst as representative, the sum of
> outstanding data on all four flows in that direction is less than 6MB at that
> moment. This is on the version which I think you’ve patched, and I assume the
> figure also includes data “in flight” in the delay line, not just in cake’s
> buffer. It’s difficult to reconcile that with the nominal 15MB limit. The
> absolute peak within-flow induced delay is 150ms, which corresponds to 1.5MB,
> not 15MB - that’s suspicious.
>
> I also get the impression that CUBIC’s RTT-insensitive behaviour isn’t
> helping matters. I’d be interested to see how more traditional
> algorithms (Reno and Westwood+ in particular) cope with this
> situation.

Can re-run with Reno.

> I’ll try to reproduce the problem locally, then see what I can do to
> fix it.

Awesome! Let me know if you need more info on the setup.

-Toke

Re: [Cake] Long-RTT broken again

[Toke Høiland-Jørgensen]

Toke Høiland-Jørgensen <toke@toke.dk> writes:

>> I also get the impression that CUBIC’s RTT-insensitive behaviour isn’t
>> helping matters. I’d be interested to see how more traditional
>> algorithms (Reno and Westwood+ in particular) cope with this
>> situation.
>
> Can re-run with Reno.

Added a run with Reno to the same directory. Smoother, but worse overall
throughput.

Will go do that Flent release now so you can actually load the files :)

-Toke

Re: [Cake] Long-RTT broken again

[Toke Høiland-Jørgensen]

Toke Høiland-Jørgensen <toke@toke.dk> writes:

> Will go do that Flent release now so you can actually load the files :)

Right, released Flent v0.13.0; try that!

-Toke

Re: [Cake] Long-RTT broken again

[Jonathan Morton]

> On 6 Nov, 2015, at 16:15, Toke Høiland-Jørgensen <toke@toke.dk> wrote:
> 
> Added a run with Reno to the same directory. Smoother, but worse overall
> throughput.

Yes, looks like it’s ramping up the slow way, and hasn’t reached the BDP by the time the test run ended.

However, it also looks like it’s properly ack-clocked, unlike CUBIC, so while it’s still bursty, it isn’t overflowing the buffer except during slow-start.

The fact that slow-start terminates (due to loss) after only a few RTTs is suspicious.  What parameters are you giving to netem?  I note that the default packet limit in netem is 1000, which is 1.5MB...

 - Jonathan Morton

Re: [Cake] Long-RTT broken again

[Dave Taht]

I note that truesize is a bad indicator for tests on the rx path where
acks are usually 2k.
Dave Täht
Let's go make home routers and wifi faster! With better software!
https://www.gofundme.com/savewifi


On Sat, Nov 7, 2015 at 12:02 AM, Jonathan Morton <chromatix99@gmail.com> wrote:
>
>> On 6 Nov, 2015, at 16:15, Toke Høiland-Jørgensen <toke@toke.dk> wrote:
>>
>> Added a run with Reno to the same directory. Smoother, but worse overall
>> throughput.
>
> Yes, looks like it’s ramping up the slow way, and hasn’t reached the BDP by the time the test run ended.
>
> However, it also looks like it’s properly ack-clocked, unlike CUBIC, so while it’s still bursty, it isn’t overflowing the buffer except during slow-start.
>
> The fact that slow-start terminates (due to loss) after only a few RTTs is suspicious.  What parameters are you giving to netem?  I note that the default packet limit in netem is 1000, which is 1.5MB...
>
>  - Jonathan Morton
>
> _______________________________________________
> Cake mailing list
> Cake@lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/cake

Re: [Cake] Long-RTT broken again

[Jonathan Morton]

> On 7 Nov, 2015, at 07:16, Dave Taht <dave.taht@gmail.com> wrote:
> 
> I note that truesize is a bad indicator for tests on the rx path where
> acks are usually 2k.

At least in my test setup, cake is on the egress side, so I would hope that doesn’t come into it.  I’m putting netem on the ingress side and giving it a limit of 102400 packets, which hopefully is never hit:

	cake bandwidth 100Mbit satellite -> GigE -> netem delay 500ms limit 102400
	netem delay 500ms limit 102400   <- GigE <- cake bandwidth 100Mbit satellite

With that setup, I’m seeing very poor throughput with CUBIC (less than 10Mbit) but much better with Westwood+ (about 50Mbit) - of course that’s the opposite of the theoretical result.  Next I’ll try increasing cake’s memory limit.  Both machines are desktop-class (with 1GB and 16Gb RAM), so that won’t be a problem for experimenting.

 - Jonathan Morton

Re: [Cake] Long-RTT broken again

[Toke Høiland-Jørgensen]

Jonathan Morton <chromatix99@gmail.com> writes:

>> On 6 Nov, 2015, at 16:15, Toke Høiland-Jørgensen <toke@toke.dk> wrote:
>> 
>> Added a run with Reno to the same directory. Smoother, but worse overall
>> throughput.
>
> Yes, looks like it’s ramping up the slow way, and hasn’t reached the
> BDP by the time the test run ended.
>
> However, it also looks like it’s properly ack-clocked, unlike CUBIC,
> so while it’s still bursty, it isn’t overflowing the buffer except
> during slow-start.

Hmm, right. Can run a longer test to see if it ever reaches 100Mbit; and
also add in a BIFO queue for comparison.

> The fact that slow-start terminates (due to loss) after only a few
> RTTs is suspicious.  What parameters are you giving to netem?  I note
> that the default packet limit in netem is 1000, which is 1.5MB...

Not using netem - don't trust it. The delay is added by dummynet which
is on a separate box in the middle. And dummynet basically has an
infinite buffer; no packets are dropped there (yes, I checked) :)

-Toke

Re: [Cake] Long-RTT broken again

[Jonathan Morton]

Okay, I’ve tracked down the problem.  Turns out to be one of C’s more subtle misfeatures.

I calculate the desired buffer size as rate * interval * 4.  Since interval is in µs, rate * interval is a 64-bit temporary.  However, I forgot to cast either of these operands to the new width first, so the upper half of the product was never calculated.  After dividing by 250000, the resulting buffer size was 8918 bytes - which was clamped up to 64KB by a sanity check immediately afterwards.

It’s thus remarkable that we got as much throughput as we did.

While fixing this, I’m also adding configurability of the buffer limit and statistics of how much of the buffer was actually ever used.  Additionally, I’m choosing a saner default for “unlimited” mode.

 - Jonathan Morton

Re: [Cake] Long-RTT broken again

[Jonathan Morton]

> On 7 Nov, 2015, at 12:51, Jonathan Morton <chromatix99@gmail.com> wrote:
> 
> While fixing this, I’m also adding configurability of the buffer limit and statistics of how much of the buffer was actually ever used.  Additionally, I’m choosing a saner default for “unlimited” mode.

Okay, that seems to be working somewhat better now.  I’m still not getting 100Mbps throughput in either direction, but now it seems to be because the receive windows won’t open far enough - there’s enough buffering to prevent any packet loss.

 - Jonathan Morton

Re: [Cake] Long-RTT broken again

[Toke Høiland-Jørgensen]

Jonathan Morton <chromatix99@gmail.com> writes:

> Okay, that seems to be working somewhat better now.  I’m still not
> getting 100Mbps throughput in either direction, but now it seems to be
> because the receive windows won’t open far enough - there’s enough
> buffering to prevent any packet loss.

Yeah, Linux is window limited at these BDPs with the default settings.
Try these sysctls on both sides:

net.core.rmem_max=16777216
net.core.wmem_max=16777216
net.ipv4.tcp_rmem = 4096 87380 104857600
net.ipv4.tcp_wmem = 4096 16384 104857600

-Toke

Re: [Cake] Long-RTT broken again

[Toke Høiland-Jørgensen]

Jonathan Morton <chromatix99@gmail.com> writes:

> Okay, that seems to be working somewhat better now.  I’m still not
> getting 100Mbps throughput in either direction, but now it seems to be
> because the receive windows won’t open far enough - there’s enough
> buffering to prevent any packet loss.

Yup, re-ran the tests in the testbed, and cake does indeed achieve the
full throughput now. Awesome! :)

Reno still goes out of slow start too early and doesn't hit full
capacity. And for some reason the reno results are asymmetrical. I'm
guessing it's due to random timing errors, though; unless someone else
has another idea?

Test data in the same directory as before; look at the time stamps :)

-Toke

Re: [Cake] Long-RTT broken again

[Toke Høiland-Jørgensen]

Jonathan Morton <chromatix99@gmail.com> writes:

> While fixing this, I’m also adding configurability of the buffer limit
> and statistics of how much of the buffer was actually ever used.
> Additionally, I’m choosing a saner default for “unlimited” mode.

Cool, will give it a spin on the testbed.

Also, I'd suggest calling the tc parameter 'memlimit' rather than
'memory' to convey the fact that it's a limit and not the expected usage...

-Toke

Re: [Cake] Long-RTT broken again

[Sebastian Moeller]

Hi Jonathan,

pretty cool, ans subtle ;)

On Nov 7, 2015, at 11:51 , Jonathan Morton <chromatix99@gmail.com> wrote:

> Okay, I’ve tracked down the problem.  Turns out to be one of C’s more subtle misfeatures.
> 
> I calculate the desired buffer size as rate * interval * 4.  Since interval is in µs, rate * interval is a 64-bit temporary.  However, I forgot to cast either of these operands to the new width first, so the upper half of the product was never calculated.  After dividing by 250000, the resulting buffer size was 8918 bytes - which was clamped up to 64KB by a sanity check immediately afterwards.
> 
> It’s thus remarkable that we got as much throughput as we did.
> 
> While fixing this, I’m also adding configurability of the buffer limit and statistics of how much of the buffer was actually ever used.  Additionally, I’m choosing a saner default for “unlimited” mode.

	Oh, shiny ;) I take it the parameter is limiting the queue memory as measured by skb->truesize ; just checked, it wants the size in KB. I just committed a change to report that in the usage help tc will display…

Best Regards
	Sebastian

> 
> - Jonathan Morton
> 

Re: [Cake] Long-RTT broken again

[Toke Høiland-Jørgensen]

Sebastian Moeller <moeller0@gmx.de> writes:

> 	Oh, shiny ;) I take it the parameter is limiting the queue
> 	memory as measured by skb->truesize ; just checked, it wants the
> 	size in KB. I just committed a change to report that in the
> 	usage help tc will display…

Changed this to use the existing functionality in tc to parse different
units of size (kbit, kbyte, etc), and default to bytes if no units
specified. Also added the parameter to the man page.

And for good measure I did a little bit of bikeshedding renamed the
parameter to 'memlimit' as well :)

-Toke

Re: [Cake] Long-RTT broken again

[Sebastian Moeller]

Hi Toke,

On Nov 7, 2015, at 17:24 , Toke Høiland-Jørgensen <toke@toke.dk> wrote:

> Sebastian Moeller <moeller0@gmx.de> writes:
> 
>> 	Oh, shiny ;) I take it the parameter is limiting the queue
>> 	memory as measured by skb->truesize ; just checked, it wants the
>> 	size in KB. I just committed a change to report that in the
>> 	usage help tc will display…
> 
> Changed this to use the existing functionality in tc to parse different
> units of size (kbit, kbyte, etc), and default to bytes if no units
> specified.

	Most excellent, I had only known about get_rate() and thought something like it for sizes would be swell ;) but I did not know it existed already ;) I guess, the best patch is the one, one does not need to write ;)

> Also added the parameter to the man page.

	Oops, I had completely overlooked/ignored that cake has a manage, I guess maybe I can contribute a bit to that.

> 
> And for good measure I did a little bit of bikeshedding renamed the
> parameter to 'memlimit' as well :)

	Not my bike shed, I am fine with both (and given my nagging for exposing the maximal buffer sizing, I would have been happy with “charles” as the name for that option ;) )

Best Regards
	Sebastian

> 
> -Toke

Re: [Cake] Long-RTT broken again

[Kevin Darbyshire-Bryant]

[-- Attachment #1: Type: text/plain, Size: 759 bytes --]



On 07/11/15 16:24, Toke Høiland-Jørgensen wrote:
> Sebastian Moeller <moeller0@gmx.de> writes:
>
>> 	Oh, shiny ;) I take it the parameter is limiting the queue
>> 	memory as measured by skb->truesize ; just checked, it wants the
>> 	size in KB. I just committed a change to report that in the
>> 	usage help tc will display…
> Changed this to use the existing functionality in tc to parse different
> units of size (kbit, kbyte, etc), and default to bytes if no units
> specified. Also added the parameter to the man page.
>
> And for good measure I did a little bit of bikeshedding renamed the
> parameter to 'memlimit' as well :)
Great stuff Toke.  And very much great stuff Jonathan for tracking the
little swine of a bug down :-)


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4816 bytes --]

Re: [Cake] Long-RTT broken again

[Dave Taht]

Applause all around!

Dave Täht
Let's go make home routers and wifi faster! With better software!
https://www.gofundme.com/savewifi


On Sat, Nov 7, 2015 at 2:32 PM, Kevin Darbyshire-Bryant
<kevin@darbyshire-bryant.me.uk> wrote:
>
>
> On 07/11/15 16:24, Toke Høiland-Jørgensen wrote:
>> Sebastian Moeller <moeller0@gmx.de> writes:
>>
>>>      Oh, shiny ;) I take it the parameter is limiting the queue
>>>      memory as measured by skb->truesize ; just checked, it wants the
>>>      size in KB. I just committed a change to report that in the
>>>      usage help tc will display…
>> Changed this to use the existing functionality in tc to parse different
>> units of size (kbit, kbyte, etc), and default to bytes if no units
>> specified. Also added the parameter to the man page.
>>
>> And for good measure I did a little bit of bikeshedding renamed the
>> parameter to 'memlimit' as well :)
> Great stuff Toke.  And very much great stuff Jonathan for tracking the
> little swine of a bug down :-)
>
>
> _______________________________________________
> Cake mailing list
> Cake@lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/cake
>

Re: [Cake] Long-RTT broken again

[Kevin Darbyshire-Bryant]

On 08/11/2015 16:29, Dave Taht wrote:
> Applause all around!
>
> Dave Täht
> Let's go make home routers and wifi faster! With better software!
> https://www.gofundme.com/savewifi

Indeed!  I don't know when this bug first surfaced, or if it's
effectively always been there, but I can definitely report an
improvement in marks vs drops vs packet loss under load since the fix
went in...and that's even on a standard 100ms rtt link.  For those
vaguely interested, I monitor my home broadband link with
'Thinkbroadband's Broadband quality monitor.  It sends a ping packet
every second and measures the reply (or not) simple but useful monitor. 
Here's a link to my active monitor
http://www.thinkbroadband.com/ping/share/c716f7a50c999e580588859eccd2cf3a.html 
- vertical red spikes are probably me fiddling with firmware ;-)