netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next v4 00/12] install libbpf headers when using the library
@ 2021-10-07 19:44 Quentin Monnet
  2021-10-07 19:44 ` [PATCH bpf-next v4 01/12] libbpf: skip re-installing headers file if source is older than target Quentin Monnet
                   ` (12 more replies)
  0 siblings, 13 replies; 18+ messages in thread
From: Quentin Monnet @ 2021-10-07 19:44 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: netdev, bpf, Quentin Monnet

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH bpf-next v4 01/12] libbpf: skip re-installing headers file if source is older than target
  2021-10-07 19:44 [PATCH bpf-next v4 00/12] install libbpf headers when using the library Quentin Monnet
@ 2021-10-07 19:44 ` Quentin Monnet
  2021-10-08 19:13   ` Andrii Nakryiko
  2021-10-07 19:44 ` [PATCH bpf-next v4 02/12] bpftool: remove unused includes to <bpf/bpf_gen_internal.h> Quentin Monnet
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 18+ messages in thread
From: Quentin Monnet @ 2021-10-07 19:44 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: netdev, bpf, Quentin Monnet

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 <quentin@isovalent.com>
---
 tools/lib/bpf/Makefile | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index 41e4f78dbad5..a92d3b9692a8 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@ -241,15 +241,23 @@ install_lib: all_cmd
 		$(call do_install_mkdir,$(libdir_SQ)); \
 		cp -fpR $(LIB_FILE) $(DESTDIR)$(libdir_SQ)
 
-INSTALL_HEADERS = bpf.h libbpf.h btf.h libbpf_common.h libbpf_legacy.h xsk.h \
-		  bpf_helpers.h $(BPF_GENERATED) bpf_tracing.h		     \
-		  bpf_endian.h bpf_core_read.h skel_internal.h		     \
-		  libbpf_version.h
+SRC_HDRS := bpf.h libbpf.h btf.h libbpf_common.h libbpf_legacy.h xsk.h	       \
+	    bpf_helpers.h bpf_tracing.h bpf_endian.h bpf_core_read.h	       \
+	    skel_internal.h libbpf_version.h
+GEN_HDRS := $(BPF_GENERATED)
+INSTALL_SRC_HDRS := $(addprefix $(DESTDIR)$(prefix)/include/bpf/,$(SRC_HDRS))
+INSTALL_GEN_HDRS := $(addprefix $(DESTDIR)$(prefix)/include/bpf/, \
+				$(notdir $(GEN_HDRS)))
+$(INSTALL_SRC_HDRS): $(DESTDIR)$(prefix)/include/bpf/%.h: %.h
+	$(call QUIET_INSTALL, $@) \
+		$(call do_install,$<,$(prefix)/include/bpf,644)
+$(INSTALL_GEN_HDRS): $(DESTDIR)$(prefix)/include/bpf/%.h: $(OUTPUT)%.h
+	$(call QUIET_INSTALL, $@) \
+		$(call do_install,$<,$(prefix)/include/bpf,644)
 
-install_headers: $(BPF_GENERATED)
-	$(call QUIET_INSTALL, headers)					     \
-		$(foreach hdr,$(INSTALL_HEADERS),			     \
-			$(call do_install,$(hdr),$(prefix)/include/bpf,644);)
+INSTALL_HEADERS := $(INSTALL_SRC_HDRS) $(INSTALL_GEN_HDRS)
+
+install_headers: $(BPF_GENERATED) $(INSTALL_HEADERS)
 
 install_pkgconfig: $(PC_FILE)
 	$(call QUIET_INSTALL, $(PC_FILE)) \
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH bpf-next v4 02/12] bpftool: remove unused includes to <bpf/bpf_gen_internal.h>
  2021-10-07 19:44 [PATCH bpf-next v4 00/12] install libbpf headers when using the library Quentin Monnet
  2021-10-07 19:44 ` [PATCH bpf-next v4 01/12] libbpf: skip re-installing headers file if source is older than target Quentin Monnet
@ 2021-10-07 19:44 ` Quentin Monnet
  2021-10-07 19:44 ` [PATCH bpf-next v4 03/12] bpftool: install libbpf headers instead of including the dir Quentin Monnet
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Quentin Monnet @ 2021-10-07 19:44 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: netdev, bpf, Quentin Monnet

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 <quentin@isovalent.com>
---
 tools/bpf/bpftool/gen.c  | 1 -
 tools/bpf/bpftool/prog.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index cc835859465b..b2ffc18eafc1 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -18,7 +18,6 @@
 #include <sys/stat.h>
 #include <sys/mman.h>
 #include <bpf/btf.h>
-#include <bpf/bpf_gen_internal.h>
 
 #include "json_writer.h"
 #include "main.h"
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index a24ea7e26aa4..277d51c4c5d9 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -25,7 +25,6 @@
 #include <bpf/bpf.h>
 #include <bpf/btf.h>
 #include <bpf/libbpf.h>
-#include <bpf/bpf_gen_internal.h>
 #include <bpf/skel_internal.h>
 
 #include "cfg.h"
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH bpf-next v4 03/12] bpftool: install libbpf headers instead of including the dir
  2021-10-07 19:44 [PATCH bpf-next v4 00/12] install libbpf headers when using the library Quentin Monnet
  2021-10-07 19:44 ` [PATCH bpf-next v4 01/12] libbpf: skip re-installing headers file if source is older than target Quentin Monnet
  2021-10-07 19:44 ` [PATCH bpf-next v4 02/12] bpftool: remove unused includes to <bpf/bpf_gen_internal.h> Quentin Monnet
@ 2021-10-07 19:44 ` Quentin Monnet
  2021-10-08 19:13   ` Andrii Nakryiko
  2021-10-07 19:44 ` [PATCH bpf-next v4 04/12] tools/resolve_btfids: install libbpf headers when building Quentin Monnet
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 18+ messages in thread
From: Quentin Monnet @ 2021-10-07 19:44 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: netdev, bpf, Quentin Monnet

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 <quentin@isovalent.com>
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(-)

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 1fcf5b01a193..ba02d71c39ef 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -17,19 +17,23 @@ endif
 BPF_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.a
 LIBBPF_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 := $(shell make -rR --no-print-directory -sC ../../.. kernelversion)
 endif
@@ -38,7 +42,13 @@ $(LIBBPF_OUTPUT) $(BOOTSTRAP_OUTPUT) $(LIBBPF_BOOTSTRAP_OUTPUT):
 	$(QUIET_MKDIR)mkdir -p $@
 
 $(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
+
+$(LIBBPF_INCLUDE)/bpf/$(LIBBPF_INTERNAL_HDRS): \
+		$(addprefix $(BPF_DIR),$(LIBBPF_INTERNAL_HDRS)) $(LIBBPF)
+	$(call QUIET_INSTALL, bpf/$(notdir $@))
+	$(Q)install -m 644 -t $(LIBBPF_INCLUDE)/bpf/ $(BPF_DIR)$(notdir $@)
 
 $(LIBBPF_BOOTSTRAP): FORCE | $(LIBBPF_BOOTSTRAP_OUTPUT)
 	$(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(LIBBPF_BOOTSTRAP_OUTPUT) \
@@ -60,10 +70,10 @@ CFLAGS += -W -Wall -Wextra -Wno-unused-parameter -Wno-missing-field-initializers
 CFLAGS += $(filter-out -Wswitch-enum -Wnested-externs,$(EXTRA_WARNINGS))
 CFLAGS += -DPACKAGE='"bpftool"' -D__EXPORTED_HEADERS__ \
 	-I$(if $(OUTPUT),$(OUTPUT),.) \
+	-I$(LIBBPF_INCLUDE) \
 	-I$(srctree)/kernel/bpf/ \
 	-I$(srctree)/tools/include \
 	-I$(srctree)/tools/include/uapi \
-	-I$(srctree)/tools/lib \
 	-I$(srctree)/tools/perf
 CFLAGS += -DBPFTOOL_VERSION='"$(BPFTOOL_VERSION)"'
 ifneq ($(EXTRA_CFLAGS),)
@@ -140,7 +150,7 @@ BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o g
 $(BOOTSTRAP_OBJS): $(LIBBPF_BOOTSTRAP)
 
 OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
-$(OBJS): $(LIBBPF)
+$(OBJS): $(LIBBPF) $(LIBBPF_INCLUDE)/bpf/$(LIBBPF_INTERNAL_HDRS)
 
 VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux)				\
 		     $(if $(KBUILD_OUTPUT),$(KBUILD_OUTPUT)/vmlinux)	\
@@ -167,8 +177,7 @@ $(OUTPUT)%.bpf.o: skeleton/%.bpf.c $(OUTPUT)vmlinux.h $(LIBBPF)
 	$(QUIET_CLANG)$(CLANG) \
 		-I$(if $(OUTPUT),$(OUTPUT),.) \
 		-I$(srctree)/tools/include/uapi/ \
-		-I$(LIBBPF_PATH) \
-		-I$(srctree)/tools/lib \
+		-I$(LIBBPF_INCLUDE) \
 		-g -O2 -Wall -target bpf -c $< -o $@ && $(LLVM_STRIP) -g $@
 
 $(OUTPUT)%.skel.h: $(OUTPUT)%.bpf.o $(BPFTOOL_BOOTSTRAP)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index c5c9a9f50d8d..849a4637f59d 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -209,6 +209,8 @@ $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile)    \
 		    CC=$(HOSTCC) LD=$(HOSTLD)				       \
 		    EXTRA_CFLAGS='-g -O0'				       \
 		    OUTPUT=$(HOST_BUILD_DIR)/bpftool/			       \
+		    LIBBPF_OUTPUT=$(HOST_BUILD_DIR)/libbpf/		       \
+		    LIBBPF_DESTDIR=$(HOST_SCRATCH_DIR)/			       \
 		    prefix= DESTDIR=$(HOST_SCRATCH_DIR)/ install
 
 all: docs
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH bpf-next v4 04/12] tools/resolve_btfids: install libbpf headers when building
  2021-10-07 19:44 [PATCH bpf-next v4 00/12] install libbpf headers when using the library Quentin Monnet
                   ` (2 preceding siblings ...)
  2021-10-07 19:44 ` [PATCH bpf-next v4 03/12] bpftool: install libbpf headers instead of including the dir Quentin Monnet
@ 2021-10-07 19:44 ` Quentin Monnet
  2021-10-07 19:44 ` [PATCH bpf-next v4 05/12] tools/runqslower: " Quentin Monnet
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Quentin Monnet @ 2021-10-07 19:44 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: netdev, bpf, Quentin Monnet

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 <quentin@isovalent.com>
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(-)

diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
index 08b75e314ae7..4eb74c7e65c4 100644
--- a/tools/bpf/resolve_btfids/Makefile
+++ b/tools/bpf/resolve_btfids/Makefile
@@ -29,25 +29,31 @@ BPFOBJ     := $(OUTPUT)/libbpf/libbpf.a
 LIBBPF_OUT := $(abspath $(dir $(BPFOBJ)))/
 SUBCMDOBJ  := $(OUTPUT)/libsubcmd/libsubcmd.a
 
+LIBBPF_DESTDIR := $(LIBBPF_OUT)
+LIBBPF_INCLUDE := $(LIBBPF_DESTDIR)include
+
 BINARY     := $(OUTPUT)/resolve_btfids
 BINARY_IN  := $(BINARY)-in.o
 
 all: $(BINARY)
 
-$(OUTPUT) $(OUTPUT)/libbpf $(OUTPUT)/libsubcmd:
+$(OUTPUT) $(OUTPUT)/libsubcmd $(LIBBPF_OUT):
 	$(call msg,MKDIR,,$@)
 	$(Q)mkdir -p $(@)
 
 $(SUBCMDOBJ): fixdep FORCE | $(OUTPUT)/libsubcmd
 	$(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(abspath $(dir $@))/ $(abspath $@)
 
-$(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(OUTPUT)/libbpf
-	$(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC)  OUTPUT=$(LIBBPF_OUT) $(abspath $@)
+$(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile)	       \
+	   | $(LIBBPF_OUT)
+	$(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC) OUTPUT=$(LIBBPF_OUT)    \
+		    DESTDIR=$(LIBBPF_DESTDIR) prefix=			       \
+		    $(abspath $@) install_headers
 
 CFLAGS := -g \
           -I$(srctree)/tools/include \
           -I$(srctree)/tools/include/uapi \
-          -I$(LIBBPF_SRC) \
+          -I$(LIBBPF_INCLUDE) \
           -I$(SUBCMD_SRC)
 
 LIBS = -lelf -lz
@@ -65,7 +71,8 @@ $(BINARY): $(BPFOBJ) $(SUBCMDOBJ) $(BINARY_IN)
 clean_objects := $(wildcard $(OUTPUT)/*.o                \
                             $(OUTPUT)/.*.o.cmd           \
                             $(OUTPUT)/.*.o.d             \
-                            $(OUTPUT)/libbpf             \
+                            $(LIBBPF_OUT)                \
+                            $(LIBBPF_DESTDIR)            \
                             $(OUTPUT)/libsubcmd          \
                             $(OUTPUT)/resolve_btfids)
 
diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
index c6c3e613858a..716e6ad1864b 100644
--- a/tools/bpf/resolve_btfids/main.c
+++ b/tools/bpf/resolve_btfids/main.c
@@ -60,8 +60,8 @@
 #include <linux/rbtree.h>
 #include <linux/zalloc.h>
 #include <linux/err.h>
-#include <btf.h>
-#include <libbpf.h>
+#include <bpf/btf.h>
+#include <bpf/libbpf.h>
 #include <parse-options.h>
 
 #define BTF_IDS_SECTION	".BTF_ids"
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 849a4637f59d..090f424ac5e1 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -122,9 +122,11 @@ BPFOBJ := $(BUILD_DIR)/libbpf/libbpf.a
 ifneq ($(CROSS_COMPILE),)
 HOST_BUILD_DIR		:= $(BUILD_DIR)/host
 HOST_SCRATCH_DIR	:= $(OUTPUT)/host-tools
+HOST_INCLUDE_DIR	:= $(HOST_SCRATCH_DIR)/include
 else
 HOST_BUILD_DIR		:= $(BUILD_DIR)
 HOST_SCRATCH_DIR	:= $(SCRATCH_DIR)
+HOST_INCLUDE_DIR	:= $(INCLUDE_DIR)
 endif
 HOST_BPFOBJ := $(HOST_BUILD_DIR)/libbpf/libbpf.a
 RESOLVE_BTFIDS := $(HOST_BUILD_DIR)/resolve_btfids/resolve_btfids
@@ -227,7 +229,7 @@ docs-clean:
 
 $(BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile)		       \
 	   ../../../include/uapi/linux/bpf.h                                   \
-	   | $(INCLUDE_DIR) $(BUILD_DIR)/libbpf
+	   | $(BUILD_DIR)/libbpf
 	$(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) OUTPUT=$(BUILD_DIR)/libbpf/ \
 		    EXTRA_CFLAGS='-g -O0'				       \
 		    DESTDIR=$(SCRATCH_DIR) prefix= all install_headers
@@ -235,7 +237,7 @@ $(BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile)		       \
 ifneq ($(BPFOBJ),$(HOST_BPFOBJ))
 $(HOST_BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile)                \
 	   ../../../include/uapi/linux/bpf.h                                   \
-	   | $(INCLUDE_DIR) $(HOST_BUILD_DIR)/libbpf
+	   | $(HOST_BUILD_DIR)/libbpf
 	$(Q)$(MAKE) $(submake_extras) -C $(BPFDIR)                             \
 		    EXTRA_CFLAGS='-g -O0'				       \
 		    OUTPUT=$(HOST_BUILD_DIR)/libbpf/ CC=$(HOSTCC) LD=$(HOSTLD) \
@@ -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
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH bpf-next v4 05/12] tools/runqslower: install libbpf headers when building
  2021-10-07 19:44 [PATCH bpf-next v4 00/12] install libbpf headers when using the library Quentin Monnet
                   ` (3 preceding siblings ...)
  2021-10-07 19:44 ` [PATCH bpf-next v4 04/12] tools/resolve_btfids: install libbpf headers when building Quentin Monnet
@ 2021-10-07 19:44 ` Quentin Monnet
  2021-10-07 19:44 ` [PATCH bpf-next v4 06/12] bpf: preload: " Quentin Monnet
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Quentin Monnet @ 2021-10-07 19:44 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: netdev, bpf, Quentin Monnet

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 <quentin@isovalent.com>
---
 tools/bpf/runqslower/Makefile        | 22 +++++++++++++---------
 tools/testing/selftests/bpf/Makefile | 15 +++++++++------
 2 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/tools/bpf/runqslower/Makefile b/tools/bpf/runqslower/Makefile
index 3818ec511fd2..bbd1150578f7 100644
--- a/tools/bpf/runqslower/Makefile
+++ b/tools/bpf/runqslower/Makefile
@@ -9,9 +9,9 @@ BPFTOOL ?= $(DEFAULT_BPFTOOL)
 LIBBPF_SRC := $(abspath ../../lib/bpf)
 BPFOBJ_OUTPUT := $(OUTPUT)libbpf/
 BPFOBJ := $(BPFOBJ_OUTPUT)libbpf.a
-BPF_INCLUDE := $(BPFOBJ_OUTPUT)
-INCLUDES := -I$(OUTPUT) -I$(BPF_INCLUDE) -I$(abspath ../../lib)        \
-       -I$(abspath ../../include/uapi)
+BPF_DESTDIR := $(BPFOBJ_OUTPUT)
+BPF_INCLUDE := $(BPF_DESTDIR)/include
+INCLUDES := -I$(OUTPUT) -I$(BPF_INCLUDE) -I$(abspath ../../include/uapi)
 CFLAGS := -g -Wall
 
 # Try to detect best kernel BTF source
@@ -33,7 +33,7 @@ endif
 
 .DELETE_ON_ERROR:
 
-.PHONY: all clean runqslower
+.PHONY: all clean runqslower libbpf_hdrs
 all: runqslower
 
 runqslower: $(OUTPUT)/runqslower
@@ -46,13 +46,15 @@ clean:
 	$(Q)$(RM) $(OUTPUT)runqslower
 	$(Q)$(RM) -r .output
 
+libbpf_hdrs: $(BPFOBJ)
+
 $(OUTPUT)/runqslower: $(OUTPUT)/runqslower.o $(BPFOBJ)
 	$(QUIET_LINK)$(CC) $(CFLAGS) $^ -lelf -lz -o $@
 
 $(OUTPUT)/runqslower.o: runqslower.h $(OUTPUT)/runqslower.skel.h	      \
-			$(OUTPUT)/runqslower.bpf.o
+			$(OUTPUT)/runqslower.bpf.o | libbpf_hdrs
 
-$(OUTPUT)/runqslower.bpf.o: $(OUTPUT)/vmlinux.h runqslower.h
+$(OUTPUT)/runqslower.bpf.o: $(OUTPUT)/vmlinux.h runqslower.h | libbpf_hdrs
 
 $(OUTPUT)/%.skel.h: $(OUTPUT)/%.bpf.o | $(BPFTOOL)
 	$(QUIET_GEN)$(BPFTOOL) gen skeleton $< > $@
@@ -81,8 +83,10 @@ else
 endif
 
 $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(BPFOBJ_OUTPUT)
-	$(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC) OUTPUT=$(BPFOBJ_OUTPUT) $@
+	$(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC) OUTPUT=$(BPFOBJ_OUTPUT) \
+		    DESTDIR=$(BPFOBJ_OUTPUT) prefix= $(abspath $@) install_headers
 
-$(DEFAULT_BPFTOOL): | $(BPFTOOL_OUTPUT)
+$(DEFAULT_BPFTOOL): $(BPFOBJ) | $(BPFTOOL_OUTPUT)
 	$(Q)$(MAKE) $(submake_extras) -C ../bpftool OUTPUT=$(BPFTOOL_OUTPUT)   \
-		    CC=$(HOSTCC) LD=$(HOSTLD)
+		    LIBBPF_OUTPUT=$(BPFOBJ_OUTPUT)			       \
+		    LIBBPF_DESTDIR=$(BPF_DESTDIR) CC=$(HOSTCC) LD=$(HOSTLD)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 090f424ac5e1..e023d734f7b0 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -130,6 +130,7 @@ HOST_INCLUDE_DIR	:= $(INCLUDE_DIR)
 endif
 HOST_BPFOBJ := $(HOST_BUILD_DIR)/libbpf/libbpf.a
 RESOLVE_BTFIDS := $(HOST_BUILD_DIR)/resolve_btfids/resolve_btfids
+RUNQSLOWER_OUTPUT := $(BUILD_DIR)/runqslower/
 
 VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux)				\
 		     $(if $(KBUILD_OUTPUT),$(KBUILD_OUTPUT)/vmlinux)	\
@@ -154,7 +155,7 @@ $(notdir $(TEST_GEN_PROGS)						\
 # sort removes libbpf duplicates when not cross-building
 MAKE_DIRS := $(sort $(BUILD_DIR)/libbpf $(HOST_BUILD_DIR)/libbpf	       \
 	       $(HOST_BUILD_DIR)/bpftool $(HOST_BUILD_DIR)/resolve_btfids      \
-	       $(INCLUDE_DIR))
+	       $(RUNQSLOWER_OUTPUT) $(INCLUDE_DIR))
 $(MAKE_DIRS):
 	$(call msg,MKDIR,,$@)
 	$(Q)mkdir -p $@
@@ -183,11 +184,13 @@ $(OUTPUT)/test_stub.o: test_stub.c $(BPFOBJ)
 
 DEFAULT_BPFTOOL := $(HOST_SCRATCH_DIR)/sbin/bpftool
 
-$(OUTPUT)/runqslower: $(BPFOBJ) | $(DEFAULT_BPFTOOL)
-	$(Q)$(MAKE) $(submake_extras) -C $(TOOLSDIR)/bpf/runqslower	\
-		    OUTPUT=$(SCRATCH_DIR)/ VMLINUX_BTF=$(VMLINUX_BTF)   \
-		    BPFOBJ=$(BPFOBJ) BPF_INCLUDE=$(INCLUDE_DIR) &&	\
-		    cp $(SCRATCH_DIR)/runqslower $@
+$(OUTPUT)/runqslower: $(BPFOBJ) | $(DEFAULT_BPFTOOL) $(RUNQSLOWER_OUTPUT)
+	$(Q)$(MAKE) $(submake_extras) -C $(TOOLSDIR)/bpf/runqslower	       \
+		    OUTPUT=$(RUNQSLOWER_OUTPUT) VMLINUX_BTF=$(VMLINUX_BTF)     \
+		    BPFTOOL_OUTPUT=$(BUILD_DIR)/bpftool/		       \
+		    BPFOBJ_OUTPUT=$(BUILD_DIR)/libbpf			       \
+		    BPFOBJ=$(BPFOBJ) BPF_INCLUDE=$(INCLUDE_DIR) &&	       \
+		    cp $(RUNQSLOWER_OUTPUT)runqslower $@
 
 TEST_GEN_PROGS_EXTENDED += $(DEFAULT_BPFTOOL)
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH bpf-next v4 06/12] bpf: preload: install libbpf headers when building
  2021-10-07 19:44 [PATCH bpf-next v4 00/12] install libbpf headers when using the library Quentin Monnet
                   ` (4 preceding siblings ...)
  2021-10-07 19:44 ` [PATCH bpf-next v4 05/12] tools/runqslower: " Quentin Monnet
@ 2021-10-07 19:44 ` Quentin Monnet
  2021-10-07 19:44 ` [PATCH bpf-next v4 07/12] bpf: iterators: " Quentin Monnet
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Quentin Monnet @ 2021-10-07 19:44 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: netdev, bpf, Quentin Monnet

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 <quentin@isovalent.com>
---
 kernel/bpf/preload/Makefile | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/kernel/bpf/preload/Makefile b/kernel/bpf/preload/Makefile
index 1951332dd15f..469d35e890eb 100644
--- a/kernel/bpf/preload/Makefile
+++ b/kernel/bpf/preload/Makefile
@@ -1,21 +1,36 @@
 # SPDX-License-Identifier: GPL-2.0
 
 LIBBPF_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.a install_headers
+
+libbpf_hdrs: $(LIBBPF_A)
+
+.PHONY: libbpf_hdrs
+
+$(LIBBPF_OUT):
+	$(call msg,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-result
 
 userprogs := bpf_preload_umd
 
 clean-files := $(userprogs) bpf_helper_defs.h FEATURE-DUMP.libbpf staticobjs/ feature/
+clean-files += $(LIBBPF_OUT) $(LIBBPF_DESTDIR)
+
+$(obj)/iterators/iterators.o: | libbpf_hdrs
 
 bpf_preload_umd-objs := iterators/iterators.o
 bpf_preload_umd-userldlibs := $(LIBBPF_A) -lelf -lz
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH bpf-next v4 07/12] bpf: iterators: install libbpf headers when building
  2021-10-07 19:44 [PATCH bpf-next v4 00/12] install libbpf headers when using the library Quentin Monnet
                   ` (5 preceding siblings ...)
  2021-10-07 19:44 ` [PATCH bpf-next v4 06/12] bpf: preload: " Quentin Monnet
@ 2021-10-07 19:44 ` Quentin Monnet
  2021-10-07 19:44 ` [PATCH bpf-next v4 08/12] samples/bpf: update .gitignore Quentin Monnet
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Quentin Monnet @ 2021-10-07 19:44 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: netdev, bpf, Quentin Monnet

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 <quentin@isovalent.com>
---
 kernel/bpf/preload/iterators/Makefile | 39 ++++++++++++++++++---------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/kernel/bpf/preload/iterators/Makefile b/kernel/bpf/preload/iterators/Makefile
index 28fa8c1440f4..ec39ccc71b8e 100644
--- a/kernel/bpf/preload/iterators/Makefile
+++ b/kernel/bpf/preload/iterators/Makefile
@@ -1,18 +1,26 @@
 # SPDX-License-Identifier: GPL-2.0
 OUTPUT := .output
+abs_out := $(abspath $(OUTPUT))
+
 CLANG ?= clang
 LLC ?= llc
 LLVM_STRIP ?= llvm-strip
+
+TOOLS_PATH := $(abspath ../../../../tools)
+BPFTOOL_SRC := $(TOOLS_PATH)/bpf/bpftool
+BPFTOOL_OUTPUT := $(abs_out)/bpftool
 DEFAULT_BPFTOOL := $(OUTPUT)/sbin/bpftool
 BPFTOOL ?= $(DEFAULT_BPFTOOL)
-LIBBPF_SRC := $(abspath ../../../../tools/lib/bpf)
-BPFOBJ := $(OUTPUT)/libbpf.a
-BPF_INCLUDE := $(OUTPUT)
-INCLUDES := -I$(OUTPUT) -I$(BPF_INCLUDE) -I$(abspath ../../../../tools/lib)        \
-       -I$(abspath ../../../../tools/include/uapi)
+
+LIBBPF_SRC := $(TOOLS_PATH)/lib/bpf
+LIBBPF_OUTPUT := $(abs_out)/libbpf
+LIBBPF_DESTDIR := $(LIBBPF_OUTPUT)
+LIBBPF_INCLUDE := $(LIBBPF_DESTDIR)/include
+BPFOBJ := $(LIBBPF_OUTPUT)/libbpf.a
+
+INCLUDES := -I$(OUTPUT) -I$(LIBBPF_INCLUDE) -I$(TOOLS_PATH)/include/uapi
 CFLAGS := -g -Wall
 
-abs_out := $(abspath $(OUTPUT))
 ifeq ($(V),1)
 Q =
 msg =
@@ -44,14 +52,19 @@ $(OUTPUT)/iterators.bpf.o: iterators.bpf.c $(BPFOBJ) | $(OUTPUT)
 		 -c $(filter %.c,$^) -o $@ &&				      \
 	$(LLVM_STRIP) -g $@
 
-$(OUTPUT):
+$(OUTPUT) $(LIBBPF_OUTPUT) $(BPFTOOL_OUTPUT):
 	$(call msg,MKDIR,$@)
-	$(Q)mkdir -p $(OUTPUT)
+	$(Q)mkdir -p $@
 
-$(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(OUTPUT)
+$(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile)	       \
+	   | $(LIBBPF_OUTPUT)
 	$(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC)			       \
-		    OUTPUT=$(abspath $(dir $@))/ $(abspath $@)
+		    OUTPUT=$(abspath $(dir $@))/ prefix=		       \
+		    DESTDIR=$(LIBBPF_DESTDIR) $(abspath $@) install_headers
 
-$(DEFAULT_BPFTOOL):
-	$(Q)$(MAKE) $(submake_extras) -C ../../../../tools/bpf/bpftool			      \
-		    prefix= OUTPUT=$(abs_out)/ DESTDIR=$(abs_out) install
+$(DEFAULT_BPFTOOL): $(BPFOBJ) | $(BPFTOOL_OUTPUT)
+	$(Q)$(MAKE) $(submake_extras) -C $(BPFTOOL_SRC)			       \
+		    OUTPUT=$(BPFTOOL_OUTPUT)/				       \
+		    LIBBPF_OUTPUT=$(LIBBPF_OUTPUT)/			       \
+		    LIBBPF_DESTDIR=$(LIBBPF_DESTDIR)/			       \
+		    prefix= DESTDIR=$(abs_out)/ install
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH bpf-next v4 08/12] samples/bpf: update .gitignore
  2021-10-07 19:44 [PATCH bpf-next v4 00/12] install libbpf headers when using the library Quentin Monnet
                   ` (6 preceding siblings ...)
  2021-10-07 19:44 ` [PATCH bpf-next v4 07/12] bpf: iterators: " Quentin Monnet
@ 2021-10-07 19:44 ` Quentin Monnet
  2021-10-07 19:44 ` [PATCH bpf-next v4 09/12] samples/bpf: install libbpf headers when building Quentin Monnet
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Quentin Monnet @ 2021-10-07 19:44 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: netdev, bpf, Quentin Monnet

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 <quentin@isovalent.com>
---
 samples/bpf/.gitignore | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/samples/bpf/.gitignore b/samples/bpf/.gitignore
index fcba217f0ae2..0e7bfdbff80a 100644
--- a/samples/bpf/.gitignore
+++ b/samples/bpf/.gitignore
@@ -57,3 +57,7 @@ testfile.img
 hbm_out.log
 iperf.*
 *.out
+*.skel.h
+/vmlinux.h
+/bpftool/
+/libbpf/
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH bpf-next v4 09/12] samples/bpf: install libbpf headers when building
  2021-10-07 19:44 [PATCH bpf-next v4 00/12] install libbpf headers when using the library Quentin Monnet
                   ` (7 preceding siblings ...)
  2021-10-07 19:44 ` [PATCH bpf-next v4 08/12] samples/bpf: update .gitignore Quentin Monnet
@ 2021-10-07 19:44 ` Quentin Monnet
  2021-10-07 19:44 ` [PATCH bpf-next v4 10/12] samples/bpf: do not FORCE-recompile libbpf Quentin Monnet
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Quentin Monnet @ 2021-10-07 19:44 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: netdev, bpf, Quentin Monnet

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 <quentin@isovalent.com>
---
 samples/bpf/Makefile | 41 +++++++++++++++++++++++++++++++----------
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index a5783749ec15..cb72198f6b48 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -61,7 +61,11 @@ tprogs-y += xdp_redirect
 tprogs-y += xdp_monitor
 
 # Libbpf dependencies
-LIBBPF = $(TOOLS_PATH)/lib/bpf/libbpf.a
+LIBBPF_SRC = $(TOOLS_PATH)/lib/bpf
+LIBBPF_OUTPUT = $(abspath $(BPF_SAMPLES_PATH))/libbpf
+LIBBPF_DESTDIR = $(LIBBPF_OUTPUT)
+LIBBPF_INCLUDE = $(LIBBPF_DESTDIR)/include
+LIBBPF = $(LIBBPF_OUTPUT)/libbpf.a
 
 CGROUP_HELPERS := ../../tools/testing/selftests/bpf/cgroup_helpers.o
 TRACE_HELPERS := ../../tools/testing/selftests/bpf/trace_helpers.o
@@ -200,7 +204,7 @@ TPROGS_CFLAGS += -Wstrict-prototypes
 
 TPROGS_CFLAGS += -I$(objtree)/usr/include
 TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
-TPROGS_CFLAGS += -I$(srctree)/tools/lib/
+TPROGS_CFLAGS += -I$(LIBBPF_INCLUDE)
 TPROGS_CFLAGS += -I$(srctree)/tools/include
 TPROGS_CFLAGS += -I$(srctree)/tools/perf
 TPROGS_CFLAGS += -DHAVE_ATTR_TEST=0
@@ -270,16 +274,28 @@ all:
 clean:
 	$(MAKE) -C ../../ M=$(CURDIR) clean
 	@find $(CURDIR) -type f -name '*~' -delete
+	@$(RM) -r $(CURDIR)/libbpf $(CURDIR)/bpftool
 
-$(LIBBPF): FORCE
+$(LIBBPF): FORCE | $(LIBBPF_OUTPUT)
 # Fix up variables inherited from Kbuild that tools/ build system won't like
-	$(MAKE) -C $(dir $@) RM='rm -rf' EXTRA_CFLAGS="$(TPROGS_CFLAGS)" \
-		LDFLAGS=$(TPROGS_LDFLAGS) srctree=$(BPF_SAMPLES_PATH)/../../ O=
+	$(MAKE) -C $(LIBBPF_SRC) RM='rm -rf' EXTRA_CFLAGS="$(TPROGS_CFLAGS)" \
+		LDFLAGS=$(TPROGS_LDFLAGS) srctree=$(BPF_SAMPLES_PATH)/../../ \
+		O= OUTPUT=$(LIBBPF_OUTPUT)/ DESTDIR=$(LIBBPF_DESTDIR) prefix= \
+		$@ install_headers
 
 BPFTOOLDIR := $(TOOLS_PATH)/bpf/bpftool
-BPFTOOL := $(BPFTOOLDIR)/bpftool
-$(BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile)
-	    $(MAKE) -C $(BPFTOOLDIR) srctree=$(BPF_SAMPLES_PATH)/../../
+BPFTOOL_OUTPUT := $(abspath $(BPF_SAMPLES_PATH))/bpftool
+BPFTOOL := $(BPFTOOL_OUTPUT)/bpftool
+$(BPFTOOL): $(LIBBPF) $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
+	    | $(BPFTOOL_OUTPUT)
+	    $(MAKE) -C $(BPFTOOLDIR) srctree=$(BPF_SAMPLES_PATH)/../../ \
+		OUTPUT=$(BPFTOOL_OUTPUT)/ \
+		LIBBPF_OUTPUT=$(LIBBPF_OUTPUT)/ \
+		LIBBPF_DESTDIR=$(LIBBPF_DESTDIR)/
+
+$(LIBBPF_OUTPUT) $(BPFTOOL_OUTPUT):
+	$(call msg,MKDIR,$@)
+	$(Q)mkdir -p $@
 
 $(obj)/syscall_nrs.h:	$(obj)/syscall_nrs.s FORCE
 	$(call filechk,offsets,__SYSCALL_NRS_H__)
@@ -311,6 +327,11 @@ verify_target_bpf: verify_cmds
 $(BPF_SAMPLES_PATH)/*.c: verify_target_bpf $(LIBBPF)
 $(src)/*.c: verify_target_bpf $(LIBBPF)
 
+libbpf_hdrs: $(LIBBPF)
+$(obj)/$(TRACE_HELPERS): | libbpf_hdrs
+
+.PHONY: libbpf_hdrs
+
 $(obj)/xdp_redirect_cpu_user.o: $(obj)/xdp_redirect_cpu.skel.h
 $(obj)/xdp_redirect_map_multi_user.o: $(obj)/xdp_redirect_map_multi.skel.h
 $(obj)/xdp_redirect_map_user.o: $(obj)/xdp_redirect_map.skel.h
@@ -369,7 +390,7 @@ $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h $(src)/x
 	$(Q)$(CLANG) -g -O2 -target bpf -D__TARGET_ARCH_$(SRCARCH) \
 		-Wno-compare-distinct-pointer-types -I$(srctree)/include \
 		-I$(srctree)/samples/bpf -I$(srctree)/tools/include \
-		-I$(srctree)/tools/lib $(CLANG_SYS_INCLUDES) \
+		-I$(LIBBPF_INCLUDE) $(CLANG_SYS_INCLUDES) \
 		-c $(filter %.bpf.c,$^) -o $@
 
 LINKED_SKELS := xdp_redirect_cpu.skel.h xdp_redirect_map_multi.skel.h \
@@ -406,7 +427,7 @@ $(obj)/%.o: $(src)/%.c
 	@echo "  CLANG-bpf " $@
 	$(Q)$(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(BPF_EXTRA_CFLAGS) \
 		-I$(obj) -I$(srctree)/tools/testing/selftests/bpf/ \
-		-I$(srctree)/tools/lib/ \
+		-I$(LIBBPF_INCLUDE) \
 		-D__KERNEL__ -D__BPF_TRACING__ -Wno-unused-value -Wno-pointer-sign \
 		-D__TARGET_ARCH_$(SRCARCH) -Wno-compare-distinct-pointer-types \
 		-Wno-gnu-variable-sized-type-not-at-end \
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH bpf-next v4 10/12] samples/bpf: do not FORCE-recompile libbpf
  2021-10-07 19:44 [PATCH bpf-next v4 00/12] install libbpf headers when using the library Quentin Monnet
                   ` (8 preceding siblings ...)
  2021-10-07 19:44 ` [PATCH bpf-next v4 09/12] samples/bpf: install libbpf headers when building Quentin Monnet
@ 2021-10-07 19:44 ` Quentin Monnet
  2021-10-07 19:44 ` [PATCH bpf-next v4 11/12] selftests/bpf: better clean up for runqslower in test_bpftool_build.sh Quentin Monnet
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 18+ messages in thread
From: Quentin Monnet @ 2021-10-07 19:44 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: netdev, bpf, Quentin Monnet

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 <quentin@isovalent.com>
---
 samples/bpf/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index cb72198f6b48..c9cee54ce79d 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -276,7 +276,8 @@ clean:
 	@find $(CURDIR) -type f -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)/../../ \
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH bpf-next v4 11/12] selftests/bpf: better clean up for runqslower in test_bpftool_build.sh
  2021-10-07 19:44 [PATCH bpf-next v4 00/12] install libbpf headers when using the library Quentin Monnet
                   ` (9 preceding siblings ...)
  2021-10-07 19:44 ` [PATCH bpf-next v4 10/12] samples/bpf: do not FORCE-recompile libbpf Quentin Monnet
@ 2021-10-07 19:44 ` Quentin Monnet
  2021-10-07 19:44 ` [PATCH bpf-next v4 12/12] bpftool: add install-bin target to install binary only Quentin Monnet
  2021-10-08 19:13 ` [PATCH bpf-next v4 00/12] install libbpf headers when using the library Andrii Nakryiko
  12 siblings, 0 replies; 18+ messages in thread
From: Quentin Monnet @ 2021-10-07 19:44 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: netdev, bpf, Quentin Monnet

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 <quentin@isovalent.com>
---
 tools/testing/selftests/bpf/test_bpftool_build.sh | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/testing/selftests/bpf/test_bpftool_build.sh b/tools/testing/selftests/bpf/test_bpftool_build.sh
index b03a87571592..1453a53ed547 100755
--- a/tools/testing/selftests/bpf/test_bpftool_build.sh
+++ b/tools/testing/selftests/bpf/test_bpftool_build.sh
@@ -90,6 +90,10 @@ echo -e "... through kbuild\n"
 
 if [ -f ".config" ] ; then
 	make_and_clean tools/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 -C tools/bpf/runqslower OUTPUT=${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.
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH bpf-next v4 12/12] bpftool: add install-bin target to install binary only
  2021-10-07 19:44 [PATCH bpf-next v4 00/12] install libbpf headers when using the library Quentin Monnet
                   ` (10 preceding siblings ...)
  2021-10-07 19:44 ` [PATCH bpf-next v4 11/12] selftests/bpf: better clean up for runqslower in test_bpftool_build.sh Quentin Monnet
@ 2021-10-07 19:44 ` Quentin Monnet
  2021-10-08 19:13 ` [PATCH bpf-next v4 00/12] install libbpf headers when using the library Andrii Nakryiko
  12 siblings, 0 replies; 18+ messages in thread
From: Quentin Monnet @ 2021-10-07 19:44 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: netdev, bpf, Quentin Monnet

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 <quentin@isovalent.com>
---
 kernel/bpf/preload/iterators/Makefile | 2 +-
 tools/bpf/bpftool/Makefile            | 6 ++++--
 tools/testing/selftests/bpf/Makefile  | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/kernel/bpf/preload/iterators/Makefile b/kernel/bpf/preload/iterators/Makefile
index ec39ccc71b8e..616a7ec0232c 100644
--- a/kernel/bpf/preload/iterators/Makefile
+++ b/kernel/bpf/preload/iterators/Makefile
@@ -67,4 +67,4 @@ $(DEFAULT_BPFTOOL): $(BPFOBJ) | $(BPFTOOL_OUTPUT)
 		    OUTPUT=$(BPFTOOL_OUTPUT)/				       \
 		    LIBBPF_OUTPUT=$(LIBBPF_OUTPUT)/			       \
 		    LIBBPF_DESTDIR=$(LIBBPF_DESTDIR)/			       \
-		    prefix= DESTDIR=$(abs_out)/ install
+		    prefix= DESTDIR=$(abs_out)/ install-bin
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index ba02d71c39ef..9c2d13c513f0 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -226,10 +226,12 @@ clean: $(LIBBPF)-clean $(LIBBPF_BOOTSTRAP)-clean feature-detect-clean
 	$(Q)$(RM) -- $(OUTPUT)FEATURE-DUMP.bpftool
 	$(Q)$(RM) -r -- $(OUTPUT)feature/
 
-install: $(OUTPUT)bpftool
+install-bin: $(OUTPUT)bpftool
 	$(call QUIET_INSTALL, bpftool)
 	$(Q)$(INSTALL) -m 0755 -d $(DESTDIR)$(prefix)/sbin
 	$(Q)$(INSTALL) $(OUTPUT)bpftool $(DESTDIR)$(prefix)/sbin/bpftool
+
+install: install-bin
 	$(Q)$(INSTALL) -m 0755 -d $(DESTDIR)$(bash_compdir)
 	$(Q)$(INSTALL) -m 0644 bash-completion/bpftool $(DESTDIR)$(bash_compdir)
 
@@ -256,6 +258,6 @@ zdep:
 	@if [ "$(feature-zlib)" != "1" ]; then echo "No zlib found"; exit 1 ; fi
 
 .SECONDARY:
-.PHONY: all FORCE clean install uninstall zdep
+.PHONY: all FORCE clean install-bin install uninstall zdep
 .PHONY: doc doc-clean doc-install doc-uninstall
 .DEFAULT_GOAL := all
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index e023d734f7b0..498222543c37 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -216,7 +216,7 @@ $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile)    \
 		    OUTPUT=$(HOST_BUILD_DIR)/bpftool/			       \
 		    LIBBPF_OUTPUT=$(HOST_BUILD_DIR)/libbpf/		       \
 		    LIBBPF_DESTDIR=$(HOST_SCRATCH_DIR)/			       \
-		    prefix= DESTDIR=$(HOST_SCRATCH_DIR)/ install
+		    prefix= DESTDIR=$(HOST_SCRATCH_DIR)/ install-bin
 
 all: docs
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH bpf-next v4 01/12] libbpf: skip re-installing headers file if source is older than target
  2021-10-07 19:44 ` [PATCH bpf-next v4 01/12] libbpf: skip re-installing headers file if source is older than target Quentin Monnet
@ 2021-10-08 19:13   ` Andrii Nakryiko
  0 siblings, 0 replies; 18+ messages in thread
From: Andrii Nakryiko @ 2021-10-08 19:13 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking, bpf

On Thu, Oct 7, 2021 at 12:44 PM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 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 <quentin@isovalent.com>
> ---
>  tools/lib/bpf/Makefile | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> index 41e4f78dbad5..a92d3b9692a8 100644
> --- a/tools/lib/bpf/Makefile
> +++ b/tools/lib/bpf/Makefile
> @@ -241,15 +241,23 @@ install_lib: all_cmd
>                 $(call do_install_mkdir,$(libdir_SQ)); \
>                 cp -fpR $(LIB_FILE) $(DESTDIR)$(libdir_SQ)
>
> -INSTALL_HEADERS = bpf.h libbpf.h btf.h libbpf_common.h libbpf_legacy.h xsk.h \
> -                 bpf_helpers.h $(BPF_GENERATED) bpf_tracing.h               \
> -                 bpf_endian.h bpf_core_read.h skel_internal.h               \
> -                 libbpf_version.h
> +SRC_HDRS := bpf.h libbpf.h btf.h libbpf_common.h libbpf_legacy.h xsk.h        \
> +           bpf_helpers.h bpf_tracing.h bpf_endian.h bpf_core_read.h           \
> +           skel_internal.h libbpf_version.h
> +GEN_HDRS := $(BPF_GENERATED)
> +INSTALL_SRC_HDRS := $(addprefix $(DESTDIR)$(prefix)/include/bpf/,$(SRC_HDRS))
> +INSTALL_GEN_HDRS := $(addprefix $(DESTDIR)$(prefix)/include/bpf/, \
> +                               $(notdir $(GEN_HDRS)))

I've added INSTALL_PFX := $(DESTDIR)$(prefix)/include/bpf and used it
where possible

> +$(INSTALL_SRC_HDRS): $(DESTDIR)$(prefix)/include/bpf/%.h: %.h
> +       $(call QUIET_INSTALL, $@) \
> +               $(call do_install,$<,$(prefix)/include/bpf,644)
> +$(INSTALL_GEN_HDRS): $(DESTDIR)$(prefix)/include/bpf/%.h: $(OUTPUT)%.h
> +       $(call QUIET_INSTALL, $@) \
> +               $(call do_install,$<,$(prefix)/include/bpf,644)
>
> -install_headers: $(BPF_GENERATED)
> -       $(call QUIET_INSTALL, headers)                                       \
> -               $(foreach hdr,$(INSTALL_HEADERS),                            \
> -                       $(call do_install,$(hdr),$(prefix)/include/bpf,644);)
> +INSTALL_HEADERS := $(INSTALL_SRC_HDRS) $(INSTALL_GEN_HDRS)

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


> +
> +install_headers: $(BPF_GENERATED) $(INSTALL_HEADERS)
>
>  install_pkgconfig: $(PC_FILE)
>         $(call QUIET_INSTALL, $(PC_FILE)) \
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH bpf-next v4 03/12] bpftool: install libbpf headers instead of including the dir
  2021-10-07 19:44 ` [PATCH bpf-next v4 03/12] bpftool: install libbpf headers instead of including the dir Quentin Monnet
@ 2021-10-08 19:13   ` Andrii Nakryiko
  0 siblings, 0 replies; 18+ messages in thread
From: Andrii Nakryiko @ 2021-10-08 19:13 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking, bpf

On Thu, Oct 7, 2021 at 12:44 PM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 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 <quentin@isovalent.com>
> 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(-)
>
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index 1fcf5b01a193..ba02d71c39ef 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -17,19 +17,23 @@ endif
>  BPF_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.a
>  LIBBPF_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 := $(shell make -rR --no-print-directory -sC ../../.. kernelversion)
>  endif
> @@ -38,7 +42,13 @@ $(LIBBPF_OUTPUT) $(BOOTSTRAP_OUTPUT) $(LIBBPF_BOOTSTRAP_OUTPUT):
>         $(QUIET_MKDIR)mkdir -p $@
>
>  $(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
> +
> +$(LIBBPF_INCLUDE)/bpf/$(LIBBPF_INTERNAL_HDRS): \

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)

> +               $(addprefix $(BPF_DIR),$(LIBBPF_INTERNAL_HDRS)) $(LIBBPF)
> +       $(call QUIET_INSTALL, bpf/$(notdir $@))
> +       $(Q)install -m 644 -t $(LIBBPF_INCLUDE)/bpf/ $(BPF_DIR)$(notdir $@)
>
>  $(LIBBPF_BOOTSTRAP): FORCE | $(LIBBPF_BOOTSTRAP_OUTPUT)
>         $(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(LIBBPF_BOOTSTRAP_OUTPUT) \
> @@ -60,10 +70,10 @@ CFLAGS += -W -Wall -Wextra -Wno-unused-parameter -Wno-missing-field-initializers
>  CFLAGS += $(filter-out -Wswitch-enum -Wnested-externs,$(EXTRA_WARNINGS))
>  CFLAGS += -DPACKAGE='"bpftool"' -D__EXPORTED_HEADERS__ \
>         -I$(if $(OUTPUT),$(OUTPUT),.) \
> +       -I$(LIBBPF_INCLUDE) \
>         -I$(srctree)/kernel/bpf/ \
>         -I$(srctree)/tools/include \
>         -I$(srctree)/tools/include/uapi \
> -       -I$(srctree)/tools/lib \
>         -I$(srctree)/tools/perf
>  CFLAGS += -DBPFTOOL_VERSION='"$(BPFTOOL_VERSION)"'
>  ifneq ($(EXTRA_CFLAGS),)
> @@ -140,7 +150,7 @@ BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o g
>  $(BOOTSTRAP_OBJS): $(LIBBPF_BOOTSTRAP)
>
>  OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
> -$(OBJS): $(LIBBPF)
> +$(OBJS): $(LIBBPF) $(LIBBPF_INCLUDE)/bpf/$(LIBBPF_INTERNAL_HDRS)

the whole $(LIBBPF_INCLUDE)/bpf/$(LIBBPF_INTERNAL_HDRS) use in
multiple places might benefit from having a dedicated variable


>
>  VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux)                           \
>                      $(if $(KBUILD_OUTPUT),$(KBUILD_OUTPUT)/vmlinux)    \
> @@ -167,8 +177,7 @@ $(OUTPUT)%.bpf.o: skeleton/%.bpf.c $(OUTPUT)vmlinux.h $(LIBBPF)
>         $(QUIET_CLANG)$(CLANG) \
>                 -I$(if $(OUTPUT),$(OUTPUT),.) \
>                 -I$(srctree)/tools/include/uapi/ \
> -               -I$(LIBBPF_PATH) \
> -               -I$(srctree)/tools/lib \
> +               -I$(LIBBPF_INCLUDE) \
>                 -g -O2 -Wall -target bpf -c $< -o $@ && $(LLVM_STRIP) -g $@
>
>  $(OUTPUT)%.skel.h: $(OUTPUT)%.bpf.o $(BPFTOOL_BOOTSTRAP)
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index c5c9a9f50d8d..849a4637f59d 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -209,6 +209,8 @@ $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile)    \
>                     CC=$(HOSTCC) LD=$(HOSTLD)                                  \
>                     EXTRA_CFLAGS='-g -O0'                                      \
>                     OUTPUT=$(HOST_BUILD_DIR)/bpftool/                          \
> +                   LIBBPF_OUTPUT=$(HOST_BUILD_DIR)/libbpf/                    \
> +                   LIBBPF_DESTDIR=$(HOST_SCRATCH_DIR)/                        \
>                     prefix= DESTDIR=$(HOST_SCRATCH_DIR)/ install
>
>  all: docs
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH bpf-next v4 00/12] install libbpf headers when using the library
  2021-10-07 19:44 [PATCH bpf-next v4 00/12] install libbpf headers when using the library Quentin Monnet
                   ` (11 preceding siblings ...)
  2021-10-07 19:44 ` [PATCH bpf-next v4 12/12] bpftool: add install-bin target to install binary only Quentin Monnet
@ 2021-10-08 19:13 ` Andrii Nakryiko
  2021-10-09 21:03   ` Quentin Monnet
  12 siblings, 1 reply; 18+ messages in thread
From: Andrii Nakryiko @ 2021-10-08 19:13 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking, bpf

On Thu, Oct 7, 2021 at 12:44 PM Quentin Monnet <quentin@isovalent.com> 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!

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH bpf-next v4 00/12] install libbpf headers when using the library
  2021-10-08 19:13 ` [PATCH bpf-next v4 00/12] install libbpf headers when using the library Andrii Nakryiko
@ 2021-10-09 21:03   ` Quentin Monnet
  2021-10-11  6:46     ` Andrii Nakryiko
  0 siblings, 1 reply; 18+ messages in thread
From: Quentin Monnet @ 2021-10-09 21:03 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking, bpf

On Fri, 8 Oct 2021 at 20:13, Andrii Nakryiko <andrii.nakryiko@gmail.com> 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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH bpf-next v4 00/12] install libbpf headers when using the library
  2021-10-09 21:03   ` Quentin Monnet
@ 2021-10-11  6:46     ` Andrii Nakryiko
  0 siblings, 0 replies; 18+ messages in thread
From: Andrii Nakryiko @ 2021-10-11  6:46 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking, bpf

On Sat, Oct 9, 2021 at 11:03 PM Quentin Monnet <quentin@isovalent.com> wrote:
>
> On Fri, 8 Oct 2021 at 20:13, Andrii Nakryiko <andrii.nakryiko@gmail.com> 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.

thanks

>
> >
> > 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.

>
> >
> > 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.

>
> >
> > 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)

>
> Thanks,
> Quentin

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-10-11  6:46 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-07 19:44 [PATCH bpf-next v4 00/12] install libbpf headers when using the library Quentin Monnet
2021-10-07 19:44 ` [PATCH bpf-next v4 01/12] libbpf: skip re-installing headers file if source is older than target Quentin Monnet
2021-10-08 19:13   ` Andrii Nakryiko
2021-10-07 19:44 ` [PATCH bpf-next v4 02/12] bpftool: remove unused includes to <bpf/bpf_gen_internal.h> Quentin Monnet
2021-10-07 19:44 ` [PATCH bpf-next v4 03/12] bpftool: install libbpf headers instead of including the dir Quentin Monnet
2021-10-08 19:13   ` Andrii Nakryiko
2021-10-07 19:44 ` [PATCH bpf-next v4 04/12] tools/resolve_btfids: install libbpf headers when building Quentin Monnet
2021-10-07 19:44 ` [PATCH bpf-next v4 05/12] tools/runqslower: " Quentin Monnet
2021-10-07 19:44 ` [PATCH bpf-next v4 06/12] bpf: preload: " Quentin Monnet
2021-10-07 19:44 ` [PATCH bpf-next v4 07/12] bpf: iterators: " Quentin Monnet
2021-10-07 19:44 ` [PATCH bpf-next v4 08/12] samples/bpf: update .gitignore Quentin Monnet
2021-10-07 19:44 ` [PATCH bpf-next v4 09/12] samples/bpf: install libbpf headers when building Quentin Monnet
2021-10-07 19:44 ` [PATCH bpf-next v4 10/12] samples/bpf: do not FORCE-recompile libbpf Quentin Monnet
2021-10-07 19:44 ` [PATCH bpf-next v4 11/12] selftests/bpf: better clean up for runqslower in test_bpftool_build.sh Quentin Monnet
2021-10-07 19:44 ` [PATCH bpf-next v4 12/12] bpftool: add install-bin target to install binary only Quentin Monnet
2021-10-08 19:13 ` [PATCH bpf-next v4 00/12] install libbpf headers when using the library Andrii Nakryiko
2021-10-09 21:03   ` Quentin Monnet
2021-10-11  6:46     ` Andrii Nakryiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).