Page MenuHomeFreeBSD
Differential D18954 Diff 53189 sys/netinet/cc/cc_cubic.c

Changeset View

Standalone View

View Options

sys/netinet/cc/cc_cubic.c

Show First 20 Lines • Show All 72 Lines • ▼ Show 20 Lines
static void cubic_cb_destroy(struct cc_var *ccv); static void cubic_cb_destroy(struct cc_var *ccv);
static int cubic_cb_init(struct cc_var *ccv); static int cubic_cb_init(struct cc_var *ccv);
static void cubic_cong_signal(struct cc_var *ccv, uint32_t type); static void cubic_cong_signal(struct cc_var *ccv, uint32_t type);
static void cubic_conn_init(struct cc_var *ccv); static void cubic_conn_init(struct cc_var *ccv);
static int cubic_mod_init(void); static int cubic_mod_init(void);
static void cubic_post_recovery(struct cc_var *ccv); static void cubic_post_recovery(struct cc_var *ccv);
static void cubic_record_rtt(struct cc_var *ccv); static void cubic_record_rtt(struct cc_var *ccv);
static void cubic_ssthresh_update(struct cc_var *ccv); static void cubic_ssthresh_update(struct cc_var *ccv);
static void cubic_after_idle(struct cc_var *ccv);
struct cubic { struct cubic {
/* Cubic K in fixed point form with CUBIC_SHIFT worth of precision. */ /* Cubic K in fixed point form with CUBIC_SHIFT worth of precision. */
int64_t K; int64_t K;
/* Sum of RTT samples across an epoch in ticks. */ /* Sum of RTT samples across an epoch in ticks. */
int64_t sum_rtt_ticks; int64_t sum_rtt_ticks;
/* cwnd at the most recent congestion event. */ /* cwnd at the most recent congestion event. */
unsigned long max_cwnd; unsigned long max_cwnd;
Show All 18 Linesstruct cc_algo cubic_cc_algo = {
.name = "cubic", .name = "cubic",
.ack_received = cubic_ack_received, .ack_received = cubic_ack_received,
.cb_destroy = cubic_cb_destroy, .cb_destroy = cubic_cb_destroy,
.cb_init = cubic_cb_init, .cb_init = cubic_cb_init,
.cong_signal = cubic_cong_signal, .cong_signal = cubic_cong_signal,
.conn_init = cubic_conn_init, .conn_init = cubic_conn_init,
.mod_init = cubic_mod_init, .mod_init = cubic_mod_init,
.post_recovery = cubic_post_recovery, .post_recovery = cubic_post_recovery,
.after_idle = cubic_after_idle,
}; };
static void static void
cubic_ack_received(struct cc_var *ccv, uint16_t type) cubic_ack_received(struct cc_var *ccv, uint16_t type)
{ {
struct cubic *cubic_data; struct cubic *cubic_data;
unsigned long w_tf, w_cubic_next; unsigned long w_tf, w_cubic_next;
int ticks_since_cong; int ticks_since_cong;
Show All 11 Lines if (type == CC_ACK && !IN_RECOVERY(CCV(ccv, t_flags)) &&
CCV(ccv, snd_cwnd) <= CCV(ccv, snd_ssthresh) || CCV(ccv, snd_cwnd) <= CCV(ccv, snd_ssthresh) ||
(V_tcp_do_rfc3465 && ccv->flags & CCF_ABC_SENTAWND))) { (V_tcp_do_rfc3465 && ccv->flags & CCF_ABC_SENTAWND))) {
/* Use the logic in NewReno ack_received() for slow start. */ /* Use the logic in NewReno ack_received() for slow start. */
if (CCV(ccv, snd_cwnd) <= CCV(ccv, snd_ssthresh) || if (CCV(ccv, snd_cwnd) <= CCV(ccv, snd_ssthresh) ||
cubic_data->min_rtt_ticks == TCPTV_SRTTBASE) cubic_data->min_rtt_ticks == TCPTV_SRTTBASE)
newreno_cc_algo.ack_received(ccv, type); newreno_cc_algo.ack_received(ccv, type);
else { else {
ticks_since_cong = ticks - cubic_data->t_last_cong; ticks_since_cong = ticks - cubic_data->t_last_cong;
rscheffAuthorUnsubmitted
Done
Inline Actions

This is unlikely, but not impossible

rscheff: This is unlikely, but not impossible
/* /*
* The mean RTT is used to best reflect the equations in * The mean RTT is used to best reflect the equations in
* the I-D. Using min_rtt in the tf_cwnd calculation * the I-D. Using min_rtt in the tf_cwnd calculation
* causes w_tf to grow much faster than it should if the * causes w_tf to grow much faster than it should if the
* RTT is dominated by network buffering rather than * RTT is dominated by network buffering rather than
ccUnsubmitted
Done
Inline Actions

Is "ticks - INT_MAX <= 0" ?

cc: Is "ticks - INT_MAX <= 0" ?
rscheffAuthorUnsubmitted
Done
Inline Actions

checking the relevant #includes, "ticks" seems to come from sys/kernel.h:71, where it is defined as an int. All other variables in this context are also int.

I assume ticks is somewhere incremented HZ times per second somewhere by ++, without a INT_MAX limits check at 2^31 (e.g. rolls over to INT_MIN).

rscheff: checking the relevant #includes, "ticks" seems to come from sys/kernel.h:71, where it is…
* propagation delay. * propagation delay.
*/ */
w_tf = tf_cwnd(ticks_since_cong, w_tf = tf_cwnd(ticks_since_cong,
cubic_data->mean_rtt_ticks, cubic_data->max_cwnd, cubic_data->mean_rtt_ticks, cubic_data->max_cwnd,
CCV(ccv, t_maxseg)); CCV(ccv, t_maxseg));
w_cubic_next = cubic_cwnd(ticks_since_cong + w_cubic_next = cubic_cwnd(ticks_since_cong +
cubic_data->mean_rtt_ticks, cubic_data->max_cwnd, cubic_data->mean_rtt_ticks, cubic_data->max_cwnd,
CCV(ccv, t_maxseg), cubic_data->K); CCV(ccv, t_maxseg), cubic_data->K);
ccv->flags &= ~CCF_ABC_SENTAWND; ccv->flags &= ~CCF_ABC_SENTAWND;
if (w_cubic_next < w_tf) if (w_cubic_next < w_tf)
/* /*
* TCP-friendly region, follow tf * TCP-friendly region, follow tf
* cwnd growth. * cwnd growth.
*/ */
CCV(ccv, snd_cwnd) = w_tf; CCV(ccv, snd_cwnd) = w_tf;
rscheffAuthorUnsubmitted
Done
Inline Actions

w_tf is sometimes smaller than the existing cwnd. not updating cwnd (shrinking) to avoid transmission stalls

rscheff: w_tf is sometimes smaller than the existing cwnd. not updating cwnd (shrinking) to avoid…
else if (CCV(ccv, snd_cwnd) < w_cubic_next) { else if (CCV(ccv, snd_cwnd) < w_cubic_next) {
/* /*
* Concave or convex region, follow CUBIC * Concave or convex region, follow CUBIC
* cwnd growth. * cwnd growth.
*/ */
if (V_tcp_do_rfc3465) if (V_tcp_do_rfc3465)
CCV(ccv, snd_cwnd) = w_cubic_next; CCV(ccv, snd_cwnd) = w_cubic_next;
Show All 13 Lines else {
*/ */
if (cubic_data->num_cong_events == 0 && if (cubic_data->num_cong_events == 0 &&
cubic_data->max_cwnd < CCV(ccv, snd_cwnd)) cubic_data->max_cwnd < CCV(ccv, snd_cwnd))
cubic_data->max_cwnd = CCV(ccv, snd_cwnd); cubic_data->max_cwnd = CCV(ccv, snd_cwnd);
} }
} }
} }
/*
* This is a Cubic specific implementation of after_idle.
* - Reset cwnd by calling New Reno implementation of after_idle.
* - Reset t_last_cong.
*/
static void static void
cubic_after_idle(struct cc_var *ccv)
{
struct cubic *cubic_data;
cubic_data = ccv->cc_data;
/*
* Set cwnd to restart window using newreno code
*/
newreno_cc_algo.after_idle(ccv);
tuexenUnsubmitted
Done
Inline Actions

Doesn't this require the NewReno module to be loaded? I would write the explicit code here.

tuexen: Doesn't this require the NewReno module to be loaded? I would write the explicit code here.
rscheffAuthorUnsubmitted
Done
Inline Actions

True.

However NewReno is the fallback for actually all modular CCs (e.g. cc_hd, cc_htcp, cc_dctcp, cc_cdg all have direct calls out to NewReno).

Also see line 141 above.

> suggest an additional change, to put a "MODULE_DEPEND" for cc_newreno at the end of all these cc's?

rscheff: True. However NewReno is the fallback for actually all modular CCs (e.g. cc_hd, cc_htcp…
tuexenUnsubmitted
Done
Inline Actions

No, then it's fine. Just keep it as it is.

tuexen: No, then it's fine. Just keep it as it is.
ccUnsubmitted
Not Done
Inline Actions

Do we have a change to reset ssthresh to max(ssthresh, 3/4 * cwnd) before the reset of cwnd for Newreno and all?

Like the Linux did here:
https://github.com/torvalds/linux/blob/master/net/ipv4/tcp_output.c#L137

At least in Linux, it follows RFC2861 and worked for 14 years since git history can be tracked in Linux-2.6.12-rc2, 2005.

cc: Do we have a change to reset ssthresh to max(ssthresh, 3/4 * cwnd) before the reset of cwnd for…
rscheffAuthorUnsubmitted
Done
Inline Actions

That is an optimization we should discuss. Consider though: if the session was app- or receiver-limited, without any loss, this will pull down ssthresh to 3/4 the prior cwnd. If there was loss earlier,
ssthresh is likely to be already small (possibly smaller than 3/4 cwnd).

So, once the session resumes after idle, it would exit slow-start earlier, and proceed with congestion avoidance from there on, with a lower likelihood of hitting a drop event to cause slow start to stop.

If I read that piece correct, it's applied always when after_idle, so would fit better in newreno_after_idle.

rscheff: That is an optimization we should discuss. Consider though: if the session was app- or receiver…
rscheffAuthorUnsubmitted
Done
Inline Actions

See D18982

rscheff: See D18982
/*
* reset cubic epoch
*/
cubic_data->t_last_cong = ticks;
}
rscheffAuthorUnsubmitted
Done
Inline Actions

excessive line

rscheff: excessive line
static void
cubic_cb_destroy(struct cc_var *ccv) cubic_cb_destroy(struct cc_var *ccv)
{ {
free(ccv->cc_data, M_CUBIC); free(ccv->cc_data, M_CUBIC);
} }
static int static int
cubic_cb_init(struct cc_var *ccv) cubic_cb_init(struct cc_var *ccv)
{ {
▲ Show 20 Lines • Show All 78 Lines • ▼ Show 20 Linescubic_conn_init(struct cc_var *ccv)
* get used. * get used.
*/ */
cubic_data->max_cwnd = CCV(ccv, snd_cwnd); cubic_data->max_cwnd = CCV(ccv, snd_cwnd);
} }
static int static int
cubic_mod_init(void) cubic_mod_init(void)
{ {
cubic_cc_algo.after_idle = newreno_cc_algo.after_idle;
return (0); return (0);
} }
/* /*
* Perform any necessary tasks before we exit congestion recovery. * Perform any necessary tasks before we exit congestion recovery.
*/ */
static void static void
cubic_post_recovery(struct cc_var *ccv) cubic_post_recovery(struct cc_var *ccv)
▲ Show 20 Lines • Show All 101 Lines • ▼ Show 20 Linescubic_ssthresh_update(struct cc_var *ccv)
* subsequent congestion events, set it to cwnd * beta. * subsequent congestion events, set it to cwnd * beta.
*/ */
if (cubic_data->num_cong_events == 0) if (cubic_data->num_cong_events == 0)
CCV(ccv, snd_ssthresh) = CCV(ccv, snd_cwnd) >> 1; CCV(ccv, snd_ssthresh) = CCV(ccv, snd_cwnd) >> 1;
else else
CCV(ccv, snd_ssthresh) = ((u_long)CCV(ccv, snd_cwnd) * CCV(ccv, snd_ssthresh) = ((u_long)CCV(ccv, snd_cwnd) *
CUBIC_BETA) >> CUBIC_SHIFT; CUBIC_BETA) >> CUBIC_SHIFT;
} }
rscheffAuthorUnsubmitted
Done
Inline Actions

missing newline

rscheff: missing newline
DECLARE_CC_MODULE(cubic, &cubic_cc_algo); DECLARE_CC_MODULE(cubic, &cubic_cc_algo);