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

[Maciej Fijalkowski <maciej.fijalkowski@intel.com>]

On Sun, Oct 18, 2026 at 10:05:54PM +0800, Bochao Cao via B4 Relay wrote:
> From: Bochao Cao <bochaolucky@gmail.com>
> 
> Replace the private-veth script with a NetDrvEpEnv test for a local DUT
> and remote tester. Build the helper with the shared BPF rules and
> installed kernel headers.
> 
> Compare detected and advertised features, wait for DUT readiness and
> bound the remote helper lifetime. Pass local commands and scp arguments
> as lists so paths containing spaces work.
> 
> Run veth XDP_TX when peer GRO or native XDP provides NAPI. Skip only
> when neither is present. Read GRO with ethtool without changing the
> peer configuration.

why is veth so special cased here? On my side [0] I added separate test
runners: selftests/net python runner runs test tool on veth pair and
selftests/drivers/net/hw is dedicated for HW-backed netdevs.

Because I assume the move of this tool is to open this up for running
against actual HW?

Also, seems two tools would be introducing similar functionality which is
a TCP control channel. Maybe we could commonize this? I'll ask LLM later
about it :P I have not looked into xdp_features C side.

Furthermore, we are gonna have a conflict with skeleton building
enablement within Makefile, but that's heads-up to maintainers.

[0]: https://lore.kernel.org/netdev/20261016174338.787135-1-maciej.fijalkowski@intel.com/
> 
> Tested the runner's skip, failure and pass paths on Ubuntu 6.8 with
> ethtool 5.16. With isolated veth modules, IPv4 and IPv6 probes receive
> 0/10 replies before the GRO-only XDP_TX flush fix and 10/10 after it.
> Existing peer XDP programs and offload settings are preserved. SSH
> deployment and execution from a path with spaces and a quote also pass.
> 
> Signed-off-by: Bochao Cao <bochaolucky@gmail.com>
> ---
>  tools/testing/selftests/bpf/.gitignore             |   1 -
>  tools/testing/selftests/bpf/Makefile               |   9 +-
>  tools/testing/selftests/bpf/test_xdp_features.sh   | 175 ---------------------
>  tools/testing/selftests/drivers/net/README.rst     |  19 +++
>  tools/testing/selftests/drivers/net/hw/.gitignore  |   3 +
>  tools/testing/selftests/drivers/net/hw/Makefile    |  15 +-
>  tools/testing/selftests/drivers/net/hw/config      |   3 +
>  .../net/hw/xdp_features.bpf.c}                     |   0
>  .../{bpf => drivers/net/hw}/xdp_features.c         |   2 +
>  .../{bpf => drivers/net/hw}/xdp_features.h         |   0
>  .../selftests/drivers/net/hw/xdp_features.py       | 146 +++++++++++++++++
>  .../selftests/drivers/net/lib/py/remote_ssh.py     |   2 +-
>  12 files changed, 189 insertions(+), 186 deletions(-)
> 

[...]

> 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 0000000000000..dda114033b8ab
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/hw/xdp_features.py
> @@ -0,0 +1,146 @@
> +#!/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, ethtool, ip, ksft_eq, ksft_exit, ksft_run
> +from lib.py import KsftFailEx, KsftSkipEx, 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)

does it have to computed every time or could we store this once at main
within cfg's built-in dict like we do for binary paths?

> +
> +
> +def feature_command(binary, feature, dut_addr, tester_addr, ifname,
> +                    tester=False):
> +    """Build the helper command with the configured endpoint addresses."""
> +    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 args
> +
> +
> +def run_feature(cfg, feature, ipver):
> +    """Run one feature with bounded helper lifetimes and verify its result."""
> +    cfg.require_cmd("timeout", local=False, remote=True)

are there any cases where timeout would not be available on the system?
even if it needs to be kept it feels to me like a pull out candidate onto
main() and skip the whole test suite once, not per each test case.

> +    probe = cmd([str(cfg.xdp_features), "-q", cfg.ifname],
> +                fail=False)
> +    if probe.ret == 4:
> +        raise KsftSkipEx(f"{cfg.ifname} does not advertise native XDP")
> +    if probe.ret:
> +        raise KsftFailEx(f"Unable to query XDP support: {probe}")
> +    if feature == "XDP_TX":
> +        link = ip(f"-d link show dev {cfg.ifname}", json=True)[0]
> +        if link.get("linkinfo", {}).get("info_kind") == "veth":
> +            peer = ip(f"-d link show dev {cfg.remote_ifname}", json=True,
> +                      host=cfg.remote)[0]
> +            if peer.get("xdp", {}).get("mode") != 1:
> +                cfg.require_cmd("ethtool", local=False, remote=True)
> +                features = ethtool(f"-k {cfg.remote_ifname}", host=cfg.remote)

isn't the proper way to utilize ethtool commands is to do:
cfg.ethnl = EthtoolFamily()

Also this feels like it should be within veth runner and scoped at
test_xdp_tx()

> +                gro = re.search(r"^generic-receive-offload: (on|off)"
> +                                r"(?: \[fixed\])?$", features.stdout, re.M)
> +                if not gro:
> +                    raise KsftFailEx("Unable to query peer GRO state")
> +                # The peer needs NAPI to receive XDP frames.
> +                if gro.group(1) == "off":
> +                    raise KsftSkipEx("veth XDP_TX needs peer GRO or an "
> +                                     "existing peer native XDP program")
> +    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):
> +        # Bound the remote process itself, not just the local SSH wait.
> +        remote_cmd = shlex.join(["timeout", "--signal=TERM", "--kill-after=5",
> +                                 "90"] + tester_cmd)
> +        result = cmd(remote_cmd,
> +                     host=cfg.remote, timeout=100)
> +
> +    output = ANSI_ESCAPE.sub("", result.stdout)
> +    match = RESULT.search(output)
> +    if not match:
> +        raise KsftFailEx(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):
> +    """Check XDP_PASS detection against the advertised capability."""
> +    run_feature(cfg, "XDP_PASS", "6")
> +
> +
> +def test_xdp_drop(cfg):
> +    """Check XDP_DROP detection against the advertised capability."""
> +    run_feature(cfg, "XDP_DROP", "4")
> +
> +
> +def test_xdp_aborted(cfg):
> +    """Check XDP_ABORTED detection against the advertised capability."""
> +    run_feature(cfg, "XDP_ABORTED", "6")
> +
> +
> +def test_xdp_tx(cfg):
> +    """Check XDP_TX detection against the advertised capability."""
> +    run_feature(cfg, "XDP_TX", "4")
> +
> +
> +def test_xdp_redirect(cfg):
> +    """Check XDP_REDIRECT detection against the advertised capability."""
> +    run_feature(cfg, "XDP_REDIRECT", "6")
> +
> +
> +def test_xdp_ndo_xmit(cfg):
> +    """Check XDP_NDO_XMIT detection against the advertised capability."""
> +    run_feature(cfg, "XDP_NDO_XMIT", "4")

Why some are v4 and v6? I was told to be flexible and support both.
Why we map v4 to v6?

> +
> +
> +def main():
> +    """Deploy the helper and run the six feature checks."""
> +    with NetDrvEpEnv(__file__, nsim_test=False) as cfg:
> +        cfg.xdp_features = (Path(__file__).parent / "xdp_features").resolve()
> +        cfg.remote_xdp_features = cfg.remote.deploy(

maybe we could refer to these via bin_{local,remote} as other hw tests do?
but that's just a nit.

> +            cfg.xdp_features.as_posix()

what does as_posix() do and why is it needed here?

> +        )
> +
> +        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()
> 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
> @@ -33,7 +33,7 @@ class Remote:
>          file_name = self._tmpdir + "/" + self._mktmp() + os.path.basename(what)
>  
>          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}"])

agree with sashiko report this deserves to be a change on its own

>          return file_name
> 
> -- 
> Git-155)
> 
> 
>