Re: [PATCH bpf-next 2/2] selftests/bpf: Expand skb dynptr selftests for tp_btf

[Philo Lu <lulie@linux.alibaba.com>]

On 2024/5/7 05:43, Martin KaFai Lau wrote:
> On 4/30/24 5:18 AM, Philo Lu wrote:
>> Add 3 test cases for skb dynptr used in tp_btf:
[...]
>> +
>> +SEC("tp_btf/kfree_skb")
>> +int BPF_PROG(test_dynptr_skb_tp_btf, struct __sk_buff *skb, void 
>> *location)
> 
> struct __sk_buff is the incorrect type. This happens to work but will be 
> a surprise for people trying to read something (e.g. skb->len). The same 
> goes for the ones in dynptr_fail.c.
> 

What do you think if I replace "struct __sk_buff" with "void"? The diffs 
are appended below.

Because we are not to read anything in these cases, I think using void* 
is enough to avoid confusion. On the other hand, to use "struct sk_buff" 
here, we have to introduce the definition, and tune codes as the input 
type of bpf_dynptr_from_skb() is defined as struct __sk_buff in 
"bpf_kfuncs.h".

Thanks.

-----------------
diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c 
b/tools/testing/selftests/bpf/progs/dynptr_fail.c
index c438d1c3cac56..42dbf8715c6a8 100644
--- a/tools/testing/selftests/bpf/progs/dynptr_fail.c
+++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c
@@ -1257,7 +1257,7 @@ int skb_invalid_ctx(void *ctx)

  SEC("fentry/skb_tx_error")
  __failure __msg("must be referenced or trusted")
-int BPF_PROG(skb_invalid_ctx_fentry, struct __sk_buff *skb)
+int BPF_PROG(skb_invalid_ctx_fentry, void *skb)
  {
         struct bpf_dynptr ptr;

@@ -1269,7 +1269,7 @@ int BPF_PROG(skb_invalid_ctx_fentry, struct 
__sk_buff *skb)

  SEC("fexit/skb_tx_error")
  __failure __msg("must be referenced or trusted")
-int BPF_PROG(skb_invalid_ctx_fexit, struct __sk_buff *skb)
+int BPF_PROG(skb_invalid_ctx_fexit, void *skb)
  {
         struct bpf_dynptr ptr;

diff --git a/tools/testing/selftests/bpf/progs/dynptr_success.c 
b/tools/testing/selftests/bpf/progs/dynptr_success.c
index 8faafab97c0ec..bfcc85686cf04 100644
--- a/tools/testing/selftests/bpf/progs/dynptr_success.c
+++ b/tools/testing/selftests/bpf/progs/dynptr_success.c
@@ -547,7 +547,7 @@ int test_dynptr_skb_strcmp(struct __sk_buff *skb)
  }

  SEC("tp_btf/kfree_skb")
-int BPF_PROG(test_dynptr_skb_tp_btf, struct __sk_buff *skb, void *location)
+int BPF_PROG(test_dynptr_skb_tp_btf, void *skb, void *location)
  {
         __u8 write_data[2] = {1, 2};
         struct bpf_dynptr ptr;