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

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

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

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

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