[PATCH net-next v3] selftests/bpf: Track test_xdp_features DUT processes

[PATCH net-next v3] selftests/bpf: Track test_xdp_features DUT processes

[Bochao Cao via B4 Relay]

From: Bochao Cao <bochaolucky@gmail.com>

test_xdp_features.sh waits for any xdp_features listener to appear and
uses pidof during cleanup. A concurrent test can therefore make another
test proceed before its own DUT is ready, and cleanup kills every
xdp_features process on the host. The readiness loop also has no timeout,
so a DUT that exits before listening leaves the test hung indefinitely.

Track one active DUT at a time, wait for ss to report that exact PID with
a bounded retry loop, and reap it after each test. Consult the shell job
table before signaling the DUT so a stale PID cannot target an unrelated
process. On failure, terminate the shell job with SIGKILL and reap it so
blocked I/O cannot hang cleanup. Install an EXIT trap and signal handlers
so failure paths also remove network setup.

Fixes: 4dba3e7852b7 ("selftests/bpf: introduce XDP compliance test tool")
Signed-off-by: Bochao Cao <bochaolucky@gmail.com>
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
---
Tests:
- bash -n tools/testing/selftests/bpf/test_xdp_features.sh
- make -C tools/testing/selftests/bpf xdp_features
- sudo tools/testing/selftests/bpf/test_xdp_features.sh
- verified cleanup terminates a blocked DUT without affecting an unrelated process
---
Changes in v3:
- Rebase onto net-next.
- Retarget the patch from bpf-next to net-next so the hardware-test
  migration can follow in the same tree without conflicts.
- Drop the Debian bug Closes tag; dependency cleanup can be handled
  independently by Debian.
- Add Emil's Reviewed-by tag.
- Link to v2: https://patch.msgid.link/20260812-xdp-dut-process-lifecycle-gmail-v2-1-b03ef2aa1b97@gmail.com

Changes in v2:
- Clarify that avoiding name-wide process matching, rather than dropping a dependency, is the motivation.
- Track and reap one active DUT at a time instead of retaining historical PIDs.
- Address PID reuse by signaling only the current Bash job during cleanup.
- Use SIGKILL on failure cleanup so blocked DUT I/O cannot hang wait indefinitely.
- Link to v1: https://patch.msgid.link/20260805-xdp-dut-process-lifecycle-gmail-v1-1-45984df8d295@gmail.com
---
 tools/testing/selftests/bpf/test_xdp_features.sh | 82 ++++++++++++++++++------
 1 file changed, 62 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_xdp_features.sh b/tools/testing/selftests/bpf/test_xdp_features.sh
index 0aa71c4455c0..35ce0d4e2418 100755
--- a/tools/testing/selftests/bpf/test_xdp_features.sh
+++ b/tools/testing/selftests/bpf/test_xdp_features.sh
@@ -8,6 +8,7 @@ readonly V0_IP6=2001:db8::11
 readonly V1_IP6=2001:db8::1
 
 ret=1
+dut_pid=""
 
 setup() {
 	{
@@ -30,77 +31,118 @@ setup() {
 	} > /dev/null 2>&1
 }
 
+terminate_dut_server() {
+	[ -z "$dut_pid" ] && return
+
+	# Use the shell job instead of a PID which may have been reused.
+	if [ "$(jobs -pr %% 2> /dev/null)" = "$dut_pid" ]; then
+		kill -KILL %% 2> /dev/null || true
+	fi
+
+	wait "$dut_pid" 2> /dev/null || true
+	dut_pid=""
+}
+
 cleanup() {
-	ip link del v1 2> /dev/null
-	ip netns del ${NS} 2> /dev/null
-	[ "$(pidof xdp_features)" = "" ] || kill $(pidof xdp_features) 2> /dev/null
+	terminate_dut_server
+	ip link del v1 2> /dev/null || true
+	ip netns del "${NS}" 2> /dev/null || true
 }
 
 wait_for_dut_server() {
-	while sleep 1; do
-		ss -tlp | grep -q xdp_features
-		[ $? -eq 0 ] && break
+	local i
+
+	for ((i = 0; i < 10; i++)); do
+		if [ "$(jobs -pr %% 2> /dev/null)" != "$dut_pid" ]; then
+			echo "xdp_features server $dut_pid exited before accepting connections" >&2
+			return 1
+		fi
+
+		if ss -tlp 2> /dev/null | grep -q "pid=$dut_pid,"; then
+			return 0
+		fi
+
+		sleep 1
 	done
+
+	echo "Timed out waiting for xdp_features server $dut_pid" >&2
+	return 1
+}
+
+start_dut_server() {
+	./xdp_features "$@" &
+	dut_pid=$!
+	wait_for_dut_server
+}
+
+reap_dut_server() {
+	local status=0
+
+	wait "$dut_pid" || status=$?
+	dut_pid=""
+	return "$status"
 }
 
 test_xdp_features() {
 	setup
 
 	## XDP_PASS
-	./xdp_features -f XDP_PASS -D $V1_IP6 -T $V0_IP6 v1 &
-	wait_for_dut_server
+	start_dut_server -f XDP_PASS -D $V1_IP6 -T $V0_IP6 v1
 	ip netns exec ${NS} ./xdp_features -t -f XDP_PASS \
 					   -D $V1_IP6 -C $V1_IP6 \
 					   -T $V0_IP6 v0
 	[ $? -ne 0 ] && exit
+	reap_dut_server
 
 	## XDP_DROP
-	./xdp_features -f XDP_DROP -D ::ffff:$V1_IP4 -T ::ffff:$V0_IP4 v1 &
-	wait_for_dut_server
+	start_dut_server -f XDP_DROP -D ::ffff:$V1_IP4 -T ::ffff:$V0_IP4 v1
 	ip netns exec ${NS} ./xdp_features -t -f XDP_DROP \
 					   -D ::ffff:$V1_IP4 \
 					   -C ::ffff:$V1_IP4 \
 					   -T ::ffff:$V0_IP4 v0
 	[ $? -ne 0 ] && exit
+	reap_dut_server
 
 	## XDP_ABORTED
-	./xdp_features -f XDP_ABORTED -D $V1_IP6 -T $V0_IP6 v1 &
-	wait_for_dut_server
+	start_dut_server -f XDP_ABORTED -D $V1_IP6 -T $V0_IP6 v1
 	ip netns exec ${NS} ./xdp_features -t -f XDP_ABORTED \
 					   -D $V1_IP6 -C $V1_IP6 \
 					   -T $V0_IP6 v0
 	[ $? -ne 0 ] && exit
+	reap_dut_server
 
 	## XDP_TX
-	./xdp_features -f XDP_TX -D ::ffff:$V1_IP4 -T ::ffff:$V0_IP4 v1 &
-	wait_for_dut_server
+	start_dut_server -f XDP_TX -D ::ffff:$V1_IP4 -T ::ffff:$V0_IP4 v1
 	ip netns exec ${NS} ./xdp_features -t -f XDP_TX \
 					   -D ::ffff:$V1_IP4 \
 					   -C ::ffff:$V1_IP4 \
 					   -T ::ffff:$V0_IP4 v0
 	[ $? -ne 0 ] && exit
+	reap_dut_server
 
 	## XDP_REDIRECT
-	./xdp_features -f XDP_REDIRECT -D $V1_IP6 -T $V0_IP6 v1 &
-	wait_for_dut_server
+	start_dut_server -f XDP_REDIRECT -D $V1_IP6 -T $V0_IP6 v1
 	ip netns exec ${NS} ./xdp_features -t -f XDP_REDIRECT \
 					   -D $V1_IP6 -C $V1_IP6 \
 					   -T $V0_IP6 v0
 	[ $? -ne 0 ] && exit
+	reap_dut_server
 
 	## XDP_NDO_XMIT
-	./xdp_features -f XDP_NDO_XMIT -D ::ffff:$V1_IP4 -T ::ffff:$V0_IP4 v1 &
-	wait_for_dut_server
+	start_dut_server -f XDP_NDO_XMIT -D ::ffff:$V1_IP4 -T ::ffff:$V0_IP4 v1
 	ip netns exec ${NS} ./xdp_features -t -f XDP_NDO_XMIT \
 					   -D ::ffff:$V1_IP4 \
 					   -C ::ffff:$V1_IP4 \
 					   -T ::ffff:$V0_IP4 v0
 	ret=$?
-	cleanup
+	reap_dut_server
 }
 
 set -e
-trap cleanup 2 3 6 9
+trap cleanup EXIT
+trap 'exit 129' HUP
+trap 'exit 130' INT
+trap 'exit 143' TERM
 
 test_xdp_features
 

---
base-commit: 6ebcf5074cff0402730c6981d2397139fee6322d
change-id: 20260805-xdp-dut-process-lifecycle-gmail-6d60bf8dcdf1

Best regards,
--  
Bochao Cao <bochaolucky@gmail.com>

Re: [PATCH net-next v3] selftests/bpf: Track test_xdp_features DUT processes

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 2 (1) · Low: 1

- [Medium] The commit message justifies part of the rewrite with a
  failure mode that cannot occur in the pre-patch script: "The readiness…
- [Medium] Signal coverage regression that contradicts the commit
  message.
- [Low] Trap race on partially published DUT state in start_dut_server()
  (tools/testing/selftests/bpf/test_xdp_features.sh, lines 72-76): the…

Pre-existing issues:
- [Medium] The selftest's pass/fail verdict cannot observe an XDP feature
  regression.

--

selftests/bpf: Track test_xdp_features DUT processes

This reworks test_xdp_features.sh so that the DUT xdp_features instance is
tracked by PID and shell job instead of by process name, waits for that
exact PID to appear in ss output with a bounded retry loop, and reaps it
after each subtest.  The old signal traps are replaced with an EXIT trap
plus HUP/INT/TERM handlers.

A few questions below.

> The readiness loop also has no timeout,
> so a DUT that exits before listening leaves the test hung indefinitely.

[Severity: Medium]
Is this description of the pre-patch behaviour accurate?  In the baseline
script, set -e is in effect before test_xdp_features is invoked, and the
readiness probe sits in the body of the while loop, not in its condition:

	wait_for_dut_server() {
		while sleep 1; do
			ss -tlp | grep -q xdp_features
			[ $? -eq 0 ] && break
		done
	}

Bash exempts only the command list immediately following the while or until
keyword from errexit, which here is sleep 1.  The grep -q in the loop body
is not exempt, so the first failing probe terminates the script with status
1.  Wouldn't that mean the loop can never reach a second iteration, and the
observable pre-patch symptom is a spurious failure about a second after the
DUT is started (plus a leaked netns and veth, since there was no EXIT trap),
rather than an indefinite hang?

Since the patch carries a Fixes: tag, could this sentence be reworded to
describe the failure mode that actually occurs?  The other two motivations
in the message, name-wide ss matching and pidof killing unrelated
xdp_features processes, do match the baseline code.

> diff --git a/tools/testing/selftests/bpf/test_xdp_features.sh b/tools/testing/selftests/bpf/test_xdp_features.sh
> index 0aa71c4455c05..35ce0d4e24180 100755
> --- a/tools/testing/selftests/bpf/test_xdp_features.sh
> +++ b/tools/testing/selftests/bpf/test_xdp_features.sh

[ ... ]

> @@ -30,77 +31,118 @@ setup() {
>  	} > /dev/null 2>&1
>  }
>  
> +terminate_dut_server() {
> +	[ -z "$dut_pid" ] && return
> +
> +	# Use the shell job instead of a PID which may have been reused.
> +	if [ "$(jobs -pr %% 2> /dev/null)" = "$dut_pid" ]; then
> +		kill -KILL %% 2> /dev/null || true
> +	fi
> +
> +	wait "$dut_pid" 2> /dev/null || true
> +	dut_pid=""
> +}
> +
>  cleanup() {
> -	ip link del v1 2> /dev/null
> -	ip netns del ${NS} 2> /dev/null
> -	[ "$(pidof xdp_features)" = "" ] || kill $(pidof xdp_features) 2> /dev/null
> +	terminate_dut_server
> +	ip link del v1 2> /dev/null || true
> +	ip netns del "${NS}" 2> /dev/null || true
>  }

[ ... ]

> +start_dut_server() {
> +	./xdp_features "$@" &
> +	dut_pid=$!
> +	wait_for_dut_server
> +}

[Severity: Low]
Can a trap serviced between these two commands orphan the DUT?  The child is
forked by the background command, but its identity becomes visible to the
cleanup path only when the separate assignment dut_pid=$! runs.  Bash runs
pending trap handlers at command boundaries.

A directed signal in that window, for example kill -TERM on the script pid,
which does not reach the async child, runs the new TERM handler and then the
EXIT trap:

	trap 'exit 143' TERM
	  -> trap cleanup EXIT
	     -> cleanup()
	        -> terminate_dut_server()

and terminate_dut_server() starts with:

	[ -z "$dut_pid" ] && return

dut_pid is still empty here, since it is initialised to "" at the top of the
script and cleared again by reap_dut_server() after each subtest, so the
just-forked xdp_features is never signalled.  cleanup() then continues into
ip link del v1 and ip netns del, leaving an xdp_features process behind with
its XDP program still attached and its device and netns removed underneath
it.  The removed pidof sweep used to cover this window.

Would signalling the job whenever jobs -pr reports one, rather than keying
off the variable assignment having completed, close this?

[ ... ]

>  	## XDP_NDO_XMIT
> -	./xdp_features -f XDP_NDO_XMIT -D ::ffff:$V1_IP4 -T ::ffff:$V0_IP4 v1 &
> -	wait_for_dut_server
> +	start_dut_server -f XDP_NDO_XMIT -D ::ffff:$V1_IP4 -T ::ffff:$V0_IP4 v1
>  	ip netns exec ${NS} ./xdp_features -t -f XDP_NDO_XMIT \
>  					   -D ::ffff:$V1_IP4 \
>  					   -C ::ffff:$V1_IP4 \
>  					   -T ::ffff:$V0_IP4 v0
>  	ret=$?
> -	cleanup
> +	reap_dut_server
>  }

[Severity: Medium]
This isn't an issue introduced by this patch, but while the ret=$? contract
is being touched here, can this script actually observe an XDP feature
regression?  The script's verdict comes only from the tester exit status,
via ret=$? here and exit $ret at the end.

On the tester side, tester_run() in xdp_features.c collects the result and
only prints it:

	detected_cap = tester_collect_detected_cap(skel, ntohl(stats));

	fprintf(stdout, "Feature %s: [%s][%s]\n", get_xdp_feature_str(), ...);
out:
	bpf_xdp_detach(env.ifindex, flags, NULL);
	close(sockfd);
	return err < 0 ? err : 0;

detected_cap never reaches the return value, and main() collapses anything
non-negative:

	return err < 0 ? -err : 0;

So a NOT DETECTED result for XDP_PASS, XDP_DROP, XDP_ABORTED, XDP_TX,
XDP_REDIRECT or XDP_NDO_XMIT still exits 0 and the harness records a pass;
only infrastructure failures such as socket, attach or protocol errors can
fail the test.  Is that intended?

>  set -e
> -trap cleanup 2 3 6 9
> +trap cleanup EXIT
> +trap 'exit 129' HUP
> +trap 'exit 130' INT
> +trap 'exit 143' TERM

[Severity: Medium]
The old trap covered signals 2, 3, 6 and 9, that is INT, QUIT, ABRT and
KILL.  The new set covers HUP, INT and TERM, so QUIT and ABRT are no longer
handled.  A shell killed by an untrapped fatal signal does not run its EXIT
trap, so does kill -QUIT on the script, or a terminal Ctrl-\, now skip
cleanup() entirely and leave the v1 veth and the ns1-XXXXXX netns behind?

The background DUT looks affected too.  A non-interactive shell sets
SIGINT and SIGQUIT to ignored in asynchronous children, so a process-group
SIGQUIT does not reach the child started by:

	./xdp_features "$@" &

and the catch-all sweep that used to collect it has been removed from
cleanup():

	-	[ "$(pidof xdp_features)" = "" ] || kill $(pidof xdp_features) 2> /dev/null

That leaves an xdp_features process holding an XDP program plus stale
network state, and the next run then fails in setup() at ip link add v1.

The commit message says "Install an EXIT trap and signal handlers so failure
paths also remove network setup" -- would adding trap 'exit 131' QUIT and
trap 'exit 134' ABRT match that claim?

>  
>  test_xdp_features
>

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260904-xdp-dut-process-lifecycle-gmail-v3-1-5b7eee4f7009%40gmail.com

Re: [PATCH net-next v3] selftests/bpf: Track test_xdp_features DUT processes

[Paolo Abeni]

On 9/4/26 6:44 AM, Bochao Cao wrote:
> test_xdp_features.sh waits for any xdp_features listener to appear and
> uses pidof during cleanup. A concurrent test can therefore make another
> test proceed before its own DUT is ready, and cleanup kills every
> xdp_features process on the host. The readiness loop also has no timeout,
> so a DUT that exits before listening leaves the test hung indefinitely.
> 
> Track one active DUT at a time, wait for ss to report that exact PID with
> a bounded retry loop, and reap it after each test. Consult the shell job
> table before signaling the DUT so a stale PID cannot target an unrelated
> process. On failure, terminate the shell job with SIGKILL and reap it so
> blocked I/O cannot hang cleanup. Install an EXIT trap and signal handlers
> so failure paths also remove network setup.
> 
> Fixes: 4dba3e7852b7 ("selftests/bpf: introduce XDP compliance test tool")
> Signed-off-by: Bochao Cao <bochaolucky@gmail.com>
> Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
Since the goal of targeting net-next is allowing shorted turn-over for
the conversion to driver/hw, I think it would be better to include the
latter change in the same series.

Also it would be possibly make sense to take care of the last sashiko
comment (about signal handling).

/P