Date: Fri, 19 May 2023 15:17:44 GMT From: Randall Stewart <rrs@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: ec6d620b197e - main - There are congestion control algorithms will that pull in srtt, and this can cause issues with rack. Message-ID: <202305191517.34JFHix2009019@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by rrs: URL: https://cgit.FreeBSD.org/src/commit/?id=ec6d620b197e0973102db134a59e34efcc4bbec8 commit ec6d620b197e0973102db134a59e34efcc4bbec8 Author: Randall Stewart <rrs@FreeBSD.org> AuthorDate: 2023-05-19 15:16:28 +0000 Commit: Randall Stewart <rrs@FreeBSD.org> CommitDate: 2023-05-19 15:16:28 +0000 There are congestion control algorithms will that pull in srtt, and this can cause issues with rack. When using rack, cubic and htcp will grab the srtt, but they think it is in ticks. For rack it is in micro-seconds (which we should probably move all stacks to actually). This causes issues so instead lets make a new interface so that any CC module can pull the srtt in whatever granularity they want. Reviewed by: tuexen Sponsored by: Netflix Inc Differential Revision:https://reviews.freebsd.org/D40146 --- sys/netinet/cc/cc_cubic.c | 4 ++-- sys/netinet/cc/cc_htcp.c | 10 +++++----- sys/netinet/tcp_subr.c | 30 +++++++++++++++++++++++++++++- sys/netinet/tcp_var.h | 1 + 4 files changed, 37 insertions(+), 8 deletions(-) diff --git a/sys/netinet/cc/cc_cubic.c b/sys/netinet/cc/cc_cubic.c index 71bd51da8d0c..8992b9beba13 100644 --- a/sys/netinet/cc/cc_cubic.c +++ b/sys/netinet/cc/cc_cubic.c @@ -570,8 +570,8 @@ cubic_record_rtt(struct cc_var *ccv) /* Ignore srtt until a min number of samples have been taken. */ if (CCV(ccv, t_rttupdated) >= CUBIC_MIN_RTT_SAMPLES) { cubic_data = ccv->cc_data; - t_srtt_ticks = CCV(ccv, t_srtt) / TCP_RTT_SCALE; - + t_srtt_ticks = tcp_get_srtt(ccv->ccvc.tcp, + TCP_TMR_GRANULARITY_TICKS); /* * Record the current SRTT as our minrtt if it's the smallest * we've seen or minrtt is currently equal to its initialised diff --git a/sys/netinet/cc/cc_htcp.c b/sys/netinet/cc/cc_htcp.c index 4dd612e2cd7d..ea0f14ed12a8 100644 --- a/sys/netinet/cc/cc_htcp.c +++ b/sys/netinet/cc/cc_htcp.c @@ -444,7 +444,7 @@ htcp_recalc_alpha(struct cc_var *ccv) */ if (V_htcp_rtt_scaling) alpha = max(1, (min(max(HTCP_MINROWE, - (CCV(ccv, t_srtt) << HTCP_SHIFT) / + (tcp_get_srtt(ccv->ccvc.tcp, TCP_TMR_GRANULARITY_TICKS) << HTCP_SHIFT) / htcp_rtt_ref), HTCP_MAXROWE) * alpha) >> HTCP_SHIFT); @@ -495,18 +495,18 @@ htcp_record_rtt(struct cc_var *ccv) * or minrtt is currently equal to its initialised value. Ignore SRTT * until a min number of samples have been taken. */ - if ((CCV(ccv, t_srtt) < htcp_data->minrtt || + if ((tcp_get_srtt(ccv->ccvc.tcp, TCP_TMR_GRANULARITY_TICKS) < htcp_data->minrtt || htcp_data->minrtt == TCPTV_SRTTBASE) && (CCV(ccv, t_rttupdated) >= HTCP_MIN_RTT_SAMPLES)) - htcp_data->minrtt = CCV(ccv, t_srtt); + htcp_data->minrtt = tcp_get_srtt(ccv->ccvc.tcp, TCP_TMR_GRANULARITY_TICKS); /* * Record the current SRTT as our maxrtt if it's the largest we've * seen. Ignore SRTT until a min number of samples have been taken. */ - if (CCV(ccv, t_srtt) > htcp_data->maxrtt + if (tcp_get_srtt(ccv->ccvc.tcp, TCP_TMR_GRANULARITY_TICKS) > htcp_data->maxrtt && CCV(ccv, t_rttupdated) >= HTCP_MIN_RTT_SAMPLES) - htcp_data->maxrtt = CCV(ccv, t_srtt); + htcp_data->maxrtt = tcp_get_srtt(ccv->ccvc.tcp, TCP_TMR_GRANULARITY_TICKS); } /* diff --git a/sys/netinet/tcp_subr.c b/sys/netinet/tcp_subr.c index a8a896b7ebe6..db0b3b76088e 100644 --- a/sys/netinet/tcp_subr.c +++ b/sys/netinet/tcp_subr.c @@ -4316,7 +4316,7 @@ tcp_http_log_req_info(struct tcpcb *tp, struct http_sendfile_track *http, memset(&log.u_bbr, 0, sizeof(log.u_bbr)); #ifdef TCPHPTS - log.u_bbr.inhpts = tcp_in_hpts(tptoinpcb(tp)); + log.u_bbr.inhpts = tcp_in_hpts(tp); #endif log.u_bbr.flex8 = val; log.u_bbr.rttProp = http->timestamp; @@ -4641,3 +4641,31 @@ tcp_log_socket_option(struct tcpcb *tp, uint32_t option_num, uint32_t option_val } } } + +uint32_t +tcp_get_srtt(struct tcpcb *tp, int granularity) +{ + uint32_t srtt; + + if (tp->t_tmr_granularity == TCP_TMR_GRANULARITY_USEC) + srtt = tp->t_srtt; + else if (tp->t_tmr_granularity == TCP_TMR_GRANULARITY_TICKS) + srtt = tp->t_srtt >> TCP_RTT_SHIFT; + if (tp->t_tmr_granularity == granularity) + return (srtt); + /* If we reach here they are oppsite what the caller wants */ + if (granularity == TCP_TMR_GRANULARITY_USEC) { + /* + * The user wants useconds and internally + * its kept in ticks, convert to useconds. + */ + srtt = TICKS_2_USEC(srtt); + } else if (granularity == TCP_TMR_GRANULARITY_TICKS) { + /* + * The user wants ticks and internally its + * kept in useconds, convert to ticks. + */ + srtt = USEC_2_TICKS(srtt); + } + return (srtt); +} diff --git a/sys/netinet/tcp_var.h b/sys/netinet/tcp_var.h index a8bd6f8732cc..680797e508a2 100644 --- a/sys/netinet/tcp_var.h +++ b/sys/netinet/tcp_var.h @@ -1400,6 +1400,7 @@ int deregister_tcp_functions(struct tcp_function_block *blk, bool quiesce, bool force); struct tcp_function_block *find_and_ref_tcp_functions(struct tcp_function_set *fs); int find_tcp_function_alias(struct tcp_function_block *blk, struct tcp_function_set *fs); +uint32_t tcp_get_srtt(struct tcpcb *tp, int granularity); void tcp_switch_back_to_default(struct tcpcb *tp); struct tcp_function_block * find_and_ref_tcp_fb(struct tcp_function_block *fs);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202305191517.34JFHix2009019>