[PATCH net] net: add and use skb_unclone_keeptruesize() helper

Subsystems: networking [general], networking [tcp], the rest

STALE1744d LANDED

Landed in mainline as c4777efa751d on 2021-11-03.

3 messages, 3 authors, 2021-11-03 · open the first message on its own page

[PATCH net] net: add and use skb_unclone_keeptruesize() helper

From: Eric Dumazet <hidden>
Date: 2021-11-02 00:46:12

From: Eric Dumazet <edumazet@google.com>

While commit 097b9146c0e2 ("net: fix up truesize of cloned
skb in skb_prepare_for_shift()") fixed immediate issues found
when KFENCE was enabled/tested, there are still similar issues,
when tcp_trim_head() hits KFENCE while the master skb
is cloned.

This happens under heavy networking TX workloads,
when the TX completion might be delayed after incoming ACK.

This patch fixes the WARNING in sk_stream_kill_queues
when sk->sk_mem_queued/sk->sk_forward_alloc are not zero.

Fixes: d3fb45f370d9 ("mm, kfence: insert KFENCE hooks for SLAB")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Marco Elver <elver@google.com>
---
 include/linux/skbuff.h | 16 ++++++++++++++++
 net/core/skbuff.c      | 14 +-------------
 net/ipv4/tcp_output.c  |  6 +++---
 3 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 841e2f0f5240ba9e210bb9a3fc1cbedc2162b2a8..b8c273af2910c780dcfbc8f18fc05e115089010b 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1671,6 +1671,22 @@ static inline int skb_unclone(struct sk_buff *skb, gfp_t pri)
 	return 0;
 }
 
+/* This variant of skb_unclone() makes sure skb->truesize is not changed */
+static inline int skb_unclone_keeptruesize(struct sk_buff *skb, gfp_t pri)
+{
+	might_sleep_if(gfpflags_allow_blocking(pri));
+
+	if (skb_cloned(skb)) {
+		unsigned int save = skb->truesize;
+		int res;
+
+		res = pskb_expand_head(skb, 0, 0, pri);
+		skb->truesize = save;
+		return res;
+	}
+	return 0;
+}
+
 /**
  *	skb_header_cloned - is the header a clone
  *	@skb: buffer to check
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index fe9358437380c826d6438efe939afc4b38135cff..38d7dee4bbe9e96a811ff9cfca33429b5f7dbff1 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3449,19 +3449,7 @@ EXPORT_SYMBOL(skb_split);
  */
 static int skb_prepare_for_shift(struct sk_buff *skb)
 {
-	int ret = 0;
-
-	if (skb_cloned(skb)) {
-		/* Save and restore truesize: pskb_expand_head() may reallocate
-		 * memory where ksize(kmalloc(S)) != ksize(kmalloc(S)), but we
-		 * cannot change truesize at this point.
-		 */
-		unsigned int save_truesize = skb->truesize;
-
-		ret = pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
-		skb->truesize = save_truesize;
-	}
-	return ret;
+	return skb_unclone_keeptruesize(skb, GFP_ATOMIC);
 }
 
 /**
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 6d72f3ea48c4ef0d193ec804653e4d4321f3f20a..0492f6942778db21f855216bf4387682fb37091e 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1562,7 +1562,7 @@ int tcp_fragment(struct sock *sk, enum tcp_queue tcp_queue,
 		return -ENOMEM;
 	}
 
-	if (skb_unclone(skb, gfp))
+	if (skb_unclone_keeptruesize(skb, gfp))
 		return -ENOMEM;
 
 	/* Get a new skb... force flag on. */
@@ -1672,7 +1672,7 @@ int tcp_trim_head(struct sock *sk, struct sk_buff *skb, u32 len)
 {
 	u32 delta_truesize;
 
-	if (skb_unclone(skb, GFP_ATOMIC))
+	if (skb_unclone_keeptruesize(skb, GFP_ATOMIC))
 		return -ENOMEM;
 
 	delta_truesize = __pskb_trim_head(skb, len);
@@ -3184,7 +3184,7 @@ int __tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb, int segs)
 				 cur_mss, GFP_ATOMIC))
 			return -ENOMEM; /* We'll try again later. */
 	} else {
-		if (skb_unclone(skb, GFP_ATOMIC))
+		if (skb_unclone_keeptruesize(skb, GFP_ATOMIC))
 			return -ENOMEM;
 
 		diff = tcp_skb_pcount(skb);
-- 
2.33.1.1089.g2158813163f-goog

Re: [PATCH net] net: add and use skb_unclone_keeptruesize() helper

From: Marco Elver <elver@google.com>
Date: 2021-11-02 08:41:02

On Tue, 2 Nov 2021 at 01:46, Eric Dumazet [off-list ref] wrote:
From: Eric Dumazet <edumazet@google.com>

While commit 097b9146c0e2 ("net: fix up truesize of cloned
skb in skb_prepare_for_shift()") fixed immediate issues found
when KFENCE was enabled/tested, there are still similar issues,
when tcp_trim_head() hits KFENCE while the master skb
is cloned.

This happens under heavy networking TX workloads,
when the TX completion might be delayed after incoming ACK.

This patch fixes the WARNING in sk_stream_kill_queues
when sk->sk_mem_queued/sk->sk_forward_alloc are not zero.

Fixes: d3fb45f370d9 ("mm, kfence: insert KFENCE hooks for SLAB")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Marco Elver <elver@google.com>
Acked-by: Marco Elver <elver@google.com>

Thanks.
quoted hunk
---
 include/linux/skbuff.h | 16 ++++++++++++++++
 net/core/skbuff.c      | 14 +-------------
 net/ipv4/tcp_output.c  |  6 +++---
 3 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 841e2f0f5240ba9e210bb9a3fc1cbedc2162b2a8..b8c273af2910c780dcfbc8f18fc05e115089010b 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1671,6 +1671,22 @@ static inline int skb_unclone(struct sk_buff *skb, gfp_t pri)
        return 0;
 }

+/* This variant of skb_unclone() makes sure skb->truesize is not changed */
+static inline int skb_unclone_keeptruesize(struct sk_buff *skb, gfp_t pri)
+{
+       might_sleep_if(gfpflags_allow_blocking(pri));
+
+       if (skb_cloned(skb)) {
+               unsigned int save = skb->truesize;
+               int res;
+
+               res = pskb_expand_head(skb, 0, 0, pri);
+               skb->truesize = save;
+               return res;
+       }
+       return 0;
+}
+
 /**
  *     skb_header_cloned - is the header a clone
  *     @skb: buffer to check
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index fe9358437380c826d6438efe939afc4b38135cff..38d7dee4bbe9e96a811ff9cfca33429b5f7dbff1 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3449,19 +3449,7 @@ EXPORT_SYMBOL(skb_split);
  */
 static int skb_prepare_for_shift(struct sk_buff *skb)
 {
-       int ret = 0;
-
-       if (skb_cloned(skb)) {
-               /* Save and restore truesize: pskb_expand_head() may reallocate
-                * memory where ksize(kmalloc(S)) != ksize(kmalloc(S)), but we
-                * cannot change truesize at this point.
-                */
-               unsigned int save_truesize = skb->truesize;
-
-               ret = pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
-               skb->truesize = save_truesize;
-       }
-       return ret;
+       return skb_unclone_keeptruesize(skb, GFP_ATOMIC);
 }

 /**
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 6d72f3ea48c4ef0d193ec804653e4d4321f3f20a..0492f6942778db21f855216bf4387682fb37091e 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1562,7 +1562,7 @@ int tcp_fragment(struct sock *sk, enum tcp_queue tcp_queue,
                return -ENOMEM;
        }

-       if (skb_unclone(skb, gfp))
+       if (skb_unclone_keeptruesize(skb, gfp))
                return -ENOMEM;

        /* Get a new skb... force flag on. */
@@ -1672,7 +1672,7 @@ int tcp_trim_head(struct sock *sk, struct sk_buff *skb, u32 len)
 {
        u32 delta_truesize;

-       if (skb_unclone(skb, GFP_ATOMIC))
+       if (skb_unclone_keeptruesize(skb, GFP_ATOMIC))
                return -ENOMEM;

        delta_truesize = __pskb_trim_head(skb, len);
@@ -3184,7 +3184,7 @@ int __tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb, int segs)
                                 cur_mss, GFP_ATOMIC))
                        return -ENOMEM; /* We'll try again later. */
        } else {
-               if (skb_unclone(skb, GFP_ATOMIC))
+               if (skb_unclone_keeptruesize(skb, GFP_ATOMIC))
                        return -ENOMEM;

                diff = tcp_skb_pcount(skb);
--
2.33.1.1089.g2158813163f-goog

Re: [PATCH net] net: add and use skb_unclone_keeptruesize() helper

From: patchwork-bot+netdevbpf@kernel.org
Date: 2021-11-03 02:30:13

Hello:

This patch was applied to netdev/net.git (master)
by Jakub Kicinski [off-list ref]:

On Mon,  1 Nov 2021 17:45:55 -0700 you wrote:
From: Eric Dumazet <edumazet@google.com>

While commit 097b9146c0e2 ("net: fix up truesize of cloned
skb in skb_prepare_for_shift()") fixed immediate issues found
when KFENCE was enabled/tested, there are still similar issues,
when tcp_trim_head() hits KFENCE while the master skb
is cloned.

[...]
Here is the summary with links:
  - [net] net: add and use skb_unclone_keeptruesize() helper
    https://git.kernel.org/netdev/net/c/c4777efa751d

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help