Message-ID: <20210512233907.skhbwmbnwaajnscm@ast-mbp.dhcp.thefacebook.com>
Date:   Wed, 12 May 2021 16:39:07 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     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>,
        Lorenz Bauer <lmb@...udflare.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, 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.

Powered by blists - more mailing lists