mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/2] selftests: bpf: run test_xdp_features in its own netns
@ 2026-09-17  1:55 Eva Kurchatova
  2026-09-17  1:55 ` [PATCH 2/2] selftests: bpf: size the map in test_lru_sanity3 to whole refills Eva Kurchatova
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Eva Kurchatova @ 2026-09-17  1:55 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, David S. Miller,
	Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
	Stanislav Fomichev, Andrii Nakryiko, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
	Yonghong Song, Jiri Olsa, Emil Tsalapatis, Ihor Solodrai,
	Shuah Khan
  Cc: linux-kernel, Eva Kurchatova, netdev, bpf, linux-kselftest

The test creates its devices in the namespace it is started in and binds
an IPv6 socket to talk between the two ends. Both are unnecessary
demands on the host it runs on.

The device name is fixed, v1, so a host that already has a device by
that name cannot run the test at all. The IPv6 control socket depends on
the host's firewall rules, and where those reject ICMPv6 or the port the
test picks, the two ends never meet:

  Failed connecting to the Device Under Test control socket

Cleanup is trapped for signals only, not for a normal exit, so a run
that fails leaves both the device and the namespace behind. Every later
run then stops in setup, before it prints anything.

Re-exec in a new network namespace, where the device name is free and no
rule of the host applies, and everything the test made goes away with
the namespace when it exits.

Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
---
 tools/testing/selftests/bpf/test_xdp_features.sh | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tools/testing/selftests/bpf/test_xdp_features.sh b/tools/testing/selftests/bpf/test_xdp_features.sh
index 0aa71c4455c0..1f6be4af6718 100755
--- a/tools/testing/selftests/bpf/test_xdp_features.sh
+++ b/tools/testing/selftests/bpf/test_xdp_features.sh
@@ -1,6 +1,19 @@
 #!/bin/bash
 # SPDX-License-Identifier: GPL-2.0
 
+# The device under test listens on the veth address of the namespace this
+# script runs in and the tester connects to it, so a firewall on the
+# machine can refuse the control connection:
+#   Failed connecting to the Device Under Test control socket
+# That side of the pair is also always called v1, and cleanup only runs
+# on a signal, so a failed run leaves the device behind and every later
+# run stops in setup.  A namespace of our own has no such rules, and
+# takes the leftovers with it when the test ends.
+if [ -z "${XDP_FEATURES_NETNS:-}" ]; then
+	XDP_FEATURES_NETNS=1 export XDP_FEATURES_NETNS
+	exec unshare -n sh -c 'ip link set lo up; exec "$0" "$@"' "$0" "$@"
+fi
+
 readonly NS="ns1-$(mktemp -u XXXXXX)"
 readonly V0_IP4=10.10.0.11
 readonly V1_IP4=10.10.0.1
-- 
2.55.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] selftests: bpf: size the map in test_lru_sanity3 to whole refills
  2026-09-17  1:55 [PATCH 1/2] selftests: bpf: run test_xdp_features in its own netns Eva Kurchatova
@ 2026-09-17  1:55 ` Eva Kurchatova
  2026-09-17  2:13 ` [PATCH 1/2] selftests: bpf: run test_xdp_features in its own netns Alexei Starovoitov
  2026-09-17  2:45 ` bot+bpf-ci
  2 siblings, 0 replies; 4+ messages in thread
From: Eva Kurchatova @ 2026-09-17  1:55 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
	Ihor Solodrai, Shuah Khan, Willem de Bruijn
  Cc: linux-kernel, Eva Kurchatova, bpf, linux-kselftest

test_lru_sanity3 fills a map of tgt_free * 2 elements and then reads
back all but the last few, which fails on a machine with six CPUs:

  test_lru_sanity3 (map_type:9 map_flags:0x0): test_lru_map.c:463:
  test_lru_sanity3: Assertion `!bpf_map_lookup_elem_with_ref_bit(
  lru_map_fd, key, value)' failed.

Elements are handed to a CPU in refills of lru->target_free, which
bpf_lru_populate() derives from the map size as clamp((nr_elems /
num_possible_cpus()) / 2, 1, LOCAL_FREE_TARGET), so 21 for a 256 element
map and six CPUs. A refill that the global free list cannot satisfy in
full does not stop there: bpf_lru_list_pop_free_to_local() calls
__bpf_lru_list_shrink() for the remainder, which evicts elements that
are still in the map. 256 is not a multiple of 21, so filling the map
ends on a partial refill that drops 17 of the elements the test goes on
to reference, and the lookup fails on the first of them.

Whether the size divides evenly depends on the CPU count alone, which is
why this passes on two and on sixty four CPUs and fails on six.

batch_size is already __tgt_size(tgt_free), the refill size of a map of
__map_size(batch_size) elements, so size the map that way and filling it
consumes whole refills and evicts nothing. Start the keys of the last
insert at map_size + 1, they were placed just past the old size.

Fixes: 5e9388f7984a ("selftests/bpf: adapt one more case in test_lru_map to the new target_free")
Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
---
 tools/testing/selftests/bpf/test_lru_map.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_lru_map.c b/tools/testing/selftests/bpf/test_lru_map.c
index 0921939532c6..fc67a337d3c1 100644
--- a/tools/testing/selftests/bpf/test_lru_map.c
+++ b/tools/testing/selftests/bpf/test_lru_map.c
@@ -441,8 +441,18 @@ static void test_lru_sanity3(int map_type, int map_flags, unsigned int tgt_free)
 	assert(sched_next_online(0, &next_cpu) != -1);
 
 	batch_size = __tgt_size(tgt_free);
+	if (!batch_size)
+		batch_size = 1;
+
+	/* The local free list is refilled lru->target_free elements at a
+	 * time, and a refill the global free list cannot satisfy in full
+	 * shrinks the LRU list, which evicts elements that are still live.
+	 * Size the map so that target_free divides it, otherwise filling it
+	 * ends on a partial refill and evicts the elements referenced below.
+	 */
+	map_size = __map_size(batch_size);
+	assert(__tgt_size(map_size) == batch_size);
 
-	map_size = tgt_free * 2;
 	lru_map_fd = create_map(map_type, map_flags, map_size);
 	assert(lru_map_fd != -1);
 
@@ -466,7 +476,7 @@ static void test_lru_sanity3(int map_type, int map_flags, unsigned int tgt_free)
 	}
 
 	/* Insert new batch_size: replaces the non-referenced elements */
-	key = 2 * tgt_free + 1;
+	key = 1 + map_size;
 	end_key = key + batch_size;
 	for (; key < end_key; key++) {
 		assert(!bpf_map_update_elem(lru_map_fd, &key, value,
-- 
2.55.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] selftests: bpf: run test_xdp_features in its own netns
  2026-09-17  1:55 [PATCH 1/2] selftests: bpf: run test_xdp_features in its own netns Eva Kurchatova
  2026-09-17  1:55 ` [PATCH 2/2] selftests: bpf: size the map in test_lru_sanity3 to whole refills Eva Kurchatova
@ 2026-09-17  2:13 ` Alexei Starovoitov
  2026-09-17  2:45 ` bot+bpf-ci
  2 siblings, 0 replies; 4+ messages in thread
From: Alexei Starovoitov @ 2026-09-17  2:13 UTC (permalink / raw)
  To: Eva Kurchatova, Daniel Borkmann, David S. Miller, Jakub Kicinski,
	Jesper Dangaard Brouer, John Fastabend, Stanislav Fomichev,
	Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
	Emil Tsalapatis, Ihor Solodrai, Shuah Khan
  Cc: linux-kernel, netdev, bpf, linux-kselftest

On Thu, Sep 17, 2026 at 04:55 AM Eva Kurchatova <eva.kurchatova@virtuozzo.com> wrote:
> The test creates its devices in the namespace it is started in and binds
> an IPv6 socket to talk between the two ends. Both are unnecessary
> demands on the host it runs on.
>
> The device name is fixed, v1, so a host that already has a device by
> that name cannot run the test at all. The IPv6 control socket depends on
> the host's firewall rules, and where those reject ICMPv6 or the port the
> test picks, the two ends never meet:
>
>   Failed connecting to the Device Under Test control socket
>
> Cleanup is trapped for signals only, not for a normal exit, so a run
> that fails leaves both the device and the namespace behind. Every later
> run then stops in setup, before it prints anything.

This script is not run by BPF CI and it's in the middle of being
reworked and moved to tools/testing/selftests/drivers/net/hw/ via
net-next, see Bochao's "Track test_xdp_features DUT processe
s" and
Daniel's replies there:
https://lore.kernel.org/bpf/20260904-xdp-dut-process-lifecycle-gmail-v3-1-5b7eee4f7009@gmail.com/
https://lore.kernel.org/bpf/10886894-081b-499f-abce-fca62b120615@iogearbox.net/
That patch already switches to trap cleanup EXIT and tracks the DUT
pid, and this one conflicts with it. Pls sync with Bochao and netdev
folks and, if the netns part is still wanted, send it against net-next
on top of that work.

Also patch 2 has nothing to do with this one. Don't send unrelated
patches as a series, and use a proper subject prefix
(see Documentation/bpf/bpf_devel_QA.rst).

> Re-exec in a new network namespace, where the device name is free and no
> rule of the host applies, and everything the test made goes away with
> the namespace when it exits.

That last part isn't true in the failure case you describe. unshare -n
doesn't give a new mount namespace, so 'ip netns add ${NS}' in setup()
still creates /var/run/netns/${NS} on the host and nothing
 deletes it.
And when the tester fails to connect, set -e exits the script while
the background './xdp_features ... v1 &' is still blocked in accept()
in dut_run(). That orphan keeps the unshared netns alive, so v1, v0,
${NS} and a stray xdp_features process are all left behind on every
failed run. They just don't collide with the next run anymore.
trap cleanup EXIT is what fixes the leftovers, with or without unshare.

pw-bot: cr

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] selftests: bpf: run test_xdp_features in its own netns
  2026-09-17  1:55 [PATCH 1/2] selftests: bpf: run test_xdp_features in its own netns Eva Kurchatova
  2026-09-17  1:55 ` [PATCH 2/2] selftests: bpf: size the map in test_lru_sanity3 to whole refills Eva Kurchatova
  2026-09-17  2:13 ` [PATCH 1/2] selftests: bpf: run test_xdp_features in its own netns Alexei Starovoitov
@ 2026-09-17  2:45 ` bot+bpf-ci
  2 siblings, 0 replies; 4+ messages in thread
From: bot+bpf-ci @ 2026-09-17  2:45 UTC (permalink / raw)
  To: eva.kurchatova, ast, daniel, davem, kuba, hawk, john.fastabend,
	sdf, andrii, eddyz87, memxor, martin.lau, song, yonghong.song,
	jolsa, emil, ihor.solodrai, shuah
  Cc: linux-kernel, eva.kurchatova, netdev, bpf, linux-kselftest, ast,
	andrii, daniel, martin.lau, eddyz87, yonghong.song, mason,
	ihor.solodrai

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

> diff --git a/tools/testing/selftests/bpf/test_xdp_features.sh b/tools/testing/selftests/bpf/test_xdp_features.sh
> index 0aa71c4455c0..1f6be4af6718 100755
> --- a/tools/testing/selftests/bpf/test_xdp_features.sh
> +++ b/tools/testing/selftests/bpf/test_xdp_features.sh
> @@ -1,6 +1,19 @@
>  #!/bin/bash
>  # SPDX-License-Identifier: GPL-2.0
>
> +# The device under test listens on the veth address of the namespace this
> +# script runs in and the tester connects to it, so a firewall on the
> +# machine can refuse the control connection:
> +#   Failed connecting to the Device Under Test control socket
> +# That side of the pair is also always called v1, and cleanup only runs
> +# on a signal, so a failed run leaves the device behind and every later
> +# run stops in setup.  A namespace of our own has no such rules, and
> +# takes the leftovers with it when the test ends.
> +if [ -z "${XDP_FEATURES_NETNS:-}" ]; then
> +	XDP_FEATURES_NETNS=1 export XDP_FEATURES_NETNS
> +	exec unshare -n sh -c 'ip link set lo up; exec "$0" "$@"' "$0" "$@"
> +fi

The comment and commit message both state "everything the test made goes
away with the namespace when it exits", but does the anonymous namespace
actually reclaim the persistent named namespace created by ip netns add?

unshare -n only unshares CLONE_NEWNET. The mount namespace remains the
host's, so when setup() later runs:

    ip netns add ${NS}
    ip link add v1 type veth peer name v0 netns ${NS}

ip netns add deliberately creates a persistent namespace by bind-mounting
/proc/self/ns/net onto /run/netns/${NS}. That bind mount is visible on
the host and pins ${NS} with the v0 veth inside it.

Exiting the anonymous netns from unshare -n doesn't remove the named
netns -- that's why cleanup() must explicitly call ip netns del ${NS}.

cleanup() is still only reachable from the signal trap or the fully
successful path. All failure arms are [ $? -ne 0 ] && exit, and there is
no trap cleanup EXIT. On a failing run after the patch:

  - v1 and the anonymous netns: reclaimed
  - /run/netns/ns1-XXXXXX and the netns holding v0: leaked on the host

The practical breakage described in the changelog (name collision
causing later runs to fail in setup) is fixed because ${NS} is
randomised.

But the claim of complete cleanup is not accurate. Should this either
narrow the claim to the device and firewall rules, or make cleanup
complete with trap cleanup EXIT (in addition to the signal trap), or use
unshare -n -m with a private /run/netns so the named namespace really
does die with the test?

> +if [ -z "${XDP_FEATURES_NETNS:-}" ]; then
> +	XDP_FEATURES_NETNS=1 export XDP_FEATURES_NETNS
> +	exec unshare -n sh -c 'ip link set lo up; exec "$0" "$@"' "$0" "$@"
> +fi

A subsystem pattern flags this as potentially concerning: the new code
hand-rolls network-namespace setup (re-exec under unshare -n plus
ip link set lo up) in a script that already hand-rolls ip netns add,
ip link del, and ip netns del.

tools/testing/selftests/net/lib.sh already provides this: setup_ns
generates a randomised namespace name, creates it, brings lo up and
normalises rp_filter; cleanup_ns and cleanup_all_ns tear the list down
and register teardown to run on normal exit as well as on a signal,
which is the gap the patch's changelog identifies.

Worth a maintainer's call on whether the shared helpers should be used
here instead of a second implementation.

The alternative view is that this may be defensible: bpf selftests do
not currently source tools/testing/selftests/net/lib.sh (no bpf/*.sh
does), pulling it in would add a TEST_FILES dependency, and setup_ns is
not a drop-in substitute for what this hunk does. setup_ns creates a
named peer namespace, whereas the hunk moves the script's own process
into an anonymous namespace so the DUT-side v1 device and the host
firewall rules stop mattering. The parts net/lib.sh would genuinely
replace are the pre-existing ip netns add and cleanup lines, not the new
ones.


---
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/35173519344

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-17  2:45 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-17  1:55 [PATCH 1/2] selftests: bpf: run test_xdp_features in its own netns Eva Kurchatova
2026-09-17  1:55 ` [PATCH 2/2] selftests: bpf: size the map in test_lru_sanity3 to whole refills Eva Kurchatova
2026-09-17  2:13 ` [PATCH 1/2] selftests: bpf: run test_xdp_features in its own netns Alexei Starovoitov
2026-09-17  2:45 ` bot+bpf-ci

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®