Magnus Karlsson [off-list ref] writes:
On Tue, 10 Nov 2026 at 12:26, Björn Töpel [off-list ref] wrote:
quoted
diff --git a/net/xdp/xsk.h b/net/xdp/xsk.h
index 7c811b5cce76..4dab7b9a1487 100644
--- a/net/xdp/xsk.h
+++ b/net/xdp/xsk.h
@@ -34,6 +34,12 @@ static inline struct xdp_sock *xdp_sk(struct sock *sk)
void xsk_map_try_sock_delete(struct xsk_map *map, struct xdp_sock *xs,
struct xdp_sock __rcu **map_entry);
+struct xsk_pp;
How about a header file instead of a forward declaration?
Thanks for taking a look, Magnus!
Hmm, my take was this this was ont by itself sufficient for a header.
Let me think about it, and I might add net/xdp/xsk_pp.h.
quoted
+extern const struct memory_provider_ops xsk_pp_ops;
+
+struct xsk_pp *xsk_pp_create(struct xsk_buff_pool *pool);
+void xsk_pp_put(struct xsk_pp *pp);
void xsk_clear_pool_at_qid(struct net_device *dev, u16 queue_id);
int xsk_reg_pool_at_qid(struct net_device *dev, struct xsk_buff_pool *pool,
u16 queue_id);diff --git a/net/xdp/xsk_pp.c b/net/xdp/xsk_pp.c
new file mode 100644
index 000000000000..fdd55265a6cd
--- /dev/null
+++ b/net/xdp/xsk_pp.c
@@ -0,0 +1,246 @@
+// SPDX-License-Identifier: GPL-2.0
+/* AF_XDP UMEM as a page pool memory provider
+ *
+ * Each aligned UMEM chunk is a readable net_iov with one owner at a time:
+ *
+ * userspace --FILL--> page pool --release--> reuse --alloc--> page pool
+ * ^ |
+ * +-----RX ring-------+
+ */
+
+#include <net/page_pool/helpers.h>
+#include <net/page_pool/memory_provider.h>
+#include <net/xdp_sock_drv.h>
+
+#include "xsk.h"
+#include "xsk_queue.h"
+
+enum xsk_pp_chunk {
+ XSK_PP_USER, /* owned by userspace */
+ XSK_PP_POOL, /* owned by a page pool */
+ XSK_PP_REUSE, /* on the reuse stack */
+};
+
+struct xsk_pp {
+ struct net_iov_area area;
+ struct xsk_buff_pool *pool;
+ /* Protects fq, state and reuse */
+ spinlock_t lock;
+ struct xsk_queue *fq;
+ refcount_t refs;
+ u32 nr_pools;
+ bool dma_need_sync;
+ u8 *state;
+ u32 reuse_cnt;
+ u32 reuse[];
Is the reason that you have a reuse stack here and not use the one in
xsk_buff_pool that it only exists in unaligned mode, which you are not
supporting? Could you still use the "pre-calculated" heads in aligned
mode and link them up? There is already a xskb_list and free_list in
there that is used for that purpose. Just an idea that could cut down
the state and maybe the amount of code.
Not quite. reuse is the counterpart of free_list, which aligned mode has
too. But free_list links xdp_buff_xsk, and this mode has none: the page
pool hands out net_iovs, and the driver has its own xdp_buff. Patch 5
stops allocating the heads here: 128 bytes per chunk, against 69 for
net_iov, reuse and state. devmem and zcrx use the same kind of index
stack.
free_list is also NAPI only, while the page pool releases chunks from
process context too (below). And state is needed either way; it is what
drops a FILL entry for a chunk the kernel still owns.
quoted
+};
+
+static void xsk_pp_free(struct xsk_pp *pp)
+{
+ kvfree(pp->state);
+ kvfree(pp->area.niovs);
+ kvfree(pp);
+}
+
+void xsk_pp_put(struct xsk_pp *pp)
+{
+ if (refcount_dec_and_test(&pp->refs))
+ xsk_pp_free(pp);
+}
+
+/* @pool must be DMA mapped and have aligned chunks. The provider must be
+ * the only consumer of its FILL ring.
+ */
+struct xsk_pp *xsk_pp_create(struct xsk_buff_pool *pool)
+{
+ struct net_iov *niov;
+ struct xsk_pp *pp;
+ dma_addr_t dma;
+ u32 i;
+
+ pp = kvzalloc_flex(*pp, reuse, pool->heads_cnt, GFP_KERNEL_ACCOUNT);
+ if (!pp)
+ return ERR_PTR(-ENOMEM);
+
+ pp->area.niovs = kvzalloc_objs(*pp->area.niovs, pool->heads_cnt,
+ GFP_KERNEL_ACCOUNT);
+ pp->state = kvzalloc(pool->heads_cnt, GFP_KERNEL_ACCOUNT);
+ if (!pp->area.niovs || !pp->state) {
+ xsk_pp_free(pp);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ pp->area.num_niovs = pool->heads_cnt;
+ pp->area.niov_shift = pool->chunk_shift;
+ pp->area.vaddr = pool->addrs;
+ pp->pool = pool;
+ pp->fq = pool->fq;
+ pp->dma_need_sync = dma_dev_need_sync(pool->dev);
Are you copying these for performance reasons, i.e. no extra pointer
dereference in the fast path? In the same manner as the pool copies
some information from the umem?
Only dma_need_sync is a cache -- I'll drop that!
fq stays. It's the provider's handle on FILL: unbind clears it under the
lock (patch 5), so the provider stops reading FILL, while pool->fq stays
with the socket for mmap, need_wakeup and copy mode until the ring is
freed. The area fields are the generic net_iov_area that
netmem_address() needs.
Does that make sense?
quoted
+ spin_lock_init(&pp->lock);
+ refcount_set(&pp->refs, 1);
+
+ for (i = 0; i < pp->area.num_niovs; i++) {
+ niov = &pp->area.niovs[i];
+ dma = xp_raw_get_dma(pool, (u64)i << pool->chunk_shift);
+ net_iov_init(niov, &pp->area, NET_IOV_XSK);
+ if (net_mp_niov_set_dma_addr(niov, dma)) {
+ xsk_pp_free(pp);
+ return ERR_PTR(-ERANGE);
+ }
+ }
+
+ return pp;
+}
+
+static int xsk_pp_init(struct page_pool *pool)
+{
+ struct xsk_pp *pp = pool->mp_priv;
+
+ if (pool->p.order || pool->p.dma_dir != DMA_BIDIRECTIONAL ||
+ pool->p.dev != pp->pool->dev)
+ return -EINVAL;
+ if (pool->p.max_len > pp->pool->chunk_size ||
+ pool->p.offset > pp->pool->chunk_size - pool->p.max_len)
+ return -EINVAL;
+
+ refcount_inc(&pp->refs);
+ spin_lock_bh(&pp->lock);
+ pp->nr_pools++;
What is the difference between this counter and the refcount?
refs is the lifetime of struct xsk_pp. The buffer pool holds one
reference, and each page pool that takes the provider holds one. A page
pool can outlive the socket, as page_pool_destroy() defers while buffers
are in flight. The struct is freed with the last reference.
nr_pools only tells whether a page pool still exists, when a failed bind
(patch 5) hands the leftover chunks to copy mode. But the page pools of
a failed bind never ran, and are gone by then, so a WARN on refs != 1
does the same job... I'll drop nr_pools. Thanks!
quoted
+ spin_unlock_bh(&pp->lock);
+ return 0;
+}
+
+static void xsk_pp_destroy(struct page_pool *pool)
+{
+ struct xsk_pp *pp = pool->mp_priv;
+
+ spin_lock_bh(&pp->lock);
+ pp->nr_pools--;
+ spin_unlock_bh(&pp->lock);
+ xsk_pp_put(pp);
+}
+
+static bool xsk_pp_claim(struct xsk_pp *pp, u32 idx, enum xsk_pp_chunk from)
+{
+ lockdep_assert_held(&pp->lock);
+
+ if (pp->state[idx] != from)
+ return false;
+
+ pp->state[idx] = XSK_PP_POOL;
+ return true;
+}
+
+static u32 xsk_pp_alloc_reuse(struct xsk_pp *pp, netmem_ref *netmems, u32 max)
+{
+ u32 idx, n = 0;
+
+ while (n < max && pp->reuse_cnt) {
+ idx = pp->reuse[--pp->reuse_cnt];
+ if (WARN_ON_ONCE(!xsk_pp_claim(pp, idx, XSK_PP_REUSE)))
+ continue;
+ netmems[n++] = net_iov_to_netmem(&pp->area.niovs[idx]);
+ }
+
+ return n;
+}
+
+static u32 xsk_pp_alloc_fill(struct xsk_pp *pp, netmem_ref *netmems, u32 max)
+{
+ struct xsk_buff_pool *pool = pp->pool;
+ struct xsk_queue *fq = pp->fq;
+ u32 i, entries, cons, idx, n = 0;
+ u64 addr;
+
+ if (!fq || !max)
+ return 0;
+
+ entries = xskq_cons_nb_entries(fq, max);
+ if (!entries) {
+ fq->queue_empty_descs++;
+ return 0;
+ }
+
+ cons = fq->cached_cons;
+ for (i = 0; i < entries; i++) {
+ __xskq_cons_read_addr_unchecked(fq, cons++, &addr);
+ addr &= pool->chunk_mask;
+ idx = addr >> pp->area.niov_shift;
+ if (unlikely(addr >= pool->addrs_cnt ||
+ !xsk_pp_claim(pp, idx, XSK_PP_USER))) {
+ fq->invalid_descs++;
+ continue;
+ }
+ netmems[n++] = net_iov_to_netmem(&pp->area.niovs[idx]);
+ }
+
+ xskq_cons_release_n(fq, entries);
+ __xskq_cons_release(fq);
+ return n;
+}
+
+static netmem_ref xsk_pp_alloc_netmems(struct page_pool *pool, gfp_t gfp)
+{
+ netmem_ref *netmems = pool->alloc.cache;
+ struct xsk_pp *pp = pool->mp_priv;
+ struct net_iov *niov;
+ dma_addr_t dma;
+ u32 i, n;
+
+ if (WARN_ON_ONCE(pool->alloc.count))
What has the driver writer or system done if this ever happens?
A correct driver can't get there; the page pool calls ->alloc_netmems()
only with an empty cache. It fires on a page_pool core bug, or when a
driver recycles with allow_direct outside the pool's NAPI, and races the
cache. The provider fills the cache from index 0, so failing beats
overwriting live entries. devmem and zcrx have the same check; I'll add
zcrx's comment.
quoted
+ return 0;
+
+ spin_lock_bh(&pp->lock);
For my understanding, what entities can call this function at the same
time? With the xsk_buff_pool we were protected by NAPI.
Yeah... this is a bit gnarley.
The provider outlives a page pool: a queue restart starts the new queue
before it frees the old pool, so the old pool releases chunks while the
new NAPI allocates. Teardown can be deferred to a work item, and the
initial fill, bind and unbind run in process context. A driver that
fills its new rings in ndo_queue_mem_alloc() even has two pools on one
FILL ring.
However, in steady state only in NAPI, once per refill, so the lock is
uncontended. I haven't been able to see it in perf...
I'll list that in a comment.
Björn