On 25/06/18 09:01AM, Alexei Starovoitov wrote:
> On Wed, Jun 18, 2025 at 7:43 AM Anton Protopopov
> <a.s.protopopov@xxxxxxxxx> wrote:
> >
> > On 25/06/17 08:24PM, Alexei Starovoitov wrote:
> > > On Sun, Jun 15, 2025 at 1:55 AM Anton Protopopov
> > > <a.s.protopopov@xxxxxxxxx> wrote:
> > > > +SEC("syscall")
> > > > +int two_towers(struct simple_ctx *ctx)
> > > > +{
> > > > + switch (ctx->x) {
> > > >
> > >
> > > Not sure why you went with switch() statements everywhere.
> > > Please add few tests with explicit indirect goto
> > > like interpreter does: goto *jumptable[insn->code];
> >
> > This requires to patch libbpf a bit more, as some meta-info
> > accompanying this instruction should be emitted, like LLVM does with
> > jump_table_sizes. And this probably should be a different section,
> > such that it doesn't conflict with LLVM/GCC. I thought to add this
> > later, but will try to add to the next version.
>
> Hmm. I'm not sure why llvm should handle explicit indirect goto
> any different than the one generated from switch.
> The generated bpf.o should be the same.
For a switch statement LLVM will create a jump table
and create the {,.rel}.llvm_jump_table_sizes tables.
For a direct goto *, say
static const void *table[] = {
&&l1, &&l2, &&l3, &&l4, &&l5,
};
if (index > ARRAY_SIZE(table))
return 0;
goto *table[index];
it will not generate {,.rel}.llvm_jump_table_sizes. I wonder, does
LLVM emit the size of `table`? (If no, then some assembly needed to
emit it.) In any case it should be easy to add this case, but still
it is a bit of coding, thus a bit different case.)
> > > Remove all bpf_printk() too and get easy on names.
> >
> > The `bpf_printk` is there to emit some instructions which later will
> > be replaced by the verifier with more instructions; this is to
> > additionally test "instruction set" basic functionality
> > (orig->xlated mapping). Do you think this selftest shouldn't have
> > this?
>
> None of the runnable tests should have bpf_printk() since
> it spams the global trace pipe.
> There are few tests that have printks, but they shouldn't be runnable.
> It's load only.
Ok, thanks, makes total sense now
> > > i_am_a_little_tiny_foo() sounds funny today, but
> > > it won't be funny at all tomorrow.
> >
> > Yeah, thanks, will rename it.
