On Wed, Jan 29, 2025 at 06:07:51PM -0800, Jakub Kicinski wrote:
> On Wed, 29 Jan 2025 17:24:25 +0000 Joe Damato wrote:
> > Test that queues which are used for AF_XDP have the xsk attribute set.
>
> > diff --git a/tools/testing/selftests/drivers/.gitignore b/tools/testing/selftests/drivers/.gitignore
> > index 09e23b5afa96..3c109144f7ff 100644
> > --- a/tools/testing/selftests/drivers/.gitignore
> > +++ b/tools/testing/selftests/drivers/.gitignore
> > @@ -1,3 +1,4 @@
> > # SPDX-License-Identifier: GPL-2.0-only
> > /dma-buf/udmabuf
> > /s390x/uvdevice/test_uvdevice
> > +/net/xdp_helper
>
> Let's create our own gitignore, under drivers/net
> we'll get conflicts with random trees if we add to the shared one
OK, SGTM.
> > def sys_get_queues(ifname, qtype='rx') -> int:
> > folders = glob.glob(f'/sys/class/net/{ifname}/queues/{qtype}-*')
> > @@ -21,6 +24,31 @@ def nl_get_queues(cfg, nl, qtype='rx'):
> > return len([q for q in queues if q['type'] == qtype])
> > return None
> >
> > +def check_xdp(cfg, nl, xdp_queue_id=0) -> None:
> > + test_dir = os.path.dirname(os.path.realpath(__file__))
> > + xdp = subprocess.Popen([f"{test_dir}/xdp_helper", f"{cfg.ifindex}", f"{xdp_queue_id}"],
> > + stdin=subprocess.PIPE, stdout=subprocess.PIPE, bufsize=1,
> > + text=True)
>
> add:
> defer(xdp.kill)
>
> here, to make sure test cleanup will always try to kill the process,
> then you can remove the xdp.kill() at the end
OK, will do.
> > + stdout, stderr = xdp.communicate(timeout=10)
> > + rx = tx = False
> > +
> > + queues = nl.queue_get({'ifindex': cfg.ifindex}, dump=True)
> > + if queues:
>
> if not queues:
> raise KsftSkipEx("Netlink reports no queues")
>
> That said only reason I can think of for no queues to be reported would
> be that the device is down, which is very weird and we could as well
> crash. So maybe the check for queues is not necessary ?
I kind of feel like raising is slightly more verbose, which I tend
to slightly prefer over just a crash that might leave a future
person confused.
I'll go with the raise as you suggested instead.
