* [PATCH v4] net/af_xdp: fix shared UMEM refcount corruption
@ 2026-08-20 23:05 sandeep.penigalapati
2026-08-21 0:44 ` [PATCH v5] " sandeep.penigalapati
0 siblings, 1 reply; 5+ messages in thread
From: sandeep.penigalapati @ 2026-08-20 23:05 UTC (permalink / raw)
To: dev
Cc: Ciara Loftus, Maryam Tahhan, Stephen Hemminger, stable,
Sandeep Penigalapati
From: Sandeep Penigalapati <sandeep.penigalapati@intel.com>
Shared UMEM is meant to be shared by a limited number of sockets,
governed by the mempool size (max_xsks). When the UMEM was already at
capacity (refcnt >= max_xsks), xdp_umem_configure() returned the UMEM
without incrementing its refcount, so the extra socket used it
unaccounted for.
This missing reference has two consequences. During queue setup the
fill-queue reservation is chosen from the refcount, so the sharing
socket reserves into its own uninitialised fill queue and crashes. At
close, the under-counted refcount reaches zero while the UMEM is still
in use, freeing it early and causing a use-after-free.
Reject sharing once the UMEM is at capacity by returning NULL. The
error is propagated from xsk_configure(), so Rx queue setup fails
cleanly with -ENOMEM. This applies the per-mempool socket limit that
shared UMEM was always intended to respect.
Harden the failure path this makes reachable:
- clear rxq->umem and its paired txq->umem when xsk_configure() fails,
and skip queues whose UMEM is not yet set in get_shared_umem(), so a
later scan over the same mempool cannot dereference a NULL or
dangling UMEM;
- free the fill-queue mbufs that were allocated but not yet handed to
the fill queue when a sharing socket fails to bind, so it no longer
leaks a burst of mbufs back out of the shared mempool;
- propagate the map-insert failures in xsk_configure() instead of
returning success, so a failed xsks_map update no longer leaves the
caller using a deleted socket;
- continue past, rather than stop at, a failed queue in eth_dev_close()
so later successful queues and their UMEM references are still freed;
- clamp max_xsks to UINT8_MAX so the cap stays within the uint8_t
refcount.
Also correct the UMEM refcount memory ordering: relaxed on the shared
increment, release on the decrements, with an acquire fence before
xdp_umem_destroy() so the final user's writes are visible to the thread
that frees the UMEM.
Document the shared mempool sizing requirement (4096 mbufs per socket).
Note: on stable branches this is a behaviour change. Shared-UMEM setups
that previously appeared to start, until the fill-queue crash or the
use-after-free at close, now fail cleanly at Rx queue setup with
-ENOMEM.
Fixes: 74b46340e2d4 ("net/af_xdp: support shared UMEM")
Cc: stable@dpdk.org
Signed-off-by: Sandeep Penigalapati <sandeep.penigalapati@intel.com>
---
v4:
- Free fill-queue mbufs on the failure path.
- Propagate map-insert failures and the -ENOMEM from xsk_configure() to
the caller instead of a blanket -EINVAL / silent success.
- eth_dev_close(): continue past failed queues instead of break.
- Correct refcount memory ordering (relaxed increment, release
decrement, acquire fence before destroy).
- Clamp max_xsks to UINT8_MAX so the cap fits the uint8_t refcount.
- Clear txq->umem on the early return; clearer log when the mempool is
too small (max_xsks == 0).
v3:
- Move the NULL umem check after ctx_exists() so a failed queue is still
checked for a duplicate context, as before.
- Shorten the "at capacity" log to one line and drop the count.
- Also clear txq->umem, not just rxq->umem, when setup fails.
- Clarify "AF_XDP socket" in the doc.
v2:
- Guard get_shared_umem() against a NULL umem and clear rxq->umem on the
xsk_configure() error path (review).
- doc: "Rx queue setup fails", one sentence per line.
- Drop unrelated reflow of the refcount increment.
doc/guides/nics/af_xdp.rst | 7 +++
drivers/net/af_xdp/rte_eth_af_xdp.c | 66 +++++++++++++++++++++++------
2 files changed, 61 insertions(+), 12 deletions(-)
diff --git a/doc/guides/nics/af_xdp.rst b/doc/guides/nics/af_xdp.rst
index c455b4c066..cf4eeb63d0 100644
--- a/doc/guides/nics/af_xdp.rst
+++ b/doc/guides/nics/af_xdp.rst
@@ -99,6 +99,13 @@ configured like so:
--vdev net_af_xdp0,iface=ens786f1,shared_umem=1 \
--vdev net_af_xdp1,iface=ens786f2,shared_umem=1
+The shared mempool must be large enough for every AF_XDP socket sharing
+the UMEM.
+Each socket requires 4096 mbufs, so a UMEM shared by ``N`` sockets needs at
+least ``4096 * N`` mbufs.
+Rx queue setup fails if the mempool is too small to add another socket to the
+UMEM.
+
xdp_prog
~~~~~~~~
diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
index 2cdb533276..8bd44b3766 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -1062,13 +1062,17 @@ eth_dev_close(struct rte_eth_dev *dev)
for (i = 0; i < internals->queue_cnt; i++) {
rxq = &internals->rx_queues[i];
+ /* Skip queues whose setup failed (umem left NULL). */
if (rxq->umem == NULL)
- break;
+ continue;
xsk_socket__delete(rxq->xsk);
if (rte_atomic_fetch_sub_explicit(&rxq->umem->refcnt, 1,
- rte_memory_order_acquire) - 1 == 0)
+ rte_memory_order_release) - 1 == 0) {
+ /* Acquire so the destroy sees all prior users' writes. */
+ rte_atomic_thread_fence(rte_memory_order_acquire);
xdp_umem_destroy(rxq->umem);
+ }
}
/* Free Tx and Rx queue arrays */
rte_free(internals->tx_queues);
@@ -1154,6 +1158,9 @@ get_shared_umem(struct pkt_rx_queue *rxq, const char *ifname,
ret = -1;
goto out;
}
+ /* A failed setup leaves mb_pool set with no umem. */
+ if (list_rxq->umem == NULL)
+ continue;
if (rte_atomic_load_explicit(&internals->rx_queues[i].umem->refcnt,
rte_memory_order_acquire)) {
*umem = internals->rx_queues[i].umem;
@@ -1188,12 +1195,24 @@ xsk_umem_info *xdp_umem_configure(struct pmd_internals *internals,
if (get_shared_umem(rxq, internals->if_name, &umem) < 0)
return NULL;
- if (umem != NULL &&
- rte_atomic_load_explicit(&umem->refcnt, rte_memory_order_acquire) <
- umem->max_xsks) {
+ if (umem != NULL) {
+ /* Reject sharing once the UMEM is at capacity. */
+ if (rte_atomic_load_explicit(&umem->refcnt,
+ rte_memory_order_acquire) >= umem->max_xsks) {
+ if (umem->max_xsks == 0)
+ AF_XDP_LOG_LINE(ERR, "%s,qid%i: mempool %s too small to share UMEM",
+ internals->if_name, rxq->xsk_queue_idx,
+ umem->mb_pool->name);
+ else
+ AF_XDP_LOG_LINE(ERR, "%s,qid%i: UMEM %s already at max %u sockets",
+ internals->if_name, rxq->xsk_queue_idx,
+ umem->mb_pool->name, umem->max_xsks);
+ return NULL;
+ }
+
AF_XDP_LOG_LINE(INFO, "%s,qid%i sharing UMEM",
internals->if_name, rxq->xsk_queue_idx);
- rte_atomic_fetch_add_explicit(&umem->refcnt, 1, rte_memory_order_acquire);
+ rte_atomic_fetch_add_explicit(&umem->refcnt, 1, rte_memory_order_relaxed);
}
}
@@ -1239,8 +1258,10 @@ xsk_umem_info *xdp_umem_configure(struct pmd_internals *internals,
umem->buffer = aligned_addr;
if (internals->shared_umem) {
- umem->max_xsks = mb_pool->populated_size /
- ETH_AF_XDP_NUM_BUFFERS;
+ /* refcnt is uint8_t, so the cap cannot exceed UINT8_MAX. */
+ umem->max_xsks = RTE_MIN(mb_pool->populated_size /
+ ETH_AF_XDP_NUM_BUFFERS,
+ (uint32_t)UINT8_MAX);
AF_XDP_LOG_LINE(INFO, "Max xsks for UMEM %s: %u",
mb_pool->name, umem->max_xsks);
}
@@ -1684,10 +1705,13 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
int reserve_size = ETH_AF_XDP_DFLT_NUM_DESCS;
struct rte_mbuf *fq_bufs[reserve_size];
bool reserve_before;
+ bool free_fq_bufs = false;
rxq->umem = xdp_umem_configure(internals, rxq);
- if (rxq->umem == NULL)
+ if (rxq->umem == NULL) {
+ txq->umem = NULL;
return -ENOMEM;
+ }
txq->umem = rxq->umem;
reserve_before = rte_atomic_load_explicit(&rxq->umem->refcnt,
rte_memory_order_acquire) <= 1;
@@ -1698,11 +1722,14 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
AF_XDP_LOG_LINE(DEBUG, "Failed to get enough buffers for fq.");
goto out_umem;
}
+ free_fq_bufs = true;
#endif
/* reserve fill queue of queues not (yet) sharing UMEM */
if (reserve_before) {
ret = reserve_fill_queue(rxq->umem, reserve_size, fq_bufs, &rxq->fq);
+ /* reserve_fill_queue() consumes fq_bufs on success and frees them on failure. */
+ free_fq_bufs = false;
if (ret) {
AF_XDP_LOG_LINE(ERR, "Failed to reserve fill queue.");
goto out_umem;
@@ -1759,6 +1786,7 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
if (!reserve_before) {
/* reserve fill queue of queues sharing UMEM */
ret = reserve_fill_queue(rxq->umem, reserve_size, fq_bufs, &rxq->fq);
+ free_fq_bufs = false;
if (ret) {
AF_XDP_LOG_LINE(ERR, "Failed to reserve fill queue.");
goto out_xsk;
@@ -1774,6 +1802,7 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
&rxq->xsk_queue_idx, &fd, 0);
if (err) {
AF_XDP_LOG_LINE(ERR, "Failed to insert xsk in map.");
+ ret = -EINVAL;
goto out_xsk;
}
}
@@ -1786,6 +1815,7 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
map_fd = uds_get_xskmap_fd(internals->if_name, internals->dp_path);
if (map_fd < 0) {
AF_XDP_LOG_LINE(ERR, "Failed to receive xskmap fd from AF_XDP Device Plugin");
+ ret = -EINVAL;
goto out_xsk;
}
} else {
@@ -1793,6 +1823,7 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
err = get_pinned_map(internals->dp_path, &map_fd);
if (err < 0 || map_fd < 0) {
AF_XDP_LOG_LINE(ERR, "Failed to retrieve pinned map fd");
+ ret = -EINVAL;
goto out_xsk;
}
}
@@ -1800,6 +1831,7 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
err = update_xskmap(rxq->xsk, map_fd, rxq->xsk_queue_idx);
if (err) {
AF_XDP_LOG_LINE(ERR, "Failed to insert xsk in map.");
+ ret = -EINVAL;
goto out_xsk;
}
@@ -1816,8 +1848,18 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
out_xsk:
xsk_socket__delete(rxq->xsk);
out_umem:
- if (rte_atomic_fetch_sub_explicit(&rxq->umem->refcnt, 1, rte_memory_order_acquire) - 1 == 0)
+ /* Free fq_bufs that were allocated but never handed to the fill queue. */
+ if (free_fq_bufs)
+ rte_pktmbuf_free_bulk(fq_bufs, reserve_size);
+ if (rte_atomic_fetch_sub_explicit(&rxq->umem->refcnt, 1,
+ rte_memory_order_release) - 1 == 0) {
+ /* Acquire so the destroy sees all prior users' writes. */
+ rte_atomic_thread_fence(rte_memory_order_acquire);
xdp_umem_destroy(rxq->umem);
+ }
+ /* Drop dangling pointers so a later shared-UMEM scan skips this queue. */
+ rxq->umem = NULL;
+ txq->umem = NULL;
return ret;
}
@@ -1858,9 +1900,9 @@ eth_rx_queue_setup(struct rte_eth_dev *dev,
rxq->mb_pool = mb_pool;
- if (xsk_configure(internals, rxq, nb_rx_desc)) {
+ ret = xsk_configure(internals, rxq, nb_rx_desc);
+ if (ret) {
AF_XDP_LOG_LINE(ERR, "Failed to configure xdp socket");
- ret = -EINVAL;
goto err;
}
--
2.27.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v5] net/af_xdp: fix shared UMEM refcount corruption
2026-08-20 23:05 [PATCH v4] net/af_xdp: fix shared UMEM refcount corruption sandeep.penigalapati
@ 2026-08-21 0:44 ` sandeep.penigalapati
2026-08-21 2:16 ` [PATCH v6] " sandeep.penigalapati
0 siblings, 1 reply; 5+ messages in thread
From: sandeep.penigalapati @ 2026-08-21 0:44 UTC (permalink / raw)
To: dev; +Cc: stable, stephen, ciara.loftus, Sandeep Penigalapati
From: Sandeep Penigalapati <sandeep.penigalapati@intel.com>
Shared UMEM is meant to be shared by a limited number of sockets,
governed by the mempool size (max_xsks). When the UMEM was already at
capacity (refcnt >= max_xsks), xdp_umem_configure() returned the UMEM
without incrementing its refcount, so the extra socket used it
unaccounted for.
This missing reference has two consequences. During queue setup the
fill-queue reservation is chosen from the refcount, so the sharing
socket reserves into its own uninitialised fill queue and crashes. At
close, the under-counted refcount reaches zero while the UMEM is still
in use, freeing it early and causing a use-after-free.
Reject sharing once the UMEM is at capacity by returning NULL. The
error is propagated from xsk_configure(), so Rx queue setup fails
cleanly with -ENOMEM. This applies the per-mempool socket limit that
shared UMEM was always intended to respect.
Harden the failure path this makes reachable:
- clear rxq->umem and its paired txq->umem when xsk_configure() fails,
and skip queues whose UMEM is not yet set in get_shared_umem(), so a
later scan over the same mempool cannot dereference a NULL or
dangling UMEM;
- free the fill-queue mbufs that were allocated but not yet handed to
the fill queue when a sharing socket fails to bind, so it no longer
leaks a burst of mbufs back out of the shared mempool;
- propagate the map-insert failures in xsk_configure() instead of
returning success, so a failed xsks_map update no longer leaves the
caller using a deleted socket;
- continue past, rather than stop at, a failed queue in eth_dev_close()
so later successful queues and their UMEM references are still freed;
- clamp max_xsks to UINT8_MAX so the cap stays within the uint8_t
refcount.
Also correct the UMEM refcount memory ordering: relaxed on the shared
increment and acquire-release on the final decrement, so the thread that
drops the last reference observes all prior users' writes before it
frees the UMEM.
Document the shared mempool sizing requirement (4096 mbufs per socket).
Note: on stable branches this is a behaviour change. Shared-UMEM setups
that previously appeared to start, until the fill-queue crash or the
use-after-free at close, now fail cleanly at Rx queue setup with
-ENOMEM.
Fixes: 74b46340e2d4 ("net/af_xdp: support shared UMEM")
Cc: stable@dpdk.org
Signed-off-by: Sandeep Penigalapati <sandeep.penigalapati@intel.com>
---
v5:
- Use acq_rel on the final refcount decrement instead of a release
decrement plus a standalone acquire fence.
- doc: one sentence per line.
- Drop the redundant fq_bufs ownership comment.
v4:
- Free fill-queue mbufs on the failure path.
- Propagate map-insert failures and the -ENOMEM from xsk_configure() to
the caller instead of a blanket -EINVAL / silent success.
- eth_dev_close(): continue past failed queues instead of break.
- Correct refcount memory ordering (relaxed increment, release
decrement, acquire fence before destroy).
- Clamp max_xsks to UINT8_MAX so the cap fits the uint8_t refcount.
- Clear txq->umem on the early return; clearer log when the mempool is
too small (max_xsks == 0).
v3:
- Move the NULL umem check after ctx_exists() so a failed queue is still
checked for a duplicate context, as before.
- Shorten the "at capacity" log to one line and drop the count.
- Also clear txq->umem, not just rxq->umem, when setup fails.
- Clarify "AF_XDP socket" in the doc.
v2:
- Guard get_shared_umem() against a NULL umem and clear rxq->umem on the
xsk_configure() error path.
- doc: "Rx queue setup fails", one sentence per line.
- Drop unrelated reflow of the refcount increment.
doc/guides/nics/af_xdp.rst | 4 ++
drivers/net/af_xdp/rte_eth_af_xdp.c | 59 +++++++++++++++++++++++------
2 files changed, 51 insertions(+), 12 deletions(-)
diff --git a/doc/guides/nics/af_xdp.rst b/doc/guides/nics/af_xdp.rst
index c455b4c066..1fff910795 100644
--- a/doc/guides/nics/af_xdp.rst
+++ b/doc/guides/nics/af_xdp.rst
@@ -99,6 +99,10 @@ configured like so:
--vdev net_af_xdp0,iface=ens786f1,shared_umem=1 \
--vdev net_af_xdp1,iface=ens786f2,shared_umem=1
+The shared mempool must be large enough for every AF_XDP socket sharing the UMEM.
+Each socket needs 4096 mbufs, so ``N`` sockets need at least ``4096 * N`` mbufs.
+Rx queue setup fails if the mempool is too small to add another socket to the UMEM.
+
xdp_prog
~~~~~~~~
diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
index 2cdb533276..16d9a62a7c 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -1062,12 +1062,13 @@ eth_dev_close(struct rte_eth_dev *dev)
for (i = 0; i < internals->queue_cnt; i++) {
rxq = &internals->rx_queues[i];
+ /* Skip queues whose setup failed (umem left NULL). */
if (rxq->umem == NULL)
- break;
+ continue;
xsk_socket__delete(rxq->xsk);
if (rte_atomic_fetch_sub_explicit(&rxq->umem->refcnt, 1,
- rte_memory_order_acquire) - 1 == 0)
+ rte_memory_order_acq_rel) - 1 == 0)
xdp_umem_destroy(rxq->umem);
}
/* Free Tx and Rx queue arrays */
@@ -1154,6 +1155,9 @@ get_shared_umem(struct pkt_rx_queue *rxq, const char *ifname,
ret = -1;
goto out;
}
+ /* A failed setup leaves mb_pool set with no umem. */
+ if (list_rxq->umem == NULL)
+ continue;
if (rte_atomic_load_explicit(&internals->rx_queues[i].umem->refcnt,
rte_memory_order_acquire)) {
*umem = internals->rx_queues[i].umem;
@@ -1188,12 +1192,24 @@ xsk_umem_info *xdp_umem_configure(struct pmd_internals *internals,
if (get_shared_umem(rxq, internals->if_name, &umem) < 0)
return NULL;
- if (umem != NULL &&
- rte_atomic_load_explicit(&umem->refcnt, rte_memory_order_acquire) <
- umem->max_xsks) {
+ if (umem != NULL) {
+ /* Reject sharing once the UMEM is at capacity. */
+ if (rte_atomic_load_explicit(&umem->refcnt,
+ rte_memory_order_acquire) >= umem->max_xsks) {
+ if (umem->max_xsks == 0)
+ AF_XDP_LOG_LINE(ERR, "%s,qid%i: mempool %s too small to share UMEM",
+ internals->if_name, rxq->xsk_queue_idx,
+ umem->mb_pool->name);
+ else
+ AF_XDP_LOG_LINE(ERR, "%s,qid%i: UMEM %s already at max %u sockets",
+ internals->if_name, rxq->xsk_queue_idx,
+ umem->mb_pool->name, umem->max_xsks);
+ return NULL;
+ }
+
AF_XDP_LOG_LINE(INFO, "%s,qid%i sharing UMEM",
internals->if_name, rxq->xsk_queue_idx);
- rte_atomic_fetch_add_explicit(&umem->refcnt, 1, rte_memory_order_acquire);
+ rte_atomic_fetch_add_explicit(&umem->refcnt, 1, rte_memory_order_relaxed);
}
}
@@ -1239,8 +1255,10 @@ xsk_umem_info *xdp_umem_configure(struct pmd_internals *internals,
umem->buffer = aligned_addr;
if (internals->shared_umem) {
- umem->max_xsks = mb_pool->populated_size /
- ETH_AF_XDP_NUM_BUFFERS;
+ /* refcnt is uint8_t, so the cap cannot exceed UINT8_MAX. */
+ umem->max_xsks = RTE_MIN(mb_pool->populated_size /
+ ETH_AF_XDP_NUM_BUFFERS,
+ (uint32_t)UINT8_MAX);
AF_XDP_LOG_LINE(INFO, "Max xsks for UMEM %s: %u",
mb_pool->name, umem->max_xsks);
}
@@ -1684,10 +1702,13 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
int reserve_size = ETH_AF_XDP_DFLT_NUM_DESCS;
struct rte_mbuf *fq_bufs[reserve_size];
bool reserve_before;
+ bool free_fq_bufs = false;
rxq->umem = xdp_umem_configure(internals, rxq);
- if (rxq->umem == NULL)
+ if (rxq->umem == NULL) {
+ txq->umem = NULL;
return -ENOMEM;
+ }
txq->umem = rxq->umem;
reserve_before = rte_atomic_load_explicit(&rxq->umem->refcnt,
rte_memory_order_acquire) <= 1;
@@ -1698,11 +1719,13 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
AF_XDP_LOG_LINE(DEBUG, "Failed to get enough buffers for fq.");
goto out_umem;
}
+ free_fq_bufs = true;
#endif
/* reserve fill queue of queues not (yet) sharing UMEM */
if (reserve_before) {
ret = reserve_fill_queue(rxq->umem, reserve_size, fq_bufs, &rxq->fq);
+ free_fq_bufs = false;
if (ret) {
AF_XDP_LOG_LINE(ERR, "Failed to reserve fill queue.");
goto out_umem;
@@ -1759,6 +1782,7 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
if (!reserve_before) {
/* reserve fill queue of queues sharing UMEM */
ret = reserve_fill_queue(rxq->umem, reserve_size, fq_bufs, &rxq->fq);
+ free_fq_bufs = false;
if (ret) {
AF_XDP_LOG_LINE(ERR, "Failed to reserve fill queue.");
goto out_xsk;
@@ -1774,6 +1798,7 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
&rxq->xsk_queue_idx, &fd, 0);
if (err) {
AF_XDP_LOG_LINE(ERR, "Failed to insert xsk in map.");
+ ret = -EINVAL;
goto out_xsk;
}
}
@@ -1786,6 +1811,7 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
map_fd = uds_get_xskmap_fd(internals->if_name, internals->dp_path);
if (map_fd < 0) {
AF_XDP_LOG_LINE(ERR, "Failed to receive xskmap fd from AF_XDP Device Plugin");
+ ret = -EINVAL;
goto out_xsk;
}
} else {
@@ -1793,6 +1819,7 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
err = get_pinned_map(internals->dp_path, &map_fd);
if (err < 0 || map_fd < 0) {
AF_XDP_LOG_LINE(ERR, "Failed to retrieve pinned map fd");
+ ret = -EINVAL;
goto out_xsk;
}
}
@@ -1800,6 +1827,7 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
err = update_xskmap(rxq->xsk, map_fd, rxq->xsk_queue_idx);
if (err) {
AF_XDP_LOG_LINE(ERR, "Failed to insert xsk in map.");
+ ret = -EINVAL;
goto out_xsk;
}
@@ -1816,8 +1844,15 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
out_xsk:
xsk_socket__delete(rxq->xsk);
out_umem:
- if (rte_atomic_fetch_sub_explicit(&rxq->umem->refcnt, 1, rte_memory_order_acquire) - 1 == 0)
+ /* Free fq_bufs that were allocated but never handed to the fill queue. */
+ if (free_fq_bufs)
+ rte_pktmbuf_free_bulk(fq_bufs, reserve_size);
+ if (rte_atomic_fetch_sub_explicit(&rxq->umem->refcnt, 1,
+ rte_memory_order_acq_rel) - 1 == 0)
xdp_umem_destroy(rxq->umem);
+ /* Drop dangling pointers so a later shared-UMEM scan skips this queue. */
+ rxq->umem = NULL;
+ txq->umem = NULL;
return ret;
}
@@ -1858,9 +1893,9 @@ eth_rx_queue_setup(struct rte_eth_dev *dev,
rxq->mb_pool = mb_pool;
- if (xsk_configure(internals, rxq, nb_rx_desc)) {
+ ret = xsk_configure(internals, rxq, nb_rx_desc);
+ if (ret) {
AF_XDP_LOG_LINE(ERR, "Failed to configure xdp socket");
- ret = -EINVAL;
goto err;
}
--
2.27.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v6] net/af_xdp: fix shared UMEM refcount corruption
2026-08-21 0:44 ` [PATCH v5] " sandeep.penigalapati
@ 2026-08-21 2:16 ` sandeep.penigalapati
2026-08-21 18:10 ` Stephen Hemminger
0 siblings, 1 reply; 5+ messages in thread
From: sandeep.penigalapati @ 2026-08-21 2:16 UTC (permalink / raw)
To: dev; +Cc: stable, stephen, ciara.loftus, Sandeep Penigalapati
From: Sandeep Penigalapati <sandeep.penigalapati@intel.com>
Shared UMEM is meant to be shared by a limited number of sockets,
governed by the mempool size (max_xsks). When the UMEM was already at
capacity (refcnt >= max_xsks), xdp_umem_configure() returned the UMEM
without incrementing its refcount, so the extra socket used it
unaccounted for.
This missing reference has two consequences. During queue setup the
fill-queue reservation is chosen from the refcount, so the sharing
socket reserves into its own uninitialised fill queue and crashes. At
close, the under-counted refcount reaches zero while the UMEM is still
in use, freeing it early and causing a use-after-free.
Reject sharing once the UMEM is at capacity by returning NULL. The
error is propagated from xsk_configure(), so Rx queue setup fails
cleanly with -ENOMEM. This applies the per-mempool socket limit that
shared UMEM was always intended to respect.
Harden the failure path this makes reachable:
- clear rxq->umem and its paired txq->umem when xsk_configure() fails,
and skip queues whose UMEM is not yet set in get_shared_umem(), so a
later scan over the same mempool cannot dereference a NULL or
dangling UMEM;
- free the fill-queue mbufs that were allocated but not yet handed to
the fill queue when a sharing socket fails to bind, so it no longer
leaks a burst of mbufs back out of the shared mempool;
- propagate the map-insert failures in xsk_configure() instead of
returning success, so a failed xsks_map update no longer leaves the
caller using a deleted socket;
- continue past, rather than stop at, a failed queue in eth_dev_close()
so later successful queues and their UMEM references are still freed;
- clamp max_xsks to UINT8_MAX so the cap stays within the uint8_t
refcount.
Also correct the UMEM refcount memory ordering: release on the shared
increment and acquire-release on the final decrement, so the thread that
drops the last reference observes all prior users' writes before it
frees the UMEM.
Document the shared mempool sizing requirement (4096 mbufs per socket).
Note: on stable branches this is a behaviour change. Shared-UMEM setups
that previously appeared to start, until the fill-queue crash or the
use-after-free at close, now fail cleanly at Rx queue setup with
-ENOMEM.
Fixes: 74b46340e2d4 ("net/af_xdp: support shared UMEM")
Cc: stable@dpdk.org
Signed-off-by: Sandeep Penigalapati <sandeep.penigalapati@intel.com>
---
v6:
- Use release ordering on the shared refcount increment.
- doc: note the 4096 mbufs are for the fill queue.
- Refer to eth_rx_queue_setup() in the eth_dev_close() skip comment.
- Log the mempool's mbuf count and the required minimum when it is too
small to share the UMEM, and wrap the log lines to stay under 100
columns.
v5:
- Use acq_rel on the final refcount decrement instead of a release
decrement plus a standalone acquire fence.
- doc: one sentence per line.
- Drop the redundant fq_bufs ownership comment.
v4:
- Free fill-queue mbufs on the failure path.
- Propagate map-insert failures and the -ENOMEM from xsk_configure() to
the caller instead of a blanket -EINVAL / silent success.
- eth_dev_close(): continue past failed queues instead of break.
- Correct refcount memory ordering (relaxed increment, release
decrement, acquire fence before destroy).
- Clamp max_xsks to UINT8_MAX so the cap fits the uint8_t refcount.
- Clear txq->umem on the early return; clearer log when the mempool is
too small (max_xsks == 0).
v3:
- Move the NULL umem check after ctx_exists() so a failed queue is still
checked for a duplicate context, as before.
- Shorten the "at capacity" log to one line and drop the count.
- Also clear txq->umem, not just rxq->umem, when setup fails.
- Clarify "AF_XDP socket" in the doc.
v2:
- Guard get_shared_umem() against a NULL umem and clear rxq->umem on the
xsk_configure() error path.
- doc: "Rx queue setup fails", one sentence per line.
- Drop unrelated reflow of the refcount increment.
doc/guides/nics/af_xdp.rst | 4 ++
drivers/net/af_xdp/rte_eth_af_xdp.c | 64 +++++++++++++++++++++++------
2 files changed, 56 insertions(+), 12 deletions(-)
diff --git a/doc/guides/nics/af_xdp.rst b/doc/guides/nics/af_xdp.rst
index c455b4c066..249efc8c43 100644
--- a/doc/guides/nics/af_xdp.rst
+++ b/doc/guides/nics/af_xdp.rst
@@ -99,6 +99,10 @@ configured like so:
--vdev net_af_xdp0,iface=ens786f1,shared_umem=1 \
--vdev net_af_xdp1,iface=ens786f2,shared_umem=1
+The shared mempool must be large enough for every AF_XDP socket sharing the UMEM.
+Each socket needs 4096 mbufs for its fill queue, so ``N`` sockets need at least ``4096 * N`` mbufs.
+Rx queue setup fails if the mempool is too small to add another socket to the UMEM.
+
xdp_prog
~~~~~~~~
diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
index 2cdb533276..5f808d1b83 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -1062,12 +1062,13 @@ eth_dev_close(struct rte_eth_dev *dev)
for (i = 0; i < internals->queue_cnt; i++) {
rxq = &internals->rx_queues[i];
+ /* Skip queues where eth_rx_queue_setup() was never called or failed. */
if (rxq->umem == NULL)
- break;
+ continue;
xsk_socket__delete(rxq->xsk);
if (rte_atomic_fetch_sub_explicit(&rxq->umem->refcnt, 1,
- rte_memory_order_acquire) - 1 == 0)
+ rte_memory_order_acq_rel) - 1 == 0)
xdp_umem_destroy(rxq->umem);
}
/* Free Tx and Rx queue arrays */
@@ -1154,6 +1155,9 @@ get_shared_umem(struct pkt_rx_queue *rxq, const char *ifname,
ret = -1;
goto out;
}
+ /* A failed setup leaves mb_pool set with no umem. */
+ if (list_rxq->umem == NULL)
+ continue;
if (rte_atomic_load_explicit(&internals->rx_queues[i].umem->refcnt,
rte_memory_order_acquire)) {
*umem = internals->rx_queues[i].umem;
@@ -1188,12 +1192,29 @@ xsk_umem_info *xdp_umem_configure(struct pmd_internals *internals,
if (get_shared_umem(rxq, internals->if_name, &umem) < 0)
return NULL;
- if (umem != NULL &&
- rte_atomic_load_explicit(&umem->refcnt, rte_memory_order_acquire) <
- umem->max_xsks) {
+ if (umem != NULL) {
+ /* Reject sharing once the UMEM is at capacity. */
+ if (rte_atomic_load_explicit(&umem->refcnt,
+ rte_memory_order_acquire) >= umem->max_xsks) {
+ if (umem->max_xsks == 0)
+ AF_XDP_LOG_LINE(ERR,
+ "%s,qid%i: mempool %s has %u mbufs, "
+ "need at least %u to share UMEM",
+ internals->if_name, rxq->xsk_queue_idx,
+ umem->mb_pool->name,
+ umem->mb_pool->populated_size,
+ ETH_AF_XDP_NUM_BUFFERS);
+ else
+ AF_XDP_LOG_LINE(ERR,
+ "%s,qid%i: UMEM %s already at max %u sockets",
+ internals->if_name, rxq->xsk_queue_idx,
+ umem->mb_pool->name, umem->max_xsks);
+ return NULL;
+ }
+
AF_XDP_LOG_LINE(INFO, "%s,qid%i sharing UMEM",
internals->if_name, rxq->xsk_queue_idx);
- rte_atomic_fetch_add_explicit(&umem->refcnt, 1, rte_memory_order_acquire);
+ rte_atomic_fetch_add_explicit(&umem->refcnt, 1, rte_memory_order_release);
}
}
@@ -1239,8 +1260,10 @@ xsk_umem_info *xdp_umem_configure(struct pmd_internals *internals,
umem->buffer = aligned_addr;
if (internals->shared_umem) {
- umem->max_xsks = mb_pool->populated_size /
- ETH_AF_XDP_NUM_BUFFERS;
+ /* refcnt is uint8_t, so the cap cannot exceed UINT8_MAX. */
+ umem->max_xsks = RTE_MIN(mb_pool->populated_size /
+ ETH_AF_XDP_NUM_BUFFERS,
+ (uint32_t)UINT8_MAX);
AF_XDP_LOG_LINE(INFO, "Max xsks for UMEM %s: %u",
mb_pool->name, umem->max_xsks);
}
@@ -1684,10 +1707,13 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
int reserve_size = ETH_AF_XDP_DFLT_NUM_DESCS;
struct rte_mbuf *fq_bufs[reserve_size];
bool reserve_before;
+ bool free_fq_bufs = false;
rxq->umem = xdp_umem_configure(internals, rxq);
- if (rxq->umem == NULL)
+ if (rxq->umem == NULL) {
+ txq->umem = NULL;
return -ENOMEM;
+ }
txq->umem = rxq->umem;
reserve_before = rte_atomic_load_explicit(&rxq->umem->refcnt,
rte_memory_order_acquire) <= 1;
@@ -1698,11 +1724,13 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
AF_XDP_LOG_LINE(DEBUG, "Failed to get enough buffers for fq.");
goto out_umem;
}
+ free_fq_bufs = true;
#endif
/* reserve fill queue of queues not (yet) sharing UMEM */
if (reserve_before) {
ret = reserve_fill_queue(rxq->umem, reserve_size, fq_bufs, &rxq->fq);
+ free_fq_bufs = false;
if (ret) {
AF_XDP_LOG_LINE(ERR, "Failed to reserve fill queue.");
goto out_umem;
@@ -1759,6 +1787,7 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
if (!reserve_before) {
/* reserve fill queue of queues sharing UMEM */
ret = reserve_fill_queue(rxq->umem, reserve_size, fq_bufs, &rxq->fq);
+ free_fq_bufs = false;
if (ret) {
AF_XDP_LOG_LINE(ERR, "Failed to reserve fill queue.");
goto out_xsk;
@@ -1774,6 +1803,7 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
&rxq->xsk_queue_idx, &fd, 0);
if (err) {
AF_XDP_LOG_LINE(ERR, "Failed to insert xsk in map.");
+ ret = -EINVAL;
goto out_xsk;
}
}
@@ -1786,6 +1816,7 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
map_fd = uds_get_xskmap_fd(internals->if_name, internals->dp_path);
if (map_fd < 0) {
AF_XDP_LOG_LINE(ERR, "Failed to receive xskmap fd from AF_XDP Device Plugin");
+ ret = -EINVAL;
goto out_xsk;
}
} else {
@@ -1793,6 +1824,7 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
err = get_pinned_map(internals->dp_path, &map_fd);
if (err < 0 || map_fd < 0) {
AF_XDP_LOG_LINE(ERR, "Failed to retrieve pinned map fd");
+ ret = -EINVAL;
goto out_xsk;
}
}
@@ -1800,6 +1832,7 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
err = update_xskmap(rxq->xsk, map_fd, rxq->xsk_queue_idx);
if (err) {
AF_XDP_LOG_LINE(ERR, "Failed to insert xsk in map.");
+ ret = -EINVAL;
goto out_xsk;
}
@@ -1816,8 +1849,15 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
out_xsk:
xsk_socket__delete(rxq->xsk);
out_umem:
- if (rte_atomic_fetch_sub_explicit(&rxq->umem->refcnt, 1, rte_memory_order_acquire) - 1 == 0)
+ /* Free fq_bufs that were allocated but never handed to the fill queue. */
+ if (free_fq_bufs)
+ rte_pktmbuf_free_bulk(fq_bufs, reserve_size);
+ if (rte_atomic_fetch_sub_explicit(&rxq->umem->refcnt, 1,
+ rte_memory_order_acq_rel) - 1 == 0)
xdp_umem_destroy(rxq->umem);
+ /* Drop dangling pointers so a later shared-UMEM scan skips this queue. */
+ rxq->umem = NULL;
+ txq->umem = NULL;
return ret;
}
@@ -1858,9 +1898,9 @@ eth_rx_queue_setup(struct rte_eth_dev *dev,
rxq->mb_pool = mb_pool;
- if (xsk_configure(internals, rxq, nb_rx_desc)) {
+ ret = xsk_configure(internals, rxq, nb_rx_desc);
+ if (ret) {
AF_XDP_LOG_LINE(ERR, "Failed to configure xdp socket");
- ret = -EINVAL;
goto err;
}
--
2.27.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v6] net/af_xdp: fix shared UMEM refcount corruption
2026-08-21 2:16 ` [PATCH v6] " sandeep.penigalapati
@ 2026-08-21 18:10 ` Stephen Hemminger
2026-08-24 12:10 ` Penigalapati, Sandeep
0 siblings, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2026-08-21 18:10 UTC (permalink / raw)
To: sandeep.penigalapati; +Cc: dev, stable, ciara.loftus
On Thu, 20 Aug 2026 22:16:07 -0400
sandeep.penigalapati@intel.com wrote:
> From: Sandeep Penigalapati <sandeep.penigalapati@intel.com>
>
> Shared UMEM is meant to be shared by a limited number of sockets,
> governed by the mempool size (max_xsks). When the UMEM was already at
> capacity (refcnt >= max_xsks), xdp_umem_configure() returned the UMEM
> without incrementing its refcount, so the extra socket used it
> unaccounted for.
>
> This missing reference has two consequences. During queue setup the
> fill-queue reservation is chosen from the refcount, so the sharing
> socket reserves into its own uninitialised fill queue and crashes. At
> close, the under-counted refcount reaches zero while the UMEM is still
> in use, freeing it early and causing a use-after-free.
>
> Reject sharing once the UMEM is at capacity by returning NULL. The
> error is propagated from xsk_configure(), so Rx queue setup fails
> cleanly with -ENOMEM. This applies the per-mempool socket limit that
> shared UMEM was always intended to respect.
>
> Harden the failure path this makes reachable:
> - clear rxq->umem and its paired txq->umem when xsk_configure() fails,
> and skip queues whose UMEM is not yet set in get_shared_umem(), so a
> later scan over the same mempool cannot dereference a NULL or
> dangling UMEM;
> - free the fill-queue mbufs that were allocated but not yet handed to
> the fill queue when a sharing socket fails to bind, so it no longer
> leaks a burst of mbufs back out of the shared mempool;
> - propagate the map-insert failures in xsk_configure() instead of
> returning success, so a failed xsks_map update no longer leaves the
> caller using a deleted socket;
> - continue past, rather than stop at, a failed queue in eth_dev_close()
> so later successful queues and their UMEM references are still freed;
> - clamp max_xsks to UINT8_MAX so the cap stays within the uint8_t
> refcount.
>
> Also correct the UMEM refcount memory ordering: release on the shared
> increment and acquire-release on the final decrement, so the thread that
> drops the last reference observes all prior users' writes before it
> frees the UMEM.
>
> Document the shared mempool sizing requirement (4096 mbufs per socket).
>
> Note: on stable branches this is a behaviour change. Shared-UMEM setups
> that previously appeared to start, until the fill-queue crash or the
> use-after-free at close, now fail cleanly at Rx queue setup with
> -ENOMEM.
>
> Fixes: 74b46340e2d4 ("net/af_xdp: support shared UMEM")
> Cc: stable@dpdk.org
>
> Signed-off-by: Sandeep Penigalapati <sandeep.penigalapati@intel.com>
I am ok with it as is but AI still has some Info level comments.
Will take it as is, or you can revise (your choice).
Trimmed away the noise..
Review of [PATCH v6] net/af_xdp: fix shared UMEM refcount corruption
1. The refcount increment does not need release ordering.
rte_atomic_fetch_add_explicit(&umem->refcnt, 1,
rte_memory_order_release);
rte_memory_order_relaxed is the correct weakest choice here. The
incrementing thread has no prior writes to publish; the UMEM was
built by whoever created it, and that publication is already covered
by the release store of refcnt = 1 at the end of
xdp_umem_configure().
The commit message attributes the guarantee to the wrong operation:
"release on the shared increment ... so the thread that drops the
last reference observes all prior users' writes" is what the acq_rel
on the fetch_sub provides, not the increment. Worth correcting in
the message even if the ordering is left as is; it is harmless but
the rationale will outlive the patch in git history.
2. The capacity check and the increment are still not atomic, and
rxq->umem is mutated outside internal_list_lock.
get_shared_umem() releases internal_list_lock before returning, so
the load of refcnt in xdp_umem_configure() and the fetch_add that
follows are separate steps; two threads configuring queues on the
same mempool can both observe refcnt < max_xsks and both increment.
Separately, xsk_configure() and eth_dev_close() write rxq->umem
without the lock that get_shared_umem() holds when reading it, so a
concurrent failure could in principle free a UMEM between the NULL
check and the dereference.
Both are pre-existing and control-path setup is single threaded in
practice, so this is a note rather than a request. A
compare-exchange loop on refcnt would make the cap the patch adds
actually enforceable if that ever changes.
3. Mbufs already submitted to the fill queue are still lost at
out_xsk.
Once reserve_fill_queue() succeeds, the 2048 mbufs live in rxq->fq.
A later failure (map insert, busy-poll config) deletes the socket
and takes out_umem, and nothing drains the fill ring, so those mbufs
never return to the mempool. free_fq_bufs is correctly false at
that point, so this is not a regression from the patch, and
recovering them would mean unwinding the fill ring. Noting it as a
remaining gap rather than something to fix here.
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v6] net/af_xdp: fix shared UMEM refcount corruption
2026-08-21 18:10 ` Stephen Hemminger
@ 2026-08-24 12:10 ` Penigalapati, Sandeep
0 siblings, 0 replies; 5+ messages in thread
From: Penigalapati, Sandeep @ 2026-08-24 12:10 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev@dpdk.org, stable@dpdk.org, Loftus, Ciara
>-----Original Message-----
>From: Stephen Hemminger <stephen@networkplumber.org>
>Sent: Friday, August 21, 2026 11:41 PM
>To: Penigalapati, Sandeep <sandeep.penigalapati@intel.com>
>Cc: dev@dpdk.org; stable@dpdk.org; Loftus, Ciara <ciara.loftus@intel.com>
>Subject: Re: [PATCH v6] net/af_xdp: fix shared UMEM refcount corruption
>
>On Thu, 20 Aug 2026 22:16:07 -0400
>sandeep.penigalapati@intel.com wrote:
>
>> From: Sandeep Penigalapati <sandeep.penigalapati@intel.com>
>>
>> Shared UMEM is meant to be shared by a limited number of sockets,
>> governed by the mempool size (max_xsks). When the UMEM was already
>at
>> capacity (refcnt >= max_xsks), xdp_umem_configure() returned the UMEM
>> without incrementing its refcount, so the extra socket used it
>> unaccounted for.
>>
>> This missing reference has two consequences. During queue setup the
>> fill-queue reservation is chosen from the refcount, so the sharing
>> socket reserves into its own uninitialised fill queue and crashes. At
>> close, the under-counted refcount reaches zero while the UMEM is still
>> in use, freeing it early and causing a use-after-free.
>>
>> Reject sharing once the UMEM is at capacity by returning NULL. The
>> error is propagated from xsk_configure(), so Rx queue setup fails
>> cleanly with -ENOMEM. This applies the per-mempool socket limit that
>> shared UMEM was always intended to respect.
>>
>> Harden the failure path this makes reachable:
>> - clear rxq->umem and its paired txq->umem when xsk_configure() fails,
>> and skip queues whose UMEM is not yet set in get_shared_umem(), so a
>> later scan over the same mempool cannot dereference a NULL or
>> dangling UMEM;
>> - free the fill-queue mbufs that were allocated but not yet handed to
>> the fill queue when a sharing socket fails to bind, so it no longer
>> leaks a burst of mbufs back out of the shared mempool;
>> - propagate the map-insert failures in xsk_configure() instead of
>> returning success, so a failed xsks_map update no longer leaves the
>> caller using a deleted socket;
>> - continue past, rather than stop at, a failed queue in eth_dev_close()
>> so later successful queues and their UMEM references are still
>> freed;
>> - clamp max_xsks to UINT8_MAX so the cap stays within the uint8_t
>> refcount.
>>
>> Also correct the UMEM refcount memory ordering: release on the shared
>> increment and acquire-release on the final decrement, so the thread
>> that drops the last reference observes all prior users' writes before
>> it frees the UMEM.
>>
>> Document the shared mempool sizing requirement (4096 mbufs per
>socket).
>>
>> Note: on stable branches this is a behaviour change. Shared-UMEM
>> setups that previously appeared to start, until the fill-queue crash
>> or the use-after-free at close, now fail cleanly at Rx queue setup
>> with -ENOMEM.
>>
>> Fixes: 74b46340e2d4 ("net/af_xdp: support shared UMEM")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Sandeep Penigalapati <sandeep.penigalapati@intel.com>
>
>I am ok with it as is but AI still has some Info level comments.
>Will take it as is, or you can revise (your choice).
Thanks, Stephen - I'm good with v6 going in as-is.
>
>Trimmed away the noise..
>
>Review of [PATCH v6] net/af_xdp: fix shared UMEM refcount corruption
>
>
>1. The refcount increment does not need release ordering.
>
> rte_atomic_fetch_add_explicit(&umem->refcnt, 1,
> rte_memory_order_release);
>
> rte_memory_order_relaxed is the correct weakest choice here. The
> incrementing thread has no prior writes to publish; the UMEM was
> built by whoever created it, and that publication is already covered
> by the release store of refcnt = 1 at the end of
> xdp_umem_configure().
>
> The commit message attributes the guarantee to the wrong operation:
> "release on the shared increment ... so the thread that drops the
> last reference observes all prior users' writes" is what the acq_rel
> on the fetch_sub provides, not the increment. Worth correcting in
> the message even if the ordering is left as is; it is harmless but
> the rationale will outlive the patch in git history.
>
>2. The capacity check and the increment are still not atomic, and
> rxq->umem is mutated outside internal_list_lock.
>
> get_shared_umem() releases internal_list_lock before returning, so
> the load of refcnt in xdp_umem_configure() and the fetch_add that
> follows are separate steps; two threads configuring queues on the
> same mempool can both observe refcnt < max_xsks and both increment.
> Separately, xsk_configure() and eth_dev_close() write rxq->umem
> without the lock that get_shared_umem() holds when reading it, so a
> concurrent failure could in principle free a UMEM between the NULL
> check and the dereference.
>
> Both are pre-existing and control-path setup is single threaded in
> practice, so this is a note rather than a request. A
> compare-exchange loop on refcnt would make the cap the patch adds
> actually enforceable if that ever changes.
>
>3. Mbufs already submitted to the fill queue are still lost at
> out_xsk.
>
> Once reserve_fill_queue() succeeds, the 2048 mbufs live in rxq->fq.
> A later failure (map insert, busy-poll config) deletes the socket
> and takes out_umem, and nothing drains the fill ring, so those mbufs
> never return to the mempool. free_fq_bufs is correctly false at
> that point, so this is not a regression from the patch, and
> recovering them would mean unwinding the fill ring. Noting it as a
> remaining gap rather than something to fix here.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-08-24 12:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-20 23:05 [PATCH v4] net/af_xdp: fix shared UMEM refcount corruption sandeep.penigalapati
2026-08-21 0:44 ` [PATCH v5] " sandeep.penigalapati
2026-08-21 2:16 ` [PATCH v6] " sandeep.penigalapati
2026-08-21 18:10 ` Stephen Hemminger
2026-08-24 12:10 ` Penigalapati, Sandeep
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox