Details

Reviewers

cc
tuexen
jtl
rrs
lstewart

Group Reviewers

transport

Commits

rS362831: MFC r361987: Prevent TCP Cubic to abruptly increase cwnd after slow-start
rS361987: Prevent TCP Cubic to abruptly increase cwnd after slow-start

Summary

This Diff changes the initial Wmax dragging to a single flag.

Another flag keeps track of when slow start is exited, to start the
t_last_cong only then.

This prevents sudden jumps in the caluclated cwnd by cubic, especially
when the flow is application limited during slow start (cwnd can not
grow as fast as expected). The downside is that cubic may remain
slightly longer in the concave region before starting the convex
region beyond Wmax again.

The rationale for this change is in RFC 8312 section 4.7 (timeout):
'''

During the first congestion avoidance after a timeout, CUBIC
increases its congestion window size using Eq. 1, where t is the
elapsed time since the beginning of the current congestion avoidance,
K is set to 0, and W_max is set to the congestion window size at the
beginning of the current congestion avoidance.

'''
This is effectively applied for any slow start scenario (initial
growth, retransmission timeout, and after_idle)

Test Plan

set up uperf for an initially application-limited flow,
before increasing the amout of data available to send. Monitor the
evolution of cwnd after slow start, after idle and past a
retransmission timeout

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 31594
Build 29184: arc lint + arc unit

Event Timeline

rscheff created this revision.Feb 12 2020, 8:46 PM

Harbormaster completed remote builds in B29331: Diff 68220.Feb 12 2020, 8:46 PM

jtl added reviewers: lstewart, transport.Feb 27 2020, 2:37 PM

Herald added a subscriber: melifaro. · View Herald TranscriptFeb 27 2020, 2:37 PM

Merge branch 'master' to keep diff current

Harbormaster completed remote builds in B31127: Diff 71876.May 16 2020, 11:10 PM

lohithbsd_gmail.com added a subscriber: lohithbsd_gmail.com.May 19 2020, 7:02 PM

rscheff mentioned this in D25065: Start cubic epoch after having been app-limited..May 30 2020, 10:43 AM

I think this has a higher priority than D25133.

cc mentioned this in D25133: Fix cubic RTO reaction.Jun 5 2020, 4:28 PM

I'm fine with it, except for the nits I commented in-line.

sys/netinet/cc/cc_cubic.c
95	Please use all CAPS in constant name. Fix typo in comment: experienced. Maybe use 0x00000001, since you use it for 32 bits. Now needed right now, but if you add (in the future) a constant which needs more than 2 digits, you will change the existing constants by adding leading zeros...
96	Please use all CAPS.
145	Maybe just setting the flag?

uppercase all flag definitions and left-pad flags to 32bit.

Harbormaster completed remote builds in B31586: Diff 72863.Jun 9 2020, 4:32 PM

rscheff marked 2 inline comments as done.Jun 9 2020, 4:32 PM

rscheff added inline comments.

sys/netinet/cc/cc_cubic.c
145	An unconditional assignment of a single bit will always be a read access, followed by a write access (and associated cache line updates), whereas the intent of the conditional was to make this mostly just a read-access, with the write rarely happening... A single (constant) write access by itself wouldn't account for much, but I'm still trying to prevent too many memory accesses...