Differential D33035
tcp: Add hystart++ to our cubic implementation. Authored by rrs on Nov 17 2021, 4:46 PM. Tags None Referenced Files
Details
As promised to the transport call on 11/4/22 here is an implementation Now question for reviewers, I have taken the sysctl's and raised them to cc.c level, I am Setup to use cubic and then test in various lab scenarios with BB logging on to make
Diff Detail
Event TimelineComment Actions Note missing yet from this diff is the ctl_output() function so you can actually enable it. Comment Actions Ok this straightens out the allowed flag to be up at the ccv->flags Comment Actions With a bit of testing I figured out I had missed a few things. Now this version Comment Actions Richard: One thing that is interesting to me about cubic is that it has this flag I am not sure if the flag is old and no longer paid attention too or Maybe you have some idea how it used to be used?? Comment Actions Ahh I see if you get a "RTO event" or an "Application limited" event you What do you think Richard? Comment Actions Update the diff to take into account Neal Cardwell's comments on tcpm.. i.e. Comment Actions I won't have the time to review this in the near future but would note that many of the things I flagged for the original newreno hystart++ implementation also apply to this implementation. However, I wouldn't call any of them blockers so don't have an objection to you committing this at your discretion. Comment Actions Forgot to update the manual pages as well since the mib variables Comment Actions Let;s expand the GT to GT or equal if snd_max is above snd_una. This makes Let's also fix the oscillation I found between CSS and out. I am not, as yet, addressing Comment Actions I would suggest to use the spelling HyStart++ in man-pages and comments consistently as in draft-ietf-tcpm-hystartplusplus.
Comment Actions Fix the inadvertent change Michael noted in the man, as well Comment Actions Not quite sure what you mean; the IN_SLOWSTART is removed in CongAvoidance effectively unconditionally (well, the condition is, that we entered CA (cwnd > ssthresh), and that the flag was set. It got added to track the best point when the cubic epoch (t_last_cong) is initialized, as we found that continously tracking /updating while still in slow start is not good enough, if you transition directly from SS to AppLimited - the app limited event can be delayed such that an "older" t_last_cong results in unexpectedly large cwnd adjustements by cubic... Hope this explanation makes sense. (The clearing is two flags, spread out across two lines: if (CCV(ccv, snd_cwnd) <= CCV(ccv, snd_ssthresh) ||
cubic_data->min_rtt_ticks == TCPTV_SRTTBASE) {
cubic_data->flags |= CUBICFLAG_IN_SLOWSTART; # < set while sending data with cwnd < ssthresh -> SS
newreno_cc_ack_received(ccv, type);
} else {
if ((cubic_data->flags & CUBICFLAG_RTO_EVENT) &&
(cubic_data->flags & CUBICFLAG_IN_SLOWSTART)) {
/* RFC8312 Section 4.7 */
cubic_data->flags &= ~(CUBICFLAG_RTO_EVENT |
CUBICFLAG_IN_SLOWSTART); # < cleared is we leave SS due to an RTO event, with different actions
cubic_data->max_cwnd = CCV(ccv, snd_cwnd);
cubic_data->K = 0;
} else if (cubic_data->flags & (CUBICFLAG_IN_SLOWSTART |
CUBICFLAG_IN_APPLIMIT)) { # < if either flag is set
cubic_data->flags &= ~(CUBICFLAG_IN_SLOWSTART |
CUBICFLAG_IN_APPLIMIT); # < we clear it here - so cleared whenever cwnd > ssthresh.
cubic_data->t_last_cong = ticks;
cubic_data->K = cubic_k(cubic_data->max_cwnd /
CCV(ccv, t_maxseg));
} | ||||||||||||||||||||||||||||||||||||||||||