On Fri, Feb 17, 2023 at 2:25 AM Puranjay Mohan <puranjay12@xxxxxxxxx> wrote: > > Hi, > Thanks for the response. > > On Thu, Feb 16, 2023 at 11:13 PM Andrii Nakryiko > <andrii.nakryiko@xxxxxxxxx> wrote: > > > > On Wed, Feb 15, 2023 at 5:48 PM Stanislav Fomichev <sdf@xxxxxxxxxx> wrote: > > > > > > On 02/15, Puranjay Mohan wrote: > > > > The BPF selftests fail to compile on 32-bit architectures as the skeleton > > > > generated by bpftool doesn’t take into consideration the size difference > > > > of > > > > variables on 32-bit/64-bit architectures. > > > > > > > As an example, > > > > If a bpf program has a global variable of type: long, its skeleton will > > > > include > > > > a bss map that will have a field for this variable. The long variable in > > > > BPF is > > > > 64-bit. if we are working on a 32-bit machine, the generated skeleton has > > > > to > > > > compile for that machine where long is 32-bit. > > > > > > > A reproducer for this issue: > > > > root@56ec59aa632f:~# cat test.bpf.c > > > > long var; > > > > > > > root@56ec59aa632f:~# clang -target bpf -g -c test.bpf.c > > > > > > > root@56ec59aa632f:~# bpftool btf dump file test.bpf.o format raw > > > > [1] INT 'long int' size=8 bits_offset=0 nr_bits=64 encoding=SIGNED > > > > [2] VAR 'var' type_id=1, linkage=global > > > > [3] DATASEC '.bss' size=0 vlen=1 > > > > type_id=2 offset=0 size=8 (VAR 'var') > > > > > > > root@56ec59aa632f:~# bpftool gen skeleton test.bpf.o > skeleton.h > > > > > > > root@56ec59aa632f:~# echo "#include \"skeleton.h\"" > test.c > > > > > > > root@56ec59aa632f:~# gcc test.c > > > > In file included from test.c:1: > > > > skeleton.h: In function 'test_bpf__assert': > > > > skeleton.h:231:2: error: static assertion failed: "unexpected > > > > size of \'var\'" > > > > 231 | _Static_assert(sizeof(s->bss->var) == 8, "unexpected > > > > size of 'var'"); > > > > | ^~~~~~~~~~~~~~ > > > > > > > One naive solution for this would be to map ‘long’ to ‘long long’ and > > > > ‘unsigned long’ to ‘unsigned long long’. But this doesn’t solve everything > > > > because this problem is also seen with pointers that are 64-bit in BPF and > > > > 32-bit in 32-bit machines. > > > > > > > I want to work on solving this and am looking for ideas to solve it > > > > efficiently. > > > > The main goal is to make libbbpf/bpftool host architecture agnostic. > > > > > > Looks like bpftool needs to be aware of the target architecture. The > > > same way gcc is doing with build-host-target triplet. I don't > > > think this can be solved with a bunch of typedefs? But I've long > > > forgotten how a pure 32-bit machine looks, so I can't give any > > > useful input :-( > > > > Yeah, I'd rather avoid making bpftool aware of target architecture. > > Three is 32 vs 64 bitness, there is also little/big endianness, etc. > > > > So I'd recommend never using "long" (and similar types that depend on > > bitness of the platform, like size_t, etc) for global variables. Also > > don't use pointer types as types of the variable. Stick to __u64, > > __u32, etc. > > I feel if we follow. this convention then it will work out but > currently a lot of selftests use these > architecture dependent variable types and therefore don't even compile > for 32-bit architectures > because of the _Static_asserts in the skeleton. > > Do you suggest replacing all these with __u64, __u32, etc. in the > selftests so that they compile on every architecture? how many changes are we talking about? my initial reaction is that yeah, if this matters for correctness, we should be strict with __u64 and __u32 use over long > > > > > Note that all this is irrelevant for static global variables, as they > > are not exposed in the BPF skeleton. > > > > In general, mixing 32-bit host architecture with (always) 64-bit BPF > > architecture always requires more care. And BPF skeleton is just one > > aspect of this. > > > > > > > > > > > > Thanks, > > > > Puranjay Mohan. > > > > -- > Thanks and Regards > > Yours Truly, > > Puranjay Mohan
