Ryan Mounce <[email protected]> writes: >> On 26 Apr 2018, at 16:09, Toke Høiland-Jørgensen <[email protected]> wrote: >> >> Ryan Mounce <[email protected]> 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 _______________________________________________ Cake mailing list [email protected] https://lists.bufferbloat.net/listinfo/cake
