Commit c4101dc4 authored by Eric Dumazet's avatar Eric Dumazet Committed by Khalid Elmously

tcp: fix a stale ooo_last_skb after a replace

BugLink: https://bugs.launchpad.net/bugs/1801893

[ Upstream commit 76f0dcbb ]

When skb replaces another one in ooo queue, I forgot to also
update tp->ooo_last_skb as well, if the replaced skb was the last one
in the queue.

To fix this, we simply can re-use the code that runs after an insertion,
trying to merge skbs at the right of current skb.

This not only fixes the bug, but also remove all small skbs that might
be a subset of the new one.

Example:

We receive segments 2001:3001,  4001:5001

Then we receive 2001:8001 : We should replace 2001:3001 with the big
skb, but also remove 4001:50001 from the queue to save space.

packetdrill test demonstrating the bug

0.000 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0 bind(3, ..., ...) = 0
+0 listen(3, 1) = 0

+0 < S 0:0(0) win 32792 <mss 1000,sackOK,nop,nop,nop,wscale 7>
+0 > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK,nop,wscale 7>
+0.100 < . 1:1(0) ack 1 win 1024
+0 accept(3, ..., ...) = 4

+0.01 < . 1001:2001(1000) ack 1 win 1024
+0    > . 1:1(0) ack 1 <nop,nop, sack 1001:2001>

+0.01 < . 1001:3001(2000) ack 1 win 1024
+0    > . 1:1(0) ack 1 <nop,nop, sack 1001:2001 1001:3001>

Fixes: 9f5afeae ("tcp: use an RB tree for ooo receive queue")
Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
Reported-by: default avatarYuchung Cheng <ycheng@google.com>
Cc: Yaogong Wang <wygivan@google.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
Signed-off-by: default avatarMao Wenan <maowenan@huawei.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: default avatarJuerg Haefliger <juergh@canonical.com>
Signed-off-by: default avatarKhalid Elmously <khalid.elmously@canonical.com>
parent 37e355f9
...@@ -4462,7 +4462,7 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb) ...@@ -4462,7 +4462,7 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
NET_INC_STATS(sock_net(sk), NET_INC_STATS(sock_net(sk),
LINUX_MIB_TCPOFOMERGE); LINUX_MIB_TCPOFOMERGE);
__kfree_skb(skb1); __kfree_skb(skb1);
goto add_sack; goto merge_right;
} }
} else if (tcp_try_coalesce(sk, skb1, skb, &fragstolen)) { } else if (tcp_try_coalesce(sk, skb1, skb, &fragstolen)) {
goto coalesce_done; goto coalesce_done;
...@@ -4474,6 +4474,7 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb) ...@@ -4474,6 +4474,7 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
rb_link_node(&skb->rbnode, parent, p); rb_link_node(&skb->rbnode, parent, p);
rb_insert_color(&skb->rbnode, &tp->out_of_order_queue); rb_insert_color(&skb->rbnode, &tp->out_of_order_queue);
merge_right:
/* Remove other segments covered by skb. */ /* Remove other segments covered by skb. */
while ((q = rb_next(&skb->rbnode)) != NULL) { while ((q = rb_next(&skb->rbnode)) != NULL) {
skb1 = rb_entry(q, struct sk_buff, rbnode); skb1 = rb_entry(q, struct sk_buff, rbnode);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment