The branch main has been updated by tuexen:
URL:
https://cgit.FreeBSD.org/src/commit/?id=f6deb9ea0a0ee760e5ff9ad5c15d0bd7a1714355
commit f6deb9ea0a0ee760e5ff9ad5c15d0bd7a1714355
Author: Michael Tuexen <tue...@freebsd.org>
AuthorDate: 2025-03-31 01:40:17 +0000
Commit: Michael Tuexen <tue...@freebsd.org>
CommitDate: 2025-03-31 01:40:17 +0000
tcp rack: cleanup storing values for beta and beta_ecn
beta and beta_ecn were stored using a variable of type struct newreno
in struct rack_control. Later, struct newreno was extended and now
contains several more fields.
This results in a memory inefficiency and also in copying around
uninitialized memory.
This patch fixes this by storing beta and beta_ecn individually in
struct rack_control.
Please note that the newreno_flags field was only stored and never
used. Therefore, this is not stored anymore in struct rack_control.
No functional change intended.
CID: 1523796
Reviewed by: rrs
MFC after: 1 week
Sponsored by: Netflix, Inc.
Differential Revision: https://reviews.freebsd.org/D49578
---
sys/netinet/tcp_stacks/rack.c | 45 +++++++++++++++++----------------------
sys/netinet/tcp_stacks/tcp_rack.h | 7 +++---
2 files changed, 24 insertions(+), 28 deletions(-)
diff --git a/sys/netinet/tcp_stacks/rack.c b/sys/netinet/tcp_stacks/rack.c
index 5a9171abeaf5..f5bc435890e7 100644
--- a/sys/netinet/tcp_stacks/rack.c
+++ b/sys/netinet/tcp_stacks/rack.c
@@ -618,8 +618,9 @@ rack_swap_beta_values(struct tcp_rack *rack, uint8_t flex8)
{
struct sockopt sopt;
struct cc_newreno_opts opt;
- struct newreno old;
struct tcpcb *tp;
+ uint32_t old_beta;
+ uint32_t old_beta_ecn;
int error, failed = 0;
tp = rack->rc_tp;
@@ -639,7 +640,6 @@ rack_swap_beta_values(struct tcp_rack *rack, uint8_t flex8)
failed = 2;
goto out;
}
- old.newreno_flags = CC_NEWRENO_BETA_ECN_ENABLED;
/* Get the current values out */
sopt.sopt_valsize = sizeof(struct cc_newreno_opts);
sopt.sopt_dir = SOPT_GET;
@@ -649,33 +649,34 @@ rack_swap_beta_values(struct tcp_rack *rack, uint8_t
flex8)
failed = 3;
goto out;
}
- old.beta = opt.val;
+ 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;
+ 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;
+ opt.val = rack->r_ctl.rc_saved_beta;
error = CC_ALGO(tp)->ctl_output(&tp->t_ccv, &sopt, &opt);
if (error) {
failed = 5;
goto out;
}
opt.name = CC_NEWRENO_BETA_ECN;
- opt.val = rack->r_ctl.rc_saved_beta.beta_ecn;
+ opt.val = rack->r_ctl.rc_saved_beta_ecn;
error = CC_ALGO(tp)->ctl_output(&tp->t_ccv, &sopt, &opt);
if (error) {
failed = 6;
goto out;
}
/* Save off the values for restoral */
- memcpy(&rack->r_ctl.rc_saved_beta, &old, sizeof(struct newreno));
+ rack->r_ctl.rc_saved_beta = old_beta;
+ rack->r_ctl.rc_saved_beta_ecn = old_beta_ecn;
out:
if (rack_verbose_logging && tcp_bblogging_on(rack->rc_tp)) {
union tcp_log_stackspecific log;
@@ -688,8 +689,8 @@ out:
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.flex4 = rack->r_ctl.rc_saved_beta;
+ log.u_bbr.flex5 = rack->r_ctl.rc_saved_beta_ecn;
log.u_bbr.flex6 = failed;
log.u_bbr.flex7 = rack->gp_ready;
log.u_bbr.flex7 <<= 1;
@@ -14608,7 +14609,6 @@ rack_init(struct tcpcb *tp, void **ptr)
rack->r_ctl.rc_split_limit = V_tcp_map_split_limit;
/* We want abe like behavior as well */
- rack->r_ctl.rc_saved_beta.newreno_flags |= CC_NEWRENO_BETA_ECN_ENABLED;
rack->r_ctl.rc_reorder_fade = rack_reorder_fade;
rack->rc_allow_data_af_clo = rack_ignore_data_after_close;
rack->r_ctl.rc_tlp_threshold = rack_tlp_thresh;
@@ -14648,13 +14648,13 @@ rack_init(struct tcpcb *tp, void **ptr)
rack->r_ctl.max_reduction = rack_max_reduce;
rack->rc_force_max_seg = 0;
TAILQ_INIT(&rack->r_ctl.opt_list);
- rack->r_ctl.rc_saved_beta.beta = V_newreno_beta_ecn;
- rack->r_ctl.rc_saved_beta.beta_ecn = V_newreno_beta_ecn;
+ rack->r_ctl.rc_saved_beta = V_newreno_beta_ecn;
+ rack->r_ctl.rc_saved_beta_ecn = V_newreno_beta_ecn;
if (rack_hibeta_setting) {
rack->rack_hibeta = 1;
if ((rack_hibeta_setting >= 50) &&
(rack_hibeta_setting <= 100)) {
- rack->r_ctl.rc_saved_beta.beta = rack_hibeta_setting;
+ rack->r_ctl.rc_saved_beta = rack_hibeta_setting;
rack->r_ctl.saved_hibeta = rack_hibeta_setting;
}
} else {
@@ -22964,7 +22964,7 @@ rack_process_option(struct tcpcb *tp, struct tcp_rack
*rack, int sopt_name,
rack->r_ctl.saved_hibeta = optval;
if (rack->rc_pacing_cc_set)
rack_undo_cc_pacing(rack);
- rack->r_ctl.rc_saved_beta.beta = optval;
+ rack->r_ctl.rc_saved_beta = optval;
}
if (rack->rc_pacing_cc_set == 0)
rack_set_cc_pacing(rack);
@@ -23015,8 +23015,7 @@ rack_process_option(struct tcpcb *tp, struct tcp_rack
*rack, int sopt_name,
* Not pacing yet so set it into our local
* rack pcb storage.
*/
- rack->r_ctl.rc_saved_beta.beta_ecn = optval;
- rack->r_ctl.rc_saved_beta.newreno_flags =
CC_NEWRENO_BETA_ECN_ENABLED;
+ rack->r_ctl.rc_saved_beta_ecn = optval;
}
break;
case TCP_DEFER_OPTIONS:
@@ -23742,7 +23741,7 @@ rack_inherit(struct tcpcb *tp, struct inpcb *parent)
if (src->rack_hibeta != dest->rack_hibeta) {
cnt++;
if (src->rack_hibeta) {
- dest->r_ctl.rc_saved_beta.beta =
src->r_ctl.rc_saved_beta.beta;
+ dest->r_ctl.rc_saved_beta = src->r_ctl.rc_saved_beta;
dest->rack_hibeta = 1;
} else {
dest->rack_hibeta = 0;
@@ -23754,12 +23753,8 @@ rack_inherit(struct tcpcb *tp, struct inpcb *parent)
cnt++;
}
/* TCP_RACK_PACING_BETA_ECN */
- if (dest->r_ctl.rc_saved_beta.beta_ecn !=
src->r_ctl.rc_saved_beta.beta_ecn) {
- dest->r_ctl.rc_saved_beta.beta_ecn =
src->r_ctl.rc_saved_beta.beta_ecn;
- cnt++;
- }
- if (dest->r_ctl.rc_saved_beta.newreno_flags !=
src->r_ctl.rc_saved_beta.newreno_flags) {
- dest->r_ctl.rc_saved_beta.newreno_flags =
src->r_ctl.rc_saved_beta.newreno_flags;
+ if (dest->r_ctl.rc_saved_beta_ecn != src->r_ctl.rc_saved_beta_ecn) {
+ dest->r_ctl.rc_saved_beta_ecn = src->r_ctl.rc_saved_beta_ecn;
cnt++;
}
/* We do not do TCP_DEFER_OPTIONS */
@@ -24437,7 +24432,7 @@ rack_get_sockopt(struct tcpcb *tp, struct sockopt *sopt)
if (strcmp(tp->t_cc->name, CCALGONAME_NEWRENO) != 0)
error = EINVAL;
else if (rack->rc_pacing_cc_set == 0)
- optval = rack->r_ctl.rc_saved_beta.beta;
+ optval = rack->r_ctl.rc_saved_beta;
else {
/*
* Reach out into the CC data and report back what
@@ -24461,7 +24456,7 @@ rack_get_sockopt(struct tcpcb *tp, struct sockopt *sopt)
if (strcmp(tp->t_cc->name, CCALGONAME_NEWRENO) != 0)
error = EINVAL;
else if (rack->rc_pacing_cc_set == 0)
- optval = rack->r_ctl.rc_saved_beta.beta_ecn;
+ optval = rack->r_ctl.rc_saved_beta_ecn;
else {
/*
* Reach out into the CC data and report back what
diff --git a/sys/netinet/tcp_stacks/tcp_rack.h
b/sys/netinet/tcp_stacks/tcp_rack.h
index a6cb0a81bb81..4374594a1d82 100644
--- a/sys/netinet/tcp_stacks/tcp_rack.h
+++ b/sys/netinet/tcp_stacks/tcp_rack.h
@@ -542,9 +542,10 @@ struct rack_control {
uint32_t last_rcv_tstmp_for_rtt;
uint32_t last_time_of_arm_rcv;
uint32_t rto_ssthresh;
- struct newreno rc_saved_beta; /*
- * For newreno cc:
- * rc_saved_cc are the values we have
had
+ uint32_t rc_saved_beta;
+ uint32_t rc_saved_beta_ecn; /*
+ * For newreno cc: rc_saved_beta and
+ * rc_saved_beta_ecn are the values we
have had
* set by the user, if pacing is not
happening
* (i.e. its early and we have not
turned on yet
* or it was turned off). The minute
pacing
