Re: [PATCH bpf-next v6 2/2] selftests/bpf: add tests for selective module BTF loading

[Fuyu Zhao <zhaofuyu@vivo.com>]

On 8/31/2026 10:46 PM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/tools/testing/selftests/bpf/prog_tests/btf_module_names.c b/tools/testing/selftests/bpf/prog_tests/btf_module_names.c
>> new file mode 100644
>> index 0000000000000..68f4a8d735608
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/prog_tests/btf_module_names.c
>> @@ -0,0 +1,162 @@
> 
> [ ... ]
> 
>> +/*
>> + * Verify that an unrequested module BTF is skipped. The BPF program
>> + * requires the BTF of bpf_testmod, but bpf_testmod is not specified in
>> + * btf_module_names, so its BTF is skipped and the BPF program fails to load.
>> + */
>> +static void btf_module_names_skip(void)
>> +{
>> +	struct btf_module_names *skel = NULL;
>> +	int ret;
>> +	const char *mod_names[] = { "module_nonexist" };
>> +
>> +	LIBBPF_OPTS(bpf_object_open_opts, opts,
>> +		.btf_module_names = mod_names,
>> +		.nr_btf_module_names = 1,
>> +	);
>> +
>> +	skel = btf_module_names__open_opts(&opts);
>> +	if (!ASSERT_OK_PTR(skel, "btf_module_names__open_opts"))
>> +		goto out;
>> +
>> +	ret = btf_module_names__load(skel);
>> +	ASSERT_EQ(ret, -ESRCH, "btf_module_names__load");
>> +
>> +out:
>> +	btf_module_names__destroy(skel);
>> +}
>> +
>> +/*
>> + * Verify that an empty filter skips loading all module BTFs. The BPF
>> + * program requires bpf_testmod BTF, so it fails to load.
>> + */
>> +static void btf_module_names_empty(void)
>> +{
>> +	struct btf_module_names *skel = NULL;
>> +	int ret;
>> +	const char *mod_names[] = { NULL };
>> +
>> +	LIBBPF_OPTS(bpf_object_open_opts, opts,
>> +		/* Non-NULL pointer with zero entries represents an empty list. */
>> +		.btf_module_names = mod_names,
>> +		.nr_btf_module_names = 0,
>> +	);
>> +
>> +	skel = btf_module_names__open_opts(&opts);
>> +	if (!ASSERT_OK_PTR(skel, "btf_module_names__open_opts empty"))
>> +		goto out;
>> +
>> +	ret = btf_module_names__load(skel);
>> +	ASSERT_EQ(ret, -ESRCH, "btf_module_names__load empty");
>> +
>> +out:
>> +	btf_module_names__destroy(skel);
>> +}
> 
> This isn't a bug, but btf_module_names_skip() and btf_module_names_empty()
> look like the same body with two initializer values changed. Would a small
> helper taking (names, nr, expected_ret) read better and keep the subtests
> to a line apiece?
> 

This makes sense. I'll add a small helper to share the common test
logic between these subtests.

> [ ... ]
> 
>> +void test_btf_module_names(void)
>> +{
>> +	struct btf *vmlinux_btf = NULL;
>> +	struct btf *module_btf = NULL;
>> +
>> +	if (!env.has_testmod) {
>> +		test__skip();
>> +		return;
>> +	}
>> +
>> +	vmlinux_btf = btf__load_vmlinux_btf();
>> +	if (libbpf_get_error(vmlinux_btf)) {
>> +		test__skip();
>> +		return;
>> +	}
>> +
>> +	/* Ensure bpf_testmod BTF is available. */
>> +	module_btf = btf__load_module_btf("bpf_testmod", vmlinux_btf);
>> +	if (libbpf_get_error(module_btf)) {
>> +		btf__free(vmlinux_btf);
>> +		test__skip();
>> +		return;
>> +	}
> 
> A subsystem pattern flags this as potentially concerning: test_btf_module_names()
> has three distinct prerequisite failures and all three collapse into a bare
> test__skip() with no reason emitted.
> 
> The resulting log line is identical in all three cases, so a reader of a CI
> result cannot tell which prerequisite was missing, and cannot distinguish
> "module BTF support is not built in" from "bpf_testmod failed to load".
> 
> test__skip() takes no arguments in the BPF test framework, so the idiom for
> conveying a skip reason is an adjacent printf("%s:SKIP:...", __func__) or a
> trailing comment. The `if (!env.has_testmod) { test__skip(); return; }` form
> is universally idiomatic in prog_tests/ and would not normally be raised.
> 
> The escalation is really about the two BTF-availability skips:
> 
> 	vmlinux_btf = btf__load_vmlinux_btf();
> 	if (libbpf_get_error(vmlinux_btf)) {
> 		test__skip();
> 		return;
> 	}
> 
> 	module_btf = btf__load_module_btf("bpf_testmod", vmlinux_btf);
> 	if (libbpf_get_error(module_btf)) {
> 		btf__free(vmlinux_btf);
> 		test__skip();
> 		return;
> 	}
> 
> Neighbouring tests like prog_tests/btf_tag.c annotate these conditions:
> 
> 	printf("%s:SKIP: btf_type_tag attribute not in vmlinux btf", __func__);
> 	printf("%s:SKIP: btf_type_tag attribute not in %s", __func__, module_name);
> 
> This matters more here than in a typical test because two of the four
> subtests assert a *failure* code (-ESRCH), so silently skipping on a machine
> without module BTF is the same observable outcome as the feature working.
> 
> Would adding skip reason strings for the BTF-availability checks help
> distinguish which prerequisite was missing?
> 

Good point. I'll add skip reasons for the vmlinux and bpf_testmod BTF
availability checks.

Thanks,
Fuyu

> [ ... ]
> 
> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33399468751