All of lore.kernel.org
 help / color / mirror / Atom feed
* A potential bug in tcp_vegas.c
@ 2007-02-01 21:03 Xiaoliang (David) Wei
  2007-02-09  0:08 ` David Miller
  0 siblings, 1 reply; 3+ messages in thread
From: Xiaoliang (David) Wei @ 2007-02-01 21:03 UTC (permalink / raw)
  To: netdev

Hi gurus,

I am testing tcp_vegas.c in Linux with the NS-2 TCP-Linux.
It seems that the ssthresh is not correctly reset to 2 in the
"tcp_vegas_cong_avoid" function.
The problem might lead to very unfair behavior among Vegas flows, when
some flows exit slow start due to loss, not delay.

Please see the detailed effect at:
http://www.cs.caltech.edu/%7Eweixl/technical/ns2linux/known_linux/#vegas
(the section of "Setting of Slow-Start-Threshold")

A patch I wrote (for Linux 2.6.19-2) is attached.
Basically, the branch that sets ssthresh to be 2 should be outside the
condition of (cwnd<=ssthresh).

--- tcp_vegas.c.old     2007-02-01 00:33:55.000000000 -0800
+++ tcp_vegas.c 2007-02-01 00:39:49.000000000 -0800
@@ -265,26 +265,25 @@
                        */
                       diff = (old_wnd << V_PARAM_SHIFT) - target_cwnd;

-                       if (tp->snd_cwnd <= tp->snd_ssthresh) {
-                               /* Slow start.  */
-                               if (diff > gamma) {
-                                       /* Going too fast. Time to slow down
-                                        * and switch to congestion avoidance.
-                                        */
-                                       tp->snd_ssthresh = 2;
+                       if (diff > gamma && tp->snd_ssthresh > 2 ) {
+                               /* Going too fast. Time to slow down
+                                * and switch to congestion avoidance.
+                                */
+                               tp->snd_ssthresh = 2;

-                                       /* Set cwnd to match the actual rate
-                                        * exactly:
-                                        *   cwnd = (actual rate) * baseRTT
-                                        * Then we add 1 because the integer
-                                        * truncation robs us of full link
-                                        * utilization.
-                                        */
-                                       tp->snd_cwnd = min(tp->snd_cwnd,
-                                                          (target_cwnd >>
-                                                           V_PARAM_SHIFT)+1);
+                               /* Set cwnd to match the actual rate
+                                * exactly:
+                                *   cwnd = (actual rate) * baseRTT
+                                * Then we add 1 because the integer
+                                * truncation robs us of full link
+                                * utilization.
+                                */
+                               tp->snd_cwnd = min(tp->snd_cwnd,
+                                                  (target_cwnd >>
+                                                   V_PARAM_SHIFT)+1);

-                               }
+                       } else if (tp->snd_cwnd <= tp->snd_ssthresh) {
+                               /* Slow start.  */
                               tcp_slow_start(tp);
                       } else {
                               /* Congestion avoidance. */



Thanks.

-David

--
Xiaoliang (David) Wei      Graduate Student, CS@Caltech
http://davidwei.org
***********************************************

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: A potential bug in tcp_vegas.c
  2007-02-01 21:03 A potential bug in tcp_vegas.c Xiaoliang (David) Wei
@ 2007-02-09  0:08 ` David Miller
  0 siblings, 0 replies; 3+ messages in thread
From: David Miller @ 2007-02-09  0:08 UTC (permalink / raw)
  To: davidwei79; +Cc: netdev


From: "Xiaoliang (David) Wei" <davidwei79@gmail.com>
Date: Thu, 1 Feb 2007 13:03:49 -0800

> I am testing tcp_vegas.c in Linux with the NS-2 TCP-Linux.
> It seems that the ssthresh is not correctly reset to 2 in the
> "tcp_vegas_cong_avoid" function.
> The problem might lead to very unfair behavior among Vegas flows, when
> some flows exit slow start due to loss, not delay.
> 
> Please see the detailed effect at:
> http://www.cs.caltech.edu/%7Eweixl/technical/ns2linux/known_linux/#vegas
> (the section of "Setting of Slow-Start-Threshold")
> 
> A patch I wrote (for Linux 2.6.19-2) is attached.
> Basically, the branch that sets ssthresh to be 2 should be outside the
> condition of (cwnd<=ssthresh).

Please provide a proper "Signed-off-by: " line, see
linux/Documentation/SubmittingPatches for details.

Your patch we not correctly rooted in the tree, the
diff file lines should use paths like:

--- a/net/ipv4/tcp_vegas.c.old     2007-02-01 00:33:55.000000000 -0800
+++ b/net/ipv4/tcp_vegas.c 2007-02-01 00:39:49.000000000 -0800

Again, see linux/Documentation/SubmittingPatches.

Also, GMAIL has corrupted your patch, turning all tab
characters into spaces, so that patch will not apply
properly.

If in doubt, try emailing the patch to yourself and
successfully applying it.

Please fix this up and resubmit, thank you.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* A potential bug in tcp_vegas.c
@ 2007-02-01  9:45 Xiaoliang (David) Wei
  0 siblings, 0 replies; 3+ messages in thread
From: Xiaoliang (David) Wei @ 2007-02-01  9:45 UTC (permalink / raw)
  To: netdev

Hi gurus,

I am testing tcp_vegas.c in Linux with the NS-2 TCP-Linux. It seems
that the ssthresh is not correctly reset in this file and the problem
might lead to very unfair behavior among Vegas flows, when some flows
exit slow start due to loss, not delay.

Please see the details at:

http://www.cs.caltech.edu/%7Eweixl/technical/ns2linux/known_linux/index.html#vegas
(the section of "Setting of Slow-Start-Threshold")

A fix I wrote (for Linux 2.6.19.2) is as follow:

--- tcp_vegas.c.old	2007-02-01 00:33:55.000000000 -0800
+++ tcp_vegas.c	2007-02-01 00:39:49.000000000 -0800
@@ -265,26 +265,25 @@
 			 */
 			diff = (old_wnd << V_PARAM_SHIFT) - target_cwnd;

-			if (tp->snd_cwnd <= tp->snd_ssthresh) {
-				/* Slow start.  */
-				if (diff > gamma) {
-					/* Going too fast. Time to slow down
-					 * and switch to congestion avoidance.
-					 */
-					tp->snd_ssthresh = 2;
+			if (diff > gamma && tp->snd_ssthresh > 2 ) {
+				/* Going too fast. Time to slow down
+				 * and switch to congestion avoidance.
+				 */
+				tp->snd_ssthresh = 2;

-					/* Set cwnd to match the actual rate
-					 * exactly:
-					 *   cwnd = (actual rate) * baseRTT
-					 * Then we add 1 because the integer
-					 * truncation robs us of full link
-					 * utilization.
-					 */
-					tp->snd_cwnd = min(tp->snd_cwnd,
-							   (target_cwnd >>
-							    V_PARAM_SHIFT)+1);
+				/* Set cwnd to match the actual rate
+				 * exactly:
+				 *   cwnd = (actual rate) * baseRTT
+				 * Then we add 1 because the integer
+				 * truncation robs us of full link
+				 * utilization.
+				 */
+				tp->snd_cwnd = min(tp->snd_cwnd,
+						   (target_cwnd >>
+						    V_PARAM_SHIFT)+1);

-				}
+			} else if (tp->snd_cwnd <= tp->snd_ssthresh) {
+				/* Slow start.  */
 				tcp_slow_start(tp);
 			} else {
 				/* Congestion avoidance. */


Thanks.

-David
-- 
Xiaoliang (David) Wei      Graduate Student, CS@Caltech
http://davidwei.org
***********************************************

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2007-02-09  0:08 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-02-01 21:03 A potential bug in tcp_vegas.c Xiaoliang (David) Wei
2007-02-09  0:08 ` David Miller
  -- strict thread matches above, loose matches on Subject: below --
2007-02-01  9:45 Xiaoliang (David) Wei

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.