Re: [PATCH net-next v3 1/4] selftests/xsk: add UMEM users refcount and centralize socket teardown
From: bot+bpf-ci@kernel.org
Date: 2026-08-20 18:42:31
Also in:
bpf
commit 90d52cd76f2fe834a2cca6abb76d0ae53c96b8e5
Author: Tushar Vyavahare [off-list ref]
Date: Wed Aug 19 02:44:40 2026 +0000
selftests/xsk: add UMEM users refcount and centralize socket teardown
UMEM ownership is currently implicit. xsk_configure_umem() maps the
buffer, but freeing it is spread across testapp_clean_xsk_umem(),
clean_sockets() and clean_umem(), each called from a different error path.
thread_common_ops() returns without releasing the UMEM when
xsk_configure() fails after the UMEM was already mapped, leaking the
mmap()ed buffer.
The pthread_barrier_destroy() failure path in __testapp_validate_traffic()
unmaps the UMEM owned by ifobj1 while sockets belonging to ifobj2 are left
alive. With shared_umem, those sockets reference the buffer that was just
unmapped.
Make ownership explicit instead. Add a refcount_t users field to struct
xsk_umem_info and release exactly one reference per socket on teardown, so
that the last socket to go away frees the UMEM regardless of which path
tears it down. Keep shared sockets pointing to the same xsk_umem_info
instead of copying ownership state, so all users update the same reference
count.
Propagate UMEM deletion errors from xsk_delete_socket() so callers with
stack-allocated state can handle a deferred cleanup before that state goes
out of scope. The in-test teardown paths cannot retry, because
__test_spec_init() clears the UMEM state at the start of the next test and
the mapping is then unrecoverable, so xsk_delete_all_ifobj_sockets()
records the failure in test->fail instead of dropping it. A UMEM that
survives teardown now fails the test rather than leaking silently.
clean_umem() skipped ifobj2 whenever it shared the UMEM owned by ifobj1,
which was only there to avoid unmapping the same buffer twice. The
reference count makes that distinction unnecessary, so the helper and its
special case are removed along with the other ad-hoc cleanup paths.
Co-developed-by: Magnus Karlsson [off-list ref]
Signed-off-by: Magnus Karlsson [off-list ref]
Signed-off-by: Tushar Vyavahare [off-list ref]
This looks like a bug fix for the shared UMEM feature. Should this
include:
Fixes: 6d198a89c004 ("selftests/xsk: Add a test for shared umem feature")
quoted hunk ↗ jump to hunk
diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c index 4549358cc8c2..bba5e8d4222d 100644 --- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c +++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1508,8 +1529,15 @@ static int thread_common_ops_tx(struct test_spec *test, struct ifobject *ifobjec } umem_rx = test->ifobj_rx->xsk_arr[0].umem; + /* Non-owning view used only for TX buffer arithmetic; the sockets below bind to + * the RX-owned UMEM, so the handle, cq and refcount stay in one place. + */ umem_tx = ifobject->xsk_arr[0].umem_real;
A subsystem pattern flags this as potentially concerning: BPF code style expects multi-line comments to have the opening /* on its own line (bpf.md guide), even when surrounding code uses the old style. The patch shows the correct format in xsk_configure_socket(): /* * For shared sockets refcount_inc hasn't run yet, so clear umem to mark this slot * as having no reference. For the owner (non-shared) the reference was taken by * xsk_configure_umem; leave umem set so the caller's rollback path can release it * via umem_ref. */ but three other new comments in this patch start their text on the same line as the opening /* (this one, the "Keep ownership explicit" comment in xsk_delete_socket, and the "A UMEM that could not be deleted" comment in xsk_delete_all_ifobj_sockets). Would it be clearer to follow the same format throughout?
quoted hunk ↗ jump to hunk
- memcpy(umem_tx, umem_rx, sizeof(*umem_tx)); + umem_tx->num_frames = umem_rx->num_frames; + umem_tx->frame_headroom = umem_rx->frame_headroom; + umem_tx->buffer = umem_rx->buffer; + umem_tx->frame_size = umem_rx->frame_size; + umem_tx->unaligned_mode = umem_rx->unaligned_mode; umem_tx->base_addr = 0; umem_tx->next_buffer = 0;@@ -1599,6 +1627,8 @@ static int thread_common_ops(struct test_spec *test, struct ifobject *ifobject) ret = xsk_configure_umem(ifobject, umem, bufs, umem_sz); if (ret) return ret; + /* Mark before xsk_configure so rollback can release the UMEM if it fails. */ + ifobject->xsk->umem_ref = true; ret = xsk_configure(test, ifobject, umem, false); if (ret)
[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.h b/tools/testing/selftests/bpf/prog_tests/test_xsk.h index 03753ddc5dcd..15d6fe2b9568 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"
[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c index 7dad8556a722..5d8a29e7224e 100644 --- a/tools/testing/selftests/bpf/xskxceiver.c +++ b/tools/testing/selftests/bpf/xskxceiver.c
[ ... ] --- 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/32400920730