Alexei Starovoitov wrote:
> On Tue, Nov 12, 2019 at 09:33:18PM -0800, John Fastabend wrote:
> >
> > In addition to above flow something like this to load libraries first should
> > also work?
> >
> > // here fw2 is a library its never attached to anything but can be
> > // used to pull functions from
> > obj = bpf_object__open("fw2.o", attr);
> > bpf_object__load(obj);
> > prog = bpf_object__find_program_by_title(obj);
> > subprog_btf_id0 = libbpf_find_obj_btf_id("name of function", obj);
> > subprog_btf_id1 = libbpf_find_obj_btf_id("name of function", obj);
> >
> > // all pairs of (prog_fd, btf_id) need to be specified at load time
> > attr.attach[0].prog_fd = fw2_fd;
> > attr.attach[0].btf_id = subprog_btf_id0;
> > attr.attach[1].prog_fd = fw2_fd;
> > attr.attach[1].btf_id = subprog_btf_id1;
> > obj = bpf_object__open("rootlet.o", attr)
> > bpf_object__load(obj)
> > prog = bpf_object__find_program_by_title(obj);
> > link = bpf_program__replace(prog);
> > // attach rootlet.o at this point with subprog_btf_id
>
> The point I'm arguing that these:
> attr.attach[0].prog_fd = fw2_fd;
> attr.attach[0].btf_id = subprog_btf_id0;
> attr.attach[1].prog_fd = fw2_fd;
> attr.attach[1].btf_id = subprog_btf_id1;
> should not be part of libbpf api. Instead libbpf should be able to adjust
> relocations inside the program. You're proposing to do linking via explicit
> calls, I'm saying such linking should be declarative. libbpf should be able to
> derive the intent from the program and patch calls.
Agree seems cleaner. So when libs are loaded we create a in-kernel table
of functions/symbols and when main program is loaded search for the funcs.
>
> Example:
> helpers.o:
> int foo(struct xdp_md *ctx, int var) {...}
> int bar(int *array, bpf_size_t size) {...}
> obj = bpf_object__open("helpers.o", attr)
> bpf_object__load(obj);
> // load and verify helpers. 'foo' and 'bar' are not attachable to anything.
> // These two programs don't have program type.
> // The kernel loaded and verified them.
> main_prog.o:
> int foo(struct xdp_md *ctx, int var);
> int bar(int *array, bpf_size_t size);
> int main_prog(struct xdp_md *ctx)
> {
> int ar[5], ret;
> ret = foo(ctx, 1) + bar(ar, 5);
> }
> // 'foo' and 'bar' are extern functions from main_prog pov.
> obj = bpf_object__open("main_prog.o", attr)
> bpf_object__load(obj);
> // libbpf finds foo/bar in the kernel and adjusts two call instructions inside
> // main_prog to point to prog_fd+btf_id
>
> That is the second use case of dynamic linking I've been talking earlier. The
This solves my use case.
> same thing should be possible to do with static linking. Then libbpf will
> adjust calls inside main_prog to be 'call pc+123' and 'foo' and 'bar' will
> become traditional bpf subprograms. main_prog() has single 'struct xdp_md *'
> argument. It is normal attachable XDP program.
>
> Loading main_prog.o first and then loading helpers.o should be possible as
> well. The verifier needs BTF of extern 'foo' and 'bar' symbols to be able to
> verify main_prog() independently. For example to check that main_prog() is
> passing correct ctx into foo(). That is the main difference vs traditional
> dynamic linking. I think we all agree that we want bpf programs to be verified
> independently. To do that the verifier needs to have BTF (function prototypes)
> of extern symbols. One can argue that it's not necessary and helpers.o can be
> loaded first. I don't think that will work in all cases. There could be many
> dependencies between helpers1.o calling another helpers2.o and so on and there
> will be no good order where calling extern foo() can be avoided.
Agree, its a bit unclear what we would do with a __weak symbol if a helper.o
is loaded after main.o with that symbol. I think you keep using the __weak
symbol instead of automatically reloading it so you would need some explicit
logic to make that happen in libbpf if user wants.
>
> This thread is getting long :) and sounds like we're converging. I'm thinking
> to combine everything we've discussed so far into dynamic/static linking doc.
>
Yep seems we are close now. FWIW I just backed into a case where at least
static linking is going to be needed so this moves it up on my list because
I'll be using it as soon as its available.
