> On Mar 1, 2023, at 11:06 PM, Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote: > > On 2/28/23 6:17 PM, Aditi Ghag wrote: >>> On Feb 28, 2023, at 3:08 PM, Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote: >>> >>> On 2/23/23 1:53 PM, Aditi Ghag wrote: >>>> The test cases for TCP and UDP iterators mirror the intended usages of the >>>> helper. >>>> The destroy helpers set `ECONNABORTED` error code that we can validate in the >>>> test code with client sockets. But UDP sockets have an overriding error code >>>> from the disconnect called during abort, so the error code the validation is >>>> only done for TCP sockets. >>>> The `struct sock` is redefined as vmlinux.h forward declares the struct, and the >>>> loader fails to load the program as it finds the BTF FWD type for the struct >>>> incompatible with the BTF STRUCT type. >>>> Here are the snippets of the verifier error, and corresponding BTF output: >>>> ``` >>>> verifier error: extern (func ksym) ...: func_proto ... incompatible with kernel >>>> BTF for selftest prog binary: >>>> [104] FWD 'sock' fwd_kind=struct >>>> [70] PTR '(anon)' type_id=104 >>>> [84] FUNC_PROTO '(anon)' ret_type_id=2 vlen=1 >>>> '(anon)' type_id=70 >>>> [85] FUNC 'bpf_sock_destroy' type_id=84 linkage=extern >>>> -- >>>> [96] DATASEC '.ksyms' size=0 vlen=1 >>>> type_id=85 offset=0 size=0 (FUNC 'bpf_sock_destroy') >>>> BTF for selftest vmlinux: >>>> [74923] FUNC 'bpf_sock_destroy' type_id=48965 linkage=static >>>> [48965] FUNC_PROTO '(anon)' ret_type_id=9 vlen=1 >>>> 'sk' type_id=1340 >>>> [1340] PTR '(anon)' type_id=2363 >>>> [2363] STRUCT 'sock' size=1280 vlen=93 >>>> ``` >>> >>>> +int bpf_sock_destroy(struct sock_common *sk) __ksym; >>> >>> This does not match the bpf prog's BTF dump above which has pointer [70] pointing to FWD 'sock' [104] as the argument. It should be at least FWD 'sock_common' if not STRUCT 'sock_common'. I tried to change the func signature to 'struct sock *sk' but cannot reproduce the issue in my environment also. >>> >>> Could you confirm the BTF paste and 'incompatible with kernel" error in the commit message do match the bpf_sock_destroy declaration? If not, can you re-paste the BTF output and libbpf error message that matches the bpf_sock_destroy signature. >> I don't think you'll be able to reproduce the issue with `sock_common`, as `struct sock_common` isn't forward declared in vmlinux.h. But I find it odd that you weren't able to reproduce it with `struct sock`. Just to confirm, did you remove the minimal `struct sock` definition from the program? Per the commit description, I added that because libbpf was throwing this error - >> `libbpf: extern (func ksym) 'bpf_sock_destroy': func_proto [83] incompatible with kernel [75285]` > > Yep, I changed the kfunc to 'struct sock *' and removed the define/undef dance. > >> Sending the BTF snippet again (full BTF - https://pastebin.com/etkFyuJk) >> ``` >> 85] FUNC 'bpf_sock_destroy' type_id=84 linkage=extern >> type_id=85 offset=0 size=0 (FUNC 'bpf_sock_destroy') >> [84] FUNC_PROTO '(anon)' ret_type_id=2 vlen=1 >> '(anon)' type_id=70 >> [70] PTR '(anon)' type_id=104 >> [104] FWD 'sock' fwd_kind=struct >> ``` >> Compare this to the BTF snippet once I undef and define the struct in the test prog: >> ``` >> [87] FUNC 'bpf_sock_destroy' type_id=84 linkage=extern >> type_id=87 offset=0 size=0 (FUNC 'bpf_sock_destroy') >> [84] FUNC_PROTO '(anon)' ret_type_id=2 vlen=1 >> '(anon)' type_id=85 >> [85] PTR '(anon)' type_id=86 >> [86] STRUCT 'sock' size=136 vlen=1 >> '__sk_common' type_id=34 bits_offset=0 >> ``` >> (Anyway looks like I needed to define the struct in the test prog only when bpf_sock_destory had `struct sock` as the argument.) > > Right, I also think it is orthogonal to your set if the kfunc is taking 'struct sock_common *' anyway. [although I do feel a kernel function taking a 'struct sock_common *' is rather odd] Yes, this wasn't a problem with the helper taking `struct sock` as the argument in v1 patch. I'm all ears if we can have a similar signature for the kfunc. > > I was only asking and also trying myself because it looks pretty wrong if it can be reproduced and it is something that should be fixed regardless. It is pretty normal to have forward declaration within a bpf prog itself (not from vmlinux.h). From the paste, it feels like the kfunc bpf_sock_destroy btf is generated earlier than the 'struct sock'. Which llvm version are you using? $ llvm-config --version 14.0.0
