Differential D9519

Don't zero out srtt after excess retransmits
ClosedPublic
Actions

Authored by rstone on Feb 9 2017, 6:18 PM.

Details

Reviewers

gnn

Group Reviewers

transport

Commits

rS313646: Don't zero out srtt after excess retransmits

Summary

If the TCP stack has retransmitted more than 1/4 of the total
number of retransmits before a connection drop, it decides that
its current RTT estimate is hopelessly out of date and decides
to recalculate it from scratch starting with the next ACK.

Unfortunately, it implements this by zeroing out the current RTT
estimate. Drop this hack entirely, as it makes it significantly more
difficult to debug connection issues. Instead check for excessive
retransmits at the point where srtt is updated from an ACK being
received. If we've exceeded 1/4 of the maximum retransmits,
discard the previous srtt estimate and replace it with the latest
rtt measurement.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 7320
Build 7488: arc lint + arc unit

Event Timeline

rstone updated this revision to Diff 24943.Feb 9 2017, 6:18 PM

rstone retitled this revision from to Don't zero out srtt after excess retransmits.

rstone edited the test plan for this revision. (Show Details)

rstone updated this object.

Herald added a reviewer: transport. · View Herald TranscriptFeb 9 2017, 6:18 PM

gnn added a subscriber: gnn.Feb 11 2017, 4:10 PM

gnn added inline comments.

sys/netinet/tcp_input.c
3497	nit pick. I'd like another set of parens here to group the sub-expressions.
sys/netinet/tcp_timer.h
122	Ooops?

fixup! Don't zero out srtt after excess retransmits

Herald edited edge metadata. · View Herald TranscriptFeb 11 2017, 4:50 PM

rstone marked 2 inline comments as done.Feb 11 2017, 4:51 PM

gnn accepted this revision.Feb 11 2017, 4:54 PM

gnn added a reviewer: gnn.

This revision is now accepted and ready to land.Feb 11 2017, 4:54 PM

Closed by commit rS313646: Don't zero out srtt after excess retransmits (authored by rstone). · Explain WhyFeb 11 2017, 5:05 PM

This revision was automatically updated to reflect the committed changes.

lstewart added a subscriber: lstewart.Feb 17 2017, 11:41 PM

lstewart added inline comments.

head/sys/netinet/tcp_input.c
3497 ↗	(On Diff #25018)	I realise you're just shuffling around the original logic, but I'm curious if, based on the investigation that led you to this nit, you have any data around why updating the smoothed estimate is appropriate when rxtshift <= 3 but not >3?