From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Quentin Monnet <quentin@isovalent.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Andrii Nakryiko <andrii@kernel.org>,
Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>
Subject: Re: [PATCH bpf-next v4 03/12] bpftool: install libbpf headers instead of including the dir
Date: Fri, 8 Oct 2021 12:13:22 -0700 [thread overview]
Message-ID: <CAEf4BzZF2Tmxr3peNQ6AgpXTjTs+g8U2aYw7pR+uMxXNETMTtQ@mail.gmail.com> (raw)
In-Reply-To: <20211007194438.34443-4-quentin@isovalent.com>
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
>
next prev parent reply other threads:[~2021-10-08 19:13 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAEf4BzZF2Tmxr3peNQ6AgpXTjTs+g8U2aYw7pR+uMxXNETMTtQ@mail.gmail.com \
--to=andrii.nakryiko@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=netdev@vger.kernel.org \
--cc=quentin@isovalent.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.