Re: [PATCH bpf-next v3 10/13] selftests/bpf: Add C tests for 16-byte returns in R0:R2

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `aggregate_ret_kfunc_c` test will fail to load instead of gracefully skipping on 32-bit kernels when compiled with LLVM >= 23, due to a mismatch in `__SIZEOF_INT128__` definitions causing a ksym resolution failure.
--

--- Patch [10]: [PATCH bpf-next v3 10/13] selftests/bpf: Add C tests for 16-byte returns in R0:R2 ---
commit 34be80f3aa8159c8e9fb57e485947ec4dc335cca
Author: Yonghong Song <[email protected]>

selftests/bpf: Add C tests for 16-byte returns in R0:R2

This commit adds selftests that exercise a 16-byte return value passed in the
R0:R2 register pair. It covers __int128, 16-byte struct, and 16-byte union
returns. It also adds tests for __int128 and 16-byte struct returns from a
kfunc, with bpf_kfunc_call_test_i128() guarded by __SIZEOF_INT128__.

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

[ ... ]

> +static void test_kfunc_c(void)
> +{
> +	struct aggregate_ret_kfunc_c *skel;
> +	bool supported;
> +	int err;
> +
> +	skel = aggregate_ret_kfunc_c__open();
> +	if (!ASSERT_OK_PTR(skel, "skel_kfunc_c_open"))
> +		return;
> +
> +	supported = skel->rodata->has_reg_pair_ret;
> +
> +	/*
> +	 * Where the JIT cannot hand the second half of a >8-byte kfunc return
> +	 * back in R0:R2, bpf_add_kfunc_call() rejects the call with
> +	 * -EOPNOTSUPP. Asking the kernel keeps this test free of a list of the
> +	 * JITs that can, which would have to be updated as the rest of them
> +	 * learn.
> +	 */
> +	err = aggregate_ret_kfunc_c__load(skel);
> +	if (err == -EOPNOTSUPP)
> +		supported = false;
> +	else if (!ASSERT_OK(err, "skel_kfunc_c_load"))
> +		goto out;

[Severity: Medium]
Will this load fail with an assertion error instead of gracefully skipping on
32-bit kernels when compiled with LLVM >= 23?

If the host kernel is 32-bit, bpf_testmod is built without __SIZEOF_INT128__,
omitting bpf_kfunc_call_test_i128 from the module BTF.

However, because the BPF program aggregate_ret_kfunc_c.c is compiled for a
64-bit BPF target, __SIZEOF_INT128__ will be defined, and it will
unconditionally reference the bpf_kfunc_call_test_i128 kfunc via the
bpf_testmod_kfunc.h header.

During aggregate_ret_kfunc_c__load(), libbpf will attempt to resolve the
kfunc, but since it is missing from the 32-bit kernel's BTF, it will fail and
return a relocation error (such as -ENOENT) rather than -EOPNOTSUPP. This
would cause the ASSERT_OK(err) check to fail.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=10