Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 26 Aug 2016 05:37:44 +0000 (UTC)
From:      Sepherosa Ziehau <sephe@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org
Subject:   svn commit: r304835 - stable/11/sys/netinet
Message-ID:  <201608260537.u7Q5biok046511@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: sephe
Date: Fri Aug 26 05:37:44 2016
New Revision: 304835
URL: https://svnweb.freebsd.org/changeset/base/304835

Log:
  MFC 303766
      tcp/lro: If timestamps mismatch or it's a FIN, force flush.
  
      This keeps the segments/ACK/FIN delivery order.
  
      Before this patch, it was observed: if A sent FIN immediately after
      an ACK, B would deliver FIN first to the TCP stack, then the ACK.
      This out-of-order delivery causes one unnecessary ACK sent from B.
  
      Reviewed by:    gallatin, hps
      Obtained from:  rrs, gallatin
      Sponsored by:   Netflix (rrs, gallatin), Microsoft (sephe)
      Differential Revision:  https://reviews.freebsd.org/D7415

Modified:
  stable/11/sys/netinet/tcp_lro.c
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/sys/netinet/tcp_lro.c
==============================================================================
--- stable/11/sys/netinet/tcp_lro.c	Fri Aug 26 05:18:27 2016	(r304834)
+++ stable/11/sys/netinet/tcp_lro.c	Fri Aug 26 05:37:44 2016	(r304835)
@@ -578,6 +578,7 @@ tcp_lro_rx(struct lro_ctrl *lc, struct m
 	tcp_seq seq;
 	int error, ip_len, l;
 	uint16_t eh_type, tcp_data_len;
+	int force_flush = 0;
 
 	/* We expect a contiguous header [eh, ip, tcp]. */
 
@@ -644,8 +645,15 @@ tcp_lro_rx(struct lro_ctrl *lc, struct m
 	 * Check TCP header constraints.
 	 */
 	/* Ensure no bits set besides ACK or PSH. */
-	if ((th->th_flags & ~(TH_ACK | TH_PUSH)) != 0)
-		return (TCP_LRO_CANNOT);
+	if ((th->th_flags & ~(TH_ACK | TH_PUSH)) != 0) {
+		if (th->th_flags & TH_SYN)
+			return (TCP_LRO_CANNOT);
+		/*
+		 * Make sure that previously seen segements/ACKs are delivered
+		 * before this segement, e.g. FIN.
+		 */
+		force_flush = 1;
+	}
 
 	/* XXX-BZ We lose a ACK|PUSH flag concatenating multiple segments. */
 	/* XXX-BZ Ideally we'd flush on PUSH? */
@@ -661,8 +669,13 @@ tcp_lro_rx(struct lro_ctrl *lc, struct m
 	ts_ptr = (uint32_t *)(th + 1);
 	if (l != 0 && (__predict_false(l != TCPOLEN_TSTAMP_APPA) ||
 	    (*ts_ptr != ntohl(TCPOPT_NOP<<24|TCPOPT_NOP<<16|
-	    TCPOPT_TIMESTAMP<<8|TCPOLEN_TIMESTAMP))))
-		return (TCP_LRO_CANNOT);
+	    TCPOPT_TIMESTAMP<<8|TCPOLEN_TIMESTAMP)))) {
+		/*
+		 * Make sure that previously seen segements/ACKs are delivered
+		 * before this segement.
+		 */
+		force_flush = 1;
+	}
 
 	/* If the driver did not pass in the checksum, set it now. */
 	if (csum == 0x0000)
@@ -696,6 +709,13 @@ tcp_lro_rx(struct lro_ctrl *lc, struct m
 #endif
 		}
 
+		if (force_flush) {
+			/* Timestamps mismatch; this is a FIN, etc */
+			tcp_lro_active_remove(le);
+			tcp_lro_flush(lc, le);
+			return (TCP_LRO_CANNOT);
+		}
+
 		/* Flush now if appending will result in overflow. */
 		if (le->p_len > (lc->lro_length_lim - tcp_data_len)) {
 			tcp_lro_active_remove(le);
@@ -772,6 +792,14 @@ tcp_lro_rx(struct lro_ctrl *lc, struct m
 		return (0);
 	}
 
+	if (force_flush) {
+		/*
+		 * Nothing to flush, but this segment can not be further
+		 * aggregated/delayed.
+		 */
+		return (TCP_LRO_CANNOT);
+	}
+
 	/* Try to find an empty slot. */
 	if (LIST_EMPTY(&lc->lro_free))
 		return (TCP_LRO_NO_ENTRIES);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201608260537.u7Q5biok046511>