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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Unconditional use of `__int128` in `bpf_testmod` breaks compilation on 32-bit host architectures.
--

--- Patch [9]: [PATCH bpf-next 09/12] selftests/bpf: Add C tests for 16-byte returns in R0:R2 ---
commit 5ed3844739f04c920bb1c37cdf474501b3ca9fef
Author: Yonghong Song <yonghong.song@linux.dev>

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

This commit adds selftests to exercise 16-byte return values passed in the
R0:R2 register pair. The tests include an __int128 return, a 16-byte struct
return, and a 16-byte union return.

[ ... ]

> diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c

[ ... ]

> @@ -825,6 +825,18 @@ __bpf_kfunc int bpf_kfunc_call_test5(u8 a, u16 b, u32 c)
>  	return 0;
>  }
>  
> +__bpf_kfunc __int128 bpf_kfunc_call_test_i128(u64 a, u64 b)
> +{
> +	return ((__int128)a << 64) | b;
> +}

[Severity: Low]
Will this unconditional use of __int128 break the build on 32-bit host
architectures? 

The __int128 type is a compiler extension that is typically only available
on 64-bit systems.

[ ... ]

> @@ -1358,6 +1370,8 @@ BTF_ID_FLAGS(func, bpf_kfunc_call_test2)
>  BTF_ID_FLAGS(func, bpf_kfunc_call_test3)
>  BTF_ID_FLAGS(func, bpf_kfunc_call_test4)
>  BTF_ID_FLAGS(func, bpf_kfunc_call_test5)
> +BTF_ID_FLAGS(func, bpf_kfunc_call_test_i128)

[Severity: Low]
If bpf_kfunc_call_test_i128 needs to be conditionally excluded for 32-bit
builds to fix the issue above, should this BTF ID registration also be
conditionally excluded to prevent a missing symbol error during the build?

[ ... ]

> diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
> --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
> +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h

[ ... ]

> @@ -118,6 +127,8 @@ int bpf_kfunc_call_test2(struct sock *sk, __u32 a, __u32 b) __ksym;
>  struct sock *bpf_kfunc_call_test3(struct sock *sk) __ksym;
>  long bpf_kfunc_call_test4(signed char a, short b, int c, long d) __ksym;
>  int bpf_kfunc_call_test5(__u8 a, __u16 b, __u32 c) __ksym;
> +__int128 bpf_kfunc_call_test_i128(__u64 a, __u64 b) __ksym;

[Severity: Low]
Does this declaration also need to be guarded for 64-bit architectures,
so that including this header doesn't fail on 32-bit host compilers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708200939.2153664-1-yonghong.song@linux.dev?part=9