[PATCH net-next v4 0/2] selftests: Fix and migrate XDP feature test

[PATCH net-next v4 0/2] selftests: Fix and migrate XDP feature test

[Bochao Cao via B4 Relay]

Fix DUT process tracking, readiness retries, and signal cleanup, then
move the XDP feature test to drivers/net/hw as requested during v3 review.
The NetDrvEpEnv runner uses configured DUT and remote interfaces and
reports detected-versus-advertised mismatches as test failures.

Collect results before teardown so the echo used to wake the DUT thread
cannot contaminate tester counters and cause a false XDP_DROP failure.

Changes in v4:
- Correct the description of the set -e readiness behavior.
- Handle signals arriving before DUT PID assignment, including QUIT/ABRT.
- Move the test and build rules to drivers/net/hw and use NetDrvEpEnv.
- Report feature mismatches as failures and sample counters before cleanup.

Testing on Ubuntu 7.0.0-30-generic:
- Built xdp_features; checked shell and Python syntax.
- Original and v4 REDIRECT/NDO_XMIT results matched in three repeated runs
  with GRO enabled and TX checksum offload disabled, as in the old setup.
- Confirmed default veth offload settings reproduce mismatches in both
  versions; these are not migration-specific regressions.
- After fixing cleanup counter contamination, XDP_DROP passed five runs
  and the complete six-test runner passed twice (12 passes, zero failures).
- SIGTERM interruption left no xdp_features processes, veths, or netns.
- Rebased onto net-next commit 8830e65ed46d; repeated the build, Python
  syntax, diff-check, and per-patch checkpatch validation.

Native-XDP hardware traffic testing remains unverified: the available
e1000e interface rejects native-mode attachment with EOPNOTSUPP.

Link: https://lore.kernel.org/r/20260904-xdp-dut-process-lifecycle-gmail-v3-1-5b7eee4f7009@gmail.com

Signed-off-by: Bochao Cao <bochaolucky@gmail.com>
---
Bochao Cao (2):
      selftests/bpf: Track test_xdp_features DUT processes
      selftests/net: Move XDP feature test to driver framework

 tools/testing/selftests/bpf/Makefile               |  11 +-
 tools/testing/selftests/bpf/test_xdp_features.sh   | 107 -----------------
 tools/testing/selftests/drivers/net/hw/.gitignore  |   3 +
 tools/testing/selftests/drivers/net/hw/Makefile    |  22 ++++
 .../net/hw/xdp_features.bpf.c}                     |   0
 .../{bpf => drivers/net/hw}/xdp_features.c         | 126 +++++++++++++++------
 .../{bpf => drivers/net/hw}/xdp_features.h         |   0
 .../selftests/drivers/net/hw/xdp_features.py       | 109 ++++++++++++++++++
 8 files changed, 226 insertions(+), 152 deletions(-)
---
base-commit: 8830e65ed46de41f849eefb8ba227d4852c460f6
change-id: 20260921-xdp-features-v4-send-2c0da5db22d7

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

[PATCH net-next v4 1/2] selftests/bpf: Track test_xdp_features DUT processes

[Bochao Cao via B4 Relay]

From: Bochao Cao <bochaolucky@gmail.com>

Track the DUT process instead of matching processes by name to avoid
interference between concurrent tests. Fix readiness retries under
set -e and clean up the DUT and network state on exit or interruption.

Signed-off-by: Bochao Cao <bochaolucky@gmail.com>
---
 tools/testing/selftests/bpf/test_xdp_features.sh | 88 ++++++++++++++++++------
 1 file changed, 68 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_xdp_features.sh b/tools/testing/selftests/bpf/test_xdp_features.sh
index 0aa71c445..a78e0fb9e 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,124 @@ setup() {
 	} > /dev/null 2>&1
 }
 
+terminate_dut_server() {
+	local pid
+
+	# Use the shell job instead of dut_pid, which may not have been assigned
+	# yet if a signal arrived immediately after the server was started.
+	pid=$(jobs -pr %% 2> /dev/null) || true
+	if [ -z "$pid" ]; then
+		dut_pid=""
+		return
+	fi
+
+	kill -KILL %% 2> /dev/null || true
+	wait "$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 131' QUIT
+trap 'exit 134' ABRT
+trap 'exit 143' TERM
 
 test_xdp_features
 

-- 
2.43.0

[PATCH net-next v4 2/2] selftests/net: Move XDP feature test to driver framework

[Bochao Cao via B4 Relay]

From: Bochao Cao <bochaolucky@gmail.com>

Move the XDP feature test to drivers/net/hw and use NetDrvEpEnv to test
configured local and remote interfaces. Sample results before cleanup
traffic and fail tests when detected and advertised features disagree.

Signed-off-by: Bochao Cao <bochaolucky@gmail.com>
---
 tools/testing/selftests/bpf/Makefile               |  11 +-
 tools/testing/selftests/bpf/test_xdp_features.sh   | 155 ---------------------
 tools/testing/selftests/drivers/net/hw/.gitignore  |   3 +
 tools/testing/selftests/drivers/net/hw/Makefile    |  22 +++
 .../net/hw/xdp_features.bpf.c}                     |   0
 .../{bpf => drivers/net/hw}/xdp_features.c         | 126 ++++++++++++-----
 .../{bpf => drivers/net/hw}/xdp_features.h         |   0
 .../selftests/drivers/net/hw/xdp_features.py       | 109 +++++++++++++++
 8 files changed, 226 insertions(+), 200 deletions(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 93c707116..cb76b68c9 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -126,7 +126,7 @@ TEST_PROGS := test_kmod.sh \
 	test_bpftool_build.sh \
 	test_doc_build.sh \
 	test_xsk.sh \
-	test_xdp_features.sh
+	#
 
 TEST_PROGS_EXTENDED := \
 	ima_setup.sh verify_sig_setup.sh
@@ -142,7 +142,6 @@ TEST_GEN_PROGS_EXTENDED = \
 	test_cpp \
 	test_lirc_mode2_user \
 	veristat \
-	xdp_features \
 	xdp_hw_metadata \
 	xdp_synproxy \
 	xskxceiver
@@ -544,7 +543,6 @@ test_subskeleton_lib.skel.h-deps := test_subskeleton_lib2.bpf.o test_subskeleton
 test_usdt.skel.h-deps := test_usdt.bpf.o test_usdt_multispec.bpf.o
 xsk_xdp_progs.skel.h-deps := xsk_xdp_progs.bpf.o
 xdp_hw_metadata.skel.h-deps := xdp_hw_metadata.bpf.o
-xdp_features.skel.h-deps := xdp_features.bpf.o
 tracing_multi.skel.h-deps := tracing_multi_attach.bpf.o tracing_multi_check.bpf.o
 tracing_multi_module.skel.h-deps := tracing_multi_attach_module.bpf.o tracing_multi_check.bpf.o
 tracing_multi_intersect.skel.h-deps := tracing_multi_intersect_attach.bpf.o tracing_multi_check.bpf.o
@@ -947,13 +945,6 @@ $(OUTPUT)/xdp_hw_metadata: xdp_hw_metadata.c xsk.c network_helpers.c \
 	$(call msg,BINARY,,$@)
 	$(Q)$(CC) $(CFLAGS) $(filter %.a %.o %.c,$^) $(LDLIBS) -o $@
 
-$(OUTPUT)/xdp_features: xdp_features.c network_helpers.c xdp_features.h \
-			   network_helpers.h \
-			   test_progs.h bpf_util.h $(OUTPUT)/xdp_features.skel.h \
-			   $(BPFOBJ) | $(OUTPUT)
-	$(call msg,BINARY,,$@)
-	$(Q)$(CC) $(CFLAGS) $(filter %.a %.o %.c,$^) $(LDLIBS) -o $@
-
 # Make sure we are able to include and link libbpf against c++.
 CXXFLAGS += $(CFLAGS)
 CXXFLAGS := $(subst -D_GNU_SOURCE=,,$(CXXFLAGS))
diff --git a/tools/testing/selftests/bpf/test_xdp_features.sh b/tools/testing/selftests/bpf/test_xdp_features.sh
deleted file mode 100755
index a78e0fb9e..000000000
--- a/tools/testing/selftests/bpf/test_xdp_features.sh
+++ /dev/null
@@ -1,155 +0,0 @@
-#!/bin/bash
-# SPDX-License-Identifier: GPL-2.0
-
-readonly NS="ns1-$(mktemp -u XXXXXX)"
-readonly V0_IP4=10.10.0.11
-readonly V1_IP4=10.10.0.1
-readonly V0_IP6=2001:db8::11
-readonly V1_IP6=2001:db8::1
-
-ret=1
-dut_pid=""
-
-setup() {
-	{
-		ip netns add ${NS}
-
-		ip link add v1 type veth peer name v0 netns ${NS}
-
-		ip link set v1 up
-		ip addr add $V1_IP4/24 dev v1
-		ip addr add $V1_IP6/64 nodad dev v1
-		ip -n ${NS} link set dev v0 up
-		ip -n ${NS} addr add $V0_IP4/24 dev v0
-		ip -n ${NS} addr add $V0_IP6/64 nodad dev v0
-
-		# Enable XDP mode and disable checksum offload
-		ethtool -K v1 gro on
-		ethtool -K v1 tx-checksumming off
-		ip netns exec ${NS} ethtool -K v0 gro on
-		ip netns exec ${NS} ethtool -K v0 tx-checksumming off
-	} > /dev/null 2>&1
-}
-
-terminate_dut_server() {
-	local pid
-
-	# Use the shell job instead of dut_pid, which may not have been assigned
-	# yet if a signal arrived immediately after the server was started.
-	pid=$(jobs -pr %% 2> /dev/null) || true
-	if [ -z "$pid" ]; then
-		dut_pid=""
-		return
-	fi
-
-	kill -KILL %% 2> /dev/null || true
-	wait "$pid" 2> /dev/null || true
-	dut_pid=""
-}
-
-cleanup() {
-	terminate_dut_server
-	ip link del v1 2> /dev/null || true
-	ip netns del "${NS}" 2> /dev/null || true
-}
-
-wait_for_dut_server() {
-	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
-	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
-	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
-	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
-	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
-	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
-	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=$?
-	reap_dut_server
-}
-
-set -e
-trap cleanup EXIT
-trap 'exit 129' HUP
-trap 'exit 130' INT
-trap 'exit 131' QUIT
-trap 'exit 134' ABRT
-trap 'exit 143' TERM
-
-test_xdp_features
-
-exit $ret
diff --git a/tools/testing/selftests/drivers/net/hw/.gitignore b/tools/testing/selftests/drivers/net/hw/.gitignore
index 46540468a..a0cd8bc37 100644
--- a/tools/testing/selftests/drivers/net/hw/.gitignore
+++ b/tools/testing/selftests/drivers/net/hw/.gitignore
@@ -2,3 +2,6 @@
 iou-zcrx
 ncdevmem
 toeplitz
+tools/
+xdp_features
+xdp_features.skel.h
diff --git a/tools/testing/selftests/drivers/net/hw/Makefile b/tools/testing/selftests/drivers/net/hw/Makefile
index 8aebdc6fe..a5dea02ee 100644
--- a/tools/testing/selftests/drivers/net/hw/Makefile
+++ b/tools/testing/selftests/drivers/net/hw/Makefile
@@ -50,6 +50,7 @@ TEST_PROGS = \
 	tso.py \
 	userns_devmem.py \
 	uso.py \
+	xdp_features.py \
 	xdp_metadata.py \
 	xsk_reconfig.py \
 	#
@@ -80,6 +81,7 @@ YNL_GEN_FILES := \
 # end of YNL_GEN_FILES
 TEST_GEN_FILES += $(YNL_GEN_FILES)
 TEST_GEN_FILES += $(patsubst %.c,%.o,$(wildcard *.bpf.c))
+TEST_GEN_PROGS += xdp_features
 
 include ../../../lib.mk
 
@@ -93,6 +95,26 @@ include ../../../net/ynl.mk
 
 include ../../../net/bpf.mk
 
+BPFTOOL ?= bpftool
+
+CFLAGS += -I$(OUTPUT) -I$(SCRATCH_DIR)/include
+CFLAGS += -I$(top_srcdir)/tools/include
+CFLAGS += -I$(top_srcdir)/tools/arch/$(SRCARCH)/include
+CFLAGS += -I$(top_srcdir)/tools/include/uapi
+CFLAGS += -I../../../net/lib
+
+$(OUTPUT)/xdp_features.skel.h: $(OUTPUT)/xdp_features.bpf.o
+	$(call msg,GEN-SKEL,,$@)
+	$(Q)$(BPFTOOL) gen skeleton $< name xdp_features > $@
+
+$(OUTPUT)/xdp_features: xdp_features.c xdp_features.h \
+			   $(OUTPUT)/xdp_features.skel.h $(BPFOBJ)
+	$(call msg,BINARY,,$@)
+	$(Q)$(CC) $(CFLAGS) $(filter %.a %.o %.c,$^) \
+		$(LDLIBS) -lelf -lz -lpthread -o $@
+
+EXTRA_CLEAN += $(OUTPUT)/xdp_features.skel.h
+
 ifeq ($(HAS_IOURING_ZCRX),y)
 $(OUTPUT)/iou-zcrx: LDLIBS += -luring -lpthread
 endif
diff --git a/tools/testing/selftests/bpf/progs/xdp_features.c b/tools/testing/selftests/drivers/net/hw/xdp_features.bpf.c
similarity index 100%
rename from tools/testing/selftests/bpf/progs/xdp_features.c
rename to tools/testing/selftests/drivers/net/hw/xdp_features.bpf.c
diff --git a/tools/testing/selftests/bpf/xdp_features.c b/tools/testing/selftests/drivers/net/hw/xdp_features.c
similarity index 86%
rename from tools/testing/selftests/bpf/xdp_features.c
rename to tools/testing/selftests/drivers/net/hw/xdp_features.c
index a27ed6639..ccc0630bb 100644
--- a/tools/testing/selftests/bpf/xdp_features.c
+++ b/tools/testing/selftests/drivers/net/hw/xdp_features.c
@@ -1,22 +1,26 @@
 // SPDX-License-Identifier: GPL-2.0
-#include <uapi/linux/bpf.h>
-#include <uapi/linux/netdev.h>
+#include <linux/bpf.h>
+#include <linux/netdev.h>
 #include <linux/if_link.h>
-#include <signal.h>
 #include <argp.h>
+#include <errno.h>
 #include <net/if.h>
+#include <pthread.h>
+#include <signal.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
 #include <sys/socket.h>
+#include <sys/time.h>
 #include <netinet/in.h>
 #include <netinet/tcp.h>
 #include <unistd.h>
 #include <arpa/inet.h>
 #include <bpf/bpf.h>
 #include <bpf/libbpf.h>
-#include <pthread.h>
 
-#include <network_helpers.h>
-
-#include "bpf_util.h"
+#include "ksft.h"
 #include "xdp_features.skel.h"
 #include "xdp_features.h"
 
@@ -40,7 +44,63 @@ static struct env {
 
 #define BUFSIZE		128
 
-void test__fail(void) { /* for network_helpers.c */ }
+static int make_sockaddr(const char *addr_str, __u16 port,
+			 struct sockaddr_storage *addr)
+{
+	struct sockaddr_in6 *sin6 = (void *)addr;
+
+	memset(addr, 0, sizeof(*addr));
+	sin6->sin6_family = AF_INET6;
+	sin6->sin6_port = htons(port);
+	if (addr_str && inet_pton(AF_INET6, addr_str, &sin6->sin6_addr) != 1)
+		return -1;
+
+	return 0;
+}
+
+static int settimeo(int fd, int timeout_ms)
+{
+	struct timeval timeout = { .tv_sec = 3 };
+
+	if (timeout_ms > 0) {
+		timeout.tv_sec = timeout_ms / 1000;
+		timeout.tv_usec = (timeout_ms % 1000) * 1000;
+	}
+
+	if (setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeout,
+		       sizeof(timeout)) ||
+	    setsockopt(fd, SOL_SOCKET, SO_SNDTIMEO, &timeout,
+		       sizeof(timeout)))
+		return -1;
+
+	return 0;
+}
+
+static int start_server(int type, __u16 port)
+{
+	struct sockaddr_storage addr;
+	int fd, on = 1;
+
+	fd = socket(AF_INET6, type, 0);
+	if (fd < 0)
+		return -1;
+
+	if (settimeo(fd, 0) ||
+	    setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &on, sizeof(on)) ||
+	    (type == SOCK_STREAM &&
+	     setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on))) ||
+	    make_sockaddr(NULL, port, &addr) ||
+	    bind(fd, (struct sockaddr *)&addr, sizeof(struct sockaddr_in6)) ||
+	    (type == SOCK_STREAM && listen(fd, 1))) {
+		int err = errno;
+
+		close(fd);
+		errno = err;
+		return -1;
+	}
+
+	return fd;
+}
 
 static int libbpf_print_fn(enum libbpf_print_level level,
 			   const char *format, va_list args)
@@ -151,8 +211,7 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
 		}
 		break;
 	case 'D':
-		if (make_sockaddr(AF_INET6, arg, DUT_ECHO_PORT,
-				  &env.dut_addr, NULL)) {
+		if (make_sockaddr(arg, DUT_ECHO_PORT, &env.dut_addr)) {
 			fprintf(stderr,
 				"Invalid address assigned to the Device Under Test: %s\n",
 				arg);
@@ -160,8 +219,7 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
 		}
 		break;
 	case 'C':
-		if (make_sockaddr(AF_INET6, arg, DUT_CTRL_PORT,
-				  &env.dut_ctrl_addr, NULL)) {
+		if (make_sockaddr(arg, DUT_CTRL_PORT, &env.dut_ctrl_addr)) {
 			fprintf(stderr,
 				"Invalid address assigned to the Device Under Test: %s\n",
 				arg);
@@ -169,7 +227,7 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
 		}
 		break;
 	case 'T':
-		if (make_sockaddr(AF_INET6, arg, 0, &env.tester_addr, NULL)) {
+		if (make_sockaddr(arg, 0, &env.tester_addr)) {
 			fprintf(stderr,
 				"Invalid address assigned to the Tester device: %s\n",
 				arg);
@@ -213,12 +271,11 @@ static void set_env_default(void)
 	env.feature.drv_feature = NETDEV_XDP_ACT_NDO_XMIT;
 	env.feature.action = -EINVAL;
 	env.ifindex = -ENODEV;
-	strscpy(env.ifname, "unknown");
-	make_sockaddr(AF_INET6, "::ffff:127.0.0.1", DUT_CTRL_PORT,
-		      &env.dut_ctrl_addr, NULL);
-	make_sockaddr(AF_INET6, "::ffff:127.0.0.1", DUT_ECHO_PORT,
-		      &env.dut_addr, NULL);
-	make_sockaddr(AF_INET6, "::ffff:127.0.0.1", 0, &env.tester_addr, NULL);
+	snprintf(env.ifname, sizeof(env.ifname), "unknown");
+	make_sockaddr("::ffff:127.0.0.1", DUT_CTRL_PORT,
+		      &env.dut_ctrl_addr);
+	make_sockaddr("::ffff:127.0.0.1", DUT_ECHO_PORT, &env.dut_addr);
+	make_sockaddr("::ffff:127.0.0.1", 0, &env.tester_addr);
 }
 
 static void *dut_echo_thread(void *arg)
@@ -229,7 +286,7 @@ static void *dut_echo_thread(void *arg)
 	while (!exiting) {
 		struct tlv_hdr *tlv = (struct tlv_hdr *)buf;
 		struct sockaddr_storage addr;
-		socklen_t addrlen;
+		socklen_t addrlen = sizeof(addr);
 		size_t n;
 
 		n = recvfrom(sockfd, buf, sizeof(buf), MSG_WAITALL,
@@ -244,7 +301,6 @@ static void *dut_echo_thread(void *arg)
 		       (struct sockaddr *)&addr, addrlen);
 	}
 
-	pthread_exit((void *)0);
 	close(sockfd);
 
 	return NULL;
@@ -254,9 +310,8 @@ static int dut_run_echo_thread(pthread_t *t, int *sockfd)
 {
 	int err;
 
-	sockfd = start_reuseport_server(AF_INET6, SOCK_DGRAM, NULL,
-					DUT_ECHO_PORT, 0, 1);
-	if (!sockfd) {
+	*sockfd = start_server(SOCK_DGRAM, DUT_ECHO_PORT);
+	if (*sockfd < 0) {
 		fprintf(stderr,
 			"Failed creating data UDP socket on device %s\n",
 			env.ifname);
@@ -269,7 +324,7 @@ static int dut_run_echo_thread(pthread_t *t, int *sockfd)
 		fprintf(stderr,
 			"Failed creating data UDP thread on device %s: %s\n",
 			env.ifname, strerror(-err));
-		free_fds(sockfd, 1);
+		close(*sockfd);
 		return -EINVAL;
 	}
 
@@ -361,25 +416,25 @@ static int recv_msg(int sockfd, void *buf, size_t bufsize, void *val,
 static int dut_run(struct xdp_features *skel)
 {
 	int flags = XDP_FLAGS_UPDATE_IF_NOEXIST | XDP_FLAGS_DRV_MODE;
-	int state, err = 0, *sockfd, ctrl_sockfd, echo_sockfd;
+	int state = CMD_STOP, err = 0, sockfd, ctrl_sockfd, echo_sockfd;
 	struct sockaddr_storage ctrl_addr;
 	pthread_t dut_thread = 0;
-	socklen_t addrlen;
+	socklen_t addrlen = sizeof(ctrl_addr);
 
-	sockfd = start_reuseport_server(AF_INET6, SOCK_STREAM, NULL,
-					DUT_CTRL_PORT, 0, 1);
-	if (!sockfd) {
+	sockfd = start_server(SOCK_STREAM, DUT_CTRL_PORT);
+	if (sockfd < 0) {
 		fprintf(stderr,
 			"Failed creating control socket on device %s\n", env.ifname);
 		return -errno;
 	}
+	ksft_ready();
 
-	ctrl_sockfd = accept(*sockfd, (struct sockaddr *)&ctrl_addr, &addrlen);
+	ctrl_sockfd = accept(sockfd, (struct sockaddr *)&ctrl_addr, &addrlen);
 	if (ctrl_sockfd < 0) {
 		fprintf(stderr,
 			"Failed accepting connections on device %s control socket\n",
 			env.ifname);
-		free_fds(sockfd, 1);
+		close(sockfd);
 		return -errno;
 	}
 
@@ -488,7 +543,7 @@ static int dut_run(struct xdp_features *skel)
 out:
 	bpf_xdp_detach(env.ifindex, flags, NULL);
 	close(ctrl_sockfd);
-	free_fds(sockfd, 1);
+	close(sockfd);
 
 	return err;
 }
@@ -636,13 +691,14 @@ static int tester_run(struct xdp_features *skel)
 	if (err)
 		goto out;
 
+	/* Collect results before cleanup traffic can reach the tester. */
+	detected_cap = tester_collect_detected_cap(skel, ntohl(stats));
+
 	/* stop the test */
 	err = send_and_recv_msg(sockfd, CMD_STOP, NULL, 0);
 	/* send a new echo message to wake echo thread of the dut */
 	send_echo_msg();
 
-	detected_cap = tester_collect_detected_cap(skel, ntohl(stats));
-
 	fprintf(stdout, "Feature %s: [%s][%s]\n", get_xdp_feature_str(),
 		detected_cap ? GREEN("DETECTED") : RED("NOT DETECTED"),
 		env.feature.drv_feature & advertised_feature ? GREEN("ADVERTISED")
diff --git a/tools/testing/selftests/bpf/xdp_features.h b/tools/testing/selftests/drivers/net/hw/xdp_features.h
similarity index 100%
rename from tools/testing/selftests/bpf/xdp_features.h
rename to tools/testing/selftests/drivers/net/hw/xdp_features.h
diff --git a/tools/testing/selftests/drivers/net/hw/xdp_features.py b/tools/testing/selftests/drivers/net/hw/xdp_features.py
new file mode 100755
index 000000000..5eeadf803
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/hw/xdp_features.py
@@ -0,0 +1,109 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+"""Check that a device's advertised XDP features match its behavior."""
+
+import ipaddress
+from pathlib import Path
+import re
+import shlex
+
+from lib.py import bkg, cmd, ksft_eq, ksft_exit, ksft_run, NetDrvEpEnv
+
+
+ANSI_ESCAPE = re.compile(r"\x1b\[[0-9;]*m")
+RESULT = re.compile(
+    r"Feature .*: \[(NOT )?DETECTED\]\[(NOT )?ADVERTISED\]"
+)
+
+
+def mapped_address(address):
+    """Return an IPv6 or IPv4-mapped IPv6 address for xdp_features."""
+    address = ipaddress.ip_address(address)
+    if address.version == 4:
+        return f"::ffff:{address}"
+    return str(address)
+
+
+def feature_command(binary, feature, dut_addr, tester_addr, ifname,
+                    tester=False):
+    args = [str(binary)]
+    if tester:
+        args.append("-t")
+    args += ["-f", feature, "-D", dut_addr]
+    if tester:
+        args += ["-C", dut_addr]
+    args += ["-T", tester_addr, ifname]
+    return shlex.join(args)
+
+
+def run_feature(cfg, feature, ipver):
+    if not cfg.addr_v[ipver]:
+        ipver = "4" if ipver == "6" else "6"
+    cfg.require_ipver(ipver)
+    dut_addr = mapped_address(cfg.addr_v[ipver])
+    tester_addr = mapped_address(cfg.remote_addr_v[ipver])
+
+    dut_cmd = feature_command(cfg.xdp_features, feature, dut_addr,
+                              tester_addr, cfg.ifname)
+    tester_cmd = feature_command(cfg.remote_xdp_features, feature, dut_addr,
+                                 tester_addr, cfg.remote_ifname, tester=True)
+
+    with bkg(dut_cmd, exit_wait=True, ksft_ready=True):
+        result = cmd(tester_cmd, host=cfg.remote)
+
+    output = ANSI_ESCAPE.sub("", result.stdout)
+    match = RESULT.search(output)
+    if not match:
+        raise Exception(f"Unable to parse xdp_features output: {output}")
+
+    detected = match.group(1) is None
+    advertised = match.group(2) is None
+    ksft_eq(detected, advertised,
+            comment=f"{feature}: detected and advertised support")
+
+
+def test_xdp_pass(cfg):
+    run_feature(cfg, "XDP_PASS", "6")
+
+
+def test_xdp_drop(cfg):
+    run_feature(cfg, "XDP_DROP", "4")
+
+
+def test_xdp_aborted(cfg):
+    run_feature(cfg, "XDP_ABORTED", "6")
+
+
+def test_xdp_tx(cfg):
+    run_feature(cfg, "XDP_TX", "4")
+
+
+def test_xdp_redirect(cfg):
+    run_feature(cfg, "XDP_REDIRECT", "6")
+
+
+def test_xdp_ndo_xmit(cfg):
+    run_feature(cfg, "XDP_NDO_XMIT", "4")
+
+
+def main():
+    with NetDrvEpEnv(__file__, nsim_test=False) as cfg:
+        cfg.xdp_features = (Path(__file__).parent / "xdp_features").resolve()
+        cfg.remote_xdp_features = cfg.remote.deploy(
+            cfg.xdp_features.as_posix()
+        )
+
+        ksft_run([
+            test_xdp_pass,
+            test_xdp_drop,
+            test_xdp_aborted,
+            test_xdp_tx,
+            test_xdp_redirect,
+            test_xdp_ndo_xmit,
+        ], args=(cfg,))
+    ksft_exit()
+
+
+if __name__ == "__main__":
+    main()

-- 
2.43.0

Re: [PATCH net-next v4 1/2] selftests/bpf: Track test_xdp_features DUT processes

[netdev-bot+sashiko]

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

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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] `wait_for_dut_server()` replaces the previously unbounded
  readiness wait with at most 10 one-second polls and turns a timeout…
- [Medium] `cleanup()` unconditionally runs `ip link del v1` and, via the
  newly added `trap cleanup EXIT`, is now reachable on every exit path…
- [Low] Missing `Fixes:` tag.
- [Low] The signal traps stay armed while the EXIT trap (`cleanup`)
  executes.

Pre-existing issues:
- [Medium] The shell selftest reports success even when the detected XDP
  capability disagrees with the capability advertised by the driver:…

--

selftests/bpf: Track test_xdp_features DUT processes

This reworks test_xdp_features.sh so the device-under-test xdp_features
server is tracked by pid and job spec instead of being matched by name with
pidof, replaces the readiness poll loop with a bounded retry loop, and moves
teardown to an EXIT trap plus explicit signal traps.

A few questions on the new shell logic below.

> selftests/bpf: Track test_xdp_features DUT processes
>
> Track the DUT process instead of matching processes by name to avoid
> interference between concurrent tests. Fix readiness retries under
> set -e and clean up the DUT and network state on exit or interruption.
>
> Signed-off-by: Bochao Cao <bochaolucky@gmail.com>

[Severity: Low]
Should this carry a Fixes: tag?  Both defects repaired here came in with
the original script.

In the baseline, wait_for_dut_server() ran the failing command in the body
of the loop:

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

errexit exempts only the loop condition, so with set -e a DUT that is not
yet listening after one second aborted the script instead of retrying.

Also, trap cleanup 2 3 6 9 never trapped EXIT (and signal 9 cannot be
trapped), while cleanup was only reached at the end of a fully successful
run, so every [ $? -ne 0 ] && exit path left the netns, the v1 veth and a
live xdp_features process behind, and the leftover v1 then makes the next
run's ip link add v1 fail.

git blame attributes both constructs to 4dba3e7852b7 ("selftests/bpf:
introduce XDP compliance test tool"), so a
Fixes: 4dba3e7852b7 ("selftests/bpf: introduce XDP compliance test tool")
trailer would let this be selected for the affected history.

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

[ ... ]

> @@ -30,77 +31,124 @@ setup() {
>  	} > /dev/null 2>&1
>  }
>  
> +terminate_dut_server() {
> +	local pid
> +
> +	# Use the shell job instead of dut_pid, which may not have been assigned
> +	# yet if a signal arrived immediately after the server was started.
> +	pid=$(jobs -pr %% 2> /dev/null) || true
> +	if [ -z "$pid" ]; then
> +		dut_pid=""
> +		return
> +	fi
> +
> +	kill -KILL %% 2> /dev/null || true
> +	wait "$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
>  }

[Severity: Medium]
Can this cleanup() tear down a veth pair that belongs to a different
instance of the script?

The host side name in setup() is fixed:

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

If instance A is mid-test and instance B starts, B's ip link add v1 fails
because the name is taken.  With set -e that exits B, which now fires the
newly added trap cleanup EXIT and runs:

	ip link del v1 2> /dev/null || true

deleting A's interface while A is still using it.  The baseline never ran
cleanup on that path, since the old trap covered only signals 2/3/6/9.

Since the commit message gives "avoid interference between concurrent
tests" as the motivation, should teardown only remove interfaces this
invocation actually created, and should the veth name (and the fixed DUT
control/echo ports) become per-invocation like ${NS} already is?

Note the follow-on patch "selftests/net: Move XDP feature test to driver
framework" deletes this script and moves the test to the NetDrvEpEnv
framework, which does not create a fixed v1 veth, so this may only matter
for the intermediate state of the series.

>  
>  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
> +}

[Severity: Medium]
Is a ten second budget enough here, and should exceeding it kill the whole
run?

wait_for_dut_server() is the last command of start_dut_server(), so its
return 1 becomes that function's status, and with set -e in effect the
script terminates at the first feature case, skipping XDP_DROP,
XDP_ABORTED, XDP_TX, XDP_REDIRECT and XDP_NDO_XMIT.

The loop also sleeps after its last observation (check at i=9, then
sleep 1, then return 1), so readiness is only sampled over roughly the
first nine seconds and the tenth second is dead time.  The DUT only reaches
listen() after xdp_features__open()/__load() has verified all the XDP
programs, which on KASAN/lockdep kernels or a busy CI host can take longer
than that.

The changelog mentions only "Fix readiness retries under set -e", with no
mention of the new deadline.  Could the retry budget be documented in the
changelog, and could a timeout fail just the current case rather than the
whole script?

The later patch "selftests/net: Move XDP feature test to driver framework"
removes this loop in favour of the framework's
bkg(..., ksft_ready=True) handshake, so this too is limited to the
intermediate state of the series.

[ ... ]

>  	## 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 a bug introduced by this patch, but while the result handling is
being reshuffled: does this script ever notice a mismatch between the
detected and the advertised XDP capability?

Every case in test_xdp_features(), including this last one, derives its
result purely from the exit status of the tester process.  In
tester_run() in xdp_features.c the two values only reach stdout:

	fprintf(stdout, "Feature %s: [%s][%s]\n", get_xdp_feature_str(),
		detected_cap ? GREEN("DETECTED") : RED("NOT DETECTED"),
		env.feature.drv_feature & advertised_feature ? ...

detected_cap and advertised_feature are never compared and never influence
the returned status, so a driver that advertises a feature it does not
implement still passes.

The follow-on patch "selftests/net: Move XDP feature test to driver
framework" closes this by adding xdp_features.py with
ksft_eq(detected, advertised, ...), so at the end of the series the gap is
gone.

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

[Severity: Low]
Should cleanup() disarm the traps on entry, for example with
trap - EXIT HUP INT QUIT ABRT TERM?

The signal traps stay armed while the EXIT trap runs, and bash does not
block signals during a trap handler.  cleanup() starts with
terminate_dut_server(), which can block in wait "$pid", so a second
Ctrl-C or SIGTERM in that window dispatches the signal trap's exit and
bash terminates without resuming or re-entering the EXIT trap:

	cleanup() {
		terminate_dut_server      <-- second signal arrives here
		ip link del v1 ...        <-- never runs
		ip netns del "${NS}" ...  <-- never runs
	}

That leaves the v1 veth and the netns behind, which is the state the new
trap machinery is meant to avoid.

As with the other comments, the following patch deletes this script, so
this only applies to the intermediate state of the series.

>  
>  test_xdp_features
>

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260921-xdp-features-v4-send-v4-0-a0d4ec392cb7%40gmail.com

Re: [PATCH net-next v4 2/2] selftests/net: Move XDP feature test to driver framework

[netdev-bot+sashiko]

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

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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] `TEST_GEN_PROGS += xdp_features` in
  tools/testing/selftests/drivers/net/hw/Makefile registers the…
- [Medium] The new skeleton rule adds an unguarded host-tool dependency
  on bpftool (`BPFTOOL ?= bpftool`, `$(BPFTOOL) gen skeleton $< name…
- [Medium] Now that the test targets real NICs in drivers/net/hw, devices
  or drivers without native (DRV mode) XDP support make the helper…
- [Medium] The deleted shell harness explicitly configured both endpoints
  before running the helper (`ethtool -K v1 gro on`, `ethtool -K v1 tx-…
- [Medium] dut_run() signals readiness with ksft_ready()
  (xdp_features.c:430) and then immediately blocks in a single accept()…
- [Medium] The test violates the net-driver framework contract for the
  remote endpoint.
- [Medium] run_feature() passes the framework-configured addresses
  (cfg.addr_v / cfg.remote_addr_v, i.e. LOCAL_V4/V6 and REMOTE_V4/V6…
- [Medium] dut_run() reaches `end_thread: pthread_join(dut_thread, NULL)`
  (xdp_features.c:542) with dut_thread still 0 whenever the echo thread…
- [Medium] The new Python harness has no cleanup that guarantees the XDP
  programs are detached from the local (cfg.ifname) and remote…
- [Medium] A failed XDP attachment still detaches whatever native-mode
  program is installed, so the test can silently remove a program it…
- [Low] The patch folds undocumented correctness fixes for the existing
  helper into a commit described as a move: dut_run() now initializes…

Pre-existing issues:
- [Medium] The privileged DUT process exposes its control protocol on all
  addresses and accepts commands from the first unauthenticated peer:…
- [Low] The control protocol assumes each send()/recv() transfers exactly
  one complete TLV: recv_msg() performs a single recv() and rejects…
- [Low] The shared termination flag is `static volatile bool exiting;`
  (xdp_features.c:113).

--

selftests/net: Move XDP feature test to driver framework

This moves the xdp_features helper and its BPF program from selftests/bpf
into selftests/drivers/net/hw and replaces the old shell wrapper with a
NetDrvEpEnv based Python test that drives the configured local and remote
interfaces.  It also samples the detected capability before the cleanup
echo traffic and turns a detected/advertised mismatch into a test failure.

A few questions below about the new build wiring, the dropped setup steps
and the new process orchestration.

> diff --git a/tools/testing/selftests/bpf/test_xdp_features.sh b/tools/testing/selftests/bpf/test_xdp_features.sh
> deleted file mode 100755
> index a78e0fb9ed63..000000000000
> --- a/tools/testing/selftests/bpf/test_xdp_features.sh
> +++ /dev/null
> @@ -1,155 +0,0 @@

[ ... ]

> -		# Enable XDP mode and disable checksum offload
> -		ethtool -K v1 gro on
> -		ethtool -K v1 tx-checksumming off
> -		ip netns exec ${NS} ethtool -K v0 gro on
> -		ip netns exec ${NS} ethtool -K v0 tx-checksumming off
> -	} > /dev/null 2>&1
> -}

[Severity: Medium]
These four ethtool calls have no counterpart in xdp_features.py, and
neither xdp_features.c nor xdp_features.bpf.c does any checksum handling.
Is the setup no longer needed?

The DUT echo reply still goes through an ordinary UDP socket in
dut_echo_thread():

	n = recvfrom(sockfd, buf, sizeof(buf), MSG_WAITALL,
		     (struct sockaddr *)&addr, &addrlen);

With NetDrvEpEnv pointed at a veth whose peer lives in a netns
(nsim_test=False excludes netdevsim only), both ends keep their default
offload settings.  Can the echo dependent subtests (XDP_PASS, XDP_TX,
XDP_REDIRECT, XDP_NDO_XMIT) then report NOT DETECTED on a working driver,
which the new ksft_eq(detected, advertised) turns into a hard failure?

[ ... ]

> diff --git a/tools/testing/selftests/drivers/net/hw/Makefile b/tools/testing/selftests/drivers/net/hw/Makefile
> index 8aebdc6feb17..a5dea02ee476 100644
> --- a/tools/testing/selftests/drivers/net/hw/Makefile
> +++ b/tools/testing/selftests/drivers/net/hw/Makefile

[ ... ]

> @@ -80,6 +81,7 @@ YNL_GEN_FILES := \
>  # end of YNL_GEN_FILES
>  TEST_GEN_FILES += $(YNL_GEN_FILES)
>  TEST_GEN_FILES += $(patsubst %.c,%.o,$(wildcard *.bpf.c))
> +TEST_GEN_PROGS += xdp_features

[Severity: Medium]
Is TEST_GEN_PROGS the right list for this helper?  lib.mk expands
$(call RUN_TESTS, $(TEST_GEN_PROGS) ...) in the run_tests target and
emit_tests iterates $(TEST_GEN_PROGS) into kselftest-list.txt, so
./xdp_features is also executed bare, with no arguments.

Without the positional interface argument, env.ifindex keeps the -ENODEV
default installed by set_env_default() and main() returns non-zero:

	if (env.ifindex < 0) {
		fprintf(stderr, "Invalid device name %s\n", env.ifname);
		return -ENODEV;
	}

Does that add a permanently failing drivers/net/hw:xdp_features entry
alongside the real xdp_features.py results?  The sibling helpers in this
same Makefile (ncdevmem, toeplitz, iou-zcrx) use TEST_GEN_FILES, and the
old selftests/bpf location used TEST_GEN_PROGS_EXTENDED.

>  
>  include ../../../lib.mk
>  
> @@ -93,6 +95,26 @@ include ../../../net/ynl.mk
>  
>  include ../../../net/bpf.mk
>  
> +BPFTOOL ?= bpftool
> +
> +CFLAGS += -I$(OUTPUT) -I$(SCRATCH_DIR)/include
> +CFLAGS += -I$(top_srcdir)/tools/include
> +CFLAGS += -I$(top_srcdir)/tools/arch/$(SRCARCH)/include
> +CFLAGS += -I$(top_srcdir)/tools/include/uapi
> +CFLAGS += -I../../../net/lib
> +
> +$(OUTPUT)/xdp_features.skel.h: $(OUTPUT)/xdp_features.bpf.o
> +	$(call msg,GEN-SKEL,,$@)
> +	$(Q)$(BPFTOOL) gen skeleton $< name xdp_features > $@

[Severity: Medium]
Should bpftool be built or at least probed here?  ../../../net/bpf.mk
provides only the clang rules for *.bpf.c plus libbpf.a; it defines no
BPFTOOL and no in-tree bpftool build.  On a host that has clang but no
bpftool installed, does the whole drivers/net/hw selftest build now fail
on this rule?

selftests/bpf, selftests/hid and selftests/sched_ext build bpftool from
tools/bpf/bpftool, and selftests/exec probes it with command -v via
HAVE_BPF_TOOLCHAIN.  Was adding a new unconditional host tool requirement
for this directory intended?  It is not mentioned in the changelog.

> +
> +$(OUTPUT)/xdp_features: xdp_features.c xdp_features.h \
> +			   $(OUTPUT)/xdp_features.skel.h $(BPFOBJ)
> +	$(call msg,BINARY,,$@)
> +	$(Q)$(CC) $(CFLAGS) $(filter %.a %.o %.c,$^) \
> +		$(LDLIBS) -lelf -lz -lpthread -o $@

[ ... ]

> diff --git a/tools/testing/selftests/bpf/xdp_features.c b/tools/testing/selftests/drivers/net/hw/xdp_features.c
> similarity index 86%
> rename from tools/testing/selftests/bpf/xdp_features.c
> rename to tools/testing/selftests/drivers/net/hw/xdp_features.c
> index a27ed663967c..ccc0630bb6cf 100644
> --- a/tools/testing/selftests/bpf/xdp_features.c
> +++ b/tools/testing/selftests/drivers/net/hw/xdp_features.c

[ ... ]

> @@ -40,7 +44,63 @@ static struct env {
>  
>  #define BUFSIZE		128
>  
> -void test__fail(void) { /* for network_helpers.c */ }

[ ... ]

> +static int settimeo(int fd, int timeout_ms)
> +{
> +	struct timeval timeout = { .tv_sec = 3 };
> +
> +	if (timeout_ms > 0) {
> +		timeout.tv_sec = timeout_ms / 1000;
> +		timeout.tv_usec = (timeout_ms % 1000) * 1000;
> +	}
> +
> +	if (setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeout,
> +		       sizeof(timeout)) ||
> +	    setsockopt(fd, SOL_SOCKET, SO_SNDTIMEO, &timeout,
> +		       sizeof(timeout)))
> +		return -1;
> +
> +	return 0;
> +}
> +
> +static int start_server(int type, __u16 port)
> +{
> +	struct sockaddr_storage addr;
> +	int fd, on = 1;
> +
> +	fd = socket(AF_INET6, type, 0);
> +	if (fd < 0)
> +		return -1;
> +
> +	if (settimeo(fd, 0) ||
> +	    setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &on, sizeof(on)) ||
> +	    (type == SOCK_STREAM &&
> +	     setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on))) ||
> +	    make_sockaddr(NULL, port, &addr) ||
> +	    bind(fd, (struct sockaddr *)&addr, sizeof(struct sockaddr_in6)) ||
> +	    (type == SOCK_STREAM && listen(fd, 1))) {

[Severity: Medium]
This isn't a bug introduced by this patch, but it is worth noting since
the exposure changes.  make_sockaddr(NULL, port, &addr) memsets the
address and therefore binds in6addr_any, and dut_run() accepts the first
connection without checking ctrl_addr, exactly as the old
start_reuseport_server(..., NULL, DUT_CTRL_PORT, ...) did.

With the control channel now living on a configured real interface rather
than a private veth, any host that can reach DUT_CTRL_PORT can connect
first and send CMD_START, making the root-privileged process attach the
XDP program, or simply occupy the single accepted connection.

If that peer then closes the connection, recv() returns 0, recv_msg()
maps it to -EINVAL through the len < sizeof(*tlv) test, and the CTRL loop
does:

		err = recv_msg(ctrl_sockfd, buf, BUFSIZE, NULL, 0);
		if (err)
			continue;

Would that spin at 100% CPU until the process is signalled?  Should the
DUT bind to the configured local address and validate the accepted peer?

[ ... ]

> @@ -229,7 +286,7 @@ static void *dut_echo_thread(void *arg)
>  	while (!exiting) {
>  		struct tlv_hdr *tlv = (struct tlv_hdr *)buf;
>  		struct sockaddr_storage addr;
> -		socklen_t addrlen;
> +		socklen_t addrlen = sizeof(addr);
>  		size_t n;

[Severity: Low]
This is a pre-existing issue and not introduced by this patch, but the
loop condition here reads the shared flag declared as:

	static volatile bool exiting;

which is written both from sig_handler() and from the DUT control thread
on CMD_STOP while this worker reads it.  volatile constrains compiler
access only; it is neither a valid asynchronous signal object
(volatile sig_atomic_t or a lock-free atomic) nor an inter-thread
synchronisation primitive.  Would sig_atomic_t or an atomic be the right
type here?

>  
>  		n = recvfrom(sockfd, buf, sizeof(buf), MSG_WAITALL,
> @@ -244,7 +301,6 @@ static void *dut_echo_thread(void *arg)
>  		       (struct sockaddr *)&addr, addrlen);
>  	}
>  
> -	pthread_exit((void *)0);
>  	close(sockfd);
>  
>  	return NULL;
> @@ -254,9 +310,8 @@ static int dut_run_echo_thread(pthread_t *t, int *sockfd)
>  {
>  	int err;
>  
> -	sockfd = start_reuseport_server(AF_INET6, SOCK_DGRAM, NULL,
> -					DUT_ECHO_PORT, 0, 1);
> -	if (!sockfd) {
> +	*sockfd = start_server(SOCK_DGRAM, DUT_ECHO_PORT);
> +	if (*sockfd < 0) {
>  		fprintf(stderr,
>  			"Failed creating data UDP socket on device %s\n",
>  			env.ifname);

[ ... ]

> @@ -361,25 +416,25 @@ static int recv_msg(int sockfd, void *buf, size_t bufsize, void *val,
>  static int dut_run(struct xdp_features *skel)
>  {
>  	int flags = XDP_FLAGS_UPDATE_IF_NOEXIST | XDP_FLAGS_DRV_MODE;
> -	int state, err = 0, *sockfd, ctrl_sockfd, echo_sockfd;
> +	int state = CMD_STOP, err = 0, sockfd, ctrl_sockfd, echo_sockfd;
>  	struct sockaddr_storage ctrl_addr;
>  	pthread_t dut_thread = 0;
> -	socklen_t addrlen;
> +	socklen_t addrlen = sizeof(ctrl_addr);

[Severity: Low]
The changelog describes a move plus the result sampling reorder, but this
hunk and the preceding ones also carry behavioural fixes for the existing
helper:

  - state is now initialised to CMD_STOP instead of being used
    uninitialised in the CTRL loop
  - addrlen is now initialised before accept() here, and before
    recvfrom() in dut_echo_thread()
  - dut_run_echo_thread() now assigns *sockfd instead of overwriting its
    local parameter, which previously left echo_sockfd uninitialised in
    the caller and leaked the listening fd
  - the dead pthread_exit((void *)0) that made the following
    close(sockfd) unreachable is removed

Could these be split into separate preceding patches with a Fixes tag
for 4dba3e7852b7 ("selftests/bpf: introduce XDP compliance test tool"),
so they are visible to maintainers and to stable backports?

The new ksft_eq(detected, advertised) assertion in xdp_features.py is
also a behaviour change in the same commit: disagreements that were
previously only printed now fail the test.

[Severity: Medium]
dut_thread stays 0 until a CMD_START is processed, and the end_thread
label is reached unconditionally:

end_thread:
	pthread_join(dut_thread, NULL);
out:
	bpf_xdp_detach(env.ifindex, flags, NULL);
	close(ctrl_sockfd);
	close(sockfd);

Can dut_run() reach that join with dut_thread still 0, for instance when
the SIGTERM handler sets exiting before any CMD_START arrives?  glibc's
pthread_join() casts the handle to struct pthread * and dereferences it
with no validity check, so joining 0 faults and the bpf_xdp_detach() and
close() cleanup under out: is skipped.

The new harness makes this path routine: bkg(exit_wait=True) sends
SIGTERM to the DUT whenever the tester command raises, and because
terminate is set, bkg defaults fail to False and the resulting crash is
swallowed.  Should the join be conditional on dut_thread having been
created?

>  
> -	sockfd = start_reuseport_server(AF_INET6, SOCK_STREAM, NULL,
> -					DUT_CTRL_PORT, 0, 1);
> -	if (!sockfd) {
> +	sockfd = start_server(SOCK_STREAM, DUT_CTRL_PORT);
> +	if (sockfd < 0) {
>  		fprintf(stderr,
>  			"Failed creating control socket on device %s\n", env.ifname);
>  		return -errno;
>  	}
> +	ksft_ready();
>  
> -	ctrl_sockfd = accept(*sockfd, (struct sockaddr *)&ctrl_addr, &addrlen);
> +	ctrl_sockfd = accept(sockfd, (struct sockaddr *)&ctrl_addr, &addrlen);
>  	if (ctrl_sockfd < 0) {
>  		fprintf(stderr,
>  			"Failed accepting connections on device %s control socket\n",
>  			env.ifname);
> -		free_fds(sockfd, 1);
> +		close(sockfd);
>  		return -errno;
>  	}

[Severity: Medium]
Is three seconds enough for this accept()?  The listening socket carries
the SO_RCVTIMEO installed by start_server() through settimeo(fd, 0):

	struct timeval timeout = { .tv_sec = 3 };

Linux applies SO_RCVTIMEO to accept() (inet_csk_accept() ->
inet_csk_wait_for_connect() returns -EAGAIN), and there is no
EAGAIN/EINTR retry here, so the DUT prints "Failed accepting
connections" and returns.

The countdown starts at ksft_ready(), and only then does the harness
proceed:

    with bkg(dut_cmd, exit_wait=True, ksft_ready=True):
        result = cmd(tester_cmd, host=cfg.remote)

So spawning ssh to the remote plus the tester's own skeleton open/load
and XDP attach all have to fit in that window.  On a loaded machine can
this time out and abort the DUT?

[Severity: Low]
This isn't a bug introduced by this patch either, since recv_msg() and
send_and_recv_msg() are moved verbatim, but the control protocol assumes
one complete TLV per send()/recv().  recv_msg() does a single recv() and
discards anything short without keeping the prefix:

	if (len != ntohs(tlv->len) || len < sizeof(*tlv))
		return -EINVAL;

and send_and_recv_msg() treats any positive send() return as a complete
write.  For 4 to 16 byte messages on an idle connected socket a short
transfer looks unreachable in practice, so this is noted only as
context.

[ ... ]

> @@ -488,7 +543,7 @@ static int dut_run(struct xdp_features *skel)
>  out:
>  	bpf_xdp_detach(env.ifindex, flags, NULL);
>  	close(ctrl_sockfd);
> -	free_fds(sockfd, 1);
> +	close(sockfd);
>  
>  	return err;
>  }

[Severity: Medium]
Can this cleanup detach an XDP program the test does not own?  Both
dut_attach_xdp_prog() and tester_run() attach with
XDP_FLAGS_UPDATE_IF_NOEXIST, so if the interface already has a native
program the attach fails with -EBUSY and both error paths fall through to
this unconditional bpf_xdp_detach(env.ifindex, flags, NULL).

libbpf implements detach as an attach with prog_fd -1, and the
UPDATE_IF_NOEXIST guard in dev_xdp_attach() only applies when new_prog is
non-NULL:

	if (new_prog) {
		if ((flags & XDP_FLAGS_UPDATE_IF_NOEXIST) && cur_prog) {
			NL_SET_ERR_MSG(extack, "XDP program already attached");
			return -EBUSY;
		}
	}

So with new_prog == NULL the pre-existing foreign program is removed.
On the shared remote endpoint that would also break a concurrent user.
Should the cleanup remember whether its own attach succeeded, or pass
old_prog_fd?

> @@ -636,13 +691,14 @@ static int tester_run(struct xdp_features *skel)
>  	if (err)
>  		goto out;
>  
> +	/* Collect results before cleanup traffic can reach the tester. */
> +	detected_cap = tester_collect_detected_cap(skel, ntohl(stats));
> +
>  	/* stop the test */
>  	err = send_and_recv_msg(sockfd, CMD_STOP, NULL, 0);
>  	/* send a new echo message to wake echo thread of the dut */
>  	send_echo_msg();
>  
> -	detected_cap = tester_collect_detected_cap(skel, ntohl(stats));
> -
>  	fprintf(stdout, "Feature %s: [%s][%s]\n", get_xdp_feature_str(),
>  		detected_cap ? GREEN("DETECTED") : RED("NOT DETECTED"),
>  		env.feature.drv_feature & advertised_feature ? GREEN("ADVERTISED")

[ ... ]

> diff --git a/tools/testing/selftests/drivers/net/hw/xdp_features.py b/tools/testing/selftests/drivers/net/hw/xdp_features.py
> new file mode 100755
> index 000000000000..5eeadf8033d8
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/hw/xdp_features.py
> @@ -0,0 +1,109 @@

[ ... ]

> +def run_feature(cfg, feature, ipver):
> +    if not cfg.addr_v[ipver]:
> +        ipver = "4" if ipver == "6" else "6"
> +    cfg.require_ipver(ipver)
> +    dut_addr = mapped_address(cfg.addr_v[ipver])
> +    tester_addr = mapped_address(cfg.remote_addr_v[ipver])

[Severity: Medium]
These addresses come from net.config (LOCAL_V4/V6, REMOTE_V4/V6) and are
handed to the helper as the exact addresses expected on the wire, but
send_echo_msg() creates an unbound UDP socket:

	sockfd = socket(AF_INET6, SOCK_DGRAM, 0);
	...
	n = sendto(sockfd, buf, sizeof(*tlv), MSG_NOSIGNAL | MSG_CONFIRM,
		   (struct sockaddr *)&env.dut_addr, sizeof(env.dut_addr));

NetDrvEpEnv does not require the configured address to be the interface's
preferred source address.  If REMOTE_V4/REMOTE_V6 is a secondary address,
the kernel picks a different source and the DUT filter rejects the probe:

		if (saddr != ih->saddr)
			return -EINVAL;

dut_stats then stays 0, detected is false while advertised is true, and
ksft_eq() fails on a working driver.  Should the probe socket bind to
env.tester_addr first?

> +
> +    dut_cmd = feature_command(cfg.xdp_features, feature, dut_addr,
> +                              tester_addr, cfg.ifname)
> +    tester_cmd = feature_command(cfg.remote_xdp_features, feature, dut_addr,
> +                                 tester_addr, cfg.remote_ifname, tester=True)

[Severity: Medium]
Does running the helper on cfg.remote with cfg.remote_ifname fit the
framework contract?  drivers/net/README.rst says "Local host is the DUT
... remote is just for traffic generation" and "Avoid making
configuration changes to the remote system as much as possible.  Remote
system may be used concurrently by multiple DUTs."

tester_run() attaches an XDP program to the remote interface with
XDP_FLAGS_DRV_MODE | XDP_FLAGS_UPDATE_IF_NOEXIST, so a perfectly good
local DUT result now depends on the remote NIC supporting native XDP and
having no program attached, and two concurrent runs from different DUTs
would collide.  With the old script the peer was a private veth, so this
dependency did not exist.

> +
> +    with bkg(dut_cmd, exit_wait=True, ksft_ready=True):
> +        result = cmd(tester_cmd, host=cfg.remote)

[Severity: Medium]
Should a device without native (DRV mode) XDP support be skipped rather
than failed?  Both ends attach with XDP_FLAGS_DRV_MODE, so on such a NIC
the tester fails in tester_run():

	err = bpf_xdp_attach(env.ifindex, bpf_program__fd(prog), flags, NULL);
	if (err) {
		fprintf(stderr, "Failed attaching XDP program to device %s\n",
			env.ifname);
		goto out;

and the DUT fails the same way in dut_attach_xdp_prog() on CMD_START,
which makes the tester's send_and_recv_msg(sockfd, CMD_START, ...) fail.
cmd() here uses the default fail=True, so a non-zero exit raises
CmdExitFailure, and the "Unable to parse xdp_features output" path raises
a bare Exception in the same situation.

The changelog says tests fail "when detected and advertised features
disagree", but with no capability pre-check (say, querying
NETDEV_XDP_ACT_BASIC on both interfaces and skipping) don't NICs that
simply lack the feature produce errors instead?

> +
> +    output = ANSI_ESCAPE.sub("", result.stdout)
> +    match = RESULT.search(output)
> +    if not match:
> +        raise Exception(f"Unable to parse xdp_features output: {output}")
> +
> +    detected = match.group(1) is None
> +    advertised = match.group(2) is None
> +    ksft_eq(detected, advertised,
> +            comment=f"{feature}: detected and advertised support")

[ ... ]

> +def main():
> +    with NetDrvEpEnv(__file__, nsim_test=False) as cfg:
> +        cfg.xdp_features = (Path(__file__).parent / "xdp_features").resolve()
> +        cfg.remote_xdp_features = cfg.remote.deploy(
> +            cfg.xdp_features.as_posix()
> +        )

[Severity: Medium]
Is there anything here that guarantees the XDP programs are detached from
cfg.ifname and cfg.remote_ifname if a helper survives the test?  The
remote tester runs with the default cmd() timeout of 20 seconds while
tester_run() itself sleeps ten times one second on top of BPF load and
attach, and on timeout cmd._process_terminate() does:

    def _process_terminate(self, terminate, timeout):
        if terminate:
            self.proc.terminate()
        stdout, stderr = self.proc.communicate(timeout=timeout)

With terminate False, communicate() raises TimeoutExpired without ever
killing the child, so the ssh session and the remote xdp_features keep
running with the program still attached.  Since the helper attaches with
XDP_FLAGS_UPDATE_IF_NOEXIST, would every following subtest then fail with
-EBUSY?

The old harness was immune because everything lived in a netns that
cleanup() deleted.  Could a defer() forcing xdp off on both hosts (and a
kill of the remote helper) be registered here?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260921-xdp-features-v4-send-v4-0-a0d4ec392cb7%40gmail.com