Page MenuHomeFreeBSD

Fix server side TCP fast open
ClosedPublic

Authored by tuexen on Jan 16 2020, 7:34 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Apr 20, 4:49 PM
Unknown Object (File)
Feb 12 2024, 7:35 AM
Unknown Object (File)
Feb 10 2024, 6:06 PM
Unknown Object (File)
Jan 29 2024, 6:01 PM
Unknown Object (File)
Jan 27 2024, 8:27 AM
Unknown Object (File)
Jan 27 2024, 8:26 AM
Unknown Object (File)
Jan 27 2024, 8:26 AM
Unknown Object (File)
Jan 27 2024, 8:26 AM
Subscribers

Details

Summary

The server side of TCP fast open relies on the delayed ACK timer for sending the SYN-AK to allow including user data in the SYN-ACK.
When DSACK support was added in r347382, an immediate ACK was requested even for the received SYN with user data. This patch fixes that.

The problem was reported by Jeremy Harris on the transport@ mailing list.

Test Plan

Run the following packetdrill script:

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 28707

Event Timeline

There is a special case handling in DSACK when start and end (2nd and 3rd param) are equal. Placing the todrop before the block will prevent DSACKs from generated if the last in-sequence packet was sent twice (is to be dsacked).

I suggest to move the (todrop > 0) check just over the tf_ACKNOW statement, possibly with a check if we are not to send out a (D)SACK block (tp->rcv_numsacks >= 1).

Same change for RACK and BBR.

In D23212#508834, @rscheff_gmx.at wrote:

There is a special case handling in DSACK when start and end (2nd and 3rd param) are equal. Placing the todrop before the block will prevent DSACKs from generated if the last in-sequence packet was sent twice (is to be dsacked).

Please note that todrop is positive when we enter the block. It is only reduced when a SYN segment is received. So the case where todrop is zero and caught by the condition I added is only when todrop is initially 1 and reduced to 0 because we received a SYN segment. The condition todrop > 0 and tlen == 0 would be handled wrong. Can it happen?

I suggest to move the (todrop > 0) check just over the tf_ACKNOW statement, possibly with a check if we are not to send out a (D)SACK block (tp->rcv_numsacks >= 1).

I can distinctively remember that i deliberately have that special case where the two last parameters are identical in the first DSACK patch.

However, going over the history, here we are save. While discussing this, all the DSACK test cases were run with this patch, and all passed. There is not yet a specific test case for DSACK for TFO though.

So, one thing to validate would be the retransmission of a TFO SYN with data, if the DSACK code emits a DSACK block for the twice received data. It may be off by 1 (including the seq no of the SYN, rather the first data byte).

This revision is now accepted and ready to land.Jan 16 2020, 10:10 PM