Message-ID: <CACAyw99uiX2rkAcqXXHivc0NZ-t2fwZSZfORpe2h_y3AvDDueQ@mail.gmail.com>
Date:   Tue, 11 May 2021 15:20:32 +0100
From:   Lorenz Bauer <lmb@...udflare.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Andrii Nakryiko <andrii.nakryiko@...il.com>,
        Yonghong Song <yhs@...com>,
        Andrii Nakryiko <andrii@...nel.org>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <kernel-team@...com>,
        John Fastabend <john.fastabend@...il.com>
Subject: Re: bpf libraries and static variables. Was: [PATCH v2 bpf-next 2/6]
 libbpf: rename static variables during linking

On Wed, 5 May 2021 at 06:22, Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
>
> > All of the above is up for discussion. I'd love to hear what golang folks
> > are thinking, since above proposal is C centric.

Sorry for the late reply, I was on holiday.

Regarding your conntrack library example:
- what is the difference between impl.bpf.c and ct_api.bpf.c? If I
understand correctly, ct_api is used to generate the skel.h, but impl
isn't?
- what file would main.bpf.c include? ct_api or skel.h?

Regarding Andrii's proposal in the forwarded email to use __hidden,
__internal etc. Are these correct:
- static int foo: this is only available in the same .o, not
accessible from user space. Can be referenced via extern int foo?
- __hidden int foo: only available in same .o, not accessible from user space
- __internal int foo: only available in same .a via extern, not
accessible from user space
- int foo: available / conflicts in all .o, accessible from user space
(aka included in skel.h)

When you speak of the linker, do you mean libbpf or the clang / llvm
linker? The Go toolchain has a simplistic linker to support bpf2bpf
calls from the same .o so I imagine libbpf has something similar.

> I want to clarify a few things that were brought up in offline discussions.
> There are several options:
> 1. don't emit statics at all.
> That will break some skeleton users and doesn't solve the name conflict issue.
> The library authors would need to be careful and use a unique enough
> prefix for all global vars (including attribute("hidden") ones).
> That's no different with traditional static linking in C.
> bpf static linker already rejects linking if file1.bpf.c is trying to
> 'extern int foo()'
> when it was '__hidden int foo();' in file2.bpf.c
> That's safer than traditional linker and the same approach can be
> applied to vars.
> So externing of __hidden vars won't be possible, but they will name conflict.
>
> 2. emit statics when they don't conflict and fail skel gen where there
> is a naming conflict.
> That helps a bit, but library authors still have to be careful with
> both static and global names.
> Which is more annoying than traditional C.

The only way I see this affecting the Go toolchain is if main.bpf.c
includes skel.h, not some other .c (or .h?) Otherwise I would work
hard to keep libraries / programs in their own namespace. The Go
toolchain might end up doing the final link of main.bpf.o and
libct.bpf.a (assuming the .a is linked by llvm or libbpf).

In general I'm with Daniel here that I prefer traditional C static
semantics aka option #1.

>
> 3. do #2 style of failing skel gen if there is a naming conflict, but
> also introduce namespacing concept, so that both global and static
> vars can be automatically namespaced.
> That's the proposal above.
> This way, I'm guessing, some libraries will use namespaces to avoid
> prefixing everything.
> The folks that hate namespaces and #pragmas will do manual prefixes for
> both static and global vars.
>
> For approaches
> char library[]="lru";'
> and
> #pragma comment(lib, "lru")
> the scope of namespace is the whole .bpf.c file.
> The clang/llvm already support it, so the job of name mangling would
> belong to linker.

I think this would work well for Go, because it makes the namespace
explicit. I can imagine that #pragma comment(lib,
"github.com/some/go/package") might be useful. How is the pragma
encoded into the ELF? Would this solve name conflict from multiple
files with the same names?

>
> For __attribute__((annotate("lib=lru"))) the scope could be any number
> of lines in C files between pragma push/pop and can be nested.
> This attribute is supported by clang, but not in the bpf backend.
> The llvm would prefix both global and static names
> in elf file and in btf.

Would there be a way to recover the "lru" part from the mangled ELF somehow?

> If another file.bpf.c needs to call a function from namespace "lru"
> it would need to prefix such a call.
> The skel gen job would be #2 above (emit both static and globals if
> they don't conflict).
> Such namespacing concept would be the closest to c++ namespaces.
>
> If I understood what folks were saying no one is excited about namespaces in C.
> So probably #3 is out and sounds like 1 is prefered?

I think at least allowing for namespaces would be great. Most
languages besides C that will wish to integrate with eBPF allow
namespacing.

--
Lorenz Bauer  |  Systems Engineer
6th Floor, County Hall/The Riverside Building, SE1 7PB, UK

www.cloudflare.com

Powered by blists - more mailing lists