modernize TCP constants
Needs RevisionPublic
Actions

Authored by kmacy on Nov 27 2015, 12:47 AM.

Details

Reviewers

hiren
lstewart
gnn
kbowling
rscheff

Group Reviewers

transport

Summary

eliminate 200ms "slop" when calculating RTO
correct the code so that it accurately reflects the comment and set TCPTV_MIN to 3 ticks not whatever hz/33 happens to be
per RFC 6298 reduce default RTO TCPTV_RTOBASE 3->1
reduce delack time to more common 40ms
track delack separately so that it is at most 1/2 RTO

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

kmacy updated this revision to Diff 10535.Nov 27 2015, 12:47 AM

kmacy retitled this revision from to modernize TCP constants.

kmacy updated this object.

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

kmacy added reviewers: hiren, lstewart.

kmacy set the repository for this revision to rS FreeBSD src repository - subversion.

kmacy added a project: transport.

kmacy added a subscriber: benno.

Herald added a subscriber: imp. · View Herald TranscriptNov 27 2015, 12:47 AM

kmacy added a subscriber: gnn.Nov 27 2015, 1:37 AM

A couple of comments in-line after a very quick glance. I'll try to take a better look next week.

Thanks for your work.

sys/netinet/tcp_subr.c
404	You may want to remove #define TCPTV_CPU_VAR ( hz/5 ) /* cpu variance allowed (200ms) */ from tcp_timer.h probably as this was the only reference?
sys/netinet/tcp_var.h
209	Is tracking this in tcpcb required?

gnn added reviewers: gnn, transport.May 5 2016, 4:26 PM

gnn removed a subscriber: gnn.

Is TCPTV_DELACK (hz/25) 40 ms, or is it 40 ms only when HZ is 1000?

@lstewart ping

Herald added 1 blocking reviewer(s): transport. · View Herald TranscriptMay 4 2018, 6:07 PM

This LGTM taking @hiren suggestion to drop the unused TCPTV_CPU_VAR define. At LLNW we ran w/o delack to work around most of the issues this addresses in a more elegant way.

This revision is now accepted and ready to land.Oct 1 2018, 2:56 AM

rscheff mentioned this in D23281: Reduce Delayed ACK timeout to 40ms.Jan 20 2020, 6:34 PM

rscheff requested changes to this revision.Jan 20 2020, 6:42 PM

rscheff added a subscriber: rscheff.

rscheff added inline comments.

sys/netinet/tcp_timer.c
651	In this and similar instances, replace tp->t_rttmin with max(tp->rttmin, TCPTV_MIN), and initialize tcp_rexmit_min = 10 * TCPTV_MIN for example.
653	Setting the delayed ack timeout dynamically to a session-specific value should be discussed as a separate topic and not bundled like this.
sys/netinet/tcp_timer.h
78	fixed by D18941
110	This won't actually change the absolute minimum. Only the default minRTO (which is exposed as sysctl nowadays). Still valuable to make sure minrto doesn't fall below 2 or 3 ticks (there are no such precautions in the current stack, other than rto_slop)
120	Taking this out into D23281