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

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

On Thu, Oct 15, 2026 at 12:29:30PM +0200, Björn Töpel wrote:
> Maciej Fijalkowski <maciej.fijalkowski@intel.com> writes:
> 
> > Run the AF_XDP cases that a physical link can carry as pairs of
> > xskxceiver processes: a zero-copy endpoint on the DUT and an SKB-mode
> > endpoint on the remote, which needs no zero-copy support. xsk.py runs
> > the cases that xskxceiver -L lists, in the DUT directions listed with
> > them, as KTAP variants named <rx|tx>_<CASE>. test_xsk runs them with
> > the DUT socket driven by interrupts and test_xsk_busy_poll with it busy
> > polling, after xsk_setup_busy_poll() sets napi_defer_hard_irqs and
> > gro_flush_timeout on the DUT so that SO_PREFER_BUSY_POLL keeps its IRQs
> > masked, as on veth.
> >
> > main() sets up what the cases share once and undoes it after them, as
> > defer() works only within a case, so xsk.py rejects -l and -h, on which
> > ksft_run() exits without returning:
> >
> > - the DUT endpoint binds the last queue with both an RX and a TX ring,
> >   which is taken out of RSS with ethtool -X equal and handed back with
> >   ethtool -X default; a TX endpoint fills no RX buffers, so in
> >   zero-copy mode its queue drops what it receives and TX cases need the
> >   queue out of RSS as much as RX cases do. A DUT with fewer than two
> >   such queues skips
> > - an ntuple rule on each host steers the test UDP flow to the receiving
> >   endpoint: the DUT queue in RX cases and queue 0 of the remote, where
> >   its endpoint binds, in TX cases. A host without ntuple support skips.
> >   Setting the rules once spares each TX case the ssh round trips of a
> >   remote rule and toggles ntuple-filters, which resets some NICs, once
> >   per run rather than per case
> > - huge pages are reserved on both hosts, and the UNALIGNED cases are
> >   left out without them
> > - both endpoints get the DUT's xdp-zc-max-segs for TOO_MANY_FRAGS,
> >   which is left out without multi-buffer zero-copy
> >
> > A DUT that does not advertise AF_XDP zero-copy skips the run.
> >
> > The remote endpoint listens on REMOTE_V4, and the DUT endpoint connects
> > once the remote says on stderr that it listens. The control connection
> > shares the tested link, where the XDP programs pass it to the stack.
> > Killing an ssh client does not stop the command it runs, so timeout(1)
> > bounds the remote endpoint, and the runner waits for it to exit before
> > the case ends. The endpoints are waited for with the test_xsk.py
> > helpers, so the output of one that does not pass is logged, timeouts
> > included.
> >
> > The endpoints detach their XDP programs and restore the MTU and ring
> > sizes when they exit; the runner detaches the programs and restores the
> > MTU only after one fails or is killed. Each endpoint sees only its own
> > device and mode, so one may skip a case that the other runs and then
> > fails once its peer is gone; such a case is reported as skipped.
> >
> > xsk.py runs the xskxceiver that net/lib builds, which drv-net already
> > installs, and copies it to the remote with cfg.remote.deploy(). Add
> > xsk.py to TEST_PROGS and to the AF_XDP entry in MAINTAINERS,
> > CONFIG_XDP_SOCKETS to the drivers/net/hw config, and the xskxceiver
> > helpers to the drivers/net/hw library exports.
> >
> > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> 
> I just skimmed this, and it seems like it needs some scrutiny on the
> cleanup side. The LLM flagged a few cleanup issues. Could you check
> these paths?
> 
>   | main() only restores shared state on the normal path. Exceptions
>   | during setup can leave RSS, steering rules or hugepage reservations
>   | changed. Would a run-scoped ExitStack help here?

If there would be no objections to ExitStack usage, then I'm gonna do it

>   |
>   | _steer_udp() enables ntuple filtering before recording its original
>   | state. If rule insertion fails, that change isn't tracked.
>   |
>   | _unsteer_udp() stops on the first error, leaving subsequent cleanup
>   | undone.

dang you nit-picking AI; these are valid tho; maybe these come from a
desire to setup steering once, not per each test-case, so I couldn't have
a matching defer()...

For gro_stats.py it's not a big deal but here we run ~80 test cases, so
pulling out things that can be configured once per ksft_run() is important
from execution time POV.

>   |
>   | _run() waits for the remote in finally, but doesn't ensure the local
>   | endpoint is stopped/reaped if its wait is interrupted.
>   |
>   | The fallback cleanup restores XDP/MTU, but not ring sizes after an
>   | endpoint is killed.

this one is also valid as cleanup_iface() wouldn't be reached - i assume
it would be nicer to setup ring sizes from python side now; i could get
back at passing the test name and based on that issue ethtool calls rather
than plain ioctls from C. We might pass the info of ring size via flag
when calling xskxceiver.

I'll see if this simplify anything or make things worse.

>   |
>   | "-X default" resets RSS rather than restoring the original table.

It means someone else could have modified the table before reserving the
last queue - should we respect it? But that sounds to me like other tests
would be doing bad job at cleaning up after themselves. My assumption was
that we start with clean state.

>   |
>   | An unconditional SKIP when either endpoint skips can hide a genuine
>   | failure on the other endpoint, including a crash or cleanup failure.
> 
> The defer ordering itself looks sensible: detach XDP before restoring
> the MTU.
> 
> 
> Björn