On Mon, Feb 20, 2023 at 5:30 PM Ilya Leoshkevich <iii@xxxxxxxxxxxxx> wrote: > > On Sat, Feb 18, 2023 at 02:23:56PM +0100, Puranjay Mohan wrote: > > On Fri, Feb 17, 2023 at 10:46 PM Andrii Nakryiko > > <andrii.nakryiko@xxxxxxxxx> wrote: > > > > > > 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 > > > > I can try to compile the selftests on arm32 and count the number of failures. > > It is important for correctness but also for testing the support of > > BPF on non-64 bit > > architectures. If the selftests don't even compile we can't test BPF properly. > > Hi, > > Does anyone plan looking into fixing selftests on 32-bit arches in the > near future (i.e. getting rid of longs and pointers)? I have an x86 JIT > change that I would like to test, and I'm also running into this issue. > I can try doing this, but I'd like to avoid doing duplicate work. > Hi, I am interested in doing this. I want to make sure that BPF is fully supported on 32-bit architectures. This is just a hobby for me but I want to work on this. I also wish to implement the trampoline for ARM some day. I will be sending patches soon for this. Currently I am working on a patch to support usdt for arm in libbpf. > Best regards, > Ilya > > [...] -- Thanks and Regards Yours Truly, Puranjay Mohan
