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

["Björn Töpel" <bjorn@kernel.org>]

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?
  |
  | _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.
  |
  | _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.
  |
  | "-X default" resets RSS rather than restoring the original table.
  |
  | 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