Re: [PATCH bpf-next v13 8/8] selftests/bpf: Add tests to verify map create failure log

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The tests `test_invalid_numa_node` and `test_invalid_token_fd` use the hardcoded value `0xFF` (255) to trigger validation failures, which can cause spurious test failures on certain systems.
--

commit 31093136dd05cf604e6a2e0d0a470807567ed4a7
Author: Leon Hwang <leon.hwang@linux.dev>

selftests/bpf: Add tests to verify map create failure log

This commit adds tests to verify that the kernel reports the expected error
messages and correct log_true_size when map creation fails.

> diff --git a/tools/testing/selftests/bpf/prog_tests/map_init.c b/tools/testing/selftests/bpf/prog_tests/map_init.c
> index 14a31109dd0e0..5c61c8e373065 100644
> --- a/tools/testing/selftests/bpf/prog_tests/map_init.c
> +++ b/tools/testing/selftests/bpf/prog_tests/map_init.c

[ ... ]

> +static void test_invalid_numa_node(void)
> +{
> +	const char *msg = "Invalid numa_node.\n";
> +	LIBBPF_OPTS(bpf_map_create_opts, opts,
> +		    .map_flags = BPF_F_NUMA_NODE,
> +		    .numa_node = 0xFF,

Is it possible for this test to fail spuriously on large systems? If the
test runs on a machine with 256 or more NUMA nodes where node 255 is
online, the kernel validation will pass. 

The map creation might then fail for another reason, skipping the expected
log message and causing the test to fail.

Would a value like 0xFFFFFFFE (avoiding NUMA_NO_NODE) be more reliable to
ensure it exceeds nr_node_ids?

> +	);
> +
> +	test_map_create_array(&opts, msg);
> +}

[ ... ]

> +static void test_invalid_token_fd(void)
> +{
> +	const char *msg = "Invalid map_token_fd.\n";
> +	LIBBPF_OPTS(bpf_map_create_opts, opts,
> +		    .map_flags = BPF_F_TOKEN_FD,
> +		    .token_fd = 0xFF,

Could this also cause an unexpected test failure if file descriptor 255
happens to be an open BPF token in the test runner? 

It looks like test_invalid_btf_fd uses -1 to guarantee an invalid file
descriptor. Does it make sense to use -1 here as well?

> +	);
> +
> +	test_map_create_array(&opts, msg);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511152817.89191-1-leon.hwang@linux.dev?part=8