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

[Eric Dumazet <eric.dumazet@gmail.com>]

On 04/30/2018 02:27 PM, Dave Taht wrote:

> I actually have a tc - bpf based ack filter, during the development of
> cake's ack-thinner, that I should submit one of these days. It
> proved to be of limited use.
> 
> Probably the biggest mistake we made is by calling this cake feature a
> filter. It isn't.
> 
> Maybe we should have called it a "thinner" or something like that? In
> order to properly "thin" or "reduce" an ack stream
> you have to have a queue to look at and some related state. TC filters
> do not operate on queues, qdiscs do. Thus the "ack-filter" here is
> deeply embedded into cake's flow isolation and queue structures.

A feature eating packets _is_ a filter.

Given that a qdisc only sees one direction, I really do not get why ack-filter
is so desirable in a packet scheduler.

You have not provided any numbers to show how useful it is to maintain this
code (probably still broken btw, considering it is changing some skb attributes).

On wifi (or any half duplex medium), you might gain something by carefully
sending ACK not too often, but ultimately this should be done by TCP stack,
in well controlled environment [1], instead of middle-boxes happily playing/breaking
some Internet standards.

[1] TCP stack has the estimations of RTT, RWIN, throughput, and might be able to
avoid flooding the network with acks under some conditions.

Say RTT is 100ms, and we receive 1 packet every 100 usec (no GRO aggregation)
Maybe we do not really _need_ to send 5000 ack per second 
(or even 10,000 ack per second if a hole needs a repair)

Also on wifi, the queue builds in the driver queues anyway, not in the qdisc.
So ACK filtering, if _really_ successful, would need to be modularized.

Please split Cake into a patch series.
Presumably putting the ack-filter on a patch of its own.