[CRIU] [PATCH] tcp: fix retransmission in repair mode

[Pavel Emelyanov]

On 11/13/2012 10:53 PM, Andrew Vagin wrote:
> On Tue, Nov 13, 2012 at 10:44:32PM +0400, Andrew Vagin wrote:
>> On Tue, Nov 13, 2012 at 07:39:26PM +0400, Pavel Emelyanov wrote:
>>> On 11/13/2012 07:29 PM, Andrey Vagin wrote:
>>>> From: Andrey Vagin <avagin at parallels.com>
>>>>
>>>> Currently if a socket has a few packet in a write queue,
>>>> a kernel bug may be triggered:
>>>>
>>>> kernel BUG at net/ipv4/tcp_output.c:2330!
>>>> RIP: 0010:[<ffffffff8155784f>] tcp_retransmit_skb+0x5ff/0x610
>>>>
>>>> According to the initial realization v3.4-rc2-963-gc0e88ff,
>>>> all skb-s should look like already posted. This patch fixes code
>>>> according with this sentence.
>>>>
>>>> Here are three points, which were not done in the initial patch:
>>>> 1. A tcp send head should not be changed
>>>> 2. Initialize TSO state of a skb
>>>> 3. Reset the retransmission time
>>>>
>>>> Signed-off-by: Andrey Vagin <avagin at openvz.org>
>>>> ---
>>>>  include/net/tcp.h     | 2 ++
>>>>  net/ipv4/tcp.c        | 6 ++++++
>>>>  net/ipv4/tcp_output.c | 3 ++-
>>>>  3 files changed, 10 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/include/net/tcp.h b/include/net/tcp.h
>>>> index 6feeccd..7582d4a 100644
>>>> --- a/include/net/tcp.h
>>>> +++ b/include/net/tcp.h
>>>> @@ -544,6 +544,8 @@ extern bool tcp_syn_flood_action(struct sock *sk,
>>>>  extern void tcp_push_one(struct sock *, unsigned int mss_now);
>>>>  extern void tcp_send_ack(struct sock *sk);
>>>>  extern void tcp_send_delayed_ack(struct sock *sk);
>>>> +extern int tcp_init_tso_segs(const struct sock *sk, struct sk_buff *skb,
>>>> +			     unsigned int mss_now);
>>>>  
>>>>  /* tcp_input.c */
>>>>  extern void tcp_cwnd_application_limited(struct sock *sk);
>>>> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
>>>> index f32c02e..11d070e 100644
>>>> --- a/net/ipv4/tcp.c
>>>> +++ b/net/ipv4/tcp.c
>>>> @@ -1196,6 +1196,12 @@ new_segment:
>>>>  			TCP_SKB_CB(skb)->end_seq += copy;
>>>>  			skb_shinfo(skb)->gso_segs = 0;
>>>>  
>>>> +			if (tp->repair) {
>>>> +				/* skb should look like already posted */
>>>> +				tcp_init_send_head(sk);
>>>
>>> When does new skb occur in the send_head?
>> tcp_sendmsg
>> 	sk_stream_alloc_skb
>> 	skb_entail(sk, skb);
>> 		tcp_add_write_queue_tail(sk, skb);
>> 			sk->sk_send_head = skb;
> I think all hacks should be in a one place, it is easier for reading.

The skb_entail is called from ->sendpages, so either the check whether or
not to update the sk_send_head should be there, or the call to reset the
send_head should be in the repair-off code.

>>>
>>>> +				tcp_init_tso_segs(sk, skb, mss_now);
>>>> +				tp->packets_out += tcp_skb_pcount(skb);
>>>> +			}
>>>>  			from += copy;
>>>>  			copied += copy;
>>>>  			if ((seglen -= copy) == 0 && iovlen == 0)
>>>> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
>>>> index cfe6ffe..3e95bc7 100644
>>>> --- a/net/ipv4/tcp_output.c
>>>> +++ b/net/ipv4/tcp_output.c
>>>> @@ -1579,7 +1579,7 @@ static inline unsigned int tcp_cwnd_test(const struct tcp_sock *tp,
>>>>   * This must be invoked the first time we consider transmitting
>>>>   * SKB onto the wire.
>>>>   */
>>>> -static int tcp_init_tso_segs(const struct sock *sk, struct sk_buff *skb,
>>>> +int tcp_init_tso_segs(const struct sock *sk, struct sk_buff *skb,
>>>>  			     unsigned int mss_now)
>>>>  {
>>>>  	int tso_segs = tcp_skb_pcount(skb);
>>>> @@ -3140,6 +3140,7 @@ void tcp_send_window_probe(struct sock *sk)
>>>>  		tcp_sk(sk)->snd_wl1 = tcp_sk(sk)->rcv_nxt - 1;
>>>>  		tcp_sk(sk)->snd_nxt = tcp_sk(sk)->write_seq;
>>>>  		tcp_xmit_probe_skb(sk, 0);
>>>> +		tcp_rearm_rto(sk);
>>>
>>> This should be done in setsockopt after calling tcp_send_window_probe
>>> since it has nothing to do with sending the probe.
>>
>> Ok
>>>
>>>>  	}
>>>>  }
>>>>  
>>>>
>>>
>>>
> .
>