Address two incorrect calculations and enhance readability of PRR code

- address second instance of cwnd potentially becoming zero
- fix sublte bug due to implicit int to uint typecase in max()
- fix bug due to typo in hand-coded CEILING() function by using howmany() macro
- use int instead of long, and add a missing long typecast
- replace if conditionals with easier to read imax/imin (as in pseudocode)

Reviewed By: #transport, kbowling
MFC after: 3 days
Sponsored by: NetApp, Inc.
Differential Revision: https://reviews.freebsd.org/D28813
This commit is contained in:
Richard Scheffenegger 2021-02-25 17:59:45 +01:00
parent 8f3c71c85e
commit 48396dc779

View File

@ -2570,8 +2570,8 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th, struct socket *so,
if (V_tcp_do_prr &&
IN_FASTRECOVERY(tp->t_flags) &&
(tp->t_flags & TF_SACK_PERMIT)) {
long snd_cnt = 0, limit = 0;
long del_data = 0, pipe = 0;
int snd_cnt = 0, limit = 0;
int del_data = 0, pipe = 0;
/*
* In a duplicate ACK del_data is only the
* diff_in_sack. If no SACK is used del_data
@ -2588,39 +2588,29 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th, struct socket *so,
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) +
1 - tp->sackhint.sack_bytes_rexmit;
imax(1, tp->snd_nxt - tp->snd_una);
snd_cnt = howmany((long)tp->sackhint.prr_delivered *
tp->snd_ssthresh, tp->sackhint.recover_fs) -
tp->sackhint.sack_bytes_rexmit;
} else {
if (V_tcp_do_prr_conservative)
limit = tp->sackhint.prr_delivered -
tp->sackhint.sack_bytes_rexmit;
else
if ((tp->sackhint.prr_delivered -
tp->sackhint.sack_bytes_rexmit) >
del_data)
limit = tp->sackhint.prr_delivered -
tp->sackhint.sack_bytes_rexmit +
maxseg;
else
limit = del_data + maxseg;
if ((tp->snd_ssthresh - pipe) < limit)
snd_cnt = tp->snd_ssthresh - pipe;
else
snd_cnt = limit;
limit = imax(tp->sackhint.prr_delivered -
tp->sackhint.sack_bytes_rexmit,
del_data) + maxseg;
snd_cnt = imin(tp->snd_ssthresh - pipe, limit);
}
snd_cnt = max((snd_cnt / maxseg), 0);
snd_cnt = imax(snd_cnt, 0) / maxseg;
/*
* Send snd_cnt new data into the network in
* response to this ACK. If there is a going
* to be a SACK retransmission, adjust snd_cwnd
* accordingly.
*/
tp->snd_cwnd = tp->snd_nxt - tp->snd_recover +
tp->sackhint.sack_bytes_rexmit +
(snd_cnt * maxseg);
tp->snd_cwnd = imax(maxseg, tp->snd_nxt - tp->snd_recover +
tp->sackhint.sack_bytes_rexmit + (snd_cnt * maxseg));
} else if ((tp->t_flags & TF_SACK_PERMIT) &&
IN_FASTRECOVERY(tp->t_flags)) {
int awnd;
@ -3948,7 +3938,7 @@ tcp_mssopt(struct in_conninfo *inc)
void
tcp_prr_partialack(struct tcpcb *tp, struct tcphdr *th)
{
long snd_cnt = 0, limit = 0, del_data = 0, pipe = 0;
int snd_cnt = 0, limit = 0, del_data = 0, pipe = 0;
int maxseg = tcp_maxseg(tp);
INP_WLOCK_ASSERT(tp->t_inpcb);
@ -3974,29 +3964,27 @@ tcp_prr_partialack(struct tcpcb *tp, struct tcphdr *th)
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) - tp->sackhint.sack_bytes_rexmit;
imax(1, tp->snd_nxt - tp->snd_una);
snd_cnt = howmany((long)tp->sackhint.prr_delivered *
tp->snd_ssthresh, tp->sackhint.recover_fs) -
tp->sackhint.sack_bytes_rexmit;
} else {
if (V_tcp_do_prr_conservative)
limit = tp->sackhint.prr_delivered -
tp->sackhint.sack_bytes_rexmit;
else
if ((tp->sackhint.prr_delivered -
tp->sackhint.sack_bytes_rexmit) > del_data)
limit = tp->sackhint.prr_delivered -
tp->sackhint.sack_bytes_rexmit + maxseg;
else
limit = del_data + maxseg;
snd_cnt = min((tp->snd_ssthresh - pipe), limit);
limit = imax(tp->sackhint.prr_delivered -
tp->sackhint.sack_bytes_rexmit,
del_data) + maxseg;
snd_cnt = imin((tp->snd_ssthresh - pipe), limit);
}
snd_cnt = max((snd_cnt / maxseg), 0);
snd_cnt = imax(snd_cnt, 0) / maxseg;
/*
* Send snd_cnt new data into the network in response to this ack.
* If there is going to be a SACK retransmission, adjust snd_cwnd
* accordingly.
*/
tp->snd_cwnd = max(maxseg, (int64_t)tp->snd_nxt - tp->snd_recover +
tp->snd_cwnd = imax(maxseg, tp->snd_nxt - tp->snd_recover +
tp->sackhint.sack_bytes_rexmit + (snd_cnt * maxseg));
tp->t_flags |= TF_ACKNOW;
(void) tcp_output(tp);