On Thu, Oct 7, 2021 at 12:44 PM Quentin Monnet <quentin@xxxxxxxxxxxxx> wrote: > > Libbpf is used at several locations in the repository. Most of the time, > the tools relying on it build the library in its own directory, and include > the headers from there. This works, but this is not the cleanest approach. > It generates objects outside of the directory of the tool which is being > built, and it also increases the risk that developers include a header file > internal to libbpf, which is not supposed to be exposed to user > applications. > > This set adjusts all involved Makefiles to make sure that libbpf is built > locally (with respect to the tool's directory or provided build directory), > and by ensuring that "make install_headers" is run from libbpf's Makefile > to export user headers properly. > > This comes at a cost: given that the libbpf was so far mostly compiled in > its own directory by the different components using it, compiling it once > would be enough for all those components. With the new approach, each > component compiles its own version. To mitigate this cost, efforts were > made to reuse the compiled library when possible: > > - Make the bpftool version in samples/bpf reuse the library previously > compiled for the selftests. > - Make the bpftool version in BPF selftests reuse the library previously > compiled for the selftests. > - Similarly, make resolve_btfids in BPF selftests reuse the same compiled > library. > - Similarly, make runqslower in BPF selftests reuse the same compiled > library; and make it rely on the bpftool version also compiled from the > selftests (instead of compiling its own version). > - runqslower, when compiled independently, needs its own version of > bpftool: make them share the same compiled libbpf. > > As a result: > > - Compiling the samples/bpf should compile libbpf just once. > - Compiling the BPF selftests should compile libbpf just once. > - Compiling the kernel (with BTF support) should now lead to compiling > libbpf twice: one for resolve_btfids, one for kernel/bpf/preload. > - Compiling runqslower individually should compile libbpf just once. Same > thing for bpftool, resolve_btfids, and kernel/bpf/preload/iterators. > > (Not accounting for the boostrap version of libbpf required by bpftool, > which was already placed under a dedicated .../boostrap/libbpf/ directory, > and for which the count remains unchanged.) > > A few commits in the series also contain drive-by clean-up changes for > bpftool includes, samples/bpf/.gitignore, or test_bpftool_build.sh. Please > refer to individual commit logs for details. > > v4: > - Make the "libbpf_hdrs" dependency an order-only dependency in > kernel/bpf/preload/Makefile, samples/bpf/Makefile, and > tools/bpf/runqslower/Makefile. This is to avoid to unconditionally > recompile the targets. > - samples/bpf/.gitignore: prefix objects with a "/" to mark that we > ignore them when at the root of the samples/bpf/ directory. > - libbpf: add a commit to make "install_headers" depend on the header > files, to avoid exporting again if the sources are older than the > targets. This ensures that applications relying on those headers are > not rebuilt unnecessarily. > - bpftool: uncouple the copy of nlattr.h from libbpf target, to have it > depend on the source header itself. By avoiding to reinstall this > header every time, we avoid unnecessary builds of bpftool. > - samples/bpf: Add a new commit to remove the FORCE dependency for > libbpf, and replace it with a "$(wildcard ...)" on the .c/.h files in > libbpf's directory. This is to avoid always recompiling libbpf/bpftool. > - Adjust prefixes in commit subjects. > > v3: > - Remove order-only dependencies on $(LIBBPF_INCLUDE) (or equivalent) > directories, given that they are created by libbpf's Makefile. > - Add libbpf as a dependency for bpftool/resolve_btfids/runqslower when > they are supposed to reuse a libbpf compiled previously. This is to > avoid having several libbpf versions being compiled simultaneously in > the same directory with parallel builds. Even if this didn't show up > during tests, let's remain on the safe side. > - kernel/bpf/preload/Makefile: Rename libbpf-hdrs (dash) dependency as > libbpf_hdrs. > - samples/bpf/.gitignore: Add bpftool/ > - samples/bpf/Makefile: Change "/bin/rm -rf" to "$(RM) -r". > - samples/bpf/Makefile: Add missing slashes for $(LIBBPF_OUTPUT) and > $(LIBBPF_DESTDIR) when buildling bpftool > - samples/bpf/Makefile: Add a dependency to libbpf's headers for > $(TRACE_HELPERS). > - bpftool's Makefile: Use $(LIBBPF) instead of equivalent (but longer) > $(LIBBPF_OUTPUT)libbpf.a > - BPF iterators' Makefile: build bpftool in .output/bpftool (instead of > .output/), add and clean up variables. > - runqslower's Makefile: Add an explicit dependency on libbpf's headers > to several objects. The dependency is not required (libbpf should have > been compiled and so the headers exported through other dependencies > for those targets), but they better mark the logical dependency and > should help if exporting the headers changed in the future. > - New commit to add an "install-bin" target to bpftool, to avoid > installing bash completion when buildling BPF iterators and selftests. > > v2: Declare an additional dependency on libbpf's headers for > iterators/iterators.o in kernel/preload/Makefile to make sure that > these headers are exported before we compile the object file (and not > just before we link it). > > Quentin Monnet (12): > libbpf: skip re-installing headers file if source is older than target > bpftool: remove unused includes to <bpf/bpf_gen_internal.h> > bpftool: install libbpf headers instead of including the dir > tools/resolve_btfids: install libbpf headers when building > tools/runqslower: install libbpf headers when building > bpf: preload: install libbpf headers when building > bpf: iterators: install libbpf headers when building > samples/bpf: update .gitignore > samples/bpf: install libbpf headers when building > samples/bpf: do not FORCE-recompile libbpf > selftests/bpf: better clean up for runqslower in test_bpftool_build.sh > bpftool: add install-bin target to install binary only > > kernel/bpf/preload/Makefile | 25 ++++++++--- > kernel/bpf/preload/iterators/Makefile | 39 +++++++++++------ > samples/bpf/.gitignore | 4 ++ > samples/bpf/Makefile | 42 ++++++++++++++----- > tools/bpf/bpftool/Makefile | 39 ++++++++++------- > tools/bpf/bpftool/gen.c | 1 - > tools/bpf/bpftool/prog.c | 1 - > tools/bpf/resolve_btfids/Makefile | 17 +++++--- > tools/bpf/resolve_btfids/main.c | 4 +- > tools/bpf/runqslower/Makefile | 22 ++++++---- > tools/lib/bpf/Makefile | 24 +++++++---- > tools/testing/selftests/bpf/Makefile | 26 ++++++++---- > .../selftests/bpf/test_bpftool_build.sh | 4 ++ > 13 files changed, 171 insertions(+), 77 deletions(-) > > -- > 2.30.2 > Tons of ungrateful work, thank you! Applied to bpf-next. I did a few clean ups (from my POV), see comments on relevant patches. Also in a bunch of Makefiles I've moved `| $(LIBBPF_OUTPUT)` to the same line if the line wasn't overly long. 80 characters is not a law, and I preferred single-line Makefile target definitions, if possible. There is one problem in bpftool's Makefile, but it works with a limited case of single file today. Please follow up with a proper fix. Btw, running make in bpftool's directory, I'm getting: make[1]: Entering directory '/data/users/andriin/linux/tools/lib/bpf' make[1]: Entering directory '/data/users/andriin/linux/tools/lib/bpf' make[1]: Nothing to be done for 'install_headers'. make[1]: Leaving directory '/data/users/andriin/linux/tools/lib/bpf' make[1]: Leaving directory '/data/users/andriin/linux/tools/lib/bpf' Not sure how useful those are, might be better to disable that. When running libbpf's make, we constantly getting this annoying warning: Warning: Kernel ABI header at 'tools/include/uapi/linux/netlink.h' differs from latest version at 'include/uapi/linux/netlink.h' Warning: Kernel ABI header at 'tools/include/uapi/linux/if_link.h' differs from latest version at 'include/uapi/linux/if_link.h' If you will get a chance, maybe you can get rid of that as well? I don't think we need to stay up to date with netlink.h and if_link.h, so this seems like just a noise. There was also make[4]: Nothing to be done for 'install_headers'. when building the kernel. It probably is coming from either bpf_preload or iterators, but maybe also resolve_btfids, I didn't try to narrow this down. Also seems like a noise, tbh. There are similar useless notifications when building selftests/bpf. If it doesn't take too much time to clean all that up, I'd greatly appreciate that! But really great work, thanks for sticking with it!
