Re: [PATCH net-next] selftests/net: Add two xdp tests to xdp.py

[Leon Hwang <leon.hwang@linux.dev>]

On 2/4/26 04:20, Jakub Kicinski wrote:
> On Wed,  1 Apr 2026 13:27:46 +0800 Leon Hwang wrote:
[...]
>> +def _set_jumbo_mtu(cfg, mtu):
>> +    ip(f"link set dev {cfg.ifname} mtu {mtu}")
>> +    defer(ip, f"link set dev {cfg.ifname} mtu 1500")
> 
> cfg records the original MTU, you can copy this line from here:
> https://elixir.bootlin.com/linux/v7.0-rc6/source/tools/testing/selftests/drivers/net/gro.py#L88
> 

Sure, will use cfg.dev['mtu'] instead of hard code 1500.

>> +def _exec_cmd(cfg, obj, sec, ip_opts=""):
>> +    return cmd(f"ip {ip_opts} link set dev {cfg.ifname} xdpdrv obj {obj} sec {sec}", shell=True, fail=False)
> 
> We shouldn't need the shell=True where?
> Also it's probably cleaner to pass fail as optional argument here
> so that you can let this throw an exception in the cases we expect
> to succeed instead of having to check in the caller explicitly
> 

Right, shell=True is unnecessary here. Will drop it.

Will pass fail as optional arg to let it fail if fail=True.

>> +def test_xdp_native_attach_sb_to_mb(cfg):
>> +    obj = cfg.net_lib_dir / "xdp_dummy.bpf.o"
>> +    mtu = 9000
>> +
>> +    _set_jumbo_mtu(cfg, mtu)
>> +
>> +    probe = _exec_cmd(cfg, obj, "xdp.frags")
>> +    if probe.ret != 0:
>> +        output = probe.stderr.strip() or probe.stdout.strip()
>> +        raise KsftSkipEx(output or "device does not support multi-buffer XDP")
> 
> May be simpler and cleaner to print the whole command separately:
> 	
> 	if probe.ret != 0:
> 		ksft_pr(probe)
> 		raise KsftSkipEx("device does not support multi-buffer XDP")
> 

Ack.

>> +    ip(f"link set dev {cfg.ifname} xdpdrv off")
>> +
>> +    probe = _exec_cmd(cfg, obj, "xdp")
>> +    if probe.ret == 0:
>> +        ip(f"link set dev {cfg.ifname} xdpdrv off")
>> +        raise KsftFailEx(f"driver unexpectedly allows non-multi-buffer XDP at MTU {mtu}")
> 
> Hm, TBH I don't think this sb_to_mb case is adding any coverage.
> Let's just add the test case below?
> 

Ok. Will drop this case.

> 
>> +def test_xdp_native_update_mb_to_sb(cfg):
>> +    obj = cfg.net_lib_dir / "xdp_dummy.bpf.o"
>> +
>> +    _set_jumbo_mtu(cfg, 9000)
>> +
>> +    attach = _exec_cmd(cfg, obj, "xdp.frags")
>> +    if attach.ret != 0:
>> +        output = attach.stderr.strip() or attach.stdout.strip()
>> +        raise KsftSkipEx(output or "device does not support multi-buffer XDP")
>> +
>> +    defer(ip, f"link set dev {cfg.ifname} xdpdrv off")
>> +
>> +    update1 = _exec_cmd(cfg, obj, "xdp.frags", "-force")
>> +    if update1.ret != 0:
>> +        raise KsftFailEx("device fails to update multi-buffer XDP")
>> +
>> +    update2 = _exec_cmd(cfg, obj, "xdp", "-force")
>> +    if update2.ret == 0:
>> +        raise KsftFailEx("device unexpectedly updates non-multi-buffer XDP")

From sashiko's review [1]:

On architectures with larger page sizes, such as 16KB or 64KB on ARM64 or
PowerPC, a 9000-byte packet can fit entirely within a single buffer. In
these environments, the driver might legally accept a non-frag XDP program,
which would lead to a false positive failure here.

WDYT?

[1]
https://sashiko.dev/#/patchset/20260401052746.314667-1-leon.huangfu%40shopee.com

Thanks,
Leon