[PATCH net-next 0/5] selftests/xsk: improve shared-UMEM coverage and infrastructure

[PATCH net-next 0/5] selftests/xsk: improve shared-UMEM coverage and infrastructure

[Tushar Vyavahare]

AF_XDP shared-UMEM support in existing XSK selftests is limited to a
small set of scenarios and cleanup paths that become fragile as coverage
expands.

This series improves shared-UMEM testing and infrastructure in five
steps.

Patch 1 introduces explicit UMEM lifetime management with refcounting
and consolidates socket/UMEM teardown into common helpers, keeping
cleanup behavior consistent across normal completion and error paths.

Patch 2 adds rollback for partial socket setup in xsk_configure() when
xsk_socket__create() retries are exhausted or busy-poll setup fails.

Patch 3 adds a length-based XDP steering helper and updates XSKMAP/test
capacity constants for 4-socket scenarios. Steering is explicitly based
on total XDP-visible packet length (data_end - data) using
SHARED_UMEM_LEN_SPLIT.

Patch 4 adds the shared-UMEM callback runner together with the first
four test cases (4-socket, length-based steering, uneven distribution,
and unaligned chunk mode) in one commit so every SHA in the series
builds and bisects cleanly. The unaligned path snapshots/restores prior
alignment flags with NULL-safe UMEM guards.

Patch 5 makes pkt_stream_even_odd_sequence() rollback-safe by staging
replacement streams in temporary arrays and committing only after all
allocations succeed.

Together, these patches make shared-UMEM setup/cleanup more robust and
extend selftest coverage to common multi-socket and distribution
scenarios.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>

Tushar Vyavahare (5):
  selftests/xsk: add UMEM users refcount and centralize socket teardown
  selftests/xsk: roll back partial socket setup on configure failures
  selftests/xsk: expand XSKMAP capacity and add length-based XDP program
  selftests/xsk: add shared-UMEM callback framework and initial test
    cases
  selftests/xsk: make pkt_stream_even_odd_sequence rollback-safe

 .../selftests/bpf/prog_tests/test_xsk.c       | 428 ++++++++++++++++--
 .../selftests/bpf/prog_tests/test_xsk.h       |  24 +
 .../selftests/bpf/progs/xsk_xdp_progs.c       |  19 +-
 tools/testing/selftests/bpf/xsk_xdp_common.h  |   3 +-
 tools/testing/selftests/bpf/xskxceiver.c      |  35 +-
 5 files changed, 436 insertions(+), 73 deletions(-)

-- 
2.43.0

[PATCH net-next 1/5] selftests/xsk: add UMEM users refcount and centralize socket teardown

[Tushar Vyavahare]

Shared-UMEM lifetime can be torn down from multiple setup and cleanup
paths. Mixing manual UMEM free/unmap in one path with helper-based
teardown in others makes ownership easier to violate and can leak or
double-release UMEM during error handling.

Track UMEM ownership with a refcount_t users field. Initialize it to 1
in xsk_configure_umem() and increment it for each additional shared
socket. Introduce xsk_delete_socket() as the single teardown helper: it
deletes the socket handle, clears state to prevent double-decrement, and
releases the UMEM when the last reference drops.

Replace testapp_clean_xsk_umem(), clean_sockets(), and clean_umem() with
xsk_delete_socket_batch() and xsk_delete_all_ifobj_sockets() wrappers at
all call sites. Convert ifobj_zc_avail() in xskxceiver.c to use stack
allocation and xsk_delete_socket() for uniform cleanup.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
---
 .../selftests/bpf/prog_tests/test_xsk.c       | 124 ++++++++++++------
 .../selftests/bpf/prog_tests/test_xsk.h       |   3 +
 tools/testing/selftests/bpf/xskxceiver.c      |  35 ++---
 3 files changed, 103 insertions(+), 59 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
index 477aedbb01ba..4ccdb0825130 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
@@ -101,6 +101,7 @@ int xsk_configure_umem(struct ifobject *ifobj, struct xsk_umem_info *umem, void
 		return ret;
 
 	umem->buffer = buffer;
+	refcount_set(&umem->users, 1);
 	if (ifobj->shared_umem && ifobj->rx_on) {
 		umem->base_addr = umem_size(umem);
 		umem->next_buffer = umem_size(umem);
@@ -154,6 +155,7 @@ int xsk_configure_socket(struct xsk_socket_info *xsk, struct xsk_umem_info *umem
 	struct xsk_socket_config cfg = {};
 	struct xsk_ring_cons *rxr;
 	struct xsk_ring_prod *txr;
+	int ret;
 
 	xsk->umem = umem;
 	cfg.rx_size = xsk->rxqsize;
@@ -170,7 +172,21 @@ int xsk_configure_socket(struct xsk_socket_info *xsk, struct xsk_umem_info *umem
 
 	txr = ifobject->tx_on ? &xsk->tx : NULL;
 	rxr = ifobject->rx_on ? &xsk->rx : NULL;
-	return xsk_socket__create(&xsk->xsk, ifobject->ifindex, 0, umem->umem, rxr, txr, &cfg);
+	ret = xsk_socket__create(&xsk->xsk, ifobject->ifindex, 0, umem->umem, rxr, txr, &cfg);
+	if (ret) {
+		if (shared)
+			/* Shared socket failed before acquiring a UMEM reference. */
+			xsk->umem = NULL;
+
+		/* Keep failed socket state inert for later cleanup paths. */
+		xsk->xsk = NULL;
+		return ret;
+	}
+
+	if (shared)
+		refcount_inc(&umem->users);
+
+	return ret;
 }
 
 static int set_ring_size(struct ifobject *ifobj)
@@ -1702,12 +1718,71 @@ void *worker_testapp_validate_rx(void *arg)
 	pthread_exit(NULL);
 }
 
-static void testapp_clean_xsk_umem(struct ifobject *ifobj)
+void xsk_delete_socket(struct xsk_socket_info *xsk)
 {
-	struct xsk_umem_info *umem = ifobj->xsk->umem;
+	struct xsk_umem_info *umem;
+
+	/*
+	 * Callers must serialize teardown for a given xsk/umem pair.
+	 * This helper does refcount-based lifetime management only.
+	 */
+	if (!xsk)
+		return;
 
-	xsk_umem__delete(umem->umem);
-	munmap(umem->buffer, umem->mmap_size);
+	umem = xsk->umem;
+	if (!umem)
+		return;
+
+	/* Delete the socket handle when available, then drop UMEM reference. */
+	if (xsk->xsk)
+		xsk_socket__delete(xsk->xsk);
+	xsk->xsk = NULL;
+
+	/* Mark this socket as cleaned up to prevent double-decrement */
+	xsk->umem = NULL;
+
+	/*
+	 * Teardown may walk the full xsk array, including slots that never completed setup but
+	 * still carry a preinitialized umem pointer. After a partial-setup rollback, the last UMEM
+	 * ref may already be dropped.
+	 */
+	if (!refcount_read(&umem->users))
+		return;
+
+	/* Always decrement refcount for this socket's UMEM reference */
+	if (refcount_dec_and_test(&umem->users)) {
+		if (umem->umem) {
+			int err = xsk_umem__delete(umem->umem);
+
+			if (err)
+				ksft_print_msg("xsk_umem__delete failed: %d (umem still busy?)\n",
+					       err);
+		}
+		if (umem->buffer && umem->mmap_size)
+			munmap(umem->buffer, umem->mmap_size);
+		umem->umem = NULL;
+		umem->buffer = NULL;
+		umem->mmap_size = 0;
+	}
+}
+
+static void xsk_delete_socket_batch(struct ifobject *ifobject, u32 count)
+{
+	u32 i;
+
+	if (!ifobject)
+		return;
+
+	for (i = count; i > 0; i--)
+		xsk_delete_socket(&ifobject->xsk_arr[i - 1]);
+}
+
+static void xsk_delete_all_ifobj_sockets(struct test_spec *test, struct ifobject *ifobj)
+{
+	if (!ifobj)
+		return;
+
+	xsk_delete_socket_batch(ifobj, test->nb_sockets);
 }
 
 static bool xdp_prog_changed_rx(struct test_spec *test)
@@ -1769,27 +1844,6 @@ static int xsk_attach_xdp_progs(struct test_spec *test, struct ifobject *ifobj_r
 	return err;
 }
 
-static void clean_sockets(struct test_spec *test, struct ifobject *ifobj)
-{
-	u32 i;
-
-	if (!ifobj || !test)
-		return;
-
-	for (i = 0; i < test->nb_sockets; i++)
-		xsk_socket__delete(ifobj->xsk_arr[i].xsk);
-}
-
-static void clean_umem(struct test_spec *test, struct ifobject *ifobj1, struct ifobject *ifobj2)
-{
-	if (!ifobj1)
-		return;
-
-	testapp_clean_xsk_umem(ifobj1);
-	if (ifobj2 && !ifobj2->shared_umem)
-		testapp_clean_xsk_umem(ifobj2);
-}
-
 static int __testapp_validate_traffic(struct test_spec *test, struct ifobject *ifobj1,
 				      struct ifobject *ifobj2)
 {
@@ -1841,8 +1895,7 @@ static int __testapp_validate_traffic(struct test_spec *test, struct ifobject *i
 		if (pthread_barrier_destroy(&barr)) {
 			test->use_barrier = false;
 			pthread_join(t0, NULL);
-			clean_sockets(test, ifobj1);
-			clean_umem(test, ifobj1, NULL);
+			xsk_delete_all_ifobj_sockets(test, ifobj1);
 			return TEST_FAILURE;
 		}
 	}
@@ -1856,9 +1909,8 @@ static int __testapp_validate_traffic(struct test_spec *test, struct ifobject *i
 	pthread_join(t0, NULL);
 
 	if (test->total_steps == test->current_step || test->fail) {
-		clean_sockets(test, ifobj1);
-		clean_sockets(test, ifobj2);
-		clean_umem(test, ifobj1, ifobj2);
+		xsk_delete_all_ifobj_sockets(test, ifobj2);
+		xsk_delete_all_ifobj_sockets(test, ifobj1);
 	}
 
 	if (test->fail)
@@ -1967,9 +2019,8 @@ int testapp_xdp_prog_cleanup(struct test_spec *test)
 		return TEST_FAILURE;
 
 	if (swap_xsk_resources(test)) {
-		clean_sockets(test, test->ifobj_rx);
-		clean_sockets(test, test->ifobj_tx);
-		clean_umem(test, test->ifobj_rx, test->ifobj_tx);
+		xsk_delete_all_ifobj_sockets(test, test->ifobj_tx);
+		xsk_delete_all_ifobj_sockets(test, test->ifobj_rx);
 		return TEST_FAILURE;
 	}
 
@@ -2498,9 +2549,8 @@ int testapp_hw_sw_max_ring_size(struct test_spec *test)
 	test->ifobj_tx->xsk->batch_size = test->ifobj_tx->ring.tx_max_pending - 8;
 	test->ifobj_rx->xsk->batch_size = test->ifobj_tx->ring.tx_max_pending - 8;
 	if (pkt_stream_replace(test, max_descs, MIN_PKT_SIZE)) {
-		clean_sockets(test, test->ifobj_tx);
-		clean_sockets(test, test->ifobj_rx);
-		clean_umem(test, test->ifobj_rx, test->ifobj_tx);
+		xsk_delete_all_ifobj_sockets(test, test->ifobj_tx);
+		xsk_delete_all_ifobj_sockets(test, test->ifobj_rx);
 		return TEST_FAILURE;
 	}
 
diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.h b/tools/testing/selftests/bpf/prog_tests/test_xsk.h
index 03753ddc5dcd..56bc134505b3 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_xsk.h
+++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.h
@@ -4,6 +4,7 @@
 
 #include <linux/ethtool.h>
 #include <linux/if_xdp.h>
+#include <linux/refcount.h>
 
 #include "../kselftest.h"
 #include "xsk.h"
@@ -104,6 +105,7 @@ struct xsk_umem_info {
 	struct xsk_umem *umem;
 	u64 next_buffer;
 	u64 mmap_size;
+	refcount_t users;
 	u32 num_frames;
 	u32 frame_headroom;
 	void *buffer;
@@ -159,6 +161,7 @@ int init_iface(struct ifobject *ifobj, thread_func_t func_ptr);
 int xsk_configure_umem(struct ifobject *ifobj, struct xsk_umem_info *umem, void *buffer, u64 size);
 int xsk_configure_socket(struct xsk_socket_info *xsk, struct xsk_umem_info *umem,
 			 struct ifobject *ifobject, bool shared);
+void xsk_delete_socket(struct xsk_socket_info *xsk);
 
 
 struct pkt {
diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index 7dad8556a722..ed7716b63756 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -117,12 +117,13 @@ static void __exit_with_error(int error, const char *file, const char *func, int
 
 #define exit_with_error(error) __exit_with_error(error, __FILE__, __func__, __LINE__)
 
-static bool ifobj_zc_avail(struct ifobject *ifobject)
+static bool ifobj_zc_avail(struct ifobject *ifobj)
 {
 	size_t umem_sz = DEFAULT_UMEM_BUFFERS * XSK_UMEM__DEFAULT_FRAME_SIZE;
 	int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE;
-	struct xsk_socket_info *xsk;
-	struct xsk_umem_info *umem;
+	struct xsk_socket_info xsk = {};
+	struct xsk_umem_info umem_info = {};
+	struct xsk_umem_info *umem = &umem_info;
 	bool zc_avail = false;
 	void *bufs;
 	int ret;
@@ -131,32 +132,22 @@ static bool ifobj_zc_avail(struct ifobject *ifobject)
 	if (bufs == MAP_FAILED)
 		exit_with_error(errno);
 
-	umem = calloc(1, sizeof(struct xsk_umem_info));
-	if (!umem) {
-		munmap(bufs, umem_sz);
-		exit_with_error(ENOMEM);
-	}
+	umem->mmap_size = umem_sz;
 	umem->frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE;
-	ret = xsk_configure_umem(ifobject, umem, bufs, umem_sz);
+	ret = xsk_configure_umem(ifobj, umem, bufs, umem_sz);
 	if (ret)
 		exit_with_error(-ret);
 
-	xsk = calloc(1, sizeof(struct xsk_socket_info));
-	if (!xsk)
-		goto out;
-	ifobject->bind_flags = XDP_USE_NEED_WAKEUP | XDP_ZEROCOPY;
-	ifobject->rx_on = true;
-	xsk->rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
-	ret = xsk_configure_socket(xsk, umem, ifobject, false);
+	ifobj->bind_flags = XDP_USE_NEED_WAKEUP | XDP_ZEROCOPY;
+	ifobj->rx_on = true;
+	xsk.rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
+	ret = xsk_configure_socket(&xsk, umem, ifobj, false);
 	if (!ret)
 		zc_avail = true;
 
-	xsk_socket__delete(xsk->xsk);
-	free(xsk);
-out:
-	munmap(umem->buffer, umem_sz);
-	xsk_umem__delete(umem->umem);
-	free(umem);
+	/* Use the same refcount-based teardown path for both success and failure. */
+	xsk_delete_socket(&xsk);
+
 	return zc_avail;
 }
 
-- 
2.43.0

Re: [PATCH net-next 1/5] selftests/xsk: add UMEM users refcount and centralize socket teardown

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Premature UMEM destruction during batch teardown of partially configured socket arrays causes `xsk_umem` file descriptor and memory leaks.
--

--- Patch [1]: [PATCH net-next 1/5] selftests/xsk: add UMEM users refcount and centralize socket teardown ---
commit 99dd730a09377ba26c180094a39cc7721ae72648
Author: Tushar Vyavahare <tushar.vyavahare@intel.com>

selftests/xsk: add UMEM users refcount and centralize socket teardown

This commit introduces a reference count for shared UMEM users in the XSK
selftests and centralizes socket teardown in xsk_delete_socket(). It
replaces manual UMEM teardown functions with a unified batch deletion
approach to avoid leaks or double-releases during error handling.

> diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> index 6eb9096d084c0..43ab61faad024 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
[ ... ]
> @@ -1702,12 +1718,71 @@ void *worker_testapp_validate_rx(void *arg)
>  	pthread_exit(NULL);
>  }
>  
> -static void testapp_clean_xsk_umem(struct ifobject *ifobj)
> +void xsk_delete_socket(struct xsk_socket_info *xsk)
>  {
> -	struct xsk_umem_info *umem = ifobj->xsk->umem;
> +	struct xsk_umem_info *umem;
[ ... ]
> +	/* Always decrement refcount for this socket's UMEM reference */
> +	if (refcount_dec_and_test(&umem->users)) {
> +		if (umem->umem) {
> +			int err = xsk_umem__delete(umem->umem);
> +
> +			if (err)
> +				ksft_print_msg("xsk_umem__delete failed: %d (umem still busy?)\n",
> +					       err);
> +		}
> +		if (umem->buffer && umem->mmap_size)
> +			munmap(umem->buffer, umem->mmap_size);
> +		umem->umem = NULL;
> +		umem->buffer = NULL;
> +		umem->mmap_size = 0;
> +	}
> +}
> +
> +static void xsk_delete_socket_batch(struct ifobject *ifobject, u32 count)
> +{
> +	u32 i;
> +
> +	if (!ifobject)
> +		return;
> +
> +	for (i = count; i > 0; i--)
> +		xsk_delete_socket(&ifobject->xsk_arr[i - 1]);
> +}

[Severity: Medium]
Could this result in a premature UMEM destruction and a subsequent file
descriptor leak during partial setup failures?

Since xsk->umem is pre-initialized for all array slots in
__test_spec_init(), uninitialized slots will still have a non-NULL
umem pointer. 

If setup fails halfway through (e.g., enable_busy_poll() fails for i = 0 in
xsk_configure()), the batch teardown iterates backwards from test->nb_sockets.
When it reaches an uninitialized slot (like i = 1), it will see the
pre-initialized umem pointer and decrement the refcount.

If this brings the refcount to 0, it prematurely attempts teardown. Since the
valid socket at i = 0 is still active, xsk_umem__delete() returns -EBUSY.
However, the code still proceeds to munmap the buffer and set umem->umem = NULL.

When the loop finally processes i = 0, the refcount is already 0, so it skips
calling xsk_umem__delete() entirely, permanently leaking the UMEM and its
file descriptor.

Is there a way to distinguish between a pre-initialized slot that never took a
reference and a successfully configured slot?

> +
> +static void xsk_delete_all_ifobj_sockets(struct test_spec *test, struct ifobject *ifobj)
> +{
> +	if (!ifobj)
> +		return;
> +
> +	xsk_delete_socket_batch(ifobj, test->nb_sockets);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807134209.3794735-1-tushar.vyavahare@intel.com?part=1

Re: [PATCH net-next 1/5] selftests/xsk: add UMEM users refcount and centralize socket teardown

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 5629 bytes --]

> diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c

[ ... ]

> @@ -1701,12 +1717,71 @@ void *worker_testapp_validate_rx(void *arg)
>  	pthread_exit(NULL);
>  }
>
> -static void testapp_clean_xsk_umem(struct ifobject *ifobj)
> +void xsk_delete_socket(struct xsk_socket_info *xsk)
>  {
> -	struct xsk_umem_info *umem = ifobj->xsk->umem;
> +	struct xsk_umem_info *umem;
> +
> +	/*
> +	 * Callers must serialize teardown for a given xsk/umem pair.
> +	 * This helper does refcount-based lifetime management only.
> +	 */
> +	if (!xsk)
> +		return;
>
> -	xsk_umem__delete(umem->umem);
> -	munmap(umem->buffer, umem->mmap_size);
> +	umem = xsk->umem;
> +	if (!umem)
> +		return;
> +
> +	/* Delete the socket handle when available, then drop UMEM reference. */
> +	if (xsk->xsk)
> +		xsk_socket__delete(xsk->xsk);
> +	xsk->xsk = NULL;
> +
> +	/* Mark this socket as cleaned up to prevent double-decrement */
> +	xsk->umem = NULL;
> +
> +	/*
> +	 * Teardown may walk the full xsk array, including slots that never completed setup but
> +	 * still carry a preinitialized umem pointer. After a partial-setup rollback, the last UMEM
> +	 * ref may already be dropped.
> +	 */
> +	if (!refcount_read(&umem->users))
> +		return;
> +
> +	/* Always decrement refcount for this socket's UMEM reference */

This isn't a bug, but does this comment still describe the code below it?
The early return just above it skips the decrement when users is already
zero, and three lines down the comment explains why that can happen, so
"Always" might not be the right word here.

Also, could the other comment blocks be trimmed? The four comments in this
roughly 50-line function mostly restate what the line below them does
("Callers must serialize" above an if (!xsk) check, "Delete the socket
handle when available", "Mark this socket as cleaned up"), and the
neighbouring helpers in this file carry no comments at all. The one
non-obvious point (why a slot can be reached with users already at 0)
might be enough on its own.

> +	if (refcount_dec_and_test(&umem->users)) {
> +		if (umem->umem) {
> +			int err = xsk_umem__delete(umem->umem);
> +
> +			if (err)
> +				ksft_print_msg("xsk_umem__delete failed: %d (umem still busy?)\n",
> +					       err);
> +		}
> +		if (umem->buffer && umem->mmap_size)
> +			munmap(umem->buffer, umem->mmap_size);
> +		umem->umem = NULL;
> +		umem->buffer = NULL;
> +		umem->mmap_size = 0;
> +	}
> +}

[ ... ]

> diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c

[ ... ]

> @@ -117,12 +117,13 @@ static void __exit_with_error(int error, const char *file, const char *func, int
>
>  #define exit_with_error(error) __exit_with_error(error, __FILE__, __func__, __LINE__)
>
> -static bool ifobj_zc_avail(struct ifobject *ifobject)
> +static bool ifobj_zc_avail(struct ifobject *ifobj)
>  {
>  	size_t umem_sz = DEFAULT_UMEM_BUFFERS * XSK_UMEM__DEFAULT_FRAME_SIZE;
>  	int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE;
> -	struct xsk_socket_info *xsk;
> -	struct xsk_umem_info *umem;
> +	struct xsk_socket_info xsk = {};
> +	struct xsk_umem_info umem_info = {};
> +	struct xsk_umem_info *umem = &umem_info;

This isn't a bug, but would declaring struct xsk_umem_info umem = {}; and
passing &umem directly read a bit more plainly here? The conversion to
stack allocation introduces umem_info for the storage and umem as a
pointer alias on the next line, but the alias is only used four times
(umem->mmap_size, umem->frame_size, and passing umem to two functions),
three of which the patch already rewrites. This struct T *p = &local;
pattern doesn't appear anywhere else in xskxceiver.c, test_xsk.c, or
xsk.c, so it's not following a local idiom.

>  	bool zc_avail = false;
>  	void *bufs;
>  	int ret;

[ ... ]

Commit message question:

>     selftests/xsk: add UMEM users refcount and centralize socket teardown
>
>     Shared-UMEM lifetime can be torn down from multiple setup and cleanup
>     paths. Mixing manual UMEM free/unmap in one path with helper-based
>     teardown in others makes ownership easier to violate and can leak or
>     double-release UMEM during error handling.
>
>     Track UMEM ownership with a refcount_t users field. Initialize it to 1
>     in xsk_configure_umem() and increment it for each additional shared
>     socket. Introduce xsk_delete_socket() as the single teardown helper: it
>     deletes the socket handle, clears state to prevent double-decrement, and
>     releases the UMEM when the last reference drops.
>
>     Replace testapp_clean_xsk_umem(), clean_sockets(), and clean_umem() with
>     xsk_delete_socket_batch() and xsk_delete_all_ifobj_sockets() wrappers at
>     all call sites.

This isn't a bug, but would it help to explain why this refactor is
needed rather than walking through what the diff does? Two of the three
paragraphs describe which function sets the counter to 1, which helper is
introduced, and which three helpers are replaced by which two wrappers.
The opening paragraph states that mixing manual and helper-based teardown
"can leak or double-release UMEM during error handling" but doesn't name
a concrete path, and there's no Fixes: tag, so it's not clear whether an
existing bug is being fixed or the change is preparatory for the
shared-UMEM tests added later in the series.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31554903166

[PATCH net-next 2/5] selftests/xsk: roll back partial socket setup on configure failures

[Tushar Vyavahare]

When xsk_socket__create() fails after all retries, or when busy-poll
setup fails after the socket is created, already-configured sockets for
the same ifobject are leaked.

Add xsk_configure_rollback() to call xsk_delete_socket_batch() on all
sockets configured so far, and wire it into both failure paths in
xsk_configure().

As part of this change, move xsk_delete_socket_batch() next to
xsk_configure_rollback() so rollback helpers are grouped with
xsk_configure()-local failure handling.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
---
 .../selftests/bpf/prog_tests/test_xsk.c       | 41 +++++++++++++------
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
index 4ccdb0825130..de0d8f51f846 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
@@ -1484,6 +1484,28 @@ static int validate_tx_invalid_descs(struct ifobject *ifobject)
 	return TEST_PASS;
 }
 
+static void xsk_delete_socket_batch(struct ifobject *ifobject, u32 count)
+{
+	u32 i;
+
+	if (!ifobject)
+		return;
+
+	for (i = count; i > 0; i--)
+		xsk_delete_socket(&ifobject->xsk_arr[i - 1]);
+}
+
+static void xsk_configure_rollback(struct ifobject *ifobject, int last)
+{
+	u32 count;
+
+	if (!ifobject || last < 0)
+		return;
+
+	count = last + 1;
+	xsk_delete_socket_batch(ifobject, count);
+}
+
 static int xsk_configure(struct test_spec *test, struct ifobject *ifobject,
 			  struct xsk_umem_info *umem, bool tx)
 {
@@ -1500,14 +1522,18 @@ static int xsk_configure(struct test_spec *test, struct ifobject *ifobject,
 				break;
 
 			/* Retry if it fails as xsk_socket__create() is asynchronous */
-			if (ctr >= SOCK_RECONF_CTR)
+			if (ctr >= SOCK_RECONF_CTR) {
+				xsk_configure_rollback(ifobject, i);
 				return ret;
+			}
 			usleep(USLEEP_MAX);
 		}
 		if (ifobject->busy_poll) {
 			ret = enable_busy_poll(&ifobject->xsk_arr[i]);
-			if (ret)
+			if (ret) {
+				xsk_configure_rollback(ifobject, i);
 				return ret;
+			}
 		}
 	}
 
@@ -1766,17 +1792,6 @@ void xsk_delete_socket(struct xsk_socket_info *xsk)
 	}
 }
 
-static void xsk_delete_socket_batch(struct ifobject *ifobject, u32 count)
-{
-	u32 i;
-
-	if (!ifobject)
-		return;
-
-	for (i = count; i > 0; i--)
-		xsk_delete_socket(&ifobject->xsk_arr[i - 1]);
-}
-
 static void xsk_delete_all_ifobj_sockets(struct test_spec *test, struct ifobject *ifobj)
 {
 	if (!ifobj)
-- 
2.43.0

Re: [PATCH net-next 2/5] selftests/xsk: roll back partial socket setup on configure failures

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Eager rollback in xsk_configure() causes a guaranteed NULL pointer dereference in the parallel TX thread when the RX thread fails.
--

--- Patch [2]: [PATCH net-next 2/5] selftests/xsk: roll back partial socket setup on configure failures ---
commit 80baccb88ac5eb975506a8efbd37137e2c6775b1
Author: Tushar Vyavahare <tushar.vyavahare@intel.com>

selftests/xsk: roll back partial socket setup on configure failures

This commit introduces an eager rollback mechanism to clean up already
configured sockets if subsequent setups or busy-poll configurations fail
during xsk_configure().

> diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> --- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
[ ... ]
> @@ -1500,14 +1522,18 @@ static int xsk_configure(struct test_spec *test, struct ifobject *ifobject,
>  				break;
>  
>  			/* Retry if it fails as xsk_socket__create() is asynchronous */
> -			if (ctr >= SOCK_RECONF_CTR)
> +			if (ctr >= SOCK_RECONF_CTR) {
> +				xsk_configure_rollback(ifobject, i);
>  				return ret;
> +			}

[Severity: Medium]
Could this new rollback cause a NULL pointer dereference in the parallel TX
thread when an RX thread setup fails?

When xsk_configure_rollback() cleans up the sockets on a failure, it calls
xsk_delete_socket_batch(), which in turn calls xsk_delete_socket(). That
function explicitly clears the umem pointer for the RX thread's sockets:

xsk_delete_socket() {
    ...
    xsk->umem = NULL;
}

If the RX thread fails and exits, the main test thread does not check
test->fail before spawning the TX thread. The TX thread then executes
thread_common_ops_tx(), which unconditionally dereferences the now-NULL
umem pointer:

thread_common_ops_tx() {
    ...
    if (!test->ifobj_rx || !test->ifobj_rx->xsk_arr[0].umem->umem) {
    ...
}

Does the main thread need to check for test failures before spawning the TX
thread, or should thread_common_ops_tx() verify xsk_arr[0].umem is valid
before dereferencing it?

>  			usleep(USLEEP_MAX);
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807134209.3794735-1-tushar.vyavahare@intel.com?part=2

Re: [PATCH net-next 2/5] selftests/xsk: roll back partial socket setup on configure failures

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1624 bytes --]

> commit 4ffc9af96ac7d98a2901bb6fe522a2076e0c73b4
> Author: Tushar Vyavahare <tushar.vyavahare@intel.com>
>
>     selftests/xsk: roll back partial socket setup on configure failures
>
>     When xsk_socket__create() fails after all retries, or when busy-poll
>     setup fails after the socket is created, already-configured sockets for
>     the same ifobject are leaked.

Could you clarify which failure path causes the leak described here?

When xsk_configure() fails, the worker thread sets test->fail:

worker_testapp_validate_rx() at line 1719-1722:
    if (err) {
        test->fail = true;
        pthread_exit(NULL);
    }

worker_testapp_validate_tx() at line 1676-1686:
    if (thread_common_ops(...)) {
        test->fail = true;
        pthread_exit(NULL);
    }

Then __testapp_validate_traffic() tears down both ifobjects after joining
the threads:

test_xsk.c:__testapp_validate_traffic() {
    if (test->total_steps == test->current_step || test->fail) {
        xsk_delete_all_ifobj_sockets(test, ifobj2);
        xsk_delete_all_ifobj_sockets(test, ifobj1);
    }
}

Since xsk_delete_all_ifobj_sockets() walks slots 0..nb_sockets-1 (the same
range xsk_configure() iterates), the already-configured sockets should be
deleted there.

If there's a failure path that escapes this teardown, naming it in the
commit message would help clarify why the rollback is needed.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31554903166

[PATCH net-next 3/5] selftests/xsk: expand XSKMAP capacity and add length-based XDP program

[Tushar Vyavahare]

Raise MAX_SOCKETS from 2 to 4 so the XSKMAP and test arrays can
accommodate the upcoming 4-socket shared-UMEM test. Update the XSKMAP
max_entries to use MAX_SOCKETS so the BPF and C sides stay in sync.

Add xsk_xdp_shared_umem_length_based() XDP program that routes packets
by total XDP-visible packet length (data_end - data): socket 0 for
packets <= SHARED_UMEM_LEN_SPLIT bytes and socket 1 for packets >
SHARED_UMEM_LEN_SPLIT bytes. Define SHARED_UMEM_LEN_SPLIT as 64 to avoid
hardcoded length thresholds in the classifier.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
---
 .../selftests/bpf/progs/xsk_xdp_progs.c       | 19 ++++++++++++++++++-
 tools/testing/selftests/bpf/xsk_xdp_common.h  |  3 ++-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c b/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
index 023d8befd4ca..543b61df3a2b 100644
--- a/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
+++ b/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
@@ -10,7 +10,7 @@
 
 struct {
 	__uint(type, BPF_MAP_TYPE_XSKMAP);
-	__uint(max_entries, 2);
+	__uint(max_entries, MAX_SOCKETS);
 	__uint(key_size, sizeof(int));
 	__uint(value_size, sizeof(int));
 } xsk SEC(".maps");
@@ -75,6 +75,23 @@ SEC("xdp") int xsk_xdp_shared_umem(struct xdp_md *xdp)
 	return bpf_redirect_map(&xsk, idx, XDP_DROP);
 }
 
+SEC("xdp") int xsk_xdp_shared_umem_length_based(struct xdp_md *xdp)
+{
+	void *data = (void *)(long)xdp->data;
+	void *data_end = (void *)(long)xdp->data_end;
+	__u32 pkt_len = data_end - data;
+
+	/* 
+	 * Route packets by total XDP-visible packet length (data_end - data):
+	 * - Socket 0: packets <= SHARED_UMEM_LEN_SPLIT bytes
+	 * - Socket 1: packets > SHARED_UMEM_LEN_SPLIT bytes
+	 */
+	if (pkt_len <= SHARED_UMEM_LEN_SPLIT)
+		return bpf_redirect_map(&xsk, 0, XDP_DROP);
+	else
+		return bpf_redirect_map(&xsk, 1, XDP_DROP);
+}
+
 SEC("xdp.frags") int xsk_xdp_adjust_tail(struct xdp_md *xdp)
 {
 	__u32 buff_len, curr_buff_len;
diff --git a/tools/testing/selftests/bpf/xsk_xdp_common.h b/tools/testing/selftests/bpf/xsk_xdp_common.h
index 45810ff552da..bd9b5dcf8c8e 100644
--- a/tools/testing/selftests/bpf/xsk_xdp_common.h
+++ b/tools/testing/selftests/bpf/xsk_xdp_common.h
@@ -3,7 +3,8 @@
 #ifndef XSK_XDP_COMMON_H_
 #define XSK_XDP_COMMON_H_
 
-#define MAX_SOCKETS 2
+#define MAX_SOCKETS 4
+#define SHARED_UMEM_LEN_SPLIT 64
 #define PKT_HDR_ALIGN (sizeof(struct ethhdr) + 2) /* Just to align the data in the packet */
 
 struct xdp_info {
-- 
2.43.0

Re: [PATCH net-next 3/5] selftests/xsk: expand XSKMAP capacity and add length-based XDP program

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2178 bytes --]

> diff --git a/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c b/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
> index 023d8befd4cab..543b61df3a2b1 100644
> --- a/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
> +++ b/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c

[ ... ]

> @@ -75,6 +75,23 @@ SEC("xdp") int xsk_xdp_shared_umem(struct xdp_md *xdp)
>  	return bpf_redirect_map(&xsk, idx, XDP_DROP);
>  }
>
> +SEC("xdp") int xsk_xdp_shared_umem_length_based(struct xdp_md *xdp)
> +{
> +	void *data = (void *)(long)xdp->data;
> +	void *data_end = (void *)(long)xdp->data_end;
> +	__u32 pkt_len = data_end - data;
> +
> +	/*
> +	 * Route packets by total XDP-visible packet length (data_end - data):
> +	 * - Socket 0: packets <= SHARED_UMEM_LEN_SPLIT bytes
> +	 * - Socket 1: packets > SHARED_UMEM_LEN_SPLIT bytes
> +	 */
> +	if (pkt_len <= SHARED_UMEM_LEN_SPLIT)
> +		return bpf_redirect_map(&xsk, 0, XDP_DROP);
> +	else
> +		return bpf_redirect_map(&xsk, 1, XDP_DROP);
> +}

This isn't a bug, but could this comment be trimmed to a single line
like the one in xsk_xdp_shared_umem() above, and the trailing space
after the opening '/*' dropped so checkpatch stays quiet?

> diff --git a/tools/testing/selftests/bpf/xsk_xdp_common.h b/tools/testing/selftests/bpf/xsk_xdp_common.h
> index 45810ff552dae..bd9b5dcf8c8ee 100644
> --- a/tools/testing/selftests/bpf/xsk_xdp_common.h
> +++ b/tools/testing/selftests/bpf/xsk_xdp_common.h
> @@ -3,7 +3,8 @@
>  #ifndef XSK_XDP_COMMON_H_
>  #define XSK_XDP_COMMON_H_
>
> -#define MAX_SOCKETS 2
> +#define MAX_SOCKETS 4
> +#define SHARED_UMEM_LEN_SPLIT 64
>  #define PKT_HDR_ALIGN (sizeof(struct ethhdr) + 2) /* Just to align the data in the packet */

This isn't a bug, but since xsk_xdp_common.h is visible to test_xsk.c
too, would it be worth deriving the test's short/long lengths from
SHARED_UMEM_LEN_SPLIT (or noting why 64 is the right split) so the two
sides cannot drift apart?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31554903166

[PATCH net-next 4/5] selftests/xsk: add shared-UMEM callback framework and initial test cases

[Tushar Vyavahare]

Add the runner callback infrastructure and the first four shared-UMEM
test cases together so every SHA in the series builds clean.

Framework:
- shared_default in __test_spec_init() derives the shared_umem flag from
  ifindex equality, matching xskxceiver startup behavior so test resets
  stay aligned with the harness invariant.
- pkt_stream_len_seq() generates per-socket streams with alternating
  short/long packet sizes.
- pkt_stream_weighted_uneven_dist_sequence() generates unequal-volume
  streams across two sockets.
- run_shared_umem_test() is a common runner: sets up the XDP program,
  validates streams are ready, runs the sequence callback, executes
  traffic, and calls an optional post-run callback.
- Callback types shared_umem_seq_fn and shared_umem_post_fn, and context
  structs shared_umem_len_ctx and shared_umem_uneven_dist_ctx are defined
  in test_xsk.h for shared use.

Tests:
- SHARED_UMEM_4_SOCKETS: runs four sockets simultaneously with an
  even/odd packet stream split across sockets.
- SHARED_UMEM_LENGTH_BASED: routes short packets to socket 0 and longer
  packets to socket 1 using the length-based XDP program.
- SHARED_UMEM_UNEVEN_DIST: generates a 1:3 volume split across two
  sockets and validates that socket 1 receives more packets.
- SHARED_UMEM_UNALIGNED: runs the even/odd split in unaligned chunk mode.

The unaligned test snapshots and restores prior alignment flags with
NULL-safe UMEM guards so it does not depend on test-order side effects
or implicit initialization assumptions in the callback.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
---
 .../selftests/bpf/prog_tests/test_xsk.c       | 250 ++++++++++++++++++
 .../selftests/bpf/prog_tests/test_xsk.h       |  21 ++
 2 files changed, 271 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
index de0d8f51f846..1665339844b5 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
@@ -219,6 +219,12 @@ int hw_ring_size_reset(struct ifobject *ifobj)
 static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 			     struct ifobject *ifobj_rx)
 {
+	/*
+	 * Keep the same default as xskxceiver startup: when TX and RX share the same netdev,
+	 * shared UMEM is the baseline mode for this test harness. Individual tests can still
+	 * override this as needed.
+	 */
+	bool shared_default = ifobj_tx->ifindex == ifobj_rx->ifindex;
 	u32 i, j;
 
 	for (i = 0; i < MAX_INTERFACES; i++) {
@@ -230,6 +236,7 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 		ifobj->use_fill_ring = true;
 		ifobj->release_rx = true;
 		ifobj->validation_func = NULL;
+		ifobj->shared_umem = shared_default;
 		ifobj->use_metadata = false;
 
 		if (i == 0) {
@@ -612,6 +619,87 @@ static int pkt_stream_even_odd_sequence(struct test_spec *test)
 	return 0;
 }
 
+static int pkt_stream_len_seq(struct test_spec *test, u32 short_len, u32 long_len)
+{
+	struct pkt_stream *tx_streams[MAX_SOCKETS] = {};
+	struct pkt_stream *rx_streams[MAX_SOCKETS] = {};
+	struct pkt_stream *pkt_stream;
+	u32 i;
+
+	for (i = 0; i < test->nb_sockets; i++) {
+		u32 pkt_len = i ? long_len : short_len;
+
+		pkt_stream = test->ifobj_tx->xsk_arr[i].pkt_stream;
+		tx_streams[i] = __pkt_stream_generate(pkt_stream->nb_pkts / 2, pkt_len, i, 2);
+		if (!tx_streams[i])
+			goto err;
+
+		pkt_stream = test->ifobj_rx->xsk_arr[i].pkt_stream;
+		rx_streams[i] = __pkt_stream_generate(pkt_stream->nb_pkts / 2, pkt_len, i, 2);
+		if (!rx_streams[i])
+			goto err;
+	}
+
+	for (i = 0; i < test->nb_sockets; i++) {
+		test->ifobj_tx->xsk_arr[i].pkt_stream = tx_streams[i];
+		test->ifobj_rx->xsk_arr[i].pkt_stream = rx_streams[i];
+	}
+
+	return 0;
+
+err:
+	for (i = 0; i < test->nb_sockets; i++) {
+		if (tx_streams[i])
+			pkt_stream_delete(tx_streams[i]);
+		if (rx_streams[i])
+			pkt_stream_delete(rx_streams[i]);
+	}
+
+	return -ENOMEM;
+}
+
+static int pkt_stream_weighted_uneven_dist_sequence(struct test_spec *test, u32 total_pkts,
+						    u32 pkt_len)
+{
+	struct pkt_stream *tx_streams[MAX_SOCKETS] = {};
+	struct pkt_stream *rx_streams[MAX_SOCKETS] = {};
+	u32 i, pkts_sock0;
+
+	if (test->nb_sockets < 2 || total_pkts < 4)
+		return -EINVAL;
+
+	pkts_sock0 = total_pkts / 4;
+
+	for (i = 0; i < test->nb_sockets; i++) {
+		u32 nb_pkts = (i == 0) ? pkts_sock0 : (total_pkts - pkts_sock0);
+
+		tx_streams[i] = __pkt_stream_generate(nb_pkts, pkt_len, i, 2);
+		if (!tx_streams[i])
+			goto err;
+
+		rx_streams[i] = __pkt_stream_generate(nb_pkts, pkt_len, i, 2);
+		if (!rx_streams[i])
+			goto err;
+	}
+
+	for (i = 0; i < test->nb_sockets; i++) {
+		test->ifobj_tx->xsk_arr[i].pkt_stream = tx_streams[i];
+		test->ifobj_rx->xsk_arr[i].pkt_stream = rx_streams[i];
+	}
+
+	return 0;
+
+err:
+	for (i = 0; i < test->nb_sockets; i++) {
+		if (tx_streams[i])
+			pkt_stream_delete(tx_streams[i]);
+		if (rx_streams[i])
+			pkt_stream_delete(rx_streams[i]);
+	}
+
+	return -ENOMEM;
+}
+
 static void release_even_odd_sequence(struct test_spec *test)
 {
 	struct pkt_stream *later_free_tx = test->ifobj_tx->xsk->pkt_stream;
@@ -2290,6 +2378,168 @@ int testapp_xdp_shared_umem(struct test_spec *test)
 	return ret;
 }
 
+static int shared_umem_test_prepare(struct test_spec *test)
+{
+	u32 i;
+
+	if (test->nb_sockets > MAX_SOCKETS) {
+		ksft_print_msg("ERROR: [%s] invalid socket count %u\n", __func__, test->nb_sockets);
+		return TEST_FAILURE;
+	}
+
+	for (i = 0; i < test->nb_sockets; i++) {
+		if (!test->ifobj_rx->xsk_arr[i].pkt_stream ||
+		    !test->ifobj_tx->xsk_arr[i].pkt_stream) {
+			ksft_print_msg("ERROR: [%s] missing stream for socket %u\n", __func__, i);
+			return TEST_FAILURE;
+		}
+	}
+
+	return TEST_PASS;
+}
+
+static int shared_umem_seq_even_odd(struct test_spec *test, const void *ctx)
+{
+	(void)ctx;
+
+	return pkt_stream_even_odd_sequence(test) ? TEST_FAILURE : TEST_PASS;
+}
+
+static int shared_umem_seq_len(struct test_spec *test, const void *ctx)
+{
+	const struct shared_umem_len_ctx *cfg = ctx;
+
+	return pkt_stream_len_seq(test, cfg->short_len, cfg->long_len) ? TEST_FAILURE : TEST_PASS;
+}
+
+static int shared_umem_seq_uneven_dist(struct test_spec *test, const void *ctx)
+{
+	const struct shared_umem_uneven_dist_ctx *cfg = ctx;
+
+	return pkt_stream_weighted_uneven_dist_sequence(test, cfg->total_pkts,
+						      cfg->pkt_len) ? TEST_FAILURE : TEST_PASS;
+}
+
+static int shared_umem_post_uneven_dist(struct test_spec *test, int ret, const void *ctx)
+{
+	struct pkt_stream *tx_stream_0, *tx_stream_1;
+	struct pkt_stream *rx_stream_0, *rx_stream_1;
+
+	(void)ctx;
+
+	tx_stream_0 = test->ifobj_tx->xsk_arr[0].pkt_stream;
+	tx_stream_1 = test->ifobj_tx->xsk_arr[1].pkt_stream;
+	rx_stream_0 = test->ifobj_rx->xsk_arr[0].pkt_stream;
+	rx_stream_1 = test->ifobj_rx->xsk_arr[1].pkt_stream;
+
+	if (tx_stream_1->nb_valid_entries <= tx_stream_0->nb_valid_entries)
+		return TEST_FAILURE;
+
+	if (!ret && rx_stream_1->nb_rx_pkts <= rx_stream_0->nb_rx_pkts) {
+		ksft_print_msg("ERROR: socket1 rx_pkts (%u) not greater than socket0 (%u)\n",
+			       rx_stream_1->nb_rx_pkts, rx_stream_0->nb_rx_pkts);
+		ret = TEST_FAILURE;
+	}
+
+	return ret;
+}
+
+static int run_shared_umem_test(struct test_spec *test, struct bpf_program *xdp_prog_rx,
+				struct bpf_program *xdp_prog_tx, struct bpf_map *xskmap_rx,
+				struct bpf_map *xskmap_tx, u32 nb_sockets,
+				shared_umem_seq_fn seq_fn, shared_umem_post_fn post_fn,
+				const void *ctx)
+{
+	int ret;
+
+	test->total_steps = 1;
+	test->nb_sockets = nb_sockets;
+
+	test_spec_set_xdp_prog(test, xdp_prog_rx, xdp_prog_tx, xskmap_rx, xskmap_tx);
+
+	ret = shared_umem_test_prepare(test);
+	if (ret)
+		return ret;
+
+	ret = seq_fn(test, ctx);
+	if (ret)
+		return ret;
+
+	ret = testapp_validate_traffic(test);
+	if (post_fn)
+		ret = post_fn(test, ret, ctx);
+
+	release_even_odd_sequence(test);
+
+	return ret;
+}
+
+int testapp_shared_umem_4_sockets(struct test_spec *test)
+{
+	struct xsk_xdp_progs *skel_rx = test->ifobj_rx->xdp_progs;
+	struct xsk_xdp_progs *skel_tx = test->ifobj_tx->xdp_progs;
+
+	return run_shared_umem_test(test, skel_rx->progs.xsk_xdp_shared_umem,
+				    skel_tx->progs.xsk_xdp_shared_umem, skel_rx->maps.xsk,
+				    skel_tx->maps.xsk, 4, shared_umem_seq_even_odd, NULL, NULL);
+}
+
+int testapp_shared_umem_length_based(struct test_spec *test)
+{
+	struct xsk_xdp_progs *skel_rx = test->ifobj_rx->xdp_progs;
+	struct xsk_xdp_progs *skel_tx = test->ifobj_tx->xdp_progs;
+	const struct shared_umem_len_ctx len_ctx = {
+		.short_len = MIN_PKT_SIZE,
+		.long_len = MIN_PKT_SIZE * 2,
+	};
+
+	return run_shared_umem_test(test, skel_rx->progs.xsk_xdp_shared_umem_length_based,
+				    skel_tx->progs.xsk_xdp_shared_umem_length_based,
+				    skel_rx->maps.xsk, skel_tx->maps.xsk, 2, shared_umem_seq_len,
+				    NULL, &len_ctx);
+}
+
+int testapp_shared_umem_uneven_dist(struct test_spec *test)
+{
+	struct xsk_xdp_progs *skel_rx = test->ifobj_rx->xdp_progs;
+	struct xsk_xdp_progs *skel_tx = test->ifobj_tx->xdp_progs;
+	const struct shared_umem_uneven_dist_ctx uneven_dist_ctx = {
+		.total_pkts = DEFAULT_PKT_CNT * 4,
+		.pkt_len = MIN_PKT_SIZE,
+	};
+
+	return run_shared_umem_test(test, skel_rx->progs.xsk_xdp_shared_umem,
+				    skel_tx->progs.xsk_xdp_shared_umem, skel_rx->maps.xsk,
+				    skel_tx->maps.xsk, 2, shared_umem_seq_uneven_dist,
+				    shared_umem_post_uneven_dist, &uneven_dist_ctx);
+}
+
+int testapp_shared_umem_unaligned(struct test_spec *test)
+{
+	struct xsk_xdp_progs *skel_rx = test->ifobj_rx->xdp_progs;
+	struct xsk_xdp_progs *skel_tx = test->ifobj_tx->xdp_progs;
+	struct xsk_umem_info *tx_umem = test->ifobj_tx && test->ifobj_tx->xsk ?
+		test->ifobj_tx->xsk->umem : NULL;
+	struct xsk_umem_info *rx_umem = test->ifobj_rx && test->ifobj_rx->xsk ?
+		test->ifobj_rx->xsk->umem : NULL;
+	bool tx_unaligned = tx_umem ? tx_umem->unaligned_mode : false;
+	bool rx_unaligned = rx_umem ? rx_umem->unaligned_mode : false;
+	int ret;
+
+	test_spec_set_unaligned(test);
+
+	ret = run_shared_umem_test(test, skel_rx->progs.xsk_xdp_shared_umem,
+				   skel_tx->progs.xsk_xdp_shared_umem, skel_rx->maps.xsk,
+				   skel_tx->maps.xsk, 2, shared_umem_seq_even_odd, NULL, NULL);
+
+	if (tx_umem)
+		tx_umem->unaligned_mode = tx_unaligned;
+	if (rx_umem)
+		rx_umem->unaligned_mode = rx_unaligned;
+
+	return ret;
+}
+
 int testapp_poll_txq_tmout(struct test_spec *test)
 {
 	bool shared_umem = test->ifobj_tx->shared_umem;
diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.h b/tools/testing/selftests/bpf/prog_tests/test_xsk.h
index 56bc134505b3..ef9c422d7763 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_xsk.h
+++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.h
@@ -80,6 +80,9 @@ struct test_spec;
 typedef int (*validation_func_t)(struct ifobject *ifobj);
 typedef void *(*thread_func_t)(void *arg);
 typedef int (*test_func_t)(struct test_spec *test);
+typedef int (*shared_umem_seq_fn)(struct test_spec *test, const void *ctx);
+typedef int (*shared_umem_post_fn)(struct test_spec *test, int ret,
+					   const void *ctx);
 
 struct xsk_socket_info {
 	struct xsk_ring_cons rx;
@@ -182,6 +185,16 @@ struct pkt_stream {
 	bool verbatim;
 };
 
+struct shared_umem_len_ctx {
+	u32 short_len;
+	u32 long_len;
+};
+
+struct shared_umem_uneven_dist_ctx {
+	u32 total_pkts;
+	u32 pkt_len;
+};
+
 static inline bool pkt_continues(u32 options)
 {
 	return options & XDP_PKT_CONTD;
@@ -271,6 +284,10 @@ int testapp_xdp_metadata(struct test_spec *test);
 int testapp_xdp_metadata_mb(struct test_spec *test);
 int testapp_xdp_prog_cleanup(struct test_spec *test);
 int testapp_xdp_shared_umem(struct test_spec *test);
+int testapp_shared_umem_4_sockets(struct test_spec *test);
+int testapp_shared_umem_length_based(struct test_spec *test);
+int testapp_shared_umem_uneven_dist(struct test_spec *test);
+int testapp_shared_umem_unaligned(struct test_spec *test);
 
 void *worker_testapp_validate_rx(void *arg);
 void *worker_testapp_validate_tx(void *arg);
@@ -294,6 +311,10 @@ static const struct test_spec tests[] = {
 	{.name = "XDP_PROG_CLEANUP", .test_func = testapp_xdp_prog_cleanup},
 	{.name = "XDP_DROP_HALF", .test_func = testapp_xdp_drop},
 	{.name = "XDP_SHARED_UMEM", .test_func = testapp_xdp_shared_umem},
+	{.name = "SHARED_UMEM_4_SOCKETS", .test_func = testapp_shared_umem_4_sockets},
+	{.name = "SHARED_UMEM_LENGTH_BASED", .test_func = testapp_shared_umem_length_based},
+	{.name = "SHARED_UMEM_UNEVEN_DIST", .test_func = testapp_shared_umem_uneven_dist},
+	{.name = "SHARED_UMEM_UNALIGNED", .test_func = testapp_shared_umem_unaligned},
 	{.name = "XDP_METADATA_COPY", .test_func = testapp_xdp_metadata},
 	{.name = "XDP_METADATA_COPY_MULTI_BUFF", .test_func = testapp_xdp_metadata_mb},
 	{.name = "ALIGNED_INV_DESC_MULTI_BUFF", .test_func = testapp_aligned_inv_desc_mb},
-- 
2.43.0

[PATCH net-next 5/5] selftests/xsk: make pkt_stream_even_odd_sequence rollback-safe

[Tushar Vyavahare]

If __pkt_stream_generate() fails midway through building per-socket
streams, the original code overwrites xsk_arr[i].pkt_stream with a
partial or NULL pointer before all allocations complete.

Allocate all TX and RX streams into temporary arrays first and only
assign them to xsk_arr after every allocation succeeds.  On any failure
free the already-allocated temporaries and return -ENOMEM with no
xsk_arr pointers modified.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>
---
 .../selftests/bpf/prog_tests/test_xsk.c       | 35 +++++++++++++------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
index 1665339844b5..ccfd9e8436a7 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
@@ -597,26 +597,41 @@ static int pkt_stream_receive_half(struct test_spec *test)
 
 static int pkt_stream_even_odd_sequence(struct test_spec *test)
 {
+	struct pkt_stream *tx_streams[MAX_SOCKETS] = {};
+	struct pkt_stream *rx_streams[MAX_SOCKETS] = {};
 	struct pkt_stream *pkt_stream;
 	u32 i;
 
 	for (i = 0; i < test->nb_sockets; i++) {
 		pkt_stream = test->ifobj_tx->xsk_arr[i].pkt_stream;
-		pkt_stream = __pkt_stream_generate(pkt_stream->nb_pkts / 2,
-						   pkt_stream->pkts[0].len, i, 2);
-		if (!pkt_stream)
-			return -ENOMEM;
-		test->ifobj_tx->xsk_arr[i].pkt_stream = pkt_stream;
+		tx_streams[i] = __pkt_stream_generate(pkt_stream->nb_pkts / 2,
+						      pkt_stream->pkts[0].len, i, 2);
+		if (!tx_streams[i])
+			goto err;
 
 		pkt_stream = test->ifobj_rx->xsk_arr[i].pkt_stream;
-		pkt_stream = __pkt_stream_generate(pkt_stream->nb_pkts / 2,
-						   pkt_stream->pkts[0].len, i, 2);
-		if (!pkt_stream)
-			return -ENOMEM;
-		test->ifobj_rx->xsk_arr[i].pkt_stream = pkt_stream;
+		rx_streams[i] = __pkt_stream_generate(pkt_stream->nb_pkts / 2,
+						      pkt_stream->pkts[0].len, i, 2);
+		if (!rx_streams[i])
+			goto err;
+	}
+
+	for (i = 0; i < test->nb_sockets; i++) {
+		test->ifobj_tx->xsk_arr[i].pkt_stream = tx_streams[i];
+		test->ifobj_rx->xsk_arr[i].pkt_stream = rx_streams[i];
 	}
 
 	return 0;
+
+err:
+	for (i = 0; i < test->nb_sockets; i++) {
+		if (tx_streams[i])
+			pkt_stream_delete(tx_streams[i]);
+		if (rx_streams[i])
+			pkt_stream_delete(rx_streams[i]);
+	}
+
+	return -ENOMEM;
 }
 
 static int pkt_stream_len_seq(struct test_spec *test, u32 short_len, u32 long_len)
-- 
2.43.0

Re: [PATCH net-next 5/5] selftests/xsk: make pkt_stream_even_odd_sequence rollback-safe

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1714 bytes --]

> If __pkt_stream_generate() fails midway through building per-socket
> streams, the original code overwrites xsk_arr[i].pkt_stream with a
> partial or NULL pointer before all allocations complete.
>
> Allocate all TX and RX streams into temporary arrays first and only
> assign them to xsk_arr after every allocation succeeds.  On any failure
> free the already-allocated temporaries and return -ENOMEM with no
> xsk_arr pointers modified.

Does the commit message accurately describe the problem being fixed?

The pre-patch code NULL-checked the allocation before assigning it to
xsk_arr[i].pkt_stream:

    pkt_stream = __pkt_stream_generate(pkt_stream->nb_pkts / 2,
                                       pkt_stream->pkts[0].len, i, 2);
    if (!pkt_stream)
            return -ENOMEM;
    test->ifobj_tx->xsk_arr[i].pkt_stream = pkt_stream;

so xsk_arr[i].pkt_stream would never hold a NULL pointer from a failed
allocation. __pkt_stream_generate() either returns a fully initialized
stream or NULL (it frees any partial allocations on failure), so no
"partial" pointer can exist either.

The actual bug appears to be different: when an allocation for socket i
fails, sockets 0..i-1 already have new streams installed in xsk_arr,
and both callers (testapp_xdp_shared_umem() and run_shared_umem_test())
return immediately on error without calling release_even_odd_sequence().
This leaks the already-published streams and leaves xsk_arr with a
half-replaced stream set.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31554903166