Re: [PATCH v8 bpf-next 11/11] selftests/bpf: add C-level selftests for indirect jumps

[Eduard Zingerman <eddyz87@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 804 bytes --]

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).

Still think that amount of tests added is a bit excessive,
but defer to you and Yonghong to decide.

Confirm that tests are passing when compiled with llvm
commit b2fe5d1482eb ("[SimplifyCFG] Hoist common code when succ is unreachable block (#165570)").

[...]

[-- Attachment #2: skip.patch --]
[-- Type: text/x-patch, Size: 7019 bytes --]

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";