From nobody Wed Dec 14 20:39:37 2022 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4NXS093DF3z4kWFt; Wed, 14 Dec 2022 20:39:37 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4NXS092g0xz46YB; Wed, 14 Dec 2022 20:39:37 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1671050377; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=2M6oNlnzCFJZYPO/yi7Rtdg+a4wBq7kHtiYL0ZcNp30=; b=GeBpRnCugjsxB+zYz4kpNQftsC3U9ujpi9EkxeZ0XkZdZ8HTeUx5Nqu0ubjhv8kWVrY/Fd Q0hK+GGS200E4zG4WJH7b1jJMeNPPvYUIohYDA3V7JxGcwLFwWfeTv6mqLl4JxHKvyJCwq q5as29YJ4CvaFD1jTrzbqn6JqIYREY6r6yvHN3MNPZOhuR0samqDCNh5DOTHtYJij1bdjo 262xPz84IoTYNEfx4B8J/RXDE/jxxpwkFrZ9WxewBoo8L9RS/Nk5KX7zhtEK2u8cE7m4NU p7TD3c8iXtVC71dL2qZkRiy1BbU/nWQdzzFBEvyUy/Msbd1r1QaKyEq20C4K/w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1671050377; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=2M6oNlnzCFJZYPO/yi7Rtdg+a4wBq7kHtiYL0ZcNp30=; b=ab5SQXB1RpMHMO9Fw+ACOPdiCqZjOjEgJmOlsvA+xsrWtvkfm5rjV/P6xV84OOqzr0w2Xs vjB+lfQ9E7dSn8oC2FzAI7eFeY3I9E/7lI+UHU7QD+8un17+6CZDAg1ICe1i5wNExDzTy3 +WykyCDaMHLiD+hpc9NuHvonW4XZ0iDf53NiEUF2qS15Wg1egYDITM9lbpSYFriAwFuVkC ic4DHWWuM1ltECeYHkyqX0rSQZ4RfLclhS1WAdopzEjl7UXe01a4YFEVac/+oA2Eq9gzEf dbQJN5J8XZUfiVKrRZFMAAqGqRRcLw1wn4IG+fuig3Z7INSUcjVuEvXZq+oakA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1671050377; a=rsa-sha256; cv=none; b=HZxz1pOTzBZr3xXb7sWiXXs17In0ScOJ96g0DZsqNU/iIBGiCC9ZV5/oiaaS/TcfzOEadl NGiPuIFbtwHWR9NKUiQr5ZZBsnCNTsVlsMiFBpFnXyOhq3PRqm4seFqncO6/sRhG56Tdjf TPnEYKHVGoXDsWV4oLZ4Q3OyM9LdqV4NPtcptanKBSoKJyWbdf56gcKwOO1AEzMi5Pm425 G0MmnME4OLXn8XgqgVo+LJ39S3hutB+mkyRusBcSRNe/WJRHWiuk2m7/YCna0wBtzIp51a zyGEzxiRRylx+fLcDREdoCongsPnb01v2USZriSr6wA1+576tGqyYpm14EnJJQ== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4NXS091h52z16J8; Wed, 14 Dec 2022 20:39:37 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 2BEKdbNP008419; Wed, 14 Dec 2022 20:39:37 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 2BEKdb4W008415; Wed, 14 Dec 2022 20:39:37 GMT (envelope-from git) Date: Wed, 14 Dec 2022 20:39:37 GMT Message-Id: <202212142039.2BEKdb4W008415@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Randall Stewart Subject: git: e2e088ae86da - main - Rack cannot be loaded without cc_newreno compiled into the kernel. List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: rrs X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: e2e088ae86da4bf5ff5ad7c04bad9d6ce84b1e0e Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by rrs: URL: https://cgit.FreeBSD.org/src/commit/?id=e2e088ae86da4bf5ff5ad7c04bad9d6ce84b1e0e commit e2e088ae86da4bf5ff5ad7c04bad9d6ce84b1e0e Author: Randall Stewart AuthorDate: 2022-12-14 20:37:48 +0000 Commit: Randall Stewart 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