From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg0-x244.google.com (mail-pg0-x244.google.com [IPv6:2607:f8b0:400e:c05::244]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id BD6B03B29E for ; Fri, 18 May 2018 00:08:58 -0400 (EDT) Received: by mail-pg0-x244.google.com with SMTP id c22-v6so2065232pgn.11 for ; Thu, 17 May 2018 21:08:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=lc4keRpO9O9jEDzIe+DJMdchmBFOGqnO3Xk9841IkvM=; b=ZSFIUvNCjkwWMzXhk9K/Pc2JQpcoK7IPTeRyNYxKYk14rX/JlUPBFrwf4zANEHsFlz MllvHrtmsjWpmSwBmyg0sxknSzUd+B61PSfHSug23WWbyLR1m2p7W4YPh9ilIqAl9mML rYs79VBY6TOug09u2ha+IMPJYqA7678kf9gXreuRaIh6K4BrKmoP6gOO0IXtRBFg8+Ti xs+gIOZpNVewKwy0JnRZ5qEedRfL6wEd7pgra/hNcU7UiMSk8wVd7bbh289BuYvYDooU awrhagWaurhzp1TI9Cthg8u7UmjoXUgnWAYQ2kc+ucNeg2ZwtUa6+lBQpXdpdNJ+Erci 0hWA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=lc4keRpO9O9jEDzIe+DJMdchmBFOGqnO3Xk9841IkvM=; b=aT2e1lxfwCBOWUGLC6T7BoKRUDP+V3xWJ63g0Dky7q+qmYkKgsjkje8sat2/3zRr+h /UbSQDwNh79w+YA3yX5oMIZaTVrXBh/eLvCgK7zEnif1ALA6fRaTMjwNvfCHSdIuuFNp SduPN6xOwXCG+r72Fq0R9SPboVjO+Mr2sa52czZE7qDbz2evbKebrCcjckZa01yixiY1 lek+R7jrTLgqBgMF9a6olkoOQYtYet120tZx/WUMEzvJZuHKDteaM+6r+9+i3bqzFHTp LYgMFBPKSpAzMzRKWYXChOxdxiaKqTfOsqIP06L91J/rAIOTNyJvfQjLespMnw2dzL72 UZFQ== X-Gm-Message-State: ALKqPwdoh5GdQ4utOCzL3pz5qJwfSyq2yn6mW3b8Lc5HPb1uBnK5gpRp 7IYobb9elqH88baxheK7NF1/rxye X-Google-Smtp-Source: AB8JxZpPePGOZjDFmPgU4Xx8Gzx5ONm4VtED5kIm5nl5FM3Dzp6ACLYThZj1p8MxpirfUUmmtjpSnw== X-Received: by 2002:a63:4004:: with SMTP id n4-v6mr6236766pga.248.1526616537666; Thu, 17 May 2018 21:08:57 -0700 (PDT) Received: from [192.168.86.235] (c-67-180-167-114.hsd1.ca.comcast.net. [67.180.167.114]) by smtp.gmail.com with ESMTPSA id l14-v6sm8923865pgu.1.2018.05.17.21.08.55 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 17 May 2018 21:08:56 -0700 (PDT) To: Ryan Mounce , =?UTF-8?B?VG9rZSBIw7hpbGFuZC1Kw7hyZ2Vu?= =?UTF-8?Q?sen?= Cc: Eric Dumazet , netdev@vger.kernel.org, Cake List References: <152650253056.25701.10138252969621361651.stgit@alrua-kau> <152650254614.25701.1377066681230937234.stgit@alrua-kau> <87in7my196.fsf@toke.dk> <541d62af-3938-5fdc-666c-dd243fa465b5@gmail.com> <877eo2xw8s.fsf@toke.dk> From: Eric Dumazet Message-ID: <4cc36c02-d656-5d0c-7313-3a1128213f12@gmail.com> Date: Thu, 17 May 2018 21:08:55 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [Cake] [PATCH net-next v12 3/7] sch_cake: Add optional ACK filter X-BeenThere: cake@lists.bufferbloat.net X-Mailman-Version: 2.1.20 Precedence: list List-Id: Cake - FQ_codel the next generation List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 18 May 2018 04:08:58 -0000 On 05/17/2018 07:36 PM, Ryan Mounce wrote: > On 17 May 2018 at 22:41, Toke Høiland-Jørgensen wrote: >> Eric Dumazet writes: >> >>> On 05/17/2018 04:23 AM, Toke Høiland-Jørgensen wrote: >>> >>>> >>>> We don't do full parsing of SACKs, no; we were trying to keep things >>>> simple... We do detect the presence of SACK options, though, and the >>>> presence of SACK options on an ACK will make previous ACKs be considered >>>> redundant. >>>> >>> >>> But they are not redundant in some cases, particularly when reorders >>> happen in the network. >> >> Huh. I was under the impression that SACKs were basically cumulative >> until cleared. >> >> I.e., in packet sequence ABCDE where B and D are lost, C would have >> SACK(B) and E would have SACK(B,D). Are you saying that E would only >> have SACK(D)? > > SACK works by acknowledging additional ranges above those that have > been ACKed, rather than ACKing up to the largest seen sequence number > and reporting missing ranges before that. > > A - ACK(A) > B - lost > C - ACK(A) + SACK(C) > D - lost > E - ACK(A) + SACK(C, E) > > Cake does check that the ACK sequence number is greater, or if it is > equal and the 'newer' ACK has the SACK option present. It doesn't > compare the sequence numbers inside two SACKs. If the two SACKs in the > above example had been reordered before reaching cake's ACK filter in > aggressive mode, the wrong one will be filtered. > > This is a limitation of my naive SACK handling in cake. The default > 'conservative' mode happens to mitigate the problem in the above > scenario, but the issue could still present itself in more > pathological cases. It's fixable, however I'm not sure this corner > case is sufficiently common or severe to warrant the extra complexity. The extra complexity is absolutely requested for inclusion in upstream linux. I recommend reading rfc 2018, whole section 4 (Generating Sack Options: Data Receiver Behavior ) Proposed ACK filter in Cake is messing the protocol, since the first rule is not respected * The first SACK block (i.e., the one immediately following the kind and length fields in the option) MUST specify the contiguous block of data containing the segment which triggered this ACK, unless that segment advanced the Acknowledgment Number field in the header. This assures that the ACK with the SACK option reflects the most recent change in the data receiver's buffer queue. An ACK filter must either : Not merge ACK if they contain different SACK blocks. Or make a precise analysis of the SACK blocks to determine if the merge is allowed, ie no useful information is lost. The sender should get all the information as which segments were received correctly, assuming no ACK are dropped because of congestion on return path.