| Message ID | 20191214014341.3442258-1-andriin@fb.com |
|---|---|
| Headers | show |
| Series | Add code-generated BPF object skeleton support | expand |
On Fri, Dec 13, 2019 at 05:43:24PM -0800, Andrii Nakryiko wrote: > This patch set introduces an alternative and complimentary to existing libbpf > API interface for working with BPF objects, maps, programs, and global data > from userspace side. This approach is relying on code generation. bpftool > produces a struct (a.k.a. skeleton) tailored and specific to provided BPF > object file. It includes hard-coded fields and data structures for every map, > program, link, and global data present. > > Altogether this approach significantly reduces amount of userspace boilerplate > code required to open, load, attach, and work with BPF objects. It improves > attach/detach story, by providing pre-allocated space for bpf_links, and > ensuring they are properly detached on shutdown. It allows to do away with by > name/title lookups of maps and programs, because libbpf's skeleton API, in > conjunction with generated code from bpftool, is filling in hard-coded fields > with actual pointers to corresponding struct bpf_map/bpf_program/bpf_link. > > Also, thanks to BPF array mmap() support, working with global data (variables) > from userspace is now as natural as it is from BPF side: each variable is just > a struct field inside skeleton struct. Furthermore, this allows to have > a natural way for userspace to pre-initialize global data (including > previously impossible to initialize .rodata) by just assigning values to the > same per-variable fields. Libbpf will carefully take into account this > initialization image, will use it to pre-populate BPF maps at creation time, > and will re-mmap() BPF map's contents at exactly the same userspace memory > address such that it can continue working with all the same pointers without > any interruptions. If kernel doesn't support mmap(), global data will still be > successfully initialized, but after map creation global data structures inside > skeleton will be NULL-ed out. This allows userspace application to gracefully > handle lack of mmap() support, if necessary. > > A bunch of selftests are also converted to using skeletons, demonstrating > significant simplification of userspace part of test and reduction in amount > of code necessary. > > v3->v4: > - add OPTS_VALID check to btf_dump__emit_type_decl (Alexei); > - expose skeleton as LIBBPF_API functions (Alexei); > - copyright clean up, update internal map init refactor (Alexei); Applied. Thanks. I really liked how much more concise test_fentry_fexit() test has become. I also liked how renaming global variable s/test1_result/_test1_result/ in bpf program became a build time error for user space part: ../prog_tests/fentry_fexit.c:49:35: error: ‘struct fentry_test__bss’ has no member named ‘test1_result’; did you mean ‘_test1_result’? printf("%lld\n", fentry_skel->bss->test1_result); Working with global variables is so much easier now. I'd like you to consider additional feature request. The following error: -BPF_EMBED_OBJ(fentry, "fentry_test.o"); -BPF_EMBED_OBJ(fexit, "fexit_test.o"); +BPF_EMBED_OBJ(fexit, "fentry_test.o"); +BPF_EMBED_OBJ(fentry, "fexit_test.o"); will not be caught. I think skeleton should get smarter somehow to catch that too. One option would be to do BPF_EMBED_OBJ() as part of *.skel.h but then accessing the same embedded .o from multiple tests will not be possible and what stacktrace_build_id.c and stacktrace_build_id_nmi.c are doing won't work anymore. Some sort of build-id/sha1 of .o can work, but it will be caught in run-time. I think build time would be better. May be generate new macro in skel.h that user space can instantiate instead of using common BPF_EMBED_OBJ ?
On Sun, Dec 15, 2019 at 4:30 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Fri, Dec 13, 2019 at 05:43:24PM -0800, Andrii Nakryiko wrote: > > This patch set introduces an alternative and complimentary to existing libbpf > > API interface for working with BPF objects, maps, programs, and global data > > from userspace side. This approach is relying on code generation. bpftool > > produces a struct (a.k.a. skeleton) tailored and specific to provided BPF > > object file. It includes hard-coded fields and data structures for every map, > > program, link, and global data present. > > > > Altogether this approach significantly reduces amount of userspace boilerplate > > code required to open, load, attach, and work with BPF objects. It improves > > attach/detach story, by providing pre-allocated space for bpf_links, and > > ensuring they are properly detached on shutdown. It allows to do away with by > > name/title lookups of maps and programs, because libbpf's skeleton API, in > > conjunction with generated code from bpftool, is filling in hard-coded fields > > with actual pointers to corresponding struct bpf_map/bpf_program/bpf_link. > > > > Also, thanks to BPF array mmap() support, working with global data (variables) > > from userspace is now as natural as it is from BPF side: each variable is just > > a struct field inside skeleton struct. Furthermore, this allows to have > > a natural way for userspace to pre-initialize global data (including > > previously impossible to initialize .rodata) by just assigning values to the > > same per-variable fields. Libbpf will carefully take into account this > > initialization image, will use it to pre-populate BPF maps at creation time, > > and will re-mmap() BPF map's contents at exactly the same userspace memory > > address such that it can continue working with all the same pointers without > > any interruptions. If kernel doesn't support mmap(), global data will still be > > successfully initialized, but after map creation global data structures inside > > skeleton will be NULL-ed out. This allows userspace application to gracefully > > handle lack of mmap() support, if necessary. > > > > A bunch of selftests are also converted to using skeletons, demonstrating > > significant simplification of userspace part of test and reduction in amount > > of code necessary. > > > > v3->v4: > > - add OPTS_VALID check to btf_dump__emit_type_decl (Alexei); > > - expose skeleton as LIBBPF_API functions (Alexei); > > - copyright clean up, update internal map init refactor (Alexei); > > Applied. Thanks. > > I really liked how much more concise test_fentry_fexit() test has become. > I also liked how renaming global variable s/test1_result/_test1_result/ > in bpf program became a build time error for user space part: > ../prog_tests/fentry_fexit.c:49:35: error: ‘struct fentry_test__bss’ has no member named ‘test1_result’; did you mean ‘_test1_result’? > printf("%lld\n", fentry_skel->bss->test1_result); > Working with global variables is so much easier now. > > I'd like you to consider additional feature request. > The following error: > -BPF_EMBED_OBJ(fentry, "fentry_test.o"); > -BPF_EMBED_OBJ(fexit, "fexit_test.o"); > +BPF_EMBED_OBJ(fexit, "fentry_test.o"); > +BPF_EMBED_OBJ(fentry, "fexit_test.o"); > will not be caught. > I think skeleton should get smarter somehow to catch that too. > > One option would be to do BPF_EMBED_OBJ() as part of *.skel.h but then > accessing the same embedded .o from multiple tests will not be possible and > what stacktrace_build_id.c and stacktrace_build_id_nmi.c are doing won't work > anymore. Some sort of build-id/sha1 of .o can work, but it will be caught > in run-time. I think build time would be better. > May be generate new macro in skel.h that user space can instantiate > instead of using common BPF_EMBED_OBJ ? > All those issues are actually very easy to solve. As part of bla.skel.h: .... #ifndef __BLA__SKEL_EMBEDDED #define __BLA__SKEL_EMBEDDED BPF_EMBED_OBJ(<some_identifier>, <path_to_.o>); #endif extern struct bpf_embed_data <some_identifier>_embed; /* we can have a variant of bla__create_skeleton() that just uses above <some_identifier>_embed */ .... That seems to solve all the problems you mentioned. But it creates the problem of knowing/specifying <some_identifier> and <path_to_.o>. While we can "dictate" <some_identifier> (e.g., based on object file name), <path_to_.o> sometimes might need to be overridden, depending on specifics of build system. But I guess we can follow convention-driven way, and in addition to above do something like: #ifndef __BLA__SKEL__OBJ_PATH #define __BLA__SKEL__OBJ_PATH "<whatever path was provided to bpftool to generate skeleton>" #endif /* then just use __BLA__SKEL__OBJ_PATH for BPF_EMBED_OBJ, * which user can override before including skeleton on userspace side */ WDYT?
On Sun, Dec 15, 2019 at 06:01:16PM -0800, Andrii Nakryiko wrote: > On Sun, Dec 15, 2019 at 4:30 PM Alexei Starovoitov > <alexei.starovoitov@gmail.com> wrote: > > > > On Fri, Dec 13, 2019 at 05:43:24PM -0800, Andrii Nakryiko wrote: > > > This patch set introduces an alternative and complimentary to existing libbpf > > > API interface for working with BPF objects, maps, programs, and global data > > > from userspace side. This approach is relying on code generation. bpftool > > > produces a struct (a.k.a. skeleton) tailored and specific to provided BPF > > > object file. It includes hard-coded fields and data structures for every map, > > > program, link, and global data present. > > > > > > Altogether this approach significantly reduces amount of userspace boilerplate > > > code required to open, load, attach, and work with BPF objects. It improves > > > attach/detach story, by providing pre-allocated space for bpf_links, and > > > ensuring they are properly detached on shutdown. It allows to do away with by > > > name/title lookups of maps and programs, because libbpf's skeleton API, in > > > conjunction with generated code from bpftool, is filling in hard-coded fields > > > with actual pointers to corresponding struct bpf_map/bpf_program/bpf_link. > > > > > > Also, thanks to BPF array mmap() support, working with global data (variables) > > > from userspace is now as natural as it is from BPF side: each variable is just > > > a struct field inside skeleton struct. Furthermore, this allows to have > > > a natural way for userspace to pre-initialize global data (including > > > previously impossible to initialize .rodata) by just assigning values to the > > > same per-variable fields. Libbpf will carefully take into account this > > > initialization image, will use it to pre-populate BPF maps at creation time, > > > and will re-mmap() BPF map's contents at exactly the same userspace memory > > > address such that it can continue working with all the same pointers without > > > any interruptions. If kernel doesn't support mmap(), global data will still be > > > successfully initialized, but after map creation global data structures inside > > > skeleton will be NULL-ed out. This allows userspace application to gracefully > > > handle lack of mmap() support, if necessary. > > > > > > A bunch of selftests are also converted to using skeletons, demonstrating > > > significant simplification of userspace part of test and reduction in amount > > > of code necessary. > > > > > > v3->v4: > > > - add OPTS_VALID check to btf_dump__emit_type_decl (Alexei); > > > - expose skeleton as LIBBPF_API functions (Alexei); > > > - copyright clean up, update internal map init refactor (Alexei); > > > > Applied. Thanks. > > > > I really liked how much more concise test_fentry_fexit() test has become. > > I also liked how renaming global variable s/test1_result/_test1_result/ > > in bpf program became a build time error for user space part: > > ../prog_tests/fentry_fexit.c:49:35: error: ‘struct fentry_test__bss’ has no member named ‘test1_result’; did you mean ‘_test1_result’? > > printf("%lld\n", fentry_skel->bss->test1_result); > > Working with global variables is so much easier now. > > > > I'd like you to consider additional feature request. > > The following error: > > -BPF_EMBED_OBJ(fentry, "fentry_test.o"); > > -BPF_EMBED_OBJ(fexit, "fexit_test.o"); > > +BPF_EMBED_OBJ(fexit, "fentry_test.o"); > > +BPF_EMBED_OBJ(fentry, "fexit_test.o"); > > will not be caught. > > I think skeleton should get smarter somehow to catch that too. > > > > One option would be to do BPF_EMBED_OBJ() as part of *.skel.h but then > > accessing the same embedded .o from multiple tests will not be possible and > > what stacktrace_build_id.c and stacktrace_build_id_nmi.c are doing won't work > > anymore. Some sort of build-id/sha1 of .o can work, but it will be caught > > in run-time. I think build time would be better. > > May be generate new macro in skel.h that user space can instantiate > > instead of using common BPF_EMBED_OBJ ? > > > > All those issues are actually very easy to solve. As part of bla.skel.h: > > .... > > #ifndef __BLA__SKEL_EMBEDDED > #define __BLA__SKEL_EMBEDDED > BPF_EMBED_OBJ(<some_identifier>, <path_to_.o>); > #endif > > extern struct bpf_embed_data <some_identifier>_embed; > > /* we can have a variant of bla__create_skeleton() that just uses > above <some_identifier>_embed */ > > .... > > > That seems to solve all the problems you mentioned. But it creates the > problem of knowing/specifying <some_identifier> and <path_to_.o>. > While we can "dictate" <some_identifier> (e.g., based on object file > name), <path_to_.o> sometimes might need to be overridden, depending > on specifics of build system. > > > But I guess we can follow convention-driven way, and in addition to > above do something like: > > > #ifndef __BLA__SKEL__OBJ_PATH > #define __BLA__SKEL__OBJ_PATH "<whatever path was provided to bpftool > to generate skeleton>" > #endif > > > /* then just use __BLA__SKEL__OBJ_PATH for BPF_EMBED_OBJ, > * which user can override before including skeleton on userspace side > */ > > WDYT? Another idea... How about __weak definition of BPF_EMBED_OBJ ? via generated macro inside .skel.h ? With another method like test_pkt_access__open_and_load() that doesn't take _embed ? Then BPF_EMBED_OBJ_DECLARE() can be removed?
On Sun, Dec 15, 2019 at 8:45 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Sun, Dec 15, 2019 at 06:01:16PM -0800, Andrii Nakryiko wrote: > > On Sun, Dec 15, 2019 at 4:30 PM Alexei Starovoitov > > <alexei.starovoitov@gmail.com> wrote: > > > > > > On Fri, Dec 13, 2019 at 05:43:24PM -0800, Andrii Nakryiko wrote: > > > > This patch set introduces an alternative and complimentary to existing libbpf > > > > API interface for working with BPF objects, maps, programs, and global data > > > > from userspace side. This approach is relying on code generation. bpftool > > > > produces a struct (a.k.a. skeleton) tailored and specific to provided BPF > > > > object file. It includes hard-coded fields and data structures for every map, > > > > program, link, and global data present. > > > > > > > > Altogether this approach significantly reduces amount of userspace boilerplate > > > > code required to open, load, attach, and work with BPF objects. It improves > > > > attach/detach story, by providing pre-allocated space for bpf_links, and > > > > ensuring they are properly detached on shutdown. It allows to do away with by > > > > name/title lookups of maps and programs, because libbpf's skeleton API, in > > > > conjunction with generated code from bpftool, is filling in hard-coded fields > > > > with actual pointers to corresponding struct bpf_map/bpf_program/bpf_link. > > > > > > > > Also, thanks to BPF array mmap() support, working with global data (variables) > > > > from userspace is now as natural as it is from BPF side: each variable is just > > > > a struct field inside skeleton struct. Furthermore, this allows to have > > > > a natural way for userspace to pre-initialize global data (including > > > > previously impossible to initialize .rodata) by just assigning values to the > > > > same per-variable fields. Libbpf will carefully take into account this > > > > initialization image, will use it to pre-populate BPF maps at creation time, > > > > and will re-mmap() BPF map's contents at exactly the same userspace memory > > > > address such that it can continue working with all the same pointers without > > > > any interruptions. If kernel doesn't support mmap(), global data will still be > > > > successfully initialized, but after map creation global data structures inside > > > > skeleton will be NULL-ed out. This allows userspace application to gracefully > > > > handle lack of mmap() support, if necessary. > > > > > > > > A bunch of selftests are also converted to using skeletons, demonstrating > > > > significant simplification of userspace part of test and reduction in amount > > > > of code necessary. > > > > > > > > v3->v4: > > > > - add OPTS_VALID check to btf_dump__emit_type_decl (Alexei); > > > > - expose skeleton as LIBBPF_API functions (Alexei); > > > > - copyright clean up, update internal map init refactor (Alexei); > > > > > > Applied. Thanks. > > > > > > I really liked how much more concise test_fentry_fexit() test has become. > > > I also liked how renaming global variable s/test1_result/_test1_result/ > > > in bpf program became a build time error for user space part: > > > ../prog_tests/fentry_fexit.c:49:35: error: ‘struct fentry_test__bss’ has no member named ‘test1_result’; did you mean ‘_test1_result’? > > > printf("%lld\n", fentry_skel->bss->test1_result); > > > Working with global variables is so much easier now. > > > > > > I'd like you to consider additional feature request. > > > The following error: > > > -BPF_EMBED_OBJ(fentry, "fentry_test.o"); > > > -BPF_EMBED_OBJ(fexit, "fexit_test.o"); > > > +BPF_EMBED_OBJ(fexit, "fentry_test.o"); > > > +BPF_EMBED_OBJ(fentry, "fexit_test.o"); > > > will not be caught. > > > I think skeleton should get smarter somehow to catch that too. > > > > > > One option would be to do BPF_EMBED_OBJ() as part of *.skel.h but then > > > accessing the same embedded .o from multiple tests will not be possible and > > > what stacktrace_build_id.c and stacktrace_build_id_nmi.c are doing won't work > > > anymore. Some sort of build-id/sha1 of .o can work, but it will be caught > > > in run-time. I think build time would be better. > > > May be generate new macro in skel.h that user space can instantiate > > > instead of using common BPF_EMBED_OBJ ? > > > > > > > All those issues are actually very easy to solve. As part of bla.skel.h: > > > > .... > > > > #ifndef __BLA__SKEL_EMBEDDED > > #define __BLA__SKEL_EMBEDDED > > BPF_EMBED_OBJ(<some_identifier>, <path_to_.o>); > > #endif > > > > extern struct bpf_embed_data <some_identifier>_embed; > > > > /* we can have a variant of bla__create_skeleton() that just uses > > above <some_identifier>_embed */ > > > > .... > > > > > > That seems to solve all the problems you mentioned. But it creates the > > problem of knowing/specifying <some_identifier> and <path_to_.o>. > > While we can "dictate" <some_identifier> (e.g., based on object file > > name), <path_to_.o> sometimes might need to be overridden, depending > > on specifics of build system. > > > > > > But I guess we can follow convention-driven way, and in addition to > > above do something like: > > > > > > #ifndef __BLA__SKEL__OBJ_PATH > > #define __BLA__SKEL__OBJ_PATH "<whatever path was provided to bpftool > > to generate skeleton>" > > #endif > > > > > > /* then just use __BLA__SKEL__OBJ_PATH for BPF_EMBED_OBJ, > > * which user can override before including skeleton on userspace side > > */ > > > > WDYT? > > Another idea... > How about __weak definition of BPF_EMBED_OBJ ? > via generated macro inside .skel.h ? > With another method like test_pkt_access__open_and_load() that > doesn't take _embed ? > Then BPF_EMBED_OBJ_DECLARE() can be removed? > So I like the idea of not having to do BPF_EMBED_OBJ. It can't be done cleanly with BPF_EMBED_OBJ, but skeleton can auto-generate array with embedded contents of object file used for skeleton generation. That will solve all the issues of keeping skeleton and object file contents in sync. I'll post a follow up patch with that change.
This patch set introduces an alternative and complimentary to existing libbpf API interface for working with BPF objects, maps, programs, and global data from userspace side. This approach is relying on code generation. bpftool produces a struct (a.k.a. skeleton) tailored and specific to provided BPF object file. It includes hard-coded fields and data structures for every map, program, link, and global data present. Altogether this approach significantly reduces amount of userspace boilerplate code required to open, load, attach, and work with BPF objects. It improves attach/detach story, by providing pre-allocated space for bpf_links, and ensuring they are properly detached on shutdown. It allows to do away with by name/title lookups of maps and programs, because libbpf's skeleton API, in conjunction with generated code from bpftool, is filling in hard-coded fields with actual pointers to corresponding struct bpf_map/bpf_program/bpf_link. Also, thanks to BPF array mmap() support, working with global data (variables) from userspace is now as natural as it is from BPF side: each variable is just a struct field inside skeleton struct. Furthermore, this allows to have a natural way for userspace to pre-initialize global data (including previously impossible to initialize .rodata) by just assigning values to the same per-variable fields. Libbpf will carefully take into account this initialization image, will use it to pre-populate BPF maps at creation time, and will re-mmap() BPF map's contents at exactly the same userspace memory address such that it can continue working with all the same pointers without any interruptions. If kernel doesn't support mmap(), global data will still be successfully initialized, but after map creation global data structures inside skeleton will be NULL-ed out. This allows userspace application to gracefully handle lack of mmap() support, if necessary. A bunch of selftests are also converted to using skeletons, demonstrating significant simplification of userspace part of test and reduction in amount of code necessary. v3->v4: - add OPTS_VALID check to btf_dump__emit_type_decl (Alexei); - expose skeleton as LIBBPF_API functions (Alexei); - copyright clean up, update internal map init refactor (Alexei); v2->v3: - make skeleton part of public API; - expose btf_dump__emit_type_decl and btf__align_of APIs; - move LIBBPF_API and DECLARE_LIBBPF_OPTS into libbpf_common.h for reuse; v1->v2: - checkpatch.pl and reverse Christmas tree styling (Jakub); - sanitize variable names to accomodate in-function static vars; rfc->v1: - runqslower moved out into separate patch set waiting for vmlinux.h improvements; - skeleton generation code deals with unknown internal maps more gracefully. Andrii Nakryiko (17): libbpf: don't require root for bpf_object__open() libbpf: add generic bpf_program__attach() libbpf: move non-public APIs from libbpf.h to libbpf_internal.h libbpf: add BPF_EMBED_OBJ macro for embedding BPF .o files libbpf: extract common user-facing helpers libbpf: expose btf__align_of() API libbpf: expose BTF-to-C type declaration emitting API libbpf: expose BPF program's function name libbpf: refactor global data map initialization libbpf: postpone BTF ID finding for TRACING programs to load phase libbpf: reduce log level of supported section names dump libbpf: add BPF object skeleton support bpftool: add skeleton codegen command selftests/bpf: add BPF skeletons selftests and convert attach_probe.c selftests/bpf: convert few more selftest to skeletons selftests/bpf: add test validating data section to struct convertion layout bpftool: add `gen skeleton` BASH completions tools/bpf/bpftool/bash-completion/bpftool | 11 + tools/bpf/bpftool/gen.c | 551 ++++++++++++++++ tools/bpf/bpftool/main.c | 3 +- tools/bpf/bpftool/main.h | 1 + tools/bpf/bpftool/net.c | 1 + tools/lib/bpf/bpf.h | 6 +- tools/lib/bpf/btf.c | 39 ++ tools/lib/bpf/btf.h | 29 +- tools/lib/bpf/btf_dump.c | 115 ++-- tools/lib/bpf/libbpf.c | 588 ++++++++++++++---- tools/lib/bpf/libbpf.h | 129 ++-- tools/lib/bpf/libbpf.map | 11 + tools/lib/bpf/libbpf_common.h | 38 ++ tools/lib/bpf/libbpf_internal.h | 17 + tools/testing/selftests/bpf/.gitignore | 2 + tools/testing/selftests/bpf/Makefile | 36 +- .../selftests/bpf/prog_tests/attach_probe.c | 154 +---- .../selftests/bpf/prog_tests/fentry_fexit.c | 105 ++-- .../selftests/bpf/prog_tests/fentry_test.c | 72 +-- tools/testing/selftests/bpf/prog_tests/mmap.c | 58 +- .../selftests/bpf/prog_tests/probe_user.c | 6 +- .../selftests/bpf/prog_tests/rdonly_maps.c | 11 +- .../selftests/bpf/prog_tests/skeleton.c | 51 ++ .../bpf/prog_tests/stacktrace_build_id.c | 79 +-- .../bpf/prog_tests/stacktrace_build_id_nmi.c | 84 +-- .../selftests/bpf/progs/test_attach_probe.c | 34 +- .../selftests/bpf/progs/test_skeleton.c | 37 ++ 27 files changed, 1598 insertions(+), 670 deletions(-) create mode 100644 tools/bpf/bpftool/gen.c create mode 100644 tools/lib/bpf/libbpf_common.h create mode 100644 tools/testing/selftests/bpf/prog_tests/skeleton.c create mode 100644 tools/testing/selftests/bpf/progs/test_skeleton.c