The branch main has been updated by rscheff: URL: https://cgit.FreeBSD.org/src/commit/?id=2d05a1c81b2cbb5468a242d0add44f850aa31811
commit 2d05a1c81b2cbb5468a242d0add44f850aa31811 Author: Richard Scheffenegger <[email protected]> AuthorDate: 2024-01-25 23:19:30 +0000 Commit: Richard Scheffenegger <[email protected]> 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; }
