On Wed, Oct 16, 2019 at 9:32 AM Stanislav Fomichev <sdf@xxxxxxxxxxx> 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@xxxxxx> > > --- > > 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. [...]
