From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Dumazet Subject: Re: [PATCH net-next v6] Add Common Applications Kept Enhanced (cake) qdisc Date: Tue, 1 May 2018 09:06:53 -0700 Message-ID: References: <20180429213439.7389-1-toke@toke.dk> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: =?UTF-8?Q?Toke_H=c3=b8iland-J=c3=b8rgensen?= , Linux Kernel Network Developers , Cake List To: Dave Taht , Cong Wang Return-path: Received: from mail-pf0-f172.google.com ([209.85.192.172]:43280 "EHLO mail-pf0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751071AbeEAQG4 (ORCPT ); Tue, 1 May 2018 12:06:56 -0400 Received: by mail-pf0-f172.google.com with SMTP id j11so9398613pff.10 for ; Tue, 01 May 2018 09:06:56 -0700 (PDT) In-Reply-To: Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: 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.