All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <olsajiri@gmail.com>
To: Yonghong Song <yonghong.song@linux.dev>
Cc: bpf@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	kernel-team@fb.com, Martin KaFai Lau <martin.lau@kernel.org>
Subject: Re: [PATCH bpf-next v2 1/3] selftests/bpf: Refactor the failed assertion to another subtest
Date: Mon, 16 Jun 2025 10:33:25 +0200	[thread overview]
Message-ID: <aE_W1ZoK6BZ6_EGA@krava> (raw)
In-Reply-To: <20250615185351.2757391-1-yonghong.song@linux.dev>

On Sun, Jun 15, 2025 at 11:53:51AM -0700, Yonghong Song wrote:

SNIP

> 
> There are total 301 locations for usdt_300. For gcc11 built binary, there are
> 300 spec's. But for clang20 built binary, there are 3 spec's. The libbpf default
> BPF_USDT_MAX_SPEC_CNT is 256. So for gcc11, the above bpf_program__attach_usdt() will
> fail, but the function will succeed for clang20.
> 
> Note that we cannot just change BPF_USDT_MAX_SPEC_CNT from 256 to 2 (through overwriting
> BPF_USDT_MAX_SPEC_CNT before usdt.bpf.h) since it will cause other test failures.
> We cannot just set BPF_USDT_MAX_SPEC_CNT to 2 for test_usdt_multispec.c since we
> have below in the Makefile:
>   test_usdt.skel.h-deps := test_usdt.bpf.o test_usdt_multispec.bpf.o
> and the linker will enforce that BPF_USDT_MAX_SPEC_CNT values for both progs must
> be the same.
> 
> The refactoring does not change existing test result. But the future change will
> allow to set BPF_USDT_MAX_SPEC_CNT to be 2 for arm64/clang20 case, which will have
> the same attachment failure as in gcc11.
> 
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---
>  tools/testing/selftests/bpf/prog_tests/usdt.c | 35 +++++++++++++------
>  1 file changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/usdt.c b/tools/testing/selftests/bpf/prog_tests/usdt.c
> index 495d66414b57..dc29ef94312a 100644
> --- a/tools/testing/selftests/bpf/prog_tests/usdt.c
> +++ b/tools/testing/selftests/bpf/prog_tests/usdt.c
> @@ -270,18 +270,8 @@ static void subtest_multispec_usdt(void)
>  	 */
>  	trigger_300_usdts();

should above line (plus the comment) ...

>  
> -	/* we'll reuse usdt_100 BPF program for usdt_300 test */
>  	bpf_link__destroy(skel->links.usdt_100);
> -	skel->links.usdt_100 = bpf_program__attach_usdt(skel->progs.usdt_100, -1, "/proc/self/exe",
> -							"test", "usdt_300", NULL);
> -	err = -errno;
> -	if (!ASSERT_ERR_PTR(skel->links.usdt_100, "usdt_300_bad_attach"))
> -		goto cleanup;
> -	ASSERT_EQ(err, -E2BIG, "usdt_300_attach_err");
>  
> -	/* let's check that there are no "dangling" BPF programs attached due
> -	 * to partial success of the above test:usdt_300 attachment
> -	 */

... and the code below (up to usdt_301_sum assert)
go to the new subtest_multispec_fail_usdt test as well?

jirka

>  	bss->usdt_100_called = 0;
>  	bss->usdt_100_sum = 0;
>  
> @@ -312,6 +302,29 @@ static void subtest_multispec_usdt(void)
>  	test_usdt__destroy(skel);
>  }
>  
> +static void subtest_multispec_fail_usdt(void)
> +{
> +	LIBBPF_OPTS(bpf_usdt_opts, opts);
> +	struct test_usdt *skel;
> +	int err;
> +
> +	skel = test_usdt__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "skel_open"))
> +		return;
> +
> +	skel->bss->my_pid = getpid();
> +
> +	skel->links.usdt_100 = bpf_program__attach_usdt(skel->progs.usdt_100, -1, "/proc/self/exe",
> +							"test", "usdt_300", NULL);
> +	err = -errno;
> +	if (!ASSERT_ERR_PTR(skel->links.usdt_100, "usdt_300_bad_attach"))
> +		goto cleanup;
> +	ASSERT_EQ(err, -E2BIG, "usdt_300_attach_err");
> +
> +cleanup:
> +	test_usdt__destroy(skel);
> +}
> +
>  static FILE *urand_spawn(int *pid)
>  {
>  	FILE *f;
> @@ -422,6 +435,8 @@ void test_usdt(void)
>  		subtest_basic_usdt();
>  	if (test__start_subtest("multispec"))
>  		subtest_multispec_usdt();
> +	if (test__start_subtest("multispec_fail"))
> +		subtest_multispec_fail_usdt();
>  	if (test__start_subtest("urand_auto_attach"))
>  		subtest_urandom_usdt(true /* auto_attach */);
>  	if (test__start_subtest("urand_pid_attach"))
> -- 
> 2.47.1
> 
> 

  reply	other threads:[~2025-06-16  8:33 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-15 18:53 [PATCH bpf-next v2 0/3] selftests/bpf: Fix usdt/multispec failure with arm64/clang20 Yonghong Song
2025-06-15 18:53 ` [PATCH bpf-next v2 1/3] selftests/bpf: Refactor the failed assertion to another subtest Yonghong Song
2025-06-16  8:33   ` Jiri Olsa [this message]
2025-06-16 15:48     ` Yonghong Song
2025-06-16 22:00   ` Andrii Nakryiko
2025-06-18  4:36     ` Yonghong Song
2025-06-24 15:36       ` Andrii Nakryiko
2025-06-24 16:15         ` Yonghong Song
2025-06-24 19:48           ` Andrii Nakryiko
2025-06-24 20:21             ` Yonghong Song
2025-06-15 18:53 ` [PATCH bpf-next v2 2/3] selftests/bpf: Add test_usdt_multispec.inc.h for sharing between multiple progs Yonghong Song
2025-06-15 18:54 ` [PATCH bpf-next v2 3/3] selftests/bpf: Add subtest usdt_multispec_fail with adjustable BPF_USDT_MAX_SPEC_CNT Yonghong Song

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=aE_W1ZoK6BZ6_EGA@krava \
    --to=olsajiri@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@fb.com \
    --cc=martin.lau@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.