Page MenuHomeFreeBSD

Several LRO fixes
ClosedPublic

Authored by kbowling on Feb 28 2018, 7:48 AM.
Referenced Files
Unknown Object (File)
Thu, Mar 7, 11:40 PM
Unknown Object (File)
Dec 28 2023, 9:17 AM
Unknown Object (File)
Dec 26 2023, 7:11 AM
Unknown Object (File)
Dec 20 2023, 5:36 AM
Unknown Object (File)
Dec 13 2023, 5:28 AM
Unknown Object (File)
Nov 26 2023, 2:14 PM
Unknown Object (File)
Nov 25 2023, 12:27 PM
Unknown Object (File)
Nov 24 2023, 2:43 PM

Details

Summary

This changeset contains a few tightly intermingled and additive bugfixes from Netflix and LLNW:

  1. rrs - Lets make the LRO code look for true dup-acks and window update acks fly on through and combine.
  2. rrs - Make the LRO engine a bit more aware of ack-only seq space. Lets not have it incorrectly wipe out newer acks for older acks when we have out-of-order acks (common in wifi environments).
  3. jeggleston - LRO eating window updates

Based on all of the above I think we are RFC compliant doing it this way:

https://tools.ietf.org/html/rfc1122

section 4.2.2.16

"Note that TCP has a heuristic to select the latest window update despite possible datagram reordering; as a result, it may ignore a window update with a smaller window than previously offered if neither the sequence number nor the acknowledgment number is increased."

Test Plan

These changes are in use at LLNW and Netflix

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

rstone added a subscriber: rstone.

I've written some unit tests that covering these cases here:

https://github.com/rysto32/sysunit/blob/master/netinet/tcp_lro.gtest.cpp#L850

I can confirm that the new behaviour is much more sane than the older behaviour.

sys/netinet/tcp_lro.c
826 ↗(On Diff #39819)

I think that you mean "old ack" here. As far as I can tell, the only way to reach this point is to have a packet with no payload and th_ack < le->ack_seq.

sys/netinet/tcp_lro.c
826 ↗(On Diff #39819)

Agreed.

Fix comment per rstone and jegg

sys/netinet/tcp_lro.c
819 ↗(On Diff #40065)

Technically we should only do this (or look at th->th_ack at all) if TH_ACK is set

sys/netinet/tcp_lro.c
819 ↗(On Diff #40065)

On the transport call we discussed that this should probably be handled in another patch as it is a generic issue with the LRO code.

sbruno added a subscriber: sbruno.

I'll grab this and shovel it in after builds are done.

This revision was not accepted when it landed; it landed in state Needs Review.Mar 9 2018, 12:08 AM
This revision was automatically updated to reflect the committed changes.