Author: sephe
Date: Fri Aug  5 09:08:00 2016
New Revision: 303766
URL: https://svnweb.freebsd.org/changeset/base/303766

Log:
  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:
  head/sys/netinet/tcp_lro.c

Modified: head/sys/netinet/tcp_lro.c
==============================================================================
--- head/sys/netinet/tcp_lro.c  Fri Aug  5 08:58:00 2016        (r303765)
+++ head/sys/netinet/tcp_lro.c  Fri Aug  5 09:08:00 2016        (r303766)
@@ -603,6 +603,7 @@ tcp_lro_rx2(struct lro_ctrl *lc, struct 
        int error, ip_len, l;
        uint16_t eh_type, tcp_data_len;
        struct lro_head *bucket;
+       int force_flush = 0;
 
        /* We expect a contiguous header [eh, ip, tcp]. */
 
@@ -669,8 +670,15 @@ tcp_lro_rx2(struct lro_ctrl *lc, struct 
         * 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? */
@@ -686,8 +694,13 @@ tcp_lro_rx2(struct lro_ctrl *lc, struct 
        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)
@@ -754,6 +767,13 @@ tcp_lro_rx2(struct lro_ctrl *lc, struct 
 #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);
@@ -830,6 +850,14 @@ tcp_lro_rx2(struct lro_ctrl *lc, struct 
                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);
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to