Message-ID: <464e0ae0-d6e3-4da4-a157-f74260f96275@bootlin.com>
Date: Sun, 22 Sep 2024 12:04:59 +0200
From: Alexis Lothoré <alexis.lothore@...tlin.com>
To: Jakub Kicinski <kuba@...nel.org>, Lorenzo Bianconi <lorenzo@...nel.org>
Cc: Martin KaFai Lau <martin.lau@...ux.dev>,
 Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
 Andrii Nakryiko <andrii@...nel.org>, Eduard Zingerman <eddyz87@...il.com>,
 Song Liu <song@...nel.org>, Yonghong Song <yonghong.song@...ux.dev>,
 John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>,
 Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>,
 Jiri Olsa <jolsa@...nel.org>, Mykola Lysenko <mykolal@...com>,
 Shuah Khan <shuah@...nel.org>, "David S. Miller" <davem@...emloft.net>,
 Jesper Dangaard Brouer <hawk@...nel.org>,
 Maxime Chevallier <maxime.chevallier@...tlin.com>, ebpf@...uxfoundation.org,
 Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
 linux-kernel@...r.kernel.org, bpf@...r.kernel.org,
 linux-kselftest@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH bpf-next v2] selftests/bpf: convert test_xdp_features.sh
 to test_progs

Hello all, sorry for the slow feedback, I have been off last week.

On 9/14/24 15:38, Jakub Kicinski wrote:
> On Sat, 14 Sep 2024 11:25:47 +0200 Lorenzo Bianconi wrote:
>> On Sep 13, Martin KaFai Lau wrote:
>>> test a physical network device that supports a certain XDP features.
>>>
>>> iiuc, test_xdp_features.sh only uses the veth and veth will also be the only
>>> device tested after moving to prog_tests/xdp_features.c? It is a reasonable
>>> addition to test_progs for an end-to-end xdp test by using veth. However,
>>> test_progs will not be able to test the physical network device.
>>>
>>> Lorenzo, is the xdp_features.c still used for device testing?
>>
>> correct, xdp_features.c is intended to test the real xdp features supported by
>> the NIC under test (DUT), not just the advertised ones (iirc that was a
>> requisite to add xdp kernel feature support). For this reason we need two
>> separated processes running on the tester device and on the DUT (they are
>> usually two different devices). test_xdp_features.sh was just a simple test
>> script used to develop xdp_features.c.
>> What about extending xdp_features.c to integrate it in the CI?

So IIUC Lorenzo's answer, we _need_ to keep the possibility to run this test on
real hardware, and so we _need_ to still be able to run two processes, possibly
on two different machines. If that's so, indeed my rework breaks this. I have
then multiple questions/doubts before being able to rework this:
- the main goal of this rework is to be able to automatically run this test in
CI, and the resulting constraint is that it must integrate in a standalone,
already-existing c program (test_progs). I guess I can preserve the standalone
xdp_features program as it is, and make test_progs just start  it twice (on two
different veths). It would involve the following changes:
  - keep a dedicated build step for this small, standalone xdp_features.c, and
add a "controller" part in test_progs (instead of fully migrating xdp_features
program into test_progs, which  is what the current series revision does)
  - simply make the controller part create the testing network in CI, fork/start
the xdp_features program on both veths, and check return codes.
My main concern is about the possible flakiness of this whole setup (multiple
processes and tcp/udp channels involved), but if keeping the standalone version
is really needed, I can give a try. Does it sound reasonable ?
- one part of my overall goal is to clean up the tools/testing/selftests/bpf
directory from anything that is not tested automatically. What should we do with
the wrapping shell script (test_xdp_features.sh) ? Since test_progs will
automate the test with veths, I guess it is still ok to just remove it ?

> No preference but just to raise awareness - drivers/net's NetDrvEpEnv
> class provides the setup for running tests with an endpoint.
> XDP tests intended for HW would fit there pretty well.

Thanks for the hint. If we want to keep some tooling for real hw xdp features
testing, maybe we could add a small part in tools/testing/selftests/drivers/net
and make it use this NetDrvEpEnv ? Or it is a bigger hint that the whole test
about xdp features could be moved there (and then tested by net kselftests
rather than by ebpf ci specifically) ? @Lorenzo and eBPF tests maintainers, any
opinion ?

Thanks,

Alexis

-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Powered by blists - more mailing lists