From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl0-x241.google.com (mail-pl0-x241.google.com [IPv6:2607:f8b0:400e:c01::241]) (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 64E2E3B29E for ; Fri, 18 May 2018 03:43:21 -0400 (EDT) Received: by mail-pl0-x241.google.com with SMTP id bi12-v6so4088088plb.12 for ; Fri, 18 May 2018 00:43:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mounce.com.au; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=E5CzaSnA9jgvhtB7saxRxs4x32yZkRI9z+3QAhwVgjc=; b=VDYPu4OvVsqCpoGSiXXIHA0HcpF1gWFGjEZ7T7XqwFoQxUwyD1JOtXOGuyJXuV7vPO 2XkkyHPZpiBi+RJPEiEYQLAK7kR7CbnFJbu6bA87zYAQD6XDfaUO2AYI7ECIeKbCbKQE SVhuz3W8mAnCFYufZIKhnaXpAjMxeN5181QUA08UyP0ETbYC9v8WD/HOUR7erKA6EsLU 32tq2JNMKWZVI3pwNcezmC5wD2NRlTwof3ll3pSaDwUV2UJcWF4CzhsgFG62oRap5RfH 8iHlH7CZu366Ln4elkTM1E2O9L7RZMNo8cRIvjd31/y7woTgVyR781xCHpaHYn3ylr9z N8RQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=E5CzaSnA9jgvhtB7saxRxs4x32yZkRI9z+3QAhwVgjc=; b=SQ1DUCxS2wx/Oy1w3P0vfX/RAYGD/e67bkHokWaKhFLsMkk0FS2n9lL+UdNfgMm4E/ 8WSDzpKjqYZ+c5uE1U5/br1zpEQOectmKeeSM8MmBuXTez+Ii52IGZw76GgbjcPk/wOs 8iDnQ9n620zabMD6mqzRrfzhgsxwfFrnhIy7zdIJMi0Qrbu0swvhzJVBdRYi3W9XoghE V8qaUg2Fuvrn6lbbQK8p6Q7/GaUjzQDqDJjJ60q+ldtGqqtb8gBG5mGFw3q/hL65Vix1 fVM1Hug2J3y39BseqIt4pkbANtyK9U2FiEVR2TwVVqtCkHcUNesGyBIuqHKBQ5oxGoed MSiw== X-Gm-Message-State: ALKqPwfUi0ScVqbkXFLTC3DZuuLFZY5KgF60Bgcf5ynpI8pnwwC4MWgp JCj0iTjezVlKm2/63jlVROUs9xtLHkJoCISX1bE5uQ== X-Google-Smtp-Source: AB8JxZqsqe6btdFQ7g0NvZCg3tiDt9gN6ALo0CPpdmIArZ7EXQvhlSZDOLM3YoPBVTsZCcGhoOjkZDV1P/KIWP3a/6Y= X-Received: by 2002:a17:902:2927:: with SMTP id g36-v6mr8293432plb.303.1526629400160; Fri, 18 May 2018 00:43:20 -0700 (PDT) MIME-Version: 1.0 Received: by 10.236.155.194 with HTTP; Fri, 18 May 2018 00:43:04 -0700 (PDT) X-Originating-IP: [45.118.67.18] In-Reply-To: <4cc36c02-d656-5d0c-7313-3a1128213f12@gmail.com> 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> <4cc36c02-d656-5d0c-7313-3a1128213f12@gmail.com> From: Ryan Mounce Date: Fri, 18 May 2018 17:13:04 +0930 Message-ID: To: Eric Dumazet Cc: =?UTF-8?B?VG9rZSBIw7hpbGFuZC1Kw7hyZ2Vuc2Vu?= , netdev@vger.kernel.org, Cake List Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 07:43:21 -0000 On 18 May 2018 at 13:38, Eric Dumazet wrote: > > > On 05/17/2018 07:36 PM, Ryan Mounce wrote: >> On 17 May 2018 at 22:41, Toke H=C3=B8iland-J=C3=B8rgensen = wrote: >>> Eric Dumazet writes: >>> >>>> On 05/17/2018 04:23 AM, Toke H=C3=B8iland-J=C3=B8rgensen 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 conside= red >>>>> 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 li= nux. > > I recommend reading rfc 2018, whole section 4 (Generating Sack Options: D= ata 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 i= s 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. I have submitted a patch to cake's ACK filter, now it will only filter an old SACK if its highest right block edge value is smaller than that of the most recently received SACK. If there is reordering, neither will be filtered.