Page MenuHomeFreeBSD

tcp: Deal with DSACKs, and adjust rescue hole on success.
ClosedPublic

Authored by rscheff on Apr 19 2021, 2:29 PM.

Details

Summary

When a rescue retransmission is successful, rather than
inserting new holes to the left of it, adjust the old
rescue entry to cover the missed sequence space.

Also, as snd_fack may be stale by that point, pull it forward
in order to never create a hole left of snd_una/th_ack.

Finally, with DSACKs, tcp_sack_doack() may be called
with new full ACKs but a DSACK block. Account for this
eventuality properly to keep sacked_bytes >= 0.

MFC after: 3 days
Sponsored by: NetApp, Inc.

Test Plan

Found by tools/test/misc/tcp4.sh and tools/test/jumbo.sh

Diff Detail

Repository
rG FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

This revision is now accepted and ready to land.Apr 19 2021, 4:13 PM

Should we produce an -EN patch in 13.0 for this?

in stable-13, this is all gated with net.inet.tcp.rfc6675_pipe, defaulted to disabled.

There are people running with 6675_pipe enabled, but my thinking is that the issues observed in low-latency, zero-loss (only heavy reordering) would not have quite the same impact with a real lossy / higher latency network in between server and client.

But I have no prior expirience on what warrants release patches, and take any advice in this space.

Ok, I asked @emaste and the process is pretty simple. Since there are tuning guides that might enable the old sysctl by name, I'll prepare the EN submission once this lands in main and has some soak time.

  • relax criteria when to move left the rescue hole
This revision now requires review to proceed.Apr 19 2021, 10:20 PM

I ran tests with D29835.87758.diff for 8 hours without seeing any issues.

This revision was not accepted when it landed; it landed in state Needs Review.Apr 20 2021, 12:56 PM
This revision was automatically updated to reflect the committed changes.