Page MenuHomeFreeBSD

Fix cubic RTO reaction
ClosedPublic

Authored by rscheff on Jun 4 2020, 8:44 PM.

Details

Summary

Proper TCP Cubic operation requires the
knowledge of the maximum congestion window prior to the
last congestion event. The old structure of the generic
cc_cong_event handler did not lend itself to retain this
value.

Restructuring the assignment of cwnd to a single MSS into
the CC-specific cong_event handlers. Those not touched
here default to using the newreno cong-event handling for
RTO.

Found by cheng during internal validation testing.

Note that jtl@ did address cubic related aspects in rS307901
before, but due to that being conflated with other, larger
changes, that cubic bugfix was fully reverted again in r321480,
with only the block brackets restored again in rS321587

Test Plan

Observe the evolution of cwnd prior and after an
RTO event. Depending on where the last congestion event prior
to the RTO playes out, dramatic deviations between the expected
and observed cwnd have been found.

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

  • move max_cwnd assignment into the conditional and recalc K
chengc_netapp.com added inline comments.
sys/netinet/cc/cc_cubic.c
301–306 ↗(On Diff #72695)

Sounds like conservative. The cwnd is likely a small value when t_rxtshift > 1.
If my evaluation is right, I have no objection with this.

But when I look at D23655, I think D23655 has higher priority than this one.

sys/netinet/cc/cc_cubic.c
304 ↗(On Diff #72695)

use t_snd_prev_cwnd instead here, and not change other CCs.

Interaction between RACK and Cubic need to be looked at.

  • simplify patch and use snd_cwnd_prev in cc_cubic only.
This revision is now accepted and ready to land.Jun 24 2020, 12:18 PM
rscheff retitled this revision from Move cwnd assignment after RTO into each CC algo to Fix cubic RTO reaction.Jun 24 2020, 12:18 PM
rscheff edited the summary of this revision. (Show Details)
This revision was automatically updated to reflect the committed changes.