BPF List
 help / color / mirror / Atom feed
* [RFC] libbbpf/bpftool: Support 32-bit Architectures.
@ 2023-02-15 14:12 Puranjay Mohan
  2023-02-16  1:42 ` Stanislav Fomichev
  0 siblings, 1 reply; 12+ messages in thread
From: Puranjay Mohan @ 2023-02-15 14:12 UTC (permalink / raw)
  To: bpf, quentin, ast, daniel, memxor

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.

Thanks,
Puranjay Mohan.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] libbbpf/bpftool: Support 32-bit Architectures.
  2023-02-15 14:12 [RFC] libbbpf/bpftool: Support 32-bit Architectures Puranjay Mohan
@ 2023-02-16  1:42 ` Stanislav Fomichev
  2023-02-16 22:13   ` Andrii Nakryiko
  0 siblings, 1 reply; 12+ messages in thread
From: Stanislav Fomichev @ 2023-02-16  1:42 UTC (permalink / raw)
  To: Puranjay Mohan; +Cc: bpf, quentin, ast, daniel, memxor

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 :-(


> Thanks,
> Puranjay Mohan.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] libbbpf/bpftool: Support 32-bit Architectures.
  2023-02-16  1:42 ` Stanislav Fomichev
@ 2023-02-16 22:13   ` Andrii Nakryiko
  2023-02-17 10:25     ` Puranjay Mohan
  0 siblings, 1 reply; 12+ messages in thread
From: Andrii Nakryiko @ 2023-02-16 22:13 UTC (permalink / raw)
  To: Stanislav Fomichev; +Cc: Puranjay Mohan, bpf, quentin, ast, daniel, memxor

On Wed, Feb 15, 2023 at 5:48 PM Stanislav Fomichev <sdf@google.com> 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.

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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] libbbpf/bpftool: Support 32-bit Architectures.
  2023-02-16 22:13   ` Andrii Nakryiko
@ 2023-02-17 10:25     ` Puranjay Mohan
  2023-02-17 11:59       ` Quentin Monnet
  2023-02-17 21:45       ` Andrii Nakryiko
  0 siblings, 2 replies; 12+ messages in thread
From: Puranjay Mohan @ 2023-02-17 10:25 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: Stanislav Fomichev, bpf, quentin, ast, daniel, memxor

Hi,
Thanks for the response.

On Thu, Feb 16, 2023 at 11:13 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Feb 15, 2023 at 5:48 PM Stanislav Fomichev <sdf@google.com> 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?

>
> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] libbbpf/bpftool: Support 32-bit Architectures.
  2023-02-17 10:25     ` Puranjay Mohan
@ 2023-02-17 11:59       ` Quentin Monnet
  2023-02-17 21:56         ` Andrii Nakryiko
  2023-02-17 21:45       ` Andrii Nakryiko
  1 sibling, 1 reply; 12+ messages in thread
From: Quentin Monnet @ 2023-02-17 11:59 UTC (permalink / raw)
  To: Puranjay Mohan, Andrii Nakryiko
  Cc: Stanislav Fomichev, bpf, ast, daniel, memxor

2023-02-17 11:25 UTC+0100 ~ Puranjay Mohan <puranjay12@gmail.com>
> Hi,
> Thanks for the response.
> 
> On Thu, Feb 16, 2023 at 11:13 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
>>
>> On Wed, Feb 15, 2023 at 5:48 PM Stanislav Fomichev <sdf@google.com> 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.

I'd rather avoid that too, but for addressing the endianness issue with
cross-compiling, reported by Christophe and where the bytecode is not
stored with the right endianness in the skeleton file, do you see an
alternative?

>>
>> 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?
> 
>>
>> 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.
> 
> 
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] libbbpf/bpftool: Support 32-bit Architectures.
  2023-02-17 10:25     ` Puranjay Mohan
  2023-02-17 11:59       ` Quentin Monnet
@ 2023-02-17 21:45       ` Andrii Nakryiko
  2023-02-18 13:23         ` Puranjay Mohan
  1 sibling, 1 reply; 12+ messages in thread
From: Andrii Nakryiko @ 2023-02-17 21:45 UTC (permalink / raw)
  To: Puranjay Mohan; +Cc: Stanislav Fomichev, bpf, quentin, ast, daniel, memxor

On Fri, Feb 17, 2023 at 2:25 AM Puranjay Mohan <puranjay12@gmail.com> wrote:
>
> Hi,
> Thanks for the response.
>
> On Thu, Feb 16, 2023 at 11:13 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Wed, Feb 15, 2023 at 5:48 PM Stanislav Fomichev <sdf@google.com> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] libbbpf/bpftool: Support 32-bit Architectures.
  2023-02-17 11:59       ` Quentin Monnet
@ 2023-02-17 21:56         ` Andrii Nakryiko
  2023-02-20 11:28           ` Quentin Monnet
  0 siblings, 1 reply; 12+ messages in thread
From: Andrii Nakryiko @ 2023-02-17 21:56 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Puranjay Mohan, Stanislav Fomichev, bpf, ast, daniel, memxor

On Fri, Feb 17, 2023 at 3:59 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2023-02-17 11:25 UTC+0100 ~ Puranjay Mohan <puranjay12@gmail.com>
> > Hi,
> > Thanks for the response.
> >
> > On Thu, Feb 16, 2023 at 11:13 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> >>
> >> On Wed, Feb 15, 2023 at 5:48 PM Stanislav Fomichev <sdf@google.com> 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.
>
> I'd rather avoid that too, but for addressing the endianness issue with
> cross-compiling, reported by Christophe and where the bytecode is not
> stored with the right endianness in the skeleton file, do you see an
> alternative?

So bytecode is just a byte array, by itself endianness shouldn't
matter. The contents of it (ELF itself) is supposed to be of correct
target endianness, though, right? Or what problem we are talking about
here? Can you please summarize?


>
> >>
> >> 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?
> >
> >>
> >> 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.
> >
> >
> >
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] libbbpf/bpftool: Support 32-bit Architectures.
  2023-02-17 21:45       ` Andrii Nakryiko
@ 2023-02-18 13:23         ` Puranjay Mohan
  2023-02-20 16:30           ` Ilya Leoshkevich
  0 siblings, 1 reply; 12+ messages in thread
From: Puranjay Mohan @ 2023-02-18 13:23 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: Stanislav Fomichev, bpf, quentin, ast, daniel, memxor

On Fri, Feb 17, 2023 at 10:46 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Fri, Feb 17, 2023 at 2:25 AM Puranjay Mohan <puranjay12@gmail.com> wrote:
> >
> > Hi,
> > Thanks for the response.
> >
> > On Thu, Feb 16, 2023 at 11:13 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Wed, Feb 15, 2023 at 5:48 PM Stanislav Fomichev <sdf@google.com> 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.

>
> >
> > >
> > > 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



-- 
Thanks and Regards

Yours Truly,

Puranjay Mohan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] libbbpf/bpftool: Support 32-bit Architectures.
  2023-02-17 21:56         ` Andrii Nakryiko
@ 2023-02-20 11:28           ` Quentin Monnet
  2023-02-28  6:31             ` Andrii Nakryiko
  0 siblings, 1 reply; 12+ messages in thread
From: Quentin Monnet @ 2023-02-20 11:28 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Puranjay Mohan, Stanislav Fomichev, bpf, ast, daniel, memxor

2023-02-17 13:56 UTC-0800 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> On Fri, Feb 17, 2023 at 3:59 AM Quentin Monnet <quentin@isovalent.com> wrote:
>>
>> 2023-02-17 11:25 UTC+0100 ~ Puranjay Mohan <puranjay12@gmail.com>
>>> Hi,
>>> Thanks for the response.
>>>
>>> On Thu, Feb 16, 2023 at 11:13 PM Andrii Nakryiko
>>> <andrii.nakryiko@gmail.com> wrote:
>>>>
>>>> On Wed, Feb 15, 2023 at 5:48 PM Stanislav Fomichev <sdf@google.com> 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.
>>
>> I'd rather avoid that too, but for addressing the endianness issue with
>> cross-compiling, reported by Christophe and where the bytecode is not
>> stored with the right endianness in the skeleton file, do you see an
>> alternative?
> 
> So bytecode is just a byte array, by itself endianness shouldn't
> matter. The contents of it (ELF itself) is supposed to be of correct
> target endianness, though, right? Or what problem we are talking about
> here? Can you please summarize?

TL;DR: When cross-compiling, host little-endian bootstrap bpftool cannot
open a big-endian ELF to generate a skeleton from it and build target
big-endian bpftool.

Long version: Currently, bpftool's Makefile compiles the
skeleton-related programs (skeletons/*.bpf.c) without paying attention
to the target architecture. When cross-compiling, say on a host with LE
for a target with BE, this leads to runtime failure on "bpftool prog
show", because bpftool cannot load the LE bytecode on the BE target
machine. This is Christophe's output in [0].

So the first fix is to make the Makefile aware of the target endianness
somehow, and to build this ELF with target endianness. But this is not
enough, because when (host) boostrap bpftool opens the ELF to generate
the skeleton from it before building the final (target) bpftool binary,
then bpf_object__check_endianness() in libbpf refuses to open the ELF if
endianness is not the same as on the host [1].

The way I see it, we'd need to make sure libbpf can work with ELFs of a
different endianness -- assuming that's doable -- and to pass it an
option to tell whether LE or BE is expected for a given ELF. Which in
turn would require bootstrap bpftool to be aware of the target
endianness. Or do you see a simpler way?


[0]
https://lore.kernel.org/bpf/0792068b-9aff-d658-5c7d-086e6d394c6c@csgroup.eu/
[1]
https://lore.kernel.org/bpf/21b09e52-142d-92f5-4f8b-e4190f89383b@csgroup.eu/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] libbbpf/bpftool: Support 32-bit Architectures.
  2023-02-18 13:23         ` Puranjay Mohan
@ 2023-02-20 16:30           ` Ilya Leoshkevich
  2023-02-20 16:47             ` Puranjay Mohan
  0 siblings, 1 reply; 12+ messages in thread
From: Ilya Leoshkevich @ 2023-02-20 16:30 UTC (permalink / raw)
  To: Puranjay Mohan, Andrii Nakryiko
  Cc: Stanislav Fomichev, bpf, quentin, ast, daniel, memxor

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@gmail.com> wrote:
> >
> > On Fri, Feb 17, 2023 at 2:25 AM Puranjay Mohan <puranjay12@gmail.com> wrote:
> > >
> > > Hi,
> > > Thanks for the response.
> > >
> > > On Thu, Feb 16, 2023 at 11:13 PM Andrii Nakryiko
> > > <andrii.nakryiko@gmail.com> wrote:
> > > >
> > > > On Wed, Feb 15, 2023 at 5:48 PM Stanislav Fomichev <sdf@google.com> 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.

Best regards,
Ilya

[...]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] libbbpf/bpftool: Support 32-bit Architectures.
  2023-02-20 16:30           ` Ilya Leoshkevich
@ 2023-02-20 16:47             ` Puranjay Mohan
  0 siblings, 0 replies; 12+ messages in thread
From: Puranjay Mohan @ 2023-02-20 16:47 UTC (permalink / raw)
  To: Ilya Leoshkevich
  Cc: Andrii Nakryiko, Stanislav Fomichev, bpf, quentin, ast, daniel,
	memxor

On Mon, Feb 20, 2023 at 5:30 PM Ilya Leoshkevich <iii@linux.ibm.com> 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@gmail.com> wrote:
> > >
> > > On Fri, Feb 17, 2023 at 2:25 AM Puranjay Mohan <puranjay12@gmail.com> wrote:
> > > >
> > > > Hi,
> > > > Thanks for the response.
> > > >
> > > > On Thu, Feb 16, 2023 at 11:13 PM Andrii Nakryiko
> > > > <andrii.nakryiko@gmail.com> wrote:
> > > > >
> > > > > On Wed, Feb 15, 2023 at 5:48 PM Stanislav Fomichev <sdf@google.com> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] libbbpf/bpftool: Support 32-bit Architectures.
  2023-02-20 11:28           ` Quentin Monnet
@ 2023-02-28  6:31             ` Andrii Nakryiko
  0 siblings, 0 replies; 12+ messages in thread
From: Andrii Nakryiko @ 2023-02-28  6:31 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Puranjay Mohan, Stanislav Fomichev, bpf, ast, daniel, memxor

On Mon, Feb 20, 2023 at 3:28 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2023-02-17 13:56 UTC-0800 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> > On Fri, Feb 17, 2023 at 3:59 AM Quentin Monnet <quentin@isovalent.com> wrote:
> >>
> >> 2023-02-17 11:25 UTC+0100 ~ Puranjay Mohan <puranjay12@gmail.com>
> >>> Hi,
> >>> Thanks for the response.
> >>>
> >>> On Thu, Feb 16, 2023 at 11:13 PM Andrii Nakryiko
> >>> <andrii.nakryiko@gmail.com> wrote:
> >>>>
> >>>> On Wed, Feb 15, 2023 at 5:48 PM Stanislav Fomichev <sdf@google.com> 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.
> >>
> >> I'd rather avoid that too, but for addressing the endianness issue with
> >> cross-compiling, reported by Christophe and where the bytecode is not
> >> stored with the right endianness in the skeleton file, do you see an
> >> alternative?
> >
> > So bytecode is just a byte array, by itself endianness shouldn't
> > matter. The contents of it (ELF itself) is supposed to be of correct
> > target endianness, though, right? Or what problem we are talking about
> > here? Can you please summarize?
>
> TL;DR: When cross-compiling, host little-endian bootstrap bpftool cannot
> open a big-endian ELF to generate a skeleton from it and build target
> big-endian bpftool.
>
> Long version: Currently, bpftool's Makefile compiles the
> skeleton-related programs (skeletons/*.bpf.c) without paying attention
> to the target architecture. When cross-compiling, say on a host with LE
> for a target with BE, this leads to runtime failure on "bpftool prog
> show", because bpftool cannot load the LE bytecode on the BE target
> machine. This is Christophe's output in [0].
>
> So the first fix is to make the Makefile aware of the target endianness
> somehow, and to build this ELF with target endianness. But this is not
> enough, because when (host) boostrap bpftool opens the ELF to generate
> the skeleton from it before building the final (target) bpftool binary,
> then bpf_object__check_endianness() in libbpf refuses to open the ELF if
> endianness is not the same as on the host [1].
>
> The way I see it, we'd need to make sure libbpf can work with ELFs of a
> different endianness -- assuming that's doable -- and to pass it an
> option to tell whether LE or BE is expected for a given ELF. Which in
> turn would require bootstrap bpftool to be aware of the target
> endianness. Or do you see a simpler way?

I don't. We might not need to instruct libbpf what endianness should
be expected, we can just say that BPF object file with wrong
endianness can't be loaded, but could be introspected. So
bpf_object__open() should succeed, all the getters and stuff as well.
But bpf_object__load() will fail. We already support opening BTF of
non-native endianness, that wasn't too bad to support. I'm not sure
how much work and extra logic would be necessary for similar
cross-endianness support in ELF processing code, though.

>
>
> [0]
> https://lore.kernel.org/bpf/0792068b-9aff-d658-5c7d-086e6d394c6c@csgroup.eu/
> [1]
> https://lore.kernel.org/bpf/21b09e52-142d-92f5-4f8b-e4190f89383b@csgroup.eu/

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-02-28  6:32 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-15 14:12 [RFC] libbbpf/bpftool: Support 32-bit Architectures Puranjay Mohan
2023-02-16  1:42 ` Stanislav Fomichev
2023-02-16 22:13   ` Andrii Nakryiko
2023-02-17 10:25     ` Puranjay Mohan
2023-02-17 11:59       ` Quentin Monnet
2023-02-17 21:56         ` Andrii Nakryiko
2023-02-20 11:28           ` Quentin Monnet
2023-02-28  6:31             ` Andrii Nakryiko
2023-02-17 21:45       ` Andrii Nakryiko
2023-02-18 13:23         ` Puranjay Mohan
2023-02-20 16:30           ` Ilya Leoshkevich
2023-02-20 16:47             ` Puranjay Mohan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox