tcp: fix tcp_grow_skb() vs tstamps
authorEric Dumazet <edumazet@google.com>
Thu, 25 Apr 2024 19:34:50 +0000 (19:34 +0000)
committerJakub Kicinski <kuba@kernel.org>
Fri, 26 Apr 2024 20:55:29 +0000 (13:55 -0700)
I forgot to call tcp_skb_collapse_tstamp() in the
case we consume the second skb in write queue.

Neal suggested to create a common helper used by tcp_mtu_probe()
and tcp_grow_skb().

Fixes: 8ee602c63520 ("tcp: try to send bigger TSO packets")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
Link: https://lore.kernel.org/r/20240425193450.411640-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
net/ipv4/tcp_output.c

index ece4726c875100ffb5140be290df40e7b7d835a6..ea7ad7d99245c26f4045ebb344bc42cd188afe37 100644 (file)
@@ -2403,6 +2403,21 @@ commit:
        return 0;
 }
 
+/* tcp_mtu_probe() and tcp_grow_skb() can both eat an skb (src) if
+ * all its payload was moved to another one (dst).
+ * Make sure to transfer tcp_flags, eor, and tstamp.
+ */
+static void tcp_eat_one_skb(struct sock *sk,
+                           struct sk_buff *dst,
+                           struct sk_buff *src)
+{
+       TCP_SKB_CB(dst)->tcp_flags |= TCP_SKB_CB(src)->tcp_flags;
+       TCP_SKB_CB(dst)->eor = TCP_SKB_CB(src)->eor;
+       tcp_skb_collapse_tstamp(dst, src);
+       tcp_unlink_write_queue(src, sk);
+       tcp_wmem_free_skb(sk, src);
+}
+
 /* Create a new MTU probe if we are ready.
  * MTU probe is regularly attempting to increase the path MTU by
  * deliberately sending larger packets.  This discovers routing
@@ -2508,16 +2523,7 @@ static int tcp_mtu_probe(struct sock *sk)
                copy = min_t(int, skb->len, probe_size - len);
 
                if (skb->len <= copy) {
-                       /* We've eaten all the data from this skb.
-                        * Throw it away. */
-                       TCP_SKB_CB(nskb)->tcp_flags |= TCP_SKB_CB(skb)->tcp_flags;
-                       /* If this is the last SKB we copy and eor is set
-                        * we need to propagate it to the new skb.
-                        */
-                       TCP_SKB_CB(nskb)->eor = TCP_SKB_CB(skb)->eor;
-                       tcp_skb_collapse_tstamp(nskb, skb);
-                       tcp_unlink_write_queue(skb, sk);
-                       tcp_wmem_free_skb(sk, skb);
+                       tcp_eat_one_skb(sk, nskb, skb);
                } else {
                        TCP_SKB_CB(nskb)->tcp_flags |= TCP_SKB_CB(skb)->tcp_flags &
                                                   ~(TCPHDR_FIN|TCPHDR_PSH);
@@ -2705,11 +2711,10 @@ static void tcp_grow_skb(struct sock *sk, struct sk_buff *skb, int amount)
        TCP_SKB_CB(next_skb)->seq += nlen;
 
        if (!next_skb->len) {
+               /* In case FIN is set, we need to update end_seq */
                TCP_SKB_CB(skb)->end_seq = TCP_SKB_CB(next_skb)->end_seq;
-               TCP_SKB_CB(skb)->eor = TCP_SKB_CB(next_skb)->eor;
-               TCP_SKB_CB(skb)->tcp_flags |= TCP_SKB_CB(next_skb)->tcp_flags;
-               tcp_unlink_write_queue(next_skb, sk);
-               tcp_wmem_free_skb(sk, next_skb);
+
+               tcp_eat_one_skb(sk, skb, next_skb);
        }
 }