Re: [PATCH net-next v6 5/5] selftests/net: Move the XDP feature test to drivers/net/hw

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The patch changes Remote.deploy() in
  tools/testing/selftests/drivers/net/lib/py/remote_ssh.py from…
- [Low] In dut_run()
  (tools/testing/selftests/drivers/net/hw/xdp_features.c line 490), the…

--

selftests/net: Move the XDP feature test to drivers/net/hw

This commit replaces the veth-based test_xdp_features.sh script in
selftests/bpf with a NetDrvEpEnv Python test under drivers/net/hw. The
xdp_features helper is now built there with the shared BPF rules. The
commit also changes Remote.deploy() to pass scp arguments as a list.

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

[ ... ]

> @@ -484,6 +485,7 @@ static int dut_run(struct xdp_features *skel)
>  			"Failed creating control socket on device %s\n", env.ifname);
>  		return -errno;
>  	}
> +	ksft_ready();
>  
>  	/* Allow remote startup and BPF loading on slow test machines. */

[Severity: Low]
This isn't a bug, but is this comment above the accept() retry loop in
dut_run() still accurate?

Nothing loads BPF while this loop waits. tester_run() on the remote side
only uses TCP, UDP and AF_PACKET sockets. On the DUT, main() already runs
xdp_features__open(), xdp_features__load() and xdp_features__attach()
before it calls dut_run().

The README added by this patch also says the remote helper "does not load
BPF".

The comment came in with the earlier series patch "selftests/bpf: Send XDP
feature probes with AF_PACKET". That patch took BPF out of the tester, so
the comment was already stale when it was added, and it stays stale here.

>  	ctrl_sockfd = -1;

[ ... ]

> diff --git a/tools/testing/selftests/drivers/net/lib/py/remote_ssh.py b/tools/testing/selftests/drivers/net/lib/py/remote_ssh.py
> index 924addde19a3e..5e2950a7c95fa 100644
> --- a/tools/testing/selftests/drivers/net/lib/py/remote_ssh.py
> +++ b/tools/testing/selftests/drivers/net/lib/py/remote_ssh.py
> @@ -35,5 +35,5 @@ class Remote:
>          if not os.path.isabs(what):
>              what = os.path.abspath(self.dir_path + "/" + what)
>  
> -        cmd(f"scp {what} {self.name}:{file_name}")
> +        cmd(["scp", what, f"{self.name}:{file_name}"])

[Severity: Low]
This change looks like a standalone fix for the shared Remote.deploy()
helper. It isn't really part of moving the XDP feature test.

When cmd() gets a string with spaces, it splits it in
net/lib/py/utils.py:

    if shell is None and isinstance(comm, str) and ' ' in comm:
        comm = comm.split()

So a local path that contains spaces reaches scp as several arguments.
Existing callers already hit this, for example main() in
drivers/net/hw/csum.py:

    cfg.bin_remote = cfg.remote.deploy(cfg.bin_local)

Would it make sense to split this out into its own patch with a Fixes: tag
for commit 1a20a9a0ddef ("selftests: drv-net: define endpoint structures"),
where the scp string was introduced? That would make the fix easier to find
and backport.

>          return file_name

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261018-xdp-v5-send-20261008-v6-0-7336e1282ca3%40gmail.com