netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Lorenz Bauer <lmb@cloudflare.com>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Yonghong Song <yhs@fb.com>, Andrii Nakryiko <andrii@kernel.org>,
	bpf <bpf@vger.kernel.org>, Networking <netdev@vger.kernel.org>,
	Alexei Starovoitov <ast@fb.com>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Kernel Team <kernel-team@fb.com>,
	John Fastabend <john.fastabend@gmail.com>
Subject: Re: bpf libraries and static variables. Was: [PATCH v2 bpf-next 2/6] libbpf: rename static variables during linking
Date: Tue, 11 May 2021 11:04:34 -0700	[thread overview]
Message-ID: <CAEf4BzYTgSTBf8+-Hqsv2SF-L17Tu9se=oZ=vEfekQY_+fLB=g@mail.gmail.com> (raw)
In-Reply-To: <CACAyw99uiX2rkAcqXXHivc0NZ-t2fwZSZfORpe2h_y3AvDDueQ@mail.gmail.com>

On Tue, May 11, 2021 at 7:20 AM Lorenz Bauer <lmb@cloudflare.com> wrote:
>
> On Wed, 5 May 2021 at 06:22, Alexei Starovoitov
> <alexei.starovoitov@gmail.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?

I don't think it matters much, the point is that your BPF library can
be compiled from multiple .c files. Same for BPF application itself,
it can be compiled from multiple .c files and use multiple BPF
libraries.

> - what file would main.bpf.c include? ct_api or skel.h?

main.bpf.c will include (if at all) anything that BPF library will
provide *for BPF side of things*. I.e., some sort of ct_api.bpf.h.
skel.h is not supposed to be included in BPF code, only user-space.

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

Yes about availability only from BPF and only within single .o. Not
true about extern, you can't extern statics (just like in user-space).

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

See my last RFC patch set for details (last patch especially with more
details in commit log). It's the other way around. __hidden means
available across multiple .o files during static linking. Once static
linking is done (e.g., you compiled BPF library into my_lib.bpf.o),
__hidden is restricted and converted into __internal. __internal means
not available outside of single .o. global __internal symbol is
equivalent to static variable, except it's accessible from BPF
skeleton/sub-skeleton and we have name uniqueness guarantee.

> - int foo: available / conflicts in all .o, accessible from user space
> (aka included in skel.h)

yes, it's a plain global symbol with STV_DEFAULT visibility.

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

We are talking about libbpf's struct bpf_linker linker.

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

[...]

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

  reply	other threads:[~2021-05-11 18:04 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-23 18:53 [PATCH v2 bpf-next 0/6] BPF static linker: support static vars and maps Andrii Nakryiko
2021-04-23 18:53 ` [PATCH v2 bpf-next 1/6] bpftool: strip const/volatile/restrict modifiers from .bss and .data vars Andrii Nakryiko
2021-04-23 20:02   ` Yonghong Song
2021-04-23 18:53 ` [PATCH v2 bpf-next 2/6] libbpf: rename static variables during linking Andrii Nakryiko
2021-04-23 20:24   ` Yonghong Song
2021-04-23 21:38     ` Andrii Nakryiko
2021-04-23 21:56       ` Yonghong Song
2021-04-23 23:05         ` Alexei Starovoitov
2021-04-23 23:26           ` Yonghong Song
2021-04-23 23:35             ` Alexei Starovoitov
2021-04-23 23:35           ` Andrii Nakryiko
2021-04-23 23:48             ` Alexei Starovoitov
2021-04-24  0:13               ` Andrii Nakryiko
2021-04-24  0:22                 ` Alexei Starovoitov
2021-04-26 15:44                   ` Andrii Nakryiko
2021-04-26 22:34                     ` Alexei Starovoitov
2021-04-26 23:11                       ` Andrii Nakryiko
2021-04-27  2:22                         ` Alexei Starovoitov
2021-04-27 21:27                           ` Andrii Nakryiko
2021-04-28  4:55                             ` Alexei Starovoitov
2021-04-28 19:33                               ` Andrii Nakryiko
2021-05-04  4:42                                 ` bpf libraries and static variables. Was: " Alexei Starovoitov
2021-05-05  5:22                                   ` Alexei Starovoitov
2021-05-06 22:54                                     ` Daniel Borkmann
2021-05-11 17:57                                       ` Andrii Nakryiko
2021-05-11 18:05                                         ` Andrii Nakryiko
2021-05-11 14:20                                     ` Lorenz Bauer
2021-05-11 18:04                                       ` Andrii Nakryiko [this message]
2021-05-11 18:59                                     ` Andrii Nakryiko
2021-05-11 23:05                                       ` Alexei Starovoitov
2021-05-12 13:40                                         ` Lorenz Bauer
2021-05-12 18:50                                         ` Andrii Nakryiko
2021-05-12 23:39                                           ` Alexei Starovoitov
2021-05-13  8:37                                           ` Lorenz Bauer
2021-05-13 15:41                                             ` Alexei Starovoitov
2021-04-24  2:36                 ` Yonghong Song
2021-04-26 15:45                   ` Andrii Nakryiko
2021-04-26 16:34                     ` Yonghong Song
2021-04-23 18:53 ` [PATCH v2 bpf-next 3/6] libbpf: support static map definitions Andrii Nakryiko
2021-04-23 20:25   ` Yonghong Song
2021-04-23 18:53 ` [PATCH v2 bpf-next 4/6] bpftool: handle transformed static map names in BPF skeleton Andrii Nakryiko
2021-04-23 22:59   ` Yonghong Song
2021-04-23 18:53 ` [PATCH v2 bpf-next 5/6] selftests/bpf: extend linked_vars selftests with static variables Andrii Nakryiko
2021-04-23 23:03   ` Yonghong Song
2021-04-23 23:03   ` Yonghong Song
2021-04-23 18:53 ` [PATCH v2 bpf-next 6/6] selftests/bpf: extend linked_maps selftests with static maps Andrii Nakryiko
2021-04-23 23:11   ` Yonghong Song

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAEf4BzYTgSTBf8+-Hqsv2SF-L17Tu9se=oZ=vEfekQY_+fLB=g@mail.gmail.com' \
    --to=andrii.nakryiko@gmail.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@fb.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=john.fastabend@gmail.com \
    --cc=kernel-team@fb.com \
    --cc=lmb@cloudflare.com \
    --cc=netdev@vger.kernel.org \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).