git: e2e088ae86da - main - Rack cannot be loaded without cc_newreno compiled into the kernel.
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Wed, 14 Dec 2022 20:39:37 UTC
The branch main has been updated by rrs:
URL: https://cgit.FreeBSD.org/src/commit/?id=e2e088ae86da4bf5ff5ad7c04bad9d6ce84b1e0e
commit e2e088ae86da4bf5ff5ad7c04bad9d6ce84b1e0e
Author: Randall Stewart <rrs@FreeBSD.org>
AuthorDate: 2022-12-14 20:37:48 +0000
Commit: Randall Stewart <rrs@FreeBSD.org>
CommitDate: 2022-12-14 20:37:48 +0000
Rack cannot be loaded without cc_newreno compiled into the kernel.
Right now rack will fail to load due to its hack in accessing symbol names
in cc_newreno. This was fine when newreno was always compiled into the
kernel but now ... not so much. Instead lets fix up rack to use the socket
option queries to get the information it wants and set the parameters. We
also fix the CC parameter so they are always settable.
Reviewed by: tuexen
Sponsored by: Netflix Inc
Differential Revision: https://reviews.freebsd.org/D37622
---
sys/netinet/cc/cc_newreno.c | 8 +--
sys/netinet/tcp_stacks/rack.c | 135 +++++++++++++++++-------------------------
2 files changed, 56 insertions(+), 87 deletions(-)
diff --git a/sys/netinet/cc/cc_newreno.c b/sys/netinet/cc/cc_newreno.c
index 7118e6432707..fb5ffed43a15 100644
--- a/sys/netinet/cc/cc_newreno.c
+++ b/sys/netinet/cc/cc_newreno.c
@@ -460,8 +460,6 @@ newreno_ctl_output(struct cc_var *ccv, struct sockopt *sopt, void *buf)
nreno->beta = opt->val;
break;
case CC_NEWRENO_BETA_ECN:
- if ((!V_cc_do_abe) && ((nreno->newreno_flags & CC_NEWRENO_BETA_ECN) == 0))
- return (EACCES);
nreno->beta_ecn = opt->val;
nreno->newreno_flags |= CC_NEWRENO_BETA_ECN_ENABLED;
break;
@@ -472,12 +470,10 @@ newreno_ctl_output(struct cc_var *ccv, struct sockopt *sopt, void *buf)
case SOPT_GET:
switch (opt->name) {
case CC_NEWRENO_BETA:
- opt->val = (nreno == NULL) ?
- V_newreno_beta : nreno->beta;
+ opt->val = nreno->beta;
break;
case CC_NEWRENO_BETA_ECN:
- opt->val = (nreno == NULL) ?
- V_newreno_beta_ecn : nreno->beta_ecn;
+ opt->val = nreno->beta_ecn;
break;
default:
return (ENOPROTOOPT);
diff --git a/sys/netinet/tcp_stacks/rack.c b/sys/netinet/tcp_stacks/rack.c
index 0aa3d0910562..7ef40350632d 100644
--- a/sys/netinet/tcp_stacks/rack.c
+++ b/sys/netinet/tcp_stacks/rack.c
@@ -566,16 +566,13 @@ rack_trace_point(struct tcp_rack *rack, int num)
}
static void
-rack_set_cc_pacing(struct tcp_rack *rack)
+rack_swap_beta_values(struct tcp_rack *rack, uint8_t flex8)
{
struct sockopt sopt;
struct cc_newreno_opts opt;
- struct newreno old, *ptr;
+ struct newreno old;
struct tcpcb *tp;
- int error;
-
- if (rack->rc_pacing_cc_set)
- return;
+ int error, failed = 0;
tp = rack->rc_tp;
if (tp->t_cc == NULL) {
@@ -585,128 +582,104 @@ rack_set_cc_pacing(struct tcp_rack *rack)
rack->rc_pacing_cc_set = 1;
if (strcmp(tp->t_cc->name, CCALGONAME_NEWRENO) != 0) {
/* Not new-reno we can't play games with beta! */
+ failed = 1;
goto out;
+
}
- ptr = ((struct newreno *)tp->t_ccv.cc_data);
if (CC_ALGO(tp)->ctl_output == NULL) {
- /* Huh, why does new_reno no longer have a set function? */
+ /* Huh, not using new-reno so no swaps.? */
+ failed = 2;
goto out;
}
- if (ptr == NULL) {
- /* Just the default values */
- old.beta = V_newreno_beta_ecn;
- old.beta_ecn = V_newreno_beta_ecn;
- old.newreno_flags = 0;
- } else {
- old.beta = ptr->beta;
- old.beta_ecn = ptr->beta_ecn;
- old.newreno_flags = ptr->newreno_flags;
- }
+ /* Get the current values out */
sopt.sopt_valsize = sizeof(struct cc_newreno_opts);
+ sopt.sopt_dir = SOPT_GET;
+ opt.name = CC_NEWRENO_BETA;
+ error = CC_ALGO(tp)->ctl_output(&tp->t_ccv, &sopt, &opt);
+ if (error) {
+ failed = 3;
+ goto out;
+ }
+ old.beta = opt.val;
+ opt.name = CC_NEWRENO_BETA_ECN;
+ error = CC_ALGO(tp)->ctl_output(&tp->t_ccv, &sopt, &opt);
+ if (error) {
+ failed = 4;
+ goto out;
+ }
+ old.beta_ecn = opt.val;
+
+ /* Now lets set in the values we have stored */
sopt.sopt_dir = SOPT_SET;
opt.name = CC_NEWRENO_BETA;
opt.val = rack->r_ctl.rc_saved_beta.beta;
error = CC_ALGO(tp)->ctl_output(&tp->t_ccv, &sopt, &opt);
if (error) {
+ failed = 5;
goto out;
}
- /*
- * Hack alert we need to set in our newreno_flags
- * so that Abe behavior is also applied.
- */
- ((struct newreno *)tp->t_ccv.cc_data)->newreno_flags |= CC_NEWRENO_BETA_ECN_ENABLED;
opt.name = CC_NEWRENO_BETA_ECN;
opt.val = rack->r_ctl.rc_saved_beta.beta_ecn;
error = CC_ALGO(tp)->ctl_output(&tp->t_ccv, &sopt, &opt);
if (error) {
+ failed = 6;
goto out;
}
- /* Save off the original values for restoral */
+ /* Save off the values for restoral */
memcpy(&rack->r_ctl.rc_saved_beta, &old, sizeof(struct newreno));
out:
if (rack_verbose_logging && (rack->rc_tp->t_logstate != TCP_LOG_STATE_OFF)) {
union tcp_log_stackspecific log;
struct timeval tv;
+ struct newreno *ptr;
ptr = ((struct newreno *)tp->t_ccv.cc_data);
memset(&log.u_bbr, 0, sizeof(log.u_bbr));
log.u_bbr.timeStamp = tcp_get_usecs(&tv);
- if (ptr) {
- log.u_bbr.flex1 = ptr->beta;
- log.u_bbr.flex2 = ptr->beta_ecn;
- log.u_bbr.flex3 = ptr->newreno_flags;
- }
+ printf("Digging into the cc mod beta:%d beta_ecn:%d\n",
+ ptr->beta, ptr->beta_ecn);
+ log.u_bbr.flex1 = ptr->beta;
+ log.u_bbr.flex2 = ptr->beta_ecn;
+ log.u_bbr.flex3 = ptr->newreno_flags;
log.u_bbr.flex4 = rack->r_ctl.rc_saved_beta.beta;
log.u_bbr.flex5 = rack->r_ctl.rc_saved_beta.beta_ecn;
- log.u_bbr.flex6 = rack->r_ctl.rc_saved_beta.newreno_flags;
+ log.u_bbr.flex6 = failed;
log.u_bbr.flex7 = rack->gp_ready;
log.u_bbr.flex7 <<= 1;
log.u_bbr.flex7 |= rack->use_fixed_rate;
log.u_bbr.flex7 <<= 1;
log.u_bbr.flex7 |= rack->rc_pacing_cc_set;
log.u_bbr.pkts_out = rack->r_ctl.rc_prr_sndcnt;
- log.u_bbr.flex8 = 3;
+ log.u_bbr.flex8 = flex8;
tcp_log_event_(tp, NULL, NULL, NULL, BBR_LOG_CWND, error,
0, &log, false, NULL, NULL, 0, &tv);
}
}
+static void
+rack_set_cc_pacing(struct tcp_rack *rack)
+{
+ if (rack->rc_pacing_cc_set)
+ return;
+ /*
+ * Use the swap utility placing in 3 for flex8 to id a
+ * set of a new set of values.
+ */
+ rack->rc_pacing_cc_set = 1;
+ rack_swap_beta_values(rack, 3);
+}
+
static void
rack_undo_cc_pacing(struct tcp_rack *rack)
{
- struct newreno old, *ptr;
- struct tcpcb *tp;
-
if (rack->rc_pacing_cc_set == 0)
return;
- tp = rack->rc_tp;
+ /*
+ * Use the swap utility placing in 4 for flex8 to id a
+ * restoral of the old values.
+ */
rack->rc_pacing_cc_set = 0;
- if (tp->t_cc == NULL)
- /* Tcb is leaving */
- return;
- if (strcmp(tp->t_cc->name, CCALGONAME_NEWRENO) != 0) {
- /* Not new-reno nothing to do! */
- return;
- }
- ptr = ((struct newreno *)tp->t_ccv.cc_data);
- if (ptr == NULL) {
- /*
- * This happens at rack_fini() if the
- * cc module gets freed on us. In that
- * case we loose our "new" settings but
- * thats ok, since the tcb is going away anyway.
- */
- return;
- }
- /* Grab out our set values */
- memcpy(&old, ptr, sizeof(struct newreno));
- /* Copy back in the original values */
- memcpy(ptr, &rack->r_ctl.rc_saved_beta, sizeof(struct newreno));
- /* Now save back the values we had set in (for when pacing is restored) */
- memcpy(&rack->r_ctl.rc_saved_beta, &old, sizeof(struct newreno));
- if (rack_verbose_logging && (rack->rc_tp->t_logstate != TCP_LOG_STATE_OFF)) {
- union tcp_log_stackspecific log;
- struct timeval tv;
-
- ptr = ((struct newreno *)tp->t_ccv.cc_data);
- memset(&log.u_bbr, 0, sizeof(log.u_bbr));
- log.u_bbr.timeStamp = tcp_get_usecs(&tv);
- log.u_bbr.flex1 = ptr->beta;
- log.u_bbr.flex2 = ptr->beta_ecn;
- log.u_bbr.flex3 = ptr->newreno_flags;
- log.u_bbr.flex4 = rack->r_ctl.rc_saved_beta.beta;
- log.u_bbr.flex5 = rack->r_ctl.rc_saved_beta.beta_ecn;
- log.u_bbr.flex6 = rack->r_ctl.rc_saved_beta.newreno_flags;
- log.u_bbr.flex7 = rack->gp_ready;
- log.u_bbr.flex7 <<= 1;
- log.u_bbr.flex7 |= rack->use_fixed_rate;
- log.u_bbr.flex7 <<= 1;
- log.u_bbr.flex7 |= rack->rc_pacing_cc_set;
- log.u_bbr.pkts_out = rack->r_ctl.rc_prr_sndcnt;
- log.u_bbr.flex8 = 4;
- tcp_log_event_(tp, NULL, NULL, NULL, BBR_LOG_CWND, 0,
- 0, &log, false, NULL, NULL, 0, &tv);
- }
+ rack_swap_beta_values(rack, 4);
}
#ifdef NETFLIX_PEAKRATE