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
The "install_headers" target in libbpf's Makefile would unconditionally
export all API headers to the target directory. When those headers are
installed to compile another application, this means that make always
finds newer dependencies for the source files relying on those headers,
and deduces that the targets should be rebuilt.
Avoid that by making "install_headers" depend on the source header
files, and (re-)install them only when necessary.
Signed-off-by: Quentin Monnet <redacted>
---
tools/lib/bpf/Makefile | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
It seems that the header file was never necessary to compile bpftool,
and it is not part of the headers exported from libbpf. Let's remove the
includes from prog.c and gen.c.
Fixes: d510296d331a ("bpftool: Use syscall/loader program in "prog load" and "gen skeleton" command.")
Signed-off-by: Quentin Monnet <redacted>
---
tools/bpf/bpftool/gen.c | 1 -
tools/bpf/bpftool/prog.c | 1 -
2 files changed, 2 deletions(-)
Bpftool relies on libbpf, therefore it relies on a number of headers
from the library and must be linked against the library. The Makefile
for bpftool exposes these objects by adding tools/lib as an include
directory ("-I$(srctree)/tools/lib"). This is a working solution, but
this is not the cleanest one. The risk is to involuntarily include
objects that are not intended to be exposed by the libbpf.
The headers needed to compile bpftool should in fact be "installed" from
libbpf, with its "install_headers" Makefile target. In addition, there
is one header which is internal to the library and not supposed to be
used by external applications, but that bpftool uses anyway.
Adjust the Makefile in order to install the header files properly before
compiling bpftool. Also copy the additional internal header file
(nlattr.h), but call it out explicitly. Build (and install headers) in a
subdirectory under bpftool/ instead of tools/lib/bpf/. When descending
from a parent Makefile, this is configurable by setting the OUTPUT,
LIBBPF_OUTPUT and LIBBPF_DESTDIR variables.
Also adjust the Makefile for BPF selftests, so as to reuse the (host)
libbpf compiled earlier and to avoid compiling a separate version of the
library just for bpftool.
Signed-off-by: Quentin Monnet <redacted>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/bpf/bpftool/Makefile | 33 ++++++++++++++++++----------
tools/testing/selftests/bpf/Makefile | 2 ++
2 files changed, 23 insertions(+), 12 deletions(-)
@@ -17,19 +17,23 @@ endifBPF_DIR=$(srctree)/tools/lib/bpf/ifneq ($(OUTPUT),)-LIBBPF_OUTPUT=$(OUTPUT)/libbpf/-LIBBPF_PATH=$(LIBBPF_OUTPUT)-BOOTSTRAP_OUTPUT=$(OUTPUT)/bootstrap/+_OUTPUT:=$(OUTPUT)else-LIBBPF_OUTPUT=-LIBBPF_PATH=$(BPF_DIR)-BOOTSTRAP_OUTPUT=$(CURDIR)/bootstrap/+_OUTPUT:=$(CURDIR)endif+BOOTSTRAP_OUTPUT:=$(_OUTPUT)/bootstrap/+LIBBPF_OUTPUT:=$(_OUTPUT)/libbpf/+LIBBPF_DESTDIR:=$(LIBBPF_OUTPUT)+LIBBPF_INCLUDE:=$(LIBBPF_DESTDIR)/include-LIBBPF=$(LIBBPF_PATH)libbpf.a+LIBBPF=$(LIBBPF_OUTPUT)libbpf.aLIBBPF_BOOTSTRAP_OUTPUT=$(BOOTSTRAP_OUTPUT)libbpf/LIBBPF_BOOTSTRAP=$(LIBBPF_BOOTSTRAP_OUTPUT)libbpf.a+# We need to copy nlattr.h which is not otherwise exported by libbpf, but still+# required by bpftool.+LIBBPF_INTERNAL_HDRS:=nlattr.h+ifeq ($(BPFTOOL_VERSION),)BPFTOOL_VERSION:=$(shellmake-rR--no-print-directory-sC../../..kernelversion)endif
API headers from libbpf should not be accessed directly from the
library's source directory. Instead, they should be exported with "make
install_headers". Let's make sure that runqslower installs the
headers properly when building.
We use a libbpf_hdrs target to mark the logical dependency on libbpf's
headers export for a number of object files, even though the headers
should have been exported at this time (since bpftool needs them, and is
required to generate the skeleton or the vmlinux.h).
When descending from a parent Makefile, the specific output directories
for building the library and exporting the headers are configurable with
BPFOBJ_OUTPUT and BPF_DESTDIR, respectively. This is in addition to
OUTPUT, on top of which those variables are constructed by default.
Also adjust the Makefile for the BPF selftests. We pass a number of
variables to the "make" invocation, because we want to point runqslower
to the (target) libbpf shared with other tools, instead of building its
own version. In addition, runqslower relies on (target) bpftool, and we
also want to pass the proper variables to its Makefile so that bpftool
itself reuses the same libbpf.
Signed-off-by: Quentin Monnet <redacted>
---
tools/bpf/runqslower/Makefile | 22 +++++++++++++---------
tools/testing/selftests/bpf/Makefile | 15 +++++++++------
2 files changed, 22 insertions(+), 15 deletions(-)
API headers from libbpf should not be accessed directly from the
library's source directory. Instead, they should be exported with "make
install_headers". Let's make sure that resolve_btfids installs the
headers properly when building.
When descending from a parent Makefile, the specific output directories
for building the library and exporting the headers are configurable with
LIBBPF_OUT and LIBBPF_DESTDIR, respectively. This is in addition to
OUTPUT, on top of which those variables are constructed by default.
Also adjust the Makefile for the BPF selftests in order to point to the
(target) libbpf shared with other tools, instead of building a version
specific to resolve_btfids. Remove libbpf's order-only dependencies on
the include directories (they are created by libbpf and don't need to
exist beforehand).
Signed-off-by: Quentin Monnet <redacted>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/bpf/resolve_btfids/Makefile | 17 ++++++++++++-----
tools/bpf/resolve_btfids/main.c | 4 ++--
tools/testing/selftests/bpf/Makefile | 7 +++++--
3 files changed, 19 insertions(+), 9 deletions(-)
@@ -260,6 +262,7 @@ $(RESOLVE_BTFIDS): $(HOST_BPFOBJ) | $(HOST_BUILD_DIR)/resolve_btfids \$(TOOLSDIR)/lib/str_error_r.c$(Q)$(MAKE)$(submake_extras)-C$(TOOLSDIR)/bpf/resolve_btfids\CC=$(HOSTCC)LD=$(HOSTLD)AR=$(HOSTAR)\+LIBBPF_INCLUDE=$(HOST_INCLUDE_DIR)\OUTPUT=$(HOST_BUILD_DIR)/resolve_btfids/BPFOBJ=$(HOST_BPFOBJ)# Get Clang's default includes on this system, as opposed to those seen by
API headers from libbpf should not be accessed directly from the
library's source directory. Instead, they should be exported with "make
install_headers". Let's make sure that bpf/preload/Makefile installs the
headers properly when building.
Note that we declare an additional dependency for iterators/iterators.o:
having $(LIBBPF_A) as a dependency to "$(obj)/bpf_preload_umd" is not
sufficient, as it makes it required only at the linking step. But we
need libbpf to be compiled, and in particular its headers to be
exported, before we attempt to compile iterators.o. The issue would not
occur before this commit, because libbpf's headers were not exported and
were always available under tools/lib/bpf.
Signed-off-by: Quentin Monnet <redacted>
---
kernel/bpf/preload/Makefile | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
@@ -1,21 +1,36 @@# SPDX-License-Identifier: GPL-2.0LIBBPF_SRCS=$(srctree)/tools/lib/bpf/-LIBBPF_A=$(obj)/libbpf.a-LIBBPF_OUT=$(abspath$(obj))+LIBBPF_OUT=$(abspath$(obj))/libbpf+LIBBPF_A=$(LIBBPF_OUT)/libbpf.a+LIBBPF_DESTDIR=$(LIBBPF_OUT)+LIBBPF_INCLUDE=$(LIBBPF_DESTDIR)/include# Although not in use by libbpf's Makefile, set $(O) so that the "dummy" test# in tools/scripts/Makefile.include always succeeds when building the kernel# with $(O) pointing to a relative path, as in "make O=build bindeb-pkg".-$(LIBBPF_A):-$(Q)$(MAKE)-C$(LIBBPF_SRCS)O=$(LIBBPF_OUT)/OUTPUT=$(LIBBPF_OUT)/$(LIBBPF_OUT)/libbpf.a+$(LIBBPF_A):|$(LIBBPF_OUT)+$(Q)$(MAKE)-C$(LIBBPF_SRCS)O=$(LIBBPF_OUT)/OUTPUT=$(LIBBPF_OUT)/\+DESTDIR=$(LIBBPF_DESTDIR)prefix=\+$(LIBBPF_OUT)/libbpf.ainstall_headers++libbpf_hdrs:$(LIBBPF_A)++.PHONY:libbpf_hdrs++$(LIBBPF_OUT):+$(callmsg,MKDIR,$@)+$(Q)mkdir-p$@userccflags+=-I$(srctree)/tools/include/-I$(srctree)/tools/include/uapi\--I$(srctree)/tools/lib/-Wno-unused-result+-I$(LIBBPF_INCLUDE)-Wno-unused-resultuserprogs:=bpf_preload_umdclean-files:=$(userprogs)bpf_helper_defs.hFEATURE-DUMP.libbpfstaticobjs/feature/+clean-files+=$(LIBBPF_OUT)$(LIBBPF_DESTDIR)++$(obj)/iterators/iterators.o:|libbpf_hdrsbpf_preload_umd-objs:=iterators/iterators.obpf_preload_umd-userldlibs:=$(LIBBPF_A)-lelf-lz
API headers from libbpf should not be accessed directly from the
library's source directory. Instead, they should be exported with "make
install_headers". Let's make sure that bpf/preload/iterators/Makefile
installs the headers properly when building.
Signed-off-by: Quentin Monnet <redacted>
---
kernel/bpf/preload/iterators/Makefile | 39 ++++++++++++++++++---------
1 file changed, 26 insertions(+), 13 deletions(-)
Update samples/bpf/.gitignore to ignore files generated when building
the samples. Add:
- vmlinux.h
- the generated skeleton files (*.skel.h)
- the samples/bpf/libbpf/ and .../bpftool/ directories, in preparation
of a future commit which introduces a local output directory for
building libbpf and bpftool.
Signed-off-by: Quentin Monnet <redacted>
---
samples/bpf/.gitignore | 4 ++++
1 file changed, 4 insertions(+)
API headers from libbpf should not be accessed directly from the source
directory. Instead, they should be exported with "make install_headers".
Make sure that samples/bpf/Makefile installs the headers properly when
building.
The object compiled from and exported by libbpf are now placed into a
subdirectory of sample/bpf/ instead of remaining in tools/lib/bpf/. We
attempt to remove this directory on "make clean". However, the "clean"
target re-enters the samples/bpf/ directory from the root of the
repository ("$(MAKE) -C ../../ M=$(CURDIR) clean"), in such a way that
$(srctree) and $(src) are not defined, making it impossible to use
$(LIBBPF_OUTPUT) and $(LIBBPF_DESTDIR) in the recipe. So we only attempt
to clean $(CURDIR)/libbpf, which is the default value.
Add a dependency on libbpf's headers for the $(TRACE_HELPERS).
We also change the output directory for bpftool, to place the generated
objects under samples/bpf/bpftool/ instead of building in bpftool's
directory directly. Doing so, we make sure bpftool reuses the libbpf
library previously compiled and installed.
Signed-off-by: Quentin Monnet <redacted>
---
samples/bpf/Makefile | 41 +++++++++++++++++++++++++++++++----------
1 file changed, 31 insertions(+), 10 deletions(-)
In samples/bpf/Makefile, libbpf has a FORCE dependency that force it to
be rebuilt. I read this as a way to keep the library up-to-date, given
that we do not have, in samples/bpf, a list of the source files for
libbpf itself. However, a better approach would be to use the
"$(wildcard ...)" function from make, and to have libbpf depend on all
the .c and .h files in its directory. This is what samples/bpf/Makefile
does for bpftool, and also what the BPF selftests' Makefile does for
libbpf.
Let's update the Makefile to avoid rebuilding libbpf all the time (and
bpftool on top of it).
Signed-off-by: Quentin Monnet <redacted>
---
samples/bpf/Makefile | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
@@ -276,7 +276,8 @@ clean:@find$(CURDIR)-typef-name'*~'-delete@$(RM)-r$(CURDIR)/libbpf$(CURDIR)/bpftool-$(LIBBPF):FORCE|$(LIBBPF_OUTPUT)+$(LIBBPF):$(wildcard$(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) \+|$(LIBBPF_OUTPUT)# Fix up variables inherited from Kbuild that tools/ build system won't like$(MAKE)-C$(LIBBPF_SRC)RM='rm -rf'EXTRA_CFLAGS="$(TPROGS_CFLAGS)"\LDFLAGS=$(TPROGS_LDFLAGS)srctree=$(BPF_SAMPLES_PATH)/../../\
The script test_bpftool_build.sh attempts to build bpftool in the
various supported ways, to make sure nothing breaks.
One of those ways is to run "make tools/bpf" from the root of the kernel
repository. This command builds bpftool, along with the other tools
under tools/bpf, and runqslower in particular. After running the
command and upon a successful bpftool build, the script attempts to
cleanup the generated objects. However, after building with this target
and in the case of runqslower, the files are not cleaned up as expected.
This is because the "tools/bpf" target sets $(OUTPUT) to
.../tools/bpf/runqslower/ when building the tool, causing the object
files to be placed directly under the runqslower directory. But when
running "cd tools/bpf; make clean", the value for $(OUTPUT) is set to
".output" (relative to the runqslower directory) by runqslower's
Makefile, and this is where the Makefile looks for files to clean up.
We cannot easily fix in the root Makefile (where "tools/bpf" is defined)
or in tools/scripts/Makefile.include (setting $(OUTPUT)), where changing
the way the output variables are passed would likely have consequences
elsewhere. We could change runqslower's Makefile to build in the
repository instead of in a dedicated ".output/", but doing so just to
accommodate a test script doesn't sound great. Instead, let's just make
sure that we clean up runqslower properly by adding the correct command
to the script.
This will attempt to clean runqslower twice: the first try with command
"cd tools/bpf; make clean" will search for tools/bpf/runqslower/.output
and fail to clean it (but will still clean the other tools, in
particular bpftool), the second one (added in this commit) sets the
$(OUTPUT) variable like for building with the "tool/bpf" target and
should succeed.
Signed-off-by: Quentin Monnet <redacted>
---
tools/testing/selftests/bpf/test_bpftool_build.sh | 4 ++++
1 file changed, 4 insertions(+)
@@ -90,6 +90,10 @@ echo -e "... through kbuild\n"if[-f".config"];thenmake_and_cleantools/bpf+## "make tools/bpf" sets $(OUTPUT) to ...tools/bpf/runqslower for+## runqslower, but the default (used for the "clean" target) is .output.+## Let's make sure we clean runqslower's directory properly.+make-Ctools/bpf/runqslowerOUTPUT=${KDIR_ROOT_DIR}/tools/bpf/runqslower/clean## $OUTPUT is overwritten in kbuild Makefile, and thus cannot be passed## down from toplevel Makefile to bpftool's Makefile.
With "make install", bpftool installs its binary and its bash completion
file. Usually, this is what we want. But a few components in the kernel
repository (namely, BPF iterators and selftests) also install bpftool
locally before using it. In such a case, bash completion is not
necessary and is just a useless build artifact.
Let's add an "install-bin" target to bpftool, to offer a way to install
the binary only.
Signed-off-by: Quentin Monnet <redacted>
---
kernel/bpf/preload/iterators/Makefile | 2 +-
tools/bpf/bpftool/Makefile | 6 ++++--
tools/testing/selftests/bpf/Makefile | 2 +-
3 files changed, 6 insertions(+), 4 deletions(-)
On Thu, Oct 7, 2021 at 12:44 PM Quentin Monnet [off-list ref] wrote:
quoted hunk
The "install_headers" target in libbpf's Makefile would unconditionally
export all API headers to the target directory. When those headers are
installed to compile another application, this means that make always
finds newer dependencies for the source files relying on those headers,
and deduces that the targets should be rebuilt.
Avoid that by making "install_headers" depend on the source header
files, and (re-)install them only when necessary.
Signed-off-by: Quentin Monnet <redacted>
---
tools/lib/bpf/Makefile | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
I felt like INSTALL_HEADERS just adds one more indirection and
otherwise is not useful, so I dropped it and inlined
$(INSTALL_SRC_HDRS) $(INSTALL_GEN_HDRS) below
On Thu, Oct 7, 2021 at 12:44 PM Quentin Monnet [off-list ref] wrote:
quoted hunk
Bpftool relies on libbpf, therefore it relies on a number of headers
from the library and must be linked against the library. The Makefile
for bpftool exposes these objects by adding tools/lib as an include
directory ("-I$(srctree)/tools/lib"). This is a working solution, but
this is not the cleanest one. The risk is to involuntarily include
objects that are not intended to be exposed by the libbpf.
The headers needed to compile bpftool should in fact be "installed" from
libbpf, with its "install_headers" Makefile target. In addition, there
is one header which is internal to the library and not supposed to be
used by external applications, but that bpftool uses anyway.
Adjust the Makefile in order to install the header files properly before
compiling bpftool. Also copy the additional internal header file
(nlattr.h), but call it out explicitly. Build (and install headers) in a
subdirectory under bpftool/ instead of tools/lib/bpf/. When descending
from a parent Makefile, this is configurable by setting the OUTPUT,
LIBBPF_OUTPUT and LIBBPF_DESTDIR variables.
Also adjust the Makefile for BPF selftests, so as to reuse the (host)
libbpf compiled earlier and to avoid compiling a separate version of the
library just for bpftool.
Signed-off-by: Quentin Monnet <redacted>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/bpf/bpftool/Makefile | 33 ++++++++++++++++++----------
tools/testing/selftests/bpf/Makefile | 2 ++
2 files changed, 23 insertions(+), 12 deletions(-)
@@ -17,19 +17,23 @@ endifBPF_DIR=$(srctree)/tools/lib/bpf/ifneq ($(OUTPUT),)-LIBBPF_OUTPUT=$(OUTPUT)/libbpf/-LIBBPF_PATH=$(LIBBPF_OUTPUT)-BOOTSTRAP_OUTPUT=$(OUTPUT)/bootstrap/+_OUTPUT:=$(OUTPUT)else-LIBBPF_OUTPUT=-LIBBPF_PATH=$(BPF_DIR)-BOOTSTRAP_OUTPUT=$(CURDIR)/bootstrap/+_OUTPUT:=$(CURDIR)endif+BOOTSTRAP_OUTPUT:=$(_OUTPUT)/bootstrap/+LIBBPF_OUTPUT:=$(_OUTPUT)/libbpf/+LIBBPF_DESTDIR:=$(LIBBPF_OUTPUT)+LIBBPF_INCLUDE:=$(LIBBPF_DESTDIR)/include-LIBBPF=$(LIBBPF_PATH)libbpf.a+LIBBPF=$(LIBBPF_OUTPUT)libbpf.aLIBBPF_BOOTSTRAP_OUTPUT=$(BOOTSTRAP_OUTPUT)libbpf/LIBBPF_BOOTSTRAP=$(LIBBPF_BOOTSTRAP_OUTPUT)libbpf.a+# We need to copy nlattr.h which is not otherwise exported by libbpf, but still+# required by bpftool.+LIBBPF_INTERNAL_HDRS:=nlattr.h+ifeq ($(BPFTOOL_VERSION),)BPFTOOL_VERSION:=$(shellmake-rR--no-print-directory-sC../../..kernelversion)endif
This worked only because LIBBPF_INTERNAL_HDRS is a single element list
right now. I didn't touch it for now, but please follow up with a
proper fix (you'd need to do % magic here)
On Thu, Oct 7, 2021 at 12:44 PM Quentin Monnet [off-list ref] 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!
On Fri, 8 Oct 2021 at 20:13, Andrii Nakryiko [off-list ref] wrote:
Tons of ungrateful work, thank you! Applied to bpf-next.
I did a few clean ups (from my POV), see comments on relevant patches.
Thanks for that. I don't mind the clean ups. There are several of them
I considered before sending but wasn't sure about, so it's a good
thing that you did it :).
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.
No particular preference on my side, so OK.
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.
Right, good catch. I'm sending the 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.
I had a look for bpftool, this is because we always descend into
libbpf's directory (FORCE target). Removing this FORCE target as I did
in samples/bpf/ avoids the descent and clears the output. I'll send a
patch.
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.
I can look into that. Are you sure you want the warnings removed? Or
would it be cleaner to simply update the headers?
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!
I haven't looked into it yet, but I can do as a follow-up. I'll post
the patches for bpftool first because I prefer to submit the fix for
bpftool's Makefile as soon as possible, and will look at this next.
Thanks,
Quentin
On Sat, Oct 9, 2021 at 11:03 PM Quentin Monnet [off-list ref] wrote:
On Fri, 8 Oct 2021 at 20:13, Andrii Nakryiko [off-list ref] wrote:
quoted
Tons of ungrateful work, thank you! Applied to bpf-next.
I did a few clean ups (from my POV), see comments on relevant patches.
Thanks for that. I don't mind the clean ups. There are several of them
I considered before sending but wasn't sure about, so it's a good
thing that you did it :).
quoted
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.
No particular preference on my side, so OK.
quoted
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.
Right, good catch. I'm sending the fix.
thanks
quoted
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.
I had a look for bpftool, this is because we always descend into
libbpf's directory (FORCE target). Removing this FORCE target as I did
in samples/bpf/ avoids the descent and clears the output. I'll send a
patch.
There is a way to prevent make from logging these enter/leave
messages, which will solve a similar problem discussed below.
quoted
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.
I can look into that. Are you sure you want the warnings removed? Or
would it be cleaner to simply update the headers?
Yeah, let's remove checks for those headers. We don't need to keep
them up to date, if there will be new features we need to use from
libbpf, we can update, if it's not already up-to-date.
quoted
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!
I haven't looked into it yet, but I can do as a follow-up. I'll post
the patches for bpftool first because I prefer to submit the fix for
bpftool's Makefile as soon as possible, and will look at this next.
sure. See also above about just silencing these somewhat useless
messages (it's some make variable or something like that, don't
remember)