Re: [PATCH bpf-next v2] selftests/bpf: improve test coverage for kfunc call

[Yonghong Song <yonghong.song@linux.dev>]

On 3/12/26 1:01 AM, Hari Bathini wrote:
> On powerpc, immediate load instructions are sign extended. In case
> of unsigned types, arguments should be explicitly zero-extended by
> the caller. For kfunc call, this needs to be handled in the JIT code.
> In bpf_kfunc_call_test4(), that tests for sign-extension of signed
> argument types in kfunc calls, add some additional failure checks.
> And add bpf_kfunc_call_test5() to test zero-extension of unsigned
> argument types in kfunc calls.
>
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>

LGTM with a nit below.

Acked-by: Yonghong Song <yonghong.song@linux.dev>

> ---
>
> Changes in v2:
> - Added asm version of the selftest for consistent testing across
>    different BPF ISA versions.
> - Added comments clearly stating the intent of the test cases.
> - Updated sign-extension selftest to have additional failure checks.
>
>
>   .../selftests/bpf/prog_tests/kfunc_call.c     |  2 +
>   .../selftests/bpf/progs/kfunc_call_test.c     | 98 +++++++++++++++++++
>   .../selftests/bpf/test_kmods/bpf_testmod.c    | 54 +++++++++-
>   .../bpf/test_kmods/bpf_testmod_kfunc.h        |  1 +
>   4 files changed, 154 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
> index f79c8e53cb3e..62f3fb79f5d1 100644
> --- a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
> +++ b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
> @@ -74,6 +74,8 @@ static struct kfunc_test_params kfunc_tests[] = {
>   	TC_TEST(kfunc_call_test1, 12),
>   	TC_TEST(kfunc_call_test2, 3),
>   	TC_TEST(kfunc_call_test4, -1234),
> +	TC_TEST(kfunc_call_test5, 0),
> +	TC_TEST(kfunc_call_test5_asm, 0),
>   	TC_TEST(kfunc_call_test_ref_btf_id, 0),
>   	TC_TEST(kfunc_call_test_get_mem, 42),
>   	SYSCALL_TEST(kfunc_syscall_test, 0),
> diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_test.c b/tools/testing/selftests/bpf/progs/kfunc_call_test.c
> index 8b86113a0126..5edc51564f71 100644
> --- a/tools/testing/selftests/bpf/progs/kfunc_call_test.c
> +++ b/tools/testing/selftests/bpf/progs/kfunc_call_test.c
> @@ -2,8 +2,106 @@
>   /* Copyright (c) 2021 Facebook */
>   #include <vmlinux.h>
>   #include <bpf/bpf_helpers.h>
> +#include "bpf_misc.h"
>   #include "../test_kmods/bpf_testmod_kfunc.h"
>   
> +SEC("tc")
> +int kfunc_call_test5(struct __sk_buff *skb)
> +{
> +	struct bpf_sock *sk = skb->sk;
> +	int ret;
> +	u32 val32;
> +	u16 val16;
> +	u8 val8;
> +
> +	if (!sk)
> +		return -1;
> +
> +	sk = bpf_sk_fullsock(sk);
> +	if (!sk)
> +		return -1;
> +
> +	/*
> +	 * Test with constant values to verify zero-extension.
> +	 * ISA-dependent BPF asm:
> +	 *   With ALU32:    w1 = 0xFF; w2 = 0xFFFF; w3 = 0xFFFFffff
> +	 *   Without ALU32: r1 = 0xFF; r2 = 0xFFFF; r3 = 0xFFFFffff
> +	 * Both zero-extend to 64-bit before the kfunc call.
> +	 */
> +	ret = bpf_kfunc_call_test5(0xFF, 0xFFFF, 0xFFFFffffULL);

Can we just use 0xFFFFffff instead of 0xFFFFffffULL?

> +	if (ret)
> +		return ret;
> +
> +	val32 = bpf_get_prandom_u32();
> +	val16 = val32 & 0xFFFF;
> +	val8 = val32 & 0xFF;
> +	ret = bpf_kfunc_call_test5(val8, val16, val32);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Test multiplication with different operand sizes:
> +	 *
> +	 * val8 * 0xFF:
> +	 *   - Both operands promote to int (32-bit signed)
> +	 *   - Result: 32-bit multiplication, truncated to u8, then zero-extended
> +	 *
> +	 * val16 * 0xFFFF:
> +	 *   - Both operands promote to int (32-bit signed)
> +	 *   - Result: 32-bit multiplication, truncated to u16, then zero-extended
> +	 *
> +	 * val32 * 0xFFFFffffULL:
> +	 *   - val32 (u32) promotes to unsigned long long (due to ULL suffix)
> +	 *   - Result: 64-bit unsigned multiplication, truncated to u32, then zero-extended
> +	 */
> +	ret = bpf_kfunc_call_test5(val8 * 0xFF, val16 * 0xFFFF, val32 * 0xFFFFffffULL);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}

[...]