Page MenuHomeFreeBSD

Cubic: prevent abrupt cwnd jumps after slow start
ClosedPublic

Authored by rscheff on Feb 12 2020, 8:46 PM.

Details

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

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

  • Merge branch 'master' to keep diff current

I think this has a higher priority than D25133.

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

sys/netinet/cc/cc_cubic.c
95 ↗(On Diff #71876)

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 ↗(On Diff #71876)

Please use all CAPS.

145 ↗(On Diff #71876)

Maybe just setting the flag?

  • uppercase all flag definitions and left-pad flags to 32bit.
rscheff added inline comments.
sys/netinet/cc/cc_cubic.c
145 ↗(On Diff #71876)

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...

This revision is now accepted and ready to land.Jun 9 2020, 5:22 PM
  • remove flag field microoptimization
This revision now requires review to proceed.Jun 9 2020, 7:21 PM
This revision is now accepted and ready to land.Jun 9 2020, 7:24 PM