selftests/bpf: restructure the Makefile as a layered build - #13415
selftests/bpf: restructure the Makefile as a layered build#13415kernel-patches-daemon-bpf[bot] wants to merge 9 commits into
Conversation
|
Upstream branch: 5e289c5 |
AI reviewed your patch. Please fix the bug or email reply why it's not a bug. In-Reply-To-Subject: |
b8b6131 to
71e031f
Compare
|
Upstream branch: 5e289c5 |
2c5b93f to
ce16537
Compare
|
Forwarding comment 5388326408 via email |
AI reviewed your patch. Please fix the bug or email reply why it's not a bug. In-Reply-To-Subject: |
|
Forwarding comment 5388352519 via email |
AI reviewed your patch. Please fix the bug or email reply why it's not a bug. In-Reply-To-Subject: |
|
Forwarding comment 5388419166 via email |
|
Upstream branch: 5e289c5 |
ce16537 to
fcf73f7
Compare
|
Upstream branch: 5e289c5 |
fcf73f7 to
7fac746
Compare
71e031f to
909ca3a
Compare
|
Upstream branch: d83fba2 |
7fac746 to
9e018be
Compare
909ca3a to
b010507
Compare
|
Upstream branch: ce36e38 |
9e018be to
32d1fc2
Compare
b010507 to
975b11a
Compare
|
Upstream branch: 05ea1b6 |
32d1fc2 to
4e936f9
Compare
975b11a to
f8c8078
Compare
The generic '$(OUTPUT)/%:%.c' rule links with '$(LINK.c) $^', so every prerequisite of such a binary lands on the compiler driver's command line. That only works while none of them is a header: gcc tolerates a stray .h argument on a link line, but clang treats it as a request to precompile the header and fails with "cannot specify -o when generating multiple output files". Filter headers out of the recipe, with a comment: header prerequisites on these targets are useful for dependency tracking - the next patch declares one - and the constraint is easy to rediscover the hard way. Signed-off-by: Mykola Lysenko <nickolay.lysenko@gmail.com> Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Four target-specific lines name objects nothing builds. Three name the BPF objects by their pre-rename names: commit afef88e ("selftests/bpf: Store BPF object files with .bpf.o extension") left them without a target; the flow_dissector_load.o dependency names an intermediate the generic one-step compile+link rule does not produce. Three linked-skeleton dependency map entries (xsk_xdp_progs, xdp_hw_metadata, xdp_features) were dead on arrival: commit f0a249d ("selftests/xsk: get rid of built-in XDP program"), commit 297a3f1 ("selftests/bpf: Simple program to dump XDP RX metadata") and commit 4dba3e7 ("selftests/bpf: introduce XDP compliance test tool") each added one, mimicking the neighboring entries without adding the skeleton to LINKED_SKELS, the only place the -deps map is read from; these three skeletons are generated by the regular pattern rule, which never consults it. The '-fno-inline' pair (test_l4lb_noinline, test_xdp_noinline) had stopped taking effect even earlier, with commit 74b5a59 ("selftests/bpf: Replace test_progs and test_maps w/ general rule"): since then the BPF compile recipe expands TRUNNER_BPF_CFLAGS, a simply-expanded copy of BPF_CFLAGS taken when the runner rules are instantiated, which a target-specific 'BPF_CFLAGS +=' cannot reach. Both programs have compiled without the flag since, and nothing was lost: every function they define carries a noinline annotation, except the SEC() entry points, which nothing in the file calls, and the single __always_inline helper in each, a deliberate exception the flag never overrode; and compiling with -fno-inline restored yields byte-identical objects. The two header dependencies with remaining value are restored in working form. flow_dissector_load.h moves to the binary itself, which is linked straight from its .c by the generic '$(OUTPUT)/%:%.c' rule - editing the header now rebuilds it - on the line that already lists the binary's helper object. cgroup_getset_retval_hooks.h - added by commit e7215f5 ("selftests/bpf: Make sure bpf_{g,s}et_retval is exposed everywhere") days before the rename orphaned it - lived in the top directory, outside the progs/*.h blanket prerequisite of the BPF object rules, so editing it never rebuilt the BPF object. Move it under progs/, next to its only BPF consumer, where the blanket prerequisite covers it - the arrangement several other headers shared between progs/ and prog_tests/ already use. Its userspace consumer is tracked exactly by the compiler-generated dependency files. 'CURDIR := $(abspath .)' redefines make's builtin to the value it already has, and 'OBJCOPY ?= $(CROSS_COMPILE)objcopy' defines a variable nothing in the selftests build or the included kselftest infrastructure ever reads; drop both. Signed-off-by: Mykola Lysenko <nickolay.lysenko@gmail.com> Acked-by: Eduard Zingerman <eddyz87@gmail.com>
With BPF_STRICT_BUILD=0, eleven recipes append the same "|| { remove
the target, print a SKIP marker, report success }" tail, each spelled
out inline. Factor the tail into skip_on_fail; every call site keeps its
exact message and behavior.
The permissive fragments of other shapes are not suffixes and keep
their current form: the rsync --ignore-missing-args flags, the
missing-input guards in the skeleton recipes, the link rule's
wildcard handling and the test_kmods copy wrapper.
Signed-off-by: Mykola Lysenko <nickolay.lysenko@gmail.com>
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
$(VERIFICATION_CERT) and $(PRIVATE_KEY) come from a single genkey invocation, but a rule with two normal targets runs its recipe once per stale target. Today only $(VERIFICATION_CERT) is ever a prerequisite (of $(VERIFY_SIG_HDR)), so the recipe runs once; the runner patch later in this series lists both files as prerequisites of the same target, at which point both runs would execute, under -j concurrently, and the openssl invocations would race on the same output files. Rewrite it as a pattern rule, the same workaround the test_kmods rule already uses, with the stem narrowed to the fixed name: genkey hardcodes signing_key.pem / signing_key.der, and a wider pattern would claim every .pem/.der under $(BUILD_DIR) and "succeed" without producing the requested file. Signed-off-by: Mykola Lysenko <nickolay.lysenko@gmail.com> Acked-by: Eduard Zingerman <eddyz87@gmail.com>
The verifier/tests.h recipe is a $(shell ...) expansion: the command runs while make expands the recipe line - including under make -n - its exit status is discarded, and the resulting (empty) expansion is what make actually executes. Signed-off-by: Mykola Lysenko <nickolay.lysenko@gmail.com> Acked-by: Eduard Zingerman <eddyz87@gmail.com>
|
Upstream branch: 1555de3 |
The bench binary links every benchs/bench_*.c object, but the link rule names them one by one in a hand-maintained list, which has to be extended by hand for every new benchmark although the pattern rule already builds any bench_*.c placed in benchs/. Derive the list with a wildcard instead: a new benchmark is compiled and linked in by dropping its source there (its skeleton dependency line, when it has one, is still declared next to the others). The derived list is sorted, which changes the link order of the bench objects (previously roughly chronological) and with it the symbol layout of the binary; no benchmark behaves differently. The trailing '#' terminator goes away together with the block it closed: the two remaining entries after the variable are fixed, so the append friendliness it provided no longer buys anything. Signed-off-by: Mykola Lysenko <nickolay.lysenko@gmail.com> Acked-by: Eduard Zingerman <eddyz87@gmail.com>
The four skeleton generation recipes (regular, light, signed light and linked) are near-identical pipelines - link via "bpftool gen object" three times, compare the second and third results as a regression test for bpftool's determinism, generate the skeleton (and possibly subskeleton), remove intermediates - duplicated with small variations inside DEFINE_TEST_RUNNER_RULES, where every line pays the double-expansion escaping tax. Move the pipeline into gen_bpf_skel.sh; the differences between the four variants become the --lskel, --sign and --subskel flags. Signing takes the key and certificate from $PRIVATE_KEY and $VERIFICATION_CERT in the environment, like the bpftool binary comes from $BPFTOOL; the script checks for both up front, before any intermediate exists. The distinct linked/llinked intermediate infixes are kept but derived inside the script (its header says why they matter). The intermediates are now named after the output header rather than the input object (foo.linked1.o instead of foo.bpf.linked1.o for the single-object variants; the linked-skeleton variant already used that form), which keeps the stems distinct just the same. The permissive-mode missing-input checks at the top of the recipes - when the object's compile already failed and was skipped, skip the skeleton quietly instead of running bpftool against a missing file - are kept, factored into a skip_if_missing helper. The build-log messages stay folded behind the guard as in the original recipes, so a skipped skeleton prints SKIP-SKEL and nothing else. What does change is the failure path: the script removes the intermediates it created (the recipes left them behind), the determinism check reports through cmp plus one message instead of a diff listing, and the linked-skeleton recipe prints its GEN-SKEL line before the link rather than after it, like the other three always did. Suggested-by: Eduard Zingerman <eddyz87@gmail.com> Assisted-by: Claude:claude-fable-5 shellcheck Signed-off-by: Mykola Lysenko <nickolay.lysenko@gmail.com> Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Move the build definitions a test runner needs - tree layout and tool locations, flag assembly, feature probes, signing key paths, the permissive-mode helpers - into Makefile.buildvars, in their current order, as preparation for building each test runner instance in its own sub-make: the definitions become includable by more than one makefile. No rules or recipes are changed. The include sits directly after ../lib.mk, ahead of the ../../../build/Makefile.feature evaluation, so the definitions are in place for everything the top Makefile parses afterwards - in particular the LLVM feature probe captures srctree and PKG_CONFIG from their new location, and the top Makefile's remaining uses of both are all recipe-time expansions. Two evaluation-time changes follow from the new position and a third is made on the way, each commented in place: CFLAGS and LDFLAGS are reassembled around a snapshot taken before lib.mk, keeping the pre-split order; CLANG_SYS_INCLUDES becomes an immediate assignment (its inputs are final by this point, and expanding it forks shells); and the CLANG_HAS_ARENA_ASAN probe now sees CLANG as resolved from LLVM=<suffix-or-path> rather than the bare 'clang' default it saw above the include, while the CLANG_CPUV4 probe, needed before lib.mk, keeps the old behavior. BPF_GCC, TEST_KMODS and the knobs the runner never reads (SKIP_*, submake_extras, TEST_KMOD_TARGETS, the VMLINUX_BTF block) stay in the Makefile - the first two are needed before lib.mk is included, where Makefile.buildvars cannot yet be, and reach the runners from there once a later patch adds them: TEST_KMODS exported in the environment, BPF_GCC as the bpf_gcc flavor's compiler parameter. Definitions that reach no runner, directly or as a sub-make parameter, also stay - except HOST_INCLUDE_DIR, which moves with the conditional block it shares with the host build directories - and the four that derive from Makefile.buildvars values sit in a block right after the include. Suggested-by: Eduard Zingerman <eddyz87@gmail.com> Assisted-by: Claude:claude-fable-5 shellcheck Signed-off-by: Mykola Lysenko <nickolay.lysenko@gmail.com>
Replace the DEFINE_TEST_RUNNER/DEFINE_TEST_RUNNER_RULES double-expansion machinery - whose rules never appear in the source as written, are invisible to make's own debugging facilities, and are easy to break for one flavor while testing another - with Makefile.runner: one sub-make invocation per test runner instance (test_progs, test_progs-no_alu32, test_progs-cpuv4, test_progs-bpf_gcc, test_maps), each in its own single-flavor namespace written in plain make - no define/eval layer, no per-flavor guards, no accumulating vpath directives. The main Makefile keeps everything that exists once - tool sub-builds, vmlinux.h, signing keys, tests.h generation, standalone binaries and the kselftest lib.mk contract - and delegates to the runner through explicit per-instance rules. The rules for one flavor's BPF objects and skeletons live in Makefile.skel, instantiated by the main Makefile for the default flavor and by each runner sub-make that builds BPF objects. lib.mk's install rule copies the default flavor's BPF objects - the files the flat copy of every flavor's objects resolved to before. No file is built twice in one build: the main Makefile produces the default flavor's artifacts before the unflavored test_progs sub-make runs, and each flavored runner writes its own subdirectory plus its binary in $(OUTPUT) (the bpftool link in $(OUTPUT) is refreshed by both unflavored sub-makes, as before). The runner instances link userspace objects the main Makefile builds once; the flavored instances link the shared objects instead of compiling their own identical copies. This also fixes a latent parallel-build race: the runner objects including libbpf's internal headers now order against the bpftool sub-build that installs them into $(INCLUDE_DIR). Smaller behaviour changes that come with the move, for the record: the skeleton recipes' build-log lines take the msg helper's format and stream, the shared userspace objects log as CC rather than EXT-OBJ/LIB-OBJ, and TEST-HDR lines lose their runner tag; the signed light-skeleton rule lists the key and certificate it uses as prerequisites; the extras copy skips an empty file list and, in permissive mode, does not wait for files whose build was skipped; $(OUTPUT)/test_maps waits for the same shared prerequisites as the other runners; the bare-name convenience targets for the linked-skeleton constituents (make linked_funcs1.bpf.o) are gone; the .d include no longer special-cases the clean, docs-clean and emit_tests goals, which a runner sub-make never runs; and the test objects' first-build ordering lists only the generated headers the runner itself consumes, the rest being built by the main Makefile before any runner starts. The runner sub-makes receive CC and CLANG as resolved by lib.mk on their command line: Makefile.buildvars probes clang's capabilities with $(CLANG), and a runner has to reach the same verdict as the top level when LLVM=<suffix or path> selects a non-default toolchain. The unflavored in-tree instance skips the extras copy as before; the runner compares the realpath of $(OUTPUT) with $(CURDIR), make's physical working directory, so a tree reached through a symlink still counts as in-tree. The userspace objects depend on the generated tests.h of the runner that includes them only (test_progs.o on prog_tests/tests.h, test_maps.o on map_tests/tests.h); before, every extra object of a runner depended on that runner's tests.h. The other headers those objects depended on - flow_dissector_load.h, ip_check_defrag_frags.h, the libarena skeletons - are included by none of them and are dropped. Co-developed-by: Eduard Zingerman <eddyz87@gmail.com> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> Assisted-by: Claude:claude-fable-5 shellcheck Signed-off-by: Mykola Lysenko <nickolay.lysenko@gmail.com>
4e936f9 to
cbe808f
Compare
|
At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=1150503 expired. Closing PR. |
Pull request for series with
subject: selftests/bpf: restructure the Makefile as a layered build
version: 7
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1150503