Re: [PATCH bpf v3 4/5] selftests/bpf: Add test for key-less BTF hash map

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 2419 bytes --]

> diff --git a/tools/testing/selftests/bpf/prog_tests/btf_map_keyless.c b/tools/testing/selftests/bpf/prog_tests/btf_map_keyless.c
> new file mode 100644
> index 0000000000000..a7d037f57a7ef
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/btf_map_keyless.c
> @@ -0,0 +1,82 @@

[ ... ]

> +	/*
> +	 * Positive control: the same map with a real key type must be accepted,
> +	 * so the -EINVAL below is about the key-less BTF and not some unrelated
> +	 * rejection (e.g. an unknown map type).
> +	 */
> +	opts.btf_key_type_id = val_id;
> +	map_fd = bpf_map_create(map_type, "keyed_map", 4, 4, 8, &opts);
> +	if (!ASSERT_GE(map_fd, 0, "keyed create is accepted"))
> +		return;
> +	close(map_fd);

A subsystem pattern flags this as potentially concerning: the positive
control turns "this kernel does not implement the map type" into a hard test
failure instead of a skip.

BPF_MAP_TYPE_RHASH (used by the rhash subtest) is new; on any kernel that
lacks it, map_create() rejects the unknown map type and this ASSERT_GE fails,
reporting a red btf_map_keyless/rhash that has nothing to do with the
behaviour under test.

libbpf already provides the probe for this: libbpf_probe_bpf_map_type()
handles BPF_MAP_TYPE_RHASH explicitly (tools/lib/bpf/libbpf_probes.c:312,
including the BPF_F_NO_PREALLOC requirement), so the subtest could probe
and test__skip() with a reason instead.

Two arguments in the patch's favour: (a) an unknown map type is rejected with
-EINVAL, not EOPNOTSUPP/ENOSYS, so the errno match isn't literal; (b) the
comment on lines 26-30 shows the failure is deliberate - the author wants an
unrelated rejection to surface as a failed positive control rather than be
silently swallowed by the -EINVAL assertion below.

Both arguments are about the choice of signal, not about whether an absent map
type is a capability-absent condition, which it is. Does the current approach
fit the test's intent better than probing and skipping?

[ ... ]

> +	if (test__start_subtest("hash"))
> +		check_keyless(BPF_MAP_TYPE_HASH, 0, btf_fd, val_id);
> +	if (test__start_subtest("rhash"))
> +		check_keyless(BPF_MAP_TYPE_RHASH, BPF_F_NO_PREALLOC, btf_fd, val_id);


---
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/33385673160