Re: [Cake] CAKE upstreaming - testers wanted, ACK filtering rescuers needed

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

Ryan Mounce <ryan@mounce.com.au> writes:

>> On 26 Apr 2018, at 16:09, Toke Høiland-Jørgensen <toke@toke.dk> wrote:
>>
>> Ryan Mounce <ryan@mounce.com.au> writes:
>>
>>> I'll investigate making the ACK filtering code safe, it is my mess after all :)
>>>
>>> Eric obviously understands this stuff a lot better than me, it looks
>>> like there are two issues?
>>> - Lack of minimum length check for TCP header, should be fairly
>>> straight-forward to fix
>>> - The possibility of unsafely filtering part of a split GSO
>>> super-packet?
>>
>> The issue with the ACK filter in relation to GSO was just that a GSO
>> segment can't contain pure ACKs, so there's no reason to check after
>> splitting. I've already removed that part, so it's just the length
>> issue. I think it's just a matter of finding the length and calling
>> pskb_may_pull() (and aborting if that returns false).
>>
>> -Toke
>
> Yep I think I’ve got all of that together now and I’ve submitted a PR
> against the cobalt branch.

Awesome thanks! Only a small nit, which I commented on the PR.

> An optimisation to what I’ve submitted would be moving the separate
> TCP header length check to only the IPv4 case, and adding
> sizeof(tcphdr) to the initial IPv6 header length check. Not that my
> ACK filtering code is so efficient to begin with...

Yeah, might as well?

Another obvious optimisation would be to add a flag to the cb struct for
pure ACKs (which would be set at the beginning, when ack_check is called
on packet enqueue). That way the loop could skip over anything that is
not marked as a pure ACK and a lot of checks could be skipped for the
packets being iterated over in the queue...

-Toke