Page MenuHomeFreeBSD

Fix spurious retransmit recovery on low latency networks
ClosedPublic

Authored by kmacy on Nov 17 2016, 11:40 PM.

Details

Summary

TCP's smoothed RTT (SRTT) can be much larger than an actual observed RTT. This can be either because of hz restricting the calculable RTT to 10ms in VMs or 1ms using the default 1000hz or simply because SRTT recently incorporated a larger value.

If an ACK arrives before the calculated badrxtwin (now + SRTT):
tp->t_badrxtwin = ticks + (tp->t_srtt >> (TCP_RTT_SHIFT + 1));

We'll erroneously reset snd_una to snd_max. If multiple segments were dropped and this happens repeatedly the transmit rate will be limited to 1MSS per RTO until we've retransmitted all drops.

Diff Detail

Repository
rS 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

kmacy retitled this revision from to Fix spurious retransmit recovery on low latency networks.Nov 17 2016, 11:40 PM
kmacy updated this object.
kmacy edited the test plan for this revision. (Show Details)
kmacy added reviewers: hiren, rstone.
kmacy set the repository for this revision to rS FreeBSD src repository.
kmacy added a project: network.
kmacy added a subscriber: transport.
kmacy updated this revision to Diff 22301.
kmacy edited edge metadata.Nov 18 2016, 3:22 AM
kmacy updated this revision to Diff 22307.

simplify badrxtwin assignment

kmacy edited edge metadata.Nov 18 2016, 4:31 AM
kmacy updated this revision to Diff 22313.

Add comment to tcp_timer.c as requested by hiren@

hiren edited edge metadata.Nov 18 2016, 4:32 AM
hiren accepted this revision.

Thanks for this fix. Nice catch.

Please also mention in the commit log that now we are tracking this functionality separately for timestamps and non-timestamps case.

sys/netinet/tcp_timer.c
717 ↗(On Diff #22307)

Please add a comment here that this is handling non-ts case and ts case is handled in tcp_output.c
It may seem obvious but comments would help. :-)

This revision is now accepted and ready to land.Nov 18 2016, 4:32 AM
emaste added a subscriber: emaste.Nov 22 2016, 1:40 AM
kmacy edited edge metadata.Nov 23 2016, 9:56 PM
kmacy updated this revision to Diff 22470.

add missed check

This revision now requires review to proceed.Nov 23 2016, 9:56 PM

@kmacy is this still relevant?

This revision is now accepted and ready to land.May 26 2017, 7:57 AM

@rstone have you committed an equivalent fix already?

rstone edited edge metadata.May 29 2017, 6:28 PM

@kmacy I have not. We probably have an equivalent fix committed locally @ $WORK but it might be tied too closely to the high-precision timestamp work to easily extract.

sbruno accepted this revision.May 8 2018, 1:14 AM
This revision was automatically updated to reflect the committed changes.