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.
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 (10):
tools: bpftool: remove unused includes to <bpf/bpf_gen_internal.h>
tools: 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: install libbpf headers when building
samples/bpf: update .gitignore
selftests/bpf: better clean up for runqslower in test_bpftool_build.sh
tools: 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 | 41 ++++++++++++++-----
tools/bpf/bpftool/Makefile | 32 +++++++++------
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/testing/selftests/bpf/Makefile | 26 ++++++++----
.../selftests/bpf/test_bpftool_build.sh | 4 ++
12 files changed, 148 insertions(+), 68 deletions(-)
--
2.30.2
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(-)
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
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 | 26 +++++++++++++++-----------
tools/testing/selftests/bpf/Makefile | 2 ++
2 files changed, 17 insertions(+), 11 deletions(-)
@@ -37,8 +37,13 @@ endif$(LIBBPF_OUTPUT) $(BOOTSTRAP_OUTPUT) $(LIBBPF_BOOTSTRAP_OUTPUT):$(QUIET_MKDIR)mkdir-p$@+# We need to copy nlattr.h which is not otherwise exported by libbpf, but still+# required by bpftool.$(LIBBPF):FORCE|$(LIBBPF_OUTPUT)-$(Q)$(MAKE)-C$(BPF_DIR)OUTPUT=$(LIBBPF_OUTPUT)$(LIBBPF_OUTPUT)libbpf.a+$(Q)$(MAKE)-C$(BPF_DIR)OUTPUT=$(LIBBPF_OUTPUT)\+DESTDIR=$(LIBBPF_DESTDIR)prefix=$(LIBBPF)install_headers+$(callQUIET_INSTALL,bpf/nlattr.h)+$(Q)install-m644-t$(LIBBPF_INCLUDE)/bpf/$(BPF_DIR)nlattr.h$(LIBBPF_BOOTSTRAP):FORCE|$(LIBBPF_BOOTSTRAP_OUTPUT)$(Q)$(MAKE)-C$(BPF_DIR)OUTPUT=$(LIBBPF_BOOTSTRAP_OUTPUT)\
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 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, recently
introduced as an output directory for building libbpf and bpftool.
Signed-off-by: Quentin Monnet <redacted>
---
samples/bpf/.gitignore | 4 ++++
1 file changed, 4 insertions(+)
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(-)
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(-)
On Sun, Oct 3, 2021 at 12:22 PM Quentin Monnet [off-list ref] wrote:
quoted hunk
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(-)
this phony dependency will cause runqslower.o to be always rebuilt,
try running make multiple times with no changes inside
tools/bpf/runqslower. Can this be done just as an order-only
dependency?
On Sun, Oct 3, 2021 at 12:22 PM Quentin Monnet [off-list ref] wrote:
quoted hunk
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_hdrs
this should probably be order-only dependency as well
On Sun, Oct 3, 2021 at 12:22 PM Quentin Monnet [off-list ref] wrote:
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(-)
On Sun, Oct 3, 2021 at 12:22 PM Quentin Monnet [off-list ref] wrote:
quoted hunk
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, recently
introduced as an output directory for building libbpf and bpftool.
Signed-off-by: Quentin Monnet <redacted>
---
samples/bpf/.gitignore | 4 ++++
1 file changed, 4 insertions(+)
this will match files and directories in any subdir as well, maybe
let's add / in front to be explicit about doing this only on current
directly level (*.skel.h is fine)?
On Wed, Oct 6, 2021 at 11:24 AM Andrii Nakryiko
[off-list ref] wrote:
On Sun, Oct 3, 2021 at 12:22 PM Quentin Monnet [off-list ref] wrote:
quoted
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, recently
introduced as an output directory for building libbpf and bpftool.
Signed-off-by: Quentin Monnet <redacted>
---
samples/bpf/.gitignore | 4 ++++
1 file changed, 4 insertions(+)
this will match files and directories in any subdir as well, maybe
let's add / in front to be explicit about doing this only on current
directly level (*.skel.h is fine)?
Oh, let's also move this patch before the samples/bpf changes, so that
there is no intermediate state between commits where we have these
files not ignored
On Sun, Oct 3, 2021 at 12:22 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.
v3:
Please see few problems with libbpf_hdrs phony targets. Seems like
they all can be order-only dependencies and not causing unnecessary
rebuilds.
Can you please also normalize your patch prefixes for bpftool and
other tools? We've been using a short and simple "bpftool: " prefix
for bpftool-related changes, and for other tools it would be just
"tools/runqslower" or "tools/resolve_btfids". Please update
accordingly. Thanks!
- 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 (10):
tools: bpftool: remove unused includes to <bpf/bpf_gen_internal.h>
tools: 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: install libbpf headers when building
samples/bpf: update .gitignore
selftests/bpf: better clean up for runqslower in test_bpftool_build.sh
tools: 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 | 41 ++++++++++++++-----
tools/bpf/bpftool/Makefile | 32 +++++++++------
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/testing/selftests/bpf/Makefile | 26 ++++++++----
.../selftests/bpf/test_bpftool_build.sh | 4 ++
12 files changed, 148 insertions(+), 68 deletions(-)
--
2.30.2
On Wed, 6 Oct 2021 at 19:28, Andrii Nakryiko [off-list ref] wrote:
On Sun, Oct 3, 2021 at 12:22 PM Quentin Monnet [off-list ref] wrote:
quoted
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.
v3:
Please see few problems with libbpf_hdrs phony targets. Seems like
they all can be order-only dependencies and not causing unnecessary
rebuilds.
Nice catch, I didn't realise it would force rebuilding :(. I'll
address it in the next version. I'll also add a few adjustments to
libbpf's and bpftool's Makefiles to make sure we don't recompile when
not necessary, because of the header files that are currently
installed unconditionally.
Can you please also normalize your patch prefixes for bpftool and
other tools? We've been using a short and simple "bpftool: " prefix
for bpftool-related changes, and for other tools it would be just
"tools/runqslower" or "tools/resolve_btfids". Please update
accordingly. Thanks!
$ git log --oneline --pretty='format:%s' -- tools/bpf/bpftool/ | \
grep -oE '^(bpftool:|tools: bpftool:)' | sort | uniq -c
128 bpftool:
194 tools: bpftool:
... And “we”'ve been using “tools: bpftool:” since the early days :).
But yeah sure, I'll adjust. Shorter looks better. Just wondering, are
those prefixes documented anywhere?
Thanks,
Quentin
On Thu, Oct 7, 2021 at 12:43 PM Quentin Monnet [off-list ref] wrote:
On Wed, 6 Oct 2021 at 19:28, Andrii Nakryiko [off-list ref] wrote:
quoted
On Sun, Oct 3, 2021 at 12:22 PM Quentin Monnet [off-list ref] wrote:
quoted
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.
v3:
Please see few problems with libbpf_hdrs phony targets. Seems like
they all can be order-only dependencies and not causing unnecessary
rebuilds.
Nice catch, I didn't realise it would force rebuilding :(. I'll
address it in the next version. I'll also add a few adjustments to
libbpf's and bpftool's Makefiles to make sure we don't recompile when
not necessary, because of the header files that are currently
installed unconditionally.
quoted
Can you please also normalize your patch prefixes for bpftool and
other tools? We've been using a short and simple "bpftool: " prefix
for bpftool-related changes, and for other tools it would be just
"tools/runqslower" or "tools/resolve_btfids". Please update
accordingly. Thanks!
But then:
$ git log --oneline --pretty='format:%s' -- tools/testing/selftests/bpf/ | \
grep -oE '^(selftests/bpf:|selftests: bpf:)' | sort | uniq -c
925 selftests/bpf:
98 selftests: bpf:
And if we expand your search a bit:
$ git log --oneline --pretty='format:%s' -- tools/bpf/bpftool/ | \
grep -oE '^(bpftool:|tools: bpftool:|tools/bpftool:)' | sort | uniq -c
130 bpftool:
52 tools/bpftool:
194 tools: bpftool:
bpftool: + tools/bpftool: almost matches up with tools: bpftool: ;)
I think the most prevailing convention was "dir1/dir2: " style overall.
... And “we”'ve been using “tools: bpftool:” since the early days :).
But yeah sure, I'll adjust. Shorter looks better. Just wondering, are
those prefixes documented anywhere?