a question on tcp_highspeed.c (in 2.6.16)

a question on tcp_highspeed.c (in 2.6.16)

[Xiaoliang (David) Wei]

Hi gurus,

    I am reading the code of tcp_highspeed.c in the kernel and have a
question on the hstcp_cong_avoid function, specifically the following
AI part (line 136~143 in net/ipv4/tcp_highspeed.c ):

                /* Do additive increase */
                if (tp->snd_cwnd < tp->snd_cwnd_clamp) {
                        tp->snd_cwnd_cnt += ca->ai;
                        if (tp->snd_cwnd_cnt >= tp->snd_cwnd) {
                                tp->snd_cwnd++;
                                tp->snd_cwnd_cnt -= tp->snd_cwnd;
                        }
                }

    In this part, when (tp->snd_cwnd_cnt == tp->snd_cwnd),
snd_cwnd_cnt will be -1... snd_cwnd_cnt is defined as u16, will this
small chance of getting -1 becomes a problem?
Shall we change it by reversing the order of the cwnd++ and cwnd_cnt -= cwnd?

    Thanks.

-David

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

[PATCH] Re: a question on tcp_highspeed.c (in 2.6.16)

[John Heffner]

[-- Attachment #1: Type: text/plain, Size: 970 bytes --]

Xiaoliang (David) Wei wrote:
> Hi gurus,
> 
>    I am reading the code of tcp_highspeed.c in the kernel and have a
> question on the hstcp_cong_avoid function, specifically the following
> AI part (line 136~143 in net/ipv4/tcp_highspeed.c ):
> 
>                /* Do additive increase */
>                if (tp->snd_cwnd < tp->snd_cwnd_clamp) {
>                        tp->snd_cwnd_cnt += ca->ai;
>                        if (tp->snd_cwnd_cnt >= tp->snd_cwnd) {
>                                tp->snd_cwnd++;
>                                tp->snd_cwnd_cnt -= tp->snd_cwnd;
>                        }
>                }
> 
>    In this part, when (tp->snd_cwnd_cnt == tp->snd_cwnd),
> snd_cwnd_cnt will be -1... snd_cwnd_cnt is defined as u16, will this
> small chance of getting -1 becomes a problem?
> Shall we change it by reversing the order of the cwnd++ and cwnd_cnt -= 
> cwnd?

Absolutely correct.  Thanks.

Signed-off-by: John Heffner <jheffner@psc.edu>

[-- Attachment #2: highspeed_cwnd_cnt.patch --]
[-- Type: text/plain, Size: 434 bytes --]

diff --git a/net/ipv4/tcp_highspeed.c b/net/ipv4/tcp_highspeed.c
index e0e9d13..b72fa55 100644
--- a/net/ipv4/tcp_highspeed.c
+++ b/net/ipv4/tcp_highspeed.c
@@ -137,8 +137,8 @@ static void hstcp_cong_avoid(struct sock
 		if (tp->snd_cwnd < tp->snd_cwnd_clamp) {
 			tp->snd_cwnd_cnt += ca->ai;
 			if (tp->snd_cwnd_cnt >= tp->snd_cwnd) {
-				tp->snd_cwnd++;
 				tp->snd_cwnd_cnt -= tp->snd_cwnd;
+				tp->snd_cwnd++;
 			}
 		}
 	}

Re: [PATCH] Re: a question on tcp_highspeed.c (in 2.6.16)

[David S. Miller]

From: John Heffner <jheffner@psc.edu>
Date: Thu, 04 May 2006 15:06:00 -0400

> Xiaoliang (David) Wei wrote:
> > Hi gurus,
> > 
> >    I am reading the code of tcp_highspeed.c in the kernel and have a
> > question on the hstcp_cong_avoid function, specifically the following
> > AI part (line 136~143 in net/ipv4/tcp_highspeed.c ):
> > 
> >                /* Do additive increase */
> >                if (tp->snd_cwnd < tp->snd_cwnd_clamp) {
> >                        tp->snd_cwnd_cnt += ca->ai;
> >                        if (tp->snd_cwnd_cnt >= tp->snd_cwnd) {
> >                                tp->snd_cwnd++;
> >                                tp->snd_cwnd_cnt -= tp->snd_cwnd;
> >                        }
> >                }
> > 
> >    In this part, when (tp->snd_cwnd_cnt == tp->snd_cwnd),
> > snd_cwnd_cnt will be -1... snd_cwnd_cnt is defined as u16, will this
> > small chance of getting -1 becomes a problem?
> > Shall we change it by reversing the order of the cwnd++ and cwnd_cnt -= 
> > cwnd?
> 
> Absolutely correct.  Thanks.
> 
> Signed-off-by: John Heffner <jheffner@psc.edu>

Applied, thanks a lot everyone.