[TCP] tcp_highspeed: Fix AI updates.
authorXiaoliang (David) Wei <davidwei79@gmail.com>
Tue, 11 Jul 2006 20:03:28 +0000 (13:03 -0700)
committerDavid S. Miller <davem@sunset.davemloft.net>
Wed, 12 Jul 2006 20:58:50 +0000 (13:58 -0700)
I think there is still a problem with the AIMD parameter update in
HighSpeed TCP code.

Line 125~138 of the code (net/ipv4/tcp_highspeed.c):

/* Update AIMD parameters */
if (tp->snd_cwnd > hstcp_aimd_vals[ca->ai].cwnd) {
while (tp->snd_cwnd > hstcp_aimd_vals[ca->ai].cwnd &&
       ca->ai < HSTCP_AIMD_MAX - 1)
ca->ai++;
} else if (tp->snd_cwnd < hstcp_aimd_vals[ca->ai].cwnd) {
while (tp->snd_cwnd > hstcp_aimd_vals[ca->ai].cwnd &&
       ca->ai > 0)
ca->ai--;

In fact, the second part (decreasing ca->ai) never decreases since the
while loop's inequality is in the reverse direction. This leads to
unfairness with multiple flows (once a flow happens to enjoy a higher
ca->ai, it keeps enjoying that even its cwnd decreases)

Here is a tentative fix (I also added a comment, trying to keep the
change clear):

Acked-by: Stephen Hemminger <shemminger@osdl.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/ipv4/tcp_highspeed.c

index aaa1538..fa3e1aa 100644 (file)
@@ -139,14 +139,19 @@ static void hstcp_cong_avoid(struct sock *sk, u32 adk, u32 rtt,
                                tp->snd_cwnd++;
                }
        } else {
-               /* Update AIMD parameters */
+               /* Update AIMD parameters.
+                *
+                * We want to guarantee that:
+                *     hstcp_aimd_vals[ca->ai-1].cwnd <
+                *     snd_cwnd <=
+                *     hstcp_aimd_vals[ca->ai].cwnd
+                */
                if (tp->snd_cwnd > hstcp_aimd_vals[ca->ai].cwnd) {
                        while (tp->snd_cwnd > hstcp_aimd_vals[ca->ai].cwnd &&
                               ca->ai < HSTCP_AIMD_MAX - 1)
                                ca->ai++;
-               } else if (tp->snd_cwnd < hstcp_aimd_vals[ca->ai].cwnd) {
-                       while (tp->snd_cwnd > hstcp_aimd_vals[ca->ai].cwnd &&
-                              ca->ai > 0)
+               } else if (ca->ai && tp->snd_cwnd <= hstcp_aimd_vals[ca->ai-1].cwnd) {
+                       while (ca->ai && tp->snd_cwnd <= hstcp_aimd_vals[ca->ai-1].cwnd)
                                ca->ai--;
                }