Re: [PATCH bpf-next v8 5/7] selftests/bpf: Cover BPF program load strategy transitions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] selftests/bpf: redundant standalone load_type test duplicates autoload test
--

commit a706cefdc370f05d909294879af1c74efbca1c5c
Author: Slava Imameev <slava.imameev@crowdstrike.com>

selftests/bpf: Cover BPF program load strategy transitions

This commit adds a standalone load_type selftest covering BPF program load
strategy transitions between disabled, auto, and manual modes,
bpf_program__set_autoload() compatibility, and the manual load and attach
cycle after the object is loaded.

> diff --git a/tools/testing/selftests/bpf/prog_tests/load_type.c b/tools/testing/selftests/bpf/prog_tests/load_type.c
> new file mode 100644
> index 0000000000000..7e344c0b04450
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/load_type.c
> @@ -0,0 +1,154 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <test_progs.h>
> +#include <time.h>
> +#include "test_load_type.skel.h"
> +
> +void test_load_type(void)
> +{
> +	struct bpf_link *link = NULL;
> +	struct test_load_type *skel;
> +	int err;
> +
> +	skel = test_load_type__open();
> +	if (!ASSERT_OK_PTR(skel, "skel_open"))
> +		return;

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/test_load_type.c b/tools/testing/selftests/bpf/progs/test_load_type.c
> new file mode 100644
> index 0000000000000..3d9b81691d7ad
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_load_type.c
> @@ -0,0 +1,31 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +
> +bool prog1_called = false;
> +bool prog2_called = false;
> +bool prog3_called = false;
> +
> +SEC("raw_tp/sys_enter")
> +int prog1(const void *ctx)
> +{
> +	prog1_called = true;
> +	return 0;
> +}

[Severity: Low]
tools/testing/selftests/bpf/prog_tests/autoload.c and
tools/testing/selftests/bpf/progs/test_autoload.c already exist in the same
directories to test bpf_program__set_autoload() and selective program
loading using prog1, prog2, prog3 and prog1_called, prog2_called,
prog3_called:

tools/testing/selftests/bpf/prog_tests/autoload.c:test_autoload() {
	int duration = 0, err;
	struct test_autoload* skel;
...
	/* don't load prog3 */
	bpf_program__set_autoload(skel->progs.prog3, false);
}

Since bpf_program__set_autoload() is a thin wrapper around
bpf_program__set_load_strategy(), and commit ("selftests/bpf: Cover BPF
program manual loading") in the same series also adds
tools/testing/selftests/bpf/prog_tests/manual_load.c and
tools/testing/selftests/bpf/progs/test_manual_load.c with the same
prog1, prog2, prog3 raw_tp/sys_enter programs and post-load manual
load/attach sequence in manual_load_verify_main_cycle(), could these
bpf_program__set_load_strategy() transition checks extend test_autoload()
in prog_tests/autoload.c or be consolidated as a subtest in
prog_tests/manual_load.c instead of adding standalone prog_tests/load_type.c
and progs/test_load_type.c files?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261024014514.2959514-1-andrey.grodzovsky@crowdstrike.com?part=5