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

[Anton Protopopov <a.s.protopopov@gmail.com>]

On 25/11/03 12:45PM, Eduard Zingerman wrote:
> On Sun, 2025-11-02 at 20:57 +0000, Anton Protopopov wrote:
> 
> [...]
> 
> > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c b/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c
> > new file mode 100644
> > index 000000000000..2a55fa91e1fa
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_gotox.c
> > @@ -0,0 +1,272 @@
> 
> [...]
> 
> > +static void check_simple_fentry(struct bpf_gotox *skel,
> > +				struct bpf_program *prog,
> > +				__u64 ctx_in,
> > +				__u64 expected)
> > +{
> > +	skel->bss->in_user = ctx_in;
> > +	skel->bss->ret_user = 0;
> > +
> > +	/* trigger */
> > +	usleep(1);
> > +
> > +	if (!ASSERT_EQ(skel->bss->ret_user, expected, "skel->bss->ret_user"))
> > +		return;
> > +}
> 
> [...]
> 
> > +static void check_other_sec(struct bpf_gotox *skel)
> > +{
> > +	__u64 in[]   = {0, 1, 2, 3, 4,  5, 77};
> > +	__u64 out[]  = {2, 3, 4, 5, 7, 19, 19};
> > +	int i;
> > +
> > +	bpf_program__attach(skel->progs.simple_test_other_sec);
> > +	for (i = 0; i < ARRAY_SIZE(in); i++)
> > +		check_simple_fentry(skel, skel->progs.simple_test_other_sec, in[i], out[i]);
> > +}
> 
> The above means that fentry programs are accumulated for 'sys_nanosleep', right? 
> In all 3 test cases that use check_simple_fentry() the identical 'out'
> values are used.  Should the programs be detached here to avoid
> possible masking of a misbehaving program?

yes, thanks, will detach it

> 
> [...]
> 
> > diff --git a/tools/testing/selftests/bpf/progs/bpf_gotox.c b/tools/testing/selftests/bpf/progs/bpf_gotox.c
> > new file mode 100644
> > index 000000000000..8a84f4b225b2
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/bpf_gotox.c
> 
> +SEC("fentry/" SYS_PREFIX "sys_nanosleep")
> +int use_nonstatic_global_other_sec(void *ctx)
> +{
> +	return __nonstatic_global(in_user);
> +}
> 
> Should this check for target process pid?

And here will add a fix as well. Thanks

> [...]
> 
> > +#define SKIP_TEST(TEST_NAME)				\
> > +	SEC("syscall") int TEST_NAME(void *ctx)		\
> > +	{						\
> > +		return 0;				\
> > +	}
> > +
> > +SKIP_TEST(one_switch);
> > +SKIP_TEST(one_switch_non_zero_sec_off);
> > +SKIP_TEST(simple_test_other_sec);
> > +SKIP_TEST(two_switches);
> > +SKIP_TEST(big_jump_table);
> > +SKIP_TEST(one_jump_two_maps);
> > +SKIP_TEST(one_map_two_jumps);
> > +SKIP_TEST(use_static_global1);
> > +SKIP_TEST(use_static_global2);
> > +SKIP_TEST(use_static_global_other_sec);
> > +SKIP_TEST(use_nonstatic_global1);
> > +SKIP_TEST(use_nonstatic_global2);
> > +SKIP_TEST(use_nonstatic_global_other_sec);
> 
> Nice.
> I double checked and tests are skipped when old clang is used and pass
> when new clang is used.
> 
> > +#endif /* __BPF_FEATURE_GOTOX */
> > +
> > +char _license[] SEC("license") = "GPL";