[PATCH] Too aggressive cwnd backoff

[PATCH] Too aggressive cwnd backoff

[Baruch Even]

The cwnd backoff is down in two places and drops the cwnd to one quarter
instead of to one half.

On congestion events we reset tp->ssthresh to the result of
tcp_recalc_ssthresh. This cuts the cwnd by half for (New)Reno or to
a convoluted calculation for BIC.

Later we will call tcp_cwnd_down for each ack and reduce the cwnd by one
for every two acks.

However, in tcp_cwnd_down we will not stop reducing the cwnd until we
get to limit which is set to tp->ssthresh/2.

The provided patch will set limit to tp->ssthresh. This was the original
behaviour in some older version of Linux.

The patch is against 2.6.11

Signed-Off-By: Baruch Even <baruch@ev-en.org>

 net/ipv4/tcp_input.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

Index: 2.6.11-perf/net/ipv4/tcp_input.c
===================================================================
--- 2.6.11-perf.orig/net/ipv4/tcp_input.c
+++ 2.6.11-perf/net/ipv4/tcp_input.c
@@ -1621,7 +1621,7 @@ static void tcp_cwnd_down(struct tcp_soc
 	 */
 
 	if (!(limit = tcp_westwood_bw_rttmin(tp)))
-		limit = tp->snd_ssthresh/2;
+		limit = tp->snd_ssthresh;
 
 	tp->snd_cwnd_cnt = decr&1;
 	decr >>= 1;

Re: [PATCH] Too aggressive cwnd backoff

[Stephen Hemminger]

On Thu, 7 Apr 2005 19:41:46 +0300
Baruch Even <baruch@ev-en.org> wrote:

> The cwnd backoff is down in two places and drops the cwnd to one quarter
> instead of to one half.
> 
> On congestion events we reset tp->ssthresh to the result of
> tcp_recalc_ssthresh. This cuts the cwnd by half for (New)Reno or to
> a convoluted calculation for BIC.
> 
> Later we will call tcp_cwnd_down for each ack and reduce the cwnd by one
> for every two acks.
> 
> However, in tcp_cwnd_down we will not stop reducing the cwnd until we
> get to limit which is set to tp->ssthresh/2.
> 
> The provided patch will set limit to tp->ssthresh. This was the original
> behaviour in some older version of Linux.
> 
> The patch is against 2.6.11
> 
> Signed-Off-By: Baruch Even <baruch@ev-en.org>

I think this is a real problem, and was observed by Werner with umlsim.
Don't know when it got introduced because it appears to pre-date the
'04 work in adding Westwood, BIC, Vegas. Perhaps Alexey can shed some
light on this.

Going back to the pre-westwood code in BK, the /2 is still there.

----------------
static void tcp_cwnd_down(struct tcp_opt *tp)
{
	int decr = tp->snd_cwnd_cnt + 1;

	tp->snd_cwnd_cnt = decr&1;
	decr >>= 1;

	if (decr && tp->snd_cwnd > tp->snd_ssthresh/2)
		tp->snd_cwnd -= decr;

	tp->snd_cwnd = min(tp->snd_cwnd, tcp_packets_in_flight(tp)+1);
	tp->snd_cwnd_stamp = tcp_time_stamp;
}

Re: [PATCH] Too aggressive cwnd backoff

[Baruch Even]

Stephen Hemminger wrote:
> On Thu, 7 Apr 2005 19:41:46 +0300
> Baruch Even <baruch@ev-en.org> wrote:
>>The provided patch will set limit to tp->ssthresh. This was the original
>>behaviour in some older version of Linux.
> 
> I think this is a real problem, and was observed by Werner with umlsim.
> Don't know when it got introduced because it appears to pre-date the
> '04 work in adding Westwood, BIC, Vegas. Perhaps Alexey can shed some
> light on this.
> 
> Going back to the pre-westwood code in BK, the /2 is still there.

This wasn't there in 2.4.23 on which on the original work of H-TCP was 
done. I've encountered it in my work on the 2.6.6 version, but didn't 
understand all the implications at the time. I've re-encountered it now 
that I'm redoing the patches to 2.6.11, and it's as good a time as ever 
to resolve it.

The effect is not catastrophic, but it does mean that we leave recovery 
into slow-start like ascend of cwnd until we get to ssthresh again. It 
does mean that after recovery we inject a lot of packets to the network 
at a very fast rate.

Baruch

Re: [PATCH] Too aggressive cwnd backoff

[David S. Miller]

On Thu, 07 Apr 2005 19:14:45 +0100
Baruch Even <baruch@ev-en.org> wrote:

> > Going back to the pre-westwood code in BK, the /2 is still there.
> 
> This wasn't there in 2.4.23 on which on the original work of H-TCP was 
> done.

Not true, that division by 2 is in the 2.4.23 sources, I just
checked.

It is there as far back as BK logs go in both the 2.4.x and 2.6.x
trees.

Perhaps the WEB100 guys patched that test, I bet that's why you don't
remember it being in 2.4.23

Re: [PATCH] Too aggressive cwnd backoff

[John Heffner]

On Thu, 7 Apr 2005, David S. Miller wrote:

> On Thu, 07 Apr 2005 19:14:45 +0100
> Baruch Even <baruch@ev-en.org> wrote:
>
> > > Going back to the pre-westwood code in BK, the /2 is still there.
> >
> > This wasn't there in 2.4.23 on which on the original work of H-TCP was
> > done.
>
> Not true, that division by 2 is in the 2.4.23 sources, I just
> checked.
>
> It is there as far back as BK logs go in both the 2.4.x and 2.6.x
> trees.
>
> Perhaps the WEB100 guys patched that test, I bet that's why you don't
> remember it being in 2.4.23

This test looks correct to me.  (We never touched it.)  It is the bounding
parameter specified in rate halving.  If you actually get down that far,
then rate halving is getting confused, though.

  -John

Re: [PATCH] Too aggressive cwnd backoff

[Baruch Even]

John Heffner wrote:
> This test looks correct to me.  (We never touched it.)  It is the bounding
> parameter specified in rate halving.  If you actually get down that far,
> then rate halving is getting confused, though.

In my tests with either NewReno at high-BDP network settings
(300Kbit/s, 120ms delay, BDP = 3200 packets), we always go into this
confused mode.

It will always upon a drop go to a point between the one-half and 
one-quarter of the original cwnd, but then, due to performance problems 
at that point the queue is filled and lots of packets are getting lost 
in bursts after I disabled throttling, with throttling it goes even 
below one quarter.

If I understand you correctly this check (that I changed) is correct and 
should not be changed but rather that the bug is elsewhere. I'll give it 
another look when I have some more time.

Baruch

Re: [PATCH] Too aggressive cwnd backoff

[Werner Almesberger]

Stephen Hemminger wrote:
> I think this is a real problem, and was observed by Werner with umlsim.
> Don't know when it got introduced because it appears to pre-date the
> '04 work in adding Westwood, BIC, Vegas. Perhaps Alexey can shed some
> light on this.

If this is still the old cwnd quartering bug, then it's very very
old. Cheng Jin and I spotted it at the end of 2002 in 2.4.18, but
didn't look at earlier versions. (Later, I used this problem to
demonstrate how to fix bugs on the fly in umlsim.)

You can find the whole discussion in the following two threads:
http://thread.gmane.org/gmane.linux.network/2094
http://thread.gmane.org/gmane.linux.network/2223

Note that the fix isn't trivial (or, at least, it wasn't when we
looked at it), because just eliminating the /2 would make us too
aggressive in another, even weirder case, as I've described in
http://article.gmane.org/gmane.linux.network/2221
(which I admit to be somewhat daunting reading).

Recently, I noticed that my umlsim-based "hotfix" wouldn't improve
things in recent kernels anymore. I thought "cool, someone finally
sat down and fixed it", but perhaps that was premature.

- Werner

-- 
  _________________________________________________________________________
 / Werner Almesberger, Buenos Aires, Argentina     werner@almesberger.net /
/_http://www.almesberger.net/____________________________________________/

Re: [PATCH] Too aggressive cwnd backoff

[David S. Miller]

On Thu, 7 Apr 2005 19:41:46 +0300
Baruch Even <baruch@ev-en.org> wrote:

> However, in tcp_cwnd_down we will not stop reducing the cwnd until we
> get to limit which is set to tp->ssthresh/2.
> 
> The provided patch will set limit to tp->ssthresh. This was the original
> behaviour in some older version of Linux.

As stated in another email, it is still unknown where this "changed"
or if it even "changed" at all in the vanilla sources.

Could you track this down?  I still strongly believe this is some
WEB100 change you actually had in your tree, or something like
that.

I don't disagree with your analysis at all.  I just want to find out
if it did change in the vanilla sources.  Because if it did we can
track down a changelog message and perhaps learn something about what
the code is the way it is.

Re: [PATCH] Too aggressive cwnd backoff

[Baruch Even]

David S. Miller wrote:
> On Thu, 7 Apr 2005 19:41:46 +0300
> Baruch Even <baruch@ev-en.org> wrote:
> 
> 
>>However, in tcp_cwnd_down we will not stop reducing the cwnd until we
>>get to limit which is set to tp->ssthresh/2.
>>
>>The provided patch will set limit to tp->ssthresh. This was the original
>>behaviour in some older version of Linux.
> 
> 
> As stated in another email, it is still unknown where this "changed"
> or if it even "changed" at all in the vanilla sources.
> 
> Could you track this down?  I still strongly believe this is some
> WEB100 change you actually had in your tree, or something like
> that.

This change seems to be specific to us (Hamilton), I didn't find it as 
far back as 2.4.12 [1], it wasn't in any 2.6.x kernel and not even in 
web100 version alpha 2.3.2 which corresponds to kernel 2.4.23.

It does appear in the original patches that Doug Leith sent, so the only 
explanation left is that it's something he did in his tree originally. 
And for some reason I believed it was reverting an old change in the 
Linux kernel.

Baruch

[1] ftp.ie.kernel.org is missing 2.4.11 and ketchup died in my hands.

Re: [PATCH] Too aggressive cwnd backoff

[David S. Miller]

On Thu, 07 Apr 2005 20:18:09 +0100
Baruch Even <baruch@ev-en.org> wrote:

> This change seems to be specific to us (Hamilton), I didn't find it as 
> far back as 2.4.12 [1], it wasn't in any 2.6.x kernel and not even in 
> web100 version alpha 2.3.2 which corresponds to kernel 2.4.23.
> 
> It does appear in the original patches that Doug Leith sent, so the only 
> explanation left is that it's something he did in his tree originally. 
> And for some reason I believed it was reverting an old change in the 
> Linux kernel.

That makes sense.

And as John Heffner has pointed out, this change is correct and is
part of rate-halving.

Re: [PATCH] Too aggressive cwnd backoff

[Herbert Xu]

David S. Miller <davem@davemloft.net> wrote:
> 
> I don't disagree with your analysis at all.  I just want to find out
> if it did change in the vanilla sources.  Because if it did we can
> track down a changelog message and perhaps learn something about what
> the code is the way it is.

Looking at the vger tree this was introduced back in 2000:

# ChangeSet
#   2000/08/09 11:59:05-00:00 davem@vger.kernel.org
#   Many files:
#     Merge in ANKs net-000808-23:20 patch.
#     Linus better eat this.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] Too aggressive cwnd backoff

[David S. Miller]

On Fri, 08 Apr 2005 07:42:18 +1000
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> Looking at the vger tree this was introduced back in 2000:
> 
> # ChangeSet
> #   2000/08/09 11:59:05-00:00 davem@vger.kernel.org
> #   Many files:
> #     Merge in ANKs net-000808-23:20 patch.
> #     Linus better eat this.

Heh, wish I was better at changelog messages back in the
CVS days :-)