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

On Mon, Apr 26, 2021 at 08:44:04AM -0700, Andrii Nakryiko wrote:

quoted

quoted

Static maps are slightly different, because we use SEC() which marks
them as used, so they should always be present.

yes. The used attribute makes the compiler keep the data,
but it can still inline it and lose the reference in the .text.

At least if the map is actually used with helpers (e.g.,
bpf_map_lookup_elem(&map, ...)) it would be invalid for compiler to do
anything crazy with that map reference, because compiler has no
visibility into what opaque helpers do with that memory. So I don't
think it can alias multiple maps, for instance. So I think static maps
should be fine.

Yeah. That makes sense.

See above about passing a pointer to map into black box functions. I'd
bet that the compiler can't merge together two different references at
least because of that.

For static maps, btw, just like for static functions and vars, there
is no symbol, it's an offset into .maps section. We use that offset to
identify the map itself.

Ok. Sounds like there is a desire to expose both static and static volatile
into skeleton.
Sure, but let's make it such the linking step doesn't change the skeleton.
Imagine a project that using single .bpf.c file and skeleton.
It grows and wants to split itself into multiple .bpf.c.
If such split would change the skeleton generated var/map names
it would be annoying user experience.

I see few options to avoid that:
- keeping the btf names as-is during linking
The final .o can have multiple vars and maps with the same name.
The skeleton gen can see the name collision and disambiguate them.
Here I think it's important to give users a choice. Blindly appending
file name is not ideal.
How to express it cleanly in .bpf.c? I don't know. SEC() would be a bit
ugly. May be similar to core flavors? ___1 and ___2 ? Also not ideal.
- another option is to fail skeleton gen if names conflict.
This way the users wold be able to link just fine and traditonal C style
linker behavior will be preserved, but if the user wants a skeleton
then the static map names across .bpf.c files shouldn't conflict.
imo that's reasonable restriction.
- maybe adopt __hidden for vars and maps? Only not hidden (which is default now)
would be seen in skeleton?