kernel - Fix stability issue with net.inet.tcp.inflight*

* net.inet.tcp.inflight_enable is (already) turned on by default.  This
  is necessary for network stability and to reduce excessive packet buffering
  to make PF and other routing protocols on the network work better.

* There is a bug at very high bandwidths where the 'bw' calculation is
  unstable and can lead to positive feedback instead of the desired negative
  feedback, preventing the tcp buffer size from ramping up properly.
  GigE links could sometimes run a tcp stream as slow as 7 MBytes/sec due
  to the bug due to bwnd limiting the tcp buffer to ~30-50KB.

* Fix the bug by increasing the minimum delta ticks for calculating (bw)
  from 1 to 2 and changing the stab parameter to scale bw instead of adding a
  fixed number of tcp maxsegs to bwnd.  This handles the high-speed issue.
  Low speed stability issues are handled by also adding 2*maxseg to bwnd.

  With this fix, GigE links stabilize better at the ~400-450KB needed to run
  the stream at full speed.

* net.inet.tcp.inflight_stab now scales bw in 1/10% increments instead
  of adding N*tcp_maxseg/10.  The default value of 50 increases the 'bw'
  calculation used to derive bwnd by 5%.  This appears to be sufficient.

Reported-by: alexh, rob__

diff --git a/share/man/man4/tcp.4 b/share/man/man4/tcp.4
index 5eb6cd1..bddd9c9 100644 (file)
--- a/share/man/man4/tcp.4
+++ b/share/man/man4/tcp.4
@@ -425,18 +425,18 @@ global per-connection limit on queued data, potentially allowing you to
 intentionally set a less than optimum limit to smooth data flow over a
 network while still being able to specify huge internal TCP buffers.
 .It tcp.inflight_stab
-The bandwidth delay product algorithm requires a slightly larger window
-than it otherwise calculates for stability.  This parameter determines the
-extra window in maximal packets / 10.  The default value of 20 represents
-2 maximal packets.  Reducing this value is not recommended but you may
-come across a situation with very slow links where the ping time
-reduction of the default inflight code is not sufficient.  If this case
-occurs you should first try reducing tcp.inflight_min and, if that does not
-work, reduce both tcp.inflight_min and tcp.inflight_stab, trying values of
-15, 10, or 5 for the latter.  Never use a value less than 5.  Reducing
-tcp.inflight_stab can lead to upwards of a 20% underutilization of the link
-as well as reducing the algorithm's ability to adapt to changing
-situations and should only be done as a last resort.
+This value stabilizes the bwnd (write window) calculation at high speeds
+by increasing the bandwidth calculation in 1/10% increments.  The default
+value of 50 represents a +5% increase.  In addition, bwnd is further increased
+by a fixed 2*maxseg bytes to stabilize the algorithm at low speeds.
+Changing the stab value is not recommended, but you may come across
+situations where tuning is beneficial.
+However, our recommendation for tuning is to stick with only adjusting
+tcp.inflight_min.
+Reducing tcp.inflight_stab too much can lead to upwards of a 20%
+underutilization of the link and prevent the algorithm from properly adapting
+to changing situations.  Increasing tcp.inflight_stab too much can lead to
+an excessive packet buffering situation.
 .El
 .Sh ERRORS
 A socket operation may fail with one of the following errors returned:

diff --git a/share/man/man7/tuning.7 b/share/man/man7/tuning.7
index 7e505f6..c3b892e 100644 (file)
--- a/share/man/man7/tuning.7
+++ b/share/man/man7/tuning.7
@@ -347,17 +347,11 @@ affect data reception (downloading).
 Adjusting
 .Va net.inet.tcp.inflight_stab
 is not recommended.
-This parameter defaults to 20, representing 2 maximal packets added
-to the bandwidth delay product window calculation.  The additional
-window is required to stabilize the algorithm and improve responsiveness
-to changing conditions, but it can also result in higher ping times
-over slow links (though still much lower than you would get without
-the inflight algorithm).  In such cases you may
-wish to try reducing this parameter to 15, 10, or 5, and you may also
-have to reduce
-.Va net.inet.tcp.inflight_min
-(for example, to 3500) to get the desired effect.  Reducing these parameters
-should be done as a last resort only.
+This parameter defaults to 50, representing +5% fudge when calculating the
+bwnd from the bw.  This fudge is on top of an additional fixed +2*maxseg
+added to bwnd.  The fudge factor is required to stabilize the algorithm
+at very high speeds while the fixed 2*maxseg stabilizes the algorithm at
+low speeds.  If you increase this value excessive packet buffering may occur.
 .Pp
 The
 .Va net.inet.ip.portrange.*

diff --git a/sys/netinet/tcp_subr.c b/sys/netinet/tcp_subr.c
index d5be4e0..51f1489 100644 (file)
--- a/sys/netinet/tcp_subr.c
+++ b/sys/netinet/tcp_subr.c
@@ -241,7 +241,11 @@ SYSCTL_INT(_net_inet_tcp, OID_AUTO, inflight_max, CTLFLAG_RW,
 
 static int tcp_inflight_stab = 50;
 SYSCTL_INT(_net_inet_tcp, OID_AUTO, inflight_stab, CTLFLAG_RW,
-    &tcp_inflight_stab, 0, "Slop in maximal packets / 10 (20 = 3 packets)");
+    &tcp_inflight_stab, 0, "Fudge bw 1/10% (50=5%)");
+
+static int tcp_inflight_adjrtt = 2;
+SYSCTL_INT(_net_inet_tcp, OID_AUTO, inflight_adjrtt, CTLFLAG_RW,
+    &tcp_inflight_adjrtt, 0, "Slop for rtt 1/(hz*32)");
 
 static int tcp_do_rfc3390 = 1;
 SYSCTL_INT(_net_inet_tcp, OID_AUTO, rfc3390, CTLFLAG_RW,
@@ -1917,6 +1921,7 @@ tcp_xmit_bandwidth_limit(struct tcpcb *tp, tcp_seq ack_seq)
         * a long time we have to reset the bandwidth calculator.
         */
        save_ticks = ticks;
+       cpu_ccfence();
        delta_ticks = save_ticks - tp->t_bw_rtttime;
        if (tp->t_bw_rtttime == 0 || delta_ticks < 0 || delta_ticks > hz * 10) {
                tp->t_bw_rtttime = ticks;
@@ -1925,7 +1930,13 @@ tcp_xmit_bandwidth_limit(struct tcpcb *tp, tcp_seq ack_seq)
                        tp->snd_bandwidth = tcp_inflight_min;
                return;
        }
-       if (delta_ticks == 0)
+
+       /*
+        * A delta of at least 1 tick is required.  Waiting 2 ticks will
+        * result in better (bw) accuracy.  More than that and the ramp-up
+        * will be too slow.
+        */
+       if (delta_ticks == 0 || delta_ticks == 1)
                return;
 
        /*
@@ -1955,6 +1966,12 @@ tcp_xmit_bandwidth_limit(struct tcpcb *tp, tcp_seq ack_seq)
         * spot and also handles the bandwidth run-up case.  Without the
         * slop we could be locking ourselves into a lower bandwidth.
         *
+        * At very high speeds the bw calculation can become overly sensitive
+        * and error prone when delta_ticks is low (e.g. usually 1).  To deal
+        * with the problem the stab must be scaled to the bw.  A stab of 50
+        * (the default) increases the bw for the purposes of the bwnd
+        * calculation by 5%.
+        *
         * Situations Handled:
         *      (1) Prevents over-queueing of packets on LANs, especially on
         *          high speed LANs, allowing larger TCP buffers to be
@@ -1977,9 +1994,10 @@ tcp_xmit_bandwidth_limit(struct tcpcb *tp, tcp_seq ack_seq)
         *          choice.
         */
 
-#define        USERTT  ((tp->t_srtt + tp->t_rttbest) / 2)
+#define        USERTT  (((tp->t_srtt + tp->t_rttbest) / 2) + tcp_inflight_adjrtt)
+       bw += bw * tcp_inflight_stab / 1000;
        bwnd = (int64_t)bw * USERTT / (hz << TCP_RTT_SHIFT) +
-              tcp_inflight_stab * (int)tp->t_maxseg / 10;
+              (int)tp->t_maxseg * 2;
 #undef USERTT
 
        if (tcp_inflight_debug > 0) {