public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] selftests/bpf: move trampoline_count to dedicated bpf_testmod target
@ 2026-03-20  7:41 Sun Jian
  2026-03-20 14:01 ` Mykyta Yatsenko
  2026-03-23 11:17 ` Jiri Olsa
  0 siblings, 2 replies; 4+ messages in thread
From: Sun Jian @ 2026-03-20  7:41 UTC (permalink / raw)
  To: bpf
  Cc: ast, daniel, andrii, shuah, martin.lau, eddyz87, song,
	yonghong.song, john.fastabend, kpsingh, sdf, haoluo,
	paul.chaignon, jolsa, linux-kselftest, linux-kernel, Sun Jian

trampoline_count fills all trampoline attachment slots for a single
target function and verifies that one extra attach fails with -E2BIG.

It currently targets bpf_modify_return_test, which is also used by
other selftests such as modify_return, get_func_ip_test, and
get_func_args_test. When such tests run in parallel, they can contend
for the same per-function trampoline quota and cause unexpected attach
failures. This issue is currently masked by harness serialization.

Move trampoline_count to a dedicated bpf_testmod target and register it
for fmod_ret attachment. Also route the final trigger through
trigger_module_test_read, so the execution path exercises the  same
dedicated target.

This keeps the test semantics unchanged while isolating it from other
selftests, so it no longer needs to run in serial mode. Remove the
TODO comment as well.

Tested:
  ./test_progs -t trampoline_count -vv
  ./test_progs -t modify_return -vv
  ./test_progs -t get_func_ip_test -vv
  ./test_progs -t get_func_args_test -vv
  ./test_progs -j$(nproc) -t trampoline_count -vv
  ./test_progs -j$(nproc) -t
    trampoline_count,modify_return,get_func_ip_test,get_func_args_test,\
kprobe_multi_test -vv
  20 runs of:
    ./test_progs -j$(nproc) -t
    trampoline_count,modify_return,get_func_ip_test,get_func_args_test,\
kprobe_multi_test

Suggested-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
Link:https://lore.kernel.org/bpf/abicUI4YQ5KkQ_Ro@mail.gmail.com/T/#m6253b7fe96fe1a4df65b274c95aac786598a9857

v3:
- route the final trigger through trigger_module_test_read() and make
  bpf_testmod_test_read() call the dedicated trampoline_count target,
  as suggested by Jiri

v2:
- rewrite the subject to describe the change
- resend with the correct patch content

 .../bpf/prog_tests/trampoline_count.c         | 17 ++++----------
 .../bpf/progs/test_trampoline_count.c         |  6 ++---
 .../selftests/bpf/test_kmods/bpf_testmod.c    | 23 +++++++++++++++++++
 3 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
index 6cd7349d4a2b..dd2e5c84a4b5 100644
--- a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
+++ b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
@@ -30,16 +30,14 @@ static struct bpf_program *load_prog(char *file, char *name, struct inst *inst)
 	return prog;
 }
 
-/* TODO: use different target function to run in concurrent mode */
-void serial_test_trampoline_count(void)
+void test_trampoline_count(void)
 {
 	char *file = "test_trampoline_count.bpf.o";
 	char *const progs[] = { "fentry_test", "fmod_ret_test", "fexit_test" };
-	int bpf_max_tramp_links, err, i, prog_fd;
+	int bpf_max_tramp_links, i;
 	struct bpf_program *prog;
 	struct bpf_link *link;
 	struct inst *inst;
-	LIBBPF_OPTS(bpf_test_run_opts, opts);
 
 	bpf_max_tramp_links = get_bpf_max_tramp_links();
 	if (!ASSERT_GE(bpf_max_tramp_links, 1, "bpf_max_tramp_links"))
@@ -80,17 +78,10 @@ void serial_test_trampoline_count(void)
 		goto cleanup;
 
 	/* and finally execute the probe */
-	prog_fd = bpf_program__fd(prog);
-	if (!ASSERT_GE(prog_fd, 0, "bpf_program__fd"))
+	if (!ASSERT_OK(trigger_module_test_read(256),
+		       "trigger_module_test_read"))
 		goto cleanup;
 
-	err = bpf_prog_test_run_opts(prog_fd, &opts);
-	if (!ASSERT_OK(err, "bpf_prog_test_run_opts"))
-		goto cleanup;
-
-	ASSERT_EQ(opts.retval & 0xffff, 33, "bpf_modify_return_test.result");
-	ASSERT_EQ(opts.retval >> 16, 2, "bpf_modify_return_test.side_effect");
-
 cleanup:
 	for (; i >= 0; i--) {
 		bpf_link__destroy(inst[i].link);
diff --git a/tools/testing/selftests/bpf/progs/test_trampoline_count.c b/tools/testing/selftests/bpf/progs/test_trampoline_count.c
index 7765720da7d5..14ad2f53cf33 100644
--- a/tools/testing/selftests/bpf/progs/test_trampoline_count.c
+++ b/tools/testing/selftests/bpf/progs/test_trampoline_count.c
@@ -3,19 +3,19 @@
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_tracing.h>
 
-SEC("fentry/bpf_modify_return_test")
+SEC("fentry/bpf_testmod_trampoline_count_test")
 int BPF_PROG(fentry_test, int a, int *b)
 {
 	return 0;
 }
 
-SEC("fmod_ret/bpf_modify_return_test")
+SEC("fmod_ret/bpf_testmod_trampoline_count_test")
 int BPF_PROG(fmod_ret_test, int a, int *b, int ret)
 {
 	return 0;
 }
 
-SEC("fexit/bpf_modify_return_test")
+SEC("fexit/bpf_testmod_trampoline_count_test")
 int BPF_PROG(fexit_test, int a, int *b, int ret)
 {
 	return 0;
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
index e62c6b78657f..47583577e021 100644
--- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
@@ -470,6 +470,8 @@ noinline void bpf_testmod_stacktrace_test_1(void)
 
 int bpf_testmod_fentry_ok;
 
+noinline int bpf_testmod_trampoline_count_test(int a, int *b);
+
 noinline ssize_t
 bpf_testmod_test_read(struct file *file, struct kobject *kobj,
 		      const struct bin_attribute *bin_attr,
@@ -548,6 +550,10 @@ bpf_testmod_test_read(struct file *file, struct kobject *kobj,
 			21, 22, 23, 24, 25, 26) != 231)
 		goto out;
 
+	i = 2;
+	if (bpf_testmod_trampoline_count_test(1, &i) != 4 || i != 3)
+		goto out;
+
 	bpf_testmod_stacktrace_test_1();
 
 	bpf_testmod_fentry_ok = 1;
@@ -581,6 +587,12 @@ noinline int bpf_fentry_shadow_test(int a)
 }
 EXPORT_SYMBOL_GPL(bpf_fentry_shadow_test);
 
+noinline int bpf_testmod_trampoline_count_test(int a, int *b)
+{
+	*b += 1;
+	return a + *b;
+}
+
 __bpf_hook_end();
 
 static struct bin_attribute bin_attr_bpf_testmod_file __ro_after_init = {
@@ -1843,6 +1855,16 @@ struct bpf_struct_ops testmod_multi_st_ops = {
 
 extern int bpf_fentry_test1(int a);
 
+BTF_KFUNCS_START(bpf_testmod_trampoline_count_ids)
+BTF_ID_FLAGS(func, bpf_testmod_trampoline_count_test)
+BTF_KFUNCS_END(bpf_testmod_trampoline_count_ids)
+
+static const struct
+btf_kfunc_id_set bpf_testmod_trampoline_count_fmodret_set = {
+	.owner = THIS_MODULE,
+	.set = &bpf_testmod_trampoline_count_ids,
+};
+
 static int bpf_testmod_init(void)
 {
 	const struct btf_id_dtor_kfunc bpf_testmod_dtors[] = {
@@ -1859,6 +1881,7 @@ static int bpf_testmod_init(void)
 	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &bpf_testmod_kfunc_set);
 	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &bpf_testmod_kfunc_set);
 	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS, &bpf_testmod_kfunc_set);
+	ret = ret ?: register_btf_fmodret_id_set(&bpf_testmod_trampoline_count_fmodret_set);
 	ret = ret ?: register_bpf_struct_ops(&bpf_bpf_testmod_ops, bpf_testmod_ops);
 	ret = ret ?: register_bpf_struct_ops(&bpf_testmod_ops2, bpf_testmod_ops2);
 	ret = ret ?: register_bpf_struct_ops(&bpf_testmod_ops3, bpf_testmod_ops3);

base-commit: 8a30aeb0d1b4e4aaf7f7bae72f20f2ae75385ccb
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] selftests/bpf: move trampoline_count to dedicated bpf_testmod target
  2026-03-20  7:41 [PATCH v3] selftests/bpf: move trampoline_count to dedicated bpf_testmod target Sun Jian
@ 2026-03-20 14:01 ` Mykyta Yatsenko
  2026-03-23  8:08   ` sun jian
  2026-03-23 11:17 ` Jiri Olsa
  1 sibling, 1 reply; 4+ messages in thread
From: Mykyta Yatsenko @ 2026-03-20 14:01 UTC (permalink / raw)
  To: Sun Jian, bpf
  Cc: ast, daniel, andrii, shuah, martin.lau, eddyz87, song,
	yonghong.song, john.fastabend, kpsingh, sdf, haoluo,
	paul.chaignon, jolsa, linux-kselftest, linux-kernel, Sun Jian

Sun Jian <sun.jian.kdev@gmail.com> writes:

> trampoline_count fills all trampoline attachment slots for a single
> target function and verifies that one extra attach fails with -E2BIG.
>
> It currently targets bpf_modify_return_test, which is also used by
> other selftests such as modify_return, get_func_ip_test, and
> get_func_args_test. When such tests run in parallel, they can contend
> for the same per-function trampoline quota and cause unexpected attach
> failures. This issue is currently masked by harness serialization.
>
> Move trampoline_count to a dedicated bpf_testmod target and register it
> for fmod_ret attachment. Also route the final trigger through
> trigger_module_test_read, so the execution path exercises the  same
> dedicated target.
>
> This keeps the test semantics unchanged while isolating it from other
> selftests, so it no longer needs to run in serial mode. Remove the
> TODO comment as well.
Is it really worth it just for ditching serial mode, does it improve
test_progs speed? Either way it's worth adding a comment for
bpf_testmod_trampoline_count_test() explaining that no other test should
attempt attaching to it.
>
> Tested:
>   ./test_progs -t trampoline_count -vv
>   ./test_progs -t modify_return -vv
>   ./test_progs -t get_func_ip_test -vv
>   ./test_progs -t get_func_args_test -vv
>   ./test_progs -j$(nproc) -t trampoline_count -vv
>   ./test_progs -j$(nproc) -t
>     trampoline_count,modify_return,get_func_ip_test,get_func_args_test,\
> kprobe_multi_test -vv
>   20 runs of:
>     ./test_progs -j$(nproc) -t
>     trampoline_count,modify_return,get_func_ip_test,get_func_args_test,\
> kprobe_multi_test
>
> Suggested-by: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> ---
> Link:https://lore.kernel.org/bpf/abicUI4YQ5KkQ_Ro@mail.gmail.com/T/#m6253b7fe96fe1a4df65b274c95aac786598a9857
>
> v3:
> - route the final trigger through trigger_module_test_read() and make
>   bpf_testmod_test_read() call the dedicated trampoline_count target,
>   as suggested by Jiri
>
> v2:
> - rewrite the subject to describe the change
> - resend with the correct patch content
>
>  .../bpf/prog_tests/trampoline_count.c         | 17 ++++----------
>  .../bpf/progs/test_trampoline_count.c         |  6 ++---
>  .../selftests/bpf/test_kmods/bpf_testmod.c    | 23 +++++++++++++++++++
>  3 files changed, 30 insertions(+), 16 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> index 6cd7349d4a2b..dd2e5c84a4b5 100644
> --- a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> +++ b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> @@ -30,16 +30,14 @@ static struct bpf_program *load_prog(char *file, char *name, struct inst *inst)
>  	return prog;
>  }
>  
> -/* TODO: use different target function to run in concurrent mode */
> -void serial_test_trampoline_count(void)
> +void test_trampoline_count(void)
>  {
>  	char *file = "test_trampoline_count.bpf.o";
>  	char *const progs[] = { "fentry_test", "fmod_ret_test", "fexit_test" };
> -	int bpf_max_tramp_links, err, i, prog_fd;
> +	int bpf_max_tramp_links, i;
>  	struct bpf_program *prog;
>  	struct bpf_link *link;
>  	struct inst *inst;
> -	LIBBPF_OPTS(bpf_test_run_opts, opts);
>  
>  	bpf_max_tramp_links = get_bpf_max_tramp_links();
>  	if (!ASSERT_GE(bpf_max_tramp_links, 1, "bpf_max_tramp_links"))
> @@ -80,17 +78,10 @@ void serial_test_trampoline_count(void)
>  		goto cleanup;
>  
>  	/* and finally execute the probe */
> -	prog_fd = bpf_program__fd(prog);
> -	if (!ASSERT_GE(prog_fd, 0, "bpf_program__fd"))
> +	if (!ASSERT_OK(trigger_module_test_read(256),
> +		       "trigger_module_test_read"))
>  		goto cleanup;
>  
> -	err = bpf_prog_test_run_opts(prog_fd, &opts);
> -	if (!ASSERT_OK(err, "bpf_prog_test_run_opts"))
> -		goto cleanup;
> -
> -	ASSERT_EQ(opts.retval & 0xffff, 33, "bpf_modify_return_test.result");
> -	ASSERT_EQ(opts.retval >> 16, 2, "bpf_modify_return_test.side_effect");
> -
>  cleanup:
>  	for (; i >= 0; i--) {
>  		bpf_link__destroy(inst[i].link);
> diff --git a/tools/testing/selftests/bpf/progs/test_trampoline_count.c b/tools/testing/selftests/bpf/progs/test_trampoline_count.c
> index 7765720da7d5..14ad2f53cf33 100644
> --- a/tools/testing/selftests/bpf/progs/test_trampoline_count.c
> +++ b/tools/testing/selftests/bpf/progs/test_trampoline_count.c
> @@ -3,19 +3,19 @@
>  #include <bpf/bpf_helpers.h>
>  #include <bpf/bpf_tracing.h>
>  
> -SEC("fentry/bpf_modify_return_test")
> +SEC("fentry/bpf_testmod_trampoline_count_test")
>  int BPF_PROG(fentry_test, int a, int *b)
>  {
>  	return 0;
>  }
>  
> -SEC("fmod_ret/bpf_modify_return_test")
> +SEC("fmod_ret/bpf_testmod_trampoline_count_test")
>  int BPF_PROG(fmod_ret_test, int a, int *b, int ret)
>  {
>  	return 0;
>  }
>  
> -SEC("fexit/bpf_modify_return_test")
> +SEC("fexit/bpf_testmod_trampoline_count_test")
>  int BPF_PROG(fexit_test, int a, int *b, int ret)
>  {
>  	return 0;
> diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> index e62c6b78657f..47583577e021 100644
> --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> @@ -470,6 +470,8 @@ noinline void bpf_testmod_stacktrace_test_1(void)
>  
>  int bpf_testmod_fentry_ok;
>  
> +noinline int bpf_testmod_trampoline_count_test(int a, int *b);
> +
>  noinline ssize_t
>  bpf_testmod_test_read(struct file *file, struct kobject *kobj,
>  		      const struct bin_attribute *bin_attr,
> @@ -548,6 +550,10 @@ bpf_testmod_test_read(struct file *file, struct kobject *kobj,
>  			21, 22, 23, 24, 25, 26) != 231)
>  		goto out;
>  
> +	i = 2;
> +	if (bpf_testmod_trampoline_count_test(1, &i) != 4 || i != 3)
> +		goto out;
> +
Do we need to add this test for the bpf_testmod_trampoline_count_test(),
if it's supposed to be used only for the trampoline count test, which
probably does not care what function actually does internally.
>  	bpf_testmod_stacktrace_test_1();
>  
>  	bpf_testmod_fentry_ok = 1;
> @@ -581,6 +587,12 @@ noinline int bpf_fentry_shadow_test(int a)
>  }
>  EXPORT_SYMBOL_GPL(bpf_fentry_shadow_test);
>  
> +noinline int bpf_testmod_trampoline_count_test(int a, int *b)
> +{
> +	*b += 1;
> +	return a + *b;
> +}
> +
>  __bpf_hook_end();
>  
>  static struct bin_attribute bin_attr_bpf_testmod_file __ro_after_init = {
> @@ -1843,6 +1855,16 @@ struct bpf_struct_ops testmod_multi_st_ops = {
>  
>  extern int bpf_fentry_test1(int a);
>  
> +BTF_KFUNCS_START(bpf_testmod_trampoline_count_ids)
> +BTF_ID_FLAGS(func, bpf_testmod_trampoline_count_test)
> +BTF_KFUNCS_END(bpf_testmod_trampoline_count_ids)
> +
> +static const struct
> +btf_kfunc_id_set bpf_testmod_trampoline_count_fmodret_set = {
> +	.owner = THIS_MODULE,
> +	.set = &bpf_testmod_trampoline_count_ids,
> +};
> +
>  static int bpf_testmod_init(void)
>  {
>  	const struct btf_id_dtor_kfunc bpf_testmod_dtors[] = {
> @@ -1859,6 +1881,7 @@ static int bpf_testmod_init(void)
>  	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &bpf_testmod_kfunc_set);
>  	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &bpf_testmod_kfunc_set);
>  	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS, &bpf_testmod_kfunc_set);
> +	ret = ret ?: register_btf_fmodret_id_set(&bpf_testmod_trampoline_count_fmodret_set);
>  	ret = ret ?: register_bpf_struct_ops(&bpf_bpf_testmod_ops, bpf_testmod_ops);
>  	ret = ret ?: register_bpf_struct_ops(&bpf_testmod_ops2, bpf_testmod_ops2);
>  	ret = ret ?: register_bpf_struct_ops(&bpf_testmod_ops3, bpf_testmod_ops3);
>
> base-commit: 8a30aeb0d1b4e4aaf7f7bae72f20f2ae75385ccb
> -- 
> 2.43.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] selftests/bpf: move trampoline_count to dedicated bpf_testmod target
  2026-03-20 14:01 ` Mykyta Yatsenko
@ 2026-03-23  8:08   ` sun jian
  0 siblings, 0 replies; 4+ messages in thread
From: sun jian @ 2026-03-23  8:08 UTC (permalink / raw)
  To: Mykyta Yatsenko
  Cc: bpf, ast, daniel, andrii, shuah, martin.lau, eddyz87, song,
	yonghong.song, john.fastabend, kpsingh, sdf, haoluo,
	paul.chaignon, jolsa, linux-kselftest, linux-kernel

On Fri, Mar 20, 2026 at 10:01 PM Mykyta Yatsenko
<mykyta.yatsenko5@gmail.com> wrote:
>
> Sun Jian <sun.jian.kdev@gmail.com> writes:
>
> > trampoline_count fills all trampoline attachment slots for a single
> > target function and verifies that one extra attach fails with -E2BIG.
> >
> > It currently targets bpf_modify_return_test, which is also used by
> > other selftests such as modify_return, get_func_ip_test, and
> > get_func_args_test. When such tests run in parallel, they can contend
> > for the same per-function trampoline quota and cause unexpected attach
> > failures. This issue is currently masked by harness serialization.
> >
> > Move trampoline_count to a dedicated bpf_testmod target and register it
> > for fmod_ret attachment. Also route the final trigger through
> > trigger_module_test_read, so the execution path exercises the  same
> > dedicated target.
> >
> > This keeps the test semantics unchanged while isolating it from other
> > selftests, so it no longer needs to run in serial mode. Remove the
> > TODO comment as well.
> Is it really worth it just for ditching serial mode, does it improve
> test_progs speed? Either way it's worth adding a comment for
> bpf_testmod_trampoline_count_test() explaining that no other test should
> attempt attaching to it.
The main point here is not to speed up test_progs. The trampoline
count limit is a
 per-func, system-wide quota. The purpose of this change is to avoid
contention with
other selftests attaching to the same target function. Allowing this
test to run in parallel
is just a consequence, not the goal, as the TODO comment already indicated.
Adding a short comment is not a bad idea, but
bpf_testmod_trampoline_count_test()
is already fairly self-descriptive.
> >
> > Tested:
> >   ./test_progs -t trampoline_count -vv
> >   ./test_progs -t modify_return -vv
> >   ./test_progs -t get_func_ip_test -vv
> >   ./test_progs -t get_func_args_test -vv
> >   ./test_progs -j$(nproc) -t trampoline_count -vv
> >   ./test_progs -j$(nproc) -t
> >     trampoline_count,modify_return,get_func_ip_test,get_func_args_test,\
> > kprobe_multi_test -vv
> >   20 runs of:
> >     ./test_progs -j$(nproc) -t
> >     trampoline_count,modify_return,get_func_ip_test,get_func_args_test,\
> > kprobe_multi_test
> >
> > Suggested-by: Jiri Olsa <jolsa@kernel.org>
> > Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> > ---
> > Link:https://lore.kernel.org/bpf/abicUI4YQ5KkQ_Ro@mail.gmail.com/T/#m6253b7fe96fe1a4df65b274c95aac786598a9857
> >
> > v3:
> > - route the final trigger through trigger_module_test_read() and make
> >   bpf_testmod_test_read() call the dedicated trampoline_count target,
> >   as suggested by Jiri
> >
> > v2:
> > - rewrite the subject to describe the change
> > - resend with the correct patch content
> >
> >  .../bpf/prog_tests/trampoline_count.c         | 17 ++++----------
> >  .../bpf/progs/test_trampoline_count.c         |  6 ++---
> >  .../selftests/bpf/test_kmods/bpf_testmod.c    | 23 +++++++++++++++++++
> >  3 files changed, 30 insertions(+), 16 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> > index 6cd7349d4a2b..dd2e5c84a4b5 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> > @@ -30,16 +30,14 @@ static struct bpf_program *load_prog(char *file, char *name, struct inst *inst)
> >       return prog;
> >  }
> >
> > -/* TODO: use different target function to run in concurrent mode */
> > -void serial_test_trampoline_count(void)
> > +void test_trampoline_count(void)
> >  {
> >       char *file = "test_trampoline_count.bpf.o";
> >       char *const progs[] = { "fentry_test", "fmod_ret_test", "fexit_test" };
> > -     int bpf_max_tramp_links, err, i, prog_fd;
> > +     int bpf_max_tramp_links, i;
> >       struct bpf_program *prog;
> >       struct bpf_link *link;
> >       struct inst *inst;
> > -     LIBBPF_OPTS(bpf_test_run_opts, opts);
> >
> >       bpf_max_tramp_links = get_bpf_max_tramp_links();
> >       if (!ASSERT_GE(bpf_max_tramp_links, 1, "bpf_max_tramp_links"))
> > @@ -80,17 +78,10 @@ void serial_test_trampoline_count(void)
> >               goto cleanup;
> >
> >       /* and finally execute the probe */
> > -     prog_fd = bpf_program__fd(prog);
> > -     if (!ASSERT_GE(prog_fd, 0, "bpf_program__fd"))
> > +     if (!ASSERT_OK(trigger_module_test_read(256),
> > +                    "trigger_module_test_read"))
> >               goto cleanup;
> >
> > -     err = bpf_prog_test_run_opts(prog_fd, &opts);
> > -     if (!ASSERT_OK(err, "bpf_prog_test_run_opts"))
> > -             goto cleanup;
> > -
> > -     ASSERT_EQ(opts.retval & 0xffff, 33, "bpf_modify_return_test.result");
> > -     ASSERT_EQ(opts.retval >> 16, 2, "bpf_modify_return_test.side_effect");
> > -
> >  cleanup:
> >       for (; i >= 0; i--) {
> >               bpf_link__destroy(inst[i].link);
> > diff --git a/tools/testing/selftests/bpf/progs/test_trampoline_count.c b/tools/testing/selftests/bpf/progs/test_trampoline_count.c
> > index 7765720da7d5..14ad2f53cf33 100644
> > --- a/tools/testing/selftests/bpf/progs/test_trampoline_count.c
> > +++ b/tools/testing/selftests/bpf/progs/test_trampoline_count.c
> > @@ -3,19 +3,19 @@
> >  #include <bpf/bpf_helpers.h>
> >  #include <bpf/bpf_tracing.h>
> >
> > -SEC("fentry/bpf_modify_return_test")
> > +SEC("fentry/bpf_testmod_trampoline_count_test")
> >  int BPF_PROG(fentry_test, int a, int *b)
> >  {
> >       return 0;
> >  }
> >
> > -SEC("fmod_ret/bpf_modify_return_test")
> > +SEC("fmod_ret/bpf_testmod_trampoline_count_test")
> >  int BPF_PROG(fmod_ret_test, int a, int *b, int ret)
> >  {
> >       return 0;
> >  }
> >
> > -SEC("fexit/bpf_modify_return_test")
> > +SEC("fexit/bpf_testmod_trampoline_count_test")
> >  int BPF_PROG(fexit_test, int a, int *b, int ret)
> >  {
> >       return 0;
> > diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> > index e62c6b78657f..47583577e021 100644
> > --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> > +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> > @@ -470,6 +470,8 @@ noinline void bpf_testmod_stacktrace_test_1(void)
> >
> >  int bpf_testmod_fentry_ok;
> >
> > +noinline int bpf_testmod_trampoline_count_test(int a, int *b);
> > +
> >  noinline ssize_t
> >  bpf_testmod_test_read(struct file *file, struct kobject *kobj,
> >                     const struct bin_attribute *bin_attr,
> > @@ -548,6 +550,10 @@ bpf_testmod_test_read(struct file *file, struct kobject *kobj,
> >                       21, 22, 23, 24, 25, 26) != 231)
> >               goto out;
> >
> > +     i = 2;
> > +     if (bpf_testmod_trampoline_count_test(1, &i) != 4 || i != 3)
> > +             goto out;
> > +
> Do we need to add this test for the bpf_testmod_trampoline_count_test(),
> if it's supposed to be used only for the trampoline count test, which
> probably does not care what function actually does internally.
This part was only added to keep the execution path conceptually aligned
with the dedicated target change. As for this and the above comment suggestion,
 I'm fine leaving both to maintainer preference.

Sun Jian
> >       bpf_testmod_stacktrace_test_1();
> >
> >       bpf_testmod_fentry_ok = 1;
> > @@ -581,6 +587,12 @@ noinline int bpf_fentry_shadow_test(int a)
> >  }
> >  EXPORT_SYMBOL_GPL(bpf_fentry_shadow_test);
> >
> > +noinline int bpf_testmod_trampoline_count_test(int a, int *b)
> > +{
> > +     *b += 1;
> > +     return a + *b;
> > +}
> > +
> >  __bpf_hook_end();
> >
> >  static struct bin_attribute bin_attr_bpf_testmod_file __ro_after_init = {
> > @@ -1843,6 +1855,16 @@ struct bpf_struct_ops testmod_multi_st_ops = {
> >
> >  extern int bpf_fentry_test1(int a);
> >
> > +BTF_KFUNCS_START(bpf_testmod_trampoline_count_ids)
> > +BTF_ID_FLAGS(func, bpf_testmod_trampoline_count_test)
> > +BTF_KFUNCS_END(bpf_testmod_trampoline_count_ids)
> > +
> > +static const struct
> > +btf_kfunc_id_set bpf_testmod_trampoline_count_fmodret_set = {
> > +     .owner = THIS_MODULE,
> > +     .set = &bpf_testmod_trampoline_count_ids,
> > +};
> > +
> >  static int bpf_testmod_init(void)
> >  {
> >       const struct btf_id_dtor_kfunc bpf_testmod_dtors[] = {
> > @@ -1859,6 +1881,7 @@ static int bpf_testmod_init(void)
> >       ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &bpf_testmod_kfunc_set);
> >       ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &bpf_testmod_kfunc_set);
> >       ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS, &bpf_testmod_kfunc_set);
> > +     ret = ret ?: register_btf_fmodret_id_set(&bpf_testmod_trampoline_count_fmodret_set);
> >       ret = ret ?: register_bpf_struct_ops(&bpf_bpf_testmod_ops, bpf_testmod_ops);
> >       ret = ret ?: register_bpf_struct_ops(&bpf_testmod_ops2, bpf_testmod_ops2);
> >       ret = ret ?: register_bpf_struct_ops(&bpf_testmod_ops3, bpf_testmod_ops3);
> >
> > base-commit: 8a30aeb0d1b4e4aaf7f7bae72f20f2ae75385ccb
> > --
> > 2.43.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] selftests/bpf: move trampoline_count to dedicated bpf_testmod target
  2026-03-20  7:41 [PATCH v3] selftests/bpf: move trampoline_count to dedicated bpf_testmod target Sun Jian
  2026-03-20 14:01 ` Mykyta Yatsenko
@ 2026-03-23 11:17 ` Jiri Olsa
  1 sibling, 0 replies; 4+ messages in thread
From: Jiri Olsa @ 2026-03-23 11:17 UTC (permalink / raw)
  To: Sun Jian
  Cc: bpf, ast, daniel, andrii, shuah, martin.lau, eddyz87, song,
	yonghong.song, john.fastabend, kpsingh, sdf, haoluo,
	paul.chaignon, linux-kselftest, linux-kernel

On Fri, Mar 20, 2026 at 03:41:50PM +0800, Sun Jian wrote:
> trampoline_count fills all trampoline attachment slots for a single
> target function and verifies that one extra attach fails with -E2BIG.
> 
> It currently targets bpf_modify_return_test, which is also used by
> other selftests such as modify_return, get_func_ip_test, and
> get_func_args_test. When such tests run in parallel, they can contend
> for the same per-function trampoline quota and cause unexpected attach
> failures. This issue is currently masked by harness serialization.
> 
> Move trampoline_count to a dedicated bpf_testmod target and register it
> for fmod_ret attachment. Also route the final trigger through
> trigger_module_test_read, so the execution path exercises the  same
> dedicated target.
> 
> This keeps the test semantics unchanged while isolating it from other
> selftests, so it no longer needs to run in serial mode. Remove the
> TODO comment as well.
> 
> Tested:
>   ./test_progs -t trampoline_count -vv
>   ./test_progs -t modify_return -vv
>   ./test_progs -t get_func_ip_test -vv
>   ./test_progs -t get_func_args_test -vv
>   ./test_progs -j$(nproc) -t trampoline_count -vv
>   ./test_progs -j$(nproc) -t
>     trampoline_count,modify_return,get_func_ip_test,get_func_args_test,\
> kprobe_multi_test -vv
>   20 runs of:
>     ./test_progs -j$(nproc) -t
>     trampoline_count,modify_return,get_func_ip_test,get_func_args_test,\
> kprobe_multi_test
> 
> Suggested-by: Jiri Olsa <jolsa@kernel.org>

I only suggested change as part of review, not the change itself ;-)
you can drop the tag

> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> ---
> Link:https://lore.kernel.org/bpf/abicUI4YQ5KkQ_Ro@mail.gmail.com/T/#m6253b7fe96fe1a4df65b274c95aac786598a9857
> 
> v3:
> - route the final trigger through trigger_module_test_read() and make
>   bpf_testmod_test_read() call the dedicated trampoline_count target,
>   as suggested by Jiri
> 
> v2:
> - rewrite the subject to describe the change
> - resend with the correct patch content
> 
>  .../bpf/prog_tests/trampoline_count.c         | 17 ++++----------
>  .../bpf/progs/test_trampoline_count.c         |  6 ++---
>  .../selftests/bpf/test_kmods/bpf_testmod.c    | 23 +++++++++++++++++++
>  3 files changed, 30 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> index 6cd7349d4a2b..dd2e5c84a4b5 100644
> --- a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> +++ b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> @@ -30,16 +30,14 @@ static struct bpf_program *load_prog(char *file, char *name, struct inst *inst)
>  	return prog;
>  }
>  
> -/* TODO: use different target function to run in concurrent mode */
> -void serial_test_trampoline_count(void)
> +void test_trampoline_count(void)
>  {
>  	char *file = "test_trampoline_count.bpf.o";
>  	char *const progs[] = { "fentry_test", "fmod_ret_test", "fexit_test" };
> -	int bpf_max_tramp_links, err, i, prog_fd;
> +	int bpf_max_tramp_links, i;
>  	struct bpf_program *prog;
>  	struct bpf_link *link;
>  	struct inst *inst;
> -	LIBBPF_OPTS(bpf_test_run_opts, opts);
>  
>  	bpf_max_tramp_links = get_bpf_max_tramp_links();
>  	if (!ASSERT_GE(bpf_max_tramp_links, 1, "bpf_max_tramp_links"))
> @@ -80,17 +78,10 @@ void serial_test_trampoline_count(void)
>  		goto cleanup;
>  
>  	/* and finally execute the probe */
> -	prog_fd = bpf_program__fd(prog);
> -	if (!ASSERT_GE(prog_fd, 0, "bpf_program__fd"))
> +	if (!ASSERT_OK(trigger_module_test_read(256),
> +		       "trigger_module_test_read"))

could be just single line, also no need for the condition and
goto cleanup, just this will do:

     ASSERT_OK(trigger_module_test_read....)

>  		goto cleanup;
>  
> -	err = bpf_prog_test_run_opts(prog_fd, &opts);
> -	if (!ASSERT_OK(err, "bpf_prog_test_run_opts"))
> -		goto cleanup;
> -
> -	ASSERT_EQ(opts.retval & 0xffff, 33, "bpf_modify_return_test.result");
> -	ASSERT_EQ(opts.retval >> 16, 2, "bpf_modify_return_test.side_effect");
> -
>  cleanup:
>  	for (; i >= 0; i--) {
>  		bpf_link__destroy(inst[i].link);
> diff --git a/tools/testing/selftests/bpf/progs/test_trampoline_count.c b/tools/testing/selftests/bpf/progs/test_trampoline_count.c
> index 7765720da7d5..14ad2f53cf33 100644
> --- a/tools/testing/selftests/bpf/progs/test_trampoline_count.c
> +++ b/tools/testing/selftests/bpf/progs/test_trampoline_count.c
> @@ -3,19 +3,19 @@
>  #include <bpf/bpf_helpers.h>
>  #include <bpf/bpf_tracing.h>
>  
> -SEC("fentry/bpf_modify_return_test")
> +SEC("fentry/bpf_testmod_trampoline_count_test")
>  int BPF_PROG(fentry_test, int a, int *b)
>  {
>  	return 0;
>  }
>  
> -SEC("fmod_ret/bpf_modify_return_test")
> +SEC("fmod_ret/bpf_testmod_trampoline_count_test")
>  int BPF_PROG(fmod_ret_test, int a, int *b, int ret)
>  {
>  	return 0;
>  }
>  
> -SEC("fexit/bpf_modify_return_test")
> +SEC("fexit/bpf_testmod_trampoline_count_test")
>  int BPF_PROG(fexit_test, int a, int *b, int ret)
>  {
>  	return 0;
> diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> index e62c6b78657f..47583577e021 100644
> --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> @@ -470,6 +470,8 @@ noinline void bpf_testmod_stacktrace_test_1(void)
>  
>  int bpf_testmod_fentry_ok;
>  
> +noinline int bpf_testmod_trampoline_count_test(int a, int *b);

we could define bpf_testmod_trampoline_count_test in here,
so we would go without the declaration

> +
>  noinline ssize_t
>  bpf_testmod_test_read(struct file *file, struct kobject *kobj,
>  		      const struct bin_attribute *bin_attr,
> @@ -548,6 +550,10 @@ bpf_testmod_test_read(struct file *file, struct kobject *kobj,
>  			21, 22, 23, 24, 25, 26) != 231)
>  		goto out;
>  
> +	i = 2;
> +	if (bpf_testmod_trampoline_count_test(1, &i) != 4 || i != 3)
> +		goto out;

do we need all those arguments? progs don't do anything with them..
I'd stick with just simple module function used by trampoline_count test

jirka

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-03-23 11:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-20  7:41 [PATCH v3] selftests/bpf: move trampoline_count to dedicated bpf_testmod target Sun Jian
2026-03-20 14:01 ` Mykyta Yatsenko
2026-03-23  8:08   ` sun jian
2026-03-23 11:17 ` Jiri Olsa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox