Re: [PATCH bpf-next v4 6/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:
- [Medium] bpf: ignored bpf_program__set_load_strategy() return value in selftest
--

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

selftests/bpf: Cover BPF program load strategy transitions

Adds a load_type test to cover load strategy transitions
(DISABLED/AUTO/MANUAL) and compatibility with boolean auto-attach states.
Also tests the manual load/attach cycle after loading an object.

> 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..ab42c3e887fce
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/load_type.c
> @@ -0,0 +1,186 @@
[ ... ]
> +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;
> +
> +	/* don't load prog1 */
> +	bpf_program__set_load_strategy(skel->progs.prog1, BPF_PROG_LOAD_STRATEGY_DISABLED);
> +
> +	/* load and attach prog2 */
> +	bpf_program__set_load_strategy(skel->progs.prog2, BPF_PROG_LOAD_STRATEGY_AUTO);

[Severity: Medium]
Is it intentional that test_load_type() ignores the return value of
bpf_program__set_load_strategy() for prog1 and prog2?

Since this selftest is designed to verify load strategy transitions, failing
to check the return value might silently mask regressions. If the API
introduces a bug where transitioning to AUTO fails (e.g., returning -EINVAL),
the API call would return an error but the program's strategy would remain
its default (which is already AUTO for prog2).

Because prog2 remains AUTO, the subsequent test assertions expecting AUTO
behavior would still pass, completely missing the API failure it was designed
to catch. Should these calls use ASSERT_OK() similar to how prog3 is checked
below?

> +	if (!ASSERT_TRUE(bpf_program__autoload(skel->progs.prog2), "prog2_autoload"))
> +		goto cleanup;
> +
> +	err = bpf_program__set_load_strategy(skel->progs.prog3, BPF_PROG_LOAD_STRATEGY_MANUAL);
> +	if (!ASSERT_OK(err, "set_load_strategy_manual"))
> +		goto cleanup;

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