From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <toke@toke.dk>
Received: from mail.toke.dk (mail.toke.dk [IPv6:2001:470:dc45:1000::1])
 (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))
 (No client certificate requested)
 by lists.bufferbloat.net (Postfix) with ESMTPS id 983653BA8E
 for <cake@lists.bufferbloat.net>; Mon, 21 May 2018 13:35:44 -0400 (EDT)
From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= <toke@toke.dk>
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=toke.dk; s=20161023;
 t=1526924142; bh=KDR0jmeqgeWjSsvC1ignvD6LQ0xPY9zFH8Z2PwTQgRo=;
 h=From:To:Subject:In-Reply-To:References:Date:From;
 b=LbzCdUDhYoi+Zqhsnd4v5ngV9tVMzfTn1ysZ35ud4Xli2QhRMunANBn33Rj3p1mvN
 d4wrSPE2GznGL/+wWvoBPqc57DT7TM4aEyR3hcp3UAYuoeuGmNMmf0/YITVVXqvcGK
 NRFX0/g1XzYwOlVPsdQqd88RkETW4N95OJvuCZ6qBCiDKqCDqiaJL+5r7HEP4A0OMj
 V2XwGO9VaeQHHvSE2diYKJ0aC8MwNg8t5tZw1jlAdfrrOq5uG9Dxznn0IVG8+O98ni
 mKsU3VEsO9PRKpwoM2Ij+Wx57U7cQgk/Rlvqdo2QhNnJBUV3foMw2zDsYzgJ1ff7qB
 RtsVdiW+SEdUg==
To: Eric Dumazet <eric.dumazet@gmail.com>, netdev@vger.kernel.org,
 cake@lists.bufferbloat.net
In-Reply-To: <f7a7917b-9d81-b192-514b-3861321a4892@gmail.com>
References: <152691970035.4083.3349902669266371765.stgit@alrua-kau>
 <152691985736.4083.3416150561524698541.stgit@alrua-kau>
 <f7a7917b-9d81-b192-514b-3861321a4892@gmail.com>
Date: Mon, 21 May 2018 19:35:35 +0200
X-Clacks-Overhead: GNU Terry Pratchett
Message-ID: <87r2m4ykrc.fsf@toke.dk>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Cake] [PATCH net-next v13 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 <cake.lists.bufferbloat.net>
List-Unsubscribe: <https://lists.bufferbloat.net/options/cake>,
 <mailto:cake-request@lists.bufferbloat.net?subject=unsubscribe>
List-Archive: <https://lists.bufferbloat.net/pipermail/cake>
List-Post: <mailto:cake@lists.bufferbloat.net>
List-Help: <mailto:cake-request@lists.bufferbloat.net?subject=help>
List-Subscribe: <https://lists.bufferbloat.net/listinfo/cake>,
 <mailto:cake-request@lists.bufferbloat.net?subject=subscribe>
X-List-Received-Date: Mon, 21 May 2018 17:35:44 -0000

Eric Dumazet <eric.dumazet@gmail.com> writes:

> On 05/21/2018 09:24 AM, Toke H=C3=B8iland-J=C3=B8rgensen wrote:
>
>> +		while (oplen_tmp >=3D 8) {
>> +			u32 right_b =3D get_unaligned_be32(sack_tmp + 4);
>> +			u32 left_b =3D get_unaligned_be32(sack_tmp);
>> +
>> +			if (left_b >=3D right_b)
>> +				continue;
>> +
>> +			if (first)
>> +				bytes_b +=3D right_b - left_b;
>> +
>> +			if (left_b <=3D left_a && right_a <=3D right_b) {
>> +				found =3D true;
>> +				if (!first)
>> +					break;
>> +			}
>> +			oplen_tmp -=3D 8;
>> +			sack_tmp +=3D 8;
>> +		}
>>
>
> This is obviously incorrect.
>
> Please take a look at TCP stack, and how it handles sequence wrapping
> with following macros :
>
> after(), before()

Ah yes, sequence number wrapping. I was thinking I needed to deal with
that, and then got sidetracked and forgot about it. Will fix.

Other than that, do you agree that this approach to SACK and header
handling can work?

> Quite frankly O wonder how this was really tested.

Getting it wrong results in a few drop decisions being wrong every 2**32
bytes; easy to miss. If you have a "mis-behaving middle-box test suite"
or something I'll be happy to hear about it :)

-Toke