| Message ID | 20191016060051.2024182-6-andriin@fb.com |
|---|---|
| State | Accepted |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | Fix, clean up, and revamp selftests/bpf Makefile | expand |
On 10/15, Andrii Nakryiko wrote: > Define test runner generation meta-rule that codifies dependencies > between test runner, its tests, and its dependent BPF programs. Use that > for defining test_progs and test_maps test-runners. Also additionally define > 2 flavors of test_progs: > - alu32, which builds BPF programs with 32-bit registers codegen; > - bpf_gcc, which build BPF programs using GCC, if it supports BPF target. Question: Why not merge test_maps tests into test_progs framework and have a single binary instead of doing all this makefile-related work? We can independently address the story with alu32/gcc progs (presumably in the same manner, with make defines). I can hardly follow the existing makefile and now with the evals it's 10x more complicated for no good reason. > Overall, this is accomplished through $(eval)'ing a set of generic > rules, which defines Makefile targets dynamically at runtime. See > comments explaining the need for 2 $(evals), though. > > For each test runner we have (test_maps and test_progs, currently), and, > optionally, their flavors, the logic of build process is modeled as > follows (using test_progs as an example): > - all BPF objects are in progs/: > - BPF object's .o file is built into output directory from > corresponding progs/.c file; > - all BPF objects in progs/*.c depend on all progs/*.h headers; > - all BPF objects depend on bpf_*.h helpers from libbpf (but not > libbpf archive). There is an extra rule to trigger bpf_helper_defs.h > (re-)build, if it's not present/outdated); > - build recipe for BPF object can be re-defined per test runner/flavor; > - test files are built from prog_tests/*.c: > - all such test file objects are built on individual file basis; > - currently, every single test file depends on all BPF object files; > this might be improved in follow up patches to do 1-to-1 dependency, > but allowing to customize this per each individual test; > - each test runner definition can specify a list of extra .c and .h > files to be built along test files and test runner binary; all such > headers are becoming automatic dependency of each test .c file; > - due to test files sometimes embedding (using .incbin assembly > directive) contents of some BPF objects at compilation time, which are > expected to be in CWD of compiler, compilation for test file object does > cd into test runner's output directory; to support this mode all the > include paths are turned into absolute paths using $(abspath) make > function; > - prog_tests/test.h is automatically (re-)generated with an entry for > each .c file in prog_tests/; > - final test runner binary is linked together from test object files and > extra object files, linking together libbpf's archive as well; > - it's possible to specify extra "resource" files/targets, which will be > copied into test runner output directory, if it differes from > Makefile-wide $(OUTPUT). This is used to ensure btf_dump test cases and > urandom_read binary is put into a test runner's CWD for tests to find > them in runtime. > > For flavored test runners, their output directory is a subdirectory of > common Makefile-wide $(OUTPUT) directory with flavor name used as > subdirectory name. > > BPF objects targets might be reused between different test runners, so > extra checks are employed to not double-define them. Similarly, we have > redefinition guards for output directories and test headers. > > test_verifier follows slightly different patterns and is simple enough > to not justify generalizing TEST_RUNNER_DEFINE/TEST_RUNNER_DEFINE_RULES > further to accomodate these differences. Instead, rules for > test_verifier are minimized and simplified, while preserving correctness > of dependencies. > > Signed-off-by: Andrii Nakryiko <andriin@fb.com> > --- > tools/testing/selftests/bpf/.gitignore | 5 +- > tools/testing/selftests/bpf/Makefile | 313 ++++++++++++++----------- > 2 files changed, 180 insertions(+), 138 deletions(-) > > diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore > index 7470327edcfe..c51f356f84b5 100644 > --- a/tools/testing/selftests/bpf/.gitignore > +++ b/tools/testing/selftests/bpf/.gitignore > @@ -7,7 +7,7 @@ FEATURE-DUMP.libbpf > fixdep > test_align > test_dev_cgroup > -test_progs > +/test_progs* > test_tcpbpf_user > test_verifier_log > feature > @@ -33,9 +33,10 @@ test_tcpnotify_user > test_libbpf > test_tcp_check_syncookie_user > test_sysctl > -alu32 > libbpf.pc > libbpf.so.* > test_hashmap > test_btf_dump > xdping > +/alu32 > +/bpf_gcc > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > index fbced23935cc..c9f43d49eac9 100644 > --- a/tools/testing/selftests/bpf/Makefile > +++ b/tools/testing/selftests/bpf/Makefile > @@ -2,10 +2,12 @@ > include ../../../../scripts/Kbuild.include > include ../../../scripts/Makefile.arch > > -LIBDIR := ../../../lib > +CURDIR := $(abspath .) > +LIBDIR := $(abspath ../../../lib) > BPFDIR := $(LIBDIR)/bpf > -APIDIR := ../../../include/uapi > -GENDIR := ../../../../include/generated > +TOOLSDIR := $(abspath ../../../include) > +APIDIR := $(TOOLSDIR)/uapi > +GENDIR := $(abspath ../../../../include/generated) > GENHDR := $(GENDIR)/autoconf.h > > ifneq ($(wildcard $(GENHDR)),) > @@ -16,8 +18,9 @@ CLANG ?= clang > LLC ?= llc > LLVM_OBJCOPY ?= llvm-objcopy > BPF_GCC ?= $(shell command -v bpf-gcc;) > -CFLAGS += -g -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(BPFDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include \ > - -Dbpf_prog_load=bpf_prog_test_load \ > +CFLAGS += -g -Wall -O2 $(GENFLAGS) -I$(APIDIR) -I$(LIBDIR) -I$(BPFDIR) \ > + -I$(GENDIR) -I$(TOOLSDIR) -I$(CURDIR) \ > + -Dbpf_prog_load=bpf_prog_test_load \ > -Dbpf_load_program=bpf_test_load_program > LDLIBS += -lcap -lelf -lrt -lpthread > > @@ -29,12 +32,6 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test > test_netcnt test_tcpnotify_user test_sock_fields test_sysctl test_hashmap \ > test_cgroup_attach xdping > > -BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c))) > -TEST_GEN_FILES = $(BPF_OBJ_FILES) > - > -BTF_C_FILES = $(wildcard progs/btf_dump_test_case_*.c) > -TEST_FILES = $(BTF_C_FILES) > - > # Also test sub-register code-gen if LLVM has eBPF v3 processor support which > # contains both ALU32 and JMP32 instructions. > SUBREG_CODEGEN := $(shell echo "int cal(int a) { return a > 0; }" | \ > @@ -42,13 +39,17 @@ SUBREG_CODEGEN := $(shell echo "int cal(int a) { return a > 0; }" | \ > $(LLC) -mattr=+alu32 -mcpu=v3 2>&1 | \ > grep 'if w') > ifneq ($(SUBREG_CODEGEN),) > -TEST_GEN_FILES += $(patsubst %.o,alu32/%.o, $(BPF_OBJ_FILES)) > +TEST_GEN_PROGS += test_progs-alu32 > endif > > +# Also test bpf-gcc, if present > ifneq ($(BPF_GCC),) > -TEST_GEN_FILES += $(patsubst %.o,bpf_gcc/%.o, $(BPF_OBJ_FILES)) > +TEST_GEN_PROGS += test_progs-bpf_gcc > endif > > +TEST_GEN_FILES = > +TEST_FILES = > + > # Order correspond to 'make run_tests' order > TEST_PROGS := test_kmod.sh \ > test_libbpf.sh \ > @@ -82,6 +83,8 @@ TEST_GEN_PROGS_EXTENDED = test_libbpf_open test_sock_addr test_skb_cgroup_id_use > flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \ > test_lirc_mode2_user > > +TEST_CUSTOM_PROGS = urandom_read > + > include ../lib.mk > > # Define simple and short `make test_progs`, `make test_sysctl`, etc targets > @@ -94,21 +97,12 @@ $(notdir $(TEST_GEN_PROGS) \ > $(TEST_GEN_PROGS_EXTENDED) \ > $(TEST_CUSTOM_PROGS)): %: $(OUTPUT)/% ; > > -# NOTE: $(OUTPUT) won't get default value if used before lib.mk > -TEST_CUSTOM_PROGS = $(OUTPUT)/urandom_read > -all: $(TEST_CUSTOM_PROGS) > - > -$(OUTPUT)/urandom_read: $(OUTPUT)/%: %.c > +$(OUTPUT)/urandom_read: urandom_read.c > $(CC) -o $@ $< -Wl,--build-id > > -$(OUTPUT)/test_stub.o: test_stub.c > - $(CC) $(TEST_PROGS_CFLAGS) $(CFLAGS) -c -o $@ $< > - > BPFOBJ := $(OUTPUT)/libbpf.a > > -$(TEST_GEN_PROGS): $(OUTPUT)/test_stub.o $(BPFOBJ) > - > -$(TEST_GEN_PROGS_EXTENDED): $(OUTPUT)/test_stub.o $(OUTPUT)/libbpf.a > +$(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED): $(OUTPUT)/test_stub.o $(BPFOBJ) > > $(OUTPUT)/test_dev_cgroup: cgroup_helpers.c > $(OUTPUT)/test_skb_cgroup_id_user: cgroup_helpers.c > @@ -118,7 +112,6 @@ $(OUTPUT)/test_socket_cookie: cgroup_helpers.c > $(OUTPUT)/test_sockmap: cgroup_helpers.c > $(OUTPUT)/test_tcpbpf_user: cgroup_helpers.c > $(OUTPUT)/test_tcpnotify_user: cgroup_helpers.c trace_helpers.c > -$(OUTPUT)/test_progs: cgroup_helpers.c trace_helpers.c > $(OUTPUT)/get_cgroup_id_user: cgroup_helpers.c > $(OUTPUT)/test_cgroup_storage: cgroup_helpers.c > $(OUTPUT)/test_netcnt: cgroup_helpers.c > @@ -134,6 +127,10 @@ force: > $(BPFOBJ): force > $(MAKE) -C $(BPFDIR) OUTPUT=$(OUTPUT)/ > > +BPF_HELPERS := $(BPFDIR)/bpf_helper_defs.h $(wildcard $(BPFDIR)/bpf_*.h) > +$(BPFDIR)/bpf_helper_defs.h: > + $(MAKE) -C $(BPFDIR) OUTPUT=$(OUTPUT)/ bpf_helper_defs.h > + > # Get Clang's default includes on this system, as opposed to those seen by > # '-target bpf'. This fixes "missing" files on some architectures/distros, > # such as asm/byteorder.h, asm/socket.h, asm/sockios.h, sys/cdefs.h etc. > @@ -144,10 +141,11 @@ define get_sys_includes > $(shell $(1) -v -E - </dev/null 2>&1 \ > | sed -n '/<...> search starts here:/,/End of search list./{ s| \(/.*\)|-idirafter \1|p }') > endef > + > CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG)) > BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH) \ > - -I. -I./include/uapi -I../../../include/uapi \ > - -I$(BPFDIR) -I$(OUTPUT)/../usr/include > + -I. -I./include/uapi -I$(APIDIR) \ > + -I$(BPFDIR) -I$(abspath $(OUTPUT)/../usr/include) > > CLANG_CFLAGS = $(CLANG_SYS_INCLUDES) \ > -Wno-compare-distinct-pointer-types > @@ -159,127 +157,170 @@ $(OUTPUT)/test_queue_map.o: test_queue_stack_map.h > $(OUTPUT)/test_stack_map.o: test_queue_stack_map.h > > $(OUTPUT)/flow_dissector_load.o: flow_dissector_load.h > -$(OUTPUT)/test_progs.o: flow_dissector_load.h > > -TEST_PROGS_CFLAGS := -I. -I$(OUTPUT) > -TEST_MAPS_CFLAGS := -I. -I$(OUTPUT) > -TEST_VERIFIER_CFLAGS := -I. -I$(OUTPUT) -Iverifier > +# Build BPF object using Clang > +# $1 - input .c file > +# $2 - output .o file > +# $3 - CFLAGS > +# $4 - LDFLAGS > +define CLANG_BPF_BUILD_RULE > + ($(CLANG) $3 -O2 -target bpf -emit-llvm \ > + -c $1 -o - || echo "BPF obj compilation failed") | \ > + $(LLC) -march=bpf -mcpu=probe $4 -filetype=obj -o $2 > +endef > +# Similar to CLANG_BPF_BUILD_RULE, but using native Clang and bpf LLC > +define CLANG_NATIVE_BPF_BUILD_RULE > + ($(CLANG) $3 -O2 -emit-llvm \ > + -c $1 -o - || echo "BPF obj compilation failed") | \ > + $(LLC) -march=bpf -mcpu=probe $4 -filetype=obj -o $2 > +endef > +# Build BPF object using GCC > +define GCC_BPF_BUILD_RULE > + $(BPF_GCC) $3 $4 -O2 -c $1 -o $2 > +endef > + > +# Set up extra TRUNNER_XXX "temporary" variables in the environment (relies on > +# $eval()) and pass control to DEFINE_TEST_RUNNER_RULES. > +# Parameters: > +# $1 - test runner base binary name (e.g., test_progs) > +# $2 - test runner extra "flavor" (e.g., alu32, gcc-bpf, etc) > +define DEFINE_TEST_RUNNER > + > +TRUNNER_OUTPUT := $(OUTPUT)$(if $2,/)$2 > +TRUNNER_BINARY := $1$(if $2,-)$2 > +TRUNNER_TEST_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.test.o, \ > + $$(notdir $$(wildcard $(TRUNNER_TESTS_DIR)/*.c))) > +TRUNNER_EXTRA_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.o, \ > + $$(filter %.c,$(TRUNNER_EXTRA_SOURCES))) > +TRUNNER_EXTRA_HDRS := $$(filter %.h,$(TRUNNER_EXTRA_SOURCES)) > +TRUNNER_TESTS_HDR := $(TRUNNER_TESTS_DIR)/tests.h > +TRUNNER_BPF_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.o, \ > + $$(notdir $$(wildcard $(TRUNNER_BPF_PROGS_DIR)/*.c))) > + > +# Evaluate rules now with extra TRUNNER_XXX variables above already defined > +$$(eval $$(call DEFINE_TEST_RUNNER_RULES,$1,$2)) > + > +endef > + > +# Using TRUNNER_XXX variables, provided by callers of DEFINE_TEST_RUNNER and > +# set up by DEFINE_TEST_RUNNER itself, create test runner build rules with: > +# $1 - test runner base binary name (e.g., test_progs) > +# $2 - test runner extra "flavor" (e.g., alu32, gcc-bpf, etc) > +define DEFINE_TEST_RUNNER_RULES > > +ifeq ($($(TRUNNER_OUTPUT)-dir),) > +$(TRUNNER_OUTPUT)-dir := y > +$(TRUNNER_OUTPUT): > + mkdir -p $$@ > +endif > + > +# ensure we set up BPF objects generation rule just once for a given > +# input/output directory combination > +ifeq ($($(TRUNNER_BPF_PROGS_DIR)$(if $2,-)$2-bpfobjs),) > +$(TRUNNER_BPF_PROGS_DIR)$(if $2,-)$2-bpfobjs := y > +$(TRUNNER_BPF_OBJS): $(TRUNNER_OUTPUT)/%.o: \ > + $(TRUNNER_BPF_PROGS_DIR)/%.c \ > + $(TRUNNER_BPF_PROGS_DIR)/*.h \ > + $$(BPF_HELPERS) | $(TRUNNER_OUTPUT) > + $$(call $(TRUNNER_BPF_BUILD_RULE),$$<,$$@, \ > + $(TRUNNER_BPF_CFLAGS), \ > + $(TRUNNER_BPF_LDFLAGS)) > +endif > + > +# ensure we set up tests.h header generation rule just once > +ifeq ($($(TRUNNER_TESTS_DIR)-tests-hdr),) > +$(TRUNNER_TESTS_DIR)-tests-hdr := y > +$(TRUNNER_TESTS_HDR): $(TRUNNER_TESTS_DIR)/*.c > + $$(shell ( cd $(TRUNNER_TESTS_DIR); \ > + echo '/* Generated header, do not edit */'; \ > + ls *.c 2> /dev/null | \ > + sed -e 's@\([^\.]*\)\.c@DEFINE_TEST(\1)@'; \ > + ) > $$@) > +endif > + > +# compile individual test files > +# Note: we cd into output directory to ensure embedded BPF object is found > +$(TRUNNER_TEST_OBJS): $(TRUNNER_OUTPUT)/%.test.o: \ > + $(TRUNNER_TESTS_DIR)/%.c \ > + $(TRUNNER_EXTRA_HDRS) \ > + $(TRUNNER_BPF_OBJS) \ > + $$(BPFOBJ) | $(TRUNNER_OUTPUT) > + cd $$(@D) && $$(CC) $$(CFLAGS) $$(LDLIBS) -c $(CURDIR)/$$< -o $$(@F) > + > +$(TRUNNER_EXTRA_OBJS): $(TRUNNER_OUTPUT)/%.o: \ > + %.c \ > + $(TRUNNER_EXTRA_HDRS) \ > + $(TRUNNER_TESTS_HDR) \ > + $$(BPFOBJ) | $(TRUNNER_OUTPUT) > + $$(CC) $$(CFLAGS) $$(LDLIBS) -c $$< -o $$@ > + > +$(TRUNNER_BINARY)-extras: $(TRUNNER_EXTRA_FILES) | $(TRUNNER_OUTPUT) > +ifneq ($2,) > + # only copy extra resources if in flavored build > + cp -a $$^ $(TRUNNER_OUTPUT)/ > +endif > + > +$(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS) \ > + $(TRUNNER_EXTRA_OBJS) $$(BPFOBJ) \ > + | $(TRUNNER_BINARY)-extras > + $$(CC) $$(CFLAGS) $$(LDLIBS) $$(filter %.a %.o,$$^) -o $$@ > + > +endef > + > +# Define test_progs test runner. > +TRUNNER_TESTS_DIR := prog_tests > +TRUNNER_BPF_PROGS_DIR := progs > +TRUNNER_EXTRA_SOURCES := test_progs.c cgroup_helpers.c trace_helpers.c \ > + flow_dissector_load.h > +TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read \ > + $(wildcard progs/btf_dump_test_case_*.c) > +TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE > +TRUNNER_BPF_CFLAGS := -I. -I$(OUTPUT) $(BPF_CFLAGS) $(CLANG_CFLAGS) > +TRUNNER_BPF_LDFLAGS := > +$(eval $(call DEFINE_TEST_RUNNER,test_progs)) > + > +# Define test_progs-alu32 test runner. > ifneq ($(SUBREG_CODEGEN),) > -ALU32_BUILD_DIR = $(OUTPUT)/alu32 > -TEST_CUSTOM_PROGS += $(ALU32_BUILD_DIR)/test_progs_32 > -$(ALU32_BUILD_DIR): > - mkdir -p $@ > - > -$(ALU32_BUILD_DIR)/urandom_read: $(OUTPUT)/urandom_read | $(ALU32_BUILD_DIR) > - cp $< $@ > - > -$(ALU32_BUILD_DIR)/test_progs_32: test_progs.c $(OUTPUT)/libbpf.a\ > - $(ALU32_BUILD_DIR)/urandom_read \ > - | $(ALU32_BUILD_DIR) > - $(CC) $(TEST_PROGS_CFLAGS) $(CFLAGS) \ > - -o $(ALU32_BUILD_DIR)/test_progs_32 \ > - test_progs.c test_stub.c cgroup_helpers.c trace_helpers.c prog_tests/*.c \ > - $(OUTPUT)/libbpf.a $(LDLIBS) > - > -$(ALU32_BUILD_DIR)/test_progs_32: $(PROG_TESTS_H) > -$(ALU32_BUILD_DIR)/test_progs_32: prog_tests/*.c > - > -$(ALU32_BUILD_DIR)/%.o: progs/%.c $(ALU32_BUILD_DIR)/test_progs_32 \ > - | $(ALU32_BUILD_DIR) > - ($(CLANG) $(BPF_CFLAGS) $(CLANG_CFLAGS) -O2 -target bpf -emit-llvm \ > - -c $< -o - || echo "clang failed") | \ > - $(LLC) -march=bpf -mcpu=probe -mattr=+alu32 $(LLC_FLAGS) \ > - -filetype=obj -o $@ > +TRUNNER_BPF_LDFLAGS += -mattr=+alu32 > +$(eval $(call DEFINE_TEST_RUNNER,test_progs,alu32)) > endif > > +# Define test_progs BPF-GCC-flavored test runner. > ifneq ($(BPF_GCC),) > -GCC_SYS_INCLUDES = $(call get_sys_includes,gcc) > IS_LITTLE_ENDIAN = $(shell $(CC) -dM -E - </dev/null | \ > grep 'define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__') > -ifeq ($(IS_LITTLE_ENDIAN),) > -MENDIAN=-mbig-endian > -else > -MENDIAN=-mlittle-endian > -endif > -BPF_GCC_CFLAGS = $(GCC_SYS_INCLUDES) $(MENDIAN) > -BPF_GCC_BUILD_DIR = $(OUTPUT)/bpf_gcc > -TEST_CUSTOM_PROGS += $(BPF_GCC_BUILD_DIR)/test_progs_bpf_gcc > -$(BPF_GCC_BUILD_DIR): > - mkdir -p $@ > - > -$(BPF_GCC_BUILD_DIR)/urandom_read: $(OUTPUT)/urandom_read | $(BPF_GCC_BUILD_DIR) > - cp $< $@ > - > -$(BPF_GCC_BUILD_DIR)/test_progs_bpf_gcc: $(OUTPUT)/test_progs \ > - | $(BPF_GCC_BUILD_DIR) > - cp $< $@ > - > -$(BPF_GCC_BUILD_DIR)/%.o: progs/%.c $(BPF_GCC_BUILD_DIR)/test_progs_bpf_gcc \ > - | $(BPF_GCC_BUILD_DIR) > - $(BPF_GCC) $(BPF_CFLAGS) $(BPF_GCC_CFLAGS) -O2 -c $< -o $@ > +MENDIAN=$(if $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian) > + > +TRUNNER_BPF_BUILD_RULE := GCC_BPF_BUILD_RULE > +TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(call get_sys_includes,gcc) $(MENDIAN) > +TRUNNER_BPF_LDFLAGS := > +$(eval $(call DEFINE_TEST_RUNNER,test_progs,bpf_gcc)) > endif > > -# Have one program compiled without "-target bpf" to test whether libbpf loads > -# it successfully > -$(OUTPUT)/test_xdp.o: progs/test_xdp.c > - ($(CLANG) $(BPF_CFLAGS) $(CLANG_CFLAGS) -O2 -emit-llvm -c $< -o - || \ > - echo "clang failed") | \ > - $(LLC) -march=bpf -mcpu=probe $(LLC_FLAGS) -filetype=obj -o $@ > - > -# libbpf has to be built before BPF programs due to bpf_helper_defs.h > -$(OUTPUT)/%.o: progs/%.c | $(BPFOBJ) > - ($(CLANG) $(BPF_CFLAGS) $(CLANG_CFLAGS) -O2 -target bpf -emit-llvm \ > - -c $< -o - || echo "clang failed") | \ > - $(LLC) -march=bpf -mcpu=probe $(LLC_FLAGS) -filetype=obj -o $@ > - > -PROG_TESTS_DIR = $(OUTPUT)/prog_tests > -$(PROG_TESTS_DIR): > - mkdir -p $@ > -PROG_TESTS_H := $(PROG_TESTS_DIR)/tests.h > -PROG_TESTS_FILES := $(wildcard prog_tests/*.c) > -test_progs.c: $(PROG_TESTS_H) > -$(OUTPUT)/test_progs: CFLAGS += $(TEST_PROGS_CFLAGS) > -$(OUTPUT)/test_progs: test_progs.c $(PROG_TESTS_FILES) | $(OUTPUT)/test_attach_probe.o $(PROG_TESTS_H) > -$(PROG_TESTS_H): $(PROG_TESTS_FILES) | $(PROG_TESTS_DIR) > - $(shell ( cd prog_tests/; \ > - echo '/* Generated header, do not edit */'; \ > - ls *.c 2> /dev/null | \ > - sed -e 's@\([^\.]*\)\.c@DEFINE_TEST(\1)@'; \ > - ) > $(PROG_TESTS_H)) > - > -MAP_TESTS_DIR = $(OUTPUT)/map_tests > -$(MAP_TESTS_DIR): > - mkdir -p $@ > -MAP_TESTS_H := $(MAP_TESTS_DIR)/tests.h > -MAP_TESTS_FILES := $(wildcard map_tests/*.c) > -test_maps.c: $(MAP_TESTS_H) > -$(OUTPUT)/test_maps: CFLAGS += $(TEST_MAPS_CFLAGS) > -$(OUTPUT)/test_maps: test_maps.c $(MAP_TESTS_FILES) | $(MAP_TESTS_H) > -$(MAP_TESTS_H): $(MAP_TESTS_FILES) | $(MAP_TESTS_DIR) > - $(shell ( cd map_tests/; \ > - echo '/* Generated header, do not edit */'; \ > - ls *.c 2> /dev/null | \ > - sed -e 's@\([^\.]*\)\.c@DEFINE_TEST(\1)@'; \ > - ) > $(MAP_TESTS_H)) > - > -VERIFIER_TESTS_DIR = $(OUTPUT)/verifier > -$(VERIFIER_TESTS_DIR): > - mkdir -p $@ > -VERIFIER_TESTS_H := $(VERIFIER_TESTS_DIR)/tests.h > -VERIFIER_TEST_FILES := $(wildcard verifier/*.c) > -test_verifier.c: $(VERIFIER_TESTS_H) > -$(OUTPUT)/test_verifier: CFLAGS += $(TEST_VERIFIER_CFLAGS) > -$(OUTPUT)/test_verifier: test_verifier.c | $(VERIFIER_TEST_FILES) $(VERIFIER_TESTS_H) > -$(VERIFIER_TESTS_H): $(VERIFIER_TEST_FILES) | $(VERIFIER_TESTS_DIR) > +# Define test_maps test runner. > +TRUNNER_TESTS_DIR := map_tests > +TRUNNER_BPF_PROGS_DIR := progs > +TRUNNER_EXTRA_SOURCES := test_maps.c > +TRUNNER_EXTRA_FILES := > +TRUNNER_BPF_BUILD_RULE := $$(error no BPF objects should be built) > +TRUNNER_BPF_CFLAGS := > +TRUNNER_BPF_LDFLAGS := > +$(eval $(call DEFINE_TEST_RUNNER,test_maps)) > + > +# Define test_verifier test runner. > +# It is much simpler than test_maps/test_progs and sufficiently different from > +# them (e.g., test.h is using completely pattern), that it's worth just > +# explicitly defining all the rules explicitly. > +verifier/tests.h: verifier/*.c > $(shell ( cd verifier/; \ > echo '/* Generated header, do not edit */'; \ > echo '#ifdef FILL_ARRAY'; \ > - ls *.c 2> /dev/null | \ > - sed -e 's@\(.*\)@#include \"\1\"@'; \ > + ls *.c 2> /dev/null | sed -e 's@\(.*\)@#include \"\1\"@'; \ > echo '#endif' \ > - ) > $(VERIFIER_TESTS_H)) > + ) > verifier/tests.h) > +$(OUTPUT)/test_verifier: test_verifier.c verifier/tests.h $(BPFOBJ) | $(OUTPUT) > + $(CC) $(CFLAGS) $(LDLIBS) $(filter %.a %.o %.c,$^) -o $@ > > -EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(ALU32_BUILD_DIR) $(BPF_GCC_BUILD_DIR) \ > - $(VERIFIER_TESTS_H) $(PROG_TESTS_H) $(MAP_TESTS_H) \ > - feature > +EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) \ > + prog_tests/tests.h map_tests/tests.h verifier/tests.h \ > + feature $(OUTPUT)/*.o $(OUTPUT)/alu32 $(OUTPUT)/bpf_gcc > -- > 2.17.1 >
On Wed, Oct 16, 2019 at 9:32 AM Stanislav Fomichev <sdf@fomichev.me> wrote: > > On 10/15, Andrii Nakryiko wrote: > > Define test runner generation meta-rule that codifies dependencies > > between test runner, its tests, and its dependent BPF programs. Use that > > for defining test_progs and test_maps test-runners. Also additionally define > > 2 flavors of test_progs: > > - alu32, which builds BPF programs with 32-bit registers codegen; > > - bpf_gcc, which build BPF programs using GCC, if it supports BPF target. > Question: > > Why not merge test_maps tests into test_progs framework and have a > single binary instead of doing all this makefile-related work? > We can independently address the story with alu32/gcc progs (presumably > in the same manner, with make defines). test_maps wasn't a reason for doing this, alue2/bpf_gcc was. test_maps is a simple sub-case that was just easy to convert to. I dare you to try solve alu32/bpf_gcc with make defines (whatever you mean by that) and in a simpler manner ;) > > I can hardly follow the existing makefile and now with the evals it's > 10x more complicated for no good reason. I agree that existing Makefile logic is hard to follow, especially given it's broken. But I think 10x more complexity is gross exaggeration and just means you haven't tried to follow rules' logic. The rules inside DEFINE_TEST_RUNNER_RULES are exactly (minus one or two ifs to prevent re-definition of target) the rules that should have been written for test_progs, test_progs-alu32, test_progs-bpf_gcc. They define a chain of BPF .c -> BPF .o -> tests .c -> tests .o -> final binary + test.h generation. Previously we were getting away with this for, e.g., test_progs-alu32, because we always also built test_progs in parallel, which generated necessary stuff. Now with recent changes to test_attach_probe.c which now embeds BPF .o file, this doesn't work anymore. And it's going to be more and more prevalent form, so we need to fix it. Surely $(eval) and $(call) are not common for simple Makefiles, but just ignore it, we need that to only dynamically generate per-test-runner rules. DEFINE_TEST_RUNNER_RULES can be almost read like a normal Makefile definitions, module $$(VAR) which is turned into a normal $(VAR) upon $(call) evaluation. But really, I'd like to be wrong and if there is simpler way to achieve the same - go for it, I'll gladly review and ack. > > > Overall, this is accomplished through $(eval)'ing a set of generic > > rules, which defines Makefile targets dynamically at runtime. See > > comments explaining the need for 2 $(evals), though. > > > > For each test runner we have (test_maps and test_progs, currently), and, > > optionally, their flavors, the logic of build process is modeled as > > follows (using test_progs as an example): > > - all BPF objects are in progs/: > > - BPF object's .o file is built into output directory from > > corresponding progs/.c file; > > - all BPF objects in progs/*.c depend on all progs/*.h headers; > > - all BPF objects depend on bpf_*.h helpers from libbpf (but not > > libbpf archive). There is an extra rule to trigger bpf_helper_defs.h > > (re-)build, if it's not present/outdated); > > - build recipe for BPF object can be re-defined per test runner/flavor; > > - test files are built from prog_tests/*.c: > > - all such test file objects are built on individual file basis; > > - currently, every single test file depends on all BPF object files; > > this might be improved in follow up patches to do 1-to-1 dependency, > > but allowing to customize this per each individual test; > > - each test runner definition can specify a list of extra .c and .h > > files to be built along test files and test runner binary; all such > > headers are becoming automatic dependency of each test .c file; > > - due to test files sometimes embedding (using .incbin assembly > > directive) contents of some BPF objects at compilation time, which are > > expected to be in CWD of compiler, compilation for test file object does > > cd into test runner's output directory; to support this mode all the > > include paths are turned into absolute paths using $(abspath) make > > function; > > - prog_tests/test.h is automatically (re-)generated with an entry for > > each .c file in prog_tests/; > > - final test runner binary is linked together from test object files and > > extra object files, linking together libbpf's archive as well; > > - it's possible to specify extra "resource" files/targets, which will be > > copied into test runner output directory, if it differes from > > Makefile-wide $(OUTPUT). This is used to ensure btf_dump test cases and > > urandom_read binary is put into a test runner's CWD for tests to find > > them in runtime. > > > > For flavored test runners, their output directory is a subdirectory of > > common Makefile-wide $(OUTPUT) directory with flavor name used as > > subdirectory name. > > > > BPF objects targets might be reused between different test runners, so > > extra checks are employed to not double-define them. Similarly, we have > > redefinition guards for output directories and test headers. > > > > test_verifier follows slightly different patterns and is simple enough > > to not justify generalizing TEST_RUNNER_DEFINE/TEST_RUNNER_DEFINE_RULES > > further to accomodate these differences. Instead, rules for > > test_verifier are minimized and simplified, while preserving correctness > > of dependencies. > > > > Signed-off-by: Andrii Nakryiko <andriin@fb.com> > > --- > > tools/testing/selftests/bpf/.gitignore | 5 +- > > tools/testing/selftests/bpf/Makefile | 313 ++++++++++++++----------- > > 2 files changed, 180 insertions(+), 138 deletions(-) > > Please truncate irrelevant parts, easier to review. [...]
On 10/16, Andrii Nakryiko wrote: > On Wed, Oct 16, 2019 at 9:32 AM Stanislav Fomichev <sdf@fomichev.me> wrote: > > > > On 10/15, Andrii Nakryiko wrote: > > > Define test runner generation meta-rule that codifies dependencies > > > between test runner, its tests, and its dependent BPF programs. Use that > > > for defining test_progs and test_maps test-runners. Also additionally define > > > 2 flavors of test_progs: > > > - alu32, which builds BPF programs with 32-bit registers codegen; > > > - bpf_gcc, which build BPF programs using GCC, if it supports BPF target. > > Question: > > > > Why not merge test_maps tests into test_progs framework and have a > > single binary instead of doing all this makefile-related work? > > We can independently address the story with alu32/gcc progs (presumably > > in the same manner, with make defines). > > test_maps wasn't a reason for doing this, alue2/bpf_gcc was. test_maps > is a simple sub-case that was just easy to convert to. I dare you to > try solve alu32/bpf_gcc with make defines (whatever you mean by that) > and in a simpler manner ;) I think my concern comes from the fact that I don't really understand why we need all that complexity (and the problem you're solving for alu/gcc; part of that is that you're replacing everything, so it's hard to understand what's the real diff). In particular, why do we need to compile test_progs 3 times for normal/alu32/gcc? Isn't it the same test_progs? Can we just teach test_progs to run the tests for 3 output dirs with different versions of BPF programs? (kind of like you do in your first patch with -<flavor>, but just in a loop). > > I can hardly follow the existing makefile and now with the evals it's > > 10x more complicated for no good reason. > > I agree that existing Makefile logic is hard to follow, especially > given it's broken. But I think 10x more complexity is gross > exaggeration and just means you haven't tried to follow rules' logic. Not 10x, but it does raise a complexity bar. I tried to follow the rules, but I admit that I didn't try too hard :-) > The rules inside DEFINE_TEST_RUNNER_RULES are exactly (minus one or > two ifs to prevent re-definition of target) the rules that should have > been written for test_progs, test_progs-alu32, test_progs-bpf_gcc. > They define a chain of BPF .c -> BPF .o -> tests .c -> tests .o -> > final binary + test.h generation. Previously we were getting away with > this for, e.g., test_progs-alu32, because we always also built > test_progs in parallel, which generated necessary stuff. Now with > recent changes to test_attach_probe.c which now embeds BPF .o file, > this doesn't work anymore. And it's going to be more and more > prevalent form, so we need to fix it. > > Surely $(eval) and $(call) are not common for simple Makefiles, but > just ignore it, we need that to only dynamically generate > per-test-runner rules. DEFINE_TEST_RUNNER_RULES can be almost read > like a normal Makefile definitions, module $$(VAR) which is turned > into a normal $(VAR) upon $(call) evaluation. > > But really, I'd like to be wrong and if there is simpler way to > achieve the same - go for it, I'll gladly review and ack. Again, it probably comes from the fact that I don't see the problem you're solving. Can we start by removing 3 test_progs variations (somthing like patch below)? If we can do it, then the leftover parts that generate alu32/gcc bpf program don't look too bad and can probably be tweaked without makefile codegen. --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -157,26 +157,10 @@ TEST_VERIFIER_CFLAGS := -I. -I$(OUTPUT) -Iverifier ifneq ($(SUBREG_CODEGEN),) ALU32_BUILD_DIR = $(OUTPUT)/alu32 -TEST_CUSTOM_PROGS += $(ALU32_BUILD_DIR)/test_progs_32 $(ALU32_BUILD_DIR): mkdir -p $@ -$(ALU32_BUILD_DIR)/urandom_read: $(OUTPUT)/urandom_read | $(ALU32_BUILD_DIR) - cp $< $@ - -$(ALU32_BUILD_DIR)/test_progs_32: test_progs.c $(OUTPUT)/libbpf.a\ - $(ALU32_BUILD_DIR)/urandom_read \ - | $(ALU32_BUILD_DIR) - $(CC) $(TEST_PROGS_CFLAGS) $(CFLAGS) \ - -o $(ALU32_BUILD_DIR)/test_progs_32 \ - test_progs.c test_stub.c cgroup_helpers.c trace_helpers.c prog_tests/*.c \ - $(OUTPUT)/libbpf.a $(LDLIBS) - -$(ALU32_BUILD_DIR)/test_progs_32: $(PROG_TESTS_H) -$(ALU32_BUILD_DIR)/test_progs_32: prog_tests/*.c - -$(ALU32_BUILD_DIR)/%.o: progs/%.c $(ALU32_BUILD_DIR)/test_progs_32 \ - | $(ALU32_BUILD_DIR) +$(ALU32_BUILD_DIR)/%.o: progs/%.c | $(ALU32_BUILD_DIR) ($(CLANG) $(BPF_CFLAGS) $(CLANG_CFLAGS) -O2 -target bpf -emit-llvm \ -c $< -o - || echo "clang failed") | \ $(LLC) -march=bpf -mcpu=probe -mattr=+alu32 $(LLC_FLAGS) \ @@ -194,19 +178,10 @@ MENDIAN=-mlittle-endian endif BPF_GCC_CFLAGS = $(GCC_SYS_INCLUDES) $(MENDIAN) BPF_GCC_BUILD_DIR = $(OUTPUT)/bpf_gcc -TEST_CUSTOM_PROGS += $(BPF_GCC_BUILD_DIR)/test_progs_bpf_gcc $(BPF_GCC_BUILD_DIR): mkdir -p $@ -$(BPF_GCC_BUILD_DIR)/urandom_read: $(OUTPUT)/urandom_read | $(BPF_GCC_BUILD_DIR) - cp $< $@ - -$(BPF_GCC_BUILD_DIR)/test_progs_bpf_gcc: $(OUTPUT)/test_progs \ - | $(BPF_GCC_BUILD_DIR) - cp $< $@ - -$(BPF_GCC_BUILD_DIR)/%.o: progs/%.c $(BPF_GCC_BUILD_DIR)/test_progs_bpf_gcc \ - | $(BPF_GCC_BUILD_DIR) +$(BPF_GCC_BUILD_DIR)/%.o: progs/%.c | $(BPF_GCC_BUILD_DIR) $(BPF_GCC) $(BPF_CFLAGS) $(BPF_GCC_CFLAGS) -O2 -c $< -o $@ endif > Please truncate irrelevant parts, easier to review. Sure, will do, but I always forget because I don't have this problem. In mutt I can press shift+s to jump to the next unquoted section.
On Thu, Oct 17, 2019 at 9:07 AM Stanislav Fomichev <sdf@fomichev.me> wrote: > > On 10/16, Andrii Nakryiko wrote: > > On Wed, Oct 16, 2019 at 9:32 AM Stanislav Fomichev <sdf@fomichev.me> wrote: > > > > > > On 10/15, Andrii Nakryiko wrote: > > > > Define test runner generation meta-rule that codifies dependencies > > > > between test runner, its tests, and its dependent BPF programs. Use that > > > > for defining test_progs and test_maps test-runners. Also additionally define > > > > 2 flavors of test_progs: > > > > - alu32, which builds BPF programs with 32-bit registers codegen; > > > > - bpf_gcc, which build BPF programs using GCC, if it supports BPF target. > > > Question: > > > > > > Why not merge test_maps tests into test_progs framework and have a > > > single binary instead of doing all this makefile-related work? > > > We can independently address the story with alu32/gcc progs (presumably > > > in the same manner, with make defines). > > > > test_maps wasn't a reason for doing this, alue2/bpf_gcc was. test_maps > > is a simple sub-case that was just easy to convert to. I dare you to > > try solve alu32/bpf_gcc with make defines (whatever you mean by that) > > and in a simpler manner ;) > I think my concern comes from the fact that I don't really understand why > we need all that complexity (and the problem you're solving for alu/gcc; > part of that is that you're replacing everything, so it's hard to > understand what's the real diff). > > In particular, why do we need to compile test_progs 3 times for > normal/alu32/gcc? Isn't it the same test_progs? Can we just teach test_progs > to run the tests for 3 output dirs with different versions of BPF programs? > (kind of like you do in your first patch with -<flavor>, but just in a loop). So that's a good question and the answer is "no, we can't". And that's why I consider alu32/bpf_gcc broken. Check progs/test_attach_probe.c, it does BPF_OBJECT_EMBED, which links BPF object into test object file. This means that if we want to compile BPF objects differently between default/alu32/gcc flavors, we need to compile test_progs independently. Embedding objects is going to be prevalent way to consume them (and it is already the only way we consume them in production at FB), so we need to accommodate it. With some more usability improvements that's on my TODO list, it will become also much more convenient and easy to consume such BPF objects. > > > > I can hardly follow the existing makefile and now with the evals it's > > > 10x more complicated for no good reason. > > > > I agree that existing Makefile logic is hard to follow, especially > > given it's broken. But I think 10x more complexity is gross > > exaggeration and just means you haven't tried to follow rules' logic. > Not 10x, but it does raise a complexity bar. I tried to follow the > rules, but I admit that I didn't try too hard :-) So see my explanation above about why we need to compile flavors independently. Rules have to be like they are here. I'd like to make dependencies between test objects and BPF objects a bit more granular, but only after we land this, it's already quite a lot of changes at once. Beyond fixing the rules, $(eval)/$(call) is a new stuff for selftests/bpf's Makefile, but it's semantics is well described in documentation and you can gloss over it for now, it shouldn't break with Makefile changes. > > > The rules inside DEFINE_TEST_RUNNER_RULES are exactly (minus one or > > two ifs to prevent re-definition of target) the rules that should have > > been written for test_progs, test_progs-alu32, test_progs-bpf_gcc. > > They define a chain of BPF .c -> BPF .o -> tests .c -> tests .o -> > > final binary + test.h generation. Previously we were getting away with > > this for, e.g., test_progs-alu32, because we always also built > > test_progs in parallel, which generated necessary stuff. Now with > > recent changes to test_attach_probe.c which now embeds BPF .o file, > > this doesn't work anymore. And it's going to be more and more > > prevalent form, so we need to fix it. > > > > Surely $(eval) and $(call) are not common for simple Makefiles, but > > just ignore it, we need that to only dynamically generate > > per-test-runner rules. DEFINE_TEST_RUNNER_RULES can be almost read > > like a normal Makefile definitions, module $$(VAR) which is turned > > into a normal $(VAR) upon $(call) evaluation. > > > > But really, I'd like to be wrong and if there is simpler way to > > achieve the same - go for it, I'll gladly review and ack. > Again, it probably comes from the fact that I don't see the problem > you're solving. Can we start by removing 3 test_progs variations > (somthing like patch below)? If we can do it, then the leftover parts > that generate alu32/gcc bpf program don't look too bad and can probably > be tweaked without makefile codegen. Yes, it probably is. See above, I tried to give more context. I've fixed some other inconveniences with current Makefile set up (e.g., on-demand bpf_helper_defs.h re-generation, etc), but those are minor changes and it was hard to de-couple from the main change. > > --- a/tools/testing/selftests/bpf/Makefile > +++ b/tools/testing/selftests/bpf/Makefile > @@ -157,26 +157,10 @@ TEST_VERIFIER_CFLAGS := -I. -I$(OUTPUT) -Iverifier > [...] > endif > > > Please truncate irrelevant parts, easier to review. > Sure, will do, but I always forget because I don't have this problem. > In mutt I can press shift+s to jump to the next unquoted section. no worries, but with a bit of recurring reminder it becomes easier, I know from my own experience :)
On Tue, Oct 15, 2019 at 11:01 PM Andrii Nakryiko <andriin@fb.com> wrote: > > Define test runner generation meta-rule that codifies dependencies > between test runner, its tests, and its dependent BPF programs. Use that > for defining test_progs and test_maps test-runners. Also additionally define > 2 flavors of test_progs: > - alu32, which builds BPF programs with 32-bit registers codegen; > - bpf_gcc, which build BPF programs using GCC, if it supports BPF target. > > Overall, this is accomplished through $(eval)'ing a set of generic > rules, which defines Makefile targets dynamically at runtime. See > comments explaining the need for 2 $(evals), though. > > For each test runner we have (test_maps and test_progs, currently), and, > optionally, their flavors, the logic of build process is modeled as > follows (using test_progs as an example): > - all BPF objects are in progs/: > - BPF object's .o file is built into output directory from > corresponding progs/.c file; > - all BPF objects in progs/*.c depend on all progs/*.h headers; > - all BPF objects depend on bpf_*.h helpers from libbpf (but not > libbpf archive). There is an extra rule to trigger bpf_helper_defs.h > (re-)build, if it's not present/outdated); > - build recipe for BPF object can be re-defined per test runner/flavor; > - test files are built from prog_tests/*.c: > - all such test file objects are built on individual file basis; > - currently, every single test file depends on all BPF object files; > this might be improved in follow up patches to do 1-to-1 dependency, > but allowing to customize this per each individual test; > - each test runner definition can specify a list of extra .c and .h > files to be built along test files and test runner binary; all such > headers are becoming automatic dependency of each test .c file; > - due to test files sometimes embedding (using .incbin assembly > directive) contents of some BPF objects at compilation time, which are > expected to be in CWD of compiler, compilation for test file object does > cd into test runner's output directory; to support this mode all the > include paths are turned into absolute paths using $(abspath) make > function; > - prog_tests/test.h is automatically (re-)generated with an entry for > each .c file in prog_tests/; > - final test runner binary is linked together from test object files and > extra object files, linking together libbpf's archive as well; > - it's possible to specify extra "resource" files/targets, which will be > copied into test runner output directory, if it differes from > Makefile-wide $(OUTPUT). This is used to ensure btf_dump test cases and > urandom_read binary is put into a test runner's CWD for tests to find > them in runtime. > > For flavored test runners, their output directory is a subdirectory of > common Makefile-wide $(OUTPUT) directory with flavor name used as > subdirectory name. > > BPF objects targets might be reused between different test runners, so > extra checks are employed to not double-define them. Similarly, we have > redefinition guards for output directories and test headers. > > test_verifier follows slightly different patterns and is simple enough > to not justify generalizing TEST_RUNNER_DEFINE/TEST_RUNNER_DEFINE_RULES > further to accomodate these differences. Instead, rules for > test_verifier are minimized and simplified, while preserving correctness > of dependencies. > > Signed-off-by: Andrii Nakryiko <andriin@fb.com> > --- BTW, if correctness and DRY-ness argument is not strong enough, these changes makes clean rebuild from scratch about 2x faster for me: BEFORE: `make clean && time make -j50` is 14-15 seconds AFTER: `make clean && time make -j50` is 7-8 seconds [...]
On 10/17/19 10:50 AM, Andrii Nakryiko wrote: > On Tue, Oct 15, 2019 at 11:01 PM Andrii Nakryiko <andriin@fb.com> wrote: >> >> Define test runner generation meta-rule that codifies dependencies >> between test runner, its tests, and its dependent BPF programs. Use that >> for defining test_progs and test_maps test-runners. Also additionally define >> 2 flavors of test_progs: >> - alu32, which builds BPF programs with 32-bit registers codegen; >> - bpf_gcc, which build BPF programs using GCC, if it supports BPF target. >> >> Overall, this is accomplished through $(eval)'ing a set of generic >> rules, which defines Makefile targets dynamically at runtime. See >> comments explaining the need for 2 $(evals), though. >> >> For each test runner we have (test_maps and test_progs, currently), and, >> optionally, their flavors, the logic of build process is modeled as >> follows (using test_progs as an example): >> - all BPF objects are in progs/: >> - BPF object's .o file is built into output directory from >> corresponding progs/.c file; >> - all BPF objects in progs/*.c depend on all progs/*.h headers; >> - all BPF objects depend on bpf_*.h helpers from libbpf (but not >> libbpf archive). There is an extra rule to trigger bpf_helper_defs.h >> (re-)build, if it's not present/outdated); >> - build recipe for BPF object can be re-defined per test runner/flavor; >> - test files are built from prog_tests/*.c: >> - all such test file objects are built on individual file basis; >> - currently, every single test file depends on all BPF object files; >> this might be improved in follow up patches to do 1-to-1 dependency, >> but allowing to customize this per each individual test; >> - each test runner definition can specify a list of extra .c and .h >> files to be built along test files and test runner binary; all such >> headers are becoming automatic dependency of each test .c file; >> - due to test files sometimes embedding (using .incbin assembly >> directive) contents of some BPF objects at compilation time, which are >> expected to be in CWD of compiler, compilation for test file object does >> cd into test runner's output directory; to support this mode all the >> include paths are turned into absolute paths using $(abspath) make >> function; >> - prog_tests/test.h is automatically (re-)generated with an entry for >> each .c file in prog_tests/; >> - final test runner binary is linked together from test object files and >> extra object files, linking together libbpf's archive as well; >> - it's possible to specify extra "resource" files/targets, which will be >> copied into test runner output directory, if it differes from >> Makefile-wide $(OUTPUT). This is used to ensure btf_dump test cases and >> urandom_read binary is put into a test runner's CWD for tests to find >> them in runtime. >> >> For flavored test runners, their output directory is a subdirectory of >> common Makefile-wide $(OUTPUT) directory with flavor name used as >> subdirectory name. >> >> BPF objects targets might be reused between different test runners, so >> extra checks are employed to not double-define them. Similarly, we have >> redefinition guards for output directories and test headers. >> >> test_verifier follows slightly different patterns and is simple enough >> to not justify generalizing TEST_RUNNER_DEFINE/TEST_RUNNER_DEFINE_RULES >> further to accomodate these differences. Instead, rules for >> test_verifier are minimized and simplified, while preserving correctness >> of dependencies. >> >> Signed-off-by: Andrii Nakryiko <andriin@fb.com> >> --- > > BTW, if correctness and DRY-ness argument is not strong enough, these > changes makes clean rebuild from scratch about 2x faster for me: > > BEFORE: `make clean && time make -j50` is 14-15 seconds > AFTER: `make clean && time make -j50` is 7-8 seconds I noticed that too and was about to ask "why?" .. :)
On Thu, Oct 17, 2019 at 10:54 AM Alexei Starovoitov <ast@fb.com> wrote: > > On 10/17/19 10:50 AM, Andrii Nakryiko wrote: > > On Tue, Oct 15, 2019 at 11:01 PM Andrii Nakryiko <andriin@fb.com> wrote: > >> > >> Define test runner generation meta-rule that codifies dependencies > >> between test runner, its tests, and its dependent BPF programs. Use that > >> for defining test_progs and test_maps test-runners. Also additionally define > >> 2 flavors of test_progs: > >> - alu32, which builds BPF programs with 32-bit registers codegen; > >> - bpf_gcc, which build BPF programs using GCC, if it supports BPF target. > >> > >> Overall, this is accomplished through $(eval)'ing a set of generic > >> rules, which defines Makefile targets dynamically at runtime. See > >> comments explaining the need for 2 $(evals), though. > >> > >> For each test runner we have (test_maps and test_progs, currently), and, > >> optionally, their flavors, the logic of build process is modeled as > >> follows (using test_progs as an example): > >> - all BPF objects are in progs/: > >> - BPF object's .o file is built into output directory from > >> corresponding progs/.c file; > >> - all BPF objects in progs/*.c depend on all progs/*.h headers; > >> - all BPF objects depend on bpf_*.h helpers from libbpf (but not > >> libbpf archive). There is an extra rule to trigger bpf_helper_defs.h > >> (re-)build, if it's not present/outdated); > >> - build recipe for BPF object can be re-defined per test runner/flavor; > >> - test files are built from prog_tests/*.c: > >> - all such test file objects are built on individual file basis; > >> - currently, every single test file depends on all BPF object files; > >> this might be improved in follow up patches to do 1-to-1 dependency, > >> but allowing to customize this per each individual test; > >> - each test runner definition can specify a list of extra .c and .h > >> files to be built along test files and test runner binary; all such > >> headers are becoming automatic dependency of each test .c file; > >> - due to test files sometimes embedding (using .incbin assembly > >> directive) contents of some BPF objects at compilation time, which are > >> expected to be in CWD of compiler, compilation for test file object does > >> cd into test runner's output directory; to support this mode all the > >> include paths are turned into absolute paths using $(abspath) make > >> function; > >> - prog_tests/test.h is automatically (re-)generated with an entry for > >> each .c file in prog_tests/; > >> - final test runner binary is linked together from test object files and > >> extra object files, linking together libbpf's archive as well; > >> - it's possible to specify extra "resource" files/targets, which will be > >> copied into test runner output directory, if it differes from > >> Makefile-wide $(OUTPUT). This is used to ensure btf_dump test cases and > >> urandom_read binary is put into a test runner's CWD for tests to find > >> them in runtime. > >> > >> For flavored test runners, their output directory is a subdirectory of > >> common Makefile-wide $(OUTPUT) directory with flavor name used as > >> subdirectory name. > >> > >> BPF objects targets might be reused between different test runners, so > >> extra checks are employed to not double-define them. Similarly, we have > >> redefinition guards for output directories and test headers. > >> > >> test_verifier follows slightly different patterns and is simple enough > >> to not justify generalizing TEST_RUNNER_DEFINE/TEST_RUNNER_DEFINE_RULES > >> further to accomodate these differences. Instead, rules for > >> test_verifier are minimized and simplified, while preserving correctness > >> of dependencies. > >> > >> Signed-off-by: Andrii Nakryiko <andriin@fb.com> > >> --- > > > > BTW, if correctness and DRY-ness argument is not strong enough, these > > changes makes clean rebuild from scratch about 2x faster for me: > > > > BEFORE: `make clean && time make -j50` is 14-15 seconds > > AFTER: `make clean && time make -j50` is 7-8 seconds > > I noticed that too and was about to ask "why?" .. :) alu32 BPF .o's were dependent on alu32/test_progs for some reason, so they blocked on all tests be built first, which is completely backwards and slower. Now all the flavors are built completely in parallel. Overall CPU usage across all cores increased (because we do more work compiling binaries), but it's more parallel.
On 10/17, Andrii Nakryiko wrote: > On Thu, Oct 17, 2019 at 9:07 AM Stanislav Fomichev <sdf@fomichev.me> wrote: > > > > On 10/16, Andrii Nakryiko wrote: > > > On Wed, Oct 16, 2019 at 9:32 AM Stanislav Fomichev <sdf@fomichev.me> wrote: > > > > > > > > On 10/15, Andrii Nakryiko wrote: > > > > > Define test runner generation meta-rule that codifies dependencies > > > > > between test runner, its tests, and its dependent BPF programs. Use that > > > > > for defining test_progs and test_maps test-runners. Also additionally define > > > > > 2 flavors of test_progs: > > > > > - alu32, which builds BPF programs with 32-bit registers codegen; > > > > > - bpf_gcc, which build BPF programs using GCC, if it supports BPF target. > > > > Question: > > > > > > > > Why not merge test_maps tests into test_progs framework and have a > > > > single binary instead of doing all this makefile-related work? > > > > We can independently address the story with alu32/gcc progs (presumably > > > > in the same manner, with make defines). > > > > > > test_maps wasn't a reason for doing this, alue2/bpf_gcc was. test_maps > > > is a simple sub-case that was just easy to convert to. I dare you to > > > try solve alu32/bpf_gcc with make defines (whatever you mean by that) > > > and in a simpler manner ;) > > I think my concern comes from the fact that I don't really understand why > > we need all that complexity (and the problem you're solving for alu/gcc; > > part of that is that you're replacing everything, so it's hard to > > understand what's the real diff). > > > > In particular, why do we need to compile test_progs 3 times for > > normal/alu32/gcc? Isn't it the same test_progs? Can we just teach test_progs > > to run the tests for 3 output dirs with different versions of BPF programs? > > (kind of like you do in your first patch with -<flavor>, but just in a loop). > > So that's a good question and the answer is "no, we can't". And that's > why I consider alu32/bpf_gcc broken. Check progs/test_attach_probe.c, > it does BPF_OBJECT_EMBED, which links BPF object into test object > file. This means that if we want to compile BPF objects differently > between default/alu32/gcc flavors, we need to compile test_progs > independently. Embedding objects is going to be prevalent way to > consume them (and it is already the only way we consume them in > production at FB), so we need to accommodate it. With some more > usability improvements that's on my TODO list, it will become also > much more convenient and easy to consume such BPF objects. Thanks for the explanation, I missed that EMBED_FILE in attach_probe.c I was going to suggest to move it out of test_progs (or use open() instead of embedding?) if you want to test bpf_object__open_mem (to avoid all that complexity and make test_progs work with any bpf alu32/gcc/clang subdir), but it seems like you have pretty much settled on the embedding. It's interesting that we try to do it a bit different here, maybe even going as far as rolling out individual BPF .o files without updating the control plane :-) > > > > I can hardly follow the existing makefile and now with the evals it's > > > > 10x more complicated for no good reason. > > > > > > I agree that existing Makefile logic is hard to follow, especially > > > given it's broken. But I think 10x more complexity is gross > > > exaggeration and just means you haven't tried to follow rules' logic. > > Not 10x, but it does raise a complexity bar. I tried to follow the > > rules, but I admit that I didn't try too hard :-) > > So see my explanation above about why we need to compile flavors > independently. Rules have to be like they are here. I'd like to make > dependencies between test objects and BPF objects a bit more granular, > but only after we land this, it's already quite a lot of changes at > once. > > Beyond fixing the rules, $(eval)/$(call) is a new stuff for > selftests/bpf's Makefile, but it's semantics is well described in > documentation and you can gloss over it for now, it shouldn't break > with Makefile changes. > > > > > > The rules inside DEFINE_TEST_RUNNER_RULES are exactly (minus one or > > > two ifs to prevent re-definition of target) the rules that should have > > > been written for test_progs, test_progs-alu32, test_progs-bpf_gcc. > > > They define a chain of BPF .c -> BPF .o -> tests .c -> tests .o -> > > > final binary + test.h generation. Previously we were getting away with > > > this for, e.g., test_progs-alu32, because we always also built > > > test_progs in parallel, which generated necessary stuff. Now with > > > recent changes to test_attach_probe.c which now embeds BPF .o file, > > > this doesn't work anymore. And it's going to be more and more > > > prevalent form, so we need to fix it. > > > > > > Surely $(eval) and $(call) are not common for simple Makefiles, but > > > just ignore it, we need that to only dynamically generate > > > per-test-runner rules. DEFINE_TEST_RUNNER_RULES can be almost read > > > like a normal Makefile definitions, module $$(VAR) which is turned > > > into a normal $(VAR) upon $(call) evaluation. > > > > > > But really, I'd like to be wrong and if there is simpler way to > > > achieve the same - go for it, I'll gladly review and ack. > > Again, it probably comes from the fact that I don't see the problem > > you're solving. Can we start by removing 3 test_progs variations > > (somthing like patch below)? If we can do it, then the leftover parts > > that generate alu32/gcc bpf program don't look too bad and can probably > > be tweaked without makefile codegen. > > Yes, it probably is. See above, I tried to give more context. Again, thanks for the explanation, I did indeed miss that EMBED_FILE case. But I'd still vote for moving that test into a dedicated binary, have single test_progs that works with a flavored BPF subdir and simplify the makefile instead of adding more stuff into it. These are my 2c, feel free to ignore :-) We have our own simplified reimplementation anyway to fit into our testing system, so I don't have a horse in this race. > I've fixed some other inconveniences with current Makefile set up > (e.g., on-demand bpf_helper_defs.h re-generation, etc), but those are > minor changes and it was hard to de-couple from the main change. > > > > > --- a/tools/testing/selftests/bpf/Makefile > > +++ b/tools/testing/selftests/bpf/Makefile > > @@ -157,26 +157,10 @@ TEST_VERIFIER_CFLAGS := -I. -I$(OUTPUT) -Iverifier > > > > [...] > > > endif > > > > > Please truncate irrelevant parts, easier to review. > > Sure, will do, but I always forget because I don't have this problem. > > In mutt I can press shift+s to jump to the next unquoted section. > > no worries, but with a bit of recurring reminder it becomes easier, I > know from my own experience :)
Hi, Andrii! The patch blanks TEST_GEN_FILES which was used by install target (lib.mk) to install test progs. How is it supposed to work? That fixes it for me btw: diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 8f25966b500b..1f878dcd2bf6 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -265,6 +265,7 @@ TRUNNER_BPF_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.o, $$(TRUNNER_BPF_SRCS) TRUNNER_BPF_SKELS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.skel.h, \ $$(filter-out $(SKEL_BLACKLIST), \ $$(TRUNNER_BPF_SRCS))) +TEST_GEN_FILES += $$(TRUNNER_BPF_OBJS) # Evaluate rules now with extra TRUNNER_XXX variables above already defined $$(eval $$(call DEFINE_TEST_RUNNER_RULES,$1,$2)) >>>>> On Tue, 15 Oct 2019 23:00:49 -0700, Andrii Nakryiko wrote: > Define test runner generation meta-rule that codifies dependencies > between test runner, its tests, and its dependent BPF programs. Use that > for defining test_progs and test_maps test-runners. Also additionally define > 2 flavors of test_progs: > - alu32, which builds BPF programs with 32-bit registers codegen; > - bpf_gcc, which build BPF programs using GCC, if it supports BPF target. > Overall, this is accomplished through $(eval)'ing a set of generic > rules, which defines Makefile targets dynamically at runtime. See > comments explaining the need for 2 $(evals), though. > For each test runner we have (test_maps and test_progs, currently), and, > optionally, their flavors, the logic of build process is modeled as > follows (using test_progs as an example): > - all BPF objects are in progs/: > - BPF object's .o file is built into output directory from > corresponding progs/.c file; > - all BPF objects in progs/*.c depend on all progs/*.h headers; > - all BPF objects depend on bpf_*.h helpers from libbpf (but not > libbpf archive). There is an extra rule to trigger bpf_helper_defs.h > (re-)build, if it's not present/outdated); > - build recipe for BPF object can be re-defined per test runner/flavor; > - test files are built from prog_tests/*.c: > - all such test file objects are built on individual file basis; > - currently, every single test file depends on all BPF object files; > this might be improved in follow up patches to do 1-to-1 dependency, > but allowing to customize this per each individual test; > - each test runner definition can specify a list of extra .c and .h > files to be built along test files and test runner binary; all such > headers are becoming automatic dependency of each test .c file; > - due to test files sometimes embedding (using .incbin assembly > directive) contents of some BPF objects at compilation time, which are > expected to be in CWD of compiler, compilation for test file object does > cd into test runner's output directory; to support this mode all the > include paths are turned into absolute paths using $(abspath) make > function; > - prog_tests/test.h is automatically (re-)generated with an entry for > each .c file in prog_tests/; > - final test runner binary is linked together from test object files and > extra object files, linking together libbpf's archive as well; > - it's possible to specify extra "resource" files/targets, which will be > copied into test runner output directory, if it differes from > Makefile-wide $(OUTPUT). This is used to ensure btf_dump test cases and > urandom_read binary is put into a test runner's CWD for tests to find > them in runtime. > For flavored test runners, their output directory is a subdirectory of > common Makefile-wide $(OUTPUT) directory with flavor name used as > subdirectory name. > BPF objects targets might be reused between different test runners, so > extra checks are employed to not double-define them. Similarly, we have > redefinition guards for output directories and test headers. > test_verifier follows slightly different patterns and is simple enough > to not justify generalizing TEST_RUNNER_DEFINE/TEST_RUNNER_DEFINE_RULES > further to accomodate these differences. Instead, rules for > test_verifier are minimized and simplified, while preserving correctness > of dependencies. > Signed-off-by: Andrii Nakryiko <andriin@fb.com> > --- > tools/testing/selftests/bpf/.gitignore | 5 +- > tools/testing/selftests/bpf/Makefile | 313 ++++++++++++++----------- > 2 files changed, 180 insertions(+), 138 deletions(-) > diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore > index 7470327edcfe..c51f356f84b5 100644 > --- a/tools/testing/selftests/bpf/.gitignore > +++ b/tools/testing/selftests/bpf/.gitignore > @@ -7,7 +7,7 @@ FEATURE-DUMP.libbpf > fixdep > test_align > test_dev_cgroup > -test_progs > +/test_progs* > test_tcpbpf_user > test_verifier_log > feature > @@ -33,9 +33,10 @@ test_tcpnotify_user > test_libbpf > test_tcp_check_syncookie_user > test_sysctl > -alu32 > libbpf.pc > libbpf.so.* > test_hashmap > test_btf_dump > xdping > +/alu32 > +/bpf_gcc > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > index fbced23935cc..c9f43d49eac9 100644 > --- a/tools/testing/selftests/bpf/Makefile > +++ b/tools/testing/selftests/bpf/Makefile > @@ -2,10 +2,12 @@ > include ../../../../scripts/Kbuild.include > include ../../../scripts/Makefile.arch > -LIBDIR := ../../../lib > +CURDIR := $(abspath .) > +LIBDIR := $(abspath ../../../lib) > BPFDIR := $(LIBDIR)/bpf > -APIDIR := ../../../include/uapi > -GENDIR := ../../../../include/generated > +TOOLSDIR := $(abspath ../../../include) > +APIDIR := $(TOOLSDIR)/uapi > +GENDIR := $(abspath ../../../../include/generated) > GENHDR := $(GENDIR)/autoconf.h > ifneq ($(wildcard $(GENHDR)),) > @@ -16,8 +18,9 @@ CLANG ?= clang > LLC ?= llc > LLVM_OBJCOPY ?= llvm-objcopy > BPF_GCC ?= $(shell command -v bpf-gcc;) > -CFLAGS += -g -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(BPFDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include \ > - -Dbpf_prog_load=bpf_prog_test_load \ > +CFLAGS += -g -Wall -O2 $(GENFLAGS) -I$(APIDIR) -I$(LIBDIR) -I$(BPFDIR) \ > + -I$(GENDIR) -I$(TOOLSDIR) -I$(CURDIR) \ > + -Dbpf_prog_load=bpf_prog_test_load \ > -Dbpf_load_program=bpf_test_load_program > LDLIBS += -lcap -lelf -lrt -lpthread > @@ -29,12 +32,6 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test > test_netcnt test_tcpnotify_user test_sock_fields test_sysctl test_hashmap \ > test_cgroup_attach xdping > -BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c))) > -TEST_GEN_FILES = $(BPF_OBJ_FILES) > - > -BTF_C_FILES = $(wildcard progs/btf_dump_test_case_*.c) > -TEST_FILES = $(BTF_C_FILES) > - > # Also test sub-register code-gen if LLVM has eBPF v3 processor support which > # contains both ALU32 and JMP32 instructions. > SUBREG_CODEGEN := $(shell echo "int cal(int a) { return a > 0; }" | \ > @@ -42,13 +39,17 @@ SUBREG_CODEGEN := $(shell echo "int cal(int a) { return a > 0; }" | \ > $(LLC) -mattr=+alu32 -mcpu=v3 2>&1 | \ > grep 'if w') > ifneq ($(SUBREG_CODEGEN),) > -TEST_GEN_FILES += $(patsubst %.o,alu32/%.o, $(BPF_OBJ_FILES)) > +TEST_GEN_PROGS += test_progs-alu32 > endif > +# Also test bpf-gcc, if present > ifneq ($(BPF_GCC),) > -TEST_GEN_FILES += $(patsubst %.o,bpf_gcc/%.o, $(BPF_OBJ_FILES)) > +TEST_GEN_PROGS += test_progs-bpf_gcc > endif > +TEST_GEN_FILES = > +TEST_FILES = > + > # Order correspond to 'make run_tests' order > TEST_PROGS := test_kmod.sh \ > test_libbpf.sh \ > @@ -82,6 +83,8 @@ TEST_GEN_PROGS_EXTENDED = test_libbpf_open test_sock_addr test_skb_cgroup_id_use > flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \ > test_lirc_mode2_user > +TEST_CUSTOM_PROGS = urandom_read > + > include ../lib.mk > # Define simple and short `make test_progs`, `make test_sysctl`, etc targets > @@ -94,21 +97,12 @@ $(notdir $(TEST_GEN_PROGS) \ > $(TEST_GEN_PROGS_EXTENDED) \ > $(TEST_CUSTOM_PROGS)): %: $(OUTPUT)/% ; > -# NOTE: $(OUTPUT) won't get default value if used before lib.mk > -TEST_CUSTOM_PROGS = $(OUTPUT)/urandom_read > -all: $(TEST_CUSTOM_PROGS) > - > -$(OUTPUT)/urandom_read: $(OUTPUT)/%: %.c > +$(OUTPUT)/urandom_read: urandom_read.c > $(CC) -o $@ $< -Wl,--build-id > -$(OUTPUT)/test_stub.o: test_stub.c > - $(CC) $(TEST_PROGS_CFLAGS) $(CFLAGS) -c -o $@ $< > - > BPFOBJ := $(OUTPUT)/libbpf.a > -$(TEST_GEN_PROGS): $(OUTPUT)/test_stub.o $(BPFOBJ) > - > -$(TEST_GEN_PROGS_EXTENDED): $(OUTPUT)/test_stub.o $(OUTPUT)/libbpf.a > +$(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED): $(OUTPUT)/test_stub.o $(BPFOBJ) > $(OUTPUT)/test_dev_cgroup: cgroup_helpers.c > $(OUTPUT)/test_skb_cgroup_id_user: cgroup_helpers.c > @@ -118,7 +112,6 @@ $(OUTPUT)/test_socket_cookie: cgroup_helpers.c > $(OUTPUT)/test_sockmap: cgroup_helpers.c > $(OUTPUT)/test_tcpbpf_user: cgroup_helpers.c > $(OUTPUT)/test_tcpnotify_user: cgroup_helpers.c trace_helpers.c > -$(OUTPUT)/test_progs: cgroup_helpers.c trace_helpers.c > $(OUTPUT)/get_cgroup_id_user: cgroup_helpers.c > $(OUTPUT)/test_cgroup_storage: cgroup_helpers.c > $(OUTPUT)/test_netcnt: cgroup_helpers.c > @@ -134,6 +127,10 @@ force: > $(BPFOBJ): force > $(MAKE) -C $(BPFDIR) OUTPUT=$(OUTPUT)/ > +BPF_HELPERS := $(BPFDIR)/bpf_helper_defs.h $(wildcard $(BPFDIR)/bpf_*.h) > +$(BPFDIR)/bpf_helper_defs.h: > + $(MAKE) -C $(BPFDIR) OUTPUT=$(OUTPUT)/ bpf_helper_defs.h > + > # Get Clang's default includes on this system, as opposed to those seen by > # '-target bpf'. This fixes "missing" files on some architectures/distros, > # such as asm/byteorder.h, asm/socket.h, asm/sockios.h, sys/cdefs.h etc. > @@ -144,10 +141,11 @@ define get_sys_includes > $(shell $(1) -v -E - </dev/null 2>&1 \ > | sed -n '/<...> search starts here:/,/End of search list./{ s| \(/.*\)|-idirafter \1|p }') > endef > + > CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG)) > BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH) \ > - -I. -I./include/uapi -I../../../include/uapi \ > - -I$(BPFDIR) -I$(OUTPUT)/../usr/include > + -I. -I./include/uapi -I$(APIDIR) \ > + -I$(BPFDIR) -I$(abspath $(OUTPUT)/../usr/include) > CLANG_CFLAGS = $(CLANG_SYS_INCLUDES) \ > -Wno-compare-distinct-pointer-types > @@ -159,127 +157,170 @@ $(OUTPUT)/test_queue_map.o: test_queue_stack_map.h > $(OUTPUT)/test_stack_map.o: test_queue_stack_map.h > $(OUTPUT)/flow_dissector_load.o: flow_dissector_load.h > -$(OUTPUT)/test_progs.o: flow_dissector_load.h > -TEST_PROGS_CFLAGS := -I. -I$(OUTPUT) > -TEST_MAPS_CFLAGS := -I. -I$(OUTPUT) > -TEST_VERIFIER_CFLAGS := -I. -I$(OUTPUT) -Iverifier > +# Build BPF object using Clang > +# $1 - input .c file > +# $2 - output .o file > +# $3 - CFLAGS > +# $4 - LDFLAGS > +define CLANG_BPF_BUILD_RULE > + ($(CLANG) $3 -O2 -target bpf -emit-llvm \ > + -c $1 -o - || echo "BPF obj compilation failed") | \ > + $(LLC) -march=bpf -mcpu=probe $4 -filetype=obj -o $2 > +endef > +# Similar to CLANG_BPF_BUILD_RULE, but using native Clang and bpf LLC > +define CLANG_NATIVE_BPF_BUILD_RULE > + ($(CLANG) $3 -O2 -emit-llvm \ > + -c $1 -o - || echo "BPF obj compilation failed") | \ > + $(LLC) -march=bpf -mcpu=probe $4 -filetype=obj -o $2 > +endef > +# Build BPF object using GCC > +define GCC_BPF_BUILD_RULE > + $(BPF_GCC) $3 $4 -O2 -c $1 -o $2 > +endef > + > +# Set up extra TRUNNER_XXX "temporary" variables in the environment (relies on > +# $eval()) and pass control to DEFINE_TEST_RUNNER_RULES. > +# Parameters: > +# $1 - test runner base binary name (e.g., test_progs) > +# $2 - test runner extra "flavor" (e.g., alu32, gcc-bpf, etc) > +define DEFINE_TEST_RUNNER > + > +TRUNNER_OUTPUT := $(OUTPUT)$(if $2,/)$2 > +TRUNNER_BINARY := $1$(if $2,-)$2 > +TRUNNER_TEST_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.test.o, \ > + $$(notdir $$(wildcard $(TRUNNER_TESTS_DIR)/*.c))) > +TRUNNER_EXTRA_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.o, \ > + $$(filter %.c,$(TRUNNER_EXTRA_SOURCES))) > +TRUNNER_EXTRA_HDRS := $$(filter %.h,$(TRUNNER_EXTRA_SOURCES)) > +TRUNNER_TESTS_HDR := $(TRUNNER_TESTS_DIR)/tests.h > +TRUNNER_BPF_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.o, \ > + $$(notdir $$(wildcard $(TRUNNER_BPF_PROGS_DIR)/*.c))) > + > +# Evaluate rules now with extra TRUNNER_XXX variables above already defined > +$$(eval $$(call DEFINE_TEST_RUNNER_RULES,$1,$2)) > + > +endef > + > +# Using TRUNNER_XXX variables, provided by callers of DEFINE_TEST_RUNNER and > +# set up by DEFINE_TEST_RUNNER itself, create test runner build rules with: > +# $1 - test runner base binary name (e.g., test_progs) > +# $2 - test runner extra "flavor" (e.g., alu32, gcc-bpf, etc) > +define DEFINE_TEST_RUNNER_RULES > +ifeq ($($(TRUNNER_OUTPUT)-dir),) > +$(TRUNNER_OUTPUT)-dir := y > +$(TRUNNER_OUTPUT): > + mkdir -p $$@ > +endif > + > +# ensure we set up BPF objects generation rule just once for a given > +# input/output directory combination > +ifeq ($($(TRUNNER_BPF_PROGS_DIR)$(if $2,-)$2-bpfobjs),) > +$(TRUNNER_BPF_PROGS_DIR)$(if $2,-)$2-bpfobjs := y > +$(TRUNNER_BPF_OBJS): $(TRUNNER_OUTPUT)/%.o: \ > + $(TRUNNER_BPF_PROGS_DIR)/%.c \ > + $(TRUNNER_BPF_PROGS_DIR)/*.h \ > + $$(BPF_HELPERS) | $(TRUNNER_OUTPUT) > + $$(call $(TRUNNER_BPF_BUILD_RULE),$$<,$$@, \ > + $(TRUNNER_BPF_CFLAGS), \ > + $(TRUNNER_BPF_LDFLAGS)) > +endif > + > +# ensure we set up tests.h header generation rule just once > +ifeq ($($(TRUNNER_TESTS_DIR)-tests-hdr),) > +$(TRUNNER_TESTS_DIR)-tests-hdr := y > +$(TRUNNER_TESTS_HDR): $(TRUNNER_TESTS_DIR)/*.c > + $$(shell ( cd $(TRUNNER_TESTS_DIR); \ > + echo '/* Generated header, do not edit */'; \ > + ls *.c 2> /dev/null | \ > + sed -e 's@\([^\.]*\)\.c@DEFINE_TEST(\1)@'; \ > + ) > $$@) > +endif > + > +# compile individual test files > +# Note: we cd into output directory to ensure embedded BPF object is found > +$(TRUNNER_TEST_OBJS): $(TRUNNER_OUTPUT)/%.test.o: \ > + $(TRUNNER_TESTS_DIR)/%.c \ > + $(TRUNNER_EXTRA_HDRS) \ > + $(TRUNNER_BPF_OBJS) \ > + $$(BPFOBJ) | $(TRUNNER_OUTPUT) > + cd $$(@D) && $$(CC) $$(CFLAGS) $$(LDLIBS) -c $(CURDIR)/$$< -o $$(@F) > + > +$(TRUNNER_EXTRA_OBJS): $(TRUNNER_OUTPUT)/%.o: \ > + %.c \ > + $(TRUNNER_EXTRA_HDRS) \ > + $(TRUNNER_TESTS_HDR) \ > + $$(BPFOBJ) | $(TRUNNER_OUTPUT) > + $$(CC) $$(CFLAGS) $$(LDLIBS) -c $$< -o $$@ > + > +$(TRUNNER_BINARY)-extras: $(TRUNNER_EXTRA_FILES) | $(TRUNNER_OUTPUT) > +ifneq ($2,) > + # only copy extra resources if in flavored build > + cp -a $$^ $(TRUNNER_OUTPUT)/ > +endif > + > +$(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS) \ > + $(TRUNNER_EXTRA_OBJS) $$(BPFOBJ) \ > + | $(TRUNNER_BINARY)-extras > + $$(CC) $$(CFLAGS) $$(LDLIBS) $$(filter %.a %.o,$$^) -o $$@ > + > +endef > + > +# Define test_progs test runner. > +TRUNNER_TESTS_DIR := prog_tests > +TRUNNER_BPF_PROGS_DIR := progs > +TRUNNER_EXTRA_SOURCES := test_progs.c cgroup_helpers.c trace_helpers.c \ > + flow_dissector_load.h > +TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read \ > + $(wildcard progs/btf_dump_test_case_*.c) > +TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE > +TRUNNER_BPF_CFLAGS := -I. -I$(OUTPUT) $(BPF_CFLAGS) $(CLANG_CFLAGS) > +TRUNNER_BPF_LDFLAGS := > +$(eval $(call DEFINE_TEST_RUNNER,test_progs)) > + > +# Define test_progs-alu32 test runner. > ifneq ($(SUBREG_CODEGEN),) > -ALU32_BUILD_DIR = $(OUTPUT)/alu32 > -TEST_CUSTOM_PROGS += $(ALU32_BUILD_DIR)/test_progs_32 > -$(ALU32_BUILD_DIR): > - mkdir -p $@ > - > -$(ALU32_BUILD_DIR)/urandom_read: $(OUTPUT)/urandom_read | $(ALU32_BUILD_DIR) > - cp $< $@ > - > -$(ALU32_BUILD_DIR)/test_progs_32: test_progs.c $(OUTPUT)/libbpf.a\ > - $(ALU32_BUILD_DIR)/urandom_read \ > - | $(ALU32_BUILD_DIR) > - $(CC) $(TEST_PROGS_CFLAGS) $(CFLAGS) \ > - -o $(ALU32_BUILD_DIR)/test_progs_32 \ > - test_progs.c test_stub.c cgroup_helpers.c trace_helpers.c prog_tests/*.c \ > - $(OUTPUT)/libbpf.a $(LDLIBS) > - > -$(ALU32_BUILD_DIR)/test_progs_32: $(PROG_TESTS_H) > -$(ALU32_BUILD_DIR)/test_progs_32: prog_tests/*.c > - > -$(ALU32_BUILD_DIR)/%.o: progs/%.c $(ALU32_BUILD_DIR)/test_progs_32 \ > - | $(ALU32_BUILD_DIR) > - ($(CLANG) $(BPF_CFLAGS) $(CLANG_CFLAGS) -O2 -target bpf -emit-llvm \ > - -c $< -o - || echo "clang failed") | \ > - $(LLC) -march=bpf -mcpu=probe -mattr=+alu32 $(LLC_FLAGS) \ > - -filetype=obj -o $@ > +TRUNNER_BPF_LDFLAGS += -mattr=+alu32 > +$(eval $(call DEFINE_TEST_RUNNER,test_progs,alu32)) > endif > +# Define test_progs BPF-GCC-flavored test runner. > ifneq ($(BPF_GCC),) > -GCC_SYS_INCLUDES = $(call get_sys_includes,gcc) > IS_LITTLE_ENDIAN = $(shell $(CC) -dM -E - </dev/null | \ > grep 'define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__') > -ifeq ($(IS_LITTLE_ENDIAN),) > -MENDIAN=-mbig-endian > -else > -MENDIAN=-mlittle-endian > -endif > -BPF_GCC_CFLAGS = $(GCC_SYS_INCLUDES) $(MENDIAN) > -BPF_GCC_BUILD_DIR = $(OUTPUT)/bpf_gcc > -TEST_CUSTOM_PROGS += $(BPF_GCC_BUILD_DIR)/test_progs_bpf_gcc > -$(BPF_GCC_BUILD_DIR): > - mkdir -p $@ > - > -$(BPF_GCC_BUILD_DIR)/urandom_read: $(OUTPUT)/urandom_read | $(BPF_GCC_BUILD_DIR) > - cp $< $@ > - > -$(BPF_GCC_BUILD_DIR)/test_progs_bpf_gcc: $(OUTPUT)/test_progs \ > - | $(BPF_GCC_BUILD_DIR) > - cp $< $@ > - > -$(BPF_GCC_BUILD_DIR)/%.o: progs/%.c $(BPF_GCC_BUILD_DIR)/test_progs_bpf_gcc \ > - | $(BPF_GCC_BUILD_DIR) > - $(BPF_GCC) $(BPF_CFLAGS) $(BPF_GCC_CFLAGS) -O2 -c $< -o $@ > +MENDIAN=$(if $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian) > + > +TRUNNER_BPF_BUILD_RULE := GCC_BPF_BUILD_RULE > +TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(call get_sys_includes,gcc) $(MENDIAN) > +TRUNNER_BPF_LDFLAGS := > +$(eval $(call DEFINE_TEST_RUNNER,test_progs,bpf_gcc)) > endif > -# Have one program compiled without "-target bpf" to test whether libbpf loads > -# it successfully > -$(OUTPUT)/test_xdp.o: progs/test_xdp.c > - ($(CLANG) $(BPF_CFLAGS) $(CLANG_CFLAGS) -O2 -emit-llvm -c $< -o - || \ > - echo "clang failed") | \ > - $(LLC) -march=bpf -mcpu=probe $(LLC_FLAGS) -filetype=obj -o $@ > - > -# libbpf has to be built before BPF programs due to bpf_helper_defs.h > -$(OUTPUT)/%.o: progs/%.c | $(BPFOBJ) > - ($(CLANG) $(BPF_CFLAGS) $(CLANG_CFLAGS) -O2 -target bpf -emit-llvm \ > - -c $< -o - || echo "clang failed") | \ > - $(LLC) -march=bpf -mcpu=probe $(LLC_FLAGS) -filetype=obj -o $@ > - > -PROG_TESTS_DIR = $(OUTPUT)/prog_tests > -$(PROG_TESTS_DIR): > - mkdir -p $@ > -PROG_TESTS_H := $(PROG_TESTS_DIR)/tests.h > -PROG_TESTS_FILES := $(wildcard prog_tests/*.c) > -test_progs.c: $(PROG_TESTS_H) > -$(OUTPUT)/test_progs: CFLAGS += $(TEST_PROGS_CFLAGS) > -$(OUTPUT)/test_progs: test_progs.c $(PROG_TESTS_FILES) | $(OUTPUT)/test_attach_probe.o $(PROG_TESTS_H) > -$(PROG_TESTS_H): $(PROG_TESTS_FILES) | $(PROG_TESTS_DIR) > - $(shell ( cd prog_tests/; \ > - echo '/* Generated header, do not edit */'; \ > - ls *.c 2> /dev/null | \ > - sed -e 's@\([^\.]*\)\.c@DEFINE_TEST(\1)@'; \ > - ) > $(PROG_TESTS_H)) > - > -MAP_TESTS_DIR = $(OUTPUT)/map_tests > -$(MAP_TESTS_DIR): > - mkdir -p $@ > -MAP_TESTS_H := $(MAP_TESTS_DIR)/tests.h > -MAP_TESTS_FILES := $(wildcard map_tests/*.c) > -test_maps.c: $(MAP_TESTS_H) > -$(OUTPUT)/test_maps: CFLAGS += $(TEST_MAPS_CFLAGS) > -$(OUTPUT)/test_maps: test_maps.c $(MAP_TESTS_FILES) | $(MAP_TESTS_H) > -$(MAP_TESTS_H): $(MAP_TESTS_FILES) | $(MAP_TESTS_DIR) > - $(shell ( cd map_tests/; \ > - echo '/* Generated header, do not edit */'; \ > - ls *.c 2> /dev/null | \ > - sed -e 's@\([^\.]*\)\.c@DEFINE_TEST(\1)@'; \ > - ) > $(MAP_TESTS_H)) > - > -VERIFIER_TESTS_DIR = $(OUTPUT)/verifier > -$(VERIFIER_TESTS_DIR): > - mkdir -p $@ > -VERIFIER_TESTS_H := $(VERIFIER_TESTS_DIR)/tests.h > -VERIFIER_TEST_FILES := $(wildcard verifier/*.c) > -test_verifier.c: $(VERIFIER_TESTS_H) > -$(OUTPUT)/test_verifier: CFLAGS += $(TEST_VERIFIER_CFLAGS) > -$(OUTPUT)/test_verifier: test_verifier.c | $(VERIFIER_TEST_FILES) $(VERIFIER_TESTS_H) > -$(VERIFIER_TESTS_H): $(VERIFIER_TEST_FILES) | $(VERIFIER_TESTS_DIR) > +# Define test_maps test runner. > +TRUNNER_TESTS_DIR := map_tests > +TRUNNER_BPF_PROGS_DIR := progs > +TRUNNER_EXTRA_SOURCES := test_maps.c > +TRUNNER_EXTRA_FILES := > +TRUNNER_BPF_BUILD_RULE := $$(error no BPF objects should be built) > +TRUNNER_BPF_CFLAGS := > +TRUNNER_BPF_LDFLAGS := > +$(eval $(call DEFINE_TEST_RUNNER,test_maps)) > + > +# Define test_verifier test runner. > +# It is much simpler than test_maps/test_progs and sufficiently different from > +# them (e.g., test.h is using completely pattern), that it's worth just > +# explicitly defining all the rules explicitly. > +verifier/tests.h: verifier/*.c > $(shell ( cd verifier/; \ > echo '/* Generated header, do not edit */'; \ > echo '#ifdef FILL_ARRAY'; \ > - ls *.c 2> /dev/null | \ > - sed -e 's@\(.*\)@#include \"\1\"@'; \ > + ls *.c 2> /dev/null | sed -e 's@\(.*\)@#include \"\1\"@'; \ > echo '#endif' \ > - ) > $(VERIFIER_TESTS_H)) > + ) > verifier/tests.h) > +$(OUTPUT)/test_verifier: test_verifier.c verifier/tests.h $(BPFOBJ) | $(OUTPUT) > + $(CC) $(CFLAGS) $(LDLIBS) $(filter %.a %.o %.c,$^) -o $@ > -EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(ALU32_BUILD_DIR) $(BPF_GCC_BUILD_DIR) \ > - $(VERIFIER_TESTS_H) $(PROG_TESTS_H) $(MAP_TESTS_H) \ > - feature > +EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) \ > + prog_tests/tests.h map_tests/tests.h verifier/tests.h \ > + feature $(OUTPUT)/*.o $(OUTPUT)/alu32 $(OUTPUT)/bpf_gcc > -- > 2.17.1
On Tue, May 12, 2020 at 1:16 PM Yauheni Kaliuta <yauheni.kaliuta@redhat.com> wrote: > > Hi, Andrii! > > The patch blanks TEST_GEN_FILES which was used by install target > (lib.mk) to install test progs. How is it supposed to work? > I actually never used install for selftests, just make and then run individual test binaries, which explains why this doesn't work :) > That fixes it for me btw: > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > index 8f25966b500b..1f878dcd2bf6 100644 > --- a/tools/testing/selftests/bpf/Makefile > +++ b/tools/testing/selftests/bpf/Makefile > @@ -265,6 +265,7 @@ TRUNNER_BPF_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.o, $$(TRUNNER_BPF_SRCS) > TRUNNER_BPF_SKELS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.skel.h, \ > $$(filter-out $(SKEL_BLACKLIST), \ > $$(TRUNNER_BPF_SRCS))) > +TEST_GEN_FILES += $$(TRUNNER_BPF_OBJS) Yeah, this makes sense, these files will be copied over along the compiled test_xxx binaries. Do you mind submitting a patch? [...]
Hi, Andrii! >>>>> On Tue, 12 May 2020 15:13:18 -0700, Andrii Nakryiko wrote: > On Tue, May 12, 2020 at 1:16 PM Yauheni Kaliuta > <yauheni.kaliuta@redhat.com> wrote: >> >> Hi, Andrii! >> >> The patch blanks TEST_GEN_FILES which was used by install target >> (lib.mk) to install test progs. How is it supposed to work? >> > I actually never used install for selftests, just make and > then run individual test binaries, which explains why this > doesn't work :) Ok :) Thanks for the clarification. >> That fixes it for me btw: >> >> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile >> index 8f25966b500b..1f878dcd2bf6 100644 >> --- a/tools/testing/selftests/bpf/Makefile >> +++ b/tools/testing/selftests/bpf/Makefile >> @@ -265,6 +265,7 @@ TRUNNER_BPF_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.o, $$(TRUNNER_BPF_SRCS) >> TRUNNER_BPF_SKELS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.skel.h, \ >> $$(filter-out $(SKEL_BLACKLIST), \ >> $$(TRUNNER_BPF_SRCS))) >> +TEST_GEN_FILES += $$(TRUNNER_BPF_OBJS) > Yeah, this makes sense, these files will be copied over along > the compiled test_xxx binaries. Do you mind submitting a > patch? No, sure, I'll do.
diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore index 7470327edcfe..c51f356f84b5 100644 --- a/tools/testing/selftests/bpf/.gitignore +++ b/tools/testing/selftests/bpf/.gitignore @@ -7,7 +7,7 @@ FEATURE-DUMP.libbpf fixdep test_align test_dev_cgroup -test_progs +/test_progs* test_tcpbpf_user test_verifier_log feature @@ -33,9 +33,10 @@ test_tcpnotify_user test_libbpf test_tcp_check_syncookie_user test_sysctl -alu32 libbpf.pc libbpf.so.* test_hashmap test_btf_dump xdping +/alu32 +/bpf_gcc diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index fbced23935cc..c9f43d49eac9 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -2,10 +2,12 @@ include ../../../../scripts/Kbuild.include include ../../../scripts/Makefile.arch -LIBDIR := ../../../lib +CURDIR := $(abspath .) +LIBDIR := $(abspath ../../../lib) BPFDIR := $(LIBDIR)/bpf -APIDIR := ../../../include/uapi -GENDIR := ../../../../include/generated +TOOLSDIR := $(abspath ../../../include) +APIDIR := $(TOOLSDIR)/uapi +GENDIR := $(abspath ../../../../include/generated) GENHDR := $(GENDIR)/autoconf.h ifneq ($(wildcard $(GENHDR)),) @@ -16,8 +18,9 @@ CLANG ?= clang LLC ?= llc LLVM_OBJCOPY ?= llvm-objcopy BPF_GCC ?= $(shell command -v bpf-gcc;) -CFLAGS += -g -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(BPFDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include \ - -Dbpf_prog_load=bpf_prog_test_load \ +CFLAGS += -g -Wall -O2 $(GENFLAGS) -I$(APIDIR) -I$(LIBDIR) -I$(BPFDIR) \ + -I$(GENDIR) -I$(TOOLSDIR) -I$(CURDIR) \ + -Dbpf_prog_load=bpf_prog_test_load \ -Dbpf_load_program=bpf_test_load_program LDLIBS += -lcap -lelf -lrt -lpthread @@ -29,12 +32,6 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test test_netcnt test_tcpnotify_user test_sock_fields test_sysctl test_hashmap \ test_cgroup_attach xdping -BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c))) -TEST_GEN_FILES = $(BPF_OBJ_FILES) - -BTF_C_FILES = $(wildcard progs/btf_dump_test_case_*.c) -TEST_FILES = $(BTF_C_FILES) - # Also test sub-register code-gen if LLVM has eBPF v3 processor support which # contains both ALU32 and JMP32 instructions. SUBREG_CODEGEN := $(shell echo "int cal(int a) { return a > 0; }" | \ @@ -42,13 +39,17 @@ SUBREG_CODEGEN := $(shell echo "int cal(int a) { return a > 0; }" | \ $(LLC) -mattr=+alu32 -mcpu=v3 2>&1 | \ grep 'if w') ifneq ($(SUBREG_CODEGEN),) -TEST_GEN_FILES += $(patsubst %.o,alu32/%.o, $(BPF_OBJ_FILES)) +TEST_GEN_PROGS += test_progs-alu32 endif +# Also test bpf-gcc, if present ifneq ($(BPF_GCC),) -TEST_GEN_FILES += $(patsubst %.o,bpf_gcc/%.o, $(BPF_OBJ_FILES)) +TEST_GEN_PROGS += test_progs-bpf_gcc endif +TEST_GEN_FILES = +TEST_FILES = + # Order correspond to 'make run_tests' order TEST_PROGS := test_kmod.sh \ test_libbpf.sh \ @@ -82,6 +83,8 @@ TEST_GEN_PROGS_EXTENDED = test_libbpf_open test_sock_addr test_skb_cgroup_id_use flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \ test_lirc_mode2_user +TEST_CUSTOM_PROGS = urandom_read + include ../lib.mk # Define simple and short `make test_progs`, `make test_sysctl`, etc targets @@ -94,21 +97,12 @@ $(notdir $(TEST_GEN_PROGS) \ $(TEST_GEN_PROGS_EXTENDED) \ $(TEST_CUSTOM_PROGS)): %: $(OUTPUT)/% ; -# NOTE: $(OUTPUT) won't get default value if used before lib.mk -TEST_CUSTOM_PROGS = $(OUTPUT)/urandom_read -all: $(TEST_CUSTOM_PROGS) - -$(OUTPUT)/urandom_read: $(OUTPUT)/%: %.c +$(OUTPUT)/urandom_read: urandom_read.c $(CC) -o $@ $< -Wl,--build-id -$(OUTPUT)/test_stub.o: test_stub.c - $(CC) $(TEST_PROGS_CFLAGS) $(CFLAGS) -c -o $@ $< - BPFOBJ := $(OUTPUT)/libbpf.a -$(TEST_GEN_PROGS): $(OUTPUT)/test_stub.o $(BPFOBJ) - -$(TEST_GEN_PROGS_EXTENDED): $(OUTPUT)/test_stub.o $(OUTPUT)/libbpf.a +$(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED): $(OUTPUT)/test_stub.o $(BPFOBJ) $(OUTPUT)/test_dev_cgroup: cgroup_helpers.c $(OUTPUT)/test_skb_cgroup_id_user: cgroup_helpers.c @@ -118,7 +112,6 @@ $(OUTPUT)/test_socket_cookie: cgroup_helpers.c $(OUTPUT)/test_sockmap: cgroup_helpers.c $(OUTPUT)/test_tcpbpf_user: cgroup_helpers.c $(OUTPUT)/test_tcpnotify_user: cgroup_helpers.c trace_helpers.c -$(OUTPUT)/test_progs: cgroup_helpers.c trace_helpers.c $(OUTPUT)/get_cgroup_id_user: cgroup_helpers.c $(OUTPUT)/test_cgroup_storage: cgroup_helpers.c $(OUTPUT)/test_netcnt: cgroup_helpers.c @@ -134,6 +127,10 @@ force: $(BPFOBJ): force $(MAKE) -C $(BPFDIR) OUTPUT=$(OUTPUT)/ +BPF_HELPERS := $(BPFDIR)/bpf_helper_defs.h $(wildcard $(BPFDIR)/bpf_*.h) +$(BPFDIR)/bpf_helper_defs.h: + $(MAKE) -C $(BPFDIR) OUTPUT=$(OUTPUT)/ bpf_helper_defs.h + # Get Clang's default includes on this system, as opposed to those seen by # '-target bpf'. This fixes "missing" files on some architectures/distros, # such as asm/byteorder.h, asm/socket.h, asm/sockios.h, sys/cdefs.h etc. @@ -144,10 +141,11 @@ define get_sys_includes $(shell $(1) -v -E - </dev/null 2>&1 \ | sed -n '/<...> search starts here:/,/End of search list./{ s| \(/.*\)|-idirafter \1|p }') endef + CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG)) BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH) \ - -I. -I./include/uapi -I../../../include/uapi \ - -I$(BPFDIR) -I$(OUTPUT)/../usr/include + -I. -I./include/uapi -I$(APIDIR) \ + -I$(BPFDIR) -I$(abspath $(OUTPUT)/../usr/include) CLANG_CFLAGS = $(CLANG_SYS_INCLUDES) \ -Wno-compare-distinct-pointer-types @@ -159,127 +157,170 @@ $(OUTPUT)/test_queue_map.o: test_queue_stack_map.h $(OUTPUT)/test_stack_map.o: test_queue_stack_map.h $(OUTPUT)/flow_dissector_load.o: flow_dissector_load.h -$(OUTPUT)/test_progs.o: flow_dissector_load.h -TEST_PROGS_CFLAGS := -I. -I$(OUTPUT) -TEST_MAPS_CFLAGS := -I. -I$(OUTPUT) -TEST_VERIFIER_CFLAGS := -I. -I$(OUTPUT) -Iverifier +# Build BPF object using Clang +# $1 - input .c file +# $2 - output .o file +# $3 - CFLAGS +# $4 - LDFLAGS +define CLANG_BPF_BUILD_RULE + ($(CLANG) $3 -O2 -target bpf -emit-llvm \ + -c $1 -o - || echo "BPF obj compilation failed") | \ + $(LLC) -march=bpf -mcpu=probe $4 -filetype=obj -o $2 +endef +# Similar to CLANG_BPF_BUILD_RULE, but using native Clang and bpf LLC +define CLANG_NATIVE_BPF_BUILD_RULE + ($(CLANG) $3 -O2 -emit-llvm \ + -c $1 -o - || echo "BPF obj compilation failed") | \ + $(LLC) -march=bpf -mcpu=probe $4 -filetype=obj -o $2 +endef +# Build BPF object using GCC +define GCC_BPF_BUILD_RULE + $(BPF_GCC) $3 $4 -O2 -c $1 -o $2 +endef + +# Set up extra TRUNNER_XXX "temporary" variables in the environment (relies on +# $eval()) and pass control to DEFINE_TEST_RUNNER_RULES. +# Parameters: +# $1 - test runner base binary name (e.g., test_progs) +# $2 - test runner extra "flavor" (e.g., alu32, gcc-bpf, etc) +define DEFINE_TEST_RUNNER + +TRUNNER_OUTPUT := $(OUTPUT)$(if $2,/)$2 +TRUNNER_BINARY := $1$(if $2,-)$2 +TRUNNER_TEST_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.test.o, \ + $$(notdir $$(wildcard $(TRUNNER_TESTS_DIR)/*.c))) +TRUNNER_EXTRA_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.o, \ + $$(filter %.c,$(TRUNNER_EXTRA_SOURCES))) +TRUNNER_EXTRA_HDRS := $$(filter %.h,$(TRUNNER_EXTRA_SOURCES)) +TRUNNER_TESTS_HDR := $(TRUNNER_TESTS_DIR)/tests.h +TRUNNER_BPF_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.o, \ + $$(notdir $$(wildcard $(TRUNNER_BPF_PROGS_DIR)/*.c))) + +# Evaluate rules now with extra TRUNNER_XXX variables above already defined +$$(eval $$(call DEFINE_TEST_RUNNER_RULES,$1,$2)) + +endef + +# Using TRUNNER_XXX variables, provided by callers of DEFINE_TEST_RUNNER and +# set up by DEFINE_TEST_RUNNER itself, create test runner build rules with: +# $1 - test runner base binary name (e.g., test_progs) +# $2 - test runner extra "flavor" (e.g., alu32, gcc-bpf, etc) +define DEFINE_TEST_RUNNER_RULES +ifeq ($($(TRUNNER_OUTPUT)-dir),) +$(TRUNNER_OUTPUT)-dir := y +$(TRUNNER_OUTPUT): + mkdir -p $$@ +endif + +# ensure we set up BPF objects generation rule just once for a given +# input/output directory combination +ifeq ($($(TRUNNER_BPF_PROGS_DIR)$(if $2,-)$2-bpfobjs),) +$(TRUNNER_BPF_PROGS_DIR)$(if $2,-)$2-bpfobjs := y +$(TRUNNER_BPF_OBJS): $(TRUNNER_OUTPUT)/%.o: \ + $(TRUNNER_BPF_PROGS_DIR)/%.c \ + $(TRUNNER_BPF_PROGS_DIR)/*.h \ + $$(BPF_HELPERS) | $(TRUNNER_OUTPUT) + $$(call $(TRUNNER_BPF_BUILD_RULE),$$<,$$@, \ + $(TRUNNER_BPF_CFLAGS), \ + $(TRUNNER_BPF_LDFLAGS)) +endif + +# ensure we set up tests.h header generation rule just once +ifeq ($($(TRUNNER_TESTS_DIR)-tests-hdr),) +$(TRUNNER_TESTS_DIR)-tests-hdr := y +$(TRUNNER_TESTS_HDR): $(TRUNNER_TESTS_DIR)/*.c + $$(shell ( cd $(TRUNNER_TESTS_DIR); \ + echo '/* Generated header, do not edit */'; \ + ls *.c 2> /dev/null | \ + sed -e 's@\([^\.]*\)\.c@DEFINE_TEST(\1)@'; \ + ) > $$@) +endif + +# compile individual test files +# Note: we cd into output directory to ensure embedded BPF object is found +$(TRUNNER_TEST_OBJS): $(TRUNNER_OUTPUT)/%.test.o: \ + $(TRUNNER_TESTS_DIR)/%.c \ + $(TRUNNER_EXTRA_HDRS) \ + $(TRUNNER_BPF_OBJS) \ + $$(BPFOBJ) | $(TRUNNER_OUTPUT) + cd $$(@D) && $$(CC) $$(CFLAGS) $$(LDLIBS) -c $(CURDIR)/$$< -o $$(@F) + +$(TRUNNER_EXTRA_OBJS): $(TRUNNER_OUTPUT)/%.o: \ + %.c \ + $(TRUNNER_EXTRA_HDRS) \ + $(TRUNNER_TESTS_HDR) \ + $$(BPFOBJ) | $(TRUNNER_OUTPUT) + $$(CC) $$(CFLAGS) $$(LDLIBS) -c $$< -o $$@ + +$(TRUNNER_BINARY)-extras: $(TRUNNER_EXTRA_FILES) | $(TRUNNER_OUTPUT) +ifneq ($2,) + # only copy extra resources if in flavored build + cp -a $$^ $(TRUNNER_OUTPUT)/ +endif + +$(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS) \ + $(TRUNNER_EXTRA_OBJS) $$(BPFOBJ) \ + | $(TRUNNER_BINARY)-extras + $$(CC) $$(CFLAGS) $$(LDLIBS) $$(filter %.a %.o,$$^) -o $$@ + +endef + +# Define test_progs test runner. +TRUNNER_TESTS_DIR := prog_tests +TRUNNER_BPF_PROGS_DIR := progs +TRUNNER_EXTRA_SOURCES := test_progs.c cgroup_helpers.c trace_helpers.c \ + flow_dissector_load.h +TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read \ + $(wildcard progs/btf_dump_test_case_*.c) +TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE +TRUNNER_BPF_CFLAGS := -I. -I$(OUTPUT) $(BPF_CFLAGS) $(CLANG_CFLAGS) +TRUNNER_BPF_LDFLAGS := +$(eval $(call DEFINE_TEST_RUNNER,test_progs)) + +# Define test_progs-alu32 test runner. ifneq ($(SUBREG_CODEGEN),) -ALU32_BUILD_DIR = $(OUTPUT)/alu32 -TEST_CUSTOM_PROGS += $(ALU32_BUILD_DIR)/test_progs_32 -$(ALU32_BUILD_DIR): - mkdir -p $@ - -$(ALU32_BUILD_DIR)/urandom_read: $(OUTPUT)/urandom_read | $(ALU32_BUILD_DIR) - cp $< $@ - -$(ALU32_BUILD_DIR)/test_progs_32: test_progs.c $(OUTPUT)/libbpf.a\ - $(ALU32_BUILD_DIR)/urandom_read \ - | $(ALU32_BUILD_DIR) - $(CC) $(TEST_PROGS_CFLAGS) $(CFLAGS) \ - -o $(ALU32_BUILD_DIR)/test_progs_32 \ - test_progs.c test_stub.c cgroup_helpers.c trace_helpers.c prog_tests/*.c \ - $(OUTPUT)/libbpf.a $(LDLIBS) - -$(ALU32_BUILD_DIR)/test_progs_32: $(PROG_TESTS_H) -$(ALU32_BUILD_DIR)/test_progs_32: prog_tests/*.c - -$(ALU32_BUILD_DIR)/%.o: progs/%.c $(ALU32_BUILD_DIR)/test_progs_32 \ - | $(ALU32_BUILD_DIR) - ($(CLANG) $(BPF_CFLAGS) $(CLANG_CFLAGS) -O2 -target bpf -emit-llvm \ - -c $< -o - || echo "clang failed") | \ - $(LLC) -march=bpf -mcpu=probe -mattr=+alu32 $(LLC_FLAGS) \ - -filetype=obj -o $@ +TRUNNER_BPF_LDFLAGS += -mattr=+alu32 +$(eval $(call DEFINE_TEST_RUNNER,test_progs,alu32)) endif +# Define test_progs BPF-GCC-flavored test runner. ifneq ($(BPF_GCC),) -GCC_SYS_INCLUDES = $(call get_sys_includes,gcc) IS_LITTLE_ENDIAN = $(shell $(CC) -dM -E - </dev/null | \ grep 'define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__') -ifeq ($(IS_LITTLE_ENDIAN),) -MENDIAN=-mbig-endian -else -MENDIAN=-mlittle-endian -endif -BPF_GCC_CFLAGS = $(GCC_SYS_INCLUDES) $(MENDIAN) -BPF_GCC_BUILD_DIR = $(OUTPUT)/bpf_gcc -TEST_CUSTOM_PROGS += $(BPF_GCC_BUILD_DIR)/test_progs_bpf_gcc -$(BPF_GCC_BUILD_DIR): - mkdir -p $@ - -$(BPF_GCC_BUILD_DIR)/urandom_read: $(OUTPUT)/urandom_read | $(BPF_GCC_BUILD_DIR) - cp $< $@ - -$(BPF_GCC_BUILD_DIR)/test_progs_bpf_gcc: $(OUTPUT)/test_progs \ - | $(BPF_GCC_BUILD_DIR) - cp $< $@ - -$(BPF_GCC_BUILD_DIR)/%.o: progs/%.c $(BPF_GCC_BUILD_DIR)/test_progs_bpf_gcc \ - | $(BPF_GCC_BUILD_DIR) - $(BPF_GCC) $(BPF_CFLAGS) $(BPF_GCC_CFLAGS) -O2 -c $< -o $@ +MENDIAN=$(if $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian) + +TRUNNER_BPF_BUILD_RULE := GCC_BPF_BUILD_RULE +TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(call get_sys_includes,gcc) $(MENDIAN) +TRUNNER_BPF_LDFLAGS := +$(eval $(call DEFINE_TEST_RUNNER,test_progs,bpf_gcc)) endif -# Have one program compiled without "-target bpf" to test whether libbpf loads -# it successfully -$(OUTPUT)/test_xdp.o: progs/test_xdp.c - ($(CLANG) $(BPF_CFLAGS) $(CLANG_CFLAGS) -O2 -emit-llvm -c $< -o - || \ - echo "clang failed") | \ - $(LLC) -march=bpf -mcpu=probe $(LLC_FLAGS) -filetype=obj -o $@ - -# libbpf has to be built before BPF programs due to bpf_helper_defs.h -$(OUTPUT)/%.o: progs/%.c | $(BPFOBJ) - ($(CLANG) $(BPF_CFLAGS) $(CLANG_CFLAGS) -O2 -target bpf -emit-llvm \ - -c $< -o - || echo "clang failed") | \ - $(LLC) -march=bpf -mcpu=probe $(LLC_FLAGS) -filetype=obj -o $@ - -PROG_TESTS_DIR = $(OUTPUT)/prog_tests -$(PROG_TESTS_DIR): - mkdir -p $@ -PROG_TESTS_H := $(PROG_TESTS_DIR)/tests.h -PROG_TESTS_FILES := $(wildcard prog_tests/*.c) -test_progs.c: $(PROG_TESTS_H) -$(OUTPUT)/test_progs: CFLAGS += $(TEST_PROGS_CFLAGS) -$(OUTPUT)/test_progs: test_progs.c $(PROG_TESTS_FILES) | $(OUTPUT)/test_attach_probe.o $(PROG_TESTS_H) -$(PROG_TESTS_H): $(PROG_TESTS_FILES) | $(PROG_TESTS_DIR) - $(shell ( cd prog_tests/; \ - echo '/* Generated header, do not edit */'; \ - ls *.c 2> /dev/null | \ - sed -e 's@\([^\.]*\)\.c@DEFINE_TEST(\1)@'; \ - ) > $(PROG_TESTS_H)) - -MAP_TESTS_DIR = $(OUTPUT)/map_tests -$(MAP_TESTS_DIR): - mkdir -p $@ -MAP_TESTS_H := $(MAP_TESTS_DIR)/tests.h -MAP_TESTS_FILES := $(wildcard map_tests/*.c) -test_maps.c: $(MAP_TESTS_H) -$(OUTPUT)/test_maps: CFLAGS += $(TEST_MAPS_CFLAGS) -$(OUTPUT)/test_maps: test_maps.c $(MAP_TESTS_FILES) | $(MAP_TESTS_H) -$(MAP_TESTS_H): $(MAP_TESTS_FILES) | $(MAP_TESTS_DIR) - $(shell ( cd map_tests/; \ - echo '/* Generated header, do not edit */'; \ - ls *.c 2> /dev/null | \ - sed -e 's@\([^\.]*\)\.c@DEFINE_TEST(\1)@'; \ - ) > $(MAP_TESTS_H)) - -VERIFIER_TESTS_DIR = $(OUTPUT)/verifier -$(VERIFIER_TESTS_DIR): - mkdir -p $@ -VERIFIER_TESTS_H := $(VERIFIER_TESTS_DIR)/tests.h -VERIFIER_TEST_FILES := $(wildcard verifier/*.c) -test_verifier.c: $(VERIFIER_TESTS_H) -$(OUTPUT)/test_verifier: CFLAGS += $(TEST_VERIFIER_CFLAGS) -$(OUTPUT)/test_verifier: test_verifier.c | $(VERIFIER_TEST_FILES) $(VERIFIER_TESTS_H) -$(VERIFIER_TESTS_H): $(VERIFIER_TEST_FILES) | $(VERIFIER_TESTS_DIR) +# Define test_maps test runner. +TRUNNER_TESTS_DIR := map_tests +TRUNNER_BPF_PROGS_DIR := progs +TRUNNER_EXTRA_SOURCES := test_maps.c +TRUNNER_EXTRA_FILES := +TRUNNER_BPF_BUILD_RULE := $$(error no BPF objects should be built) +TRUNNER_BPF_CFLAGS := +TRUNNER_BPF_LDFLAGS := +$(eval $(call DEFINE_TEST_RUNNER,test_maps)) + +# Define test_verifier test runner. +# It is much simpler than test_maps/test_progs and sufficiently different from +# them (e.g., test.h is using completely pattern), that it's worth just +# explicitly defining all the rules explicitly. +verifier/tests.h: verifier/*.c $(shell ( cd verifier/; \ echo '/* Generated header, do not edit */'; \ echo '#ifdef FILL_ARRAY'; \ - ls *.c 2> /dev/null | \ - sed -e 's@\(.*\)@#include \"\1\"@'; \ + ls *.c 2> /dev/null | sed -e 's@\(.*\)@#include \"\1\"@'; \ echo '#endif' \ - ) > $(VERIFIER_TESTS_H)) + ) > verifier/tests.h) +$(OUTPUT)/test_verifier: test_verifier.c verifier/tests.h $(BPFOBJ) | $(OUTPUT) + $(CC) $(CFLAGS) $(LDLIBS) $(filter %.a %.o %.c,$^) -o $@ -EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(ALU32_BUILD_DIR) $(BPF_GCC_BUILD_DIR) \ - $(VERIFIER_TESTS_H) $(PROG_TESTS_H) $(MAP_TESTS_H) \ - feature +EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) \ + prog_tests/tests.h map_tests/tests.h verifier/tests.h \ + feature $(OUTPUT)/*.o $(OUTPUT)/alu32 $(OUTPUT)/bpf_gcc
Define test runner generation meta-rule that codifies dependencies between test runner, its tests, and its dependent BPF programs. Use that for defining test_progs and test_maps test-runners. Also additionally define 2 flavors of test_progs: - alu32, which builds BPF programs with 32-bit registers codegen; - bpf_gcc, which build BPF programs using GCC, if it supports BPF target. Overall, this is accomplished through $(eval)'ing a set of generic rules, which defines Makefile targets dynamically at runtime. See comments explaining the need for 2 $(evals), though. For each test runner we have (test_maps and test_progs, currently), and, optionally, their flavors, the logic of build process is modeled as follows (using test_progs as an example): - all BPF objects are in progs/: - BPF object's .o file is built into output directory from corresponding progs/.c file; - all BPF objects in progs/*.c depend on all progs/*.h headers; - all BPF objects depend on bpf_*.h helpers from libbpf (but not libbpf archive). There is an extra rule to trigger bpf_helper_defs.h (re-)build, if it's not present/outdated); - build recipe for BPF object can be re-defined per test runner/flavor; - test files are built from prog_tests/*.c: - all such test file objects are built on individual file basis; - currently, every single test file depends on all BPF object files; this might be improved in follow up patches to do 1-to-1 dependency, but allowing to customize this per each individual test; - each test runner definition can specify a list of extra .c and .h files to be built along test files and test runner binary; all such headers are becoming automatic dependency of each test .c file; - due to test files sometimes embedding (using .incbin assembly directive) contents of some BPF objects at compilation time, which are expected to be in CWD of compiler, compilation for test file object does cd into test runner's output directory; to support this mode all the include paths are turned into absolute paths using $(abspath) make function; - prog_tests/test.h is automatically (re-)generated with an entry for each .c file in prog_tests/; - final test runner binary is linked together from test object files and extra object files, linking together libbpf's archive as well; - it's possible to specify extra "resource" files/targets, which will be copied into test runner output directory, if it differes from Makefile-wide $(OUTPUT). This is used to ensure btf_dump test cases and urandom_read binary is put into a test runner's CWD for tests to find them in runtime. For flavored test runners, their output directory is a subdirectory of common Makefile-wide $(OUTPUT) directory with flavor name used as subdirectory name. BPF objects targets might be reused between different test runners, so extra checks are employed to not double-define them. Similarly, we have redefinition guards for output directories and test headers. test_verifier follows slightly different patterns and is simple enough to not justify generalizing TEST_RUNNER_DEFINE/TEST_RUNNER_DEFINE_RULES further to accomodate these differences. Instead, rules for test_verifier are minimized and simplified, while preserving correctness of dependencies. Signed-off-by: Andrii Nakryiko <andriin@fb.com> --- tools/testing/selftests/bpf/.gitignore | 5 +- tools/testing/selftests/bpf/Makefile | 313 ++++++++++++++----------- 2 files changed, 180 insertions(+), 138 deletions(-)