Re: [PATCH bpf-next v2 8/9] selftests/bpf: Verify iter link_create logs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] selftests/bpf: swapped iter_info and iter_info_len subtest names
--

commit 05954811719a2e7df15c96088c216104e09ba282
Author: Leon Hwang <leon.hwang@linux.dev>

selftests/bpf: Verify iter link_create logs

This commit adds selftests verifying the error codes, log messages, and
reported log sizes for invalid bpf_link_create() iterator arguments and
programs.

> diff --git a/tools/testing/selftests/bpf/prog_tests/iter_link.c b/tools/testing/selftests/bpf/prog_tests/iter_link.c
> new file mode 100644
> index 0000000000000..623a84ba612d5
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/iter_link.c

[ ... ]

> +static void test_log_invalid_iter_info(void)
> +{
> +	unsigned char info[sizeof(union bpf_iter_link_info) + 1] = {};
> +	const char *msg = "Invalid iter_info_len.\n";
> +	struct bpf_iter_bpf_map *skel;
> +	struct bpf_link *link;
> +	char log_buf[128] = {};
> +	LIBBPF_OPTS(bpf_log_opts, log_opts,
> +		    .buf = log_buf,
> +		    .size = sizeof(log_buf),
> +		    .level = 1,
> +	);
> +	LIBBPF_OPTS(bpf_iter_attach_opts, opts,
> +		    .link_info = (union bpf_iter_link_info *)info,
> +		    .link_info_len = sizeof(info),
> +		    .log_opts = &log_opts,
> +	);
> +
> +	skel = bpf_iter_bpf_map__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "bpf_iter_bpf_map__open_and_load"))
> +		return;
> +
> +	info[sizeof(info) - 1] = 1;
> +	link = bpf_program__attach_iter(skel->progs.dump_bpf_map, &opts);
> +	if (!ASSERT_ERR_PTR(link, "bpf_program__attach_iter")) {
> +		bpf_link__destroy(link);
> +		goto out;
> +	}
> +
> +	ASSERT_EQ(libbpf_get_error(link), -E2BIG, "err");
> +	ASSERT_STREQ(log_buf, msg, "log_buf");
> +	ASSERT_EQ(log_opts.true_size, strlen(msg) + 1, "log_size");
> +
> +out:
> +	bpf_iter_bpf_map__destroy(skel);
> +}
> +
> +static void test_log_invalid_iter_info_len(void)
> +{
> +	const char *msg = "Invalid iter_info.\n";
> +	LIBBPF_OPTS(bpf_link_create_opts, opts,
> +		    .iter.info = (union bpf_iter_link_info *)1,
> +		    .iter.len = sizeof(union bpf_iter_link_info),
> +	);
> +
> +	test_iter_link_create_log(0, &opts, 0, -EFAULT, msg);
> +}
> +
> +static void test_log_invalid_prog(void)
> +{
> +	const char *msg = "The iter target does not support the bpf prog.\n";
> +	LIBBPF_OPTS(bpf_link_create_opts, opts);
> +
> +	test_iter_link_create_log(0, &opts, BPF_F_SLEEPABLE, -EINVAL, msg);
> +}
> +
> +void test_iter_link_log(void)
> +{
> +	if (test__start_subtest("invalid_target_fd"))
> +		test_log_invalid_target_fd();
> +	if (test__start_subtest("invalid_flags"))
> +		test_log_invalid_flags();
> +	if (test__start_subtest("iter_info_only"))
> +		test_log_iter_info_only();
> +	if (test__start_subtest("iter_info_len_only"))
> +		test_log_iter_info_len_only();
> +	if (test__start_subtest("invalid_iter_info"))
> +		test_log_invalid_iter_info();
> +	if (test__start_subtest("invalid_iter_info_len"))
> +		test_log_invalid_iter_info_len();

[Severity: Low]
Are the function and subtest names for test_log_invalid_iter_info() and
test_log_invalid_iter_info_len() swapped?

test_log_invalid_iter_info() passes an oversized link_info_len with a
non-zero trailing byte and checks for -E2BIG ("Invalid iter_info_len."),
whereas test_log_invalid_iter_info_len() passes a bad iter.info pointer
and checks for -EFAULT ("Invalid iter_info.").  If either subtest fails,
the reported subtest name will point to the opposite check.

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