bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/6] Fix, clean up, and revamp selftests/bpf Makefile
@ 2019-10-15 22:03 Andrii Nakryiko
  2019-10-15 22:03 ` [PATCH bpf-next 1/6] selftest/bpf: teach test_progs to cd into subdir Andrii Nakryiko
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Andrii Nakryiko @ 2019-10-15 22:03 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, 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 5 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.

Andrii Nakryiko (6):
  selftest/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

 tools/testing/selftests/bpf/.gitignore        |   6 +-
 tools/testing/selftests/bpf/Makefile          | 344 ++++++++++--------
 .../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_maps.c       |   8 +-
 tools/testing/selftests/bpf/test_progs.c      |  33 +-
 8 files changed, 242 insertions(+), 158 deletions(-)
 rename tools/testing/selftests/bpf/{ => progs}/test_queue_stack_map.h (100%)

-- 
2.17.1


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

* [PATCH bpf-next 1/6] selftest/bpf: teach test_progs to cd into subdir
  2019-10-15 22:03 [PATCH bpf-next 0/6] Fix, clean up, and revamp selftests/bpf Makefile Andrii Nakryiko
@ 2019-10-15 22:03 ` Andrii Nakryiko
  2019-10-15 22:03 ` [PATCH bpf-next 2/6] selftests/bpf: make CO-RE reloc test impartial to test_progs flavor Andrii Nakryiko
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Andrii Nakryiko @ 2019-10-15 22:03 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, 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


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

* [PATCH bpf-next 2/6] selftests/bpf: make CO-RE reloc test impartial to test_progs flavor
  2019-10-15 22:03 [PATCH bpf-next 0/6] Fix, clean up, and revamp selftests/bpf Makefile Andrii Nakryiko
  2019-10-15 22:03 ` [PATCH bpf-next 1/6] selftest/bpf: teach test_progs to cd into subdir Andrii Nakryiko
@ 2019-10-15 22:03 ` Andrii Nakryiko
  2019-10-15 22:03 ` [PATCH bpf-next 3/6] selftests/bpf: switch test_maps to test_progs' test.h format Andrii Nakryiko
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Andrii Nakryiko @ 2019-10-15 22:03 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, 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 21a0dff66241..17980189d9e5 100644
--- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c
+++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
@@ -195,8 +195,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 9a6bdeb4894c..f4f16c30c60c 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


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

* [PATCH bpf-next 3/6] selftests/bpf: switch test_maps to test_progs' test.h format
  2019-10-15 22:03 [PATCH bpf-next 0/6] Fix, clean up, and revamp selftests/bpf Makefile Andrii Nakryiko
  2019-10-15 22:03 ` [PATCH bpf-next 1/6] selftest/bpf: teach test_progs to cd into subdir Andrii Nakryiko
  2019-10-15 22:03 ` [PATCH bpf-next 2/6] selftests/bpf: make CO-RE reloc test impartial to test_progs flavor Andrii Nakryiko
@ 2019-10-15 22:03 ` Andrii Nakryiko
  2019-10-15 22:03 ` [PATCH bpf-next 4/6] selftests/bpf: add simple per-test targets to Makefile Andrii Nakryiko
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Andrii Nakryiko @ 2019-10-15 22:03 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, 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


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

* [PATCH bpf-next 4/6] selftests/bpf: add simple per-test targets to Makefile
  2019-10-15 22:03 [PATCH bpf-next 0/6] Fix, clean up, and revamp selftests/bpf Makefile Andrii Nakryiko
                   ` (2 preceding siblings ...)
  2019-10-15 22:03 ` [PATCH bpf-next 3/6] selftests/bpf: switch test_maps to test_progs' test.h format Andrii Nakryiko
@ 2019-10-15 22:03 ` Andrii Nakryiko
  2019-10-15 22:03 ` [PATCH bpf-next 5/6] selftests/bpf: replace test_progs and test_maps w/ general rule Andrii Nakryiko
  2019-10-15 22:03 ` [PATCH bpf-next 6/6] selftests/bpf: move test_queue_stack_map.h into progs/ where it belongs Andrii Nakryiko
  5 siblings, 0 replies; 10+ messages in thread
From: Andrii Nakryiko @ 2019-10-15 22:03 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, 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


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

* [PATCH bpf-next 5/6] selftests/bpf: replace test_progs and test_maps w/ general rule
  2019-10-15 22:03 [PATCH bpf-next 0/6] Fix, clean up, and revamp selftests/bpf Makefile Andrii Nakryiko
                   ` (3 preceding siblings ...)
  2019-10-15 22:03 ` [PATCH bpf-next 4/6] selftests/bpf: add simple per-test targets to Makefile Andrii Nakryiko
@ 2019-10-15 22:03 ` Andrii Nakryiko
  2019-10-15 23:41   ` Alexei Starovoitov
  2019-10-15 22:03 ` [PATCH bpf-next 6/6] selftests/bpf: move test_queue_stack_map.h into progs/ where it belongs Andrii Nakryiko
  5 siblings, 1 reply; 10+ messages in thread
From: Andrii Nakryiko @ 2019-10-15 22:03 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, 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
3 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;
- native, which uses a mix of native Clang target and BPF target for LLC.

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 |   6 +-
 tools/testing/selftests/bpf/Makefile   | 325 ++++++++++++++-----------
 2 files changed, 191 insertions(+), 140 deletions(-)

diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index 7470327edcfe..c2ce8515a48e 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,11 @@ 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
+/native
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index fbced23935cc..1c321caf2d64 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,25 +18,21 @@ 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
 
 # Order correspond to 'make run_tests' order
-TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \
+TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map \
+	 test_progs test_progs-native \
 	test_align test_verifier_log test_dev_cgroup test_tcpbpf_user \
 	test_sock test_btf test_sockmap get_cgroup_id_user test_socket_cookie \
 	test_cgroup_storage test_select_reuseport test_section_names \
 	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 +40,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 +84,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 +98,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 +113,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 +128,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 +142,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 +158,177 @@ $(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-native test runner with BPF build rule using native Clang
+# target with bpf LLC target
+TRUNNER_BPF_BUILD_RULE := CLANG_NATIVE_BPF_BUILD_RULE
+TRUNNER_BPF_LDFLAGS :=
+$(eval $(call DEFINE_TEST_RUNNER,test_progs,native))
+
+# 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))
-
-EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(ALU32_BUILD_DIR) $(BPF_GCC_BUILD_DIR) \
-	$(VERIFIER_TESTS_H) $(PROG_TESTS_H) $(MAP_TESTS_H) \
-	feature
+		) > 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)					\
+	prog_tests/tests.h map_tests/tests.h verifier/tests.h		\
+	feature $(OUTPUT)/*.o $(OUTPUT)/alu32 $(OUTPUT)/bpf_gcc		\
+	$(OUTPUT)/native
-- 
2.17.1


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

* [PATCH bpf-next 6/6] selftests/bpf: move test_queue_stack_map.h into progs/ where it belongs
  2019-10-15 22:03 [PATCH bpf-next 0/6] Fix, clean up, and revamp selftests/bpf Makefile Andrii Nakryiko
                   ` (4 preceding siblings ...)
  2019-10-15 22:03 ` [PATCH bpf-next 5/6] selftests/bpf: replace test_progs and test_maps w/ general rule Andrii Nakryiko
@ 2019-10-15 22:03 ` Andrii Nakryiko
  5 siblings, 0 replies; 10+ messages in thread
From: Andrii Nakryiko @ 2019-10-15 22:03 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, 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 1c321caf2d64..fa6a40d679d9 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -154,9 +154,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


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

* Re: [PATCH bpf-next 5/6] selftests/bpf: replace test_progs and test_maps w/ general rule
  2019-10-15 22:03 ` [PATCH bpf-next 5/6] selftests/bpf: replace test_progs and test_maps w/ general rule Andrii Nakryiko
@ 2019-10-15 23:41   ` Alexei Starovoitov
  2019-10-15 23:50     ` Andrii Nakryiko
  0 siblings, 1 reply; 10+ messages in thread
From: Alexei Starovoitov @ 2019-10-15 23:41 UTC (permalink / raw)
  To: Andrii Nakryiko, bpf, netdev, daniel; +Cc: andrii.nakryiko, Kernel Team

On 10/15/19 3:03 PM, 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
> 3 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;
> - native, which uses a mix of native Clang target and BPF target for LLC.

Great improvement, but it's taking it too far.
(clang  -I. -I/data/users/ast/net-next/tools/testing/selftests/bpf -g 
-D__TARGET_ARCH_x86 -I. -I./include/uapi 
-I/data/users/ast/net-next/tools/include/uapi 
-I/data/users/ast/net-next/tools/lib/bpf 
-I/data/users/ast/net-next/tools/testing/selftests/usr/include 
-idirafter /usr/local/include -idirafter 
/data/users/ast/llvm/bld/lib/clang/10.0.0/include -idirafter 
/usr/include -Wno-compare-distinct-pointer-types -O2 -emit-llvm -c 
progs/test_core_reloc_existence.c -o - || echo "BPF obj compilation 
failed") | llc -march=bpf -mcpu=probe   -filetype=obj -o 
/data/users/ast/net-next/tools/testing/selftests/bpf/native/test_core_reloc_existence.o
progs/test_core_reloc_existence.c:47:18: error: use of unknown builtin 
'__builtin_preserve_field_info' [-Wimplicit-function-declaration]
         out->a_exists = bpf_core_field_exists(in->a);

native clang + llc is useful for old school tracing only (before CO-RE).

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

* Re: [PATCH bpf-next 5/6] selftests/bpf: replace test_progs and test_maps w/ general rule
  2019-10-15 23:41   ` Alexei Starovoitov
@ 2019-10-15 23:50     ` Andrii Nakryiko
  2019-10-16  3:34       ` Andrii Nakryiko
  0 siblings, 1 reply; 10+ messages in thread
From: Andrii Nakryiko @ 2019-10-15 23:50 UTC (permalink / raw)
  To: Alexei Starovoitov; +Cc: Andrii Nakryiko, bpf, netdev, daniel, Kernel Team

On Tue, Oct 15, 2019 at 4:41 PM Alexei Starovoitov <ast@fb.com> wrote:
>
> On 10/15/19 3:03 PM, 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
> > 3 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;
> > - native, which uses a mix of native Clang target and BPF target for LLC.
>
> Great improvement, but it's taking it too far.
> (clang  -I. -I/data/users/ast/net-next/tools/testing/selftests/bpf -g
> -D__TARGET_ARCH_x86 -I. -I./include/uapi
> -I/data/users/ast/net-next/tools/include/uapi
> -I/data/users/ast/net-next/tools/lib/bpf
> -I/data/users/ast/net-next/tools/testing/selftests/usr/include
> -idirafter /usr/local/include -idirafter
> /data/users/ast/llvm/bld/lib/clang/10.0.0/include -idirafter
> /usr/include -Wno-compare-distinct-pointer-types -O2 -emit-llvm -c
> progs/test_core_reloc_existence.c -o - || echo "BPF obj compilation
> failed") | llc -march=bpf -mcpu=probe   -filetype=obj -o
> /data/users/ast/net-next/tools/testing/selftests/bpf/native/test_core_reloc_existence.o
> progs/test_core_reloc_existence.c:47:18: error: use of unknown builtin
> '__builtin_preserve_field_info' [-Wimplicit-function-declaration]
>          out->a_exists = bpf_core_field_exists(in->a);

Do you use latest clang that supports __builtin_preserve_field_info()?
All the flavors are building just fine for me with latest clang.

>
> native clang + llc is useful for old school tracing only (before CO-RE).

Don't disagree (I actually have little context why we needed this
special case at all), but I had no errors or warnings whatsoever. I
think in this particular case it's not specific to test_progs-native
build, can you please double-check on your side?

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

* Re: [PATCH bpf-next 5/6] selftests/bpf: replace test_progs and test_maps w/ general rule
  2019-10-15 23:50     ` Andrii Nakryiko
@ 2019-10-16  3:34       ` Andrii Nakryiko
  0 siblings, 0 replies; 10+ messages in thread
From: Andrii Nakryiko @ 2019-10-16  3:34 UTC (permalink / raw)
  To: Alexei Starovoitov; +Cc: Andrii Nakryiko, bpf, netdev, daniel, Kernel Team

On Tue, Oct 15, 2019 at 4:50 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Tue, Oct 15, 2019 at 4:41 PM Alexei Starovoitov <ast@fb.com> wrote:
> >
> > On 10/15/19 3:03 PM, 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
> > > 3 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;
> > > - native, which uses a mix of native Clang target and BPF target for LLC.
> >
> > Great improvement, but it's taking it too far.
> > (clang  -I. -I/data/users/ast/net-next/tools/testing/selftests/bpf -g
> > -D__TARGET_ARCH_x86 -I. -I./include/uapi
> > -I/data/users/ast/net-next/tools/include/uapi
> > -I/data/users/ast/net-next/tools/lib/bpf
> > -I/data/users/ast/net-next/tools/testing/selftests/usr/include
> > -idirafter /usr/local/include -idirafter
> > /data/users/ast/llvm/bld/lib/clang/10.0.0/include -idirafter
> > /usr/include -Wno-compare-distinct-pointer-types -O2 -emit-llvm -c
> > progs/test_core_reloc_existence.c -o - || echo "BPF obj compilation
> > failed") | llc -march=bpf -mcpu=probe   -filetype=obj -o
> > /data/users/ast/net-next/tools/testing/selftests/bpf/native/test_core_reloc_existence.o
> > progs/test_core_reloc_existence.c:47:18: error: use of unknown builtin
> > '__builtin_preserve_field_info' [-Wimplicit-function-declaration]
> >          out->a_exists = bpf_core_field_exists(in->a);
>
> Do you use latest clang that supports __builtin_preserve_field_info()?
> All the flavors are building just fine for me with latest clang.

Ok, so I didn't have __builtin_preserve_field_info() tests together
with these Makefile changes and thus test_progs-native was compiling
just fine for me. __builtin_preserve_access_index() is not
BPF-target-only built-in, so it was compiling fine for non-BPF target.

I've dropped test_progs-native and added back test_xdp.o override, but
now **after** we define generic rule for test_progs, which will cause
it to override previous test_xdp.o recipe. This causes make to emit
warning about rule re-definition, which I'm not excited about, but
avoiding it would require some further checks and filterings in
DEFINE_TEST_RUNNER_RULES just for the sake of avoiding this warning,
which seems to be an overkill...

I'm wondering if we can just drop that mixed native Clang/bpf LLC mode
altogether? But for now we have it with a warning in v2.

>
> >
> > native clang + llc is useful for old school tracing only (before CO-RE).
>
> Don't disagree (I actually have little context why we needed this
> special case at all), but I had no errors or warnings whatsoever. I
> think in this particular case it's not specific to test_progs-native
> build, can you please double-check on your side?

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

end of thread, other threads:[~2019-10-16  3:34 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-15 22:03 [PATCH bpf-next 0/6] Fix, clean up, and revamp selftests/bpf Makefile Andrii Nakryiko
2019-10-15 22:03 ` [PATCH bpf-next 1/6] selftest/bpf: teach test_progs to cd into subdir Andrii Nakryiko
2019-10-15 22:03 ` [PATCH bpf-next 2/6] selftests/bpf: make CO-RE reloc test impartial to test_progs flavor Andrii Nakryiko
2019-10-15 22:03 ` [PATCH bpf-next 3/6] selftests/bpf: switch test_maps to test_progs' test.h format Andrii Nakryiko
2019-10-15 22:03 ` [PATCH bpf-next 4/6] selftests/bpf: add simple per-test targets to Makefile Andrii Nakryiko
2019-10-15 22:03 ` [PATCH bpf-next 5/6] selftests/bpf: replace test_progs and test_maps w/ general rule Andrii Nakryiko
2019-10-15 23:41   ` Alexei Starovoitov
2019-10-15 23:50     ` Andrii Nakryiko
2019-10-16  3:34       ` Andrii Nakryiko
2019-10-15 22:03 ` [PATCH bpf-next 6/6] selftests/bpf: move test_queue_stack_map.h into progs/ where it belongs Andrii Nakryiko

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