Re: [PATCH net-next v3] Add Common Applications Kept Enhanced (cake) qdisc

["Toke Høiland-Jørgensen" <toke@toke.dk>]

Eric Dumazet <eric.dumazet@gmail.com> writes:

> On 04/25/2018 06:42 AM, Toke Høiland-Jørgensen wrote:
>> sch_cake targets the home router use case and is intended to squeeze the
>> most bandwidth and latency out of even the slowest ISP links and routers,
>> while presenting an API simple enough that even an ISP can configure it.
>> 
>
> * Support for ack filtering.
>
> Oh my god. Cake became a monster.

Haha, you either die a hero or live long enough to see yourself become
the villain, I guess? ;)

We do realise there are lots of good reasons not to do ACK filtering;
but it does make a difference on highly asymmetrical links (see
http://blog.cerowrt.org/post/ack_filtering/), which sadly some people
are still behind.

> syzkaller will be very happy to trigger all kind of bugs in it.

Heh, right.

> Lack of any pskb_may_pull() is really concerning.

By this you mean "check that the packet is long enough to contain the
header we are looking for before trying to do ACK filtering", right?

> How ack filter deals with reorders ?

I think this will do the right thing?

		/* new ack sequence must be greater
		 */
		if (thisconn &&
		    (ntohl(tcph_check->ack_seq) > ntohl(tcph->ack_seq)))
			continue;

> Also the forced GSO segmentation looks wrong to me.

Wrong as in it's done wrong, or wrong as in you object to the whole
concept?

> This kills xmit_more gain we have when GSO is performed after
> qdisc dequeue before hitting device.
>
> This should be driven by a parameter really, some threshold on the skb size.
>
> What performance number do you get on a 10Gbit NIC for example ?

Single-flow throughput through 2 hops on a 40Gbit connection (with CAKE
in unlimited mode vs pfifo_fast on the router):

MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to testbed-40g-2 () port 0 AF_INET : demo
Recv   Send    Send                          
Socket Socket  Message  Elapsed              
Size   Size    Size     Time     Throughput  
bytes  bytes   bytes    secs.    10^6bits/sec  

 87380  16384  16384    10.00    18840.40   

MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to testbed-40g-2 () port 0 AF_INET : demo
Recv   Send    Send                          
Socket Socket  Message  Elapsed              
Size   Size    Size     Time     Throughput  
bytes  bytes   bytes    secs.    10^6bits/sec  

 87380  16384  16384    10.00    24804.77   


Note, however, that CAKE is explicitly targeted at home gateways, which
generally run at speed a few orders of magnitude slower than this, and
we have heavily optimised for lowest possible latency. So this is a
conscious design choice, I guess you could say.

> Also, how ack filter can suppress packets after skb_gso_segment() ?

Hmm, because pure ACKs are not generally aggregated (sorry, I'm not
quite clear on when exactly GSO will kick in)?

-Toke