On Thu, Aug 21, 2025 at 6:00 AM Anton Protopopov
<a.s.protopopov@xxxxxxxxx> wrote:
>
> On 25/08/20 05:20PM, Andrii Nakryiko wrote:
> > On Sat, Aug 16, 2025 at 11:02 AM Anton Protopopov
> > <a.s.protopopov@xxxxxxxxx> wrote:
> > >
> > > For v5 instruction set, LLVM now is allowed to generate indirect
> > > jumps for switch statements and for 'goto *rX' assembly. Every such a
> > > jump will be accompanied by necessary metadata, e.g. (`llvm-objdump
> > > -Sr ...`):
> > >
> > > 0: r2 = 0x0 ll
> > > 0000000000000030: R_BPF_64_64 BPF.JT.0.0
> > >
> > > Here BPF.JT.1.0 is a symbol residing in the .jumptables section:
> > >
> > > Symbol table:
> > > 4: 0000000000000000 240 OBJECT GLOBAL DEFAULT 4 BPF.JT.0.0
> > >
> > > The -bpf-min-jump-table-entries llvm option may be used to control
> > > the minimal size of a switch which will be converted to an indirect
> > > jumps.
> > >
> > > The code generated by LLVM for a switch will look, approximately,
> > > like this:
> > >
> > > 0: rX <- jump_table_x[i]
> > > 2: rX <<= 3
> > > 3: gotox *rX
> > >
> > > Right now there is no robust way to associate the jump with the
> > > corresponding map, so libbpf doesn't insert map file descriptor
> > > inside the gotox instruction.
> >
> > Just from the commit description it's not clear whether that's
> > something that needs fixing or is OK? If it's OK, why call it out?..
>
> Right, will rephrase.
>
> The idea here is that if you have, say, a switch, then, most
> probably, it is compiled into 1 jump table and 1 gotox. And, if
> compiler can provide enough metadata, then this makes sense for
> libbpf to also associate JT with gotox by inserting the same map
> descriptor inside both instructions. However now this doesn't
> work, and also there are cases when one gotox can be associated with
> multiple JTs.
Ok, and right now we'll basically generate two identical BPF maps? If
we wanted to optimize this, wouldn't it be sufficient to just reuse
maps if relocation points to the same symbol?
>
> > >
> > > Signed-off-by: Anton Protopopov <a.s.protopopov@xxxxxxxxx>
> > > ---
> > > .../bpf/bpftool/Documentation/bpftool-map.rst | 2 +-
> > > tools/bpf/bpftool/map.c | 2 +-
> > > tools/lib/bpf/libbpf.c | 159 +++++++++++++++---
> > > tools/lib/bpf/libbpf_probes.c | 4 +
> > > tools/lib/bpf/linker.c | 12 +-
> > > 5 files changed, 153 insertions(+), 26 deletions(-)
> > >
> > > diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst b/tools/bpf/bpftool/Documentation/bpftool-map.rst
> > > index 252e4c538edb..3377d4a01c62 100644
> > > --- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
> > > +++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst
> > > @@ -55,7 +55,7 @@ MAP COMMANDS
> > > | | **devmap** | **devmap_hash** | **sockmap** | **cpumap** | **xskmap** | **sockhash**
> > > | | **cgroup_storage** | **reuseport_sockarray** | **percpu_cgroup_storage**
> > > | | **queue** | **stack** | **sk_storage** | **struct_ops** | **ringbuf** | **inode_storage**
> > > -| | **task_storage** | **bloom_filter** | **user_ringbuf** | **cgrp_storage** | **arena** }
> > > +| | **task_storage** | **bloom_filter** | **user_ringbuf** | **cgrp_storage** | **arena** | **insn_array** }
> > >
> > > DESCRIPTION
> > > ===========
> > > diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
> > > index c9de44a45778..79b90f274bef 100644
> > > --- a/tools/bpf/bpftool/map.c
> > > +++ b/tools/bpf/bpftool/map.c
> > > @@ -1477,7 +1477,7 @@ static int do_help(int argc, char **argv)
> > > " devmap | devmap_hash | sockmap | cpumap | xskmap | sockhash |\n"
> > > " cgroup_storage | reuseport_sockarray | percpu_cgroup_storage |\n"
> > > " queue | stack | sk_storage | struct_ops | ringbuf | inode_storage |\n"
> > > - " task_storage | bloom_filter | user_ringbuf | cgrp_storage | arena }\n"
> > > + " task_storage | bloom_filter | user_ringbuf | cgrp_storage | arena | insn_array }\n"
> > > " " HELP_SPEC_OPTIONS " |\n"
> > > " {-f|--bpffs} | {-n|--nomount} }\n"
> > > "",
> >
> > bpftool changes sifted through into libbpf patch?
>
> Yes thanks. I think I've sqhashed the fix here, becase it broke
> the `test_progs -a libbpf_str` test.
>
libbpf_str test doesn't rely on bpftool, so fixing up selftest in the
same patch makes sense (to not break bisection), but bpftool changes
still make no change and should be done separately
[...]
> >
> > > +
> > > + return -prog->sec_insn_off;
> >
> > why this return value?... can you elaborate?
>
> Jump tables generated by LLVM contain offsets relative to the
> beginning of a section. The offsets inside a BPF_INSN_ARRAY
> are absolute (for a "load unit", i.e., insns in bpf_prog_load).
> So if, say, a section A contains two progs, f1 and f2, then,
> f1 starts at 0 and f2 at F2_START. So when the f2 is loaded
> jump tables needs to be adjusted by -F2_START such that offsets
> are correct.
the thing I missed is that this isn't some sort of error condition,
it's just when offset falls into main program function
naming is also a bit misleading, IMO because it doesn't just return
instruction offset, but rather an *adjustment* to an offset in jump
table
[...]
> > where does .rel.rodata come from?
> >
> > and we don't need to adjust the contents of any of those sections, right?...
> >
> > can you please add some tests validating that two object files with
> > jumptables can be linked together and end up with proper combined
> > .jumptables section?
> >
> >
> > and in terms of code, can we do
> >
> > } else if (strcmp(..., JUMPTABLES_REL_SEC) == 0) {
> > /* nothing to do for .rel.jumptables */
> > } else {
> > pr_warn(...);
> > }
> >
> > It makes it more apparent what is supported and what's not.
>
> Yes, sure. The rodata might be obsolete, I will check, and
> .rel.jumptables is actually not used. This should be cleaned up
> once LLVM patch stabilizes. Thanks for noticing this,
> this way it is for sure added to my checklist :-)
>
ok, thanks
> >
> > > + pr_warn("relocation against STT_SECTION in section %s is not supported!\n",
> > > + src_sec->sec_name);
> > > return -EINVAL;
> > > }
> > > }
> > > --
> > > 2.34.1
> > >
