netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: 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>,
	Lorenz Bauer <lmb@cloudflare.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: Wed, 12 May 2021 16:39:07 -0700	[thread overview]
Message-ID: <20210512233907.skhbwmbnwaajnscm@ast-mbp.dhcp.thefacebook.com> (raw)
In-Reply-To: <CAEf4BzbJ==4iUFp4pYpkgbKy40+Q6+RTPJVh0gUANHajs88ZTg@mail.gmail.com>

On Wed, May 12, 2021 at 11:50:19AM -0700, Andrii Nakryiko wrote:
> 
> It's not so clear. static allows to have different library names for
> different files. Currently we enforce that version and license
> contents match. It's part of what I said earlier that it feels like we
> need two separate linking commands: one for building BPF libraries and
> one for linking BPF applications. Which is not that far from
> user-space, where you linked shared libraries with a special options.
> We just want BPF static libraries to have properties of user-space BPF
> shared libraries (w.r.t. protection at least). We can discuss it at
> office hours, though.

If we're saying "no" to .a archives (which is user space definition
of static library) then we can reuse the name "BPF static library"
to mean linked .o that is intermediate step towards bpf application .o.
We also need to distinguish BPF static and dynamic libraries.
The dynamic libs would be already loaded in the kernel.
They will be seen by the kernel as partially verified bpf programs.
We can support both global and static style of
the verification for such dynamic libs. The global entry functions
will be verified as global funcs and static funcs can be loaded
without verification if they're not called.
The static funcs wouldn't be static in C file, of course,
since we've put a stop on static visibility.
They would probably need to be global __hidden similar to what
we already do in libbpf with static linking.
The rules we pick should be consistent for dynamic and static libs.
The workflow of loading bpf dynamic library into the kernel and using
it from the application can be made to look very similar to
using bpf static library.

> > After that is done and mmap-ing of data/rodata/bss is done
> > the main skeleton will init sub-skeleton with offsets to their
> > corresponding data based on these offsets?
> > I think that will work for light skel.
> 
> What I had in mind kept skeleton completely isolated from
> sub-skeleton. Think about this, when BPF library author is compiling
> it's user-space parts that use sub-skeleton, they don't and generally
> speaking can't know anything about the final BPF application, so they
> can't have any access to the final skeleton. But they need
> code-generated sub-skeleton header file, similarly to BPF skeleton
> today. So at least for BPF skeleton, the flow I was imagining would be
> like this.
> 
> 1. BPF library abc consists of abc1.bpf.c and abc2.bpf.c. It also has
> user-space component in abc.c.
> 2. BPF app uses abs library and has its own app1.bpf.c and app2.bpf.c
> and app.c for user-space.
> 3. BPF library author sets up its Makefile to do
>   a. clang -target bpf -g -O2 -c abc1.bpf.c -o abc1.bpf.o
>   b. clang -target bpf -g -O2 -c abc2.bpf.c -o abc2.bpf.o
>   c. bpftool gen lib libabc.bpf.o abc1.bpf.o abc2.bpf.o
>   d. bpftool gen subskeleton libabc.bpf.o > libabc.subskel.h
>   e. abc.c (user-space library) is of the form
> 
> #include "libabc.subskel.h"
> 
> static struct libabc_bpf *subskel;
> 
> int libabc__init(struct bpf_object *obj)
> {
>     subskel = libabc_bpf__open_subskel(obj);

right. I was thinking the same for lskel except
there is no 'bpf_object'.
Either subskel_open will receive already adjusted addresses
from the main skel or they will be grouped into aux struct.

>     subskel->data->abc_my_var = 123;

and then library's custom init can do exactly this line.

> }
> 
> int libabc__attach()
> {
>     libabc_bpf__attach(subskel);
> }
> 
>   f. cc abc.c into libabc.a and then libabc.a and libabc.bpf.o are
> distributed to end user
> 
> 3. Now, from BPF application author side:
>   a. clang -target bpf -g -O2 -c app1.bpf.c -o app1.bpf.o
>   b. clang -target bpf -g -O2 -c app2.bpf.c -o app2.bpf.o
>   c. bpftool gen object app.bpf.o app1.bpf.o app2.bpf.o libabc.bpf.o
>   d. on user-space side of app in app.c
> 
> #include "app.skel.h"
> 
> int main()
> {
>     struct app_bpf *skel;
> 
>     skel = app_bpf__open();
>     skel->rodata->app_var = 123;
> 
>     libabc__init(skel->obj);
> 
>     app_bpf__load(skel);
> 
>     libabc__attach();
> 
>     /* probably shouldn't auto-attach library progs, but don't know
> yet how to prevent that */
>     app_bpf__attach(skel);
> 
>     /* do some useful logic now */
> }
> 
>   e. cc app.c -o app && sudo ./app

right. That's a necessary workflow.

> So, app author doesn't need and doesn't have direct access to
> subskeleton header. And sub-skeleton header is generated by BPF
> library way before the library is linked into the final application.

right. We certainly need that for dynamic and static bpf libs.

  reply	other threads:[~2021-05-13  0:05 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
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 [this message]
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=20210512233907.skhbwmbnwaajnscm@ast-mbp.dhcp.thefacebook.com \
    --to=alexei.starovoitov@gmail.com \
    --cc=andrii.nakryiko@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).