* [PATCH net-next] tcp: tcp_fragment() should not assume rtx skbs
@ 2017-11-03 1:10 Eric Dumazet
2017-11-03 1:16 ` Neal Cardwell
2017-11-03 7:03 ` David Miller
0 siblings, 2 replies; 4+ messages in thread
From: Eric Dumazet @ 2017-11-03 1:10 UTC (permalink / raw)
To: David Miller
Cc: netdev, Yuchung Cheng, Neal Cardwell, Soheil Hassas Yeganeh,
Alexei Starovoitov, Priyaranjan Jha
From: Eric Dumazet <edumazet@google.com>
While stress testing MTU probing, we had crashes in list_del() that we root-caused
to the fact that tcp_fragment() is unconditionally inserting the freshly allocated
skb into tsorted_sent_queue list.
But this list is supposed to contain skbs that were sent.
This was mostly harmless until MTU probing was enabled.
Fortunately we can use the tcp_queue enum added later (but in same linux version)
for rtx-rb-tree to fix the bug.
Fixes: e2080072ed2d ("tcp: new list for sent but unacked skbs for RACK recovery")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Yuchung Cheng <ycheng@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
Cc: Soheil Hassas Yeganeh <soheil@google.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Priyaranjan Jha <priyarjha@google.com>
---
net/ipv4/tcp_output.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index a85e8a282d173983e35a2a1e3135ca2a63f1699e..36a3e7c909caacd981b84d1d8820a33d922f5a4e 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1395,7 +1395,8 @@ int tcp_fragment(struct sock *sk, enum tcp_queue tcp_queue,
/* Link BUFF into the send queue. */
__skb_header_release(buff);
tcp_insert_write_queue_after(skb, buff, sk, tcp_queue);
- list_add(&buff->tcp_tsorted_anchor, &skb->tcp_tsorted_anchor);
+ if (tcp_queue == TCP_FRAG_IN_RTX_QUEUE)
+ list_add(&buff->tcp_tsorted_anchor, &skb->tcp_tsorted_anchor);
return 0;
}
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] tcp: tcp_fragment() should not assume rtx skbs
2017-11-03 1:10 [PATCH net-next] tcp: tcp_fragment() should not assume rtx skbs Eric Dumazet
@ 2017-11-03 1:16 ` Neal Cardwell
2017-11-03 2:33 ` Soheil Hassas Yeganeh
2017-11-03 7:03 ` David Miller
1 sibling, 1 reply; 4+ messages in thread
From: Neal Cardwell @ 2017-11-03 1:16 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, netdev, Yuchung Cheng, Soheil Hassas Yeganeh,
Alexei Starovoitov, Priyaranjan Jha
On Thu, Nov 2, 2017 at 9:10 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> While stress testing MTU probing, we had crashes in list_del() that we root-caused
> to the fact that tcp_fragment() is unconditionally inserting the freshly allocated
> skb into tsorted_sent_queue list.
>
> But this list is supposed to contain skbs that were sent.
> This was mostly harmless until MTU probing was enabled.
>
> Fortunately we can use the tcp_queue enum added later (but in same linux version)
> for rtx-rb-tree to fix the bug.
>
> Fixes: e2080072ed2d ("tcp: new list for sent but unacked skbs for RACK recovery")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
Thanks, Eric!
neal
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] tcp: tcp_fragment() should not assume rtx skbs
2017-11-03 1:16 ` Neal Cardwell
@ 2017-11-03 2:33 ` Soheil Hassas Yeganeh
0 siblings, 0 replies; 4+ messages in thread
From: Soheil Hassas Yeganeh @ 2017-11-03 2:33 UTC (permalink / raw)
To: Neal Cardwell
Cc: Eric Dumazet, David Miller, netdev, Yuchung Cheng,
Alexei Starovoitov, Priyaranjan Jha
On Thu, Nov 2, 2017 at 9:16 PM, Neal Cardwell <ncardwell@google.com> wrote:
> On Thu, Nov 2, 2017 at 9:10 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> From: Eric Dumazet <edumazet@google.com>
>>
>> While stress testing MTU probing, we had crashes in list_del() that we root-caused
>> to the fact that tcp_fragment() is unconditionally inserting the freshly allocated
>> skb into tsorted_sent_queue list.
>>
>> But this list is supposed to contain skbs that were sent.
>> This was mostly harmless until MTU probing was enabled.
>>
>> Fortunately we can use the tcp_queue enum added later (but in same linux version)
>> for rtx-rb-tree to fix the bug.
>>
>> Fixes: e2080072ed2d ("tcp: new list for sent but unacked skbs for RACK recovery")
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>
> Acked-by: Neal Cardwell <ncardwell@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Nice! Thank you, Eric!
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] tcp: tcp_fragment() should not assume rtx skbs
2017-11-03 1:10 [PATCH net-next] tcp: tcp_fragment() should not assume rtx skbs Eric Dumazet
2017-11-03 1:16 ` Neal Cardwell
@ 2017-11-03 7:03 ` David Miller
1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2017-11-03 7:03 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, ycheng, ncardwell, soheil, ast, priyarjha
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 02 Nov 2017 18:10:03 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> While stress testing MTU probing, we had crashes in list_del() that we root-caused
> to the fact that tcp_fragment() is unconditionally inserting the freshly allocated
> skb into tsorted_sent_queue list.
>
> But this list is supposed to contain skbs that were sent.
> This was mostly harmless until MTU probing was enabled.
>
> Fortunately we can use the tcp_queue enum added later (but in same linux version)
> for rtx-rb-tree to fix the bug.
>
> Fixes: e2080072ed2d ("tcp: new list for sent but unacked skbs for RACK recovery")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks Eric.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-11-03 7:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-03 1:10 [PATCH net-next] tcp: tcp_fragment() should not assume rtx skbs Eric Dumazet
2017-11-03 1:16 ` Neal Cardwell
2017-11-03 2:33 ` Soheil Hassas Yeganeh
2017-11-03 7:03 ` David Miller
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.