Re: [PATCH bpf-next v6 4/9] selftests/bpf: generate the signing key and certificate once

[email protected]
Newsgroups org.kernel.vger.bpf
Message-ID <e9f88f9519021ce15bb886a84246e786aba20cb6395d7c73bbe14905dd329967@mail.kernel.org>
> selftests/bpf: generate the signing key and certificate once
>
> $(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 - under -j both runs execute concurrently and the
> openssl invocations race on the same output files.

The -j race described here isn't reachable at this commit. Can the
changelog clarify the ordering?

At af5eea54, $(PRIVATE_KEY) appears only at Makefile:569 where it is
embedded in LSKEL_SIGN, and at Makefile:790 as a variable assignment. It
never appears in a prerequisite list. The only consumer is

    $(VERIFY_SIG_HDR): $(VERIFICATION_CERT)

which lists a single stale target, so the old recipe could only run once.
The -j race becomes reachable five commits later (8cedb1c0019b, 'build
each test runner instance in its own sub-make') when that commit adds

    RUNNER_PREREQS := ... $(VERIFY_SIG_HDR) $(PRIVATE_KEY) $(VERIFICATION_CERT) ...

listing both files as prerequisites of the same target. This change is
correct and needed, but as a prerequisite for that later commit rather
than as a fix for current behaviour, which is also consistent with the
absence of a Fixes: tag.

> Rewrite it as a
> pattern rule: pattern rules with multiple targets are implicitly
> grouped even before make 4.3 introduced grouped (&:) targets - the
> same workaround the test_kmods rule already uses.

> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -789,7 +789,11 @@
>  VERIFICATION_CERT   := $(BUILD_DIR)/signing_key.der
>  PRIVATE_KEY := $(BUILD_DIR)/signing_key.pem
>
> -$(VERIFICATION_CERT) $(PRIVATE_KEY): $(VERIFY_SIG_SETUP)
> +# One genkey run produces both files. A plain two-target rule is not
> +# grouped - under -j make would run genkey twice concurrently and the
> +# openssl invocations race; the pattern form is implicitly grouped even
> +# with make < 4.3.
> +$(BUILD_DIR)/%.pem $(BUILD_DIR)/%.der: $(VERIFY_SIG_SETUP)
>  	$(Q)mkdir -p $(BUILD_DIR)
>  	$(Q)$(VERIFY_SIG_SETUP) genkey $(BUILD_DIR)

Is this pattern too broad?

The old rule named exactly two targets, $(BUILD_DIR)/signing_key.der and
$(BUILD_DIR)/signing_key.pem. The new pattern claims every .pem and .der
under $(BUILD_DIR):

    $(BUILD_DIR)/%.pem $(BUILD_DIR)/%.der: $(VERIFY_SIG_SETUP)

Because the target patterns contain a slash, GNU make matches them against
the whole target name and the stem may itself contain slashes, so the rule
also covers arbitrary depths such as $(BUILD_DIR)/libbpf/anything.der.
$(BUILD_DIR) is $(OUTPUT)/tools/build, the same directory the libbpf,
bpftool and resolve_btfids sub-builds write into (Makefile:223-236,
255-257).

The recipe only ever produces one stem. verify_sig_setup.sh genkey()
hardcodes the names:

    -outform PEM -out ${tmp_dir}/signing_key.pem -keyout ${tmp_dir}/signing_key.pem
    openssl x509 -in ${tmp_dir}/signing_key.pem -out ${tmp_dir}/signing_key.der -outform der

So for any stem other than signing_key the rule matches, make runs genkey,
openssl succeeds, make reports no error, and the requested file still does
not exist. The failure surfaces at the consumer instead of as 'No rule to
make target', and it repeats on every invocation. The same applies to a
mistyped .der/.pem prerequisite anywhere under $(BUILD_DIR). Also, 'mkdir
-p $(BUILD_DIR)' only creates $(BUILD_DIR) itself, so a stem containing a
directory component would not even have a writable destination.

The grouping the change needs does not require this width. The precedent
the changelog cites, Makefile:313, deliberately keeps its multi-target
pattern narrow:

    $(addprefix test_kmods/,$(subst .ko,%ko,$(TEST_KMODS))): ...

where % only stands in for the '.' of each known module name. The same
trick applies here, such as

    $(BUILD_DIR)/signing%key.pem $(BUILD_DIR)/signing%key.der

or

    $(BUILD_DIR)/%_key.pem $(BUILD_DIR)/%_key.der

with stem 'signing', which keeps the implicit grouping while matching only
the intended pair.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32610372822
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.