git: 2d05a1c81b2c - main - tcp: commonize check for more data to send, style changes
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Fri, 26 Jan 2024 00:22:03 UTC
The branch main has been updated by rscheff:
URL: https://cgit.FreeBSD.org/src/commit/?id=2d05a1c81b2cbb5468a242d0add44f850aa31811
commit 2d05a1c81b2cbb5468a242d0add44f850aa31811
Author: Richard Scheffenegger <rscheff@FreeBSD.org>
AuthorDate: 2024-01-25 23:19:30 +0000
Commit: Richard Scheffenegger <rscheff@FreeBSD.org>
CommitDate: 2024-01-26 00:20:35 +0000
tcp: commonize check for more data to send, style changes
Use SEQ_SUB instead of a plain subtraction, for an implict
type conversion and prevention of a possible overflow.
Use curly brackets in stacked if statements throughout.
Use of the ? operator to enhance readability when clearing
the FIN flag in tcp_output().
None of the above change the function.
Reviewed By: tuexen, cc, #transport
Sponsored by: NetApp, Inc.
Differential Revision: https://reviews.freebsd.org/D43539
---
sys/netinet/tcp_ecn.c | 9 ++---
sys/netinet/tcp_input.c | 94 ++++++++++++++++++++++++++++--------------------
sys/netinet/tcp_output.c | 12 +++----
sys/netinet/tcp_sack.c | 31 +++++++++-------
4 files changed, 83 insertions(+), 63 deletions(-)
diff --git a/sys/netinet/tcp_ecn.c b/sys/netinet/tcp_ecn.c
index 34ecfe1c83c0..5332b3caf950 100644
--- a/sys/netinet/tcp_ecn.c
+++ b/sys/netinet/tcp_ecn.c
@@ -135,9 +135,11 @@ tcp_ecn_input_syn_sent(struct tcpcb *tp, uint16_t thflags, int iptos)
case 3:
/* FALLTHROUGH */
case 4:
- /* decoding Accurate ECN according to
+ /*
+ * Decoding Accurate ECN according to
* table in section 3.1.1
- * on the SYN,ACK, process the AccECN
+ *
+ * On the SYN,ACK, process the AccECN
* flags indicating the state the SYN
* was delivered.
* Reactions to Path ECN mangling can
@@ -382,8 +384,7 @@ tcp_ecn_output_syn_sent(struct tcpcb *tp)
thflags = TH_ECE|TH_CWR;
} else
thflags = TH_ECE|TH_CWR;
- } else
- if (V_tcp_do_ecn == 3) {
+ } else if (V_tcp_do_ecn == 3) {
/* Send an Accurate ECN setup <SYN> packet */
if (tp->t_rxtshift >= 1) {
if (tp->t_rxtshift <= V_tcp_ecn_maxretries)
diff --git a/sys/netinet/tcp_input.c b/sys/netinet/tcp_input.c
index 02b042fd3273..98564ff67f22 100644
--- a/sys/netinet/tcp_input.c
+++ b/sys/netinet/tcp_input.c
@@ -1624,13 +1624,14 @@ tcp_do_segment(struct tcpcb *tp, struct mbuf *m, struct tcphdr *th,
*/
if ((to.to_flags & TOF_TS) && (to.to_tsecr != 0)) {
to.to_tsecr -= tp->ts_offset;
- if (TSTMP_GT(to.to_tsecr, tcp_ts_getticks()))
+ if (TSTMP_GT(to.to_tsecr, tcp_ts_getticks())) {
to.to_tsecr = 0;
- else if (tp->t_rxtshift == 1 &&
+ } else if (tp->t_rxtshift == 1 &&
tp->t_flags & TF_PREVVALID &&
tp->t_badrxtwin != 0 &&
- TSTMP_LT(to.to_tsecr, tp->t_badrxtwin))
+ TSTMP_LT(to.to_tsecr, tp->t_badrxtwin)) {
cc_cong_signal(tp, th, CC_RTO_ERR);
+ }
}
/*
* Process options only when we get SYN/ACK back. The SYN case
@@ -1647,8 +1648,9 @@ tcp_do_segment(struct tcpcb *tp, struct mbuf *m, struct tcphdr *th,
!(tp->t_flags & TF_NOOPT)) {
tp->t_flags |= TF_RCVD_SCALE;
tp->snd_scale = to.to_wscale;
- } else
+ } else {
tp->t_flags &= ~TF_REQ_SCALE;
+ }
/*
* Initial send window. It will be updated with
* the next incoming segment to the scaled value.
@@ -1660,30 +1662,36 @@ tcp_do_segment(struct tcpcb *tp, struct mbuf *m, struct tcphdr *th,
tp->t_flags |= TF_RCVD_TSTMP;
tp->ts_recent = to.to_tsval;
tp->ts_recent_age = tcp_ts_getticks();
- } else
+ } else {
tp->t_flags &= ~TF_REQ_TSTMP;
- if (to.to_flags & TOF_MSS)
+ }
+ if (to.to_flags & TOF_MSS) {
tcp_mss(tp, to.to_mss);
+ }
if ((tp->t_flags & TF_SACK_PERMIT) &&
(!(to.to_flags & TOF_SACKPERM) ||
- (tp->t_flags & TF_NOOPT)))
+ (tp->t_flags & TF_NOOPT))) {
tp->t_flags &= ~TF_SACK_PERMIT;
+ }
if (IS_FASTOPEN(tp->t_flags)) {
if ((to.to_flags & TOF_FASTOPEN) &&
!(tp->t_flags & TF_NOOPT)) {
uint16_t mss;
- if (to.to_flags & TOF_MSS)
+ if (to.to_flags & TOF_MSS) {
mss = to.to_mss;
- else
- if ((inp->inp_vflag & INP_IPV6) != 0)
+ } else {
+ if ((inp->inp_vflag & INP_IPV6) != 0) {
mss = TCP6_MSS;
- else
+ } else {
mss = TCP_MSS;
+ }
+ }
tcp_fastopen_update_cache(tp, mss,
to.to_tfo_len, to.to_tfo_cookie);
- } else
+ } else {
tcp_fastopen_disable_path(tp);
+ }
}
}
@@ -1872,9 +1880,11 @@ tcp_do_segment(struct tcpcb *tp, struct mbuf *m, struct tcphdr *th,
* is new data available to be sent
* or we need to send an ACK.
*/
- if (SEQ_GT(tp->snd_una + sbavail(&so->so_snd),
- tp->snd_max) || tp->t_flags & TF_ACKNOW)
+ if ((tp->t_flags & TF_ACKNOW) ||
+ (sbavail(&so->so_snd) >=
+ SEQ_SUB(tp->snd_max, tp->snd_una))) {
(void) tcp_output(tp);
+ }
goto check_delack;
}
} else if (th->th_ack == tp->snd_una &&
@@ -2585,12 +2595,12 @@ tcp_do_segment(struct tcpcb *tp, struct mbuf *m, struct tcphdr *th,
*/
if (th->th_ack != tp->snd_una ||
(tcp_is_sack_recovery(tp, &to) &&
- (sack_changed == SACK_NOCHANGE)))
+ (sack_changed == SACK_NOCHANGE))) {
break;
- else if (!tcp_timer_active(tp, TT_REXMT))
+ } else if (!tcp_timer_active(tp, TT_REXMT)) {
tp->t_dupacks = 0;
- else if (++tp->t_dupacks > tcprexmtthresh ||
- IN_FASTRECOVERY(tp->t_flags)) {
+ } else if (++tp->t_dupacks > tcprexmtthresh ||
+ IN_FASTRECOVERY(tp->t_flags)) {
cc_ack_received(tp, th, nsegs,
CC_DUPACK);
if (V_tcp_do_prr &&
@@ -2599,7 +2609,7 @@ tcp_do_segment(struct tcpcb *tp, struct mbuf *m, struct tcphdr *th,
tcp_do_prr_ack(tp, th, &to,
sack_changed, &maxseg);
} else if (tcp_is_sack_recovery(tp, &to) &&
- IN_FASTRECOVERY(tp->t_flags)) {
+ IN_FASTRECOVERY(tp->t_flags)) {
int awnd;
/*
@@ -2608,19 +2618,20 @@ tcp_do_segment(struct tcpcb *tp, struct mbuf *m, struct tcphdr *th,
* we have less than 1/2 the original window's
* worth of data in flight.
*/
- if (V_tcp_do_newsack)
+ if (V_tcp_do_newsack) {
awnd = tcp_compute_pipe(tp);
- else
+ } else {
awnd = (tp->snd_nxt - tp->snd_fack) +
tp->sackhint.sack_bytes_rexmit;
-
+ }
if (awnd < tp->snd_ssthresh) {
tp->snd_cwnd += maxseg;
if (tp->snd_cwnd > tp->snd_ssthresh)
tp->snd_cwnd = tp->snd_ssthresh;
}
- } else
+ } else {
tp->snd_cwnd += maxseg;
+ }
(void) tcp_output(tp);
goto drop;
} else if (tp->t_dupacks == tcprexmtthresh ||
@@ -2688,13 +2699,13 @@ enter_recovery:
tp->snd_nxt - tp->snd_una);
}
if (tcp_is_sack_recovery(tp, &to)) {
- TCPSTAT_INC(
- tcps_sack_recovery_episode);
+ TCPSTAT_INC(tcps_sack_recovery_episode);
tp->snd_recover = tp->snd_nxt;
tp->snd_cwnd = maxseg;
(void) tcp_output(tp);
- if (SEQ_GT(th->th_ack, tp->snd_una))
+ if (SEQ_GT(th->th_ack, tp->snd_una)) {
goto resume_partialack;
+ }
goto drop;
}
tp->snd_nxt = th->th_ack;
@@ -2720,8 +2731,7 @@ enter_recovery:
* segment. Restore the original
* snd_cwnd after packet transmission.
*/
- cc_ack_received(tp, th, nsegs,
- CC_DUPACK);
+ cc_ack_received(tp, th, nsegs, CC_DUPACK);
uint32_t oldcwnd = tp->snd_cwnd;
tcp_seq oldsndmax = tp->snd_max;
u_int sent;
@@ -2743,12 +2753,14 @@ enter_recovery:
* or we need to send an ACK.
*/
SOCKBUF_LOCK(&so->so_snd);
- avail = sbavail(&so->so_snd) -
- (tp->snd_nxt - tp->snd_una);
+ avail = sbavail(&so->so_snd);
SOCKBUF_UNLOCK(&so->so_snd);
- if (avail > 0 || tp->t_flags & TF_ACKNOW)
+ if (tp->t_flags & TF_ACKNOW ||
+ (avail >=
+ SEQ_SUB(tp->snd_nxt, tp->snd_una))) {
(void) tcp_output(tp);
- sent = tp->snd_max - oldsndmax;
+ }
+ sent = SEQ_SUB(tp->snd_max, oldsndmax);
if (sent > maxseg) {
KASSERT((tp->t_dupacks == 2 &&
tp->snd_limited == 0) ||
@@ -2757,8 +2769,9 @@ enter_recovery:
("%s: sent too much",
__func__));
tp->snd_limited = 2;
- } else if (sent > 0)
+ } else if (sent > 0) {
++tp->snd_limited;
+ }
tp->snd_cwnd = oldcwnd;
goto drop;
}
@@ -3316,9 +3329,9 @@ dodata: /* XXX */
/*
* Return any desired output.
*/
- if (needoutput || (tp->t_flags & TF_ACKNOW))
+ if (needoutput || (tp->t_flags & TF_ACKNOW)) {
(void) tcp_output(tp);
-
+ }
check_delack:
INP_WLOCK_ASSERT(inp);
@@ -3982,8 +3995,9 @@ tcp_do_prr_ack(struct tcpcb *tp, struct tcphdr *th, struct tcpopt *to,
tp->sackhint.sack_bytes_rexmit;
} else {
if (tp->sackhint.prr_delivered < (tcprexmtthresh * maxseg +
- tp->snd_recover - tp->snd_una))
+ tp->snd_recover - tp->snd_una)) {
del_data = maxseg;
+ }
pipe = imax(0, tp->snd_max - tp->snd_una -
imin(INT_MAX / 65536, tp->t_dupacks) * maxseg);
}
@@ -4009,13 +4023,14 @@ tcp_do_prr_ack(struct tcpcb *tp, struct tcphdr *th, struct tcpopt *to,
* - Prevent ACK splitting attacks, by being conservative
* when no new data is acked.
*/
- if ((sack_changed == SACK_NEWLOSS) || (del_data == 0))
+ if ((sack_changed == SACK_NEWLOSS) || (del_data == 0)) {
limit = tp->sackhint.prr_delivered -
tp->sackhint.prr_out;
- else
+ } else {
limit = imax(tp->sackhint.prr_delivered -
tp->sackhint.prr_out, del_data) +
maxseg;
+ }
snd_cnt = imin((tp->snd_ssthresh - pipe), limit);
}
snd_cnt = imax(snd_cnt, 0) / maxseg;
@@ -4033,8 +4048,9 @@ tcp_do_prr_ack(struct tcpcb *tp, struct tcphdr *th, struct tcpopt *to,
tp->snd_cwnd = (tp->snd_max - tp->snd_una) +
(snd_cnt * maxseg);
}
- } else if (IN_CONGRECOVERY(tp->t_flags))
+ } else if (IN_CONGRECOVERY(tp->t_flags)) {
tp->snd_cwnd = pipe - del_data + (snd_cnt * maxseg);
+ }
tp->snd_cwnd = imax(maxseg, tp->snd_cwnd);
}
diff --git a/sys/netinet/tcp_output.c b/sys/netinet/tcp_output.c
index 58f63b593b2a..50dc05e9c55a 100644
--- a/sys/netinet/tcp_output.c
+++ b/sys/netinet/tcp_output.c
@@ -392,10 +392,10 @@ after_sack_rexmit:
* in which case len is already set.
*/
if (sack_rxmit == 0) {
- if (sack_bytes_rxmt == 0)
+ if (sack_bytes_rxmt == 0) {
len = ((int32_t)min(sbavail(&so->so_snd), sendwin) -
off);
- else {
+ } else {
int32_t cwin;
/*
@@ -558,12 +558,8 @@ after_sack_rexmit:
ipoptlen == 0 && !(flags & TH_SYN))
tso = 1;
- if (sack_rxmit) {
- if (SEQ_LT(p->rxmit + len, tp->snd_una + sbused(&so->so_snd)))
- flags &= ~TH_FIN;
- } else {
- if (SEQ_LT(tp->snd_nxt + len, tp->snd_una +
- sbused(&so->so_snd)))
+ if (SEQ_LT((sack_rxmit ? p->rxmit : tp->snd_nxt) + len,
+ tp->snd_una + sbused(&so->so_snd))) {
flags &= ~TH_FIN;
}
diff --git a/sys/netinet/tcp_sack.c b/sys/netinet/tcp_sack.c
index f517bb9fcdb7..0c557dc4579d 100644
--- a/sys/netinet/tcp_sack.c
+++ b/sys/netinet/tcp_sack.c
@@ -988,12 +988,13 @@ tcp_sack_partialack(struct tcpcb *tp, struct tcphdr *th, u_int *maxsegp)
if (tp->t_flags & TF_SENTFIN)
highdata--;
highdata = SEQ_MIN(highdata, tp->snd_recover);
- if (th->th_ack != highdata) {
+ if (SEQ_LT(th->th_ack, highdata)) {
tp->snd_fack = th->th_ack;
if ((temp = tcp_sackhole_insert(tp, SEQ_MAX(th->th_ack,
- highdata - maxseg), highdata, NULL)) != NULL)
- tp->sackhint.hole_bytes += temp->end -
- temp->start;
+ highdata - maxseg), highdata, NULL)) != NULL) {
+ tp->sackhint.hole_bytes +=
+ temp->end - temp->start;
+ }
}
}
(void) tcp_output(tp);
@@ -1065,27 +1066,33 @@ tcp_sack_adjust(struct tcpcb *tp)
struct sackhole *p, *cur = TAILQ_FIRST(&tp->snd_holes);
INP_WLOCK_ASSERT(tptoinpcb(tp));
- if (cur == NULL)
- return; /* No holes */
- if (SEQ_GEQ(tp->snd_nxt, tp->snd_fack))
- return; /* We're already beyond any SACKed blocks */
+ if (cur == NULL) {
+ /* No holes */
+ return;
+ }
+ if (SEQ_GEQ(tp->snd_nxt, tp->snd_fack)) {
+ /* We're already beyond any SACKed blocks */
+ return;
+ }
/*-
* Two cases for which we want to advance snd_nxt:
* i) snd_nxt lies between end of one hole and beginning of another
* ii) snd_nxt lies between end of last hole and snd_fack
*/
while ((p = TAILQ_NEXT(cur, scblink)) != NULL) {
- if (SEQ_LT(tp->snd_nxt, cur->end))
+ if (SEQ_LT(tp->snd_nxt, cur->end)) {
return;
- if (SEQ_GEQ(tp->snd_nxt, p->start))
+ }
+ if (SEQ_GEQ(tp->snd_nxt, p->start)) {
cur = p;
- else {
+ } else {
tp->snd_nxt = p->start;
return;
}
}
- if (SEQ_LT(tp->snd_nxt, cur->end))
+ if (SEQ_LT(tp->snd_nxt, cur->end)) {
return;
+ }
tp->snd_nxt = tp->snd_fack;
}