From: Anton Protopopov <a.s.protopopov@gmail.com>
To: Eduard Zingerman <eddyz87@gmail.com>
Cc: bpf@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>,
Andrii Nakryiko <andrii@kernel.org>,
Anton Protopopov <aspsk@isovalent.com>,
Daniel Borkmann <daniel@iogearbox.net>,
Quentin Monnet <qmo@kernel.org>,
Yonghong Song <yonghong.song@linux.dev>
Subject: Re: [PATCH v8 bpf-next 11/11] selftests/bpf: add C-level selftests for indirect jumps
Date: Thu, 30 Oct 2025 07:46:03 +0000 [thread overview]
Message-ID: <aQMXuySPuw1r7emc@mail.gmail.com> (raw)
In-Reply-To: <aa216ba69c31ae6cb253813379e3065ae5a850d6.camel@gmail.com>
On 25/10/29 02:49PM, Eduard Zingerman wrote:
> On Tue, 2025-10-28 at 14:20 +0000, Anton Protopopov wrote:
> > Add C-level selftests for indirect jumps to validate LLVM and libbpf
> > functionality. The tests are intentionally disabled, to be run
> > locally by developers, but will not make the CI red.
> >
> > Signed-off-by: Anton Protopopov <a.s.protopopov@gmail.com>
> > ---
>
> Yonghong added __BPF_FEATURE_GOTOX macro to llvm yesterday.
> I think it should be used instead of '#if 0' things.
> E.g. as in the attached diff (does not handle one-map-two-jumps and
> one-jump-two-maps).
Thanks! I will use some form of your patch. Somehow, having a flu
while sending the last versions prevented my brain from figuring
this out myself :(
> Still think that amount of tests added is a bit excessive,
> but defer to you and Yonghong to decide.
I would keep them, this file is more about testing llvm->libbpf.
(Though, I think, I need to convert most of them into `goto
*table[i]` form, and only leave a few big switches which
are guaranteed to be transformed to an indirect jump by LLVM.)
> Confirm that tests are passing when compiled with llvm
> commit b2fe5d1482eb ("[SimplifyCFG] Hoist common code when succ is unreachable block (#165570)").
>
> [...]
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c b/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c
> index bb0ebd16df43..252fb9019d70 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c
> @@ -15,7 +15,6 @@
>
> #include "bpf_gotox.skel.h"
>
> -#if 0
> static void __test_run(struct bpf_program *prog, void *ctx_in, size_t ctx_size_in)
> {
> LIBBPF_OPTS(bpf_test_run_opts, topts,
> @@ -29,6 +28,16 @@ static void __test_run(struct bpf_program *prog, void *ctx_in, size_t ctx_size_i
> ASSERT_OK(err, "test_run_opts err");
> }
>
> +static bool skip(struct bpf_gotox *skel)
> +{
> + if (skel->bss->skip) {
> + test__skip();
> + skel->bss->skip = 0;
> + return true;
> + }
> + return false;
> +}
> +
> static void check_simple(struct bpf_gotox *skel,
> struct bpf_program *prog,
> __u64 ctx_in,
> @@ -37,6 +46,8 @@ static void check_simple(struct bpf_gotox *skel,
> skel->bss->ret_user = 0;
>
> __test_run(prog, &ctx_in, sizeof(ctx_in));
> + if (skip(skel))
> + return;
>
> if (!ASSERT_EQ(skel->bss->ret_user, expected, "skel->bss->ret_user"))
> return;
> @@ -53,6 +64,8 @@ static void check_simple_fentry(struct bpf_gotox *skel,
> /* trigger */
> usleep(1);
>
> + if (skip(skel))
> + return;
> if (!ASSERT_EQ(skel->bss->ret_user, expected, "skel->bss->ret_user"))
> return;
> }
> @@ -215,7 +228,7 @@ static void check_nonstatic_global_other_sec(struct bpf_gotox *skel)
> check_simple_fentry(skel, skel->progs.use_nonstatic_global_other_sec, in[i], out[i]);
> }
>
> -static void __test_bpf_gotox(void)
> +void test_bpf_gotox(void)
> {
> struct bpf_gotox *skel;
> int ret;
> @@ -263,14 +276,3 @@ static void __test_bpf_gotox(void)
>
> bpf_gotox__destroy(skel);
> }
> -#else
> -static void __test_bpf_gotox(void)
> -{
> - test__skip();
> -}
> -#endif
> -
> -void test_bpf_gotox(void)
> -{
> - __test_bpf_gotox();
> -}
> diff --git a/tools/testing/selftests/bpf/progs/bpf_gotox.c b/tools/testing/selftests/bpf/progs/bpf_gotox.c
> index 16ad6cf279c0..2f704f260874 100644
> --- a/tools/testing/selftests/bpf/progs/bpf_gotox.c
> +++ b/tools/testing/selftests/bpf/progs/bpf_gotox.c
> @@ -6,9 +6,9 @@
> #include <bpf/bpf_core_read.h>
> #include "bpf_misc.h"
>
> -#if 0
> __u64 in_user;
> __u64 ret_user;
> +__u64 skip;
>
> struct simple_ctx {
> __u64 x;
> @@ -21,14 +21,17 @@ __u64 some_var;
> * number of instructions by the verifier. This adds additional
> * stress on testing the insn_array maps corresponding to indirect jumps.
> */
> +#ifdef __BPF_FEATURE_GOTOX
> static __always_inline void adjust_insns(__u64 x)
> {
> some_var ^= x + bpf_jiffies64();
> }
> +#endif
>
> SEC("syscall")
> int one_switch(struct simple_ctx *ctx)
> {
> +#ifdef __BPF_FEATURE_GOTOX
> switch (ctx->x) {
> case 0:
> adjust_insns(ctx->x + 1);
> @@ -55,13 +58,16 @@ int one_switch(struct simple_ctx *ctx)
> ret_user = 19;
> break;
> }
> -
> +#else
> + skip = 1;
> +#endif
> return 0;
> }
>
> SEC("syscall")
> int one_switch_non_zero_sec_off(struct simple_ctx *ctx)
> {
> +#ifdef __BPF_FEATURE_GOTOX
> switch (ctx->x) {
> case 0:
> adjust_insns(ctx->x + 1);
> @@ -90,11 +96,16 @@ int one_switch_non_zero_sec_off(struct simple_ctx *ctx)
> }
>
> return 0;
> +#else
> + skip = 1;
> + return 0;
> +#endif
> }
>
> SEC("fentry/" SYS_PREFIX "sys_nanosleep")
> int simple_test_other_sec(struct pt_regs *ctx)
> {
> +#ifdef __BPF_FEATURE_GOTOX
> __u64 x = in_user;
>
> switch (x) {
> @@ -125,11 +136,16 @@ int simple_test_other_sec(struct pt_regs *ctx)
> }
>
> return 0;
> +#else
> + skip = 1;
> + return 0;
> +#endif
> }
>
> SEC("syscall")
> int two_switches(struct simple_ctx *ctx)
> {
> +#ifdef __BPF_FEATURE_GOTOX
> switch (ctx->x) {
> case 0:
> adjust_insns(ctx->x + 1);
> @@ -185,11 +201,16 @@ int two_switches(struct simple_ctx *ctx)
> }
>
> return 0;
> +#else
> + skip = 1;
> + return 0;
> +#endif
> }
>
> SEC("syscall")
> int big_jump_table(struct simple_ctx *ctx __attribute__((unused)))
> {
> +#ifdef __BPF_FEATURE_GOTOX
> const void *const jt[256] = {
> [0 ... 255] = &&default_label,
> [0] = &&l0,
> @@ -223,12 +244,16 @@ int big_jump_table(struct simple_ctx *ctx __attribute__((unused)))
> default_label:
> adjust_insns(ctx->x + 177);
> ret_user = 19;
> +#else
> + skip = 1;
> +#endif
> return 0;
> }
>
> SEC("syscall")
> int one_jump_two_maps(struct simple_ctx *ctx __attribute__((unused)))
> {
> +#ifdef __BPF_FEATURE_GOTOX
> __label__ l1, l2, l3, l4;
> void *jt1[2] = { &&l1, &&l2 };
> void *jt2[2] = { &&l3, &&l4 };
> @@ -251,11 +276,16 @@ int one_jump_two_maps(struct simple_ctx *ctx __attribute__((unused)))
>
> ret_user = ret;
> return ret;
> +#else
> + skip = 1;
> + return 0;
> +#endif
> }
>
> SEC("syscall")
> int one_map_two_jumps(struct simple_ctx *ctx __attribute__((unused)))
> {
> +#ifdef __BPF_FEATURE_GOTOX
> __label__ l1, l2, l3;
> void *jt[3] = { &&l1, &&l2, &&l3 };
> unsigned int a = (ctx->x >> 2) & 1;
> @@ -274,9 +304,14 @@ int one_map_two_jumps(struct simple_ctx *ctx __attribute__((unused)))
>
> ret_user = ret;
> return ret;
> +#else
> + skip = 1;
> + return 0;
> +#endif
> }
>
> /* Just to introduce some non-zero offsets in .text */
> +#ifdef __BPF_FEATURE_GOTOX
> static __noinline int f0(volatile struct simple_ctx *ctx __arg_ctx)
> {
> if (ctx)
> @@ -284,13 +319,20 @@ static __noinline int f0(volatile struct simple_ctx *ctx __arg_ctx)
> else
> return 13;
> }
> +#endif
>
> SEC("syscall") int f1(struct simple_ctx *ctx)
> {
> +#ifdef __BPF_FEATURE_GOTOX
> ret_user = 0;
> return f0(ctx);
> +#else
> + skip = 1;
> + return 0;
> +#endif
> }
>
> +#ifdef __BPF_FEATURE_GOTOX
> static __noinline int __static_global(__u64 x)
> {
> switch (x) {
> @@ -322,30 +364,47 @@ static __noinline int __static_global(__u64 x)
>
> return 0;
> }
> +#endif
>
> SEC("syscall")
> int use_static_global1(struct simple_ctx *ctx)
> {
> +#ifdef __BPF_FEATURE_GOTOX
> ret_user = 0;
> return __static_global(ctx->x);
> +#else
> + skip = 1;
> + return 0;
> +#endif
> }
>
> SEC("syscall")
> int use_static_global2(struct simple_ctx *ctx)
> {
> +#ifdef __BPF_FEATURE_GOTOX
> ret_user = 0;
> adjust_insns(ctx->x + 1);
> return __static_global(ctx->x);
> +#else
> + skip = 1;
> + return 0;
> +#endif
> }
>
> SEC("fentry/" SYS_PREFIX "sys_nanosleep")
> int use_static_global_other_sec(void *ctx)
> {
> +#ifdef __BPF_FEATURE_GOTOX
> return __static_global(in_user);
> +#else
> + skip = 1;
> + return 0;
> +#endif
> }
>
> __noinline int __nonstatic_global(__u64 x)
> {
> +#ifdef __BPF_FEATURE_GOTOX
> switch (x) {
> case 0:
> adjust_insns(x + 1);
> @@ -374,28 +433,46 @@ __noinline int __nonstatic_global(__u64 x)
> }
>
> return 0;
> +#else
> + skip = 1;
> + return 0;
> +#endif
> }
>
> SEC("syscall")
> int use_nonstatic_global1(struct simple_ctx *ctx)
> {
> +#ifdef __BPF_FEATURE_GOTOX
> ret_user = 0;
> return __nonstatic_global(ctx->x);
> +#else
> + skip = 1;
> + return 0;
> +#endif
> }
>
> SEC("syscall")
> int use_nonstatic_global2(struct simple_ctx *ctx)
> {
> +#ifdef __BPF_FEATURE_GOTOX
> ret_user = 0;
> adjust_insns(ctx->x + 1);
> return __nonstatic_global(ctx->x);
> +#else
> + skip = 1;
> + return 0;
> +#endif
> }
>
> SEC("fentry/" SYS_PREFIX "sys_nanosleep")
> int use_nonstatic_global_other_sec(void *ctx)
> {
> +#ifdef __BPF_FEATURE_GOTOX
> return __nonstatic_global(in_user);
> -}
> +#else
> + skip = 1;
> + return 0;
> #endif
> +}
>
> char _license[] SEC("license") = "GPL";
prev parent reply other threads:[~2025-10-30 7:39 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-28 14:20 [PATCH v8 bpf-next 00/11] BPF indirect jumps Anton Protopopov
2025-10-28 14:20 ` [PATCH v8 bpf-next 01/11] bpf, x86: add new map type: instructions array Anton Protopopov
2025-10-30 22:50 ` Alexei Starovoitov
2025-10-31 7:23 ` Anton Protopopov
2025-10-31 15:04 ` Alexei Starovoitov
2025-10-31 15:20 ` Anton Protopopov
2025-10-28 14:20 ` [PATCH v8 bpf-next 02/11] selftests/bpf: add selftests for new insn_array map Anton Protopopov
2025-10-28 14:20 ` [PATCH v8 bpf-next 03/11] bpf: support instructions arrays with constants blinding Anton Protopopov
2025-10-28 14:20 ` [PATCH v8 bpf-next 04/11] selftests/bpf: test instructions arrays with blinding Anton Protopopov
2025-10-28 14:20 ` [PATCH v8 bpf-next 05/11] bpf, x86: allow indirect jumps to r8...r15 Anton Protopopov
2025-10-28 14:20 ` [PATCH v8 bpf-next 06/11] bpf, x86: add support for indirect jumps Anton Protopopov
2025-10-28 14:20 ` [PATCH v8 bpf-next 07/11] bpf: disasm: add support for BPF_JMP|BPF_JA|BPF_X Anton Protopopov
2025-10-28 14:20 ` [PATCH v8 bpf-next 08/11] libbpf: support llvm-generated indirect jumps Anton Protopopov
2025-10-29 21:31 ` Eduard Zingerman
2025-11-03 4:15 ` Yonghong Song
2025-10-30 21:00 ` Andrii Nakryiko
2025-10-31 7:30 ` Anton Protopopov
2025-10-28 14:20 ` [PATCH v8 bpf-next 09/11] bpftool: Recognize insn_array map type Anton Protopopov
2025-10-28 14:20 ` [PATCH v8 bpf-next 10/11] selftests/bpf: add new verifier_gotox test Anton Protopopov
2025-10-28 14:45 ` bot+bpf-ci
2025-10-28 15:05 ` Anton Protopopov
2025-10-28 14:20 ` [PATCH v8 bpf-next 11/11] selftests/bpf: add C-level selftests for indirect jumps Anton Protopopov
2025-10-29 21:49 ` Eduard Zingerman
2025-10-30 7:46 ` Anton Protopopov [this message]
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=aQMXuySPuw1r7emc@mail.gmail.com \
--to=a.s.protopopov@gmail.com \
--cc=andrii@kernel.org \
--cc=aspsk@isovalent.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=qmo@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