From mboxrd@z Thu Jan 1 00:00:00 1970 From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= Subject: Re: [PATCH net-next v6] Add Common Applications Kept Enhanced (cake) qdisc Date: Tue, 01 May 2018 20:53:26 +0200 Message-ID: <878t932ont.fsf@toke.dk> References: <20180429213439.7389-1-toke@toke.dk> Mime-Version: 1.0 Content-Type: text/plain Cc: Linux Kernel Network Developers , Cake List To: Eric Dumazet , Dave Taht , Cong Wang Return-path: Received: from mail.toke.dk ([52.28.52.200]:34757 "EHLO mail.toke.dk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756382AbeEASx1 (ORCPT ); Tue, 1 May 2018 14:53:27 -0400 In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: Eric Dumazet 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