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.