[PATCH v4 bpf-next 0/7] Fix, clean up, and revamp selftests/bpf Makefile

[PATCH v4 bpf-next 0/7] Fix, clean up, and revamp selftests/bpf Makefile

[Andrii Nakryiko]

This patch set extensively revamps selftests/bpf's Makefile to generalize test
runner concept and apply it uniformly to test_maps and test_progs test
runners, along with test_progs' few build "flavors", exercising various ways
to build BPF programs.

As we do that, we fix dependencies between various phases of test runners, and
simplify some one-off rules and dependencies currently present in Makefile.
test_progs' flavors are now built into root $(OUTPUT) directory and can be run
without any extra steps right from there. E.g., test_progs-alu32 is built and
is supposed to be run from $(OUTPUT). It will cd into alu32/ subdirectory to
load correct set of BPF object files (which are different from the ones built
for test_progs).

Outline:
- patch #1 teaches test_progs about flavor sub-directories;
- patch #2 fixes one of CO-RE tests to not depend strictly on process name;
- patch #3 changes test_maps's usage of map_tests/tests.h to be the same as
  test_progs' one;
- patch #4 adds convenient short `make test_progs`-like targets to build only
  individual tests, if necessary;
- patch #5 is a main patch in the series; it uses a bunch of make magic
  (mainly $(call) and $(eval)) to define test runner "skeleton" and apply it
  to 4 different test runners, lots more details in corresponding commit
  description;
- patch #6 does a bit of post-clean up for test_queue_map and test_stack_map
  BPF programs;
- patch #7 cleans up test_libbpf.sh/test_libbpf_open superseded by test_progs.

v3->v4:
- remove accidentally checked in binaries;

v2->v3:
- drop test_xdp.o mixed compilation mode, remove test_libbpf.sh (Alexei);

v1->v2:
- drop test_progs-native causing compilation failures due to
  __builtin_preserve_field_access, add back test_xdp.o override, which will
  now emit rule re-definition warning.

Andrii Nakryiko (7):
  selftests/bpf: teach test_progs to cd into subdir
  selftests/bpf: make CO-RE reloc test impartial to test_progs flavor
  selftests/bpf: switch test_maps to test_progs' test.h format
  selftests/bpf: add simple per-test targets to Makefile
  selftests/bpf: replace test_progs and test_maps w/ general rule
  selftests/bpf: move test_queue_stack_map.h into progs/ where it
    belongs
  selftest/bpf: remove test_libbpf.sh and test_libbpf_open

 tools/testing/selftests/bpf/.gitignore        |   6 +-
 tools/testing/selftests/bpf/Makefile          | 335 ++++++++++--------
 .../selftests/bpf/prog_tests/core_reloc.c     |   4 +-
 .../selftests/bpf/progs/core_reloc_types.h    |   2 +-
 .../bpf/progs/test_core_reloc_kernel.c        |   3 +-
 .../bpf/{ => progs}/test_queue_stack_map.h    |   0
 tools/testing/selftests/bpf/test_libbpf.sh    |  43 ---
 .../testing/selftests/bpf/test_libbpf_open.c  | 144 --------
 tools/testing/selftests/bpf/test_maps.c       |   8 +-
 tools/testing/selftests/bpf/test_progs.c      |  33 +-
 10 files changed, 232 insertions(+), 346 deletions(-)
 rename tools/testing/selftests/bpf/{ => progs}/test_queue_stack_map.h (100%)
 delete mode 100755 tools/testing/selftests/bpf/test_libbpf.sh
 delete mode 100644 tools/testing/selftests/bpf/test_libbpf_open.c

-- 
2.17.1

[PATCH v4 bpf-next 1/7] selftests/bpf: teach test_progs to cd into subdir

[Andrii Nakryiko]

We are building a bunch of "flavors" of test_progs, e.g., w/ alu32 flag
for Clang when building BPF object. test_progs setup is relying on
having all the BPF object files and extra resources to be available in
current working directory, though. But we actually build all these files
into a separate sub-directory. Next set of patches establishes
convention of naming "flavored" test_progs (and test runner binaries in
general) as test_progs-flavor (e.g., test_progs-alu32), for each such
extra flavor. This patch teaches test_progs binary to automatically
detect its own extra flavor based on its argv[0], and if present, to
change current directory to a flavor-specific subdirectory.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/testing/selftests/bpf/test_progs.c | 33 +++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index af75a1c7a458..c7e52f4194e2 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -306,7 +306,7 @@ void *spin_lock_thread(void *arg)
 }
 
 /* extern declarations for test funcs */
-#define DEFINE_TEST(name) extern void test_##name();
+#define DEFINE_TEST(name) extern void test_##name(void);
 #include <prog_tests/tests.h>
 #undef DEFINE_TEST
 
@@ -518,6 +518,33 @@ static void stdio_restore(void)
 #endif
 }
 
+/*
+ * Determine if test_progs is running as a "flavored" test runner and switch
+ * into corresponding sub-directory to load correct BPF objects.
+ *
+ * This is done by looking at executable name. If it contains "-flavor"
+ * suffix, then we are running as a flavored test runner.
+ */
+int cd_flavor_subdir(const char *exec_name)
+{
+	/* General form of argv[0] passed here is:
+	 * some/path/to/test_progs[-flavor], where -flavor part is optional.
+	 * First cut out "test_progs[-flavor]" part, then extract "flavor"
+	 * part, if it's there.
+	 */
+	const char *flavor = strrchr(exec_name, '/');
+
+	if (!flavor)
+		return 0;
+	flavor++;
+	flavor = strrchr(flavor, '-');
+	if (!flavor)
+		return 0;
+	flavor++;
+	printf("Switching to flavor '%s' subdirectory...\n", flavor);
+	return chdir(flavor);
+}
+
 int main(int argc, char **argv)
 {
 	static const struct argp argp = {
@@ -531,6 +558,10 @@ int main(int argc, char **argv)
 	if (err)
 		return err;
 
+	err = cd_flavor_subdir(argv[0]);
+	if (err)
+		return err;
+
 	libbpf_set_print(libbpf_print_fn);
 
 	srand(time(NULL));
-- 
2.17.1

[PATCH v4 bpf-next 2/7] selftests/bpf: make CO-RE reloc test impartial to test_progs flavor

[Andrii Nakryiko]

test_core_reloc_kernel test captures its own process name and validates
it as part of the test. Given extra "flavors" of test_progs, this break
for anything by default test_progs binary. Fix the test to cut out
flavor part of the process name.

Fixes: ee2eb063d330 ("selftests/bpf: Add BPF_CORE_READ and BPF_CORE_READ_STR_INTO macro tests")
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/testing/selftests/bpf/prog_tests/core_reloc.c        | 4 ++--
 tools/testing/selftests/bpf/progs/core_reloc_types.h       | 2 +-
 tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c | 3 ++-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
index 7e2f5b4bf7f3..2b3586dc6c86 100644
--- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c
+++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
@@ -211,8 +211,8 @@ static struct core_reloc_test_case test_cases[] = {
 		.input_len = 0,
 		.output = STRUCT_TO_CHAR_PTR(core_reloc_kernel_output) {
 			.valid = { 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, },
-			.comm = "test_progs\0\0\0\0\0",
-			.comm_len = 11,
+			.comm = "test_progs",
+			.comm_len = sizeof("test_progs"),
 		},
 		.output_len = sizeof(struct core_reloc_kernel_output),
 	},
diff --git a/tools/testing/selftests/bpf/progs/core_reloc_types.h b/tools/testing/selftests/bpf/progs/core_reloc_types.h
index ad763ec0ba8f..f5939d9d5c61 100644
--- a/tools/testing/selftests/bpf/progs/core_reloc_types.h
+++ b/tools/testing/selftests/bpf/progs/core_reloc_types.h
@@ -6,7 +6,7 @@
 
 struct core_reloc_kernel_output {
 	int valid[10];
-	char comm[16];
+	char comm[sizeof("test_progs")];
 	int comm_len;
 };
 
diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c b/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
index 50f609618b65..a4b5e0562ed5 100644
--- a/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
@@ -15,7 +15,8 @@ static volatile struct data {
 
 struct core_reloc_kernel_output {
 	int valid[10];
-	char comm[16];
+	/* we have test_progs[-flavor], so cut flavor part */
+	char comm[sizeof("test_progs")];
 	int comm_len;
 };
 
-- 
2.17.1

[PATCH v4 bpf-next 3/7] selftests/bpf: switch test_maps to test_progs' test.h format

[Andrii Nakryiko]

Make test_maps use tests.h header format consistent with the one used by
test_progs, to facilitate unification.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/testing/selftests/bpf/Makefile    | 8 +-------
 tools/testing/selftests/bpf/test_maps.c | 8 ++++----
 2 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 00d05c5e2d57..5f97262e5fcb 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -249,14 +249,8 @@ $(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 */'; \
-		  echo '#ifdef DECLARE'; \
 		  ls *.c 2> /dev/null | \
-			sed -e 's@\([^\.]*\)\.c@extern void test_\1(void);@'; \
-		  echo '#endif'; \
-		  echo '#ifdef CALL'; \
-		  ls *.c 2> /dev/null | \
-			sed -e 's@\([^\.]*\)\.c@test_\1();@'; \
-		  echo '#endif' \
+			sed -e 's@\([^\.]*\)\.c@DEFINE_TEST(\1)@'; \
 		 ) > $(MAP_TESTS_H))
 
 VERIFIER_TESTS_DIR = $(OUTPUT)/verifier
diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index e1f1becda529..806b298397d3 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -1717,9 +1717,9 @@ static void run_all_tests(void)
 	test_map_in_map();
 }
 
-#define DECLARE
+#define DEFINE_TEST(name) extern void test_##name(void);
 #include <map_tests/tests.h>
-#undef DECLARE
+#undef DEFINE_TEST
 
 int main(void)
 {
@@ -1731,9 +1731,9 @@ int main(void)
 	map_flags = BPF_F_NO_PREALLOC;
 	run_all_tests();
 
-#define CALL
+#define DEFINE_TEST(name) test_##name();
 #include <map_tests/tests.h>
-#undef CALL
+#undef DEFINE_TEST
 
 	printf("test_maps: OK, %d SKIPPED\n", skips);
 	return 0;
-- 
2.17.1

[PATCH v4 bpf-next 4/7] selftests/bpf: add simple per-test targets to Makefile

[Andrii Nakryiko]

Currently it's impossible to do `make test_progs` and have only
test_progs be built, because all the binary targets are defined in terms
of $(OUTPUT)/<binary>, and $(OUTPUT) is absolute path to current
directory (or whatever gets overridden to by user).

This patch adds simple re-directing targets for all test targets making
it possible to do simple and nice `make test_progs` (and any other
target).

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/testing/selftests/bpf/Makefile | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 5f97262e5fcb..fbced23935cc 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -84,6 +84,16 @@ TEST_GEN_PROGS_EXTENDED = test_libbpf_open test_sock_addr test_skb_cgroup_id_use
 
 include ../lib.mk
 
+# Define simple and short `make test_progs`, `make test_sysctl`, etc targets
+# to build individual tests.
+# NOTE: Semicolon at the end is critical to override lib.mk's default static
+# rule for binaries.
+$(notdir $(TEST_GEN_PROGS)						\
+	 $(TEST_PROGS)							\
+	 $(TEST_PROGS_EXTENDED)						\
+	 $(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)
-- 
2.17.1

[PATCH v4 bpf-next 5/7] selftests/bpf: replace test_progs and test_maps w/ general rule

[Andrii Nakryiko]

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

[PATCH v4 bpf-next 6/7] selftests/bpf: move test_queue_stack_map.h into progs/ where it belongs

[Andrii Nakryiko]

test_queue_stack_map.h is used only from BPF programs. Thus it should be
part of progs/ subdir. An added benefit of moving it there is that new
TEST_RUNNER_DEFINE_RULES macro-rule will properly capture dependency on
this header for all BPF objects and trigger re-build, if it changes.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/testing/selftests/bpf/Makefile                           | 3 ---
 tools/testing/selftests/bpf/{ => progs}/test_queue_stack_map.h | 0
 2 files changed, 3 deletions(-)
 rename tools/testing/selftests/bpf/{ => progs}/test_queue_stack_map.h (100%)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index c9f43d49eac9..16056e5d399d 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -153,9 +153,6 @@ CLANG_CFLAGS = $(CLANG_SYS_INCLUDES) \
 $(OUTPUT)/test_l4lb_noinline.o: BPF_CFLAGS += -fno-inline
 $(OUTPUT)/test_xdp_noinline.o: BPF_CFLAGS += -fno-inline
 
-$(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
 
 # Build BPF object using Clang
diff --git a/tools/testing/selftests/bpf/test_queue_stack_map.h b/tools/testing/selftests/bpf/progs/test_queue_stack_map.h
similarity index 100%
rename from tools/testing/selftests/bpf/test_queue_stack_map.h
rename to tools/testing/selftests/bpf/progs/test_queue_stack_map.h
-- 
2.17.1

[PATCH v4 bpf-next 7/7] selftest/bpf: remove test_libbpf.sh and test_libbpf_open

[Andrii Nakryiko]

test_progs is much more sophisticated superset of tests compared to
test_libbpf.sh and test_libbpf_open. Remove test_libbpf.sh and
test_libbpf_open.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/testing/selftests/bpf/.gitignore        |   1 -
 tools/testing/selftests/bpf/Makefile          |   3 +-
 tools/testing/selftests/bpf/test_libbpf.sh    |  43 ------
 .../testing/selftests/bpf/test_libbpf_open.c  | 144 ------------------
 4 files changed, 1 insertion(+), 190 deletions(-)
 delete mode 100755 tools/testing/selftests/bpf/test_libbpf.sh
 delete mode 100644 tools/testing/selftests/bpf/test_libbpf_open.c

diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index c51f356f84b5..6f46170e09c1 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -11,7 +11,6 @@ test_dev_cgroup
 test_tcpbpf_user
 test_verifier_log
 feature
-test_libbpf_open
 test_sock
 test_sock_addr
 test_sock_fields
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 16056e5d399d..4ff5f4aada08 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -52,7 +52,6 @@ TEST_FILES =
 
 # Order correspond to 'make run_tests' order
 TEST_PROGS := test_kmod.sh \
-	test_libbpf.sh \
 	test_xdp_redirect.sh \
 	test_xdp_meta.sh \
 	test_xdp_veth.sh \
@@ -79,7 +78,7 @@ TEST_PROGS_EXTENDED := with_addr.sh \
 	test_xdp_vlan.sh
 
 # Compile but not part of 'make run_tests'
-TEST_GEN_PROGS_EXTENDED = test_libbpf_open test_sock_addr test_skb_cgroup_id_user \
+TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \
 	flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \
 	test_lirc_mode2_user
 
diff --git a/tools/testing/selftests/bpf/test_libbpf.sh b/tools/testing/selftests/bpf/test_libbpf.sh
deleted file mode 100755
index 2989b2e2d856..000000000000
--- a/tools/testing/selftests/bpf/test_libbpf.sh
+++ /dev/null
@@ -1,43 +0,0 @@
-#!/bin/sh
-# SPDX-License-Identifier: GPL-2.0
-
-export TESTNAME=test_libbpf
-
-# Determine selftest success via shell exit code
-exit_handler()
-{
-	if [ $? -eq 0 ]; then
-		echo "selftests: $TESTNAME [PASS]";
-	else
-		echo "$TESTNAME: failed at file $LAST_LOADED" 1>&2
-		echo "selftests: $TESTNAME [FAILED]";
-	fi
-}
-
-libbpf_open_file()
-{
-	LAST_LOADED=$1
-	if [ -n "$VERBOSE" ]; then
-	    ./test_libbpf_open $1
-	else
-	    ./test_libbpf_open --quiet $1
-	fi
-}
-
-# Exit script immediately (well catched by trap handler) if any
-# program/thing exits with a non-zero status.
-set -e
-
-# (Use 'trap -l' to list meaning of numbers)
-trap exit_handler 0 2 3 6 9
-
-libbpf_open_file test_l4lb.o
-
-# Load a program with BPF-to-BPF calls
-libbpf_open_file test_l4lb_noinline.o
-
-# Load a program compiled without the "-target bpf" flag
-libbpf_open_file test_xdp.o
-
-# Success
-exit 0
diff --git a/tools/testing/selftests/bpf/test_libbpf_open.c b/tools/testing/selftests/bpf/test_libbpf_open.c
deleted file mode 100644
index 9e9db202d218..000000000000
--- a/tools/testing/selftests/bpf/test_libbpf_open.c
+++ /dev/null
@@ -1,144 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0
- * Copyright (c) 2018 Jesper Dangaard Brouer, Red Hat Inc.
- */
-static const char *__doc__ =
-	"Libbpf test program for loading BPF ELF object files";
-
-#include <stdlib.h>
-#include <stdio.h>
-#include <string.h>
-#include <stdarg.h>
-#include <bpf/libbpf.h>
-#include <getopt.h>
-
-#include "bpf_rlimit.h"
-
-static const struct option long_options[] = {
-	{"help",	no_argument,		NULL, 'h' },
-	{"debug",	no_argument,		NULL, 'D' },
-	{"quiet",	no_argument,		NULL, 'q' },
-	{0, 0, NULL,  0 }
-};
-
-static void usage(char *argv[])
-{
-	int i;
-
-	printf("\nDOCUMENTATION:\n%s\n\n", __doc__);
-	printf(" Usage: %s (options-see-below) BPF_FILE\n", argv[0]);
-	printf(" Listing options:\n");
-	for (i = 0; long_options[i].name != 0; i++) {
-		printf(" --%-12s", long_options[i].name);
-		printf(" short-option: -%c",
-		       long_options[i].val);
-		printf("\n");
-	}
-	printf("\n");
-}
-
-static bool debug = 0;
-static int libbpf_debug_print(enum libbpf_print_level level,
-			      const char *fmt, va_list args)
-{
-	if (level == LIBBPF_DEBUG && !debug)
-		return 0;
-
-	fprintf(stderr, "[%d] ", level);
-	return vfprintf(stderr, fmt, args);
-}
-
-#define EXIT_FAIL_LIBBPF EXIT_FAILURE
-#define EXIT_FAIL_OPTION 2
-
-int test_walk_progs(struct bpf_object *obj, bool verbose)
-{
-	struct bpf_program *prog;
-	int cnt = 0;
-
-	bpf_object__for_each_program(prog, obj) {
-		cnt++;
-		if (verbose)
-			printf("Prog (count:%d) section_name: %s\n", cnt,
-			       bpf_program__title(prog, false));
-	}
-	return 0;
-}
-
-int test_walk_maps(struct bpf_object *obj, bool verbose)
-{
-	struct bpf_map *map;
-	int cnt = 0;
-
-	bpf_object__for_each_map(map, obj) {
-		cnt++;
-		if (verbose)
-			printf("Map (count:%d) name: %s\n", cnt,
-			       bpf_map__name(map));
-	}
-	return 0;
-}
-
-int test_open_file(char *filename, bool verbose)
-{
-	struct bpf_object *bpfobj = NULL;
-	long err;
-
-	if (verbose)
-		printf("Open BPF ELF-file with libbpf: %s\n", filename);
-
-	/* Load BPF ELF object file and check for errors */
-	bpfobj = bpf_object__open(filename);
-	err = libbpf_get_error(bpfobj);
-	if (err) {
-		char err_buf[128];
-		libbpf_strerror(err, err_buf, sizeof(err_buf));
-		if (verbose)
-			printf("Unable to load eBPF objects in file '%s': %s\n",
-			       filename, err_buf);
-		return EXIT_FAIL_LIBBPF;
-	}
-	test_walk_progs(bpfobj, verbose);
-	test_walk_maps(bpfobj, verbose);
-
-	if (verbose)
-		printf("Close BPF ELF-file with libbpf: %s\n",
-		       bpf_object__name(bpfobj));
-	bpf_object__close(bpfobj);
-
-	return 0;
-}
-
-int main(int argc, char **argv)
-{
-	char filename[1024] = { 0 };
-	bool verbose = 1;
-	int longindex = 0;
-	int opt;
-
-	libbpf_set_print(libbpf_debug_print);
-
-	/* Parse commands line args */
-	while ((opt = getopt_long(argc, argv, "hDq",
-				  long_options, &longindex)) != -1) {
-		switch (opt) {
-		case 'D':
-			debug = 1;
-			break;
-		case 'q': /* Use in scripting mode */
-			verbose = 0;
-			break;
-		case 'h':
-		default:
-			usage(argv);
-			return EXIT_FAIL_OPTION;
-		}
-	}
-	if (optind >= argc) {
-		usage(argv);
-		printf("ERROR: Expected BPF_FILE argument after options\n");
-		return EXIT_FAIL_OPTION;
-	}
-	snprintf(filename, sizeof(filename), "%s", argv[optind]);
-
-	return test_open_file(filename, verbose);
-}
-- 
2.17.1

Re: [PATCH v4 bpf-next 5/7] selftests/bpf: replace test_progs and test_maps w/ general rule

[Stanislav Fomichev]

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
> 

Re: [PATCH v4 bpf-next 5/7] selftests/bpf: replace test_progs and test_maps w/ general rule

[Andrii Nakryiko]

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.

[...]

Re: [PATCH v4 bpf-next 0/7] Fix, clean up, and revamp selftests/bpf Makefile

[Alexei Starovoitov]

On Wed, Oct 16, 2019 at 4:49 AM Andrii Nakryiko <andriin@fb.com> wrote:
>
> This patch set extensively revamps selftests/bpf's Makefile to generalize test
> runner concept and apply it uniformly to test_maps and test_progs test
> runners, along with test_progs' few build "flavors", exercising various ways
> to build BPF programs.
>
> As we do that, we fix dependencies between various phases of test runners, and
> simplify some one-off rules and dependencies currently present in Makefile.
> test_progs' flavors are now built into root $(OUTPUT) directory and can be run
> without any extra steps right from there. E.g., test_progs-alu32 is built and
> is supposed to be run from $(OUTPUT). It will cd into alu32/ subdirectory to
> load correct set of BPF object files (which are different from the ones built
> for test_progs).
>
> Outline:
> - patch #1 teaches test_progs about flavor sub-directories;
> - patch #2 fixes one of CO-RE tests to not depend strictly on process name;
> - patch #3 changes test_maps's usage of map_tests/tests.h to be the same as
>   test_progs' one;
> - patch #4 adds convenient short `make test_progs`-like targets to build only
>   individual tests, if necessary;
> - patch #5 is a main patch in the series; it uses a bunch of make magic
>   (mainly $(call) and $(eval)) to define test runner "skeleton" and apply it
>   to 4 different test runners, lots more details in corresponding commit
>   description;
> - patch #6 does a bit of post-clean up for test_queue_map and test_stack_map
>   BPF programs;
> - patch #7 cleans up test_libbpf.sh/test_libbpf_open superseded by test_progs.
>
> v3->v4:
> - remove accidentally checked in binaries;

something really odd here.
Before the patchset ./test_progs -n 27 passes
after the patch it simply hangs.
Though strace -f ./test_progs -n 27 passes.
Any idea?

Re: [PATCH v4 bpf-next 0/7] Fix, clean up, and revamp selftests/bpf Makefile

[Andrii Nakryiko]

On Wed, Oct 16, 2019 at 9:28 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Oct 16, 2019 at 4:49 AM Andrii Nakryiko <andriin@fb.com> wrote:
> >
> > This patch set extensively revamps selftests/bpf's Makefile to generalize test
> > runner concept and apply it uniformly to test_maps and test_progs test
> > runners, along with test_progs' few build "flavors", exercising various ways
> > to build BPF programs.
> >
> > As we do that, we fix dependencies between various phases of test runners, and
> > simplify some one-off rules and dependencies currently present in Makefile.
> > test_progs' flavors are now built into root $(OUTPUT) directory and can be run
> > without any extra steps right from there. E.g., test_progs-alu32 is built and
> > is supposed to be run from $(OUTPUT). It will cd into alu32/ subdirectory to
> > load correct set of BPF object files (which are different from the ones built
> > for test_progs).
> >
> > Outline:
> > - patch #1 teaches test_progs about flavor sub-directories;
> > - patch #2 fixes one of CO-RE tests to not depend strictly on process name;
> > - patch #3 changes test_maps's usage of map_tests/tests.h to be the same as
> >   test_progs' one;
> > - patch #4 adds convenient short `make test_progs`-like targets to build only
> >   individual tests, if necessary;
> > - patch #5 is a main patch in the series; it uses a bunch of make magic
> >   (mainly $(call) and $(eval)) to define test runner "skeleton" and apply it
> >   to 4 different test runners, lots more details in corresponding commit
> >   description;
> > - patch #6 does a bit of post-clean up for test_queue_map and test_stack_map
> >   BPF programs;
> > - patch #7 cleans up test_libbpf.sh/test_libbpf_open superseded by test_progs.
> >
> > v3->v4:
> > - remove accidentally checked in binaries;
>
> something really odd here.
> Before the patchset ./test_progs -n 27 passes
> after the patch it simply hangs.
> Though strace -f ./test_progs -n 27 passes.
> Any idea?

Interesting. For me test_progs -n27 passes by itself, whether with or
without Makefile changes. But when run together with #8
flow_dissector_reattach, it fails with
"(prog_tests/sockopt_inherit.c:28: errno: Network is unreachable) Fail
to connect to server", even without Makefile changes. It doesn't hang,
but the test has server and client threads being coordinated, so I
wouldn't be surprised that under some specific timing and error
conditions it can hang.

I bisected this failure to f97eea1756f3 ("selftests/bpf: Check that
flow dissector can be re-attached"), that's when
flow_dissector_reattach test was added. So apparently there is some
bad interaction there.

So I suspect my Makefile changes have nothing to do with this, it
would be really bizarre...

Jakub, do you mind checking as well?

Re: [PATCH v4 bpf-next 0/7] Fix, clean up, and revamp selftests/bpf Makefile

[Jakub Sitnicki]

On Thu, Oct 17, 2019 at 08:52 AM CEST, Andrii Nakryiko wrote:
> On Wed, Oct 16, 2019 at 9:28 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
>>
>> On Wed, Oct 16, 2019 at 4:49 AM Andrii Nakryiko <andriin@fb.com> wrote:
>> >
>> > This patch set extensively revamps selftests/bpf's Makefile to generalize test
>> > runner concept and apply it uniformly to test_maps and test_progs test
>> > runners, along with test_progs' few build "flavors", exercising various ways
>> > to build BPF programs.
>> >
>> > As we do that, we fix dependencies between various phases of test runners, and
>> > simplify some one-off rules and dependencies currently present in Makefile.
>> > test_progs' flavors are now built into root $(OUTPUT) directory and can be run
>> > without any extra steps right from there. E.g., test_progs-alu32 is built and
>> > is supposed to be run from $(OUTPUT). It will cd into alu32/ subdirectory to
>> > load correct set of BPF object files (which are different from the ones built
>> > for test_progs).
>> >
>> > Outline:
>> > - patch #1 teaches test_progs about flavor sub-directories;
>> > - patch #2 fixes one of CO-RE tests to not depend strictly on process name;
>> > - patch #3 changes test_maps's usage of map_tests/tests.h to be the same as
>> >   test_progs' one;
>> > - patch #4 adds convenient short `make test_progs`-like targets to build only
>> >   individual tests, if necessary;
>> > - patch #5 is a main patch in the series; it uses a bunch of make magic
>> >   (mainly $(call) and $(eval)) to define test runner "skeleton" and apply it
>> >   to 4 different test runners, lots more details in corresponding commit
>> >   description;
>> > - patch #6 does a bit of post-clean up for test_queue_map and test_stack_map
>> >   BPF programs;
>> > - patch #7 cleans up test_libbpf.sh/test_libbpf_open superseded by test_progs.
>> >
>> > v3->v4:
>> > - remove accidentally checked in binaries;
>>
>> something really odd here.
>> Before the patchset ./test_progs -n 27 passes
>> after the patch it simply hangs.
>> Though strace -f ./test_progs -n 27 passes.
>> Any idea?
>
> Interesting. For me test_progs -n27 passes by itself, whether with or
> without Makefile changes. But when run together with #8
> flow_dissector_reattach, it fails with
> "(prog_tests/sockopt_inherit.c:28: errno: Network is unreachable) Fail
> to connect to server", even without Makefile changes. It doesn't hang,
> but the test has server and client threads being coordinated, so I
> wouldn't be surprised that under some specific timing and error
> conditions it can hang.
>
> I bisected this failure to f97eea1756f3 ("selftests/bpf: Check that
> flow dissector can be re-attached"), that's when
> flow_dissector_reattach test was added. So apparently there is some
> bad interaction there.
>
> So I suspect my Makefile changes have nothing to do with this, it
> would be really bizarre...
>
> Jakub, do you mind checking as well?

This is my fault. flow_dissector_reattach test is not returning to the
netns we started in. Sorry about the oversight. Let me post a fix.

-Jakub

Re: [PATCH v4 bpf-next 5/7] selftests/bpf: replace test_progs and test_maps w/ general rule

[Stanislav Fomichev]

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.

Re: [PATCH v4 bpf-next 5/7] selftests/bpf: replace test_progs and test_maps w/ general rule

[Andrii Nakryiko]

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

Re: [PATCH v4 bpf-next 5/7] selftests/bpf: replace test_progs and test_maps w/ general rule

[Andrii Nakryiko]

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


[...]

Re: [PATCH v4 bpf-next 5/7] selftests/bpf: replace test_progs and test_maps w/ general rule

[Alexei Starovoitov]

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?" .. :)

Re: [PATCH v4 bpf-next 5/7] selftests/bpf: replace test_progs and test_maps w/ general rule

[Andrii Nakryiko]

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.

Re: [PATCH v4 bpf-next 0/7] Fix, clean up, and revamp selftests/bpf Makefile

[Alexei Starovoitov]

On Thu, Oct 17, 2019 at 1:09 AM Jakub Sitnicki <jakub@cloudflare.com> wrote:
>
> On Thu, Oct 17, 2019 at 08:52 AM CEST, Andrii Nakryiko wrote:
> > On Wed, Oct 16, 2019 at 9:28 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> >>
> >> On Wed, Oct 16, 2019 at 4:49 AM Andrii Nakryiko <andriin@fb.com> wrote:
> >> >
> >> > This patch set extensively revamps selftests/bpf's Makefile to generalize test
> >> > runner concept and apply it uniformly to test_maps and test_progs test
> >> > runners, along with test_progs' few build "flavors", exercising various ways
> >> > to build BPF programs.
> >> >
> >> > As we do that, we fix dependencies between various phases of test runners, and
> >> > simplify some one-off rules and dependencies currently present in Makefile.
> >> > test_progs' flavors are now built into root $(OUTPUT) directory and can be run
> >> > without any extra steps right from there. E.g., test_progs-alu32 is built and
> >> > is supposed to be run from $(OUTPUT). It will cd into alu32/ subdirectory to
> >> > load correct set of BPF object files (which are different from the ones built
> >> > for test_progs).
> >> >
> >> > Outline:
> >> > - patch #1 teaches test_progs about flavor sub-directories;
> >> > - patch #2 fixes one of CO-RE tests to not depend strictly on process name;
> >> > - patch #3 changes test_maps's usage of map_tests/tests.h to be the same as
> >> >   test_progs' one;
> >> > - patch #4 adds convenient short `make test_progs`-like targets to build only
> >> >   individual tests, if necessary;
> >> > - patch #5 is a main patch in the series; it uses a bunch of make magic
> >> >   (mainly $(call) and $(eval)) to define test runner "skeleton" and apply it
> >> >   to 4 different test runners, lots more details in corresponding commit
> >> >   description;
> >> > - patch #6 does a bit of post-clean up for test_queue_map and test_stack_map
> >> >   BPF programs;
> >> > - patch #7 cleans up test_libbpf.sh/test_libbpf_open superseded by test_progs.
> >> >
> >> > v3->v4:
> >> > - remove accidentally checked in binaries;
> >>
> >> something really odd here.
> >> Before the patchset ./test_progs -n 27 passes
> >> after the patch it simply hangs.
> >> Though strace -f ./test_progs -n 27 passes.
> >> Any idea?
> >
> > Interesting. For me test_progs -n27 passes by itself, whether with or
> > without Makefile changes. But when run together with #8
> > flow_dissector_reattach, it fails with
> > "(prog_tests/sockopt_inherit.c:28: errno: Network is unreachable) Fail
> > to connect to server", even without Makefile changes. It doesn't hang,
> > but the test has server and client threads being coordinated, so I
> > wouldn't be surprised that under some specific timing and error
> > conditions it can hang.
> >
> > I bisected this failure to f97eea1756f3 ("selftests/bpf: Check that
> > flow dissector can be re-attached"), that's when
> > flow_dissector_reattach test was added. So apparently there is some
> > bad interaction there.
> >
> > So I suspect my Makefile changes have nothing to do with this, it
> > would be really bizarre...
> >
> > Jakub, do you mind checking as well?
>
> This is my fault. flow_dissector_reattach test is not returning to the
> netns we started in. Sorry about the oversight. Let me post a fix.

Thanks for the fix.
Now it all works nicely and selftests/bpf build time improvement
is dramatic.

Applied the set to bpf-next.
Thanks

Re: [PATCH v4 bpf-next 5/7] selftests/bpf: replace test_progs and test_maps w/ general rule

[Stanislav Fomichev]

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

Re: [PATCH v4 bpf-next 5/7] selftests/bpf: replace test_progs and test_maps w/ general rule

[Yauheni Kaliuta]

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


-- 
WBR,
Yauheni Kaliuta

Re: [PATCH v4 bpf-next 5/7] selftests/bpf: replace test_progs and test_maps w/ general rule

[Andrii Nakryiko]

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?

[...]

Re: [PATCH v4 bpf-next 5/7] selftests/bpf: replace test_progs and test_maps w/ general rule

[Yauheni Kaliuta]

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.


-- 
WBR,
Yauheni Kaliuta