d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Some correctness and robustness fixes related to CUBIC's mean RTT estimate:

Browse Source 
- The mean RTT is updated at the end of each congestion epoch, but if we switch
  to congestion avoidance within the first epoch (e.g. if ssthresh was primed
  from the hostcache), we'll trigger a divide by zero panic in
  cubic_ack_received(). Set the mean to the min in cubic_record_rtt() if the
  mean is less than the min to ensure we have a sane mean for use in this
  situation. This fixes the panic reported by Nick Hibma.

- Adjust conditions under which we update the mean RTT in cubic_post_recovery()
  to ensure a low latency path won't yield an RTT of less than 1. This avoids
  another potential divide by zero panic when running CUBIC in networks with
  sub-millisecond latencies.

- Remove the "safety" assignment of min into mean when we don't update the mean
  because of failed conditions. The above change to the conditions for updating
  the mean ensures the safety issue is addressed and I feel it is better to keep
  our previous mean estimate around if we can't update than to revert to the
  min.

- Initialise the mean RTT to 1 on connection startup to act as a safety belt if
  a situation we haven't considered and addressed with the above changes were to
  crop up in the wild.

Sponsored by:	FreeBSD Foundation
Reported and tested by:	Nick Hibma
Discussed with:	David Hayes <dahayes at swin edu au>
MFC after:	5 weeks
X-MFC with:	r216114
This commit is contained in:
Lawrence Stewart 2011-01-21 05:19:47 +00:00
parent eefbdb2a6e
commit 47f44cdd93
1 changed files with 17 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
sys/netinet/cc/cc_cubic.c
View File

@ -212,7 +212,7 @@ cubic_cb_init(struct cc_var *ccv)
/* Init some key variables with sensible defaults. */
cubic_data->t_last_cong = ticks;
cubic_data->min_rtt_ticks = TCPTV_SRTTBASE;
cubic_data->mean_rtt_ticks = TCPTV_SRTTBASE;
cubic_data->mean_rtt_ticks = 1;
ccv->cc_data = cubic_data;
@ -328,12 +328,11 @@ cubic_post_recovery(struct cc_var *ccv)
cubic_data->t_last_cong = ticks;
/* Calculate the average RTT between congestion epochs. */
if (cubic_data->epoch_ack_count > 0 && cubic_data->sum_rtt_ticks > 0)
if (cubic_data->epoch_ack_count > 0 &&
cubic_data->sum_rtt_ticks >= cubic_data->epoch_ack_count) {
cubic_data->mean_rtt_ticks = (int)(cubic_data->sum_rtt_ticks /
cubic_data->epoch_ack_count);
else
/* For safety. */
cubic_data->mean_rtt_ticks = cubic_data->min_rtt_ticks;
}
cubic_data->epoch_ack_count = 0;
cubic_data->sum_rtt_ticks = 0;
@ -362,9 +361,21 @@ cubic_record_rtt(struct cc_var *ccv)
* XXXLAS: Should there be some hysteresis for minrtt?
*/
if ((t_srtt_ticks < cubic_data->min_rtt_ticks ||
cubic_data->min_rtt_ticks == TCPTV_SRTTBASE))
cubic_data->min_rtt_ticks == TCPTV_SRTTBASE)) {
cubic_data->min_rtt_ticks = max(1, t_srtt_ticks);
/*
* If the connection is within its first congestion
* epoch, ensure we prime mean_rtt_ticks with a
* reasonable value until the epoch average RTT is
* calculated in cubic_post_recovery().
*/
if (cubic_data->min_rtt_ticks >
cubic_data->mean_rtt_ticks)
cubic_data->mean_rtt_ticks =
cubic_data->min_rtt_ticks;
}
/* Sum samples for epoch average RTT calculation. */
cubic_data->sum_rtt_ticks += t_srtt_ticks;
cubic_data->epoch_ack_count++;