On Tue, Mar 3, 2026 at 5:15 AM Hari Bathini <[email protected]> wrote:
>
>
> +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;
> +
> + ret = bpf_kfunc_call_test5(0xFF, 0xFFFF, 0xFFFFFFFF);
maybe add a comment with bpf asm to highlight what this is ?
Also 0xFFFFffffULL ?
8 "F"s in a row is harder on the eyes.
and ULL to make it explicit ?
> + 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;
> +
> + ret = bpf_kfunc_call_test5(val8 * 0xFF, val16 * 0xFFFF, val32 *
> 0xFFFFFFFF);
I'm struggling to decipher it. Pls add a comment with asm to explain.
I think the last multiplication is still done in 32-bit domain ?
or not? 0xFFFFFFFF is a 64-bit constant in C. I think...
Also we have 4 ISA versions. test_progs-no_alu32 and test_progs
compile it differently.
Maybe let's add another version of this test but fully in asm ?
Keep the C version too.
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> SEC("tc")
> int kfunc_call_test4(struct __sk_buff *skb)
> {
> diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> index e62c6b78657f..de4897ddcff1 100644
> --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> @@ -766,6 +766,33 @@ __bpf_kfunc long noinline bpf_kfunc_call_test4(signed
> char a, short b, int c, lo
> return (long)a + (long)b + (long)c + d;
> }
>
> +__bpf_kfunc int bpf_kfunc_call_test5(u8 a, u16 b, u32 c)
> +{
> + /* Make val as volatile to avoid compiler optimizations on the below
> checks */
> + volatile long val = a;
Pls add a comment that this is zero extended in C.
> +
> + /* Check zero-extension */
> + if (val != (unsigned long)a)
> + return 1;
> + /* Check no sign-extension */
> + if (val < 0)
> + return 2;
> +
> + val = b;
> + if (val != (unsigned long)b)
> + return 3;
> + if (val < 0)
> + return 4;
> +
> + val = c;
> + if (val != (unsigned long)c)
> + return 5;
> + if (val < 0)
> + return 6;
> +
> + return 0;
> +}
Overall this looks very useful.
I would expand with another test where a,b,c are s8,s16,s32.
Please resend with [PATCH bpf-next] in the subject, so that CIs
can pick it up correctly.
pw-bot: cr