Re: [Codel] sprout

[Dave Taht <dave.taht@gmail.com>]

On Thu, Jul 11, 2013 at 9:54 AM, Kathleen Nichols <nichols@pollere.com> wrote:
>
> Yes. I think that's a sort of "application - dependent" bit of code perhaps.
> Van and I had various discussions about this and put that in with very
> low bandwidth applications in mind, where some "maximums" might
> be much smaller than others.

Well perhaps a better number to turn off the scheduler at is a backlog
of 0, as a single packet is 13ms at 1Mbit. I'll fiddle at some really
low rates over the next  couple days.

Stopping at 1500 makes not a lot of sense as at the level codel is at
there is always underlying buffering everywhere in the linux stack I
see.

Even with htb there is always one packet outstanding outside of codel...

(note: in the nfq_codel case I'd also tried keeping a per queue
maxpacket, so as an ack-only stream would retain some probability of a
drop)

>
> On 7/11/13 9:22 AM, Eric Dumazet wrote:
>> On Thu, 2013-07-11 at 08:45 -0700, Kathleen Nichols wrote:
>>
>>> Dave, not completely sure which reservation about maxpacket is in reference.
>>
>> Hi Kathleen
>>
>> I believe Dave is referring to the fact that we update maxpacket every
>> time we dequeue a packet, and with GSO packet it makes little sense
>> because after a while maxpacket is set to ~65535, the limit of the GSO
>> packet size.
>>
>> We might remove this code, and make maxpacket a constant.
>>
>> diff --git a/include/net/codel.h b/include/net/codel.h
>> index 389cf62..470e1ff 100644
>> --- a/include/net/codel.h
>> +++ b/include/net/codel.h
>> @@ -170,7 +170,7 @@ static void codel_vars_init(struct codel_vars *vars)
>>
>>  static void codel_stats_init(struct codel_stats *stats)
>>  {
>> -       stats->maxpacket = 256;
>> +       stats->maxpacket = 1500;
>>  }
>>
>>  /*
>> @@ -221,9 +221,6 @@ static bool codel_should_drop(const struct sk_buff *skb,
>>         vars->ldelay = now - codel_get_enqueue_time(skb);
>>         sch->qstats.backlog -= qdisc_pkt_len(skb);
>>
>> -       if (unlikely(qdisc_pkt_len(skb) > stats->maxpacket))
>> -               stats->maxpacket = qdisc_pkt_len(skb);
>> -
>>         if (codel_time_before(vars->ldelay, params->target) ||
>>             sch->qstats.backlog <= stats->maxpacket) {
>>                 /* went below - stay below for at least interval */

Well you can nuke the variable entirely and the second part of the
conditional...

>>
>>
>



-- 
Dave Täht

Fixing bufferbloat with cerowrt: http://www.teklibre.com/cerowrt/subscribe.html