Re: [PATCH v2 bpf-next 2/6] libbpf: rename static variables during linking

On Fri, Apr 23, 2021 at 4:26 PM Yonghong Song [off-list ref] wrote:

On 4/23/21 4:05 PM, Alexei Starovoitov wrote:

quoted

On Fri, Apr 23, 2021 at 2:56 PM Yonghong Song [off-list ref] wrote:

quoted

quoted

quoted

quoted

-static volatile const __u32 print_len;
-static volatile const __u32 ret1;
+volatile const __u32 print_len = 0;
+volatile const __u32 ret1 = 0;

I am little bit puzzled why bpf_iter_test_kern4.c is impacted. I think
this is not in a static link test, right? The same for a few tests below.

All the selftests are passed through a static linker, so it will
append obj_name to each static variable. So I just minimized use of
static variables to avoid too much code churn. If this variable was
static, it would have to be accessed as
skel->rodata->bpf_iter_test_kern4__print_len, for example.

Okay this should be fine. selftests/bpf specific. I just feel that
some people may get confused if they write/see a single program in
selftest and they have to use obj_varname format and thinking this
is a new standard, but actually it is due to static linking buried
in Makefile. Maybe add a note in selftests/README.rst so we
can point to people if there is confusion.

I'm not sure I understand.
Are you saying that
bpftool gen object out_file.o in_file.o
is no longer equivalent to llvm-strip ?

This is more about BTF and ELF.
Give a simple example,
$ cat t1.c
volatile static int aa = 10;
int foo() { return aa; }
$ clang -O2 -g -c -target bpf t1.c

Using bpftool compiled with this patch:
$ bpftool gen object output.o t1.o
$ llvm-readelf -s t1.o | grep aa
      3: 0000000000000000     4 OBJECT  LOCAL  DEFAULT     4 aa
$ llvm-readelf -s output.o | grep aa
      3: 0000000000000000     4 OBJECT  LOCAL  DEFAULT     4 aa

$ bpftool btf dump file t1.o | grep aa
[5] VAR 'aa' type_id=4, linkage=static
$ bpftool btf dump file output.o | grep aa
[5] VAR 't1..aa' type_id=4, linkage=static

Right. That's how I understood it.

So yes you are right, this will affect skeleton user
if you use static linker with single file.

quoted

Since during that step static vars will get their names mangled?
So a good chunk of code that uses skeleton right now should either
1. don't do the linking step
or
2. adjust their code to use global vars
or
3. adjust the usage of skel.h in their corresponding user code
   to accommodate mangled static names?
Did it get it right?

It feels that the strategy of mangling every static name is too harsh.
Maybe sub-skeleton is an answer?
Something that would do a sub-skeleton for each file?
Mainly to keep "bpftool gen object output.o t1.o" from messing with btf names.
Maybe linking of btf-s from multiple .o should somehow scope them?
I don't have a concrete suggestion yet.