BPF List
 help / color / mirror / Atom feed
From: Anton Protopopov <a.s.protopopov@gmail.com>
To: bot+bpf-ci@kernel.org
Cc: sidchintamaneni@gmail.com, bpf@vger.kernel.org, ast@kernel.org,
	daniel@iogearbox.net, john.fastabend@gmail.com,
	andrii@kernel.org, eddyz87@gmail.com, memxor@gmail.com,
	martin.lau@linux.dev, song@kernel.org, yonghong.song@linux.dev,
	jolsa@kernel.org, emil@etsalapatis.com, ihor.solodrai@linux.dev,
	puranjay@kernel.org, rlmenge@gmail.com, hargar@microsoft.com,
	apais@microsoft.com, martin.lau@kernel.org, mason@kernel.org
Subject: Re: [PATCH bpf-next v2 2/2] selftests/bpf: Test terminal gotox instructions
Date: Tue, 1 Sep 2026 13:58:24 +0000	[thread overview]
Message-ID: <apbaAHkjpsgJMtk0@mail.gmail.com> (raw)
In-Reply-To: <882f92965fedba779d677b2b8600682bb0ea33673121e42b81a430d3a3636745@mail.kernel.org>

On 26/08/31 11:26PM, bot+bpf-ci@kernel.org wrote:
> > diff --git a/tools/testing/selftests/bpf/progs/verifier_gotox.c b/tools/testing/selftests/bpf/progs/verifier_gotox.c
> > index 5b18c9a27717b..89067ab7baaa4 100644
> > --- a/tools/testing/selftests/bpf/progs/verifier_gotox.c
> > +++ b/tools/testing/selftests/bpf/progs/verifier_gotox.c
> > @@ -47,6 +47,67 @@ DEFINE_SIMPLE_JUMP_TABLE_PROG(reserved_field_src_reg,      BPF_REG_1, 0, 0, __fa
> >  DEFINE_SIMPLE_JUMP_TABLE_PROG(reserved_field_non_zero_off, BPF_REG_0, 1, 0, __failure __msg("BPF_JA|BPF_X uses reserved fields"))
> >  DEFINE_SIMPLE_JUMP_TABLE_PROG(reserved_field_non_zero_imm, BPF_REG_0, 0, 1, __failure __msg("BPF_JA|BPF_X uses reserved fields"))
> >
> > +SEC("socket")
> > +__success __retval(0)
> > +__naked void jump_table_terminal_gotox(void)
> > +{
> > +	asm volatile ("						\
> > +	.pushsection .jumptables,\"\",@progbits;		\
> > +jt0_%=:							\
> > +	.quad ret0_%= - socket;					\
> > +	.size jt0_%=, 8;					\
> > +	.global jt0_%=;						\
> > +	.popsection;						\
> > +								\
> > +	r0 = jt0_%= ll;						\
> > +	r0 = *(u64 *)(r0 + 0);					\
> > +	goto end_%=;						\
> > +ret0_%=:							\
> > +	r0 = 0;							\
> > +	exit;							\
> > +end_%=:							\
> > +	.8byte %[gotox_r0];					\
> > +"	:
> > +	: __imm_insn(gotox_r0, BPF_RAW_INSN(BPF_JMP | BPF_JA | BPF_X,
> > +					    BPF_REG_0, 0, 0, 0))
> > +	: __clobber_all);
> > +}
> > +
> > +static __naked __noinline __used
> > +void terminal_gotox_subprog(void)
> > +{
> > +	asm volatile ("						\
> > +	.pushsection .jumptables,\"\",@progbits;		\
> > +jt0_%=:							\
> > +	.quad ret0_%= - .text;					\
> > +	.size jt0_%=, 8;					\
> > +	.global jt0_%=;						\
> > +	.popsection;						\
> > +								\
> > +	r0 = jt0_%= ll;						\
> > +	r0 = *(u64 *)(r0 + 0);					\
> > +	goto end_%=;						\
> > +ret0_%=:							\
> > +	r0 = 0;							\
> > +	exit;							\
> > +end_%=:							\
> > +	.8byte %[gotox_r0];					\
> > +"	:
> > +	: __imm_insn(gotox_r0, BPF_RAW_INSN(BPF_JMP | BPF_JA | BPF_X,
> > +					    BPF_REG_0, 0, 0, 0))
> > +	: __clobber_all);
> > +}
> 
> This isn't a bug, but terminal_gotox_subprog() differs from
> jump_table_terminal_gotox() above only in the jump-table base symbol
> (- .text vs - socket). The file elsewhere factors similar shapes into
> macros (DEFINE_SIMPLE_JUMP_TABLE_PROG, DEFINE_INVALID_SIZE_PROG,
> DEFINE_JUMP_TABLE_USE_REG). Would a small macro parameterised on the
> base read better here?
> 
> Also, since libbpf appends the only called subprogram last, this
> subprogram's gotox ends up as the final instruction of the whole loaded
> program. Would adding a further subprogram after it give coverage of the
> fall-through-into-the-next-subprog case that the relaxed check in
> check_subprogs() guards?

If you add a macro as robot suggested, then the
jump_table_terminal_gotox_subprog() below can call two subprogs
(same, with different names).

Otherwise, looks good to me!

> > +
> > +SEC("socket")
> > +__success __retval(0)
> > +__naked void jump_table_terminal_gotox_subprog(void)
> > +{
> > +	asm volatile ("						\
> > +	call terminal_gotox_subprog;				\
> > +	exit;							\
> > +"	::: __clobber_all);
> > +}
> 
> [ ... ]
> 
> 
> ---
> 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/33447655511


  reply	other threads:[~2026-09-01 13:47 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-31 22:33 [PATCH bpf-next v2 1/2] bpf: allow terminal gotox instructions Siddharth Chintamaneni
2026-08-31 22:33 ` [PATCH bpf-next v2 2/2] selftests/bpf: Test " Siddharth Chintamaneni
2026-08-31 23:26   ` bot+bpf-ci
2026-09-01 13:58     ` Anton Protopopov [this message]
2026-09-01 16:21       ` Siddharth Chintamaneni
2026-08-31 23:26 ` [PATCH bpf-next v2 1/2] bpf: allow " bot+bpf-ci
2026-09-01 13:51   ` Anton Protopopov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=apbaAHkjpsgJMtk0@mail.gmail.com \
    --to=a.s.protopopov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=apais@microsoft.com \
    --cc=ast@kernel.org \
    --cc=bot+bpf-ci@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=emil@etsalapatis.com \
    --cc=hargar@microsoft.com \
    --cc=ihor.solodrai@linux.dev \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=martin.lau@kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=mason@kernel.org \
    --cc=memxor@gmail.com \
    --cc=puranjay@kernel.org \
    --cc=rlmenge@gmail.com \
    --cc=sidchintamaneni@gmail.com \
    --cc=song@kernel.org \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox