Re: [PATCH bpf-next 3/3] selftests/bpf: Add tests with u8/s16 kfunc return types

[Yonghong Song <yhs@fb.com>]

On 8/8/22 4:25 PM, Andrii Nakryiko wrote:
> On Sun, Aug 7, 2022 at 10:51 AM Yonghong Song <yhs@fb.com> wrote:
>>
>> Add two program tests with kfunc return types u8/s16.
>> With previous patch, xlated codes looks like below:
>>    ...
>>    ; return bpf_kfunc_call_test4((struct sock *)sk, (1 << 16) + 0xff00, (1 << 16) + 0xff);
>>       5: (bf) r1 = r0
>>       6: (b4) w2 = 130816
>>       7: (b4) w3 = 65791
>>       8: (85) call bpf_kfunc_call_test4#8931696
>>       9: (67) r0 <<= 48
>>      10: (c7) r0 s>>= 48
>>      11: (bc) w6 = w0
>>    ; }
>>      12: (bc) w0 = w6
>>      13: (95) exit
>>    ...
>>    ; return bpf_kfunc_call_test5((struct sock *)sk, (1 << 8) + 1, (1 << 8) + 2);
>>       5: (bf) r1 = r0
>>       6: (b4) w2 = 257
>>       7: (b4) w3 = 258
>>       8: (85) call bpf_kfunc_call_test5#8931712
>>       9: (67) r0 <<= 56
>>      10: (77) r0 >>= 56
>>      11: (bc) w6 = w0
>>    ; }
>>      12: (bc) w0 = w6
>>      13: (95) exit
>>
>> For return type s16, proper sign extension for the return value is done
>> for kfunc bpf_kfunc_call_test4(). For return type s8, proper zero
>> extension for the return value is done for bpf_kfunc_call_test5().
>>
>> Without the previous patch, the test kfunc_call will fail with
>>    ...
>>    test_main:FAIL:test4-retval unexpected test4-retval: actual 196607 != expected 4294967295
>>    ...
>>    test_main:FAIL:test5-retval unexpected test5-retval: actual 515 != expected 3
>>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>>   net/bpf/test_run.c                            | 12 +++++++
>>   .../selftests/bpf/prog_tests/kfunc_call.c     | 10 ++++++
>>   .../selftests/bpf/progs/kfunc_call_test.c     | 32 +++++++++++++++++++
>>   3 files changed, 54 insertions(+)
>>
>> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
>> index cbc9cd5058cb..3a17ab4107f5 100644
>> --- a/net/bpf/test_run.c
>> +++ b/net/bpf/test_run.c
>> @@ -551,6 +551,16 @@ struct sock * noinline bpf_kfunc_call_test3(struct sock *sk)
>>          return sk;
>>   }
>>
>> +s16 noinline bpf_kfunc_call_test4(struct sock *sk, u32 a, u32 b)
>> +{
>> +       return a + b;
>> +}
>> +
>> +u8 noinline bpf_kfunc_call_test5(struct sock *sk, u32 a, u32 b)
>> +{
>> +       return a + b;
>> +}
> 
> Is there any upside of adding this to net/bpf/test_run.c instead of
> defining it in bpf_testmod?

I put these two functions in test_run.c since bpf_kfunc_call_test{1,2,3}
are defined here and they easily fit the existing kfunc_call testing code.

But yes, I just checked the bpf_testmod.c. Looks like I am able
to define kfunc's in bpf_testmod. Will respin in v2 with this change.

> 
>> +
>>   struct prog_test_member1 {
>>          int a;
>>   };
> 
> [...]