Date: Wed, 20 Jan 2021 11:21:35 GMT From: Richard Scheffenegger <rscheff@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: bc7ee8e5bc55 - main - Address panic with PRR due to missed initialization of recover_fs Message-ID: <202101201121.10KBLZE7053425@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by rscheff: URL: https://cgit.FreeBSD.org/src/commit/?id=bc7ee8e5bc555c246bad8bbb9cdf964fa0a08f41 commit bc7ee8e5bc555c246bad8bbb9cdf964fa0a08f41 Author: Richard Scheffenegger <srichard@netapp.com> AuthorDate: 2021-01-20 11:06:34 +0000 Commit: Richard Scheffenegger <rscheff@FreeBSD.org> CommitDate: 2021-01-20 11:06:34 +0000 Address panic with PRR due to missed initialization of recover_fs Summary: When using the base stack in conjunction with RACK, it appears that infrequently, ++tp->t_dupacks is instantly larger than tcprexmtthresh. This leaves the recover flightsize (sackhint.recover_fs) uninitialized, leading to a div/0 panic. Address this by properly initializing the variable just prior to first use, if it is not properly initialized. In order to prevent stale information from a prior recovery to negatively impact the PRR calculations in this event, also clear recover_fs once loss recovery is finished. Finally, improve the readability of the initialization of recover_fs when t_dupacks == tcprexmtthresh by adjusting the indentation and using the max(1, snd_nxt - snd_una) macro. Reviewers: rrs, kbowling, tuexen, jtl, #transport, gnn!, jmg, manu, #manpages Reviewed By: rrs, kbowling, #transport Subscribers: bdrewery, andrew, rpokala, ae, emaste, bz, bcran, #linuxkpi, imp, melifaro Differential Revision: https://reviews.freebsd.org/D28114 --- sys/netinet/tcp_input.c | 20 ++++++++++++-------- sys/netinet/tcp_subr.c | 10 ++++++++++ 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/sys/netinet/tcp_input.c b/sys/netinet/tcp_input.c index 283d42b594bd..75718352da00 100644 --- a/sys/netinet/tcp_input.c +++ b/sys/netinet/tcp_input.c @@ -510,6 +510,7 @@ cc_post_recovery(struct tcpcb *tp, struct tcphdr *th) } /* XXXLAS: EXIT_RECOVERY ? */ tp->t_bytes_acked = 0; + tp->sackhint.recover_fs = 0; } /* @@ -2590,6 +2591,9 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th, struct socket *so, tp->sackhint.sack_bytes_rexmit; tp->sackhint.prr_delivered += del_data; if (pipe > tp->snd_ssthresh) { + if (tp->sackhint.recover_fs == 0) + tp->sackhint.recover_fs = + max(1, tp->snd_nxt - tp->snd_una); snd_cnt = (tp->sackhint.prr_delivered * tp->snd_ssthresh / tp->sackhint.recover_fs) + @@ -2677,14 +2681,14 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th, struct socket *so, tcp_timer_activate(tp, TT_REXMT, 0); tp->t_rtttime = 0; if (V_tcp_do_prr) { - /* - * snd_ssthresh is already updated by - * cc_cong_signal. - */ - tp->sackhint.prr_delivered = 0; - tp->sackhint.sack_bytes_rexmit = 0; - if (!(tp->sackhint.recover_fs = tp->snd_nxt - tp->snd_una)) - tp->sackhint.recover_fs = 1; + /* + * snd_ssthresh is already updated by + * cc_cong_signal. + */ + tp->sackhint.prr_delivered = 0; + tp->sackhint.sack_bytes_rexmit = 0; + tp->sackhint.recover_fs = max(1, + tp->snd_nxt - tp->snd_una); } if (tp->t_flags & TF_SACK_PERMIT) { TCPSTAT_INC( diff --git a/sys/netinet/tcp_subr.c b/sys/netinet/tcp_subr.c index b39d2b43d3a2..c49ff680d201 100644 --- a/sys/netinet/tcp_subr.c +++ b/sys/netinet/tcp_subr.c @@ -742,6 +742,16 @@ tcp_default_fb_init(struct tcpcb *tp) TCPS_HAVEESTABLISHED(tp->t_state) ? TP_KEEPIDLE(tp) : TP_KEEPINIT(tp)); + /* + * Make sure critical variables are initialized + * if transitioning while in Recovery. + */ + if IN_FASTRECOVERY(tp->t_flags) { + if (tp->sackhint.recover_fs == 0) + tp->sackhint.recover_fs = max(1, + tp->snd_nxt - tp->snd_una); + } + return (0); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202101201121.10KBLZE7053425>