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
This commit is contained in:
Richard Scheffenegger 2021-01-20 12:06:34 +01:00 committed by Richard Scheffenegger
parent cef1942711
commit bc7ee8e5bc
2 changed files with 22 additions and 8 deletions

View file

@ -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(

View file

@ -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);
}