Patchwork fix pushed_seq in keepalive / zero window probe timer

login
register
mail settings
Submitter Li Yu
Date Aug. 22, 2011, 6:57 a.m.
Message ID <4E51FDD1.2010607@gmail.com>
Download mbox | patch
Permalink /patch/110858/
State Rejected
Delegated to: David Miller
Headers show

Comments

Li Yu - Aug. 22, 2011, 6:57 a.m.
In tcp_write_wakeup(), we may split the probe segment since send window or mss is larger than it.
so I think that we should update tp->pushed_seq after tcp_fragment(), is it right? thanks.

Signed-off-by: Li Yu <raise.sail@gmail.com>
CC: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
CC: David S. Miller <davem@davemloft.net>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet - Aug. 22, 2011, 11:25 a.m.
Le lundi 22 août 2011 à 14:57 +0800, Li Yu a écrit :
> In tcp_write_wakeup(), we may split the probe segment since send window or mss is larger than it.
> so I think that we should update tp->pushed_seq after tcp_fragment(), is it right? thanks.
> 

I am not sure what you want to fix.

Fact is the skb has the TCPHDR_PSH, even if tcp_transmit_skb() (or
tcp_fragment()) is/are not able to clone/split it.


> Signed-off-by: Li Yu <raise.sail@gmail.com>
> CC: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> CC: David S. Miller <davem@davemloft.net>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 882e0b0..659a71f 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -2784,9 +2784,6 @@ int tcp_write_wakeup(struct sock *sk)
>  		unsigned int mss = tcp_current_mss(sk);
>  		unsigned int seg_size = tcp_wnd_end(tp) - TCP_SKB_CB(skb)->seq;
>  
> -		if (before(tp->pushed_seq, TCP_SKB_CB(skb)->end_seq))
> -			tp->pushed_seq = TCP_SKB_CB(skb)->end_seq;
> -
>  		/* We are probing the opening of a window
>  		 * but the window size is != 0
>  		 * must have been a result SWS avoidance ( sender )
> @@ -2803,8 +2800,11 @@ int tcp_write_wakeup(struct sock *sk)
>  		TCP_SKB_CB(skb)->flags |= TCPHDR_PSH;
>  		TCP_SKB_CB(skb)->when = tcp_time_stamp;
>  		err = tcp_transmit_skb(sk, skb, 1, GFP_ATOMIC);
> -		if (!err)
> +		if (!err) {
>  			tcp_event_new_data_sent(sk, skb);
> +			if (before(tp->pushed_seq, TCP_SKB_CB(skb)->end_seq))
> +				tp->pushed_seq = TCP_SKB_CB(skb)->end_seq;
> +		}
>  		return err;
>  	} else {
>  		if (between(tp->snd_up, tp->snd_una + 1, tp->snd_una + 0xFFFF))
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Li Yu - Aug. 22, 2011, 1:41 p.m.
2011/8/22 Eric Dumazet <eric.dumazet@gmail.com>:
> Le lundi 22 août 2011 à 14:57 +0800, Li Yu a écrit :
>> In tcp_write_wakeup(), we may split the probe segment since send window or mss is larger than it.
>> so I think that we should update tp->pushed_seq after tcp_fragment(), is it right? thanks.
>>
>
> I am not sure what you want to fix.
>
> Fact is the skb has the TCPHDR_PSH, even if tcp_transmit_skb() (or
> tcp_fragment()) is/are not able to clone/split it.
>
>

Indeed, you are right. The goal of this patch is to fix tp->pushed_seq
may point to unsent byte seq number.

>> Signed-off-by: Li Yu <raise.sail@gmail.com>
>> CC: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
>> CC: David S. Miller <davem@davemloft.net>
>> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
>> index 882e0b0..659a71f 100644
>> --- a/net/ipv4/tcp_output.c
>> +++ b/net/ipv4/tcp_output.c
>> @@ -2784,9 +2784,6 @@ int tcp_write_wakeup(struct sock *sk)
>>               unsigned int mss = tcp_current_mss(sk);
>>               unsigned int seg_size = tcp_wnd_end(tp) - TCP_SKB_CB(skb)->seq;
>>
>> -             if (before(tp->pushed_seq, TCP_SKB_CB(skb)->end_seq))
>> -                     tp->pushed_seq = TCP_SKB_CB(skb)->end_seq;
>> -
>>               /* We are probing the opening of a window
>>                * but the window size is != 0
>>                * must have been a result SWS avoidance ( sender )
>> @@ -2803,8 +2800,11 @@ int tcp_write_wakeup(struct sock *sk)
>>               TCP_SKB_CB(skb)->flags |= TCPHDR_PSH;
>>               TCP_SKB_CB(skb)->when = tcp_time_stamp;
>>               err = tcp_transmit_skb(sk, skb, 1, GFP_ATOMIC);
>> -             if (!err)
>> +             if (!err) {
>>                       tcp_event_new_data_sent(sk, skb);
>> +                     if (before(tp->pushed_seq, TCP_SKB_CB(skb)->end_seq))
>> +                             tp->pushed_seq = TCP_SKB_CB(skb)->end_seq;
>> +             }
>>               return err;
>>       } else {
>>               if (between(tp->snd_up, tp->snd_una + 1, tp->snd_una + 0xFFFF))
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 882e0b0..659a71f 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2784,9 +2784,6 @@  int tcp_write_wakeup(struct sock *sk)
 		unsigned int mss = tcp_current_mss(sk);
 		unsigned int seg_size = tcp_wnd_end(tp) - TCP_SKB_CB(skb)->seq;
 
-		if (before(tp->pushed_seq, TCP_SKB_CB(skb)->end_seq))
-			tp->pushed_seq = TCP_SKB_CB(skb)->end_seq;
-
 		/* We are probing the opening of a window
 		 * but the window size is != 0
 		 * must have been a result SWS avoidance ( sender )
@@ -2803,8 +2800,11 @@  int tcp_write_wakeup(struct sock *sk)
 		TCP_SKB_CB(skb)->flags |= TCPHDR_PSH;
 		TCP_SKB_CB(skb)->when = tcp_time_stamp;
 		err = tcp_transmit_skb(sk, skb, 1, GFP_ATOMIC);
-		if (!err)
+		if (!err) {
 			tcp_event_new_data_sent(sk, skb);
+			if (before(tp->pushed_seq, TCP_SKB_CB(skb)->end_seq))
+				tp->pushed_seq = TCP_SKB_CB(skb)->end_seq;
+		}
 		return err;
 	} else {
 		if (between(tp->snd_up, tp->snd_una + 1, tp->snd_una + 0xFFFF))