BPF List
 help / color / mirror / Atom feed
* [PATCH bpf-next] selftests/bpf: Track test_xdp_features DUT processes
@ 2026-08-05  7:29 Bochao Cao via B4 Relay
  2026-08-05  7:40 ` sashiko-bot
  2026-08-05  8:18 ` Jiayuan Chen
  0 siblings, 2 replies; 3+ messages in thread
From: Bochao Cao via B4 Relay @ 2026-08-05  7:29 UTC (permalink / raw)
  To: Andrii Nakryiko, Eduard Zingerman
  Cc: Ihor Solodrai, Lorenzo Bianconi, bpf, netdev, Bochao Cao

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.

Record each DUT PID when it is spawned, wait for ss to report that exact
PID with a bounded retry loop, and only terminate and reap recorded
children. Install an EXIT trap and signal handlers so failure paths also
remove child processes and network setup.

This also removes the runtime dependency on pidof.

Fixes: 4dba3e7852b7 ("selftests/bpf: introduce XDP compliance test tool")
Closes: https://bugs.debian.org/1136522
Signed-off-by: Bochao Cao <bochaolucky@gmail.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 leaves an unrelated xdp_features process running
---
 tools/testing/selftests/bpf/test_xdp_features.sh | 60 ++++++++++++++++--------
 1 file changed, 40 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_xdp_features.sh b/tools/testing/selftests/bpf/test_xdp_features.sh
index 0aa71c4..f3aa4d1 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_pids=()
 
 setup() {
 	{
@@ -31,32 +32,53 @@ setup() {
 }
 
 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
+	for pid in "${dut_pids[@]}"; do
+		kill "$pid" 2> /dev/null || true
+		wait "$pid" 2> /dev/null || true
+	done
+	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 pid=$1
+	local i
+
+	for ((i = 0; i < 10; i++)); do
+		if ! kill -0 "$pid" 2> /dev/null; then
+			echo "xdp_features server $pid exited before accepting connections" >&2
+			return 1
+		fi
+
+		if ss -tlp 2> /dev/null | grep -q "pid=$pid,"; then
+			return 0
+		fi
+
+		sleep 1
 	done
+
+	echo "Timed out waiting for xdp_features server $pid" >&2
+	return 1
+}
+
+start_dut_server() {
+	./xdp_features "$@" &
+	dut_pids+=("$!")
+	wait_for_dut_server "$!"
 }
 
 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
 
 	## 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 \
@@ -64,16 +86,14 @@ test_xdp_features() {
 	[ $? -ne 0 ] && exit
 
 	## 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
 
 	## 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 \
@@ -81,26 +101,26 @@ test_xdp_features() {
 	[ $? -ne 0 ] && exit
 
 	## 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
 
 	## 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
 }
 
 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: 6655c409707ec8ce9ce0850ffe4fe02331fd4d9c
change-id: 20260805-xdp-dut-process-lifecycle-gmail-6d60bf8dcdf1

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



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

* Re: [PATCH bpf-next] selftests/bpf: Track test_xdp_features DUT processes
  2026-08-05  7:29 [PATCH bpf-next] selftests/bpf: Track test_xdp_features DUT processes Bochao Cao via B4 Relay
@ 2026-08-05  7:40 ` sashiko-bot
  2026-08-05  8:18 ` Jiayuan Chen
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-08-05  7:40 UTC (permalink / raw)
  To: Bochao Cao; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The test script deadlocks indefinitely during cleanup if terminated while the DUT server is blocked in I/O.
- [High] The cleanup routine sends `SIGTERM` to historical PIDs that have already exited, risking the termination of unrelated host processes if PIDs wrap around.
--

--- Patch [1]: [PATCH bpf-next] selftests/bpf: Track test_xdp_features DUT processes ---
commit 3a3cec0883174021af4192cc572fb4e4f76dbd7c
Author: Bochao Cao <bochaolucky@gmail.com>

selftests/bpf: Track test_xdp_features DUT processes

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. Record each DUT PID when it is spawned
and only terminate and reap recorded children.

Closes: https://bugs.debian.org/1136522

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

[ ... ]

> @@ -31,32 +32,53 @@ setup() {
>  }
>  
>  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
> +	for pid in "${dut_pids[@]}"; do
> +		kill "$pid" 2> /dev/null || true

[Severity: High]
Does this risk killing unrelated host processes if PIDs wrap around?

The script spawns a background xdp_features process for each test and records
its PID in dut_pids. During a normal test, the tester process sends a
CMD_STOP, causing the DUT to exit cleanly, at which point the OS reaps
the PID.

At the end of the script, cleanup blindly loops over all recorded PIDs (most
of which have already exited) and unconditionally executes kill "$pid". If
the system is busy and any of those freed PIDs have been reassigned to
unrelated host processes, they will receive SIGTERM as root.

> +		wait "$pid" 2> /dev/null || true

[Severity: High]
Can this script deadlock indefinitely during cleanup?

If the script fails or receives a signal, the EXIT trap runs cleanup, sending
SIGTERM to the DUT processes and calling wait on the PID.

The xdp_features server uses glibc's signal(), which defaults to SA_RESTART:

tools/testing/selftests/bpf/xdp_features.c:main() {
    ...
    signal(SIGTERM, sig_handler);
    ...
}

When the server receives SIGTERM, its handler sets exiting = true, but the
blocking accept() or recv() syscall is automatically restarted by the kernel.
The server remains blocked forever and never exits, causing the script's
wait to hang indefinitely.

> +	done
> +	ip link del v1 2> /dev/null || true
> +	ip netns del "${NS}" 2> /dev/null || true
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805-xdp-dut-process-lifecycle-gmail-v1-1-45984df8d295@gmail.com?part=1

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

* Re: [PATCH bpf-next] selftests/bpf: Track test_xdp_features DUT processes
  2026-08-05  7:29 [PATCH bpf-next] selftests/bpf: Track test_xdp_features DUT processes Bochao Cao via B4 Relay
  2026-08-05  7:40 ` sashiko-bot
@ 2026-08-05  8:18 ` Jiayuan Chen
  1 sibling, 0 replies; 3+ messages in thread
From: Jiayuan Chen @ 2026-08-05  8:18 UTC (permalink / raw)
  To: bochaolucky, Andrii Nakryiko, Eduard Zingerman
  Cc: Ihor Solodrai, Lorenzo Bianconi, bpf, netdev


On 8/5/26 3:29 PM, Bochao Cao via B4 Relay wrote:
> 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.
>
> Record each DUT PID when it is spawned, wait for ss to report that exact
> PID with a bounded retry loop, and only terminate and reap recorded
> children. Install an EXIT trap and signal handlers so failure paths also
> remove child processes and network setup.
>
> This also removes the runtime dependency on pidof.
>
> Fixes: 4dba3e7852b7 ("selftests/bpf: introduce XDP compliance test tool")
> Closes: https://bugs.debian.org/1136522


It looks like part of the motivation here is dropping the pidof dependency?
I don't think that's worth optimizing. Selftests already require a bunch of
tools from non-essential packages (ip, ss and ethtool in this very script)


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

end of thread, other threads:[~2026-08-05  8:18 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-05  7:29 [PATCH bpf-next] selftests/bpf: Track test_xdp_features DUT processes Bochao Cao via B4 Relay
2026-08-05  7:40 ` sashiko-bot
2026-08-05  8:18 ` Jiayuan Chen

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