BPF Archive on lore.kernel.org
 help / color / Atom feed
* libbpf/bpftool inconsistent handling og .data and .bss ?
@ 2020-10-07 14:01 Luigi Rizzo
  2020-10-07 15:58 ` Yonghong Song
  2020-10-07 18:35 ` Andrii Nakryiko
  0 siblings, 2 replies; 12+ messages in thread
From: Luigi Rizzo @ 2020-10-07 14:01 UTC (permalink / raw)
  To: bpf, ppenkov, Luigi Rizzo, andriin, sdf

I am experiencing some weirdness in global variables handling
in bpftool and libbpf, as described below.

This happens happen with code in foo_bpf.c compiled with
   clang-10 -O2 -Wall -Werror -target bpf ...
and subsequently exported with
   bpftool gen skeleton ...
(i have tried bpftool 5.8.7 and 5.9.0-rc6)

1. uninitialized globals are not recognised
   The following code in the bpf program

     int x;
     SEC("fentry/bar")
     int BPF_PROG(bar) { return 0;}

   compiles ok but bpftool then complains

      libbpf: prog 'bar': invalid relo against 'x' in special section
0xfff2; forgot to initialize global var?..

   The error disappears if I initialize x=0 or x=1
   (in the skeleton, x=0 ends up in .bss, x=1 ends up in .data)

2. .bss overrides from userspace are not seen in bpf at runtime

    In foo_bpf.c I have "int x = 0;"
    In the userspace program, before foo_bpf__load(), I do
       obj->bss->x = 1
    but after attach, the bpf code does not see the change, ie
        "if (x == 0) { .. } else { .. }"
    always takes the first branch.

    If I initialize "int x = 2" and then do
       obj->data->x = 1
    the update is seen correctly ie
          "if (x == 2) { .. } else { .. }"
     takes one or the other depending on whether userspace overrides
     the value before foo_bpf__load()

3. .data overrides do not seem to work for non-scalar types
    In foo_bpf.c I have
          struct one { int a; }; // type also visible to userspace
          struct one x { .a = 2 }; // avoid bugs #1 and #2
    If in userspace I do
          obj->data->x.a = 1
    the update is not seen in the kernel, ie
            "if (x.a == 2) { .. } else { .. }"
     always takes the first branch

Are these known issues ?

thanks
luigi

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

* Re: libbpf/bpftool inconsistent handling og .data and .bss ?
  2020-10-07 14:01 libbpf/bpftool inconsistent handling og .data and .bss ? Luigi Rizzo
@ 2020-10-07 15:58 ` Yonghong Song
  2020-10-07 18:35 ` Andrii Nakryiko
  1 sibling, 0 replies; 12+ messages in thread
From: Yonghong Song @ 2020-10-07 15:58 UTC (permalink / raw)
  To: Luigi Rizzo, bpf, ppenkov, Luigi Rizzo, andriin, sdf



On 10/7/20 7:01 AM, Luigi Rizzo wrote:
> I am experiencing some weirdness in global variables handling
> in bpftool and libbpf, as described below.
> 
> This happens happen with code in foo_bpf.c compiled with
>     clang-10 -O2 -Wall -Werror -target bpf ...
> and subsequently exported with
>     bpftool gen skeleton ...
> (i have tried bpftool 5.8.7 and 5.9.0-rc6)
> 
> 1. uninitialized globals are not recognised
>     The following code in the bpf program
> 
>       int x;
>       SEC("fentry/bar")
>       int BPF_PROG(bar) { return 0;}
> 
>     compiles ok but bpftool then complains
> 
>        libbpf: prog 'bar': invalid relo against 'x' in special section
> 0xfff2; forgot to initialize global var?..
> 
>     The error disappears if I initialize x=0 or x=1
>     (in the skeleton, x=0 ends up in .bss, x=1 ends up in .data)

Yes, this is a particular issue with llvm10 which without "=0", put
into "common" section which we do not support. The issue is correct
in llvm11 if I remember correctly. But please just use "x = 0" it works
for all versions of compilers.

> 
> 2. .bss overrides from userspace are not seen in bpf at runtime
> 
>      In foo_bpf.c I have "int x = 0;"
>      In the userspace program, before foo_bpf__load(), I do
>         obj->bss->x = 1
>      but after attach, the bpf code does not see the change, ie
>          "if (x == 0) { .. } else { .. }"
>      always takes the first branch.
> 
>      If I initialize "int x = 2" and then do
>         obj->data->x = 1
>      the update is seen correctly ie
>            "if (x == 2) { .. } else { .. }"
>       takes one or the other depending on whether userspace overrides
>       the value before foo_bpf__load()
> 
> 3. .data overrides do not seem to work for non-scalar types
>      In foo_bpf.c I have
>            struct one { int a; }; // type also visible to userspace
>            struct one x { .a = 2 }; // avoid bugs #1 and #2
>      If in userspace I do
>            obj->data->x.a = 1
>      the update is not seen in the kernel, ie
>              "if (x.a == 2) { .. } else { .. }"
>       always takes the first branch
> 
> Are these known issues ?
> 
> thanks
> luigi
> 

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

* Re: libbpf/bpftool inconsistent handling og .data and .bss ?
  2020-10-07 14:01 libbpf/bpftool inconsistent handling og .data and .bss ? Luigi Rizzo
  2020-10-07 15:58 ` Yonghong Song
@ 2020-10-07 18:35 ` Andrii Nakryiko
  2020-10-07 20:31   ` Luigi Rizzo
  1 sibling, 1 reply; 12+ messages in thread
From: Andrii Nakryiko @ 2020-10-07 18:35 UTC (permalink / raw)
  To: Luigi Rizzo
  Cc: bpf, Petar Penkov, Luigi Rizzo, Andrii Nakryiko, Stanislav Fomichev

On Wed, Oct 7, 2020 at 9:03 AM Luigi Rizzo <rizzo@iet.unipi.it> wrote:
>
> I am experiencing some weirdness in global variables handling
> in bpftool and libbpf, as described below.
>
> This happens happen with code in foo_bpf.c compiled with
>    clang-10 -O2 -Wall -Werror -target bpf ...
> and subsequently exported with
>    bpftool gen skeleton ...
> (i have tried bpftool 5.8.7 and 5.9.0-rc6)
>
> 1. uninitialized globals are not recognised
>    The following code in the bpf program
>
>      int x;
>      SEC("fentry/bar")
>      int BPF_PROG(bar) { return 0;}
>
>    compiles ok but bpftool then complains
>
>       libbpf: prog 'bar': invalid relo against 'x' in special section
> 0xfff2; forgot to initialize global var?..
>
>    The error disappears if I initialize x=0 or x=1
>    (in the skeleton, x=0 ends up in .bss, x=1 ends up in .data)

Yonghong addressed this. Just zero-initialize them.

>
> 2. .bss overrides from userspace are not seen in bpf at runtime
>
>     In foo_bpf.c I have "int x = 0;"
>     In the userspace program, before foo_bpf__load(), I do
>        obj->bss->x = 1
>     but after attach, the bpf code does not see the change, ie
>         "if (x == 0) { .. } else { .. }"
>     always takes the first branch.
>
>     If I initialize "int x = 2" and then do
>        obj->data->x = 1
>     the update is seen correctly ie
>           "if (x == 2) { .. } else { .. }"
>      takes one or the other depending on whether userspace overrides
>      the value before foo_bpf__load()

This is quite surprising, given we have explicit selftests validating
that all this works. And it seems to work. Please check
prog_tests/skeleton.c and progs/test_skeleton.c. Can you try running
it and confirm that it works in your setup?


>
> 3. .data overrides do not seem to work for non-scalar types
>     In foo_bpf.c I have
>           struct one { int a; }; // type also visible to userspace
>           struct one x { .a = 2 }; // avoid bugs #1 and #2
>     If in userspace I do
>           obj->data->x.a = 1
>     the update is not seen in the kernel, ie
>             "if (x.a == 2) { .. } else { .. }"
>      always takes the first branch
>

Similarly, the same skeleton selftest tests this situation. So please
check selftests first and report if selftests for some reason don't
work in your case.

> Are these known issues ?
>
> thanks
> luigi

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

* Re: libbpf/bpftool inconsistent handling og .data and .bss ?
  2020-10-07 18:35 ` Andrii Nakryiko
@ 2020-10-07 20:31   ` Luigi Rizzo
  2020-10-07 20:40     ` Andrii Nakryiko
  0 siblings, 1 reply; 12+ messages in thread
From: Luigi Rizzo @ 2020-10-07 20:31 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Luigi Rizzo, bpf, Petar Penkov, Andrii Nakryiko, Stanislav Fomichev

TL;DR; there seems to be a compiler bug with clang-10 and -O2
when struct are in .data -- details below.

On Wed, Oct 7, 2020 at 8:35 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Oct 7, 2020 at 9:03 AM Luigi Rizzo <rizzo@iet.unipi.it> wrote:
> >
> > I am experiencing some weirdness in global variables handling
> > in bpftool and libbpf, as described below.
...
> > 2. .bss overrides from userspace are not seen in bpf at runtime
> >
> >     In foo_bpf.c I have "int x = 0;"
> >     In the userspace program, before foo_bpf__load(), I do
> >        obj->bss->x = 1
> >     but after attach, the bpf code does not see the change, ie
> >         "if (x == 0) { .. } else { .. }"
> >     always takes the first branch.
> >
> >     If I initialize "int x = 2" and then do
> >        obj->data->x = 1
> >     the update is seen correctly ie
> >           "if (x == 2) { .. } else { .. }"
> >      takes one or the other depending on whether userspace overrides
> >      the value before foo_bpf__load()
>
> This is quite surprising, given we have explicit selftests validating
> that all this works. And it seems to work. Please check
> prog_tests/skeleton.c and progs/test_skeleton.c. Can you try running
> it and confirm that it works in your setup?

Ah, this was non intuitive but obvious in hindsight:

.bss is zeroed by the kernel after load(), and since my program
changed the value before foo_bpf__load() , the memory was overwritten
with 0s. I could confirm this by printing the value after load.

If I update obj->data-><something> after __load(),
or even after __attach() given that userspace mmaps .bss and .data,
everything works as expected both for scalars and structs.

> >
> > 3. .data overrides do not seem to work for non-scalar types
> >     In foo_bpf.c I have
> >           struct one { int a; }; // type also visible to userspace
> >           struct one x { .a = 2 }; // avoid bugs #1 and #2
> >     If in userspace I do
> >           obj->data->x.a = 1
> >     the update is not seen in the kernel, ie
> >             "if (x.a == 2) { .. } else { .. }"
> >      always takes the first branch
> >
>
> Similarly, the same skeleton selftest tests this situation. So please
> check selftests first and report if selftests for some reason don't
> work in your case.

Actually test_skeleton.c does _not_ test for struct in .data,
only in .rodata and .bss

There seems to be a compiler error, at least with clang-10 and -O2

Note how the struct case the compiler uses '2' as immediate value
when reading, whereas in the scalar case it correctly dereferences
the pointer to the variable


Disassembly of section fexit/bar:

0000000000000000 foo_struct:
; int BPF_PROG(foo_struct) {
       0:       b7 01 00 00 02 00 00 00 r1 = 2
;   if (x.a == 2 || x.a > 10) { x.a += 10; }
       1:       15 01 02 00 02 00 00 00 if r1 == 2 goto +2 <LBB1_2>
       2:       b7 02 00 00 0b 00 00 00 r2 = 11
       3:       2d 12 04 00 00 00 00 00 if r2 > r1 goto +4 <LBB1_3>

0000000000000020 LBB1_2:
       4:       07 01 00 00 0a 00 00 00 r1 += 10
       5:       18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r2 = 0 ll
       7:       63 12 00 00 00 00 00 00 *(u32 *)(r2 + 0) = r1

0000000000000040 LBB1_3:
; int BPF_PROG(foo_struct) {
       8:       b7 00 00 00 00 00 00 00 r0 = 0
       9:       95 00 00 00 00 00 00 00 exit

Disassembly of section fexit/baz:

0000000000000000 foo_scalar:
;   if (count_off == 2 || count_off > 10) { count_off += 10; }
       0:       18 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r1 = 0 ll
       2:       61 12 00 00 00 00 00 00 r2 = *(u32 *)(r1 + 0)
       3:       15 02 02 00 02 00 00 00 if r2 == 2 goto +2 <LBB2_2>
       4:       b7 03 00 00 0b 00 00 00 r3 = 11
       5:       2d 23 02 00 00 00 00 00 if r3 > r2 goto +2 <LBB2_3>

0000000000000030 LBB2_2:
       6:       07 02 00 00 0a 00 00 00 r2 += 10
       7:       63 21 00 00 00 00 00 00 *(u32 *)(r1 + 0) = r2

0000000000000040 LBB2_3:
; int BPF_PROG(foo_scalar) {
       8:       b7 00 00 00 00 00 00 00 r0 = 0
       9:       95 00 00 00 00 00 00 00 exit

------------

If I put the struct in .bss then it gets translated correctly:

Disassembly of section fexit/bar:

0000000000000000 foo_struct:
;   if (x.a == 2 || x.a > 10) { x.a += 10; }
       0:       18 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r1 = 0 ll
       2:       61 12 00 00 00 00 00 00 r2 = *(u32 *)(r1 + 0)
       3:       15 02 02 00 02 00 00 00 if r2 == 2 goto +2 <LBB1_2>
       4:       b7 03 00 00 0b 00 00 00 r3 = 11
       5:       2d 23 02 00 00 00 00 00 if r3 > r2 goto +2 <LBB1_3>

0000000000000030 LBB1_2:
       6:       07 02 00 00 0a 00 00 00 r2 += 10
       7:       63 21 00 00 00 00 00 00 *(u32 *)(r1 + 0) = r2

0000000000000040 LBB1_3:
; int BPF_PROG(foo_struct) {
       8:       b7 00 00 00 00 00 00 00 r0 = 0
       9:       95 00 00 00 00 00 00 00 exit

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

* Re: libbpf/bpftool inconsistent handling og .data and .bss ?
  2020-10-07 20:31   ` Luigi Rizzo
@ 2020-10-07 20:40     ` Andrii Nakryiko
  2020-10-07 21:29       ` Luigi Rizzo
  0 siblings, 1 reply; 12+ messages in thread
From: Andrii Nakryiko @ 2020-10-07 20:40 UTC (permalink / raw)
  To: Luigi Rizzo
  Cc: Luigi Rizzo, bpf, Petar Penkov, Andrii Nakryiko, Stanislav Fomichev

On Wed, Oct 7, 2020 at 1:31 PM Luigi Rizzo <lrizzo@google.com> wrote:
>
> TL;DR; there seems to be a compiler bug with clang-10 and -O2
> when struct are in .data -- details below.
>
> On Wed, Oct 7, 2020 at 8:35 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Wed, Oct 7, 2020 at 9:03 AM Luigi Rizzo <rizzo@iet.unipi.it> wrote:
> > >
> > > I am experiencing some weirdness in global variables handling
> > > in bpftool and libbpf, as described below.
> ...
> > > 2. .bss overrides from userspace are not seen in bpf at runtime
> > >
> > >     In foo_bpf.c I have "int x = 0;"
> > >     In the userspace program, before foo_bpf__load(), I do
> > >        obj->bss->x = 1
> > >     but after attach, the bpf code does not see the change, ie
> > >         "if (x == 0) { .. } else { .. }"
> > >     always takes the first branch.
> > >
> > >     If I initialize "int x = 2" and then do
> > >        obj->data->x = 1
> > >     the update is seen correctly ie
> > >           "if (x == 2) { .. } else { .. }"
> > >      takes one or the other depending on whether userspace overrides
> > >      the value before foo_bpf__load()
> >
> > This is quite surprising, given we have explicit selftests validating
> > that all this works. And it seems to work. Please check
> > prog_tests/skeleton.c and progs/test_skeleton.c. Can you try running
> > it and confirm that it works in your setup?
>
> Ah, this was non intuitive but obvious in hindsight:
>
> .bss is zeroed by the kernel after load(), and since my program
> changed the value before foo_bpf__load() , the memory was overwritten
> with 0s. I could confirm this by printing the value after load.
>
> If I update obj->data-><something> after __load(),
> or even after __attach() given that userspace mmaps .bss and .data,
> everything works as expected both for scalars and structs.

Check prog_tests/skeleton.c again, it sets .data, .bss, and .rodata
before the load. And checks that those values are preserved after
load. So .bss, if you initialize it manually, shouldn't zero-out what
you set.

>
> > >
> > > 3. .data overrides do not seem to work for non-scalar types
> > >     In foo_bpf.c I have
> > >           struct one { int a; }; // type also visible to userspace
> > >           struct one x { .a = 2 }; // avoid bugs #1 and #2
> > >     If in userspace I do
> > >           obj->data->x.a = 1
> > >     the update is not seen in the kernel, ie
> > >             "if (x.a == 2) { .. } else { .. }"
> > >      always takes the first branch
> > >
> >
> > Similarly, the same skeleton selftest tests this situation. So please
> > check selftests first and report if selftests for some reason don't
> > work in your case.
>
> Actually test_skeleton.c does _not_ test for struct in .data,
> only in .rodata and .bss

It doesn't matter which section it's in, I meant it's testing struct
field accesses from at least one of global data sections.

>
> There seems to be a compiler error, at least with clang-10 and -O2
>
> Note how the struct case the compiler uses '2' as immediate value
> when reading, whereas in the scalar case it correctly dereferences
> the pointer to the variable

It would be useful to include your original source code, especially
the variable declaration parts. I suspect that you declared your
struct variable as a static variable? In that case Clang will assume
nothing can change the value and can inline values like 2. So either
make sure you have a global variable declaration or use `static
volatile`. See how `const volatile` is used throughout all selftests
when working with the .rodata section.

[...]

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

* Re: libbpf/bpftool inconsistent handling og .data and .bss ?
  2020-10-07 20:40     ` Andrii Nakryiko
@ 2020-10-07 21:29       ` Luigi Rizzo
  2020-10-07 22:26         ` Andrii Nakryiko
  2020-10-10 22:49         ` Luigi Rizzo
  0 siblings, 2 replies; 12+ messages in thread
From: Luigi Rizzo @ 2020-10-07 21:29 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Luigi Rizzo, bpf, Petar Penkov, Andrii Nakryiko, Stanislav Fomichev

On Wed, Oct 7, 2020 at 10:40 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Oct 7, 2020 at 1:31 PM Luigi Rizzo <lrizzo@google.com> wrote:
> >
> > TL;DR; there seems to be a compiler bug with clang-10 and -O2
> > when struct are in .data -- details below.
> >
> > On Wed, Oct 7, 2020 at 8:35 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Wed, Oct 7, 2020 at 9:03 AM Luigi Rizzo <rizzo@iet.unipi.it> wrote:
> > > >
> > > > I am experiencing some weirdness in global variables handling
> > > > in bpftool and libbpf, as described below.
> > ...
> > > > 2. .bss overrides from userspace are not seen in bpf at runtime
> > > >
> > > >     In foo_bpf.c I have "int x = 0;"
> > > >     In the userspace program, before foo_bpf__load(), I do
> > > >        obj->bss->x = 1
> > > >     but after attach, the bpf code does not see the change, ie
> > > >         "if (x == 0) { .. } else { .. }"
> > > >     always takes the first branch.
> > > >
> > > >     If I initialize "int x = 2" and then do
> > > >        obj->data->x = 1
> > > >     the update is seen correctly ie
> > > >           "if (x == 2) { .. } else { .. }"
> > > >      takes one or the other depending on whether userspace overrides
> > > >      the value before foo_bpf__load()
> > >
> > > This is quite surprising, given we have explicit selftests validating
> > > that all this works. And it seems to work. Please check
> > > prog_tests/skeleton.c and progs/test_skeleton.c. Can you try running
> > > it and confirm that it works in your setup?
> >
> > Ah, this was non intuitive but obvious in hindsight:
> >
> > .bss is zeroed by the kernel after load(), and since my program
> > changed the value before foo_bpf__load() , the memory was overwritten
> > with 0s. I could confirm this by printing the value after load.
> >
> > If I update obj->data-><something> after __load(),
> > or even after __attach() given that userspace mmaps .bss and .data,
> > everything works as expected both for scalars and structs.
>
> Check prog_tests/skeleton.c again, it sets .data, .bss, and .rodata
> before the load. And checks that those values are preserved after
> load. So .bss, if you initialize it manually, shouldn't zero-out what
> you set.

Don't know what to say: it is cleared on my laptop 5.7.17

I printed the values around assignments and calls
(also verified that obj->bss does not change):
Below, x is "uint32_t x = 0" in .bss
struct one { uint32_t a } s = { .a = 2} " in .data
Program output:

before load, obj->bss is 0x7fb0698b6000
initially x is 0 s.a is 2
// x = 1; s.a = 3
before load x is 1 s.a is 3
after load, obj->bss is 0x7fb0698b6000
after load x is 0 s.a is 3 // note x is cleared, s is left alone
// x = 2; s.a = 4;
after assign x is 2 s.a is 4 variables by 10 every 5ms
// attach, when the program runs (every 5ms) does
// if (s.a == 2 || s.a > 10) { x += 10; s.a += 10}
after attach x is 12 s.a is 12
at runtime count_off is 2382 x is 12
at runtime count_off is 2382 x is 12
...

Could it be some security setting ?

>
> >
> > > >
> > > > 3. .data overrides do not seem to work for non-scalar types
> > > >     In foo_bpf.c I have
> > > >           struct one { int a; }; // type also visible to userspace
> > > >           struct one x { .a = 2 }; // avoid bugs #1 and #2
> > > >     If in userspace I do
> > > >           obj->data->x.a = 1
> > > >     the update is not seen in the kernel, ie
> > > >             "if (x.a == 2) { .. } else { .. }"
> > > >      always takes the first branch
> > > >
> > >
> > > Similarly, the same skeleton selftest tests this situation. So please
> > > check selftests first and report if selftests for some reason don't
> > > work in your case.
> >
> > Actually test_skeleton.c does _not_ test for struct in .data,
> > only in .rodata and .bss
>
> It doesn't matter which section it's in, I meant it's testing struct
> field accesses from at least one of global data sections.

Right but as the llvm-objdump shows, the compiler is treating
.bss and .data differently, at least for struct reads.

>
> >
> > There seems to be a compiler error, at least with clang-10 and -O2
> >
> > Note how the struct case the compiler uses '2' as immediate value
> > when reading, whereas in the scalar case it correctly dereferences
> > the pointer to the variable
>
> It would be useful to include your original source code, especially
> the variable declaration parts. I suspect that you declared your
> struct variable as a static variable? In that case Clang will assume
> nothing can change the value and can inline values like 2. So either
> make sure you have a global variable declaration or use `static
> volatile`. See how `const volatile` is used throughout all selftests
> when working with the .rodata section.

Perhaps the easiest is to see it on godbolt:

https://godbolt.org/z/Mnx38v

and how clang gets terribly confused when compiling read access
to the struct_in_data field

cheers
luigi

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

* Re: libbpf/bpftool inconsistent handling og .data and .bss ?
  2020-10-07 21:29       ` Luigi Rizzo
@ 2020-10-07 22:26         ` Andrii Nakryiko
  2020-10-08  1:33           ` Yonghong Song
  2020-10-10 22:49         ` Luigi Rizzo
  1 sibling, 1 reply; 12+ messages in thread
From: Andrii Nakryiko @ 2020-10-07 22:26 UTC (permalink / raw)
  To: Luigi Rizzo
  Cc: Luigi Rizzo, bpf, Petar Penkov, Andrii Nakryiko, Stanislav Fomichev

On Wed, Oct 7, 2020 at 2:29 PM Luigi Rizzo <lrizzo@google.com> wrote:
>
> On Wed, Oct 7, 2020 at 10:40 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Wed, Oct 7, 2020 at 1:31 PM Luigi Rizzo <lrizzo@google.com> wrote:
> > >
> > > TL;DR; there seems to be a compiler bug with clang-10 and -O2
> > > when struct are in .data -- details below.
> > >
> > > On Wed, Oct 7, 2020 at 8:35 PM Andrii Nakryiko
> > > <andrii.nakryiko@gmail.com> wrote:
> > > >
> > > > On Wed, Oct 7, 2020 at 9:03 AM Luigi Rizzo <rizzo@iet.unipi.it> wrote:
> > > > >
> > > > > I am experiencing some weirdness in global variables handling
> > > > > in bpftool and libbpf, as described below.
> > > ...
> > > > > 2. .bss overrides from userspace are not seen in bpf at runtime
> > > > >
> > > > >     In foo_bpf.c I have "int x = 0;"
> > > > >     In the userspace program, before foo_bpf__load(), I do
> > > > >        obj->bss->x = 1
> > > > >     but after attach, the bpf code does not see the change, ie
> > > > >         "if (x == 0) { .. } else { .. }"
> > > > >     always takes the first branch.
> > > > >
> > > > >     If I initialize "int x = 2" and then do
> > > > >        obj->data->x = 1
> > > > >     the update is seen correctly ie
> > > > >           "if (x == 2) { .. } else { .. }"
> > > > >      takes one or the other depending on whether userspace overrides
> > > > >      the value before foo_bpf__load()
> > > >
> > > > This is quite surprising, given we have explicit selftests validating
> > > > that all this works. And it seems to work. Please check
> > > > prog_tests/skeleton.c and progs/test_skeleton.c. Can you try running
> > > > it and confirm that it works in your setup?
> > >
> > > Ah, this was non intuitive but obvious in hindsight:
> > >
> > > .bss is zeroed by the kernel after load(), and since my program
> > > changed the value before foo_bpf__load() , the memory was overwritten
> > > with 0s. I could confirm this by printing the value after load.
> > >
> > > If I update obj->data-><something> after __load(),
> > > or even after __attach() given that userspace mmaps .bss and .data,
> > > everything works as expected both for scalars and structs.
> >
> > Check prog_tests/skeleton.c again, it sets .data, .bss, and .rodata
> > before the load. And checks that those values are preserved after
> > load. So .bss, if you initialize it manually, shouldn't zero-out what
> > you set.
>
> Don't know what to say: it is cleared on my laptop 5.7.17
>
> I printed the values around assignments and calls
> (also verified that obj->bss does not change):
> Below, x is "uint32_t x = 0" in .bss
> struct one { uint32_t a } s = { .a = 2} " in .data
> Program output:
>
> before load, obj->bss is 0x7fb0698b6000
> initially x is 0 s.a is 2
> // x = 1; s.a = 3
> before load x is 1 s.a is 3
> after load, obj->bss is 0x7fb0698b6000
> after load x is 0 s.a is 3 // note x is cleared, s is left alone
> // x = 2; s.a = 4;
> after assign x is 2 s.a is 4 variables by 10 every 5ms
> // attach, when the program runs (every 5ms) does
> // if (s.a == 2 || s.a > 10) { x += 10; s.a += 10}
> after attach x is 12 s.a is 12
> at runtime count_off is 2382 x is 12
> at runtime count_off is 2382 x is 12
> ...
>
> Could it be some security setting ?
>
> >
> > >
> > > > >
> > > > > 3. .data overrides do not seem to work for non-scalar types
> > > > >     In foo_bpf.c I have
> > > > >           struct one { int a; }; // type also visible to userspace
> > > > >           struct one x { .a = 2 }; // avoid bugs #1 and #2
> > > > >     If in userspace I do
> > > > >           obj->data->x.a = 1
> > > > >     the update is not seen in the kernel, ie
> > > > >             "if (x.a == 2) { .. } else { .. }"
> > > > >      always takes the first branch
> > > > >
> > > >
> > > > Similarly, the same skeleton selftest tests this situation. So please
> > > > check selftests first and report if selftests for some reason don't
> > > > work in your case.
> > >
> > > Actually test_skeleton.c does _not_ test for struct in .data,
> > > only in .rodata and .bss
> >
> > It doesn't matter which section it's in, I meant it's testing struct
> > field accesses from at least one of global data sections.
>
> Right but as the llvm-objdump shows, the compiler is treating
> .bss and .data differently, at least for struct reads.
>
> >
> > >
> > > There seems to be a compiler error, at least with clang-10 and -O2
> > >
> > > Note how the struct case the compiler uses '2' as immediate value
> > > when reading, whereas in the scalar case it correctly dereferences
> > > the pointer to the variable
> >
> > It would be useful to include your original source code, especially
> > the variable declaration parts. I suspect that you declared your
> > struct variable as a static variable? In that case Clang will assume
> > nothing can change the value and can inline values like 2. So either
> > make sure you have a global variable declaration or use `static
> > volatile`. See how `const volatile` is used throughout all selftests
> > when working with the .rodata section.
>
> Perhaps the easiest is to see it on godbolt:
>
> https://godbolt.org/z/Mnx38v
>

Thanks for the example. I can also reproduce this locally. It does
seem like a Clang/LLVM bug at this point. The generated code makes
absolutely no sense to me:

r1 = 100
if r1 > 3 goto +5
r1 = 3
r1 += 111

Something fishy is going on there. I bet Yonghong will quickly figure
out what's going on.

BTW, I tried `static volatile` for the variable, marking volatile
field a, marking variable as __attribute__((weak)). Nothing really
helps, generated code is still weird and inlines constants.

> and how clang gets terribly confused when compiling read access
> to the struct_in_data field
>
> cheers
> luigi

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

* Re: libbpf/bpftool inconsistent handling og .data and .bss ?
  2020-10-07 22:26         ` Andrii Nakryiko
@ 2020-10-08  1:33           ` Yonghong Song
  0 siblings, 0 replies; 12+ messages in thread
From: Yonghong Song @ 2020-10-08  1:33 UTC (permalink / raw)
  To: Andrii Nakryiko, Luigi Rizzo
  Cc: Luigi Rizzo, bpf, Petar Penkov, Andrii Nakryiko, Stanislav Fomichev



On 10/7/20 3:26 PM, Andrii Nakryiko wrote:
> On Wed, Oct 7, 2020 at 2:29 PM Luigi Rizzo <lrizzo@google.com> wrote:
>>
>> On Wed, Oct 7, 2020 at 10:40 PM Andrii Nakryiko
>> <andrii.nakryiko@gmail.com> wrote:
>>>
>>> On Wed, Oct 7, 2020 at 1:31 PM Luigi Rizzo <lrizzo@google.com> wrote:
>>>>
>>>> TL;DR; there seems to be a compiler bug with clang-10 and -O2
>>>> when struct are in .data -- details below.
>>>>
>>>> On Wed, Oct 7, 2020 at 8:35 PM Andrii Nakryiko
>>>> <andrii.nakryiko@gmail.com> wrote:
>>>>>
>>>>> On Wed, Oct 7, 2020 at 9:03 AM Luigi Rizzo <rizzo@iet.unipi.it> wrote:
>>>>>>
>>>>>> I am experiencing some weirdness in global variables handling
>>>>>> in bpftool and libbpf, as described below.
>>>> ...
>>>>>> 2. .bss overrides from userspace are not seen in bpf at runtime
>>>>>>
>>>>>>      In foo_bpf.c I have "int x = 0;"
>>>>>>      In the userspace program, before foo_bpf__load(), I do
>>>>>>         obj->bss->x = 1
>>>>>>      but after attach, the bpf code does not see the change, ie
>>>>>>          "if (x == 0) { .. } else { .. }"
>>>>>>      always takes the first branch.
>>>>>>
>>>>>>      If I initialize "int x = 2" and then do
>>>>>>         obj->data->x = 1
>>>>>>      the update is seen correctly ie
>>>>>>            "if (x == 2) { .. } else { .. }"
>>>>>>       takes one or the other depending on whether userspace overrides
>>>>>>       the value before foo_bpf__load()
>>>>>
>>>>> This is quite surprising, given we have explicit selftests validating
>>>>> that all this works. And it seems to work. Please check
>>>>> prog_tests/skeleton.c and progs/test_skeleton.c. Can you try running
>>>>> it and confirm that it works in your setup?
>>>>
>>>> Ah, this was non intuitive but obvious in hindsight:
>>>>
>>>> .bss is zeroed by the kernel after load(), and since my program
>>>> changed the value before foo_bpf__load() , the memory was overwritten
>>>> with 0s. I could confirm this by printing the value after load.
>>>>
>>>> If I update obj->data-><something> after __load(),
>>>> or even after __attach() given that userspace mmaps .bss and .data,
>>>> everything works as expected both for scalars and structs.
>>>
>>> Check prog_tests/skeleton.c again, it sets .data, .bss, and .rodata
>>> before the load. And checks that those values are preserved after
>>> load. So .bss, if you initialize it manually, shouldn't zero-out what
>>> you set.
>>
>> Don't know what to say: it is cleared on my laptop 5.7.17
>>
>> I printed the values around assignments and calls
>> (also verified that obj->bss does not change):
>> Below, x is "uint32_t x = 0" in .bss
>> struct one { uint32_t a } s = { .a = 2} " in .data
>> Program output:
>>
>> before load, obj->bss is 0x7fb0698b6000
>> initially x is 0 s.a is 2
>> // x = 1; s.a = 3
>> before load x is 1 s.a is 3
>> after load, obj->bss is 0x7fb0698b6000
>> after load x is 0 s.a is 3 // note x is cleared, s is left alone
>> // x = 2; s.a = 4;
>> after assign x is 2 s.a is 4 variables by 10 every 5ms
>> // attach, when the program runs (every 5ms) does
>> // if (s.a == 2 || s.a > 10) { x += 10; s.a += 10}
>> after attach x is 12 s.a is 12
>> at runtime count_off is 2382 x is 12
>> at runtime count_off is 2382 x is 12
>> ...
>>
>> Could it be some security setting ?
>>
>>>
>>>>
>>>>>>
>>>>>> 3. .data overrides do not seem to work for non-scalar types
>>>>>>      In foo_bpf.c I have
>>>>>>            struct one { int a; }; // type also visible to userspace
>>>>>>            struct one x { .a = 2 }; // avoid bugs #1 and #2
>>>>>>      If in userspace I do
>>>>>>            obj->data->x.a = 1
>>>>>>      the update is not seen in the kernel, ie
>>>>>>              "if (x.a == 2) { .. } else { .. }"
>>>>>>       always takes the first branch
>>>>>>
>>>>>
>>>>> Similarly, the same skeleton selftest tests this situation. So please
>>>>> check selftests first and report if selftests for some reason don't
>>>>> work in your case.
>>>>
>>>> Actually test_skeleton.c does _not_ test for struct in .data,
>>>> only in .rodata and .bss
>>>
>>> It doesn't matter which section it's in, I meant it's testing struct
>>> field accesses from at least one of global data sections.
>>
>> Right but as the llvm-objdump shows, the compiler is treating
>> .bss and .data differently, at least for struct reads.
>>
>>>
>>>>
>>>> There seems to be a compiler error, at least with clang-10 and -O2
>>>>
>>>> Note how the struct case the compiler uses '2' as immediate value
>>>> when reading, whereas in the scalar case it correctly dereferences
>>>> the pointer to the variable
>>>
>>> It would be useful to include your original source code, especially
>>> the variable declaration parts. I suspect that you declared your
>>> struct variable as a static variable? In that case Clang will assume
>>> nothing can change the value and can inline values like 2. So either
>>> make sure you have a global variable declaration or use `static
>>> volatile`. See how `const volatile` is used throughout all selftests
>>> when working with the .rodata section.
>>
>> Perhaps the easiest is to see it on godbolt:
>>
>> https://godbolt.org/z/Mnx38v
>>
> 
> Thanks for the example. I can also reproduce this locally. It does
> seem like a Clang/LLVM bug at this point. The generated code makes
> absolutely no sense to me:
> 
> r1 = 100
> if r1 > 3 goto +5
> r1 = 3
> r1 += 111
> 
> Something fishy is going on there. I bet Yonghong will quickly figure
> out what's going on.

Thanks a lot for the test! This exposed a serious bug in llvm backend
for load optimization. The original opt pass is implemented in 2017
with local variables with initializer. Now *more* uses of global 
variables exposed additional bugs. I have posted a fix at
    https://reviews.llvm.org/D89021

> 
> BTW, I tried `static volatile` for the variable, marking volatile
> field a, marking variable as __attribute__((weak)). Nothing really
> helps, generated code is still weird and inlines constants.
> 
>> and how clang gets terribly confused when compiling read access
>> to the struct_in_data field
>>
>> cheers
>> luigi

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

* Re: libbpf/bpftool inconsistent handling og .data and .bss ?
  2020-10-07 21:29       ` Luigi Rizzo
  2020-10-07 22:26         ` Andrii Nakryiko
@ 2020-10-10 22:49         ` Luigi Rizzo
  2020-10-10 23:11           ` Andrii Nakryiko
  1 sibling, 1 reply; 12+ messages in thread
From: Luigi Rizzo @ 2020-10-10 22:49 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Luigi Rizzo, bpf, Petar Penkov, Andrii Nakryiko, Stanislav Fomichev

Coming back to .bss handling:

On Wed, Oct 7, 2020 at 11:29 PM Luigi Rizzo <lrizzo@google.com> wrote:
>
> On Wed, Oct 7, 2020 at 10:40 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Wed, Oct 7, 2020 at 1:31 PM Luigi Rizzo <lrizzo@google.com> wrote:
> > >
> > > TL;DR; there seems to be a compiler bug with clang-10 and -O2
> > > when struct are in .data -- details below.
> > >
> > > On Wed, Oct 7, 2020 at 8:35 PM Andrii Nakryiko
> > > <andrii.nakryiko@gmail.com> wrote:
> > > >
> > > > On Wed, Oct 7, 2020 at 9:03 AM Luigi Rizzo <rizzo@iet.unipi.it> wrote:
> > > ...
> > > > > 2. .bss overrides from userspace are not seen in bpf at runtime
...
> > > >
> > > > This is quite surprising, given we have explicit selftests validating
> > > > that all this works. And it seems to work. Please check
> > > > prog_tests/skeleton.c and progs/test_skeleton.c. Can you try running
> > > > it and confirm that it works in your setup?
> > >
> > > Ah, this was non intuitive but obvious in hindsight:
> > >
> > > .bss is zeroed by the kernel after load(), and since my program
> > > changed the value before foo_bpf__load() , the memory was overwritten
> > > with 0s. I could confirm this by printing the value after load.
> > >
> > > If I update obj->data-><something> after __load(),
> > > or even after __attach() given that userspace mmaps .bss and .data,
> > > everything works as expected both for scalars and structs.
> >
> > Check prog_tests/skeleton.c again, it sets .data, .bss, and .rodata
> > before the load. And checks that those values are preserved after
> > load. So .bss, if you initialize it manually, shouldn't zero-out what
> > you set.

strace reveals that the .bss is initially created as anonymous memory:

  mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_ANONYMOUS, -1,
0) = 0x7fd074a5f000
  write(2, "after open bss is at 0x7fd074a5f"..., 36after open bss is
at 0x7fd074a5f000) = 36

and then remapped after the map has been created:
  bpf(BPF_MAP_CREATE, {map_type=BPF_MAP_TYPE_ARRAY, key_size=4,
value_size=144,  max_entries=1, map_flags=0x400 /* BPF_F_??? */,
inner_map_fd=0, map_name="hstats_b.bss", map_ifindex=0, ...}, 120) = 6
  ...
  mmap(0x7fd074a5f000, 4096, PROT_READ|PROT_WRITE,
MAP_SHARED|MAP_FIXED, 6, 0) = 0x7fd074a5f000

so the original content is gone.

cheers
luigi

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

* Re: libbpf/bpftool inconsistent handling og .data and .bss ?
  2020-10-10 22:49         ` Luigi Rizzo
@ 2020-10-10 23:11           ` Andrii Nakryiko
  2020-10-11  0:31             ` Luigi Rizzo
  0 siblings, 1 reply; 12+ messages in thread
From: Andrii Nakryiko @ 2020-10-10 23:11 UTC (permalink / raw)
  To: Luigi Rizzo
  Cc: Luigi Rizzo, bpf, Petar Penkov, Andrii Nakryiko, Stanislav Fomichev

On Sat, Oct 10, 2020 at 3:49 PM Luigi Rizzo <lrizzo@google.com> wrote:
>
> Coming back to .bss handling:
>
> On Wed, Oct 7, 2020 at 11:29 PM Luigi Rizzo <lrizzo@google.com> wrote:
> >
> > On Wed, Oct 7, 2020 at 10:40 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Wed, Oct 7, 2020 at 1:31 PM Luigi Rizzo <lrizzo@google.com> wrote:
> > > >
> > > > TL;DR; there seems to be a compiler bug with clang-10 and -O2
> > > > when struct are in .data -- details below.
> > > >
> > > > On Wed, Oct 7, 2020 at 8:35 PM Andrii Nakryiko
> > > > <andrii.nakryiko@gmail.com> wrote:
> > > > >
> > > > > On Wed, Oct 7, 2020 at 9:03 AM Luigi Rizzo <rizzo@iet.unipi.it> wrote:
> > > > ...
> > > > > > 2. .bss overrides from userspace are not seen in bpf at runtime
> ...
> > > > >
> > > > > This is quite surprising, given we have explicit selftests validating
> > > > > that all this works. And it seems to work. Please check
> > > > > prog_tests/skeleton.c and progs/test_skeleton.c. Can you try running
> > > > > it and confirm that it works in your setup?
> > > >
> > > > Ah, this was non intuitive but obvious in hindsight:
> > > >
> > > > .bss is zeroed by the kernel after load(), and since my program
> > > > changed the value before foo_bpf__load() , the memory was overwritten
> > > > with 0s. I could confirm this by printing the value after load.
> > > >
> > > > If I update obj->data-><something> after __load(),
> > > > or even after __attach() given that userspace mmaps .bss and .data,
> > > > everything works as expected both for scalars and structs.
> > >
> > > Check prog_tests/skeleton.c again, it sets .data, .bss, and .rodata
> > > before the load. And checks that those values are preserved after
> > > load. So .bss, if you initialize it manually, shouldn't zero-out what
> > > you set.
>
> strace reveals that the .bss is initially created as anonymous memory:
>
>   mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_ANONYMOUS, -1,
> 0) = 0x7fd074a5f000
>   write(2, "after open bss is at 0x7fd074a5f"..., 36after open bss is
> at 0x7fd074a5f000) = 36
>
> and then remapped after the map has been created:
>   bpf(BPF_MAP_CREATE, {map_type=BPF_MAP_TYPE_ARRAY, key_size=4,
> value_size=144,  max_entries=1, map_flags=0x400 /* BPF_F_??? */,
> inner_map_fd=0, map_name="hstats_b.bss", map_ifindex=0, ...}, 120) = 6
>   ...
>   mmap(0x7fd074a5f000, 4096, PROT_READ|PROT_WRITE,
> MAP_SHARED|MAP_FIXED, 6, 0) = 0x7fd074a5f000
>
> so the original content is gone.

not exactly, all of .bss, .rodata, .data and .kconfig are first
mmap()'ed as anonymous memory. I've modified test_skeleton.c to
increase .bss size to 8192 bytes size to distinguish it from other
maps:

1. mmap() anonymous memory (just allocating memory that would keep
initial values that you set with skel->bss->my_var = 123):

mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_ANONYMOUS, -1,
0) = 0x7fb3b406f000

2. use that anonymous memory with initialized variables to update map
contents during bpf_object's load:

bpf(BPF_MAP_UPDATE_ELEM, {map_fd=7, key=0x7ffdab521d50,
value=0x7fb3b406f000, flags=BPF_ANY}, 120) = 0

3. now re-mmap() it with MAP_FIXED, so the same memory address is
pointing to ARRAY's content in the kernel. Because of
BPF_MAP_UPDATE_ELEM above, contents should be identical:

mmap(0x7fb3b406f000, 8192, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_FIXED,
7, 0) = 0x7fb3b406f000

4. later we are done with it:

munmap(0x7fb3b406f000, 8192)            = 0

>
> cheers
> luigi

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

* Re: libbpf/bpftool inconsistent handling og .data and .bss ?
  2020-10-10 23:11           ` Andrii Nakryiko
@ 2020-10-11  0:31             ` Luigi Rizzo
  2020-10-11  1:36               ` Andrii Nakryiko
  0 siblings, 1 reply; 12+ messages in thread
From: Luigi Rizzo @ 2020-10-11  0:31 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Luigi Rizzo, bpf, Petar Penkov, Andrii Nakryiko, Stanislav Fomichev

On Sun, Oct 11, 2020 at 1:11 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Sat, Oct 10, 2020 at 3:49 PM Luigi Rizzo <lrizzo@google.com> wrote:
> >
> > Coming back to .bss handling:
> >
> > On Wed, Oct 7, 2020 at 11:29 PM Luigi Rizzo <lrizzo@google.com> wrote:
> > >
> > > On Wed, Oct 7, 2020 at 10:40 PM Andrii Nakryiko
> > > <andrii.nakryiko@gmail.com> wrote:
> > > >
> > > > On Wed, Oct 7, 2020 at 1:31 PM Luigi Rizzo <lrizzo@google.com> wrote:
> > > > >
> > > > > TL;DR; there seems to be a compiler bug with clang-10 and -O2
> > > > > when struct are in .data -- details below.
> > > > >
> > > > > On Wed, Oct 7, 2020 at 8:35 PM Andrii Nakryiko
> > > > > <andrii.nakryiko@gmail.com> wrote:
> > > > > >
> > > > > > On Wed, Oct 7, 2020 at 9:03 AM Luigi Rizzo <rizzo@iet.unipi.it> wrote:
> > > > > ...
> > > > > > > 2. .bss overrides from userspace are not seen in bpf at runtime
> > ...
> > > > > >
> > > > > > This is quite surprising, given we have explicit selftests validating
> > > > > > that all this works. And it seems to work. Please check
> > > > > > prog_tests/skeleton.c and progs/test_skeleton.c. Can you try running
> > > > > > it and confirm that it works in your setup?
> > > > >
> > > > > Ah, this was non intuitive but obvious in hindsight:
> > > > >
> > > > > .bss is zeroed by the kernel after load(), and since my program
> > > > > changed the value before foo_bpf__load() , the memory was overwritten
> > > > > with 0s. I could confirm this by printing the value after load.
> > > > >
> > > > > If I update obj->data-><something> after __load(),
> > > > > or even after __attach() given that userspace mmaps .bss and .data,
> > > > > everything works as expected both for scalars and structs.
> > > >
> > > > Check prog_tests/skeleton.c again, it sets .data, .bss, and .rodata
> > > > before the load. And checks that those values are preserved after
> > > > load. So .bss, if you initialize it manually, shouldn't zero-out what
> > > > you set.
> >
> > strace reveals that the .bss is initially created as anonymous memory:
> >
> >   mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_ANONYMOUS, -1,
> > 0) = 0x7fd074a5f000
> >   write(2, "after open bss is at 0x7fd074a5f"..., 36after open bss is
> > at 0x7fd074a5f000) = 36
> >
> > and then remapped after the map has been created:
> >   bpf(BPF_MAP_CREATE, {map_type=BPF_MAP_TYPE_ARRAY, key_size=4,
> > value_size=144,  max_entries=1, map_flags=0x400 /* BPF_F_??? */,
> > inner_map_fd=0, map_name="hstats_b.bss", map_ifindex=0, ...}, 120) = 6
> >   ...
> >   mmap(0x7fd074a5f000, 4096, PROT_READ|PROT_WRITE,
> > MAP_SHARED|MAP_FIXED, 6, 0) = 0x7fd074a5f000
> >
> > so the original content is gone.
>
> not exactly, all of .bss, .rodata, .data and .kconfig are first
> mmap()'ed as anonymous memory. I've modified test_skeleton.c to
> increase .bss size to 8192 bytes size to distinguish it from other
> maps:
>
> 1. mmap() anonymous memory (just allocating memory that would keep
> initial values that you set with skel->bss->my_var = 123):
>
> mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_ANONYMOUS, -1,
> 0) = 0x7fb3b406f000
>
> 2. use that anonymous memory with initialized variables to update map
> contents during bpf_object's load:
>
> bpf(BPF_MAP_UPDATE_ELEM, {map_fd=7, key=0x7ffdab521d50,
> value=0x7fb3b406f000, flags=BPF_ANY}, 120) = 0

I do not see this BPF_MAP_UPDATE_ELEM for the .bss segment in my strace.
What I see (repeated at the end) is that the .bss map is
created and then just remapped as you indicate below in #3

Maybe this was added in a more recent version of the library
than the one I have?

$ apt info libbpf-dev
Package: libbpf-dev
Version: 1:0.0.8-1
Priority: optional
Section: libdevel
Source: libbpf (0.0.8-1)


cheers
luigi

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

* Re: libbpf/bpftool inconsistent handling og .data and .bss ?
  2020-10-11  0:31             ` Luigi Rizzo
@ 2020-10-11  1:36               ` Andrii Nakryiko
  0 siblings, 0 replies; 12+ messages in thread
From: Andrii Nakryiko @ 2020-10-11  1:36 UTC (permalink / raw)
  To: Luigi Rizzo
  Cc: Luigi Rizzo, bpf, Petar Penkov, Andrii Nakryiko, Stanislav Fomichev

On Sat, Oct 10, 2020 at 5:31 PM Luigi Rizzo <lrizzo@google.com> wrote:
>
> On Sun, Oct 11, 2020 at 1:11 AM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Sat, Oct 10, 2020 at 3:49 PM Luigi Rizzo <lrizzo@google.com> wrote:
> > >
> > > Coming back to .bss handling:
> > >
> > > On Wed, Oct 7, 2020 at 11:29 PM Luigi Rizzo <lrizzo@google.com> wrote:
> > > >
> > > > On Wed, Oct 7, 2020 at 10:40 PM Andrii Nakryiko
> > > > <andrii.nakryiko@gmail.com> wrote:
> > > > >
> > > > > On Wed, Oct 7, 2020 at 1:31 PM Luigi Rizzo <lrizzo@google.com> wrote:
> > > > > >
> > > > > > TL;DR; there seems to be a compiler bug with clang-10 and -O2
> > > > > > when struct are in .data -- details below.
> > > > > >
> > > > > > On Wed, Oct 7, 2020 at 8:35 PM Andrii Nakryiko
> > > > > > <andrii.nakryiko@gmail.com> wrote:
> > > > > > >
> > > > > > > On Wed, Oct 7, 2020 at 9:03 AM Luigi Rizzo <rizzo@iet.unipi.it> wrote:
> > > > > > ...
> > > > > > > > 2. .bss overrides from userspace are not seen in bpf at runtime
> > > ...
> > > > > > >
> > > > > > > This is quite surprising, given we have explicit selftests validating
> > > > > > > that all this works. And it seems to work. Please check
> > > > > > > prog_tests/skeleton.c and progs/test_skeleton.c. Can you try running
> > > > > > > it and confirm that it works in your setup?
> > > > > >
> > > > > > Ah, this was non intuitive but obvious in hindsight:
> > > > > >
> > > > > > .bss is zeroed by the kernel after load(), and since my program
> > > > > > changed the value before foo_bpf__load() , the memory was overwritten
> > > > > > with 0s. I could confirm this by printing the value after load.
> > > > > >
> > > > > > If I update obj->data-><something> after __load(),
> > > > > > or even after __attach() given that userspace mmaps .bss and .data,
> > > > > > everything works as expected both for scalars and structs.
> > > > >
> > > > > Check prog_tests/skeleton.c again, it sets .data, .bss, and .rodata
> > > > > before the load. And checks that those values are preserved after
> > > > > load. So .bss, if you initialize it manually, shouldn't zero-out what
> > > > > you set.
> > >
> > > strace reveals that the .bss is initially created as anonymous memory:
> > >
> > >   mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_ANONYMOUS, -1,
> > > 0) = 0x7fd074a5f000
> > >   write(2, "after open bss is at 0x7fd074a5f"..., 36after open bss is
> > > at 0x7fd074a5f000) = 36
> > >
> > > and then remapped after the map has been created:
> > >   bpf(BPF_MAP_CREATE, {map_type=BPF_MAP_TYPE_ARRAY, key_size=4,
> > > value_size=144,  max_entries=1, map_flags=0x400 /* BPF_F_??? */,
> > > inner_map_fd=0, map_name="hstats_b.bss", map_ifindex=0, ...}, 120) = 6
> > >   ...
> > >   mmap(0x7fd074a5f000, 4096, PROT_READ|PROT_WRITE,
> > > MAP_SHARED|MAP_FIXED, 6, 0) = 0x7fd074a5f000
> > >
> > > so the original content is gone.
> >
> > not exactly, all of .bss, .rodata, .data and .kconfig are first
> > mmap()'ed as anonymous memory. I've modified test_skeleton.c to
> > increase .bss size to 8192 bytes size to distinguish it from other
> > maps:
> >
> > 1. mmap() anonymous memory (just allocating memory that would keep
> > initial values that you set with skel->bss->my_var = 123):
> >
> > mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_ANONYMOUS, -1,
> > 0) = 0x7fb3b406f000
> >
> > 2. use that anonymous memory with initialized variables to update map
> > contents during bpf_object's load:
> >
> > bpf(BPF_MAP_UPDATE_ELEM, {map_fd=7, key=0x7ffdab521d50,
> > value=0x7fb3b406f000, flags=BPF_ANY}, 120) = 0
>
> I do not see this BPF_MAP_UPDATE_ELEM for the .bss segment in my strace.
> What I see (repeated at the end) is that the .bss map is
> created and then just remapped as you indicate below in #3
>
> Maybe this was added in a more recent version of the library
> than the one I have?
>
> $ apt info libbpf-dev
> Package: libbpf-dev
> Version: 1:0.0.8-1

Your version is almost 3 full releases behind. Yes, this was fixed a
long time ago. Please update your libbpf.

> Priority: optional
> Section: libdevel
> Source: libbpf (0.0.8-1)
>
>
> cheers
> luigi

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

end of thread, back to index

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-07 14:01 libbpf/bpftool inconsistent handling og .data and .bss ? Luigi Rizzo
2020-10-07 15:58 ` Yonghong Song
2020-10-07 18:35 ` Andrii Nakryiko
2020-10-07 20:31   ` Luigi Rizzo
2020-10-07 20:40     ` Andrii Nakryiko
2020-10-07 21:29       ` Luigi Rizzo
2020-10-07 22:26         ` Andrii Nakryiko
2020-10-08  1:33           ` Yonghong Song
2020-10-10 22:49         ` Luigi Rizzo
2020-10-10 23:11           ` Andrii Nakryiko
2020-10-11  0:31             ` Luigi Rizzo
2020-10-11  1:36               ` Andrii Nakryiko

BPF Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/bpf/0 bpf/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 bpf bpf/ https://lore.kernel.org/bpf \
		bpf@vger.kernel.org
	public-inbox-index bpf

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.bpf


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git