* [PATCH net-next v3 0/2] net: netmem: document design principles and intended direction
@ 2026-10-10 3:36 Mina Almasry
2026-10-10 3:36 ` [PATCH net-next v3 1/2] net: netmem: document netmem and memory provider design in comments Mina Almasry
2026-10-10 3:36 ` [PATCH net-next v3 2/2] docs: netmem: document netmem and memory provider design principles Mina Almasry
0 siblings, 2 replies; 5+ messages in thread
From: Mina Almasry @ 2026-10-10 3:36 UTC (permalink / raw)
To: netdev, linux-doc, linux-kernel, bpf
Cc: Mina Almasry, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Jonathan Corbet, Shuah Khan,
Randy Dunlap, Jesper Dangaard Brouer, Ilias Apalodimas,
Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Stanislav Fomichev, Luigi Rizzo, Björn Töpel,
Pavel Begunkov
Because the networking stack and drivers are only partially converted to
netmem_ref and current memory providers only supply unreadable net_iovs,
automated code review and analysis tools (such as LLMs) frequently infer
the wrong architectural invariants from existing code. Specifically,
they often assume that page_pool's struct page APIs are primary rather
than legacy wrappers, that memory providers always imply net_iov, that
net_iov is inherently unreadable, or that callers should branch on or
downcast netmem_ref directly.
Document the intended netmem, memory provider, page_pool, and skb
fragment design principles concisely in the relevant headers, code
comments, and netmem documentation so both developers and automated
tools follow the intended abstractions. This series adds comments and
documentation rather than performing a large refactor all at once, so
that future incremental changes nudge the codebase in the intended
direction.
This reflects my mental model of the netmem, page_pool, and memory
provider architecture; I welcome feedback and disagreements,
particularly from major contributors to page_pool and memory providers.
Cc: Luigi Rizzo <lrizzo@google.com>
Cc: Björn Töpel <bjorn@kernel.org>
Cc: Stanislav Fomichev <sdf@fomichev.me>
Cc: Pavel Begunkov <asml.silence@gmail.com>
---
v3:
- Collect Reviewed-by/Acked-by tags from Björn, Pavel, and Stanislav.
- Clarify skb fragment homogeneity rule: all frags in an skb must either
be struct page-backed or belong to the same memory provider instance
(Pavel Begunkov).
- Link to v2: https://lore.kernel.org/netdev/20261008023030.1089616-1-almasrymina@google.com/
v2:
- Document both current implementation status (memory providers currently
supply net_iov, and net_iov is currently unreadable) and target design
principles, and clarify that new code should generalize existing
limitations as much as possible (Stanislav Fomichev).
- Link to v1: https://lore.kernel.org/netdev/20261005004958.3603059-1-almasrymina@google.com/
Mina Almasry (2):
net: netmem: document netmem and memory provider design in comments
docs: netmem: document netmem and memory provider design principles
Documentation/networking/netmem.rst | 53 +++++++++++++++++++++++++
include/linux/skbuff.h | 6 +++
include/net/netmem.h | 30 +++++++++-----
include/net/page_pool/helpers.h | 18 ++++++---
include/net/page_pool/memory_provider.h | 10 +++++
include/net/page_pool/types.h | 6 +--
net/core/skbuff.c | 4 ++
7 files changed, 109 insertions(+), 18 deletions(-)
base-commit: d8674294aefef02266c4d47ad10131f1bffbe534
--
2.56.0.385.gd3acb90ef8-goog
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net-next v3 1/2] net: netmem: document netmem and memory provider design in comments
2026-10-10 3:36 [PATCH net-next v3 0/2] net: netmem: document design principles and intended direction Mina Almasry
@ 2026-10-10 3:36 ` Mina Almasry
2026-10-11 3:51 ` netdev-bot+sashiko
2026-10-10 3:36 ` [PATCH net-next v3 2/2] docs: netmem: document netmem and memory provider design principles Mina Almasry
1 sibling, 1 reply; 5+ messages in thread
From: Mina Almasry @ 2026-10-10 3:36 UTC (permalink / raw)
To: netdev, linux-doc, linux-kernel, bpf
Cc: Mina Almasry, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Jonathan Corbet, Shuah Khan,
Randy Dunlap, Jesper Dangaard Brouer, Ilias Apalodimas,
Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Stanislav Fomichev, Luigi Rizzo, Björn Töpel,
Pavel Begunkov
Clarify the netmem, memory provider, page_pool, and skb fragment design
principles in header and code comments:
- Memory providers allocate struct net_iov or struct page, cast them to
netmem_ref, and pass them to page_pool; page_pool, drivers, and the
core stack operate on netmem_ref and must not downcast to page or
net_iov outside dedicated netmem helpers.
- While current memory providers supply struct net_iov, they are not
architecturally restricted to net_iov and may supply page-backed
netmems.
- While current net_iov types are CPU-unreadable, net_iov has no
inherent restrictions and may be readable or unreadable.
- New code should generalize existing limitations as much as possible to
match these design principles.
- Per-provider logic belongs in memory_provider_ops, and per-netmem-type
logic belongs in netmem helpers.
- All frags in an skb must either all be struct page-backed or all
belong to the same memory provider instance; mixing them (including
via coalescing) is not allowed.
Cc: Luigi Rizzo <lrizzo@google.com>
Cc: Pavel Begunkov <asml.silence@gmail.com>
Signed-off-by: Mina Almasry <almasrymina@google.com>
Acked-by: Jesper Dangaard Brouer <hawk@kernel.org>
Reviewed-by: Björn Töpel <bjorn@kernel.org>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
---
v3:
- Collect Björn's Reviewed-by and Stanislav's Acked-by tags.
- Clarify skb fragment comments: all frags must be struct page-backed or
belong to the same memory provider instance (Pavel Begunkov).
- Link to v2: https://lore.kernel.org/netdev/20261008023030.1089616-1-almasrymina@google.com/
v2:
- Add Jesper's Acked-by tag.
- Note current implementation status (memory providers currently supply
net_iov, and net_iov is currently unreadable) alongside target design
principles and expectation for new code to generalize existing
limitations (Stanislav Fomichev).
- Link to v1: https://lore.kernel.org/netdev/20261005004958.3603059-1-almasrymina@google.com/
---
include/linux/skbuff.h | 6 +++++
include/net/netmem.h | 30 ++++++++++++++++---------
include/net/page_pool/helpers.h | 18 ++++++++++-----
include/net/page_pool/memory_provider.h | 10 +++++++++
include/net/page_pool/types.h | 6 ++---
net/core/skbuff.c | 4 ++++
6 files changed, 56 insertions(+), 18 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 27ec1e38c8283..af1cbc2799b77 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -358,6 +358,12 @@ struct sk_buff;
*/
#define GSO_BY_FRAGS 0xFFFF
+/* All frags in an skb must either all be struct page-backed or all belong to
+ * the same memory provider instance (e.g. the same devmem binding or io_uring
+ * area). Mixing page and memory-provider frags, or mixing frags from different
+ * memory providers within a single skb (including via coalescing), is not
+ * allowed.
+ */
typedef struct skb_frag {
netmem_ref netmem;
unsigned int len;
diff --git a/include/net/netmem.h b/include/net/netmem.h
index 0cc8572f62caf..c4beb6fbc1306 100644
--- a/include/net/netmem.h
+++ b/include/net/netmem.h
@@ -70,16 +70,18 @@ enum net_iov_type {
NET_IOV_IOURING,
};
-/* A memory descriptor representing abstract networking I/O vectors,
- * generally for non-pages memory that doesn't have its corresponding
- * struct page and needs to be explicitly allocated through slab.
+/* A memory descriptor representing abstract networking I/O vectors.
*
* net_iovs are allocated and used by networking code, and the size of
* the chunk is PAGE_SIZE.
*
- * This memory can be any form of non-struct paged memory. Examples
- * include imported dmabuf memory and imported io_uring memory. See
- * net_iov_type for all the supported types.
+ * Examples include imported dmabuf memory and imported io_uring memory. See
+ * net_iov_type for all the supported types. While current net_iov types are
+ * unreadable by the CPU, net_iov has no inherent restrictions and may be
+ * CPU-readable or unreadable. New code must not assume net_iov implies
+ * unreadable memory (check readability via netmem_address() or
+ * skb_frags_readable() instead) and should, as much as possible, generalize
+ * existing limitations to match the design principles.
*
* @pp_magic: pp field, similar to the one in struct page/struct
* netmem_desc.
@@ -131,8 +133,17 @@ static inline void net_iov_init(struct net_iov *niov,
* network memory.
*
* A netmem_ref can be a struct page* or a struct net_iov* underneath.
+ * Memory providers (or the default page_pool allocator) allocate struct
+ * net_iov or struct page, cast them to netmem_ref, and hand them to
+ * page_pool.
*
- * Use the supplied helpers to obtain the underlying memory pointer and fields.
+ * The page_pool, drivers, and core networking stack should operate on
+ * netmem_ref rather than struct page or struct net_iov. Downcasting
+ * netmem_ref via netmem_to_page() or netmem_to_net_iov() in callers is
+ * not allowed unless a code path strictly requires a specific backing
+ * type (e.g., kmap_local_page()). In such cases, add a netmem helper here
+ * that handles both page and net_iov cases and returns an error if the
+ * underlying type cannot support the operation.
*/
typedef unsigned long __bitwise netmem_ref;
@@ -297,9 +308,8 @@ static inline atomic_long_t *netmem_get_pp_ref_count_ref(netmem_ref netmem)
static inline bool netmem_is_pref_nid(netmem_ref netmem, int pref_nid)
{
- /* NUMA node preference only makes sense if we're allocating
- * system memory. Memory providers (which give us net_iovs)
- * choose for us.
+ /* NUMA node preference only applies to struct page; net_iovs are
+ * managed by their memory provider.
*/
if (netmem_is_net_iov(netmem))
return true;
diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h
index cd021832c3fa3..28635ce9454e8 100644
--- a/include/net/page_pool/helpers.h
+++ b/include/net/page_pool/helpers.h
@@ -8,12 +8,20 @@
/**
* DOC: page_pool allocator
*
- * The page_pool allocator is optimized for recycling page or page fragment used
- * by skb packet and xdp frame.
+ * The page_pool allocator is optimized for recycling network memory
+ * (netmem_ref) or fragments used by skb packets and xdp frames.
*
- * Basic use involves replacing any alloc_pages() calls with page_pool_alloc(),
- * which allocate memory with or without page splitting depending on the
- * requested memory size.
+ * page_pool natively operates on netmem_ref, which abstracts the underlying
+ * memory type (struct page or struct net_iov) supplied by the page allocator
+ * or a memory provider. Drivers and core networking code should use the
+ * netmem-based APIs (e.g. page_pool_alloc_netmem(), page_pool_put_netmem()).
+ * The struct page-based APIs (e.g. page_pool_alloc(), page_pool_alloc_pages(),
+ * page_pool_put_page()) are legacy compatibility wrappers for drivers not yet
+ * converted to netmem.
+ *
+ * Basic use involves replacing any alloc_pages() calls with
+ * page_pool_alloc_netmem() (or legacy page_pool_alloc()), which allocate memory
+ * with or without splitting depending on the requested memory size.
*
* If the driver knows that it always requires full pages or its allocations are
* always smaller than half a page, it can use one of the more specific API
diff --git a/include/net/page_pool/memory_provider.h b/include/net/page_pool/memory_provider.h
index 255ce4cfd9755..137cfc50833ac 100644
--- a/include/net/page_pool/memory_provider.h
+++ b/include/net/page_pool/memory_provider.h
@@ -9,6 +9,16 @@ struct netdev_rx_queue;
struct netlink_ext_ack;
struct sk_buff;
+/* Memory providers allocate underlying memory (struct net_iov or struct page),
+ * cast it to netmem_ref, and supply it to page_pool. While current memory
+ * providers only return struct net_iov, they are not architecturally limited to
+ * net_iov; a provider returning page-backed netmems is allowed. New code must
+ * not assume a memory provider implies net_iov and should, as much as possible,
+ * generalize existing limitations to match the design principles.
+ *
+ * Per-provider custom logic must be delegated to memory_provider_ops rather
+ * than handled directly in page_pool core code.
+ */
struct memory_provider_ops {
netmem_ref (*alloc_netmems)(struct page_pool *pool, gfp_t gfp);
bool (*release_netmem)(struct page_pool *pool, netmem_ref netmem);
diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h
index 03da138722f58..6d543076a0c0b 100644
--- a/include/net/page_pool/types.h
+++ b/include/net/page_pool/types.h
@@ -22,9 +22,9 @@
*/
#define PP_FLAG_SYSTEM_POOL BIT(2) /* Global system page_pool */
-/* Allow unreadable (net_iov backed) netmem in this page_pool. Drivers setting
- * this must be able to support unreadable netmem, where netmem_address() would
- * return NULL. This flag should not be set for header page_pools.
+/* Allow unreadable netmem in this page_pool. Drivers setting this must be able
+ * to support unreadable netmem, where netmem_address() returns NULL. This flag
+ * should not be set for header page_pools.
*
* If the driver sets PP_FLAG_ALLOW_UNREADABLE_NETMEM, it should also set
* page_pool_params.slow.queue_idx.
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index d56f4f0102f75..69d3d27e29d97 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -6210,6 +6210,10 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from,
if (to->pp_recycle != from->pp_recycle)
return false;
+ /* All frags in an skb must either all be struct page-backed or all
+ * belong to the same memory provider instance; do not coalesce skbs
+ * that would mix them.
+ */
if (skb_frags_readable(from) != skb_frags_readable(to))
return false;
--
2.56.0.385.gd3acb90ef8-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH net-next v3 2/2] docs: netmem: document netmem and memory provider design principles
2026-10-10 3:36 [PATCH net-next v3 0/2] net: netmem: document design principles and intended direction Mina Almasry
2026-10-10 3:36 ` [PATCH net-next v3 1/2] net: netmem: document netmem and memory provider design in comments Mina Almasry
@ 2026-10-10 3:36 ` Mina Almasry
2026-10-11 3:51 ` netdev-bot+sashiko
1 sibling, 1 reply; 5+ messages in thread
From: Mina Almasry @ 2026-10-10 3:36 UTC (permalink / raw)
To: netdev, linux-doc, linux-kernel, bpf
Cc: Mina Almasry, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Jonathan Corbet, Shuah Khan,
Randy Dunlap, Jesper Dangaard Brouer, Ilias Apalodimas,
Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Stanislav Fomichev, Luigi Rizzo, Björn Töpel,
Pavel Begunkov
Add a Design Principles section to Documentation/networking/netmem.rst
covering the netmem_ref abstraction, the prohibition on direct
downcasting in callers, decoupling memory providers from net_iov,
decoupling net_iov from unreadability, delegating provider/type logic to
memory_provider_ops and netmem helpers, and the rule against mixing skb
fragment memory sources.
Cc: Luigi Rizzo <lrizzo@google.com>
Signed-off-by: Mina Almasry <almasrymina@google.com>
Reviewed-by: Björn Töpel <bjorn@kernel.org>
Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
---
v3:
- Collect Reviewed-by/Acked-by tags from Björn, Pavel, and Stanislav.
- Clarify item 5: all frags in an skb must either be struct page-backed
or belong to the same memory provider instance (Pavel Begunkov).
- Link to v2: https://lore.kernel.org/netdev/20261008023030.1089616-1-almasrymina@google.com/
v2:
- Document both current implementation status (mp returns net_iov,
net_iov is unreadable) and target design principles in items 2 & 3,
and note that new code should generalize existing limitations as much
as possible (Stanislav Fomichev).
- Link to v1: https://lore.kernel.org/netdev/20261005004958.3603059-1-almasrymina@google.com/
---
Documentation/networking/netmem.rst | 53 +++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/Documentation/networking/netmem.rst b/Documentation/networking/netmem.rst
index 217869d1108dd..6871dcdd700cc 100644
--- a/Documentation/networking/netmem.rst
+++ b/Documentation/networking/netmem.rst
@@ -19,6 +19,59 @@ Benefits of Netmem :
* Simplified Development: Drivers interact with a consistent API,
regardless of the underlying memory implementation.
+Design Principles
+=================
+
+Memory providers (or the default ``page_pool`` allocator) allocate underlying
+memory (``struct net_iov`` or ``struct page``), cast it to ``netmem_ref``, and
+supply it to ``page_pool``. The ``page_pool``, drivers, and networking stack
+operate on ``netmem_ref`` as the abstract type. Existing ``page_pool`` APIs
+that allocate or free ``struct page`` are legacy compatibility wrappers for
+drivers that do not yet support ``netmem_ref``. Code that is not yet
+``netmem``-aware should be converted to ``netmem_ref`` unless it will never
+need to support ``netmem``.
+
+1. **Operate on netmem_ref, do not downcast**: ``page_pool``, drivers, and the
+ core networking stack should deal with ``netmem_ref`` rather than
+ ``struct net_iov`` or ``struct page``. Downcasting ``netmem_ref`` to
+ ``struct net_iov`` or ``struct page`` is not allowed unless a code path
+ strictly cannot function without knowing the underlying memory type (for
+ example, ``kmap_local_page()``). In those cases, to keep call sites simple,
+ add a ``netmem`` helper that performs the operation on behalf of the caller,
+ cleanly handles all ``net_iov`` and ``page`` cases, and returns an error if
+ the ``netmem`` type cannot support the requested operation.
+
+2. **Decouple memory providers from net_iov**: Memory providers are not
+ architecturally limited to ``struct net_iov``; a memory provider that returns
+ ``struct page``-backed ``netmem_ref``\ s to upper layers is allowed. Today,
+ in-tree memory providers only supply ``struct net_iov`` and some existing
+ code still reflects that limitation, but new code must not assume that using
+ a memory provider implies ``net_iov`` memory and should, as much as possible,
+ generalize existing limitations to match the design principles.
+
+3. **Decouple net_iov from unreadability**: ``struct net_iov`` is flexible and
+ has no inherent restrictions; it may represent either CPU-readable or
+ unreadable memory. Today, in-tree ``net_iov`` implementations are unreadable
+ by the CPU (``netmem_address()`` returns ``NULL``) and some existing code
+ still reflects that limitation, but new code must not assume ``net_iov``
+ implies unreadable memory (check readability via ``netmem_address()`` or
+ ``skb_frags_readable()`` instead) and should, as much as possible, generalize
+ existing limitations to match the design principles.
+
+4. **Delegate complexity to the lowest layer**: Each layer must respect its
+ abstraction boundary. ``page_pool`` must not implement per-memory-provider
+ custom logic in its main code; instead, it delegates provider-specific
+ handling to ``struct memory_provider_ops``. Similarly, core networking code
+ should avoid per-``netmem``-type branching and instead delegate operations
+ to ``netmem`` helpers that handle the underlying memory type.
+
+5. **Do not mix skb fragment memory sources**: All ``frags[]`` in an
+ ``sk_buff`` must either be ``struct page``-backed or belong to the same
+ memory provider instance (for example, the same ``devmem`` binding or
+ ``io_uring`` area). Mixing ``struct page`` and memory-provider fragments,
+ or mixing fragments from different memory providers within a single
+ ``sk_buff``, is not allowed (including when coalescing ``sk_buff``\ s).
+
Driver RX Requirements
======================
--
2.56.0.385.gd3acb90ef8-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next v3 1/2] net: netmem: document netmem and memory provider design in comments
2026-10-10 3:36 ` [PATCH net-next v3 1/2] net: netmem: document netmem and memory provider design in comments Mina Almasry
@ 2026-10-11 3:51 ` netdev-bot+sashiko
0 siblings, 0 replies; 5+ messages in thread
From: netdev-bot+sashiko @ 2026-10-11 3:51 UTC (permalink / raw)
To: almasrymina
Cc: netdev, linux-doc, linux-kernel, bpf, davem, edumazet, kuba,
pabeni, horms, corbet, skhan, rdunlap, hawk, ilias.apalodimas,
ast, daniel, john.fastabend, sdf, lrizzo, bjorn, asml.silence
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 adds a comment in skb_try_coalesce()
(net/core/skbuff.c, lines 6213-6216).
- [Low] The new skb_frag_t comment (include/linux/skbuff.h:361-363) gives
'the same io_uring area' as an example of 'the same memory provider…
--
net: netmem: document netmem and memory provider design in comments
This commit adds comments describing the design principles for netmem,
memory providers, page_pool and skb frags. It documents that all frags in
an skb must be page-backed or come from the same memory provider instance.
It also adds a comment to that effect in skb_try_coalesce().
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 27ec1e38c8283..af1cbc2799b77 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -358,6 +358,12 @@ struct sk_buff;
> */
> #define GSO_BY_FRAGS 0xFFFF
>
> +/* All frags in an skb must either all be struct page-backed or all belong to
> + * the same memory provider instance (e.g. the same devmem binding or io_uring
> + * area). Mixing page and memory-provider frags, or mixing frags from different
[Severity: Low]
Is "io_uring area" the right example here? For io_uring zcrx, the provider
instance is the ifq, not the area:
io_uring/zcrx.c:io_pp_to_ifq() {
return pp->mp_priv;
}
io_zcrx_recv_frag() also checks identity per ifq:
if (!pp || pp->mp_ops != &io_uring_pp_zc_ops || io_pp_to_ifq(pp) != ifq)
return -EFAULT;
One ifq can own several areas. io_zcrx_append_area() grows ifq->areas[] up
to ZCRX_MAX_AREAS, and userspace can add areas to an existing ifq with
ZCRX_CTRL_ADD_AREA. Frags from two areas of the same ifq share one provider
instance and one page_pool, and nothing stops them from sharing an skb.
Read literally, the example says those frags must not share an skb. Would
"the same io_uring zcrx ifq" be more accurate?
The later commit "docs: netmem: document netmem and memory provider design
principles" repeats this example in Documentation/networking/netmem.rst.
> + * memory providers within a single skb (including via coalescing), is not
> + * allowed.
> + */
> typedef struct skb_frag {
> netmem_ref netmem;
> unsigned int len;
[ ... ]
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index d56f4f0102f75..69d3d27e29d97 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -6210,6 +6210,10 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from,
> if (to->pp_recycle != from->pp_recycle)
> return false;
>
> + /* All frags in an skb must either all be struct page-backed or all
> + * belong to the same memory provider instance; do not coalesce skbs
> + * that would mix them.
> + */
> if (skb_frags_readable(from) != skb_frags_readable(to))
> return false;
[Severity: Medium]
Does this check enforce the rule stated in the comment above it?
skb_frags_readable() only looks at a single bit:
include/linux/skbuff.h:skb_frags_readable() {
return !skb->unreadable;
}
Nothing in skb_try_coalesce() compares pp->mp_priv, the devmem binding or
the zcrx ifq of to and from.
Two unreadable skbs from different provider instances would pass this
check, and their frag arrays would be memcpy'd together. For example, two
devmem bindings, or a devmem binding and a zcrx ifq, could reach one TCP
socket through different RX queues via
tcp_try_coalesce()->skb_try_coalesce().
Also, when an unreadable from skb has a non-zero, stealable linear head,
the skb_headlen(from) != 0 branch adds the head page as a page frag next to
the net_iov frags:
page = virt_to_head_page(from->head);
...
skb_fill_page_desc(to, to_shinfo->nr_frags,
page, offset, skb_headlen(from));
skb_gro_receive() in net/core/gro.c is another coalescing path. It checks
only pp_recycle and skb_zcopy, with no readability or provider check.
Existing consumers already handle mixed skbs one frag at a time:
io_zcrx_recv_frag() rejects frags from another ifq, and
tcp_recvmsg_dmabuf() re-checks each frag with skb_frag_net_iov().
The same patch also says providers may supply page-backed netmems and that
net_iovs may be readable. Under those principles, readability alone can't
tell page frags from provider frags.
The new skb_frag_t comment and the commit message state this as a hard
invariant ("is not allowed", "including via coalescing"). Could the wording
describe what the code currently guarantees instead? Otherwise the per-frag
checks above might later be removed because of it.
Or should skb_try_coalesce() and skb_gro_receive() actually compare
provider identity?
The later commit "docs: netmem: document netmem and memory provider design
principles" repeats the same claim in Documentation/networking/netmem.rst.
The comment here is not corrected anywhere in the series.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261010033630.1171692-1-almasrymina%40google.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next v3 2/2] docs: netmem: document netmem and memory provider design principles
2026-10-10 3:36 ` [PATCH net-next v3 2/2] docs: netmem: document netmem and memory provider design principles Mina Almasry
@ 2026-10-11 3:51 ` netdev-bot+sashiko
0 siblings, 0 replies; 5+ messages in thread
From: netdev-bot+sashiko @ 2026-10-11 3:51 UTC (permalink / raw)
To: almasrymina
Cc: netdev, linux-doc, linux-kernel, bpf, davem, edumazet, kuba,
pabeni, horms, corbet, skhan, rdunlap, hawk, ilias.apalodimas,
ast, daniel, john.fastabend, sdf, lrizzo, bjorn, asml.silence
Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · Low: 2
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] Item 5 of the new Design Principles section in
Documentation/networking/netmem.rst says every frag in an skb must be…
- [Low] Items 2 and 5 of the new Design Principles section contradict
each other once a page-backed memory provider exists.
- [Low] Item 3 of netmem.rst, and the matching net_iov comment in
include/net/netmem.h added by the commit "net: netmem: document netmem…
--
docs: netmem: document netmem and memory provider design principles
This commit adds a Design Principles section to
Documentation/networking/netmem.rst. The section covers the netmem_ref
abstraction, the rule against downcasting, decoupling memory providers
from net_iov and net_iov from unreadability, and delegating type-specific
logic to the lower layers. It also adds a rule against mixing skb fragment
memory sources.
> diff --git a/Documentation/networking/netmem.rst b/Documentation/networking/netmem.rst
> index 217869d1108dd..6871dcdd700cc 100644
> --- a/Documentation/networking/netmem.rst
> +++ b/Documentation/networking/netmem.rst
> @@ -19,6 +19,59 @@ Benefits of Netmem :
[ ... ]
> +3. **Decouple net_iov from unreadability**: ``struct net_iov`` is flexible and
> + has no inherent restrictions; it may represent either CPU-readable or
> + unreadable memory. Today, in-tree ``net_iov`` implementations are unreadable
> + by the CPU (``netmem_address()`` returns ``NULL``) and some existing code
> + still reflects that limitation, but new code must not assume ``net_iov``
> + implies unreadable memory (check readability via ``netmem_address()`` or
> + ``skb_frags_readable()`` instead) and should, as much as possible, generalize
> + existing limitations to match the design principles.
[Severity: Low]
Can netmem_address() reliably test readability for page-backed netmem?
For pages, it ends up calling page_address():
include/net/netmem.h:netmem_address() {
if (netmem_is_net_iov(netmem))
return NULL;
return __netmem_address(netmem);
}
With CONFIG_HIGHMEM, page_address() in mm/highmem.c returns NULL for a
highmem page that has no permanent mapping. The CPU can still read that
page through kmap_local_page(), which item 1 uses as its example.
Highmem page frags are normal in skbs on 32-bit kernels. They come from
sendfile/splice or from MSG_ZEROCOPY user pages.
Could new code that follows this advice and treats a NULL return from
netmem_address() as "unreadable" wrongly reject or skip valid highmem page
frags?
The same advice appears in the struct net_iov comment in
include/net/netmem.h. That comment was added by "net: netmem: document
netmem and memory provider design in comments".
Would it be more accurate to say that a NULL netmem_address() means "no
permanent kernel mapping"? Another option is to point only to
skb_frags_readable() or to a dedicated netmem readability helper.
[ ... ]
> +5. **Do not mix skb fragment memory sources**: All ``frags[]`` in an
> + ``sk_buff`` must either be ``struct page``-backed or belong to the same
> + memory provider instance (for example, the same ``devmem`` binding or
> + ``io_uring`` area). Mixing ``struct page`` and memory-provider fragments,
> + or mixing fragments from different memory providers within a single
> + ``sk_buff``, is not allowed (including when coalescing ``sk_buff``\ s).
[Severity: Medium]
Is this invariant enforced anywhere today? Items 2 and 3 note that some
existing code does not follow them yet, but this item has no such note.
The companion commit "net: netmem: document netmem and memory provider
design in comments" adds this comment in skb_try_coalesce():
net/core/skbuff.c:skb_try_coalesce() {
...
/* All frags in an skb must either all be struct page-backed or all
* belong to the same memory provider instance; do not coalesce skbs
* that would mix them.
*/
if (skb_frags_readable(from) != skb_frags_readable(to))
return false;
...
}
skb_frags_readable() only returns !skb->unreadable. Two unreadable skbs
can carry net_iovs from different devmem bindings, or from a devmem
binding and an io_uring zcrx area. Both would pass this check, and their
frags would then be merged.
Other places look similar:
- tcp_skb_can_collapse() in include/net/tcp.h compares only readability.
- skb_gro_receive() in net/core/gro.c checks pp_recycle and skb_zcopy(),
but not readability or provider identity.
On TX, tcp_sendmsg_locked() looks up the binding again on every sendmsg
call with net_devmem_get_binding(sk, sockc.dmabuf_id). Before it appends
to the tail skb, it only checks:
if (copy <= 0 || !tcp_skb_can_collapse_to(skb) ||
unlikely(skb_frags_readable(skb) != !binding)) {
msg_zerocopy_realloc() then extends the tail skb's uarg. Because of that,
the -EEXIST check in skb_zerocopy_iter_stream() never triggers.
bind-tx does not require privileges. Could a user bind two dmabufs to one
netdev, then send two back-to-back MSG_ZEROCOPY sendmsg calls with
different dmabuf_ids? That would leave net_iovs from two bindings in one
skb.
validate_xmit_unreadable_skb() in net/core/dev.c already relies on this
invariant, because it only checks frags[0]:
niov = netmem_to_net_iov(skb_frag_netmem(&shinfo->frags[0]));
if (net_is_devmem_iov(niov) &&
READ_ONCE(net_devmem_iov_binding(niov)->dev) != dev)
goto out_free;
Should these places get a provider-identity check? Otherwise, could this
item and the skb_try_coalesce() comment say that only readability is
enforced today, the way items 2 and 3 describe their own limitations?
[Severity: Low]
Do items 2 and 5 conflict once a page-backed memory provider exists?
Item 2 says "a memory provider that returns struct page-backed
netmem_refs to upper layers is allowed". A frag from such a provider
fits both categories in the first sentence here: "struct page-backed"
and "same memory provider instance". Read that way, it could share an
skb with ordinary page frags, or with page frags from a different
page-backed provider. skb_try_coalesce() would allow that today.
The second sentence, however, forbids "Mixing struct page and
memory-provider fragments, or mixing fragments from different memory
providers".
The new skb_frag_t comment in include/linux/skbuff.h uses the same
wording.
Could the two categories be stated so they don't overlap? For example,
"non-provider struct pages" versus "frags from a single provider
instance, whatever their backing".
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261010033630.1171692-1-almasrymina%40google.com
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-11 3:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-10 3:36 [PATCH net-next v3 0/2] net: netmem: document design principles and intended direction Mina Almasry
2026-10-10 3:36 ` [PATCH net-next v3 1/2] net: netmem: document netmem and memory provider design in comments Mina Almasry
2026-10-11 3:51 ` netdev-bot+sashiko
2026-10-10 3:36 ` [PATCH net-next v3 2/2] docs: netmem: document netmem and memory provider design principles Mina Almasry
2026-10-11 3:51 ` netdev-bot+sashiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox