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

From: Toke Høiland-Jørgensen <[email-masked]>
Date: Tue, 01 May 2018 20:53:26 +0200

Eric Dumazet <[email-masked]> writes:

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

The ACK filter in CAKE is there to solve a very particular use case:
Residential internet connections with bandwidths so asymmetric that it
hurts TCP performance. It is not on by default; but rather meant to be
configured by users which suffer from this particular ISP brokenness
(which, sadly, does happen; there are ISPs who believe a 50/1 bandwidth
ratio is reasonable). For those users, the ACK filter can help. 

We certainly do not advise people to turn it on in the general case! As
you point, in general TCP performance is best improved in the TCP stack...

> You have not provided any numbers to show how useful it is to maintain
> this code

You mean apart from that in the linked blog post and the paper?
Here's an executive summary:

On a 30/1 Mbps connection with a bidirectional traffic test (RRUL),
turning on the ACK filter improves downstream throughput by ~20% and
upstream throughput by ~12% in conservative mode and ~40% in aggressive
mode, at the cost of ~5ms of inter-flow latency due to the increased
congestion.

In *really* pathological cases, the effect can be a lot more; for
instance, the ACK filter increases the achievable downstream throughput
on a link with 100 Kbps in the upstream direction by an order of
magnitude (from ~2.5 Mbps to ~25 Mbps).

> (probably still broken btw, considering it is changing some skb
> attributes).

As you may have noticed over the last few iterations, I've actually been
trying to fix any brokenness. If you could be specific as to what is
still broken, that would be helpful.

(I'm assuming are referring to the calls to skb_set_inner* - but do you
mean all of them, or just the ones that set inner == outer?)

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

You are quite right that in general, TCP performance is best improved in
the TCP stack. But home users are not generally in control of that; the
ACK filter helps in those specific deployments (again, it's off by
default, and you shouldn't turn it on in the general case!).

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

Yes, please do fix that.

> Also on wifi, the queue builds in the driver queues anyway, not in the
> qdisc.

Actually, we've fixed that (for some drivers); there is now no qdisc on
WiFi interfaces, and we've integrated FQ-CoDel'ed queueing into the
stack where it can be effective. But no, running CAKE with an ACK filter
on a WiFi link is not going to be effective. Don't do that.

> So ACK filtering, if _really_ successful, would need to be
> modularized.

I really hope ACK filtering won't be "_really_ successful". Again, it is
not meant to be a feature that's on everywhere, it's targeting a very
specific use case that CAKE is optimised for: The home router use case.

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

*sigh* - can do, I guess. Though I'm not sure what that is going to
accomplish, exactly?

-Toke