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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240911141824.GZ572255@kernel.org>
Date: Wed, 11 Sep 2024 15:18:24 +0100
From: Simon Horman <horms@...nel.org>
To: Alexis Lothoré (eBPF Foundation) <alexis.lothore@...tlin.com>
Cc: Alexei Starovoitov <ast@...nel.org>,
	Daniel Borkmann <daniel@...earbox.net>,
	Andrii Nakryiko <andrii@...nel.org>,
	Martin KaFai Lau <martin.lau@...ux.dev>,
	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>,
	Jakub Kicinski <kuba@...nel.org>,
	Jesper Dangaard Brouer <hawk@...nel.org>,
	Lorenzo Bianconi <lorenzo@...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

On Tue, Sep 10, 2024 at 08:10:35PM +0200, 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) <alexis.lothore@...tlin.com>

...

> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_features.c b/tools/testing/selftests/bpf/prog_tests/xdp_features.c
> new file mode 100644
> index 000000000000..bcb36a2d2767
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_features.c
> @@ -0,0 +1,446 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/**
> + * Test XDP features
> + *
> + * Sets up a veth pair, and for each xdp feature under test:
> + * - asks the tested interface its xdp capabilities through bpf_xdp_query
> + * - attach and run some specific programs on both interfaces to check if
> + *   announced capability is respected
> + */

Hi Alexis,

This is neither a full review nor an issue that needs to block progress.
But, FWIIW, the comment above is not a Kernel doc, yet starts with '/**'.
I suggest that it should start with '/*' instead.

...