Re: [PATCH v11 bpf-next 00/12] BPF indirect jumps

[Anton Protopopov <a.s.protopopov@gmail.com>]

On 25/11/05 12:51PM, Eduard Zingerman wrote:
> On Wed, 2025-11-05 at 09:03 +0000, Anton Protopopov wrote:
> > This patchset implements a new type of map, instruction set, and uses
> > it to build support for indirect branches in BPF (on x86). (The same
> > map will be later used to provide support for indirect calls and static
> > keys.) See [1], [2] for more context.
> > 
> > Short table of contents:
> > 
> >   * Patches 1-6 implement the new map of type
> >     BPF_MAP_TYPE_INSN_SET and corresponding selftests. This map can
> >     be used to track the "original -> xlated -> jitted mapping" for
> >     a given program.
> > 
> >   * Patches 7-12 implement the support for indirect jumps on x86 and add libbpf
> >     support for LLVM-compiled programs containing indirect jumps, and selftests.
> > 
> > The jump table support was merged to LLVM and now can be
> > enabled with -mcpu=v4, see [3]. The __BPF_FEATURE_GOTOX
> > macros can be used to check if the compiler supports the
> > feature or not.
> > 
> > See individual patches for more details on the implementation details.
> 
> I retested this series with upstream clang [1] (includes latest
> changes for relocations handling from Yonghong), and all works as
> expected.
> 
> The series is ready to land from my perspective.
> (AI has a few notes on tests, though).

Thanks.  The fixes to the latest AI comments are as follows:

diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c b/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c
index ea1cd3cda156..d138cc7b1bda 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c
@@ -90,7 +90,7 @@ static void check_one_map_two_jumps(struct bpf_gotox *skel)
 
 	for (i = 0; i < prog_info.nr_map_ids; i++) {
 		map_fd  = bpf_map_get_fd_by_id(map_ids[i]);
-		if (!ASSERT_GE(map_fd, 0, "bpf_program__fd(one_map_two_jumps)"))
+		if (!ASSERT_GE(map_fd, 0, "bpf_map_get_fd_by_id"))
 			return;
 
 		len = sizeof(map_info);
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_insn_array.c b/tools/testing/selftests/bpf/prog_tests/bpf_insn_array.c
index cf852318eeb2..269870bec941 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_insn_array.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_insn_array.c
@@ -406,7 +406,7 @@ static void check_no_map_reuse(void)
 
 	/* correctness: check that prog is still loadable without fd_array */
 	extra_fd = prog_load(insns, ARRAY_SIZE(insns), NULL, 0);
-	if (!ASSERT_GE(prog_fd, 0, "bpf(BPF_PROG_LOAD): expected no error"))
+	if (!ASSERT_GE(extra_fd, 0, "bpf(BPF_PROG_LOAD): expected no error"))
 		goto cleanup;
 
 cleanup:

> [1] f60e69315e9e ("[llvm] Emit canonical linkage correct function
> symbol (#166487)")
> 
> > v10 -> v11 (this series):
> > 
> >   * rearranged patches and split libbpf patch such that first 6 patches
> >     implementing instruction arrays can be applied independently
> 
> I actually tried applying first 6 patches and then removing patch #3
> "libbpf: Recognize insn_array map type", nothing broke: kernel and
> selftests compile, relevant selftests passing.

The `test_progs -a libbpf_str` should fail without this patch.

> So, not sure if splitting patch #3 as a separate thing is really
> necessary.
> 
> > 
> >   * instruction arrays:
> >     * move [fake] aux->used_maps assignment in this patch
> > 
> >   * indirect jumps:
> >     * call clear_insn_aux_data before bpf_remove_insns (AI)
> > 
> >   * libbpf:
> >     * remove the relocations check after the new LLVM is released (Eduard, Yonghong)
> >     * libbpf: fix an index printed in pr_warn (AI)
> > 
> >   * selftests:
> >     * protect programs triggered by nanosleep from fake runs (Eduard)
> >     * patch verifier_gotox to not emit .rel.jumptables
> > 
> 
> [...]