Re: [PATCH v2 net-next 13/14] selftests: drv-net: test AF_XDP zero-copy with an SKB peer

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

On Thu, Oct 08, 2026 at 02:37:10PM -0700, Jakub Kicinski wrote:
> On Thu,  8 Oct 2026 13:49:08 +0200 Maciej Fijalkowski wrote:
> > +XSK_BIN = (Path(__file__).parent / "../../../net/lib/xskxceiver").resolve()
> 
> cfg.test_dir ? Please don't reinvent the wheel

ok I can convert from global var and base this on cfg.net_lib_dir. This
was done in this ugly form as list of test cases was generated without
creating an instance of NetDrvEpEnv.

> 
> > +XSK_CASES = XSK_BIN.parent / "xsk/test_xsk_case_defs.h"
> 
> > +def _hardware_cases():
> > +    defs = XSK_CASES.read_text()
> > +    entries = re.findall(r"(?m)^XSK_TEST_CASE\((\w+),\s*\w+,\s*([^)]+)\)", defs)
> > +    if not entries or len(entries) != defs.count("XSK_TEST_CASE("):
> > +        raise KsftFailEx(f"cannot parse {XSK_CASES}")
> > +    variants = []
> > +    for test_id, (name, hw) in enumerate(entries):
> > +        flags = {flag.strip() for flag in hw.split("|")}
> > +        if not flags <= {"0", "XSK_HW_RX", "XSK_HW_TX"}:
> > +            raise KsftFailEx(f"invalid hardware directions for {name}: {hw}")
> > +        variants += [KsftNamedVariant(f"{d}_{name.lower()}", name, d, test_id)
> > +                     for d in ("rx", "tx") if f"XSK_HW_{d.upper()}" in flags]
> > +    return variants
> 
> ?? Just list them, don't over optimize for de-duplication
> it's obviously all LLM generated now.

Looks nasty but it spits out a test case in correlation with direction we
will be testing. I wrote in cover letter that, speaking in AF_XDP ZC ice,
SEND_RECEIVE in XSK_HW_RX would focus on testing ice_clean_rx_irq_zc() and
analogously this same case in Tx direction would be testing ice_xmit_zc().

Some tests are only Rx or Tx, some don't define these flags.

> 
> > +def _netns_remote(cfg):
> > +    return cfg.env.get("REMOTE_TYPE") == "netns"
> 
> You shouldn't have to care, please don't break the abstractions
> 
> > +def _control_addr(cfg):
> > +    """An SSH remote is reached over its management link. A netns remote is
> > +    reached only over the tested link, where the XDP programs pass the
> > +    control connection to the stack."""
> > +    if _netns_remote(cfg):
> > +        return cfg.remote_addr_v["4"]
> > +    return cfg.remote.name.rpartition("@")[2]
> 
> Suspicious? Not sure why you need this. No existing test does
> something like this

I'll try to simplify the noise behind setup variables, these mostly come
from my intial local setup where I had issues on remote host, sorry about
that. Also as you say test code should not be special casing what remote
type comes from net.config.

> 
> > +def _root_cmd(command, host=None, sudo=False, **kwargs):
> > +    if sudo:
> > +        command = "sudo -n " + command
> > +    return cmd(command, host=host, **kwargs)
> 
> No hacks like this please, all networking tests can assume root
> 
> > +def _remote_binary(cfg, local_binary):
> > +    binary = cfg.env.get("XSK_REMOTE_BIN")
> 
> Why this variable? Just assume the binary in tree is the one we need?
> 
> > +    if not binary:
> > +        return cfg.remote.deploy(local_binary)
> > +    if cfg.env.get("REMOTE_TYPE") != "ssh":
> > +        raise KsftSkipEx("XSK_REMOTE_BIN requires the SSH remote backend")
> > +    if cfg.env.get("XSK_REMOTE_DEPLOY", "1") == "0":
> > +        result = cmd(f"test -x {shlex.quote(binary)}", host=cfg.remote,
> > +                     fail=False)
> > +        if result.ret:
> > +            raise KsftSkipEx(f"remote xskxceiver is not executable: {binary}")
> > +        return binary
> > +    upload = binary + ".upload"
> > +    cmd(["scp", "-q", str(local_binary), f"{cfg.remote.name}:{upload}"],
> > +        shell=False)
> > +    cmd(f"mv -f {shlex.quote(upload)} {shlex.quote(binary)}",
> > +        host=cfg.remote)
> 
> Why any of these hacks/complexity? I think you need a better LLM and
> point it at the existing tests. Weird stuff going on here :(