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

Reply via email to