Commit fe4eb069edb7 ("bpftool: Use linux/types.h from source tree for profiler build") added a build dependency on tools/testing/selftests/bpf to tools/bpf/bpftool. This is suboptimal with respect to a possible stand-alone build of bpftool.
Fix this by moving tools/testing/selftests/bpf/include/uapi/linux/types.h to tools/include/uapi/linux/types.h
This requires an adjustment in the include search path order for the tests in tools/testing/selftests/bpf so that tools/include/linux/types.h is selected when building host binaries and tools/include/uapi/linux/types.h is selected when building bpf binaries.
Verified by compiling bpftool and the bpf selftests on x86_64 with this change.
Fixes: fe4eb069edb7 ("bpftool: Use linux/types.h from source tree for profiler build") Cc: Daniel Borkmann daniel@iogearbox.net Cc: Quentin Monnet quentin@isovalent.com Suggested-by: Andrii Nakryiko andrii.nakryiko@gmail.com Signed-off-by: Tobias Klauser tklauser@distanz.ch --- tools/bpf/bpftool/Makefile | 1 - .../{testing/selftests/bpf => }/include/uapi/linux/types.h | 0 tools/testing/selftests/bpf/Makefile | 7 ++++--- 3 files changed, 4 insertions(+), 4 deletions(-) rename tools/{testing/selftests/bpf => }/include/uapi/linux/types.h (100%)
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile index 9ca3bfbb9ac4..f584d1fdfc64 100644 --- a/tools/bpf/bpftool/Makefile +++ b/tools/bpf/bpftool/Makefile @@ -129,7 +129,6 @@ $(OUTPUT)_bpftool: $(_OBJS) $(LIBBPF) skeleton/profiler.bpf.o: skeleton/profiler.bpf.c $(LIBBPF) $(QUIET_CLANG)$(CLANG) \ -I$(srctree)/tools/include/uapi/ \ - -I$(srctree)/tools/testing/selftests/bpf/include/uapi \ -I$(LIBBPF_PATH) -I$(srctree)/tools/lib \ -g -O2 -target bpf -c $< -o $@
diff --git a/tools/testing/selftests/bpf/include/uapi/linux/types.h b/tools/include/uapi/linux/types.h similarity index 100% rename from tools/testing/selftests/bpf/include/uapi/linux/types.h rename to tools/include/uapi/linux/types.h diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index da4389dde9f7..074a05efd1ca 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -20,8 +20,9 @@ CLANG ?= clang LLC ?= llc LLVM_OBJCOPY ?= llvm-objcopy BPF_GCC ?= $(shell command -v bpf-gcc;) -CFLAGS += -g -rdynamic -Wall -O2 $(GENFLAGS) -I$(CURDIR) -I$(APIDIR) \ +CFLAGS += -g -rdynamic -Wall -O2 $(GENFLAGS) -I$(CURDIR) \ -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) -I$(TOOLSINCDIR) \ + -I$(APIDIR) \ -Dbpf_prog_load=bpf_prog_test_load \ -Dbpf_load_program=bpf_test_load_program LDLIBS += -lcap -lelf -lz -lrt -lpthread @@ -194,8 +195,8 @@ MENDIAN=$(if $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian)
CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG)) BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) \ - -I$(INCLUDE_DIR) -I$(CURDIR) -I$(CURDIR)/include/uapi \ - -I$(APIDIR) -I$(abspath $(OUTPUT)/../usr/include) + -I$(INCLUDE_DIR) -I$(CURDIR) -I$(APIDIR) \ + -I$(abspath $(OUTPUT)/../usr/include)
CLANG_CFLAGS = $(CLANG_SYS_INCLUDES) \ -Wno-compare-distinct-pointer-types
2020-03-13 12:31 UTC+0100 ~ Tobias Klauser tklauser@distanz.ch
Commit fe4eb069edb7 ("bpftool: Use linux/types.h from source tree for profiler build") added a build dependency on tools/testing/selftests/bpf to tools/bpf/bpftool. This is suboptimal with respect to a possible stand-alone build of bpftool.
Fix this by moving tools/testing/selftests/bpf/include/uapi/linux/types.h to tools/include/uapi/linux/types.h
This requires an adjustment in the include search path order for the tests in tools/testing/selftests/bpf so that tools/include/linux/types.h is selected when building host binaries and tools/include/uapi/linux/types.h is selected when building bpf binaries.
Verified by compiling bpftool and the bpf selftests on x86_64 with this change.
Compiles on my setup too.
Reviewed-by: Quentin Monnet quentin@isovalent.com
On Fri, Mar 13, 2020 at 4:31 AM Tobias Klauser tklauser@distanz.ch wrote:
Commit fe4eb069edb7 ("bpftool: Use linux/types.h from source tree for profiler build") added a build dependency on tools/testing/selftests/bpf to tools/bpf/bpftool. This is suboptimal with respect to a possible stand-alone build of bpftool.
Fix this by moving tools/testing/selftests/bpf/include/uapi/linux/types.h to tools/include/uapi/linux/types.h
This requires an adjustment in the include search path order for the tests in tools/testing/selftests/bpf so that tools/include/linux/types.h is selected when building host binaries and tools/include/uapi/linux/types.h is selected when building bpf binaries.
Verified by compiling bpftool and the bpf selftests on x86_64 with this change.
Thanks for following up!
My only concern is that tools/include/uapi/ is also used at least by perf and libperf, we need to double check that they are fine with this as well.
Given this is needed for BPF target compilation only, one way to limit the scope of this change would be to have a `#if defined(__bpf__)` check and falling back to "normal" uapi/linux/types.h. Alternatively, we could have a bpf-specific subdirectory and put this header into tools/include/bpf/uapi/linux/types.h.
I don't have any strong preferences, whatever maintainers are happy with.
Fixes: fe4eb069edb7 ("bpftool: Use linux/types.h from source tree for profiler build") Cc: Daniel Borkmann daniel@iogearbox.net Cc: Quentin Monnet quentin@isovalent.com Suggested-by: Andrii Nakryiko andrii.nakryiko@gmail.com Signed-off-by: Tobias Klauser tklauser@distanz.ch
tools/bpf/bpftool/Makefile | 1 - .../{testing/selftests/bpf => }/include/uapi/linux/types.h | 0 tools/testing/selftests/bpf/Makefile | 7 ++++--- 3 files changed, 4 insertions(+), 4 deletions(-) rename tools/{testing/selftests/bpf => }/include/uapi/linux/types.h (100%)
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile index 9ca3bfbb9ac4..f584d1fdfc64 100644 --- a/tools/bpf/bpftool/Makefile +++ b/tools/bpf/bpftool/Makefile @@ -129,7 +129,6 @@ $(OUTPUT)_bpftool: $(_OBJS) $(LIBBPF) skeleton/profiler.bpf.o: skeleton/profiler.bpf.c $(LIBBPF) $(QUIET_CLANG)$(CLANG) \ -I$(srctree)/tools/include/uapi/ \
-I$(srctree)/tools/testing/selftests/bpf/include/uapi \ -I$(LIBBPF_PATH) -I$(srctree)/tools/lib \ -g -O2 -target bpf -c $< -o $@
diff --git a/tools/testing/selftests/bpf/include/uapi/linux/types.h b/tools/include/uapi/linux/types.h similarity index 100% rename from tools/testing/selftests/bpf/include/uapi/linux/types.h rename to tools/include/uapi/linux/types.h diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index da4389dde9f7..074a05efd1ca 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -20,8 +20,9 @@ CLANG ?= clang LLC ?= llc LLVM_OBJCOPY ?= llvm-objcopy BPF_GCC ?= $(shell command -v bpf-gcc;) -CFLAGS += -g -rdynamic -Wall -O2 $(GENFLAGS) -I$(CURDIR) -I$(APIDIR) \ +CFLAGS += -g -rdynamic -Wall -O2 $(GENFLAGS) -I$(CURDIR) \ -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) -I$(TOOLSINCDIR) \
-I$(APIDIR) \ -Dbpf_prog_load=bpf_prog_test_load \ -Dbpf_load_program=bpf_test_load_program
LDLIBS += -lcap -lelf -lz -lrt -lpthread @@ -194,8 +195,8 @@ MENDIAN=$(if $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian)
CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG)) BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) \
-I$(INCLUDE_DIR) -I$(CURDIR) -I$(CURDIR)/include/uapi \
-I$(APIDIR) -I$(abspath $(OUTPUT)/../usr/include)
-I$(INCLUDE_DIR) -I$(CURDIR) -I$(APIDIR) \
-I$(abspath $(OUTPUT)/../usr/include)
CLANG_CFLAGS = $(CLANG_SYS_INCLUDES) \ -Wno-compare-distinct-pointer-types -- 2.24.0
[ +acme ]
On 3/13/20 5:52 PM, Andrii Nakryiko wrote:
On Fri, Mar 13, 2020 at 4:31 AM Tobias Klauser tklauser@distanz.ch wrote:
Commit fe4eb069edb7 ("bpftool: Use linux/types.h from source tree for profiler build") added a build dependency on tools/testing/selftests/bpf to tools/bpf/bpftool. This is suboptimal with respect to a possible stand-alone build of bpftool.
Fix this by moving tools/testing/selftests/bpf/include/uapi/linux/types.h to tools/include/uapi/linux/types.h
This requires an adjustment in the include search path order for the tests in tools/testing/selftests/bpf so that tools/include/linux/types.h is selected when building host binaries and tools/include/uapi/linux/types.h is selected when building bpf binaries.
Verified by compiling bpftool and the bpf selftests on x86_64 with this change.
Thanks for following up!
My only concern is that tools/include/uapi/ is also used at least by perf and libperf, we need to double check that they are fine with this as well.
Given this is needed for BPF target compilation only, one way to limit the scope of this change would be to have a `#if defined(__bpf__)` check and falling back to "normal" uapi/linux/types.h. Alternatively, we could have a bpf-specific subdirectory and put this header into tools/include/bpf/uapi/linux/types.h.
I don't have any strong preferences, whatever maintainers are happy with.
I would prefer to keep it generic if possible before we take measures of making special cases for bpf in tools include infra. Compilation of perf and libperf seems fine on my side as well, so I've applied it, thanks!
linux-kselftest-mirror@lists.linaro.org