Re: [PATCH bpf-next v5 04/16] selftests/bpf: Clone a program that calls a module kfunc

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] selftests/bpf: standalone test file added for narrow kfunc clone case
--

commit 19b505b6bd62b4d72675abd0ca5df741fd9d15b1
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

selftests/bpf: Clone a program that calls a module kfunc

This commit adds a selftest that opens kfunc_call_test, prepares the BPF
object, and clones kfunc_call_test1 via bpf_program__clone() to verify module
BTF fd handling in fd_array.

> diff --git a/tools/testing/selftests/bpf/prog_tests/clone_module_kfunc.c b/tools/testing/selftests/bpf/prog_tests/clone_module_kfunc.c
> new file mode 100644
> index 0000000000000..fa6ccbaa7fd98
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/clone_module_kfunc.c
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
> +#include <test_progs.h>
> +#include "kfunc_call_test.skel.h"
> +
> +/*
> + * bpf_program__clone() loads a program from a prepared object with the
> + * fd_array of the object, where module BTF fds start at slot 1 and slot 0 is
> + * empty. It used to pass the fill index of that array as fd_array_cnt, which
> + * made the kernel resolve slot 0 and fail the load.
> + */
> +void test_clone_module_kfunc(void)
> +{
> +	struct kfunc_call_test *skel;
> +	int err, fd;
> +
> +	skel = kfunc_call_test__open();
> +	if (!ASSERT_OK_PTR(skel, "skel_open"))
> +		return;
> +
> +	err = bpf_object__prepare(skel->obj);
> +	if (!ASSERT_OK(err, "prepare"))
> +		goto out;
> +
> +	fd = bpf_program__clone(skel->progs.kfunc_call_test1, NULL);
> +	if (ASSERT_GE(fd, 0, "clone"))
> +		close(fd);
> +out:
> +	kfunc_call_test__destroy(skel);
> +}

[Severity: Low]
This isn't a bug, but tools/testing/selftests/bpf/prog_tests/kfunc_call.c in
the same directory already includes kfunc_call_test.skel.h and exercises
loading kfunc_call_test1 across multiple loader paths in verify_success() and
subtests in test_kfunc_call():

tools/testing/selftests/bpf/prog_tests/kfunc_call.c:test_kfunc_call() {
	test_main();

	if (test__start_subtest("subprog"))
		test_subprog();

	if (test__start_subtest("subprog_lskel"))
		test_subprog_lskel();

	if (test__start_subtest("destructive"))
		test_destructive();
}

Rather than adding a new standalone file for this narrow bpf_program__clone()
variant on kfunc_call_test1, should this be added as a subtest in
test_kfunc_call() (or an additional check in verify_success()) in
tools/testing/selftests/bpf/prog_tests/kfunc_call.c?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019132203.3034698-1-memxor@gmail.com?part=4