* [PATCH net-next v2 0/6] net: unify symmetric netmem and page_pool refcounting
@ 2026-10-12 8:21 Mina Almasry
2026-10-12 8:21 ` [PATCH net-next v2 1/6] netmem: rename __get/__put_netmem() to get/put_net_iov() Mina Almasry
` (5 more replies)
0 siblings, 6 replies; 18+ messages in thread
From: Mina Almasry @ 2026-10-12 8:21 UTC (permalink / raw)
To: netdev, linux-kernel, linux-rdma, bpf
Cc: Mina Almasry, Ayush Sawal, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Tariq Toukan,
Simon Horman, Steffen Klassert, Herbert Xu, Neal Cardwell,
Kuniyuki Iwashima, John Fastabend, Sabrina Dubroca, Eric Biggers,
Kees Cook, Aleksandr Loktionov,
Uwe Kleine-König (The Capable Hub), Jijie Shao,
Michael Grzeschik, Jakub Raczynski
netmems (pages and net_iovs) have two independent refcounts: a non-pp
refcount (page._refcount or binding->ref) governing backing memory
lifetime, and pp_ref_count governing recycling within a page_pool while
page_pool holds a single backing non-pp reference. SKBs with
pp_recycle=1 own pp_ref_count references on page_pool fragments; SKBs
with pp_recycle=0 own non-pp references.
Core helpers currently mix these two refcounts asymmetrically:
skb_frag_ref() always increments the non-pp refcount via get_netmem(),
whereas skb_frag_unref() decrements pp_ref_count when skb->pp_recycle
is set. Any path that refs a fragment on a pp_recycle=1 SKB (or unrefs
with a hardcoded recycle=false) increments one counter and decrements
the other, leaking the backing memory while underflowing pp_ref_count
(e.g., IP-TFS [1], skb_split(), skb_shift(), and cloned SKB uncloning).
Unclear core APIs also led drivers and ULPs to open-code pp_ref_count
manipulations or add one-off helpers like skb_pp_frag_ref().
Unify fragment refcounting into symmetric pairs where each layer clearly
specifies which refcount it touches:
- Non-PP refcount: get/put_page(), get/put_net_iov(), get/put_netmem()
- PP refcount: napi_pp_get/put_page()
- SKB netmem: skb_netmem_ref/unref(netmem, recycle)
- SKB fragment: skb_frag_ref/unref(skb, f)
Found by code inspection while reviewing [1] and confirmed with the KUnit
tests in patch 6; not observed in production. Driver and ULP call-site
changes are compile-tested only.
Link: https://lore.kernel.org/netdev/xfrm-iptfs-pp_ref_count-underflow-v4-1-912fa72106f0@secunet.com/ [1]
---
v2:
- Clear NETIF_F_SG in skb_segment() when frag_list SKBs mix pp_recycle
flags (patch 4).
- Export skb_shift() via EXPORT_SYMBOL_IF_KUNIT() to fix the
CONFIG_NET_TEST=m modpost error reported by NIPA (patch 6).
- Reset mac_header before skb_segment() and check skb_clone() results in
KUnit (Sashiko), and extend KUnit coverage to compound tail pages,
skb_try_coalesce(), TCP devmem recvmsg, skb_zerocopy() rollbacks,
skb_split()/skb_shift() edge cases, and mixed pp_recycle frag_list GSO.
- Note how the issues were found and tested (netdev-bot), and add
Assisted-by tags.
- Link to v1: https://lore.kernel.org/netdev/20261010083935.3274178-1-almasrymina@google.com/
Mina Almasry (6):
netmem: rename __get/__put_netmem() to get/put_net_iov()
net: skbuff: add napi_pp_get_page() and use it in tcp_recvmsg_dmabuf()
net: skbuff: replace skb_page_unref() and __skb_frag_unref() with
skb_netmem_unref()
net: skbuff: replace __skb_frag_ref() with symmetric skb_netmem_ref()
net: skbuff: use skb_frag_ref() in skb_try_coalesce()
net: kunit: test netmem, page_pool, and skb frag refcounting
.../chelsio/inline_crypto/ch_ktls/chcr_ktls.c | 2 +-
drivers/net/ethernet/marvell/sky2.c | 2 +-
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 2 +-
drivers/net/ethernet/sun/cassini.c | 4 +-
drivers/net/veth.c | 2 +-
include/linux/skbuff_ref.h | 64 +-
include/net/netmem.h | 24 +-
net/core/net_test.c | 1204 +++++++++++++++++
net/core/skbuff.c | 161 ++-
net/ipv4/esp4.c | 4 +-
net/ipv4/tcp.c | 6 +-
net/ipv6/esp6.c | 4 +-
net/tls/tls_device.c | 2 +-
net/tls/tls_device_fallback.c | 2 +-
net/tls/tls_strp.c | 2 +-
net/xfrm/xfrm_iptfs.c | 3 +-
16 files changed, 1373 insertions(+), 115 deletions(-)
base-commit: d8674294aefef02266c4d47ad10131f1bffbe534
--
2.56.0.385.gd3acb90ef8-goog
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH net-next v2 1/6] netmem: rename __get/__put_netmem() to get/put_net_iov()
2026-10-12 8:21 [PATCH net-next v2 0/6] net: unify symmetric netmem and page_pool refcounting Mina Almasry
@ 2026-10-12 8:21 ` Mina Almasry
2026-10-13 8:22 ` sashiko-bot
2026-10-13 8:34 ` netdev-bot+sashiko
2026-10-12 8:21 ` [PATCH net-next v2 2/6] net: skbuff: add napi_pp_get_page() and use it in tcp_recvmsg_dmabuf() Mina Almasry
` (4 subsequent siblings)
5 siblings, 2 replies; 18+ messages in thread
From: Mina Almasry @ 2026-10-12 8:21 UTC (permalink / raw)
To: netdev, linux-kernel, linux-rdma, bpf
Cc: Mina Almasry, Ayush Sawal, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Tariq Toukan,
Simon Horman, Steffen Klassert, Herbert Xu, Neal Cardwell,
Kuniyuki Iwashima, John Fastabend, Sabrina Dubroca, Eric Biggers,
Kees Cook, Aleksandr Loktionov,
Uwe Kleine-König (The Capable Hub), Jijie Shao,
Michael Grzeschik, Jakub Raczynski
netmems carry both a non-pp backing refcount and a page_pool
pp_ref_count. Make the non-pp layer explicit and symmetric across both
netmem types: get/put_netmem() dispatches to get/put_page() for pages
and get/put_net_iov() for net_iovs, always acquiring or releasing a
non-pp reference regardless of page_pool membership.
Warn via DEBUG_NET_WARN_ON_ONCE() for net_iov types that do not support
non-pp references.
Assisted-by: LLM
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
include/net/netmem.h | 24 ++++++++++++++++++++----
net/core/skbuff.c | 36 ++++++++++++++++++++++++++----------
2 files changed, 46 insertions(+), 14 deletions(-)
diff --git a/include/net/netmem.h b/include/net/netmem.h
index 0cc8572f62caf..697d50c51a5ee 100644
--- a/include/net/netmem.h
+++ b/include/net/netmem.h
@@ -399,21 +399,37 @@ static inline bool net_is_devmem_iov(const struct net_iov *niov)
}
#endif
-void __get_netmem(netmem_ref netmem);
-void __put_netmem(netmem_ref netmem);
+void get_net_iov(struct net_iov *niov);
+void put_net_iov(struct net_iov *niov);
+/**
+ * get_netmem - acquire a non-page_pool reference on a netmem
+ * @netmem: netmem to reference
+ *
+ * Acquires a non-pp backing reference (page._refcount via get_page() or
+ * binding->ref via get_net_iov()) regardless of page_pool membership.
+ * Counterpart to put_netmem().
+ */
static __always_inline void get_netmem(netmem_ref netmem)
{
if (netmem_is_net_iov(netmem))
- __get_netmem(netmem);
+ get_net_iov(netmem_to_net_iov(netmem));
else
get_page(netmem_to_page(netmem));
}
+/**
+ * put_netmem - release a non-page_pool reference on a netmem
+ * @netmem: netmem to unreference
+ *
+ * Drops a non-pp backing reference (page._refcount via put_page() or
+ * binding->ref via put_net_iov()) regardless of page_pool membership.
+ * Counterpart to get_netmem().
+ */
static __always_inline void put_netmem(netmem_ref netmem)
{
if (netmem_is_net_iov(netmem))
- __put_netmem(netmem);
+ put_net_iov(netmem_to_net_iov(netmem));
else
put_page(netmem_to_page(netmem));
}
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index d56f4f0102f75..b459264ba5180 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -7551,23 +7551,39 @@ bool csum_and_copy_from_iter_full(void *addr, size_t bytes,
}
EXPORT_SYMBOL(csum_and_copy_from_iter_full);
-void __get_netmem(netmem_ref netmem)
+/**
+ * get_net_iov - acquire a non-page_pool reference on a net_iov
+ * @niov: net_iov to reference
+ *
+ * Acquires a non-pp backing reference (binding->ref) on @niov regardless of
+ * page_pool membership, warning if @niov does not support non-pp references.
+ * Counterpart to put_net_iov().
+ */
+void get_net_iov(struct net_iov *niov)
{
- struct net_iov *niov = netmem_to_net_iov(netmem);
-
if (net_is_devmem_iov(niov))
- net_devmem_get_net_iov(netmem_to_net_iov(netmem));
+ net_devmem_get_net_iov(niov);
+ else
+ DEBUG_NET_WARN_ON_ONCE(true);
}
-EXPORT_SYMBOL(__get_netmem);
+EXPORT_SYMBOL(get_net_iov);
-void __put_netmem(netmem_ref netmem)
+/**
+ * put_net_iov - release a non-page_pool reference on a net_iov
+ * @niov: net_iov to unreference
+ *
+ * Drops a non-pp backing reference (binding->ref) on @niov regardless of
+ * page_pool membership, warning if @niov does not support non-pp references.
+ * Counterpart to get_net_iov().
+ */
+void put_net_iov(struct net_iov *niov)
{
- struct net_iov *niov = netmem_to_net_iov(netmem);
-
if (net_is_devmem_iov(niov))
- net_devmem_put_net_iov(netmem_to_net_iov(netmem));
+ net_devmem_put_net_iov(niov);
+ else
+ DEBUG_NET_WARN_ON_ONCE(true);
}
-EXPORT_SYMBOL(__put_netmem);
+EXPORT_SYMBOL(put_net_iov);
struct vlan_type_depth __vlan_get_protocol_offset(const struct sk_buff *skb,
__be16 type,
--
2.56.0.385.gd3acb90ef8-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH net-next v2 2/6] net: skbuff: add napi_pp_get_page() and use it in tcp_recvmsg_dmabuf()
2026-10-12 8:21 [PATCH net-next v2 0/6] net: unify symmetric netmem and page_pool refcounting Mina Almasry
2026-10-12 8:21 ` [PATCH net-next v2 1/6] netmem: rename __get/__put_netmem() to get/put_net_iov() Mina Almasry
@ 2026-10-12 8:21 ` Mina Almasry
2026-10-13 8:22 ` sashiko-bot
2026-10-12 8:21 ` [PATCH net-next v2 3/6] net: skbuff: replace skb_page_unref() and __skb_frag_unref() with skb_netmem_unref() Mina Almasry
` (3 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Mina Almasry @ 2026-10-12 8:21 UTC (permalink / raw)
To: netdev, linux-kernel, linux-rdma, bpf
Cc: Mina Almasry, Ayush Sawal, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Tariq Toukan,
Simon Horman, Steffen Klassert, Herbert Xu, Neal Cardwell,
Kuniyuki Iwashima, John Fastabend, Sabrina Dubroca, Eric Biggers,
Kees Cook, Aleksandr Loktionov,
Uwe Kleine-König (The Capable Hub), Jijie Shao,
Michael Grzeschik, Jakub Raczynski
While napi_pp_put_page() releases a page_pool pp_ref_count reference on
a netmem, no symmetric helper existed to acquire one, forcing callers
like tcp_recvmsg_dmabuf() to open-code atomic_long_inc() on
niov->desc.pp_ref_count.
Add napi_pp_get_page() to pair symmetrically with napi_pp_put_page(),
and use WARN_ON_ONCE(!napi_pp_get_page(netmem)) in tcp_recvmsg_dmabuf()
to match WARN_ON_ONCE(!napi_pp_put_page(netmem)) in
tcp_release_user_frags().
Assisted-by: LLM
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
include/linux/skbuff_ref.h | 2 ++
net/core/skbuff.c | 31 +++++++++++++++++++++++++++++++
net/ipv4/tcp.c | 6 ++++--
3 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/include/linux/skbuff_ref.h b/include/linux/skbuff_ref.h
index 05c8486bafac8..0441c220fdbdf 100644
--- a/include/linux/skbuff_ref.h
+++ b/include/linux/skbuff_ref.h
@@ -9,6 +9,8 @@
#include <linux/skbuff.h>
+bool napi_pp_get_page(netmem_ref netmem);
+
/**
* __skb_frag_ref - take an addition reference on a paged fragment.
* @frag: the paged fragment
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index b459264ba5180..8a8be746828db 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1026,6 +1026,15 @@ int skb_cow_data_for_xdp(struct page_pool *pool, struct sk_buff **pskb,
EXPORT_SYMBOL(skb_cow_data_for_xdp);
#if IS_ENABLED(CONFIG_PAGE_POOL)
+/**
+ * napi_pp_put_page - release a page_pool reference on a netmem
+ * @netmem: netmem to unreference
+ *
+ * Drops a page_pool reference (pp_ref_count) on @netmem if it belongs to a
+ * page_pool. Counterpart to napi_pp_get_page().
+ *
+ * Return: true if @netmem belongs to a page_pool, false otherwise.
+ */
bool napi_pp_put_page(netmem_ref netmem)
{
netmem = netmem_compound_head(netmem);
@@ -1038,6 +1047,28 @@ bool napi_pp_put_page(netmem_ref netmem)
return true;
}
EXPORT_SYMBOL(napi_pp_put_page);
+
+/**
+ * napi_pp_get_page - acquire a page_pool reference on a netmem
+ * @netmem: netmem to reference
+ *
+ * Acquires a page_pool reference (pp_ref_count) on @netmem if it belongs to a
+ * page_pool. Counterpart to napi_pp_put_page().
+ *
+ * Return: true if @netmem belongs to a page_pool, false otherwise.
+ */
+bool napi_pp_get_page(netmem_ref netmem)
+{
+ netmem = netmem_compound_head(netmem);
+
+ if (unlikely(!netmem_is_pp(netmem)))
+ return false;
+
+ page_pool_ref_netmem(netmem);
+
+ return true;
+}
+EXPORT_SYMBOL(napi_pp_get_page);
#endif
static bool skb_pp_recycle(struct sk_buff *skb, void *data)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index f4c9101e5996a..99d7237a623d8 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -254,6 +254,7 @@
#include <linux/init.h>
#include <linux/fs.h>
#include <linux/skbuff.h>
+#include <linux/skbuff_ref.h>
#include <linux/splice.h>
#include <linux/net.h>
#include <linux/socket.h>
@@ -2559,6 +2560,7 @@ static int tcp_recvmsg_dmabuf(struct sock *sk, const struct sk_buff *skb,
*/
for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
+ netmem_ref netmem = skb_frag_netmem(frag);
struct net_iov *niov;
u64 frag_offset;
int end;
@@ -2612,8 +2614,8 @@ static int tcp_recvmsg_dmabuf(struct sock *sk, const struct sk_buff *skb,
if (err)
goto out;
- atomic_long_inc(&niov->desc.pp_ref_count);
- tcp_xa_pool.netmems[tcp_xa_pool.idx++] = skb_frag_netmem(frag);
+ WARN_ON_ONCE(!napi_pp_get_page(netmem));
+ tcp_xa_pool.netmems[tcp_xa_pool.idx++] = netmem;
sent += copy;
--
2.56.0.385.gd3acb90ef8-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH net-next v2 3/6] net: skbuff: replace skb_page_unref() and __skb_frag_unref() with skb_netmem_unref()
2026-10-12 8:21 [PATCH net-next v2 0/6] net: unify symmetric netmem and page_pool refcounting Mina Almasry
2026-10-12 8:21 ` [PATCH net-next v2 1/6] netmem: rename __get/__put_netmem() to get/put_net_iov() Mina Almasry
2026-10-12 8:21 ` [PATCH net-next v2 2/6] net: skbuff: add napi_pp_get_page() and use it in tcp_recvmsg_dmabuf() Mina Almasry
@ 2026-10-12 8:21 ` Mina Almasry
2026-10-13 8:22 ` sashiko-bot
2026-10-13 8:34 ` netdev-bot+sashiko
2026-10-12 8:21 ` [PATCH net-next v2 4/6] net: skbuff: replace __skb_frag_ref() with symmetric skb_netmem_ref() Mina Almasry
` (2 subsequent siblings)
5 siblings, 2 replies; 18+ messages in thread
From: Mina Almasry @ 2026-10-12 8:21 UTC (permalink / raw)
To: netdev, linux-kernel, linux-rdma, bpf
Cc: Mina Almasry, Ayush Sawal, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Tariq Toukan,
Simon Horman, Steffen Klassert, Herbert Xu, Neal Cardwell,
Kuniyuki Iwashima, John Fastabend, Sabrina Dubroca, Eric Biggers,
Kees Cook, Aleksandr Loktionov,
Uwe Kleine-König (The Capable Hub), Jijie Shao,
Michael Grzeschik, Jakub Raczynski
Unreferencing a page_pool fragment with recycle=false decrements the
non-pp backing refcount instead of pp_ref_count, underflowing the page
refcount when the fragment was owned via page_pool (as on mlx4's RX
error path). Having both skb_page_unref() and __skb_frag_unref(frag,
recycle) made it easy for callers to pass hardcoded recycle=false on
SKB-owned fragments.
Replace skb_page_unref() and __skb_frag_unref() with
skb_netmem_unref(netmem, recycle), which dispatches to
napi_pp_put_page() when recycle is true and put_netmem() otherwise:
- Use skb_frag_unref(skb, ...) on SKB-owned fragments in mlx4, sky2,
tls_strp, and skb_shift() so unrefs always match skb->pp_recycle.
- Use put_netmem() directly in tls_device's destroy_record(), which
releases raw non-pp page references outside an SKB.
Found by code inspection and confirmed with the KUnit tests added later
in this series; not observed in production. The mlx4, sky2, and TLS
call-site changes are compile-tested only.
Assisted-by: LLM
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
v2:
- Note how the issue was found and how the change was tested.
- Link to v1: https://lore.kernel.org/netdev/20261010083935.3274178-4-almasrymina@google.com/
---
drivers/net/ethernet/marvell/sky2.c | 2 +-
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 2 +-
include/linux/skbuff_ref.h | 34 +++++++++++-----------
net/core/skbuff.c | 7 +++--
net/ipv4/esp4.c | 4 +--
net/ipv6/esp6.c | 4 +--
net/tls/tls_device.c | 2 +-
net/tls/tls_strp.c | 2 +-
8 files changed, 29 insertions(+), 28 deletions(-)
diff --git a/drivers/net/ethernet/marvell/sky2.c b/drivers/net/ethernet/marvell/sky2.c
index 30facdece3e29..1d42a82db8f2b 100644
--- a/drivers/net/ethernet/marvell/sky2.c
+++ b/drivers/net/ethernet/marvell/sky2.c
@@ -2501,7 +2501,7 @@ static void skb_put_frags(struct sk_buff *skb, unsigned int hdr_space,
if (length == 0) {
/* don't need this page */
- __skb_frag_unref(frag, false);
+ skb_frag_unref(skb, i);
--skb_shinfo(skb)->nr_frags;
} else {
size = min(length, (unsigned) PAGE_SIZE);
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index 96f97fa0f5489..fe18606c2ab55 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -497,7 +497,7 @@ static int mlx4_en_complete_rx_desc(struct mlx4_en_priv *priv,
fail:
while (nr > 0) {
nr--;
- __skb_frag_unref(skb_shinfo(skb)->frags + nr, false);
+ skb_frag_unref(skb, nr);
}
return 0;
}
diff --git a/include/linux/skbuff_ref.h b/include/linux/skbuff_ref.h
index 0441c220fdbdf..04114c64dad99 100644
--- a/include/linux/skbuff_ref.h
+++ b/include/linux/skbuff_ref.h
@@ -36,7 +36,16 @@ static __always_inline void skb_frag_ref(struct sk_buff *skb, int f)
bool napi_pp_put_page(netmem_ref netmem);
-static __always_inline void skb_page_unref(netmem_ref netmem, bool recycle)
+/**
+ * skb_netmem_unref - release a fragment reference on a netmem
+ * @netmem: netmem to unreference
+ * @recycle: whether page_pool recycling is enabled (e.g. skb->pp_recycle)
+ *
+ * Drops pp_ref_count via napi_pp_put_page() if @recycle is true and @netmem
+ * belongs to a page_pool; otherwise drops a non-pp backing reference via
+ * put_netmem(). Counterpart to skb_netmem_ref().
+ */
+static __always_inline void skb_netmem_unref(netmem_ref netmem, bool recycle)
{
#ifdef CONFIG_PAGE_POOL
if (recycle && napi_pp_put_page(netmem))
@@ -46,31 +55,22 @@ static __always_inline void skb_page_unref(netmem_ref netmem, bool recycle)
}
/**
- * __skb_frag_unref - release a reference on a paged fragment.
- * @frag: the paged fragment
- * @recycle: recycle the page if allocated via page_pool
- *
- * Releases a reference on the paged fragment @frag
- * or recycles the page via the page_pool API.
- */
-static __always_inline void __skb_frag_unref(skb_frag_t *frag, bool recycle)
-{
- skb_page_unref(skb_frag_netmem(frag), recycle);
-}
-
-/**
- * skb_frag_unref - release a reference on a paged fragment of an skb.
+ * skb_frag_unref - release a reference on a paged fragment of an skb
* @skb: the buffer
* @f: the fragment offset
*
- * Releases a reference on the @f'th paged fragment of @skb.
+ * Drops a reference on the @f'th fragment of @skb via skb_netmem_unref()
+ * (pp_ref_count if skb->pp_recycle and page_pool-backed, else non-pp
+ * backing refcount), skipping managed zerocopy fragments. Counterpart to
+ * skb_frag_ref().
*/
static __always_inline void skb_frag_unref(struct sk_buff *skb, int f)
{
struct skb_shared_info *shinfo = skb_shinfo(skb);
if (!skb_zcopy_managed(skb))
- __skb_frag_unref(&shinfo->frags[f], skb->pp_recycle);
+ skb_netmem_unref(skb_frag_netmem(&shinfo->frags[f]),
+ skb->pp_recycle);
}
#endif /* _LINUX_SKBUFF_REF_H */
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 8a8be746828db..46da61ad1f9eb 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1144,7 +1144,8 @@ static void skb_release_data(struct sk_buff *skb, enum skb_drop_reason reason)
}
for (i = 0; i < shinfo->nr_frags; i++)
- __skb_frag_unref(&shinfo->frags[i], skb->pp_recycle);
+ skb_netmem_unref(skb_frag_netmem(&shinfo->frags[i]),
+ skb->pp_recycle);
free_head:
if (shinfo->frag_list)
@@ -1154,7 +1155,7 @@ static void skb_release_data(struct sk_buff *skb, enum skb_drop_reason reason)
exit:
/* When we clone an SKB we copy the reycling bit. The pp_recycle
* bit is only set on the head though, so in order to avoid races
- * while trying to recycle fragments on __skb_frag_unref() we need
+ * while trying to recycle fragments on skb_netmem_unref() we need
* to make one SKB responsible for triggering the recycle path.
* So disable the recycling bit if an SKB is cloned and we have
* additional references to the fragmented part of the SKB.
@@ -4412,7 +4413,7 @@ int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen)
fragto = &skb_shinfo(tgt)->frags[merge];
skb_frag_size_add(fragto, skb_frag_size(fragfrom));
- __skb_frag_unref(fragfrom, skb->pp_recycle);
+ skb_frag_unref(skb, 0);
}
/* Reposition in the original skb */
diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index e76db5817e78e..ae701561d9f12 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -117,8 +117,8 @@ static void esp_ssg_unref(struct xfrm_state *x, void *tmp, struct sk_buff *skb,
struct scatterlist *src = already_unref ? esp_req_sg(aead, req) : req->src;
for (sg = sg_next(src); sg; sg = sg_next(sg))
- skb_page_unref(page_to_netmem(sg_page(sg)),
- skb->pp_recycle);
+ skb_netmem_unref(page_to_netmem(sg_page(sg)),
+ skb->pp_recycle);
}
}
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index b1c9b36f76dc4..d654f0f442388 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -134,8 +134,8 @@ static void esp_ssg_unref(struct xfrm_state *x, void *tmp, struct sk_buff *skb,
struct scatterlist *src = already_unref ? esp_req_sg(aead, req) : req->src;
for (sg = sg_next(src); sg; sg = sg_next(sg))
- skb_page_unref(page_to_netmem(sg_page(sg)),
- skb->pp_recycle);
+ skb_netmem_unref(page_to_netmem(sg_page(sg)),
+ skb->pp_recycle);
}
}
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index f11d0528fc431..b333a9a72cdae 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -143,7 +143,7 @@ static void destroy_record(struct tls_record_info *record)
int i;
for (i = 0; i < record->num_frags; i++)
- __skb_frag_unref(&record->frags[i], false);
+ put_netmem(skb_frag_netmem(&record->frags[i]));
kfree(record);
}
diff --git a/net/tls/tls_strp.c b/net/tls/tls_strp.c
index 6cc222008d95c..53d3256d0114a 100644
--- a/net/tls/tls_strp.c
+++ b/net/tls/tls_strp.c
@@ -197,7 +197,7 @@ static void tls_strp_flush_anchor_copy(struct tls_strparser *strp)
DEBUG_NET_WARN_ON_ONCE(atomic_read(&shinfo->dataref) != 1);
for (i = 0; i < shinfo->nr_frags; i++)
- __skb_frag_unref(&shinfo->frags[i], false);
+ skb_frag_unref(strp->anchor, i);
shinfo->nr_frags = 0;
if (strp->copy_mode) {
kfree_skb_list(shinfo->frag_list);
--
2.56.0.385.gd3acb90ef8-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH net-next v2 4/6] net: skbuff: replace __skb_frag_ref() with symmetric skb_netmem_ref()
2026-10-12 8:21 [PATCH net-next v2 0/6] net: unify symmetric netmem and page_pool refcounting Mina Almasry
` (2 preceding siblings ...)
2026-10-12 8:21 ` [PATCH net-next v2 3/6] net: skbuff: replace skb_page_unref() and __skb_frag_unref() with skb_netmem_unref() Mina Almasry
@ 2026-10-12 8:21 ` Mina Almasry
2026-10-13 8:22 ` sashiko-bot
2026-10-13 8:34 ` netdev-bot+sashiko
2026-10-12 8:21 ` [PATCH net-next v2 5/6] net: skbuff: use skb_frag_ref() in skb_try_coalesce() Mina Almasry
2026-10-12 8:21 ` [PATCH net-next v2 6/6] net: kunit: test netmem, page_pool, and skb frag refcounting Mina Almasry
5 siblings, 2 replies; 18+ messages in thread
From: Mina Almasry @ 2026-10-12 8:21 UTC (permalink / raw)
To: netdev, linux-kernel, linux-rdma, bpf
Cc: Mina Almasry, Ayush Sawal, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Tariq Toukan,
Simon Horman, Steffen Klassert, Herbert Xu, Neal Cardwell,
Kuniyuki Iwashima, John Fastabend, Sabrina Dubroca, Eric Biggers,
Kees Cook, Aleksandr Loktionov,
Uwe Kleine-König (The Capable Hub), Jijie Shao,
Michael Grzeschik, Jakub Raczynski, Antony Antony,
Stanislav Fomichev, Daniel Borkmann, Alexei Starovoitov,
Jesper Dangaard Brouer
Previously, __skb_frag_ref() and skb_frag_ref() unconditionally
incremented the non-pp backing refcount via get_netmem(), while
skb_frag_unref() decremented pp_ref_count whenever skb->pp_recycle was
set. Ref-ing a page_pool fragment into a pp_recycle=1 SKB (or clearing
pp_recycle=0 on an uncloned SKB while clones still held pp_recycle=1)
leaked the backing non-pp refcount and underflowed pp_ref_count across
IP-TFS, skb_split(), skb_shift(), and cloned unclone paths.
Replace __skb_frag_ref() with skb_netmem_ref(netmem, recycle) as the
exact counterpart to skb_netmem_unref(netmem, recycle):
- Keep skb->pp_recycle intact in skb_release_data() so uncloning paths
(pskb_expand_head() and pskb_carve_inside_{header,nonlinear}()) keep
trading pp_ref_count references symmetrically across clones.
- Propagate pp_recycle and call skb_frag_ref() on the destination SKB in
__pskb_copy_fclone(), skb_zerocopy(), skb_split(), skb_shift(), and
skb_segment().
- Clear NETIF_F_SG in skb_segment() when frag_list SKBs mix pp_recycle
flags, since each output segment carries a single pp_recycle bit.
- Use skb_frag_ref() in xfrm_iptfs, chcr_ktls, and cassini, and call
get_netmem() directly in veth and tls_device_fallback where callers
explicitly take a raw non-pp reference.
Found by code inspection while reviewing the IP-TFS pp_ref_count fix [1]
and confirmed with the KUnit tests added later in this series; not
observed in production. The xfrm_iptfs, chcr_ktls, cassini, veth, and
tls_device_fallback changes are compile-tested only.
Link: https://lore.kernel.org/netdev/xfrm-iptfs-pp_ref_count-underflow-v4-1-912fa72106f0@secunet.com/ [1]
Cc: Antony Antony <antony.antony@secunet.com>
Cc: Stanislav Fomichev <sdf@fomichev.me>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Jesper Dangaard Brouer <hawk@kernel.org>
Cc: bpf@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
v2:
- Clear NETIF_F_SG in skb_segment() when frag_list SKBs mix pp_recycle
flags.
- Note how the issue was found and how the change was tested.
- Link to v1: https://lore.kernel.org/netdev/20261010083935.3274178-5-almasrymina@google.com/
---
.../chelsio/inline_crypto/ch_ktls/chcr_ktls.c | 2 +-
drivers/net/ethernet/sun/cassini.c | 4 +-
drivers/net/veth.c | 2 +-
include/linux/skbuff_ref.h | 28 ++++++----
net/core/skbuff.c | 52 +++++++++++--------
net/tls/tls_device_fallback.c | 2 +-
net/xfrm/xfrm_iptfs.c | 3 +-
7 files changed, 55 insertions(+), 38 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
index dbec95b497773..4d645771596a8 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
@@ -1660,7 +1660,7 @@ static void chcr_ktls_copy_record_in_skb(struct sk_buff *nskb,
for (i = 0; i < record->num_frags; i++) {
skb_shinfo(nskb)->frags[i] = record->frags[i];
/* increase the frag ref count */
- __skb_frag_ref(&skb_shinfo(nskb)->frags[i]);
+ skb_frag_ref(nskb, i);
}
skb_shinfo(nskb)->nr_frags = record->num_frags;
diff --git a/drivers/net/ethernet/sun/cassini.c b/drivers/net/ethernet/sun/cassini.c
index 9a8b5cf2d6485..02158a1bb2f70 100644
--- a/drivers/net/ethernet/sun/cassini.c
+++ b/drivers/net/ethernet/sun/cassini.c
@@ -1998,7 +1998,7 @@ static int cas_rx_process_pkt(struct cas *cp, struct cas_rx_comp *rxc,
skb->len += hlen - swivel;
skb_frag_fill_page_desc(frag, page->buffer, off, hlen - swivel);
- __skb_frag_ref(frag);
+ skb_frag_ref(skb, 0);
/* any more data? */
if ((words[0] & RX_COMP1_SPLIT_PKT) && ((dlen -= hlen) > 0)) {
@@ -2022,7 +2022,7 @@ static int cas_rx_process_pkt(struct cas *cp, struct cas_rx_comp *rxc,
frag++;
skb_frag_fill_page_desc(frag, page->buffer, 0, hlen);
- __skb_frag_ref(frag);
+ skb_frag_ref(skb, 1);
RX_USED_ADD(page, hlen + cp->crc_size);
}
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 643b97dc52456..90a333ebab683 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -746,7 +746,7 @@ static void veth_xdp_get(struct xdp_buff *xdp)
return;
for (i = 0; i < sinfo->nr_frags; i++)
- __skb_frag_ref(&sinfo->frags[i]);
+ get_netmem(skb_frag_netmem(&sinfo->frags[i]));
}
static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq,
diff --git a/include/linux/skbuff_ref.h b/include/linux/skbuff_ref.h
index 04114c64dad99..f768a3fa1d606 100644
--- a/include/linux/skbuff_ref.h
+++ b/include/linux/skbuff_ref.h
@@ -12,26 +12,36 @@
bool napi_pp_get_page(netmem_ref netmem);
/**
- * __skb_frag_ref - take an addition reference on a paged fragment.
- * @frag: the paged fragment
+ * skb_netmem_ref - acquire a fragment reference on a netmem
+ * @netmem: netmem to reference
+ * @recycle: whether page_pool recycling is enabled (e.g. skb->pp_recycle)
*
- * Takes an additional reference on the paged fragment @frag.
+ * Acquires pp_ref_count via napi_pp_get_page() if @recycle is true and
+ * @netmem belongs to a page_pool; otherwise acquires a non-pp backing
+ * reference via get_netmem(). Counterpart to skb_netmem_unref().
*/
-static __always_inline void __skb_frag_ref(skb_frag_t *frag)
+static __always_inline void skb_netmem_ref(netmem_ref netmem, bool recycle)
{
- get_netmem(skb_frag_netmem(frag));
+#ifdef CONFIG_PAGE_POOL
+ if (recycle && napi_pp_get_page(netmem))
+ return;
+#endif
+ get_netmem(netmem);
}
/**
- * skb_frag_ref - take an addition reference on a paged fragment of an skb.
+ * skb_frag_ref - acquire a reference on a paged fragment of an skb
* @skb: the buffer
- * @f: the fragment offset.
+ * @f: the fragment offset
*
- * Takes an additional reference on the @f'th paged fragment of @skb.
+ * Acquires a reference on the @f'th fragment of @skb via skb_netmem_ref()
+ * (pp_ref_count if skb->pp_recycle and page_pool-backed, else non-pp
+ * backing refcount). Counterpart to skb_frag_unref().
*/
static __always_inline void skb_frag_ref(struct sk_buff *skb, int f)
{
- __skb_frag_ref(&skb_shinfo(skb)->frags[f]);
+ skb_netmem_ref(skb_frag_netmem(&skb_shinfo(skb)->frags[f]),
+ skb->pp_recycle);
}
bool napi_pp_put_page(netmem_ref netmem);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 46da61ad1f9eb..fe73f6e8d2b3e 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1133,7 +1133,7 @@ static void skb_release_data(struct sk_buff *skb, enum skb_drop_reason reason)
int i;
if (!skb_data_unref(skb, shinfo))
- goto exit;
+ return;
if (skb_zcopy(skb)) {
bool skip_unref = shinfo->flags & SKBFL_MANAGED_FRAG_REFS;
@@ -1152,17 +1152,6 @@ static void skb_release_data(struct sk_buff *skb, enum skb_drop_reason reason)
kfree_skb_list_reason(shinfo->frag_list, reason);
skb_free_head(skb);
-exit:
- /* When we clone an SKB we copy the reycling bit. The pp_recycle
- * bit is only set on the head though, so in order to avoid races
- * while trying to recycle fragments on skb_netmem_unref() we need
- * to make one SKB responsible for triggering the recycle path.
- * So disable the recycling bit if an SKB is cloned and we have
- * additional references to the fragmented part of the SKB.
- * Eventually the last SKB will have the recycling bit set and it's
- * dataref set to 0, which will trigger the recycling
- */
- skb->pp_recycle = 0;
}
/*
@@ -2287,9 +2276,10 @@ struct sk_buff *__pskb_copy_fclone(struct sk_buff *skb, int headroom,
n = NULL;
goto out;
}
+ n->pp_recycle = skb->pp_recycle;
for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
skb_shinfo(n)->frags[i] = skb_shinfo(skb)->frags[i];
- skb_frag_ref(skb, i);
+ skb_frag_ref(n, i);
}
skb_shinfo(n)->nr_frags = i;
skb_shinfo(n)->flags |= skb_shinfo(skb)->flags & SKBFL_SHARED_FRAG;
@@ -3927,6 +3917,7 @@ skb_zerocopy(struct sk_buff *to, struct sk_buff *from, int len, int hlen)
if (len <= skb_tailroom(to))
return skb_copy_bits(from, 0, skb_put(to, len), len);
+ to->pp_recycle = from->pp_recycle;
if (hlen) {
ret = skb_copy_bits(from, 0, skb_put(to, hlen), hlen);
if (unlikely(ret))
@@ -3939,14 +3930,14 @@ skb_zerocopy(struct sk_buff *to, struct sk_buff *from, int len, int hlen)
offset = from->data - (unsigned char *)page_address(page);
__skb_fill_netmem_desc(to, 0, page_to_netmem(page),
offset, plen);
- get_page(page);
+ skb_frag_ref(to, 0);
j = 1;
len -= plen;
}
}
if (!skb_frags_readable(from) && j > 0 && len) {
- put_page(page);
+ skb_frag_unref(to, 0);
return -EFAULT;
}
@@ -3954,7 +3945,7 @@ skb_zerocopy(struct sk_buff *to, struct sk_buff *from, int len, int hlen)
if (unlikely(skb_orphan_frags(from, GFP_ATOMIC))) {
if (j > 0)
- put_page(page);
+ skb_frag_unref(to, 0);
return -ENOMEM;
}
skb_zerocopy_clone(to, from, GFP_ATOMIC);
@@ -4255,7 +4246,7 @@ static inline void skb_split_no_header(struct sk_buff *skb,
* where splitting is expensive.
* 2. Split is accurately. We make this.
*/
- skb_frag_ref(skb, i);
+ skb_frag_ref(skb1, k);
skb_frag_off_add(&skb_shinfo(skb1)->frags[0], len - pos);
skb_frag_size_sub(&skb_shinfo(skb1)->frags[0], len - pos);
skb_frag_size_set(&skb_shinfo(skb)->frags[i], len - pos);
@@ -4286,6 +4277,7 @@ void skb_split(struct sk_buff *skb, struct sk_buff *skb1, const u32 len)
skb_shinfo(skb1)->flags |= skb_shinfo(skb)->flags & zc_flags;
skb_zerocopy_clone(skb1, skb, 0);
+ skb1->pp_recycle = skb->pp_recycle;
if (len < pos) /* Split line is inside header. */
skb_split_inside_header(skb, skb1, len, pos);
else /* Second chunk has no header, nothing to copy. */
@@ -4343,8 +4335,8 @@ int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen)
/* Actual merge is delayed until the point when we know we can
* commit all, so that we don't have to undo partial changes
*/
- if (!skb_can_coalesce(tgt, to, skb_frag_page(fragfrom),
- skb_frag_off(fragfrom))) {
+ if (!skb_can_coalesce_netmem(tgt, to, skb_frag_netmem(fragfrom),
+ skb_frag_off(fragfrom))) {
merge = -1;
} else {
merge = to - 1;
@@ -4391,8 +4383,8 @@ int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen)
to++;
} else {
- __skb_frag_ref(fragfrom);
skb_frag_page_copy(fragto, fragfrom);
+ skb_frag_ref(tgt, to);
skb_frag_off_copy(fragto, fragfrom);
skb_frag_size_set(fragto, todo);
@@ -4866,6 +4858,20 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
}
}
+ if (list_skb) {
+ bool recycle = skb_shinfo(head_skb)->nr_frags ?
+ head_skb->pp_recycle : list_skb->pp_recycle;
+ struct sk_buff *check_skb;
+
+ for (check_skb = list_skb; check_skb;
+ check_skb = check_skb->next) {
+ if (check_skb->pp_recycle != recycle) {
+ features &= ~NETIF_F_SG;
+ break;
+ }
+ }
+ }
+
__skb_push(head_skb, doffset);
proto = skb_network_protocol(head_skb, NULL);
if (unlikely(!proto))
@@ -5087,8 +5093,10 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
goto err;
}
+ if (!skb_shinfo(nskb)->nr_frags)
+ nskb->pp_recycle = frag_skb->pp_recycle;
*nskb_frag = (i < 0) ? skb_head_frag_to_page_desc(frag_skb) : *frag;
- __skb_frag_ref(nskb_frag);
+ skb_frag_ref(nskb, skb_shinfo(nskb)->nr_frags);
size = skb_frag_size(nskb_frag);
if (pos < offset) {
@@ -6303,7 +6311,7 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from,
*/
if (skb_pp_frag_ref(from)) {
for (i = 0; i < from_shinfo->nr_frags; i++)
- __skb_frag_ref(&from_shinfo->frags[i]);
+ get_netmem(skb_frag_netmem(&from_shinfo->frags[i]));
}
to->truesize += delta;
diff --git a/net/tls/tls_device_fallback.c b/net/tls/tls_device_fallback.c
index 3b7d0ab2bcf17..35cc510db4321 100644
--- a/net/tls/tls_device_fallback.c
+++ b/net/tls/tls_device_fallback.c
@@ -256,7 +256,7 @@ static int fill_sg_in(struct scatterlist *sg_in,
for (i = 0; remaining > 0; i++) {
skb_frag_t *frag = &record->frags[i];
- __skb_frag_ref(frag);
+ get_netmem(skb_frag_netmem(frag));
sg_set_page(sg_in + i, skb_frag_page(frag),
skb_frag_size(frag), skb_frag_off(frag));
diff --git a/net/xfrm/xfrm_iptfs.c b/net/xfrm/xfrm_iptfs.c
index 6920940a35b49..103cf7c5dbfb9 100644
--- a/net/xfrm/xfrm_iptfs.c
+++ b/net/xfrm/xfrm_iptfs.c
@@ -486,8 +486,7 @@ static int iptfs_skb_add_frags(struct sk_buff *skb,
tofrag->len -= offset;
offset = 0;
}
- __skb_frag_ref(tofrag);
- shinfo->nr_frags++;
+ skb_frag_ref(skb, shinfo->nr_frags++);
shinfo->flags |= SKBFL_SHARED_FRAG;
/* see if we are done */
--
2.56.0.385.gd3acb90ef8-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH net-next v2 5/6] net: skbuff: use skb_frag_ref() in skb_try_coalesce()
2026-10-12 8:21 [PATCH net-next v2 0/6] net: unify symmetric netmem and page_pool refcounting Mina Almasry
` (3 preceding siblings ...)
2026-10-12 8:21 ` [PATCH net-next v2 4/6] net: skbuff: replace __skb_frag_ref() with symmetric skb_netmem_ref() Mina Almasry
@ 2026-10-12 8:21 ` Mina Almasry
2026-10-13 8:22 ` sashiko-bot
2026-10-12 8:21 ` [PATCH net-next v2 6/6] net: kunit: test netmem, page_pool, and skb frag refcounting Mina Almasry
5 siblings, 1 reply; 18+ messages in thread
From: Mina Almasry @ 2026-10-12 8:21 UTC (permalink / raw)
To: netdev, linux-kernel, linux-rdma, bpf
Cc: Mina Almasry, Ayush Sawal, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Tariq Toukan,
Simon Horman, Steffen Klassert, Herbert Xu, Neal Cardwell,
Kuniyuki Iwashima, John Fastabend, Sabrina Dubroca, Eric Biggers,
Kees Cook, Aleksandr Loktionov,
Uwe Kleine-König (The Capable Hub), Jijie Shao,
Michael Grzeschik, Jakub Raczynski
skb_pp_frag_ref() was introduced solely to work around skb_frag_ref()
ignoring skb->pp_recycle and incrementing the non-pp backing refcount.
Now that skb_frag_ref() handles both pp_recycle=1 and pp_recycle=0
symmetrically, call skb_frag_ref() directly in skb_try_coalesce() and
remove skb_pp_frag_ref().
Assisted-by: LLM
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
net/core/skbuff.c | 37 ++-----------------------------------
1 file changed, 2 insertions(+), 35 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index fe73f6e8d2b3e..20fca895d7090 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1078,37 +1078,6 @@ static bool skb_pp_recycle(struct sk_buff *skb, void *data)
return napi_pp_put_page(page_to_netmem(virt_to_page(data)));
}
-/**
- * skb_pp_frag_ref() - Increase fragment references of a page pool aware skb
- * @skb: page pool aware skb
- *
- * Increase the fragment reference count (pp_ref_count) of a skb. This is
- * intended to gain fragment references only for page pool aware skbs,
- * i.e. when skb->pp_recycle is true, and not for fragments in a
- * non-pp-recycling skb. It has a fallback to increase references on normal
- * pages, as page pool aware skbs may also have normal page fragments.
- */
-static int skb_pp_frag_ref(struct sk_buff *skb)
-{
- struct skb_shared_info *shinfo;
- netmem_ref head_netmem;
- int i;
-
- if (!skb->pp_recycle)
- return -EINVAL;
-
- shinfo = skb_shinfo(skb);
-
- for (i = 0; i < shinfo->nr_frags; i++) {
- head_netmem = netmem_compound_head(shinfo->frags[i].netmem);
- if (likely(netmem_is_pp(head_netmem)))
- page_pool_ref_netmem(head_netmem);
- else
- page_ref_inc(netmem_to_page(head_netmem));
- }
- return 0;
-}
-
static void skb_kfree_head(void *head)
{
kfree(head);
@@ -6309,10 +6278,8 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from,
/* if the skb is not cloned this does nothing
* since we set nr_frags to 0.
*/
- if (skb_pp_frag_ref(from)) {
- for (i = 0; i < from_shinfo->nr_frags; i++)
- get_netmem(skb_frag_netmem(&from_shinfo->frags[i]));
- }
+ for (i = 0; i < from_shinfo->nr_frags; i++)
+ skb_frag_ref(from, i);
to->truesize += delta;
to->len += len;
--
2.56.0.385.gd3acb90ef8-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH net-next v2 6/6] net: kunit: test netmem, page_pool, and skb frag refcounting
2026-10-12 8:21 [PATCH net-next v2 0/6] net: unify symmetric netmem and page_pool refcounting Mina Almasry
` (4 preceding siblings ...)
2026-10-12 8:21 ` [PATCH net-next v2 5/6] net: skbuff: use skb_frag_ref() in skb_try_coalesce() Mina Almasry
@ 2026-10-12 8:21 ` Mina Almasry
2026-10-13 8:22 ` sashiko-bot
2026-10-13 8:34 ` netdev-bot+sashiko
5 siblings, 2 replies; 18+ messages in thread
From: Mina Almasry @ 2026-10-12 8:21 UTC (permalink / raw)
To: netdev, linux-kernel, linux-rdma, bpf
Cc: Mina Almasry, Ayush Sawal, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Tariq Toukan,
Simon Horman, Steffen Klassert, Herbert Xu, Neal Cardwell,
Kuniyuki Iwashima, John Fastabend, Sabrina Dubroca, Eric Biggers,
Kees Cook, Aleksandr Loktionov,
Uwe Kleine-König (The Capable Hub), Jijie Shao,
Michael Grzeschik, Jakub Raczynski
Verify that non-pp (page._refcount / binding->ref) and page_pool
(pp_ref_count) references stay balanced without cross-counter leaks or
underflows across regular pages, page_pool pages, non-pp net_iovs, and
page_pool net_iovs with both pp_recycle=0 and pp_recycle=1.
Cover get/put_net_iov(), get/put_netmem(), napi_pp_get/put_page(),
skb_netmem_ref/unref(), skb_frag_ref/unref(), pskb_copy(),
skb_zerocopy(), skb_shift(), skb_split(), skb_try_coalesce(),
skb_release_data() (clones, unclone ordering, pskb_extract(), and
skb_morph()), skb_segment(), and devmem tcp_recvmsg_dmabuf() /
SO_DEVMEM_DONTNEED.
Export skb_shift() via EXPORT_SYMBOL_IF_KUNIT() so the test links when
built as a module (CONFIG_NET_TEST=m).
Assisted-by: LLM
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
v2:
- Export skb_shift() via EXPORT_SYMBOL_IF_KUNIT() to fix the
CONFIG_NET_TEST=m modpost error reported by NIPA.
- Reset mac_header before skb_segment() and check skb_clone() results
before reuse (Sashiko).
- Cover compound tail pages, skb_try_coalesce(), TCP devmem recvmsg and
SO_DEVMEM_DONTNEED, skb_zerocopy() rollbacks, skb_split()/skb_shift()
edge cases, and mixed pp_recycle frag_list GSO.
- Link to v1: https://lore.kernel.org/netdev/20261010083935.3274178-7-almasrymina@google.com/
---
net/core/net_test.c | 1204 +++++++++++++++++++++++++++++++++++++++++++
net/core/skbuff.c | 2 +
2 files changed, 1206 insertions(+)
diff --git a/net/core/net_test.c b/net/core/net_test.c
index 9c3a590865d26..6f21e6baf7fb3 100644
--- a/net/core/net_test.c
+++ b/net/core/net_test.c
@@ -370,10 +370,1213 @@ static void ip_tunnel_flags_test_run(struct kunit *test)
KUNIT_ASSERT_TRUE(test, __ipt_flag_op(bitmap_equal, exp, out));
}
+/* Netmem & SKB fragment refcounting */
+
+#include <linux/skbuff_ref.h>
+#include <net/netmem.h>
+#include <net/netdev_rx_queue.h>
+#include <net/page_pool/helpers.h>
+#include <net/page_pool/memory_provider.h>
+#include <net/tcp.h>
+#include "devmem.h"
+#include "netmem_priv.h"
+
+static void netmem_ref_test_page(struct kunit *test)
+{
+ struct sk_buff *skb;
+ netmem_ref netmem;
+ struct page *page;
+ int base_ref;
+
+ page = alloc_page(GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, page);
+ netmem = page_to_netmem(page);
+ base_ref = page_ref_count(page);
+
+ /* Layer 1: get_netmem / put_netmem use page non-pp refcount */
+ get_netmem(netmem);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref + 1);
+ put_netmem(netmem);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+
+#ifdef CONFIG_PAGE_POOL
+ /* Layer 2: napi_pp_get/put_page return false on non-PP page */
+ KUNIT_EXPECT_FALSE(test, napi_pp_get_page(netmem));
+ KUNIT_EXPECT_FALSE(test, napi_pp_put_page(netmem));
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+#endif
+
+ /* Layer 3: skb_netmem_ref / unref fall back to non-pp on non-PP page */
+ skb_netmem_ref(netmem, false);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref + 1);
+ skb_netmem_unref(netmem, false);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+
+ skb_netmem_ref(netmem, true);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref + 1);
+ skb_netmem_unref(netmem, true);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+
+ /* Layer 4: skb_frag_ref / skb_frag_unref on non-PP page */
+ skb = alloc_skb(64, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, skb);
+ skb_fill_netmem_desc(skb, 0, netmem, 0, 64);
+
+ skb->pp_recycle = 1;
+ skb_frag_ref(skb, 0);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref + 1);
+ skb_frag_unref(skb, 0);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+
+ skb_shinfo(skb)->nr_frags = 0;
+ consume_skb(skb);
+ __free_page(page);
+}
+
+#ifdef CONFIG_PAGE_POOL
+static void netmem_ref_test_pp_page(struct kunit *test)
+{
+ struct page_pool_params pp_params = {
+ .order = 0,
+ .pool_size = 4,
+ .nid = NUMA_NO_NODE,
+ };
+ struct page_pool *pool;
+ struct sk_buff *skb, *copy, *clone, *split;
+ netmem_ref netmem;
+ struct page *page;
+ long base_pp_ref;
+ int base_ref;
+
+ pool = page_pool_create(&pp_params);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pool);
+
+ page = page_pool_dev_alloc_pages(pool);
+ KUNIT_ASSERT_NOT_NULL(test, page);
+ netmem = page_to_netmem(page);
+ base_ref = page_ref_count(page);
+ base_pp_ref = atomic_long_read(&page->pp_ref_count);
+
+ /* Layer 1: get_netmem / put_netmem always use non-pp refcount */
+ get_netmem(netmem);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref + 1);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref);
+ put_netmem(netmem);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+
+ /* Layer 2: napi_pp_get/put_page use pp_ref_count */
+ KUNIT_EXPECT_TRUE(test, napi_pp_get_page(netmem));
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 1);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+ KUNIT_EXPECT_TRUE(test, napi_pp_put_page(netmem));
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref);
+
+ /* Layer 3: skb_netmem_ref / unref obey recycle flag */
+ skb_netmem_ref(netmem, false);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref + 1);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref);
+ skb_netmem_unref(netmem, false);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+
+ skb_netmem_ref(netmem, true);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 1);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+ skb_netmem_unref(netmem, true);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref);
+
+ /* Layer 4: skb_frag_ref / skb_frag_unref match on pp_recycle=1 & 0 */
+ skb = alloc_skb(64, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, skb);
+ skb_fill_netmem_desc(skb, 0, netmem, 0, 64);
+ skb->len = 64;
+ skb->data_len = 64;
+
+ skb->pp_recycle = 1;
+ skb_frag_ref(skb, 0);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 1);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+ skb_frag_unref(skb, 0);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref);
+
+ /* pskb_copy on pp_recycle=1 skb propagates pp_recycle & pp_ref_count */
+ copy = pskb_copy(skb, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, copy);
+ KUNIT_EXPECT_TRUE(test, copy->pp_recycle);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 1);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+ consume_skb(copy);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref);
+
+ /* skb_zerocopy on pp_recycle=1 skb propagates pp_ref_count */
+ copy = alloc_skb(0, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, copy);
+ skb_put(copy, skb_tailroom(copy));
+ skb->head_frag = 1;
+ KUNIT_ASSERT_EQ(test, skb_zerocopy(copy, skb, skb->len, 0), 0);
+ skb->head_frag = 0;
+ KUNIT_EXPECT_TRUE(test, copy->pp_recycle);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 1);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+ consume_skb(copy);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref);
+
+ /* skb_clone + pskb_expand_head preserves pp_recycle and pp_ref_count */
+ clone = skb_clone(skb, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, clone);
+ KUNIT_ASSERT_EQ(test, pskb_expand_head(clone, 32, 0, GFP_KERNEL), 0);
+ KUNIT_EXPECT_TRUE(test, clone->pp_recycle);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 1);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+ consume_skb(clone);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref);
+
+ /* skb_split inside linear header & inside nonlinear page_pool frags */
+ skb_shinfo(skb)->nr_frags = 0;
+ skb->len = 0;
+ skb->data_len = 0;
+ skb_reset_tail_pointer(skb);
+ skb_put(skb, 32);
+ skb_fill_netmem_desc(skb, 0, netmem, 0, 64);
+ skb->len += 64;
+ skb->data_len = 64;
+ split = alloc_skb(32, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, split);
+ skb_split(skb, split, 16);
+ KUNIT_EXPECT_TRUE(test, split->pp_recycle);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+ skb_shinfo(split)->nr_frags = 0;
+ consume_skb(split);
+ skb->len = 0;
+ skb->data_len = 0;
+ skb_reset_tail_pointer(skb);
+ skb_fill_netmem_desc(skb, 0, netmem, 0, 64);
+ skb->len = 64;
+ skb->data_len = 64;
+
+ split = alloc_skb(0, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, split);
+ skb_split(skb, split, 32);
+ KUNIT_EXPECT_TRUE(test, split->pp_recycle);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 1);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+ consume_skb(split);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref);
+
+ /* skb_segment on pp_recycle=1 skb propagates pp_recycle to segments */
+ skb->protocol = htons(ETH_P_IP);
+ skb_reset_mac_header(skb);
+ skb_reset_network_header(skb);
+ skb_shinfo(skb)->gso_size = 16;
+ split = skb_segment(skb, NETIF_F_SG | NETIF_F_HW_CSUM);
+ KUNIT_ASSERT_FALSE(test, IS_ERR_OR_NULL(split));
+ KUNIT_EXPECT_TRUE(test, split->pp_recycle);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 2);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+ kfree_skb_list(split);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref);
+
+ /* skb_shift balances pp_ref_count across split, partial coalesce,
+ * multi-frag coalesce + partial next-frag shift, and full merge.
+ */
+ skb_fill_netmem_desc(skb, 0, netmem, 0, 32);
+ skb_fill_netmem_desc(skb, 1, netmem, 64, 32);
+ page_pool_ref_page(page);
+ skb->len = 64;
+ skb->data_len = 64;
+ split = alloc_skb(0, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, split);
+ split->pp_recycle = 1;
+ KUNIT_EXPECT_EQ(test, skb_shift(split, skb, 16), 16);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 2);
+ KUNIT_EXPECT_EQ(test, skb_shift(split, skb, 8), 8);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 2);
+ KUNIT_EXPECT_EQ(test, skb_shift(split, skb, 24), 24);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 2);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+ KUNIT_EXPECT_EQ(test, skb_shift(split, skb, 16), 16);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 1);
+ skb_shinfo(split)->nr_frags = 0;
+ consume_skb(split);
+ page_pool_put_full_page(pool, page, false);
+ skb_fill_netmem_desc(skb, 0, netmem, 0, 64);
+ skb->len = 64;
+ skb->data_len = 64;
+
+ skb->pp_recycle = 0;
+ skb_frag_ref(skb, 0);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref + 1);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref);
+ skb_frag_unref(skb, 0);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+
+ skb_shinfo(skb)->nr_frags = 0;
+ consume_skb(skb);
+ page_pool_put_full_page(pool, page, false);
+ page_pool_destroy(pool);
+
+ /* Order-1 compound page_pool page: tail netmem resolves to head &
+ * build_skb(head_frag=1, pp_recycle=1) skb_zerocopy uses pp_ref_count.
+ */
+ pp_params.order = 1;
+ pool = page_pool_create(&pp_params);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pool);
+ page = page_pool_dev_alloc_pages(pool);
+ KUNIT_ASSERT_NOT_NULL(test, page);
+ base_ref = page_ref_count(page);
+ base_pp_ref = atomic_long_read(&page->pp_ref_count);
+
+ netmem = page_to_netmem(page + 1);
+ KUNIT_EXPECT_TRUE(test, napi_pp_get_page(netmem));
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 1);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+ KUNIT_EXPECT_TRUE(test, napi_pp_put_page(netmem));
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref);
+
+ skb = build_skb(page_address(page), PAGE_SIZE);
+ KUNIT_ASSERT_NOT_NULL(test, skb);
+ skb->pp_recycle = 1;
+ skb_put(skb, 64);
+ page_pool_ref_page(page);
+
+ copy = alloc_skb(0, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, copy);
+ skb_put(copy, skb_tailroom(copy));
+ KUNIT_ASSERT_EQ(test, skb_zerocopy(copy, skb, skb->len, 0), 0);
+ KUNIT_EXPECT_TRUE(test, copy->pp_recycle);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 2);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+ consume_skb(copy);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 1);
+ consume_skb(skb);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref);
+
+ page_pool_put_full_page(pool, page, false);
+ page_pool_destroy(pool);
+}
+#endif
+
+#if defined(CONFIG_PAGE_POOL) && defined(CONFIG_NET_DEVMEM)
+static void dummy_percpu_ref_release(struct percpu_ref *ref)
+{
+}
+
+static void netmem_ref_test_net_iov(struct kunit *test)
+{
+ struct net_devmem_dmabuf_binding binding = {};
+ struct page_pool_params pp_params = {
+ .order = 0,
+ .pool_size = 4,
+ .nid = NUMA_NO_NODE,
+ };
+ struct sk_buff *skb, *clone, *copy;
+ struct page_pool *pool;
+ struct net_iov niov = {};
+ netmem_ref netmem;
+ long base_ref;
+ int err;
+
+ err = percpu_ref_init(&binding.ref, dummy_percpu_ref_release,
+ PERCPU_REF_INIT_ATOMIC, GFP_KERNEL);
+ KUNIT_ASSERT_EQ(test, err, 0);
+
+ net_iov_init(&niov, &binding.area, NET_IOV_DMABUF);
+ netmem = net_iov_to_netmem(&niov);
+ base_ref = atomic_long_read(&binding.ref.data->count);
+
+ /* Non-PP net_iov: get/put_net_iov & get/put_netmem */
+ get_net_iov(&niov);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&binding.ref.data->count),
+ base_ref + 1);
+ put_net_iov(&niov);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&binding.ref.data->count),
+ base_ref);
+
+ KUNIT_EXPECT_FALSE(test, napi_pp_get_page(netmem));
+ KUNIT_EXPECT_FALSE(test, napi_pp_put_page(netmem));
+
+ skb_netmem_ref(netmem, true);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&binding.ref.data->count),
+ base_ref + 1);
+ skb_netmem_unref(netmem, true);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&binding.ref.data->count),
+ base_ref);
+
+ /* Attach net_iov to a page_pool to test PP net_iov behavior */
+ pool = page_pool_create(&pp_params);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pool);
+ netmem_set_pp(netmem, pool);
+ netmem_or_pp_magic(netmem, PP_SIGNATURE);
+ atomic_long_set(&niov.desc.pp_ref_count, 2);
+
+ /* Layer 0 & 1: get_net_iov / get_netmem grab non-pp ref even on PP */
+ get_net_iov(&niov);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&binding.ref.data->count),
+ base_ref + 1);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 2L);
+ put_net_iov(&niov);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&binding.ref.data->count),
+ base_ref);
+
+ /* Layer 2: napi_pp_get/put_page use pp_ref_count */
+ KUNIT_EXPECT_TRUE(test, napi_pp_get_page(netmem));
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 3L);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&binding.ref.data->count),
+ base_ref);
+ KUNIT_EXPECT_TRUE(test, napi_pp_put_page(netmem));
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 2L);
+
+ /* Layer 4: skb_frag_ref / skb_frag_unref on PP net_iov */
+ skb = alloc_skb(64, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, skb);
+ skb_fill_netmem_desc(skb, 0, netmem, 0, 64);
+ skb->len = 64;
+ skb->data_len = 64;
+
+ skb->pp_recycle = 1;
+ skb_frag_ref(skb, 0);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 3L);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&binding.ref.data->count),
+ base_ref);
+ skb_frag_unref(skb, 0);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 2L);
+
+ /* pskb_copy on pp_recycle=1 net_iov skb keeps pp_ref_count */
+ copy = pskb_copy(skb, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, copy);
+ KUNIT_EXPECT_TRUE(test, copy->pp_recycle);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 3L);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&binding.ref.data->count),
+ base_ref);
+ consume_skb(copy);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 2L);
+
+ /* skb_zerocopy on pp_recycle=1 net_iov skb keeps pp_ref_count */
+ copy = alloc_skb(0, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, copy);
+ skb_put(copy, skb_tailroom(copy));
+ skb->head_frag = 1;
+ skb->unreadable = 1;
+ KUNIT_ASSERT_EQ(test, skb_zerocopy(copy, skb, skb->len, 0), 0);
+ skb->head_frag = 0;
+ KUNIT_EXPECT_TRUE(test, copy->pp_recycle);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 3L);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&binding.ref.data->count),
+ base_ref);
+ consume_skb(copy);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 2L);
+
+ /* skb_shift balances pp_ref_count on split and merge */
+ copy = alloc_skb(0, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, copy);
+ copy->pp_recycle = 1;
+ KUNIT_EXPECT_EQ(test, skb_shift(copy, skb, 16), 16);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 3L);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&binding.ref.data->count),
+ base_ref);
+ KUNIT_EXPECT_EQ(test, skb_shift(copy, skb, 48), 48);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 2L);
+ skb_shinfo(copy)->nr_frags = 0;
+ consume_skb(copy);
+ skb_fill_netmem_desc(skb, 0, netmem, 0, 64);
+ skb->len = 64;
+ skb->data_len = 64;
+
+ /* Uncloning a cloned pp_recycle=1 net_iov skb keeps pp_ref_count */
+ clone = skb_clone(skb, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, clone);
+ KUNIT_ASSERT_EQ(test, pskb_expand_head(skb, 32, 0, GFP_KERNEL), 0);
+ KUNIT_EXPECT_TRUE(test, skb->pp_recycle);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 3L);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&binding.ref.data->count),
+ base_ref);
+ consume_skb(clone);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 2L);
+
+ skb->pp_recycle = 0;
+ skb_frag_ref(skb, 0);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&binding.ref.data->count),
+ base_ref + 1);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 2L);
+ skb_frag_unref(skb, 0);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&binding.ref.data->count),
+ base_ref);
+
+ skb_shinfo(skb)->nr_frags = 0;
+ consume_skb(skb);
+ netmem_clear_pp_magic(netmem);
+ netmem_set_pp(netmem, NULL);
+ page_pool_destroy(pool);
+ percpu_ref_exit(&binding.ref);
+}
+
+static void netmem_ref_test_release_data(struct kunit *test)
+{
+ struct page_pool_params pp_params = {
+ .order = 0,
+ .pool_size = 4,
+ .nid = NUMA_NO_NODE,
+ };
+ struct sk_buff *skb, *clone1, *clone2, *ext;
+ struct net_devmem_dmabuf_binding binding = {};
+ struct net_iov niov = {};
+ struct page_pool *pool;
+ struct page *page;
+ netmem_ref netmem;
+ long base_pp_ref;
+ int base_ref;
+ int i;
+
+ pool = page_pool_create(&pp_params);
+ KUNIT_ASSERT_FALSE(test, IS_ERR(pool));
+ page = page_pool_dev_alloc_pages(pool);
+ KUNIT_ASSERT_NOT_NULL(test, page);
+ netmem = page_to_netmem(page);
+ page_pool_ref_netmem(netmem);
+
+ /* 3-way clone chain: only last clone drops pp_ref_count */
+ skb = alloc_skb(64, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, skb);
+ skb_put(skb, 32);
+ skb->pp_recycle = 1;
+ skb_fill_netmem_desc(skb, 0, netmem, 0, 64);
+ skb->len += 64;
+ skb->data_len = 64;
+ base_ref = page_ref_count(page);
+ base_pp_ref = atomic_long_read(&page->pp_ref_count);
+
+ page_pool_ref_netmem(netmem);
+ clone1 = skb_clone(skb, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, clone1);
+ clone2 = skb_clone(clone1, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, clone2);
+
+ /* Freeing head skb first leaves clone1/clone2 pp_recycle=1 intact */
+ consume_skb(skb);
+ KUNIT_EXPECT_TRUE(test, clone1->pp_recycle);
+ KUNIT_EXPECT_TRUE(test, clone2->pp_recycle);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 1);
+ consume_skb(clone2);
+ KUNIT_EXPECT_TRUE(test, clone1->pp_recycle);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 1);
+ consume_skb(clone1);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref);
+
+ /* All 4 combinations of (unclone skb vs clone) x (free order) */
+ for (i = 0; i < 4; i++) {
+ skb = alloc_skb(64, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, skb);
+ skb_put(skb, 32);
+ skb->pp_recycle = 1;
+ skb_fill_netmem_desc(skb, 0, netmem, 0, 64);
+ skb->len += 64;
+ skb->data_len = 64;
+ page_pool_ref_netmem(netmem);
+
+ clone1 = skb_clone(skb, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, clone1);
+ KUNIT_ASSERT_EQ(test,
+ pskb_expand_head((i & 1) ? clone1 : skb,
+ 32, 0, GFP_KERNEL), 0);
+ KUNIT_EXPECT_TRUE(test, skb->pp_recycle);
+ KUNIT_EXPECT_TRUE(test, clone1->pp_recycle);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 2);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+
+ if (i & 2) {
+ consume_skb(clone1);
+ KUNIT_EXPECT_EQ(test,
+ atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 1);
+ consume_skb(skb);
+ } else {
+ consume_skb(skb);
+ KUNIT_EXPECT_EQ(test,
+ atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 1);
+ consume_skb(clone1);
+ }
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+ }
+
+ /* pskb_extract carving inside header & inside nonlinear frags */
+ skb = alloc_skb(64, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, skb);
+ skb_put(skb, 32);
+ skb->pp_recycle = 1;
+ skb_fill_netmem_desc(skb, 0, netmem, 0, 64);
+ skb->len += 64;
+ skb->data_len = 64;
+ page_pool_ref_netmem(netmem);
+
+ /* Carve inside header keeping frag -> pp_ref_count +1 until free */
+ ext = pskb_extract(skb, 16, skb->len - 16, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, ext);
+ KUNIT_EXPECT_TRUE(test, ext->pp_recycle);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 2);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+ consume_skb(ext);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 1);
+
+ /* Carve + trim + skb_condense ref/unrefs pp_ref_count cleanly */
+ ext = pskb_extract(skb, 16, 48, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, ext);
+ KUNIT_EXPECT_TRUE(test, ext->pp_recycle);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 1);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+ consume_skb(ext);
+
+ /* Carve inside nonlinear keeping frag -> pp_ref_count +1 until free */
+ ext = pskb_extract(skb, 48, skb->len - 48, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, ext);
+ KUNIT_EXPECT_TRUE(test, ext->pp_recycle);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 2);
+ KUNIT_EXPECT_EQ(test, page_ref_count(page), base_ref);
+ consume_skb(ext);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 1);
+
+ /* skb_morph drops old skb data via skb_release_data */
+ page_pool_ref_netmem(netmem);
+ clone1 = alloc_skb(64, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, clone1);
+ clone1->pp_recycle = 1;
+ skb_fill_netmem_desc(clone1, 0, netmem, 0, 64);
+ clone1->len = 64;
+ clone1->data_len = 64;
+ skb_morph(clone1, skb);
+ KUNIT_EXPECT_TRUE(test, clone1->pp_recycle);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&page->pp_ref_count),
+ base_pp_ref + 1);
+ consume_skb(clone1);
+ kfree_skb(skb);
+ page_pool_put_full_page(pool, page, false);
+ page_pool_put_full_page(pool, page, false);
+
+ /* Also verify pskb_extract on page_pool net_iov */
+ KUNIT_ASSERT_EQ(test,
+ percpu_ref_init(&binding.ref, dummy_percpu_ref_release,
+ PERCPU_REF_INIT_ATOMIC, GFP_KERNEL),
+ 0);
+ net_iov_init(&niov, &binding.area, NET_IOV_DMABUF);
+ netmem = net_iov_to_netmem(&niov);
+ netmem_set_pp(netmem, pool);
+ netmem_or_pp_magic(netmem, PP_SIGNATURE);
+ atomic_long_set(&niov.desc.pp_ref_count, 2);
+
+ skb = alloc_skb(64, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, skb);
+ skb_put(skb, 32);
+ skb->pp_recycle = 1;
+ skb_fill_netmem_desc(skb, 0, netmem, 0, 64);
+ skb->len += 64;
+ skb->data_len = 64;
+
+ ext = pskb_extract(skb, 48, skb->len - 48, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, ext);
+ KUNIT_EXPECT_TRUE(test, ext->pp_recycle);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 3L);
+ consume_skb(ext);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 2L);
+
+ skb_shinfo(skb)->nr_frags = 0;
+ consume_skb(skb);
+ netmem_clear_pp_magic(netmem);
+ netmem_set_pp(netmem, NULL);
+ page_pool_destroy(pool);
+ percpu_ref_exit(&binding.ref);
+}
+
+static void netmem_ref_test_skb_segment(struct kunit *test)
+{
+ struct page_pool_params pp_params = {
+ .order = 0,
+ .pool_size = 4,
+ .nid = NUMA_NO_NODE,
+ };
+ struct ubuf_info uarg = {
+ .flags = SKBFL_ZEROCOPY_ENABLE,
+ };
+ struct sk_buff *head, *list, *segs, *seg;
+ struct net_devmem_dmabuf_binding binding = {};
+ struct net_iov niov = {};
+ struct page *pp_page, *reg_page;
+ netmem_ref pp_nm, niov_nm;
+ struct page_pool *pool;
+ long base_pp_ref, base_iov_ref;
+ int base_pp_page_ref, base_reg_ref;
+
+ pool = page_pool_create(&pp_params);
+ KUNIT_ASSERT_FALSE(test, IS_ERR(pool));
+ pp_page = page_pool_dev_alloc_pages(pool);
+ KUNIT_ASSERT_NOT_NULL(test, pp_page);
+ pp_nm = page_to_netmem(pp_page);
+ page_pool_ref_netmem(pp_nm);
+ reg_page = alloc_page(GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, reg_page);
+
+ KUNIT_ASSERT_EQ(test,
+ percpu_ref_init(&binding.ref, dummy_percpu_ref_release,
+ PERCPU_REF_INIT_ATOMIC, GFP_KERNEL),
+ 0);
+ net_iov_init(&niov, &binding.area, NET_IOV_DMABUF);
+ niov_nm = net_iov_to_netmem(&niov);
+ netmem_set_pp(niov_nm, pool);
+ netmem_or_pp_magic(niov_nm, PP_SIGNATURE);
+ atomic_long_set(&niov.desc.pp_ref_count, 2);
+
+ base_pp_ref = atomic_long_read(&pp_page->pp_ref_count);
+ base_pp_page_ref = page_ref_count(pp_page);
+ base_reg_ref = page_ref_count(reg_page);
+ base_iov_ref = atomic_long_read(&binding.ref.data->count);
+
+ /* 1. Single GSO skb with page_pool net_iov frags */
+ head = alloc_skb(64, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, head);
+ head->pp_recycle = 1;
+ head->protocol = htons(ETH_P_IP);
+ skb_reset_mac_header(head);
+ skb_reset_network_header(head);
+ skb_fill_netmem_desc(head, 0, niov_nm, 0, 64);
+ head->len = 64;
+ head->data_len = 64;
+ skb_shinfo(head)->gso_size = 16;
+
+ segs = skb_segment(head, NETIF_F_SG | NETIF_F_HW_CSUM);
+ KUNIT_ASSERT_FALSE(test, IS_ERR_OR_NULL(segs));
+ for (seg = segs; seg; seg = seg->next)
+ KUNIT_EXPECT_TRUE(test, seg->pp_recycle);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 6L);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&binding.ref.data->count),
+ base_iov_ref);
+ kfree_skb_list(segs);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 2L);
+ skb_shinfo(head)->nr_frags = 0;
+ consume_skb(head);
+
+ /* 2. GRO frag_list fast path (skb_clone of pp_recycle=1 list_skb) */
+ head = alloc_skb(64, GFP_KERNEL);
+ list = alloc_skb(64, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, head);
+ KUNIT_ASSERT_NOT_NULL(test, list);
+ head->pp_recycle = 1;
+ list->pp_recycle = 1;
+ head->protocol = htons(ETH_P_IP);
+ skb_reset_mac_header(head);
+ skb_reset_network_header(head);
+ skb_put(head, 16);
+ skb_fill_netmem_desc(head, 0, pp_nm, 0, 16);
+ head->len += 16;
+ head->data_len = 16;
+ page_pool_ref_netmem(pp_nm);
+
+ skb_put(list, 16);
+ skb_fill_netmem_desc(list, 0, pp_nm, 16, 16);
+ list->len += 16;
+ list->data_len = 16;
+ page_pool_ref_netmem(pp_nm);
+
+ skb_shinfo(head)->frag_list = list;
+ head->len += list->len;
+ head->data_len += list->len;
+ skb_shinfo(head)->gso_size = 32;
+
+ segs = skb_segment(head, NETIF_F_SG | NETIF_F_HW_CSUM);
+ KUNIT_ASSERT_FALSE(test, IS_ERR_OR_NULL(segs));
+ for (seg = segs; seg; seg = seg->next)
+ KUNIT_EXPECT_TRUE(test, seg->pp_recycle);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&pp_page->pp_ref_count),
+ base_pp_ref + 4);
+ KUNIT_EXPECT_EQ(test, page_ref_count(pp_page), base_pp_page_ref);
+ kfree_skb_list(segs);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&pp_page->pp_ref_count),
+ base_pp_ref + 2);
+ consume_skb(head);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&pp_page->pp_ref_count),
+ base_pp_ref);
+
+ /* 3. Mixed GRO frag_list (head pp_recycle=1 + list pp_recycle=0):
+ * skb_segment disables SG page sharing so no nskb mixes incompatible
+ * fragment refcount types; all counters stay balanced.
+ */
+ head = alloc_skb(64, GFP_KERNEL);
+ list = alloc_skb(0, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, head);
+ KUNIT_ASSERT_NOT_NULL(test, list);
+ head->pp_recycle = 1;
+ list->pp_recycle = 0;
+ head->protocol = htons(ETH_P_IP);
+ skb_reset_mac_header(head);
+ skb_reset_network_header(head);
+ skb_fill_netmem_desc(head, 0, pp_nm, 0, 24);
+ head->len = 24;
+ head->data_len = 24;
+ page_pool_ref_netmem(pp_nm);
+
+ skb_fill_page_desc(list, 0, reg_page, 0, 24);
+ get_page(reg_page);
+ list->len = 24;
+ list->data_len = 24;
+
+ skb_shinfo(head)->frag_list = list;
+ head->len += list->len;
+ head->data_len += list->len;
+ skb_shinfo(head)->gso_size = 16;
+
+ segs = skb_segment(head, NETIF_F_SG | NETIF_F_HW_CSUM);
+ KUNIT_ASSERT_FALSE(test, IS_ERR_OR_NULL(segs));
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&pp_page->pp_ref_count),
+ base_pp_ref + 1);
+ KUNIT_EXPECT_EQ(test, page_ref_count(pp_page), base_pp_page_ref);
+ KUNIT_EXPECT_EQ(test, page_ref_count(reg_page), base_reg_ref + 1);
+ kfree_skb_list(segs);
+ consume_skb(head);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&pp_page->pp_ref_count),
+ base_pp_ref);
+ KUNIT_EXPECT_EQ(test, page_ref_count(pp_page), base_pp_page_ref);
+ KUNIT_EXPECT_EQ(test, page_ref_count(reg_page), base_reg_ref);
+
+ /* 4. Reverse mixed frag_list (head pp_recycle=0 + list pp_recycle=1):
+ * Crossing from pp_recycle=0 head into pp_recycle=1 list_skb must not
+ * elevate page_ref_count(pp_page) instead of pp_ref_count.
+ */
+ head = alloc_skb(64, GFP_KERNEL);
+ list = alloc_skb(0, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, head);
+ KUNIT_ASSERT_NOT_NULL(test, list);
+ head->pp_recycle = 0;
+ list->pp_recycle = 1;
+ head->protocol = htons(ETH_P_IP);
+ skb_reset_mac_header(head);
+ skb_reset_network_header(head);
+ skb_fill_page_desc(head, 0, reg_page, 0, 24);
+ get_page(reg_page);
+ head->len = 24;
+ head->data_len = 24;
+
+ skb_fill_netmem_desc(list, 0, pp_nm, 0, 24);
+ page_pool_ref_netmem(pp_nm);
+ list->len = 24;
+ list->data_len = 24;
+
+ skb_shinfo(head)->frag_list = list;
+ head->len += list->len;
+ head->data_len += list->len;
+ skb_shinfo(head)->gso_size = 16;
+
+ segs = skb_segment(head, NETIF_F_SG | NETIF_F_HW_CSUM);
+ KUNIT_ASSERT_FALSE(test, IS_ERR_OR_NULL(segs));
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&pp_page->pp_ref_count),
+ base_pp_ref + 1);
+ KUNIT_EXPECT_EQ(test, page_ref_count(pp_page), base_pp_page_ref);
+ KUNIT_EXPECT_EQ(test, page_ref_count(reg_page), base_reg_ref + 1);
+ kfree_skb_list(segs);
+ consume_skb(head);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&pp_page->pp_ref_count),
+ base_pp_ref);
+ KUNIT_EXPECT_EQ(test, page_ref_count(pp_page), base_pp_page_ref);
+ KUNIT_EXPECT_EQ(test, page_ref_count(reg_page), base_reg_ref);
+
+ /* 5. Linear head_skb (nr_frags=0, pp_recycle=0) + homogeneous
+ * pp_recycle=1 frag_list keeps zero-copy SG enabled.
+ */
+ head = alloc_skb(64, GFP_KERNEL);
+ list = alloc_skb(0, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, head);
+ KUNIT_ASSERT_NOT_NULL(test, list);
+ head->pp_recycle = 0;
+ list->pp_recycle = 1;
+ head->protocol = htons(ETH_P_IP);
+ skb_reset_mac_header(head);
+ skb_reset_network_header(head);
+ skb_fill_netmem_desc(list, 0, pp_nm, 0, 32);
+ page_pool_ref_netmem(pp_nm);
+ list->len = 32;
+ list->data_len = 32;
+ skb_shinfo(head)->frag_list = list;
+ head->len = 32;
+ head->data_len = 32;
+ skb_shinfo(head)->gso_size = 16;
+
+ segs = skb_segment(head, NETIF_F_SG | NETIF_F_HW_CSUM);
+ KUNIT_ASSERT_FALSE(test, IS_ERR_OR_NULL(segs));
+ KUNIT_EXPECT_TRUE(test, segs->pp_recycle);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&pp_page->pp_ref_count),
+ base_pp_ref + 3);
+ KUNIT_EXPECT_EQ(test, page_ref_count(pp_page), base_pp_page_ref);
+ kfree_skb_list(segs);
+ consume_skb(head);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&pp_page->pp_ref_count),
+ base_pp_ref);
+
+ /* 6. skb_zerocopy -EFAULT rollback when from->head_frag (pp_page) is
+ * followed by unreadable net_iov frag drops pp_ref_count via
+ * skb_frag_unref(to, 0).
+ */
+ head = build_skb(page_address(pp_page), PAGE_SIZE);
+ KUNIT_ASSERT_NOT_NULL(test, head);
+ head->pp_recycle = 1;
+ skb_put(head, 32);
+ page_pool_ref_netmem(pp_nm);
+ skb_fill_netmem_desc(head, 0, niov_nm, 0, 32);
+ napi_pp_get_page(niov_nm);
+ head->len += 32;
+ head->data_len += 32;
+
+ seg = alloc_skb(0, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, seg);
+ skb_put(seg, skb_tailroom(seg));
+ KUNIT_EXPECT_EQ(test, skb_zerocopy(seg, head, head->len, 0), -EFAULT);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&pp_page->pp_ref_count),
+ base_pp_ref + 1);
+ skb_shinfo(seg)->nr_frags = 0;
+ consume_skb(seg);
+ consume_skb(head);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&pp_page->pp_ref_count),
+ base_pp_ref);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 2L);
+
+ /* 7. skb_zerocopy -ENOMEM rollback when skb_orphan_frags() fails on
+ * shared zerocopy skb drops pp_ref_count via skb_frag_unref(to, 0).
+ */
+ head = build_skb(page_address(pp_page), PAGE_SIZE);
+ KUNIT_ASSERT_NOT_NULL(test, head);
+ head->pp_recycle = 1;
+ skb_put(head, 32);
+ page_pool_ref_netmem(pp_nm);
+ skb_zcopy_init(head, &uarg);
+ skb_get(head);
+
+ seg = alloc_skb(0, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, seg);
+ skb_put(seg, skb_tailroom(seg));
+ KUNIT_EXPECT_EQ(test, skb_zerocopy(seg, head, head->len, 0), -ENOMEM);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&pp_page->pp_ref_count),
+ base_pp_ref + 1);
+ KUNIT_EXPECT_EQ(test, page_ref_count(pp_page), base_pp_page_ref);
+ skb_shinfo(seg)->nr_frags = 0;
+ consume_skb(seg);
+ consume_skb(head);
+ skb_shinfo(head)->flags &= ~SKBFL_ZEROCOPY_ENABLE;
+ consume_skb(head);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&pp_page->pp_ref_count),
+ base_pp_ref);
+
+ page_pool_put_full_page(pool, pp_page, false);
+ page_pool_put_full_page(pool, pp_page, false);
+ __free_page(reg_page);
+ netmem_clear_pp_magic(niov_nm);
+ netmem_set_pp(niov_nm, NULL);
+ page_pool_destroy(pool);
+ percpu_ref_exit(&binding.ref);
+}
+
+static void netmem_ref_test_try_coalesce(struct kunit *test)
+{
+ struct page_pool_params pp_params = {
+ .order = 0,
+ .pool_size = 4,
+ .nid = NUMA_NO_NODE,
+ };
+ struct sk_buff *to, *from, *clone;
+ struct net_devmem_dmabuf_binding binding = {};
+ struct net_iov niov = {};
+ struct page *pp_page, *reg_page;
+ netmem_ref pp_nm, niov_nm;
+ struct page_pool *pool;
+ long base_pp_ref, base_iov_ref;
+ int base_pp_page_ref, base_reg_ref;
+ bool fragstolen;
+ int delta;
+
+ pool = page_pool_create(&pp_params);
+ KUNIT_ASSERT_FALSE(test, IS_ERR(pool));
+ pp_page = page_pool_dev_alloc_pages(pool);
+ KUNIT_ASSERT_NOT_NULL(test, pp_page);
+ pp_nm = page_to_netmem(pp_page);
+ page_pool_ref_netmem(pp_nm);
+
+ reg_page = alloc_page(GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, reg_page);
+
+ KUNIT_ASSERT_EQ(test,
+ percpu_ref_init(&binding.ref, dummy_percpu_ref_release,
+ PERCPU_REF_INIT_ATOMIC, GFP_KERNEL),
+ 0);
+ net_iov_init(&niov, &binding.area, NET_IOV_DMABUF);
+ niov_nm = net_iov_to_netmem(&niov);
+ netmem_set_pp(niov_nm, pool);
+ netmem_or_pp_magic(niov_nm, PP_SIGNATURE);
+ atomic_long_set(&niov.desc.pp_ref_count, 2);
+
+ base_pp_ref = atomic_long_read(&pp_page->pp_ref_count);
+ base_pp_page_ref = page_ref_count(pp_page);
+ base_reg_ref = page_ref_count(reg_page);
+ base_iov_ref = atomic_long_read(&binding.ref.data->count);
+
+ /* 1. Uncloned pp_recycle=1 from: frags stolen without extra ref */
+ to = alloc_skb(0, GFP_KERNEL);
+ from = alloc_skb(0, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, to);
+ KUNIT_ASSERT_NOT_NULL(test, from);
+ skb_put(to, skb_tailroom(to));
+ to->pp_recycle = 1;
+ from->pp_recycle = 1;
+ skb_add_rx_frag(from, 0, pp_page, 0, 64, PAGE_SIZE);
+ page_pool_ref_netmem(pp_nm);
+ KUNIT_EXPECT_TRUE(test, skb_try_coalesce(to, from, &fragstolen,
+ &delta));
+ KUNIT_EXPECT_EQ(test, skb_shinfo(from)->nr_frags, 0);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&pp_page->pp_ref_count),
+ base_pp_ref + 1);
+ consume_skb(from);
+ consume_skb(to);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&pp_page->pp_ref_count),
+ base_pp_ref);
+
+ /* 2. Cloned pp_recycle=1 from (pp_page + non-PP reg_page fallback):
+ * skb_frag_ref(from, i) increments pp_ref_count for pp_page and
+ * page_ref_count for reg_page.
+ */
+ to = alloc_skb(0, GFP_KERNEL);
+ from = alloc_skb(0, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, to);
+ KUNIT_ASSERT_NOT_NULL(test, from);
+ skb_put(to, skb_tailroom(to));
+ to->pp_recycle = 1;
+ from->pp_recycle = 1;
+ skb_add_rx_frag(from, 0, pp_page, 0, 32, PAGE_SIZE);
+ page_pool_ref_netmem(pp_nm);
+ skb_add_rx_frag(from, 1, reg_page, 0, 32, PAGE_SIZE);
+ get_page(reg_page);
+
+ clone = skb_clone(from, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, clone);
+ KUNIT_EXPECT_TRUE(test, skb_try_coalesce(to, from, &fragstolen,
+ &delta));
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&pp_page->pp_ref_count),
+ base_pp_ref + 2);
+ KUNIT_EXPECT_EQ(test, page_ref_count(pp_page), base_pp_page_ref);
+ KUNIT_EXPECT_EQ(test, page_ref_count(reg_page), base_reg_ref + 2);
+ consume_skb(from);
+ consume_skb(clone);
+ consume_skb(to);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&pp_page->pp_ref_count),
+ base_pp_ref);
+ KUNIT_EXPECT_EQ(test, page_ref_count(pp_page), base_pp_page_ref);
+ KUNIT_EXPECT_EQ(test, page_ref_count(reg_page), base_reg_ref);
+
+ /* 3. Cloned unreadable pp_recycle=1 net_iov skb: skb_try_coalesce
+ * increments niov.desc.pp_ref_count via skb_frag_ref(from, i).
+ */
+ to = alloc_skb(0, GFP_KERNEL);
+ from = alloc_skb(0, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, to);
+ KUNIT_ASSERT_NOT_NULL(test, from);
+ skb_put(to, skb_tailroom(to));
+ to->pp_recycle = 1;
+ to->unreadable = 1;
+ from->pp_recycle = 1;
+ skb_add_rx_frag_netmem(from, 0, niov_nm, 0, 64, PAGE_SIZE);
+ napi_pp_get_page(niov_nm);
+
+ clone = skb_clone(from, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, clone);
+ KUNIT_EXPECT_TRUE(test, skb_try_coalesce(to, from, &fragstolen,
+ &delta));
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 4L);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&binding.ref.data->count),
+ base_iov_ref);
+ consume_skb(from);
+ consume_skb(clone);
+ consume_skb(to);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 2L);
+
+ page_pool_put_full_page(pool, pp_page, false);
+ page_pool_put_full_page(pool, pp_page, false);
+ __free_page(reg_page);
+ netmem_clear_pp_magic(niov_nm);
+ netmem_set_pp(niov_nm, NULL);
+ page_pool_destroy(pool);
+ percpu_ref_exit(&binding.ref);
+}
+
+#ifdef CONFIG_INET
+static void netmem_ref_test_tcp_devmem_recv(struct kunit *test)
+{
+ char cmsg_buf[CMSG_SPACE(sizeof(struct dmabuf_cmsg))]
+ __aligned(__alignof__(struct cmsghdr)) = {};
+ struct page_pool_params pp_params = {
+ .order = 0,
+ .pool_size = 4,
+ .nid = NUMA_NO_NODE,
+ };
+ struct net_devmem_dmabuf_binding binding = {};
+ struct msghdr msg = {};
+ struct dmabuf_token token;
+ struct dmabuf_cmsg *cmsg;
+ struct net_iov niov = {};
+ struct page_pool *pool;
+ struct cmsghdr *cm;
+ struct socket *sock;
+ struct sk_buff *skb;
+ struct kvec iov = {};
+ netmem_ref niov_nm;
+ long base_iov_ref;
+ int ret;
+
+ KUNIT_ASSERT_EQ(test,
+ sock_create_kern(&init_net, AF_INET, SOCK_STREAM,
+ IPPROTO_TCP, &sock),
+ 0);
+ sock->sk->sk_state = TCP_ESTABLISHED;
+ inet_csk(sock->sk)->icsk_ack.rcv_mss = 64;
+ tcp_sk(sock->sk)->copied_seq = 0;
+
+ pool = page_pool_create(&pp_params);
+ if (IS_ERR(pool)) {
+ sock->sk->sk_state = TCP_CLOSE;
+ sock_release(sock);
+ KUNIT_ASSERT_FALSE(test, IS_ERR(pool));
+ }
+
+ ret = percpu_ref_init(&binding.ref, dummy_percpu_ref_release,
+ PERCPU_REF_INIT_ATOMIC, GFP_KERNEL);
+ if (ret) {
+ page_pool_destroy(pool);
+ sock->sk->sk_state = TCP_CLOSE;
+ sock_release(sock);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+ }
+ binding.area.niovs = &niov;
+ net_iov_init(&niov, &binding.area, NET_IOV_DMABUF);
+ niov_nm = net_iov_to_netmem(&niov);
+ netmem_set_pp(niov_nm, pool);
+ netmem_or_pp_magic(niov_nm, PP_SIGNATURE);
+ atomic_long_set(&niov.desc.pp_ref_count, 2);
+ base_iov_ref = atomic_long_read(&binding.ref.data->count);
+
+ /* 1. Receive via tcp_recvmsg_dmabuf -> SO_DEVMEM_DONTNEED release */
+ skb = alloc_skb(0, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, skb);
+ skb->pp_recycle = 1;
+ skb_add_rx_frag_netmem(skb, 0, niov_nm, 0, 64, PAGE_SIZE);
+ napi_pp_get_page(niov_nm);
+ TCP_SKB_CB(skb)->seq = 0;
+ TCP_SKB_CB(skb)->end_seq = 64;
+ skb_queue_tail(&sock->sk->sk_receive_queue, skb);
+
+ msg.msg_control = cmsg_buf;
+ msg.msg_controllen = sizeof(cmsg_buf);
+ ret = kernel_recvmsg(sock, &msg, &iov, 0, 64,
+ MSG_SOCK_DEVMEM | MSG_DONTWAIT);
+ KUNIT_ASSERT_EQ(test, ret, 64);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 3L);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&binding.ref.data->count),
+ base_iov_ref);
+
+ cm = (struct cmsghdr *)cmsg_buf;
+ cmsg = CMSG_DATA(cm);
+ token.token_start = cmsg->frag_token;
+ token.token_count = 1;
+ KUNIT_EXPECT_EQ(test,
+ sock_setsockopt(sock, SOL_SOCKET, SO_DEVMEM_DONTNEED,
+ KERNEL_SOCKPTR(&token), sizeof(token)),
+ 1);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 2L);
+
+ /* 2. Receive second packet & destroy socket to test
+ * tcp_release_user_frags() -> napi_pp_put_page().
+ */
+ skb = alloc_skb(0, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, skb);
+ skb->pp_recycle = 1;
+ skb_add_rx_frag_netmem(skb, 0, niov_nm, 0, 64, PAGE_SIZE);
+ napi_pp_get_page(niov_nm);
+ TCP_SKB_CB(skb)->seq = 64;
+ TCP_SKB_CB(skb)->end_seq = 128;
+ skb_queue_tail(&sock->sk->sk_receive_queue, skb);
+
+ msg.msg_control = cmsg_buf;
+ msg.msg_controllen = sizeof(cmsg_buf);
+ ret = kernel_recvmsg(sock, &msg, &iov, 0, 64,
+ MSG_SOCK_DEVMEM | MSG_DONTWAIT);
+ KUNIT_ASSERT_EQ(test, ret, 64);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 3L);
+
+ sock->sk->sk_state = TCP_CLOSE;
+ sock_release(sock);
+ KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 2L);
+
+ netmem_clear_pp_magic(niov_nm);
+ netmem_set_pp(niov_nm, NULL);
+ page_pool_destroy(pool);
+ percpu_ref_exit(&binding.ref);
+}
+#endif
+#endif
+
static struct kunit_case net_test_cases[] = {
KUNIT_CASE_PARAM(gso_test_func, gso_test_gen_params),
KUNIT_CASE_PARAM(ip_tunnel_flags_test_run,
ip_tunnel_flags_test_gen_params),
+ KUNIT_CASE(netmem_ref_test_page),
+#ifdef CONFIG_PAGE_POOL
+ KUNIT_CASE(netmem_ref_test_pp_page),
+#endif
+#if defined(CONFIG_PAGE_POOL) && defined(CONFIG_NET_DEVMEM)
+ KUNIT_CASE(netmem_ref_test_net_iov),
+ KUNIT_CASE(netmem_ref_test_release_data),
+ KUNIT_CASE(netmem_ref_test_skb_segment),
+ KUNIT_CASE(netmem_ref_test_try_coalesce),
+#ifdef CONFIG_INET
+ KUNIT_CASE(netmem_ref_test_tcp_devmem_recv),
+#endif
+#endif
{ },
};
@@ -384,4 +1587,5 @@ static struct kunit_suite net_test_suite = {
kunit_test_suite(net_test_suite);
MODULE_DESCRIPTION("KUnit tests for networking core");
+MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
MODULE_LICENSE("GPL");
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 20fca895d7090..ebd1af82c0e22 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -65,6 +65,7 @@
#include <linux/kcov.h>
#include <linux/iov_iter.h>
#include <linux/crc32.h>
+#include <kunit/visibility.h>
#include <net/protocol.h>
#include <net/dst.h>
@@ -4399,6 +4400,7 @@ int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen)
return shiftlen;
}
+EXPORT_SYMBOL_IF_KUNIT(skb_shift);
/**
* skb_prepare_seq_read - Prepare a sequential read of skb data
--
2.56.0.385.gd3acb90ef8-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v2 1/6] netmem: rename __get/__put_netmem() to get/put_net_iov()
2026-10-12 8:21 ` [PATCH net-next v2 1/6] netmem: rename __get/__put_netmem() to get/put_net_iov() Mina Almasry
@ 2026-10-13 8:22 ` sashiko-bot
2026-10-13 8:34 ` netdev-bot+sashiko
1 sibling, 0 replies; 18+ messages in thread
From: sashiko-bot @ 2026-10-13 8:22 UTC (permalink / raw)
To: Mina Almasry, linux-rdma
> netmems carry both a non-pp backing refcount and a page_pool
> pp_ref_count. Make the non-pp layer explicit and symmetric across both
> netmem types: get/put_netmem() dispatches to get/put_page() for pages
> and get/put_net_iov() for net_iovs, always acquiring or releasing a
> non-pp reference regardless of page_pool membership.
>
> Warn via DEBUG_NET_WARN_ON_ONCE() for net_iov types that do not support
> non-pp references.
>
> Assisted-by: LLM
> Signed-off-by: Mina Almasry <almasrymina@google.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261012082148.1698596-1-almasrymina@google.com?part=1
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v2 2/6] net: skbuff: add napi_pp_get_page() and use it in tcp_recvmsg_dmabuf()
2026-10-12 8:21 ` [PATCH net-next v2 2/6] net: skbuff: add napi_pp_get_page() and use it in tcp_recvmsg_dmabuf() Mina Almasry
@ 2026-10-13 8:22 ` sashiko-bot
0 siblings, 0 replies; 18+ messages in thread
From: sashiko-bot @ 2026-10-13 8:22 UTC (permalink / raw)
To: Mina Almasry, linux-rdma
> While napi_pp_put_page() releases a page_pool pp_ref_count reference on
> a netmem, no symmetric helper existed to acquire one, forcing callers
> like tcp_recvmsg_dmabuf() to open-code atomic_long_inc() on
> niov->desc.pp_ref_count.
>
> Add napi_pp_get_page() to pair symmetrically with napi_pp_put_page(),
> and use WARN_ON_ONCE(!napi_pp_get_page(netmem)) in tcp_recvmsg_dmabuf()
> to match WARN_ON_ONCE(!napi_pp_put_page(netmem)) in
> tcp_release_user_frags().
>
> Assisted-by: LLM
> Signed-off-by: Mina Almasry <almasrymina@google.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261012082148.1698596-1-almasrymina@google.com?part=2
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v2 3/6] net: skbuff: replace skb_page_unref() and __skb_frag_unref() with skb_netmem_unref()
2026-10-12 8:21 ` [PATCH net-next v2 3/6] net: skbuff: replace skb_page_unref() and __skb_frag_unref() with skb_netmem_unref() Mina Almasry
@ 2026-10-13 8:22 ` sashiko-bot
2026-10-13 8:34 ` netdev-bot+sashiko
1 sibling, 0 replies; 18+ messages in thread
From: sashiko-bot @ 2026-10-13 8:22 UTC (permalink / raw)
To: Mina Almasry, linux-rdma
> Unreferencing a page_pool fragment with recycle=false decrements the
> non-pp backing refcount instead of pp_ref_count, underflowing the page
> refcount when the fragment was owned via page_pool (as on mlx4's RX
> error path). Having both skb_page_unref() and __skb_frag_unref(frag,
> recycle) made it easy for callers to pass hardcoded recycle=false on
> SKB-owned fragments.
>
> Replace skb_page_unref() and __skb_frag_unref() with
> skb_netmem_unref(netmem, recycle), which dispatches to
> napi_pp_put_page() when recycle is true and put_netmem() otherwise:
>
> - Use skb_frag_unref(skb, ...) on SKB-owned fragments in mlx4, sky2,
> tls_strp, and skb_shift() so unrefs always match skb->pp_recycle.
> - Use put_netmem() directly in tls_device's destroy_record(), which
> releases raw non-pp page references outside an SKB.
>
> Found by code inspection and confirmed with the KUnit tests added later
> in this series; not observed in production. The mlx4, sky2, and TLS
> call-site changes are compile-tested only.
>
> Assisted-by: LLM
> Signed-off-by: Mina Almasry <almasrymina@google.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261012082148.1698596-1-almasrymina@google.com?part=3
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v2 5/6] net: skbuff: use skb_frag_ref() in skb_try_coalesce()
2026-10-12 8:21 ` [PATCH net-next v2 5/6] net: skbuff: use skb_frag_ref() in skb_try_coalesce() Mina Almasry
@ 2026-10-13 8:22 ` sashiko-bot
0 siblings, 0 replies; 18+ messages in thread
From: sashiko-bot @ 2026-10-13 8:22 UTC (permalink / raw)
To: Mina Almasry, linux-rdma
> skb_pp_frag_ref() was introduced solely to work around skb_frag_ref()
> ignoring skb->pp_recycle and incrementing the non-pp backing refcount.
> Now that skb_frag_ref() handles both pp_recycle=1 and pp_recycle=0
> symmetrically, call skb_frag_ref() directly in skb_try_coalesce() and
> remove skb_pp_frag_ref().
>
> Assisted-by: LLM
> Signed-off-by: Mina Almasry <almasrymina@google.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261012082148.1698596-1-almasrymina@google.com?part=5
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v2 6/6] net: kunit: test netmem, page_pool, and skb frag refcounting
2026-10-12 8:21 ` [PATCH net-next v2 6/6] net: kunit: test netmem, page_pool, and skb frag refcounting Mina Almasry
@ 2026-10-13 8:22 ` sashiko-bot
2026-10-13 15:01 ` Mina Almasry
2026-10-13 8:34 ` netdev-bot+sashiko
1 sibling, 1 reply; 18+ messages in thread
From: sashiko-bot @ 2026-10-13 8:22 UTC (permalink / raw)
To: Mina Almasry, linux-rdma, bpf
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] net/kunit: kernel panic in TCP devmem test due to unset skb->unreadable
--
commit 432a5016a6d079e3237437e79dd79f25313c4f7a
Author: Mina Almasry <almasrymina@google.com>
net: kunit: test netmem, page_pool, and skb frag refcounting
Verify that non-pp (page._refcount / binding->ref) and page_pool
(pp_ref_count) references stay balanced without cross-counter leaks or
underflows across regular pages, page_pool pages, non-pp net_iovs, and
page_pool net_iovs with both pp_recycle=0 and pp_recycle=1.
> diff --git a/net/core/net_test.c b/net/core/net_test.c
> index 9c3a590865d26..6f21e6baf7fb3 100644
> --- a/net/core/net_test.c
> +++ b/net/core/net_test.c
[ ... ]
> @@ -370,10 +370,1213 @@ static void ip_tunnel_flags_test_run(struct kunit *test)
[ ... ]
> +static void netmem_ref_test_tcp_devmem_recv(struct kunit *test)
> +{
[ ... ]
> + /* 1. Receive via tcp_recvmsg_dmabuf -> SO_DEVMEM_DONTNEED release */
> + skb = alloc_skb(0, GFP_KERNEL);
> + KUNIT_ASSERT_NOT_NULL(test, skb);
> + skb->pp_recycle = 1;
> + skb_add_rx_frag_netmem(skb, 0, niov_nm, 0, 64, PAGE_SIZE);
> + napi_pp_get_page(niov_nm);
> + TCP_SKB_CB(skb)->seq = 0;
> + TCP_SKB_CB(skb)->end_seq = 64;
> + skb_queue_tail(&sock->sk->sk_receive_queue, skb);
[Severity: Medium]
Does this code need to set skb->unreadable = 1 when injecting this devmem
fragment?
Without it, tcp_recvmsg_locked() checks skb_frags_readable(skb) and returns
true. The TCP stack will attempt to copy the payload using
skb_copy_datagram_msg() instead of processing it via tcp_recvmsg_dmabuf().
This leads to a kernel panic when __skb_datagram_iter() executes
skb_frag_foreach_page() and retrieves the page pointer via netmem_to_page().
Because the underlying memory is a net_iov, netmem_to_page() returns NULL and
the subsequent kmap_local_page(NULL) call crashes the kernel during test
execution.
> +
> + msg.msg_control = cmsg_buf;
> + msg.msg_controllen = sizeof(cmsg_buf);
> + ret = kernel_recvmsg(sock, &msg, &iov, 0, 64,
> + MSG_SOCK_DEVMEM | MSG_DONTWAIT);
> + KUNIT_ASSERT_EQ(test, ret, 64);
> + KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 3L);
> + KUNIT_EXPECT_EQ(test, atomic_long_read(&binding.ref.data->count),
> + base_iov_ref);
[ ... ]
> + /* 2. Receive second packet & destroy socket to test
> + * tcp_release_user_frags() -> napi_pp_put_page().
> + */
> + skb = alloc_skb(0, GFP_KERNEL);
> + KUNIT_ASSERT_NOT_NULL(test, skb);
> + skb->pp_recycle = 1;
> + skb_add_rx_frag_netmem(skb, 0, niov_nm, 0, 64, PAGE_SIZE);
> + napi_pp_get_page(niov_nm);
> + TCP_SKB_CB(skb)->seq = 64;
> + TCP_SKB_CB(skb)->end_seq = 128;
> + skb_queue_tail(&sock->sk->sk_receive_queue, skb);
[Severity: Medium]
Will this second injected SKB also trigger the same panic since it lacks
skb->unreadable = 1?
> +
> + msg.msg_control = cmsg_buf;
> + msg.msg_controllen = sizeof(cmsg_buf);
> + ret = kernel_recvmsg(sock, &msg, &iov, 0, 64,
> + MSG_SOCK_DEVMEM | MSG_DONTWAIT);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261012082148.1698596-1-almasrymina@google.com?part=6
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v2 4/6] net: skbuff: replace __skb_frag_ref() with symmetric skb_netmem_ref()
2026-10-12 8:21 ` [PATCH net-next v2 4/6] net: skbuff: replace __skb_frag_ref() with symmetric skb_netmem_ref() Mina Almasry
@ 2026-10-13 8:22 ` sashiko-bot
2026-10-13 8:34 ` netdev-bot+sashiko
1 sibling, 0 replies; 18+ messages in thread
From: sashiko-bot @ 2026-10-13 8:22 UTC (permalink / raw)
To: Mina Almasry, linux-rdma
> Previously, __skb_frag_ref() and skb_frag_ref() unconditionally
> incremented the non-pp backing refcount via get_netmem(), while
> skb_frag_unref() decremented pp_ref_count whenever skb->pp_recycle was
> set. Ref-ing a page_pool fragment into a pp_recycle=1 SKB (or clearing
> pp_recycle=0 on an uncloned SKB while clones still held pp_recycle=1)
> leaked the backing non-pp refcount and underflowed pp_ref_count across
> IP-TFS, skb_split(), skb_shift(), and cloned unclone paths.
>
> Replace __skb_frag_ref() with skb_netmem_ref(netmem, recycle) as the
> exact counterpart to skb_netmem_unref(netmem, recycle):
>
> - Keep skb->pp_recycle intact in skb_release_data() so uncloning paths
> (pskb_expand_head() and pskb_carve_inside_{header,nonlinear}()) keep
> trading pp_ref_count references symmetrically across clones.
> - Propagate pp_recycle and call skb_frag_ref() on the destination SKB in
> [ ... ]
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Jesper Dangaard Brouer <hawk@kernel.org>
> Cc: bpf@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Mina Almasry <almasrymina@google.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261012082148.1698596-1-almasrymina@google.com?part=4
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v2 1/6] netmem: rename __get/__put_netmem() to get/put_net_iov()
2026-10-12 8:21 ` [PATCH net-next v2 1/6] netmem: rename __get/__put_netmem() to get/put_net_iov() Mina Almasry
2026-10-13 8:22 ` sashiko-bot
@ 2026-10-13 8:34 ` netdev-bot+sashiko
1 sibling, 0 replies; 18+ messages in thread
From: netdev-bot+sashiko @ 2026-10-13 8:34 UTC (permalink / raw)
To: almasrymina
Cc: netdev, linux-kernel, linux-rdma, bpf, ayush.sawal, andrew+netdev,
davem, edumazet, kuba, pabeni, tariqt, horms, steffen.klassert,
herbert, ncardwell, kuniyu, john.fastabend, sd, ebiggers, kees,
aleksandr.loktionov, u.kleine-koenig, shaojijie, mgr, j.raczynski
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] The new kernel-doc for get_netmem() and put_netmem() in
include/net/netmem.h is wrong for one of the two net_iov types.
--
netmem: rename __get/__put_netmem() to get/put_net_iov()
This commit renames __get_netmem()/__put_netmem() to get_net_iov() and
put_net_iov(), changing them to take a struct net_iov pointer. It adds
kernel-doc to get_netmem()/put_netmem() and the new helpers, and adds a
DEBUG_NET_WARN_ON_ONCE() for net_iov types without non-pp reference
support.
> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index 0cc8572f62caf..697d50c51a5ee 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h
> @@ -399,21 +399,37 @@ static inline bool net_is_devmem_iov(const struct net_iov *niov)
> }
> #endif
>
> -void __get_netmem(netmem_ref netmem);
> -void __put_netmem(netmem_ref netmem);
> +void get_net_iov(struct net_iov *niov);
> +void put_net_iov(struct net_iov *niov);
>
> +/**
> + * get_netmem - acquire a non-page_pool reference on a netmem
> + * @netmem: netmem to reference
> + *
> + * Acquires a non-pp backing reference (page._refcount via get_page() or
> + * binding->ref via get_net_iov()) regardless of page_pool membership.
> + * Counterpart to put_netmem().
> + */
[Severity: Low]
Is this kernel-doc accurate for every net_iov type?
get_net_iov() only takes a reference when net_is_devmem_iov() is true,
which means the NET_IOV_DMABUF type:
net/core/skbuff.c:get_net_iov() {
if (net_is_devmem_iov(niov))
net_devmem_get_net_iov(niov);
else
DEBUG_NET_WARN_ON_ONCE(true);
}
For NET_IOV_IOURING (io_uring zcrx) niovs, and for all niovs when
CONFIG_NET_DEVMEM=n, neither get_netmem() nor put_netmem() takes or drops
any reference.
Without CONFIG_DEBUG_NET, DEBUG_NET_WARN_ON_ONCE() expands to
BUILD_BUG_ON_INVALID(). On production kernels that case is skipped
silently, so the "warning if @niov does not support non-pp references"
in the get_net_iov()/put_net_iov() kernel-doc never fires there.
The commit message has a similar statement:
Make the non-pp layer explicit and symmetric across both
netmem types: get/put_netmem() dispatches to get/put_page() for pages
and get/put_net_iov() for net_iovs, always acquiring or releasing a
non-pp reference regardless of page_pool membership.
This doesn't change runtime behavior, since __get_netmem()/__put_netmem()
also did nothing for io_uring niovs. A future caller that relies on this
kernel-doc could still pair get_netmem()/put_netmem() on a zcrx niov and
get no lifetime protection.
Could the get_netmem()/put_netmem() kernel-doc say that NET_IOV_IOURING
has no non-pp refcount, so these helpers do nothing for it? It could also
say that the warning only exists under CONFIG_DEBUG_NET.
> static __always_inline void get_netmem(netmem_ref netmem)
> {
> if (netmem_is_net_iov(netmem))
> - __get_netmem(netmem);
> + get_net_iov(netmem_to_net_iov(netmem));
> else
> get_page(netmem_to_page(netmem));
> }
>
> +/**
> + * put_netmem - release a non-page_pool reference on a netmem
> + * @netmem: netmem to unreference
> + *
> + * Drops a non-pp backing reference (page._refcount via put_page() or
> + * binding->ref via put_net_iov()) regardless of page_pool membership.
> + * Counterpart to get_netmem().
> + */
The same question applies to put_netmem(). put_net_iov() only calls
net_devmem_put_net_iov() for NET_IOV_DMABUF niovs.
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012082148.1698596-1-almasrymina%40google.com
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v2 3/6] net: skbuff: replace skb_page_unref() and __skb_frag_unref() with skb_netmem_unref()
2026-10-12 8:21 ` [PATCH net-next v2 3/6] net: skbuff: replace skb_page_unref() and __skb_frag_unref() with skb_netmem_unref() Mina Almasry
2026-10-13 8:22 ` sashiko-bot
@ 2026-10-13 8:34 ` netdev-bot+sashiko
1 sibling, 0 replies; 18+ messages in thread
From: netdev-bot+sashiko @ 2026-10-13 8:34 UTC (permalink / raw)
To: almasrymina
Cc: netdev, linux-kernel, linux-rdma, bpf, ayush.sawal, andrew+netdev,
davem, edumazet, kuba, pabeni, tariqt, horms, steffen.klassert,
herbert, ncardwell, kuniyu, john.fastabend, sd, ebiggers, kees,
aleksandr.loktionov, u.kleine-koenig, shaojijie, mgr, j.raczynski
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · Low: 1
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] The patch fixes a real page refcount bug on mlx4's RX error
path but has no Fixes: tag, and the fix is folded into a tree-wide API…
- [Low] At this commit, the new kernel-doc for skb_netmem_unref() in
include/linux/skbuff_ref.h says it is the "Counterpart to…
--
net: skbuff: replace skb_page_unref() and __skb_frag_unref() with skb_netmem_unref()
This patch replaces skb_page_unref() and __skb_frag_unref() with a single
helper, skb_netmem_unref(netmem, recycle). The SKB-owned fragment unrefs
in mlx4, sky2, tls_strp and skb_shift() now use skb_frag_unref(), so they
follow skb->pp_recycle. tls_device's destroy_record() now calls
put_netmem() directly.
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> index 96f97fa0f5489..fe18606c2ab55 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> @@ -497,7 +497,7 @@ static int mlx4_en_complete_rx_desc(struct mlx4_en_priv *priv,
> fail:
> while (nr > 0) {
> nr--;
> - __skb_frag_unref(skb_shinfo(skb)->frags + nr, false);
> + skb_frag_unref(skb, nr);
> }
> return 0;
> }
[Severity: Medium]
This looks like a real fix for mlx4. Should it have a Fixes: tag and go
in separately from the API rework?
Commit 82b023c97f60 ("eth: mlx4: use the page pool for Rx buffers")
changed each attached frag to hold a page_pool pp_ref_count reference,
using page_pool_fragment_page(), page_pool_ref_page() and
skb_mark_for_recycle(). It did not update this fail path, which still
called put_page() through __skb_frag_unref(..., false).
mlx4_en_process_rx_cq() calls skb_mark_for_recycle() before it calls
mlx4_en_complete_rx_desc(). That means skb_frag_unref() here correctly
goes through napi_pp_put_page().
In the XDP layout (frag_stride == PAGE_SIZE with rx_headroom set),
release stays true and frags->page is cleared. The skb then holds the
only reference. The old put_page() could free a page that page_pool
still counts as in flight.
As posted, the patch has no Fixes: trailer and the subject reads like a
cleanup. The one-line mlx4 change also depends on removing
__skb_frag_unref() and skb_page_unref(), and on edits to sky2, tls,
esp4/esp6 and skbuff.c.
Stable trees that contain 82b023c97f60 won't pick this up, and the mlx4
change can't be backported on its own.
Could the mlx4 change be sent first as a standalone patch with
Fixes: 82b023c97f60, with the rename built on top of it?
> diff --git a/include/linux/skbuff_ref.h b/include/linux/skbuff_ref.h
> index 0441c220fdbdf..04114c64dad99 100644
> --- a/include/linux/skbuff_ref.h
> +++ b/include/linux/skbuff_ref.h
> @@ -36,7 +36,16 @@ static __always_inline void skb_frag_ref(struct sk_buff *skb, int f)
>
> bool napi_pp_put_page(netmem_ref netmem);
>
> -static __always_inline void skb_page_unref(netmem_ref netmem, bool recycle)
> +/**
> + * skb_netmem_unref - release a fragment reference on a netmem
> + * @netmem: netmem to unreference
> + * @recycle: whether page_pool recycling is enabled (e.g. skb->pp_recycle)
> + *
> + * Drops pp_ref_count via napi_pp_put_page() if @recycle is true and @netmem
> + * belongs to a page_pool; otherwise drops a non-pp backing reference via
> + * put_netmem(). Counterpart to skb_netmem_ref().
> + */
[ ... ]
> - * skb_frag_unref - release a reference on a paged fragment of an skb.
> + * skb_frag_unref - release a reference on a paged fragment of an skb
> * @skb: the buffer
> * @f: the fragment offset
> *
> - * Releases a reference on the @f'th paged fragment of @skb.
> + * Drops a reference on the @f'th fragment of @skb via skb_netmem_unref()
> + * (pp_ref_count if skb->pp_recycle and page_pool-backed, else non-pp
> + * backing refcount), skipping managed zerocopy fragments. Counterpart to
> + * skb_frag_ref().
> */
[Severity: Low]
This isn't a bug, but are these kernel-doc references accurate at this
point in the series?
The skb_netmem_unref() doc calls it the "Counterpart to
skb_netmem_ref()", but skb_netmem_ref() doesn't exist yet at this commit.
The skb_frag_unref() doc calls it the "Counterpart to skb_frag_ref()".
At this commit, though, skb_frag_ref() always takes a non-pp reference:
static __always_inline void __skb_frag_ref(skb_frag_t *frag)
{
get_netmem(skb_frag_netmem(frag));
}
On a pp_recycle skb, skb_frag_unref() drops pp_ref_count instead, so the
two helpers change different counters. skb_shift() uses both, calling
__skb_frag_ref(fragfrom) and the pp-aware skb_frag_unref(skb, 0).
A later patch in the series resolves both: "net: skbuff: replace
__skb_frag_ref() with symmetric skb_netmem_ref()". It adds
skb_netmem_ref() and makes skb_frag_ref() dispatch on skb->pp_recycle.
Would it be cleaner to add the "Counterpart to" wording in that patch
instead?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012082148.1698596-1-almasrymina%40google.com
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v2 4/6] net: skbuff: replace __skb_frag_ref() with symmetric skb_netmem_ref()
2026-10-12 8:21 ` [PATCH net-next v2 4/6] net: skbuff: replace __skb_frag_ref() with symmetric skb_netmem_ref() Mina Almasry
2026-10-13 8:22 ` sashiko-bot
@ 2026-10-13 8:34 ` netdev-bot+sashiko
1 sibling, 0 replies; 18+ messages in thread
From: netdev-bot+sashiko @ 2026-10-13 8:34 UTC (permalink / raw)
To: almasrymina
Cc: netdev, linux-kernel, linux-rdma, bpf, ayush.sawal, andrew+netdev,
davem, edumazet, kuba, pabeni, tariqt, horms, steffen.klassert,
herbert, ncardwell, kuniyu, john.fastabend, sd, ebiggers, kees,
aleksandr.loktionov, u.kleine-koenig, shaojijie, mgr, j.raczynski
Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 2 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] The new SG-disable block in skb_segment() (net/core/skbuff.c,
lines 4861-4873) clears NETIF_F_SG whenever frag_list members have…
- [Medium] The commit fixes pp_ref_count/page refcount corruption that is
reachable today, but it has no Fixes: tag and no stable Cc.
- [Medium] The commit message overstates which paths were buggy and
leaves out real behavioural changes.
--
net: skbuff: replace __skb_frag_ref() with symmetric skb_netmem_ref()
This replaces __skb_frag_ref() with skb_netmem_ref(netmem, recycle). As a
result, skb_frag_ref() takes a pp_ref_count reference when the destination
skb has pp_recycle set, mirroring skb_frag_unref(). The patch also stops
clearing pp_recycle in skb_release_data(), propagates pp_recycle in several
copy/split/segment paths, and clears NETIF_F_SG in skb_segment() when
frag_list members have mixed pp_recycle bits.
Link: https://lore.kernel.org/netdev/xfrm-iptfs-pp_ref_count-underflow-v4-1-912fa72106f0@secunet.com/ [1]
> Previously, __skb_frag_ref() and skb_frag_ref() unconditionally
> incremented the non-pp backing refcount via get_netmem(), while
> skb_frag_unref() decremented pp_ref_count whenever skb->pp_recycle was
> set. Ref-ing a page_pool fragment into a pp_recycle=1 SKB (or clearing
> pp_recycle=0 on an uncloned SKB while clones still held pp_recycle=1)
> leaked the backing non-pp refcount and underflowed pp_ref_count across
> IP-TFS, skb_split(), skb_shift(), and cloned unclone paths.
[Severity: Medium]
Is this list of buggy paths accurate?
skb_shift() is only called from TCP write/retransmit queue paths:
tcp_skb_shift(), tcp_grow_skb() and tcp_collapse_retrans(). pp_recycle is
never set on those skbs, and skb_shift() already asserts:
DEBUG_NET_WARN_ON_ONCE(tgt->pp_recycle != skb->pp_recycle);
How would the described leak happen across skb_shift()?
Before this patch, the cloned unclone paths (pskb_expand_head(),
pskb_carve_inside_header() and pskb_carve_inside_nonlinear()),
__pskb_copy_fclone(), skb_zerocopy() and skb_segment() all looked
self-consistent. Each took plain page refs, and in each case one of these
held:
- the destination had pp_recycle=0, since __copy_skb_header() does not
copy it
- skb_release_data() had cleared pp_recycle, so the release was a
put_page()
Isn't removing skb->pp_recycle = 0 then required by the new skb_frag_ref()
semantics, rather than a fix for an existing bug?
The removed comment and behaviour in skb_release_data() came from "skbuff:
Fix a potential race while recycling page_pool packets". Could the commit
message explain why that race no longer applies?
The message also leaves out some behaviour changes:
- skb_shift() switches from skb_can_coalesce() to
skb_can_coalesce_netmem(), so adjacent net_iov frags can now coalesce.
Before, page_to_netmem(NULL) never matched.
- Every skb_frag_ref() caller this patch does not touch now takes pp refs
when pp_recycle is set. That includes iptfs_consume_frags(),
tcp_clone_payload(), __skb_zcopy_downgrade_managed(),
pskb_expand_head() and the pskb_carve_*() helpers.
Should these be called out?
[ ... ]
> Link: https://lore.kernel.org/netdev/xfrm-iptfs-pp_ref_count-underflow-v4-1-912fa72106f0@secunet.com/ [1]
> Cc: Antony Antony <antony.antony@secunet.com>
> Cc: Stanislav Fomichev <sdf@fomichev.me>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Jesper Dangaard Brouer <hawk@kernel.org>
> Cc: bpf@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Mina Almasry <almasrymina@google.com>
[Severity: Medium]
This fixes page_pool refcount corruption that can be reached today, but the
trailers have no Fixes: tag or stable Cc. Should they be added?
For IP-TFS, the actual fix appears to be a behaviour change at a call site
in iptfs_consume_frags() that this patch does not touch:
iptfs_skb_head_to_frag(from, &toi->frags[toi->nr_frags]);
skb_frag_ref(to, toi->nr_frags++);
Before this patch, that took a plain get_page() on from's page_pool head
page. Then consume_skb(from) dropped a pp ref, and to (pp_recycle=1) later
dropped another pp ref through napi_pp_put_page(). Could the commit message
name this call site as the IP-TFS fix?
For skb_split(), batadv_frag_create() calls:
skb_split(skb, skb_fragment, skb->len - fragment_size);
skb_fragment comes from dev_alloc_skb(), so it has pp_recycle=0.
batadv_frag_send_packet() only linearizes frag_list, not frags[].
Before this patch, page_pool-backed frags moved into skb_fragment were
later released with put_page() while page_pool still considered them in
flight. The new skb1->pp_recycle = skb->pp_recycle in skb_split() fixes
this, but the caller is not mentioned.
The fix is mixed in with an API rename and a semantic change to every
skb_frag_ref() caller. Would splitting the fixes out make them easier to
backport?
Also, how does this patch relate to the separately linked IP-TFS fix [1],
which touches the same code?
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 46da61ad1f9eb..fe73f6e8d2b3e 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
[ ... ]
> @@ -4866,6 +4858,20 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
> }
> }
>
> + if (list_skb) {
> + bool recycle = skb_shinfo(head_skb)->nr_frags ?
> + head_skb->pp_recycle : list_skb->pp_recycle;
> + struct sk_buff *check_skb;
> +
> + for (check_skb = list_skb; check_skb;
> + check_skb = check_skb->next) {
> + if (check_skb->pp_recycle != recycle) {
> + features &= ~NETIF_F_SG;
> + break;
> + }
> + }
> + }
[Severity: High]
Can clearing NETIF_F_SG here lead skb_segment() into BUG_ON(i >= nfrags)?
Each iteration of the !sg copy path ends with continue. It never advances
pos, i, frag, nfrags or list_skb:
if (!sg) {
...
if (skb_copy_bits(head_skb, offset, skb_put(nskb, len), len))
goto err;
}
continue;
}
Once hsize <= 0 and i >= nfrags, a later segment can take the clone branch
with a stale list_skb and pos. This happens when its len equals
skb_headlen() of that stale list_skb:
if (hsize <= 0 && i >= nfrags && skb_headlen(list_skb) &&
(skb_headlen(list_skb) == len || sg)) {
...
while (pos < offset + len) {
BUG_ON(i >= nfrags);
Take this example with mss 1000:
- head_skb has 1000 bytes of linear payload and no frags
- frag_list member A has 100 linear bytes, one 900 byte frag, and
pp_recycle=1
- member B has 100 linear bytes and pp_recycle=0
With SG cleared, the iterations go like this:
- iteration 1 copies the head
- iteration 2 has hsize 0 and headlen(A) 100 != len 1000, so it copies A
without advancing the cursor
- iteration 3 has len 100 == headlen(A), so it clones the stale A
- pos goes from doffset+1000 to doffset+1100, and then to doffset+2000
after the 900 byte frag
- pos is still below offset + len (doffset+2100), so BUG_ON(1 >= 1)
fires
With other geometries, could nskb instead be cloned from the wrong
frag_list member and carry the wrong payload?
Mixed pp_recycle frag_lists look reachable:
- skb_gro_receive() rejects mismatched pp_recycle, but
skb_gro_receive_list() does not check it.
- Drivers like bnxt mix copybreak napi_alloc_skb() skbs (pp_recycle=0)
with page_pool skbs (pp_recycle=1).
- tcp4_gso_segment() falls back from __tcp4_gso_segment_list() to
skb_segment() when SKB_GSO_DODGY is set (for example by a tc BPF
program) or when skb_pagelen() no longer matches gso_size.
The !sg cursor issue predates this patch. But before this change, these
skbs took the SG path, where the cursor stays in sync.
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012082148.1698596-1-almasrymina%40google.com
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v2 6/6] net: kunit: test netmem, page_pool, and skb frag refcounting
2026-10-12 8:21 ` [PATCH net-next v2 6/6] net: kunit: test netmem, page_pool, and skb frag refcounting Mina Almasry
2026-10-13 8:22 ` sashiko-bot
@ 2026-10-13 8:34 ` netdev-bot+sashiko
1 sibling, 0 replies; 18+ messages in thread
From: netdev-bot+sashiko @ 2026-10-13 8:34 UTC (permalink / raw)
To: almasrymina
Cc: netdev, linux-kernel, linux-rdma, bpf, ayush.sawal, andrew+netdev,
davem, edumazet, kuba, pabeni, tariqt, horms, steffen.klassert,
herbert, ncardwell, kuniyu, john.fastabend, sd, ebiggers, kees,
aleksandr.loktionov, u.kleine-koenig, shaojijie, mgr, j.raczynski
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] None of the new test cases register KUnit cleanup actions.
--
net: kunit: test netmem, page_pool, and skb frag refcounting
This adds KUnit cases to net/core/net_test.c. They check that non-pp
refcounts and page_pool pp_ref_count stay balanced across regular pages,
page_pool pages, and net_iovs, through the various skb frag helpers. It
also exports skb_shift() via EXPORT_SYMBOL_IF_KUNIT() so the test links
when built as a module.
> diff --git a/net/core/net_test.c b/net/core/net_test.c
> index 9c3a590865d26..6f21e6baf7fb3 100644
> --- a/net/core/net_test.c
> +++ b/net/core/net_test.c
> @@ -370,10 +370,1213 @@ static void ip_tunnel_flags_test_run(struct kunit *test)
[ ... ]
> +#ifdef CONFIG_INET
> +static void netmem_ref_test_tcp_devmem_recv(struct kunit *test)
> +{
[ ... ]
> + KUNIT_ASSERT_EQ(test,
> + sock_create_kern(&init_net, AF_INET, SOCK_STREAM,
> + IPPROTO_TCP, &sock),
> + 0);
> + sock->sk->sk_state = TCP_ESTABLISHED;
> + inet_csk(sock->sk)->icsk_ack.rcv_mss = 64;
> + tcp_sk(sock->sk)->copied_seq = 0;
> +
> + pool = page_pool_create(&pp_params);
> + if (IS_ERR(pool)) {
> + sock->sk->sk_state = TCP_CLOSE;
> + sock_release(sock);
> + KUNIT_ASSERT_FALSE(test, IS_ERR(pool));
> + }
> +
> + ret = percpu_ref_init(&binding.ref, dummy_percpu_ref_release,
> + PERCPU_REF_INIT_ATOMIC, GFP_KERNEL);
> + if (ret) {
> + page_pool_destroy(pool);
> + sock->sk->sk_state = TCP_CLOSE;
> + sock_release(sock);
> + KUNIT_ASSERT_EQ(test, ret, 0);
> + }
[ ... ]
> + /* 1. Receive via tcp_recvmsg_dmabuf -> SO_DEVMEM_DONTNEED release */
> + skb = alloc_skb(0, GFP_KERNEL);
> + KUNIT_ASSERT_NOT_NULL(test, skb);
> + skb->pp_recycle = 1;
> + skb_add_rx_frag_netmem(skb, 0, niov_nm, 0, 64, PAGE_SIZE);
> + napi_pp_get_page(niov_nm);
> + TCP_SKB_CB(skb)->seq = 0;
> + TCP_SKB_CB(skb)->end_seq = 64;
> + skb_queue_tail(&sock->sk->sk_receive_queue, skb);
> +
> + msg.msg_control = cmsg_buf;
> + msg.msg_controllen = sizeof(cmsg_buf);
> + ret = kernel_recvmsg(sock, &msg, &iov, 0, 64,
> + MSG_SOCK_DEVMEM | MSG_DONTWAIT);
> + KUNIT_ASSERT_EQ(test, ret, 64);
[Severity: Low]
If this assert fails, is the kernel TCP socket leaked along with the pool
and the percpu_ref data of the on-stack binding?
The page_pool_create() and percpu_ref_init() failure paths above release
everything by hand before asserting. The later asserts do not, which
includes this one, the KUNIT_ASSERT_NOT_NULL(test, skb) checks, and the
second KUNIT_ASSERT_EQ(test, ret, 64) after the second kernel_recvmsg().
None of sock_release(), page_pool_destroy() or percpu_ref_exit() run on
those paths.
The leaked socket's sk_receive_queue or sk_user_frags can still refer to
the on-stack niov. Nothing touches them afterwards, though, because the
socket is never released.
Other new cases follow the same pattern:
netmem_ref_test_page(): the alloc_skb() assert runs while the test
still owns the page from alloc_page().
netmem_ref_test_pp_page(): the page_pool_dev_alloc_pages() assert
skips page_pool_destroy().
netmem_ref_test_release_data(): the percpu_ref_init() assert runs
while the pp page still holds extra refs, so the pool and page leak.
netmem_ref_test_skb_segment(): the reg_page, percpu_ref_init() and
skb allocation asserts come after the pool and pp_page are set up,
with no cleanup.
Could these use kunit_add_action_or_reset() or similar? Then teardown
would still happen when an assert aborts the case.
> + KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 3L);
> + KUNIT_EXPECT_EQ(test, atomic_long_read(&binding.ref.data->count),
> + base_iov_ref);
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012082148.1698596-1-almasrymina%40google.com
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v2 6/6] net: kunit: test netmem, page_pool, and skb frag refcounting
2026-10-13 8:22 ` sashiko-bot
@ 2026-10-13 15:01 ` Mina Almasry
0 siblings, 0 replies; 18+ messages in thread
From: Mina Almasry @ 2026-10-13 15:01 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-rdma, bpf
On Tue, Oct 13, 2026 at 1:22 AM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] net/kunit: kernel panic in TCP devmem test due to unset skb->unreadable
> --
>
> commit 432a5016a6d079e3237437e79dd79f25313c4f7a
> Author: Mina Almasry <almasrymina@google.com>
>
> net: kunit: test netmem, page_pool, and skb frag refcounting
>
> Verify that non-pp (page._refcount / binding->ref) and page_pool
> (pp_ref_count) references stay balanced without cross-counter leaks or
> underflows across regular pages, page_pool pages, non-pp net_iovs, and
> page_pool net_iovs with both pp_recycle=0 and pp_recycle=1.
>
> > diff --git a/net/core/net_test.c b/net/core/net_test.c
> > index 9c3a590865d26..6f21e6baf7fb3 100644
> > --- a/net/core/net_test.c
> > +++ b/net/core/net_test.c
> [ ... ]
> > @@ -370,10 +370,1213 @@ static void ip_tunnel_flags_test_run(struct kunit *test)
> [ ... ]
> > +static void netmem_ref_test_tcp_devmem_recv(struct kunit *test)
> > +{
> [ ... ]
> > + /* 1. Receive via tcp_recvmsg_dmabuf -> SO_DEVMEM_DONTNEED release */
> > + skb = alloc_skb(0, GFP_KERNEL);
> > + KUNIT_ASSERT_NOT_NULL(test, skb);
> > + skb->pp_recycle = 1;
> > + skb_add_rx_frag_netmem(skb, 0, niov_nm, 0, 64, PAGE_SIZE);
> > + napi_pp_get_page(niov_nm);
> > + TCP_SKB_CB(skb)->seq = 0;
> > + TCP_SKB_CB(skb)->end_seq = 64;
> > + skb_queue_tail(&sock->sk->sk_receive_queue, skb);
>
> [Severity: Medium]
> Does this code need to set skb->unreadable = 1 when injecting this devmem
> fragment?
>
> Without it, tcp_recvmsg_locked() checks skb_frags_readable(skb) and returns
> true. The TCP stack will attempt to copy the payload using
> skb_copy_datagram_msg() instead of processing it via tcp_recvmsg_dmabuf().
>
> This leads to a kernel panic when __skb_datagram_iter() executes
> skb_frag_foreach_page() and retrieves the page pointer via netmem_to_page().
> Because the underlying memory is a net_iov, netmem_to_page() returns NULL and
> the subsequent kmap_local_page(NULL) call crashes the kernel during test
> execution.
>
It certainly doesn't panic when I run it. And we're testing
refcounting here, I feel like going through a rabbithole of what skb
flags may be wrong on these test skbs may be pointless. And general
design is not necesasrily net-iov == unreadable.
> > +
> > + msg.msg_control = cmsg_buf;
> > + msg.msg_controllen = sizeof(cmsg_buf);
> > + ret = kernel_recvmsg(sock, &msg, &iov, 0, 64,
> > + MSG_SOCK_DEVMEM | MSG_DONTWAIT);
> > + KUNIT_ASSERT_EQ(test, ret, 64);
> > + KUNIT_EXPECT_EQ(test, atomic_long_read(&niov.desc.pp_ref_count), 3L);
> > + KUNIT_EXPECT_EQ(test, atomic_long_read(&binding.ref.data->count),
> > + base_iov_ref);
> [ ... ]
> > + /* 2. Receive second packet & destroy socket to test
> > + * tcp_release_user_frags() -> napi_pp_put_page().
> > + */
> > + skb = alloc_skb(0, GFP_KERNEL);
> > + KUNIT_ASSERT_NOT_NULL(test, skb);
> > + skb->pp_recycle = 1;
> > + skb_add_rx_frag_netmem(skb, 0, niov_nm, 0, 64, PAGE_SIZE);
> > + napi_pp_get_page(niov_nm);
> > + TCP_SKB_CB(skb)->seq = 64;
> > + TCP_SKB_CB(skb)->end_seq = 128;
> > + skb_queue_tail(&sock->sk->sk_receive_queue, skb);
>
> [Severity: Medium]
> Will this second injected SKB also trigger the same panic since it lacks
> skb->unreadable = 1?
>
Same.
--
Thanks,
Mina
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2026-10-13 15:01 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-12 8:21 [PATCH net-next v2 0/6] net: unify symmetric netmem and page_pool refcounting Mina Almasry
2026-10-12 8:21 ` [PATCH net-next v2 1/6] netmem: rename __get/__put_netmem() to get/put_net_iov() Mina Almasry
2026-10-13 8:22 ` sashiko-bot
2026-10-13 8:34 ` netdev-bot+sashiko
2026-10-12 8:21 ` [PATCH net-next v2 2/6] net: skbuff: add napi_pp_get_page() and use it in tcp_recvmsg_dmabuf() Mina Almasry
2026-10-13 8:22 ` sashiko-bot
2026-10-12 8:21 ` [PATCH net-next v2 3/6] net: skbuff: replace skb_page_unref() and __skb_frag_unref() with skb_netmem_unref() Mina Almasry
2026-10-13 8:22 ` sashiko-bot
2026-10-13 8:34 ` netdev-bot+sashiko
2026-10-12 8:21 ` [PATCH net-next v2 4/6] net: skbuff: replace __skb_frag_ref() with symmetric skb_netmem_ref() Mina Almasry
2026-10-13 8:22 ` sashiko-bot
2026-10-13 8:34 ` netdev-bot+sashiko
2026-10-12 8:21 ` [PATCH net-next v2 5/6] net: skbuff: use skb_frag_ref() in skb_try_coalesce() Mina Almasry
2026-10-13 8:22 ` sashiko-bot
2026-10-12 8:21 ` [PATCH net-next v2 6/6] net: kunit: test netmem, page_pool, and skb frag refcounting Mina Almasry
2026-10-13 8:22 ` sashiko-bot
2026-10-13 15:01 ` Mina Almasry
2026-10-13 8:34 ` netdev-bot+sashiko
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.