[PATCH bpf-next v7 0/9] selftests/bpf: restructure the Makefile as a layered build
Mykola Lysenko <[email protected]>
| Newsgroups | org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
The BPF selftests Makefile has grown to ~1,100 lines, with much of the
complexity concentrated in the DEFINE_TEST_RUNNER double-expansion
machinery: the per-runner rules are written in $$-escaped make and
eval'd once per flavor, so the rules that actually run never appear in
the source, are invisible to make's own debugging facilities, and are
easy to break for one flavor while testing another.
The series opens with six small stand-alone changes, each usable
(or droppable) on its own: patch 1 filters headers off the generic
link rule's command line (gcc tolerates a stray .h there, clang
refuses); patch 2 drops four stale target-specific lines that name
objects nothing builds - restoring the two dependencies of value
among them in working form (flow_dissector_load.h against the
binary; cgroup_getset_retval_hooks.h by moving it under progs/,
where the blanket header prerequisite of the BPF object rules covers
it) - plus three never-referenced linked-skeleton dependency-map
entries, a no-op CURDIR override and the OBJCOPY definition nothing
reads; patch 3 factors the eleven
identical permissive-mode "remove the target, print SKIP, report
success" recipe tails into a skip_on_fail helper; patch 4 makes the
signing key generation race-free under -j; patch 5 turns the
verifier/tests.h $(shell)-in-recipe into a regular recipe; patch 6
derives the bench object list from the sources with a wildcard. The
restructure itself is three steps, each independently buildable and
each producing byte-identical artifacts:
7) the four near-identical skeleton generation recipes move into a
parameterized helper script (gen_bpf_skel.sh);
8) shared (non-rule) definitions move into Makefile.buildvars;
9) each test runner instance (test_progs, test_progs-<flavor>,
test_maps) becomes its own plain-make sub-make (Makefile.runner),
with the BPF object and skeleton rules both sides share in
Makefile.skel, replacing the eval/escaping layer.
After the series the top Makefile owns everything that exists once
(toolchain setup, the libbpf/bpftool/resolve_btfids sub-builds,
vmlinux.h, all userspace objects the runners link, the default
flavor's BPF objects and skeletons, standalone binaries, and the
lib.mk run/install contract), while every runner instance is an
ordinary single-flavor makefile with no escaping. Total line count is
roughly a wash; the point is the structure, not the size.
Validation (each patch, x86_64, identical toolchains, clean builds):
3,031 BPF objects and 5,517 skeleton headers byte-identical to the
current Makefile's output - except the one object whose header
patch 2 moves (and its skeletons), which differs only in the
source-path strings recorded in BTF/debug info, verified by
comparing the objects with those sections stripped - with identical
build-artifact file lists through patch 8. For the end state
additionally: all userspace objects and binaries byte-identical
except (a) bench (from patch 6 on) and test_maps, which differ only
in object link order, proven by relinking the current Makefile's
objects in the new order and comparing bytes, (b) the test_progs binaries and two test
objects that embed the moved header's skeleton or record its path,
and (c) the flavor output directories, which no longer contain their
21 duplicate copies each of the shared userspace objects (the
flavored binaries link the shared objects); emit_tests output and
the installed tree identical; BPF CI green on manual pre-submission
runs of the full series and of every cumulative intermediate
(patch 1, patches 1-2, ..., patches 1-8) across x86_64 gcc/llvm,
aarch64 and s390x, including the GCC-BPF, ASAN and veristat jobs;
and all 78 benchmarks produce identical outcomes on old- and
new-built bench binaries.
v6 -> v7 (BPF CI AI review of v6, all findings verified):
- patch 8: an exported CFLAGS/LDFLAGS changed position - the split
had put Makefile.buildvars' flags in front of the environment's
instead of after them, so e.g. an exported -O2 overrode the
selftests' -O0 and environment -I paths moved behind the tree's.
The Makefile now snapshots the inherited flags before including
lib.mk and Makefile.buildvars rebuilds both variables as
[inherited] [its own] [lib.mk's additions]; the runner sub-makes
start from the same snapshot (handed down in the environment)
rather than from the assembled value. With CFLAGS, LDFLAGS,
USERCFLAGS and USERLDFLAGS all set, the top-level and runner-side
values are now identical to the current Makefile's
- patch 9: CLANG is passed to the runner sub-makes like CC, so the
clang feature probe in Makefile.buildvars tests the LLVM=-selected
compiler there too (it fell back to plain clang, and could disagree
with the top level on HAS_BPF_ARENA_ASAN); the unflavored in-tree
instance recognizes a tree reached through a symlink (realpath of
OUTPUT against CURDIR; the two compared a logical and a physical
path); the userspace objects depend on the tests.h of the runner
that includes them only, instead of every object on both; the
objects order against $(TRUNNER_BPFTOOL) rather than $(BPFTOOL),
which is the sub-make that installs libbpf's internal headers into
$(INCLUDE_DIR) when cross-compiling as well; the dead BPF_GCC export
is gone (the bpf_gcc flavor gets it as its compiler parameter)
- patch 2: the '-fno-inline' pair had been ineffective since commit
74b5a5968fe8 (2019), not since the .bpf.o rename: the compile recipe
expands a simply-expanded copy of BPF_CFLAGS, which a target-specific
append cannot reach - the changelog said so incorrectly
- every commit message was checked claim by claim against its diff
and its parent tree, and every hunk against the message: stale
statements left over from earlier respins are corrected, the
behaviour deltas the runner patch carries are now listed in its
message, and stock comments that had been reworded along the way
are back to their original text
- patch 4: the genkey pattern rule is narrowed to the signing_key
stem ($(BUILD_DIR)/%_key.pem %_key.der) - the .pem/.der form matched
every such file under tools/build; the changelog now says the
two-target race is a prerequisite for patch 9 rather than reachable
today (nothing lists $(PRIVATE_KEY) as a prerequisite before then)
- patch 7: gen_bpf_skel.sh checks for PRIVATE_KEY/VERIFICATION_CERT
before creating any intermediate (the ${VAR:?} expansion fired after
them, skipping the cleanup); the changelog notes that intermediates
are named after the output header now (foo.linked1.o, not
foo.bpf.linked1.o)
- patch 2: retitled (it restores two header dependencies, not only
drops lines) and the flow_dissector_load.h dependency sits on the
line that already lists the binary's helper object
- patch 6, 8: changelog corrections (bench list tail, VERIFY_SIG_HDR,
LIBARENA_BENCH_SKEL, the third evaluation-time change: the arena
ASAN probe runs after lib.mk resolves CLANG)
- Acked-by on patches 4 and 7 kept through the small code changes
above
- v6: https://lore.kernel.org/bpf/[email protected]/
v5 -> v6 (all on review of v5 - thanks Eduard!):
- the permissive-mode missing-input guards at the top of the skeleton
recipes are restored, factored into a skip_if_missing helper next
to skip_on_fail: when a BPF object's compile already failed and was
skipped, the skeleton skips quietly again (SKIP-SKEL and nothing
else) instead of running bpftool against the missing file first
(reported by Sashiko AI review on v5 patch 6; v5 had regressed the
guards away, changing permissive-mode build logs)
- patch 2: cgroup_getset_retval_hooks.h moves under progs/ next to
its BPF consumer instead of growing a hand-written per-flavor
dependency line - the blanket progs/*.h prerequisite covers it
(suggested by Eduard)
- patch 2 also picks up the three linked-skeleton -deps map entries
(xsk_xdp_progs, xdp_hw_metadata, xdp_features) that were dead on
arrival - added without ever being listed in LINKED_SKELS, the only
reader of that map - instead of letting them disappear silently
with the machinery in patch 9
- patch 7: the Makefile.buildvars include is hoisted to directly
after ../lib.mk, ahead of the Makefile.feature evaluation, so the
srctree fallback and PKG_CONFIG live only in Makefile.buildvars and
the top Makefile's duplicates are gone (suggested by Eduard)
- patch 8: the userspace objects the runners link are built once by
the main Makefile from per-runner lists in Makefile.buildvars; the
runner loses its object compile rules, disables make's built-in
rules so a missing object is an error instead of a silent rebuild
with default flags, and the flavored instances link the shared
objects instead of compiling identical copies (patch by Eduard,
replacing the HELPER_OBJS pre-build arrangement and its
keep-in-sync invariant)
- the rules for one flavor's BPF objects and skeletons move into
Makefile.skel, instantiated by the main Makefile
for the default flavor and by Makefile.runner for the flavored
instances - bench, xskxceiver, xdp_*, test_cpp and the install list
depend on exactly the skeletons they consume again (cold "make
bench" builds 19 BPF objects and 19 skeletons instead of the full
default-flavor pass), and the default-skels indirection with its
delegation rules is gone; Makefile.skel is self-contained - the
hand-maintained skeleton metadata (demand lists, LINKED_SKELS with
its -deps map, the blacklist, the header list, per-source flag
overrides) sits next to the derivations and rules that read it,
and Makefile.buildvars keeps only definitions shared between files
(the Makefile.skel split suggested by Eduard)
- the buildvars move set is trimmed to match: CXX, BPFTOOLDIR,
HOST_BPFOBJ, BPF_TARGET_ENDIAN, CLANG_CFLAGS and VERIFY_SIG_HDR are
read by nothing outside the main Makefile and stay in it
- the bench object list derivation is split out of the runner patch
into its own lead patch (6), which also carries the resulting bench
link-order change; rebasing onto current bpf-next, the new bpf_for
and libarena benchmarks are picked up by the derived list, and the
xskxceiver/xdp decoupling commit is untouched by the series (those
standalone binaries keep their stock rules and exact dependencies)
- the runner patch carries a FORCE comment; each patch cleans up only
its own comment and blank-line leftovers (suggested by Eduard)
- Acked-by collected on patches 1, 3, 4, 5 and 7 (patch 7 was acked
as v5 patch 6; it since gained the skip_if_missing guards above)
- the RFC tag is dropped - the direction question the RFC asked has
its answer in the v5 review
- v5: https://lore.kernel.org/bpf/[email protected]/
v4 -> v5:
- the four stale target-specific lines (test_l4lb_noinline /
test_xdp_noinline -fno-inline, flow_dissector_load.o /
cgroup_getset_retval_hooks.o header deps) move out of the runner
patch into a leading cleanup patch (which also redeclares the
flow_dissector_load.h and cgroup_getset_retval_hooks.h
dependencies, the two of the four with value, in working form), preceded by a patch filtering headers off the
generic link command line so every intermediate state builds with
either compiler - the series is now 6 patches; per-patch validation
notes leave the commit messages - validation for every patch is
summarized above instead.
- the permissive-mode skip suffix becomes the skip_on_fail helper in
its own patch before the restructure, so the script patch's new
skeleton recipes never carry the open-coded tails and the buildvars
patch stays a pure move; the signing-key race fix and the
verifier/tests.h recipe conversion likewise become their own
patches instead of riding inside the runner patch.
- runner patch sheds incidental churn: the '# Some utility functions
use LLVM libraries' comment moves to Makefile.runner together with
the jit_disasm_helpers CFLAGS line it describes; the notdir
convenience rule, the generic compile rule, BPF_GCC's stock ?=
probe and surrounding blank lines are left exactly as they are in
the current Makefile.
- buildvars patch: moving the srctree fallback and PKG_CONFIG into
Makefile.buildvars put them after ../../../build/Makefile.feature is
parsed, which captures both at parse time - in a standalone build
srctree was empty there, so the LLVM feature probe targeted
/tools/build/feature, failed with stderr discarded, and feature-llvm
silently read 0: the jit-disassembler helpers compiled into their
-EOPNOTSUPP stubs and the __jited verifier tests
(verifier_private_stack, verifier_jit_inline, verifier_jit_convergence,
verifier_tailcall_jit) silently flipped to SKIP, confirmed in the v4
BPF CI logs ("llvm: [ OFF ]" in the feature display). Both now stay
untouched at their original positions in the Makefile, with guarded
copies kept in Makefile.buildvars for the runner sub-makes (found by
the bpf-ci AI reviewer).
- buildvars patch: the changelog now names both evaluation-time changes
(CFLAGS prepend, CLANG_SYS_INCLUDES immediate assignment) instead of
claiming a single exception (bpf-ci AI reviewer).
- runner patch: the commit message no longer claims the light-skeleton
demand lists are derived from #include lines - that derivation was
dropped in v4; the lists stay hand-maintained (found by Sashiko AI
review).
- v4: https://lore.kernel.org/bpf/[email protected]/
v3 -> v4:
- fix patch 2: "export BPF_GCC TEST_KMODS" preceded the TEST_KMODS
definition, and 'export NAME' on an undefined variable creates it
empty, turning the ?= default into a no-op - the kernel test modules
were silently neither built nor installed and every BPF CI
test-runner job failed with "Can't find bpf_testmod.ko" (caught by
BPF CI; not reproducible locally where TEST_KMODS is overridden).
The definitions now precede the export, with a comment on the trap.
- addressed Eduard's (partial) review of patch 3: header comment
trimmed; CHECK_FEAT cosmetic reverted; the stock named helper
variables and per-binary dependency lines are kept (HELPER_OBJS is
defined from them); map_tests/tests.h added to the runner
prerequisites; the include-derived skeleton lists are dropped from
this series in favor of the current hand-maintained lists (the
derivation can be a follow-up); the per-flavor BPF compiler
differences became plain sub-make parameters, removing the runner's
only conditional compile block; content the series does not change
(helper variables and dependency lines, docs and resolve_btfids
rules, uprobe_multi, verifier/tests.h, the libarena targets) keeps
its original file position, so it appears as context rather than
churn in the diff
- addressed Eduard's review of patch 2: Makefile.buildvars keeps the
definitions in the order they had in the original Makefile; knobs
the runner never reads (SKIP_*, submake_extras, VMLINUX_BTF,
TEST_KMOD_TARGETS) stay in the main Makefile (TEST_KMOD_TARGETS is
inlined at its single runner use); patch 2 no longer touches
TEST_KMODS, TEST_KMOD_TARGETS or the VMLINUX_BTF block at all - they
keep their current positions, and the export the runners need moves
to patch 3 next to the LLVM-probe export; file header comments
trimmed
- addressed Eduard's review of patch 1: the gen_bpf_skel.sh interface
shrinks to --name/--skel/--subskel/--lskel/--sign - the
linked/llinked intermediate infix is derived inside the script,
--sign is a boolean taking the key and certificate from
$PRIVATE_KEY/$VERIFICATION_CERT in the environment, the build-log
lines moved into the make recipes ($(call msg,...)), and the unused
--no-determinism-check escape hatch is gone (the determinism check
is unconditional)
- v3: https://lore.kernel.org/bpf/[email protected]/
v2 -> v3:
- BPF_GCC and TEST_KMODS are defined once (main Makefile, which needs
them before lib.mk) and exported to the runner sub-makes instead of
being duplicated in Makefile.buildvars, where the copies could
silently drift (reported by Sashiko AI review)
- the permissive-mode runner link rule keeps the current Makefile's
incremental semantics: test objects existing at parse time are
normal prerequisites again, so editing a test source relinks the
runner (reported by Sashiko AI review; v2 had regressed this to
order-only)
- v2: https://lore.kernel.org/bpf/[email protected]/
v1 -> v2:
- rebased onto current bpf-next: ported the semantics of commit
0b236ac75d04 ("selftests/bpf: Fix make install target") - bpftool
installed under tools/sbin/, *.BTF files installed - into the new
install rule (resolves the CI apply conflict against v1)
- gen_bpf_skel.sh is a prerequisite of every skeleton rule, so editing
the script regenerates the headers (reported by Sashiko AI review)
- the shared helper objects the top Makefile pre-builds now depend on
a superset of the prerequisites the runner-side object rule uses
(libarena skeletons, both generated tests.h headers), closing a
window where the unflavored test_progs and test_maps sub-makes could
both consider a shared helper object stale and recompile it
concurrently in the shared output directory (reported by Sashiko AI
review)
- v1: https://lore.kernel.org/bpf/[email protected]/
Mykola Lysenko (9):
selftests/bpf: keep headers off the generic link command line
selftests/bpf: drop stale lines, restore two header dependencies
selftests/bpf: factor the permissive-mode skip suffix into a helper
selftests/bpf: generate the signing key and certificate once
selftests/bpf: generate verifier/tests.h in a regular recipe
selftests/bpf: derive the bench object list from the sources
selftests/bpf: extract BPF skeleton generation into a helper script
selftests/bpf: move shared build definitions into Makefile.buildvars
selftests/bpf: build each test runner instance in its own sub-make
tools/testing/selftests/bpf/Makefile | 741 ++++--------------
.../testing/selftests/bpf/Makefile.buildvars | 196 +++++
tools/testing/selftests/bpf/Makefile.runner | 199 +++++
tools/testing/selftests/bpf/Makefile.skel | 141 ++++
tools/testing/selftests/bpf/gen_bpf_skel.sh | 99 +++
.../bpf/prog_tests/cgroup_getset_retval.c | 2 +-
.../{ => progs}/cgroup_getset_retval_hooks.h | 0
7 files changed, 795 insertions(+), 583 deletions(-)
create mode 100644 tools/testing/selftests/bpf/Makefile.buildvars
create mode 100644 tools/testing/selftests/bpf/Makefile.runner
create mode 100644 tools/testing/selftests/bpf/Makefile.skel
create mode 100755 tools/testing/selftests/bpf/gen_bpf_skel.sh
rename tools/testing/selftests/bpf/{ => progs}/cgroup_getset_retval_hooks.h (100%)
base-commit: a284ed47ec1fd4aa63d2318d87f457cc421a93b5
--
2.43.0