Re: [Cake] [PATCH net-next v13 3/7] sch_cake: Add optional ACK filter

["Toke Høiland-Jørgensen" <toke@toke.dk>]

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

> On 05/21/2018 09:24 AM, Toke Høiland-Jørgensen wrote:
>
>> +		while (oplen_tmp >= 8) {
>> +			u32 right_b = get_unaligned_be32(sack_tmp + 4);
>> +			u32 left_b = get_unaligned_be32(sack_tmp);
>> +
>> +			if (left_b >= right_b)
>> +				continue;
>> +
>> +			if (first)
>> +				bytes_b += right_b - left_b;
>> +
>> +			if (left_b <= left_a && right_a <= right_b) {
>> +				found = true;
>> +				if (!first)
>> +					break;
>> +			}
>> +			oplen_tmp -= 8;
>> +			sack_tmp += 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