Subject: Re: [PATCH bpf-next v2] selftests/bpf: convert test_xdp_features.sh to test_progs

From: Lorenzo Bianconi <[email-masked]>
Date: Sat, 14 Sep 2024 11:25:47 +0200

On Sep 13, Martin KaFai Lau wrote:
> On 9/10/24 11:10 AM, Alexis Lothoré (eBPF Foundation) wrote:
> > test_xdp_features.sh is a shell script allowing to test that xdp features
> > advertised by an interface are indeed delivered. The test works by starting
> > two instance of the same program, both attaching specific xdp programs to
> > each side of a veth link, and then make those programs manage packets and
> > collect stats to check whether tested XDP feature is indeed delivered or
> > not. However this test is not integrated in test_progs framework and so can
> > not run automatically in CI.
> > 
> > Rewrite test_xdp_features to integrate it in test_progs so it can run
> > automatically in CI. The main changes brought by the rewrite are the
> > following:
> > - instead of running to separated processes (each one managing either the
> >    tester veth or the DUT vet), run a single process
> > - slightly change testing direction (v0 is the tester in local namespace,
> >    v1 is the Device Under Test in remote namespace)
> > - group all tests previously managed by test_xdp_features as subtests (one
> >    per tested XDP feature). As a consequence, run only once some steps
> >    instead of once per subtest (eg: starting/stopping the udp server). On
> >    the contrary, make sure that each subtest properly cleans up its state
> >    (ie detach xdp programs, reset test stats, etc)
> > - since there is now a single process, get rid of the "control" tcp channel
> >    used to configure DUT. Configuring the DUT now only consists in switching
> >    to DUT network namespace and run the relevant commands
> > - since there is no more control channel, get rid of TLVs, keep only the
> >    CMD_ECHO packet type, and set it as a magic
> > - simplify network setup: use only ipv6 instead of both ipv4 and ipv6,
> >    force static neighbours instead of waiting for autoconfiguration, do not
> >    force gro (fetch xdp features only once xdp programs are loaded instead)
> > 
> > The existing XDP programs are reused, with some minor changes:
> > - tester and dut stats maps are converted to global variables for easier
> >    usage
> > - programs do not use TLV struct anymore but the magic replacing the echo
> >    command
> > - avoid to accidentally make tests pass: drop packets instead of forwarding
> >    them to userspace when they do not match the expected payload
> > - make sure to perform host <-> network endianness conversion on constants
> >    rather than packet parts
> > 
> > Signed-off-by: Alexis Lothoré (eBPF Foundation) <[email-masked]>
> > ---
> > Changes in v2:
> > - fix endianness management in userspace packet parsing (call htonl on
> >    constant rather than packet part)
> > 
> > The xdp_features rewrite has been tested in a x86_64 qemu environment on my
> > machine and in CI. In my environment, the test takes a bit less than 2s to
> > execute.
> > 
> >    # ./test_progs -a xdp_features
> >    #561/1   xdp_features/XDP_PASS:OK
> >    #561/2   xdp_features/XDP_DROP:OK
> >    #561/3   xdp_features/XDP_ABORTED:OK
> >    #561/4   xdp_features/XDP_TX:OK
> >    #561/5   xdp_features/XDP_REDIRECT:OK
> >    #561/6   xdp_features/XDP_NDO_XMIT:OK
> >    #561     xdp_features:OK
> >    Summary: 1/6 PASSED, 0 SKIPPED, 0 FAILED
> > ---
> >   tools/testing/selftests/bpf/.gitignore             |   1 -
> >   tools/testing/selftests/bpf/Makefile               |  10 +-
> >   .../selftests/bpf/prog_tests/xdp_features.c        | 446 +++++++++++++
> >   tools/testing/selftests/bpf/progs/xdp_features.c   |  49 +-
> >   tools/testing/selftests/bpf/test_xdp_features.sh   | 107 ---
> >   tools/testing/selftests/bpf/xdp_features.c         | 718 ---------------------

Hi Alexis.

sorry for the late reply.

> 
> From the initial commit message of xdp_features.c, its primary usage is to
> 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?

Regards,
Lorenzo