[RFC] bpf: Issue with bpf_fentry_test7 call

Message ID	ZXwZa_eK7bWXjJk7@krava (mailing list archive)
State	RFC
Delegated to:	BPF
Headers	show Received: from mail-lj1-f178.google.com (mail-lj1-f178.google.com [209.85.208.178]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6C86E182CC for <bpf@vger.kernel.org>; Fri, 15 Dec 2023 09:16:32 +0000 (UTC) From: Jiri Olsa <olsajiri@gmail.com> Date: Fri, 15 Dec 2023 10:16:27 +0100 To: Alexei Starovoitov <ast@kernel.org>, Daniel Borkmann <daniel@iogearbox.net>, Andrii Nakryiko <andrii@kernel.org>, Yonghong Song <yhs@fb.com> Cc: bpf@vger.kernel.org, Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>, John Fastabend <john.fastabend@gmail.com>, KP Singh <kpsingh@chromium.org>, Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com> Subject: [RFC] bpf: Issue with bpf_fentry_test7 call Message-ID: <ZXwZa_eK7bWXjJk7@krava> Precedence: bulk MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline
Series	[RFC] bpf: Issue with bpf_fentry_test7 call \| expand [RFC] bpf: Issue with bpf_fentry_test7 call

Context	Check	Description
bpf/vmtest-bpf-VM_Test-2	success	Logs for Unittests
bpf/vmtest-bpf-VM_Test-3	success	Logs for Validate matrix.py
bpf/vmtest-bpf-VM_Test-0	success	Logs for Lint
bpf/vmtest-bpf-VM_Test-1	success	Logs for ShellCheck
bpf/vmtest-bpf-VM_Test-5	success	Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-VM_Test-6	success	Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-10	success	Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-VM_Test-12	success	Logs for s390x-gcc / build-release
bpf/vmtest-bpf-VM_Test-4	success	Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-VM_Test-9	success	Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
netdev/tree_selection	success	Not a local patch
bpf/vmtest-bpf-VM_Test-7	success	Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-22	success	Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-8	success	Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-32	success	Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-VM_Test-18	success	Logs for set-matrix
bpf/vmtest-bpf-VM_Test-25	success	Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-17	success	Logs for s390x-gcc / veristat
bpf/vmtest-bpf-VM_Test-30	success	Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-VM_Test-21	success	Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-20	success	Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-VM_Test-31	success	Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-VM_Test-37	success	Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-VM_Test-39	fail	Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-VM_Test-34	success	Logs for x86_64-llvm-17 / veristat
bpf/vmtest-bpf-VM_Test-26	success	Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-19	success	Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-VM_Test-35	success	Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-VM_Test-23	success	Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-11	success	Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-VM_Test-42	success	Logs for x86_64-llvm-18 / veristat
bpf/vmtest-bpf-VM_Test-33	success	Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-VM_Test-40	fail	Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-VM_Test-41	success	Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-VM_Test-36	success	Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18 and -O2 optimization
bpf/vmtest-bpf-VM_Test-24	success	Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-38	fail	Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-VM_Test-27	success	Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-28	success	Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-VM_Test-16	success	Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-VM_Test-29	success	Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17 and -O2 optimization
bpf/vmtest-bpf-VM_Test-15	success	Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-PR	fail	PR summary
bpf/vmtest-bpf-VM_Test-14	success	Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc
bpf/vmtest-bpf-VM_Test-13	success	Logs for s390x-gcc / test (test_maps, false, 360) / test_maps on s390x with gcc

Jiri Olsa Dec. 15, 2023, 9:16 a.m. UTC

hi,   
The bpf CI is broken due to clang emitting 2 functions for
bpf_fentry_test7:

  # cat available_filter_functions | grep bpf_fentry_test7
  bpf_fentry_test7
  bpf_fentry_test7.specialized.1

The tests attach to 'bpf_fentry_test7' while the function with
'.specialized.1' suffix is executed in bpf_prog_test_run_tracing.

It looks like clang optimalization that comes from passing 0
as argument and returning it directly in bpf_fentry_test7.

I'm not sure there's a way to disable this, so far I came
up with solution below that passes real pointer, but I think
that was not the original intention for the test.

We had issue with this function back in august:
  32337c0a2824 bpf: Prevent inlining of bpf_fentry_test7()

I'm not sure why it started to show now? was clang updated for CI?

I'll try to find out more, but any clang ideas are welcome ;-)

thanks,
jirka


---

Jiri Olsa Dec. 15, 2023, 9:43 a.m. UTC | #1

On Fri, Dec 15, 2023 at 10:16:27AM +0100, Jiri Olsa wrote:
> hi,   
> The bpf CI is broken due to clang emitting 2 functions for
> bpf_fentry_test7:
> 
>   # cat available_filter_functions | grep bpf_fentry_test7
>   bpf_fentry_test7
>   bpf_fentry_test7.specialized.1
> 
> The tests attach to 'bpf_fentry_test7' while the function with
> '.specialized.1' suffix is executed in bpf_prog_test_run_tracing.
> 
> It looks like clang optimalization that comes from passing 0
> as argument and returning it directly in bpf_fentry_test7.
> 
> I'm not sure there's a way to disable this, so far I came
> up with solution below that passes real pointer, but I think
> that was not the original intention for the test.
> 
> We had issue with this function back in august:
>   32337c0a2824 bpf: Prevent inlining of bpf_fentry_test7()
> 
> I'm not sure why it started to show now? was clang updated for CI?
> 
> I'll try to find out more, but any clang ideas are welcome ;-)

fyi also there's probably another related usse in global_func17 test:

	run_subtest:FAIL:unexpected_load_success unexpected success: 0
	#290/17  test_global_funcs/global_func17:FAIL

looks like clang optimized the call out and returns the value directly:

	Disassembly of section .text:

	0000000000000000 <foo>:
	       0:       b4 00 00 00 00 00 00 00 w0 = 0x0
	       1:       15 01 02 00 00 00 00 00 if r1 == 0x0 goto +0x2 <LBB0_2>
	       2:       b4 00 00 00 2a 00 00 00 w0 = 0x2a
	       3:       63 01 00 00 00 00 00 00 *(u32 *)(r1 + 0x0) = r0

	0000000000000020 <LBB0_2>:
	       4:       95 00 00 00 00 00 00 00 exit

	Disassembly of section tc:

	0000000000000000 <global_func17>:
	       0:       b4 00 00 00 2a 00 00 00 w0 = 0x2a
	       1:       95 00 00 00 00 00 00 00 exit

jirka


> 
> thanks,
> jirka
> 
> 
> ---
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index c9fdcc5cdce1..33208eec9361 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -543,7 +543,7 @@ struct bpf_fentry_test_t {
>  int noinline bpf_fentry_test7(struct bpf_fentry_test_t *arg)
>  {
>  	asm volatile ("");
> -	return (long)arg;
> +	return 0;
>  }
>  
>  int noinline bpf_fentry_test8(struct bpf_fentry_test_t *arg)
> @@ -668,7 +668,7 @@ int bpf_prog_test_run_tracing(struct bpf_prog *prog,
>  		    bpf_fentry_test4((void *)7, 8, 9, 10) != 34 ||
>  		    bpf_fentry_test5(11, (void *)12, 13, 14, 15) != 65 ||
>  		    bpf_fentry_test6(16, (void *)17, 18, 19, (void *)20, 21) != 111 ||
> -		    bpf_fentry_test7((struct bpf_fentry_test_t *)0) != 0 ||
> +		    bpf_fentry_test7(&arg) != 0 ||
>  		    bpf_fentry_test8(&arg) != 0 ||
>  		    bpf_fentry_test9(&retval) != 0)
>  			goto out;
> diff --git a/tools/testing/selftests/bpf/progs/fentry_test.c b/tools/testing/selftests/bpf/progs/fentry_test.c
> index 52a550d281d9..95c5c34ccaa8 100644
> --- a/tools/testing/selftests/bpf/progs/fentry_test.c
> +++ b/tools/testing/selftests/bpf/progs/fentry_test.c
> @@ -64,7 +64,7 @@ __u64 test7_result = 0;
>  SEC("fentry/bpf_fentry_test7")
>  int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
>  {
> -	if (!arg)
> +	if (arg)
>  		test7_result = 1;
>  	return 0;
>  }
> diff --git a/tools/testing/selftests/bpf/progs/fexit_test.c b/tools/testing/selftests/bpf/progs/fexit_test.c
> index 8f1ccb7302e1..ffb30236ca02 100644
> --- a/tools/testing/selftests/bpf/progs/fexit_test.c
> +++ b/tools/testing/selftests/bpf/progs/fexit_test.c
> @@ -65,7 +65,7 @@ __u64 test7_result = 0;
>  SEC("fexit/bpf_fentry_test7")
>  int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
>  {
> -	if (!arg)
> +	if (arg)
>  		test7_result = 1;
>  	return 0;
>  }

Jiri Olsa Dec. 15, 2023, 2:24 p.m. UTC | #2

On Fri, Dec 15, 2023 at 10:16:27AM +0100, Jiri Olsa wrote:
> hi,   
> The bpf CI is broken due to clang emitting 2 functions for
> bpf_fentry_test7:
> 
>   # cat available_filter_functions | grep bpf_fentry_test7
>   bpf_fentry_test7
>   bpf_fentry_test7.specialized.1
> 
> The tests attach to 'bpf_fentry_test7' while the function with
> '.specialized.1' suffix is executed in bpf_prog_test_run_tracing.
> 
> It looks like clang optimalization that comes from passing 0
> as argument and returning it directly in bpf_fentry_test7.
> 
> I'm not sure there's a way to disable this, so far I came
> up with solution below that passes real pointer, but I think
> that was not the original intention for the test.
> 
> We had issue with this function back in august:
>   32337c0a2824 bpf: Prevent inlining of bpf_fentry_test7()
> 
> I'm not sure why it started to show now? was clang updated for CI?
> 
> I'll try to find out more, but any clang ideas are welcome ;-)
> 
> thanks,
> jirka


hm, there seems to be fix in bpf-next for this one:

  b16904fd9f01 bpf: Fix a few selftest failures due to llvm18 change

jirka

> 
> 
> ---
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index c9fdcc5cdce1..33208eec9361 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -543,7 +543,7 @@ struct bpf_fentry_test_t {
>  int noinline bpf_fentry_test7(struct bpf_fentry_test_t *arg)
>  {
>  	asm volatile ("");
> -	return (long)arg;
> +	return 0;
>  }
>  
>  int noinline bpf_fentry_test8(struct bpf_fentry_test_t *arg)
> @@ -668,7 +668,7 @@ int bpf_prog_test_run_tracing(struct bpf_prog *prog,
>  		    bpf_fentry_test4((void *)7, 8, 9, 10) != 34 ||
>  		    bpf_fentry_test5(11, (void *)12, 13, 14, 15) != 65 ||
>  		    bpf_fentry_test6(16, (void *)17, 18, 19, (void *)20, 21) != 111 ||
> -		    bpf_fentry_test7((struct bpf_fentry_test_t *)0) != 0 ||
> +		    bpf_fentry_test7(&arg) != 0 ||
>  		    bpf_fentry_test8(&arg) != 0 ||
>  		    bpf_fentry_test9(&retval) != 0)
>  			goto out;
> diff --git a/tools/testing/selftests/bpf/progs/fentry_test.c b/tools/testing/selftests/bpf/progs/fentry_test.c
> index 52a550d281d9..95c5c34ccaa8 100644
> --- a/tools/testing/selftests/bpf/progs/fentry_test.c
> +++ b/tools/testing/selftests/bpf/progs/fentry_test.c
> @@ -64,7 +64,7 @@ __u64 test7_result = 0;
>  SEC("fentry/bpf_fentry_test7")
>  int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
>  {
> -	if (!arg)
> +	if (arg)
>  		test7_result = 1;
>  	return 0;
>  }
> diff --git a/tools/testing/selftests/bpf/progs/fexit_test.c b/tools/testing/selftests/bpf/progs/fexit_test.c
> index 8f1ccb7302e1..ffb30236ca02 100644
> --- a/tools/testing/selftests/bpf/progs/fexit_test.c
> +++ b/tools/testing/selftests/bpf/progs/fexit_test.c
> @@ -65,7 +65,7 @@ __u64 test7_result = 0;
>  SEC("fexit/bpf_fentry_test7")
>  int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
>  {
> -	if (!arg)
> +	if (arg)
>  		test7_result = 1;
>  	return 0;
>  }

Yonghong Song Dec. 15, 2023, 2:42 p.m. UTC | #3

On 12/15/23 6:24 AM, Jiri Olsa wrote:
> On Fri, Dec 15, 2023 at 10:16:27AM +0100, Jiri Olsa wrote:
>> hi,
>> The bpf CI is broken due to clang emitting 2 functions for
>> bpf_fentry_test7:
>>
>>    # cat available_filter_functions | grep bpf_fentry_test7
>>    bpf_fentry_test7
>>    bpf_fentry_test7.specialized.1
>>
>> The tests attach to 'bpf_fentry_test7' while the function with
>> '.specialized.1' suffix is executed in bpf_prog_test_run_tracing.
>>
>> It looks like clang optimalization that comes from passing 0
>> as argument and returning it directly in bpf_fentry_test7.
>>
>> I'm not sure there's a way to disable this, so far I came
>> up with solution below that passes real pointer, but I think
>> that was not the original intention for the test.
>>
>> We had issue with this function back in august:
>>    32337c0a2824 bpf: Prevent inlining of bpf_fentry_test7()
>>
>> I'm not sure why it started to show now? was clang updated for CI?
>>
>> I'll try to find out more, but any clang ideas are welcome ;-)
>>
>> thanks,
>> jirka
>
> hm, there seems to be fix in bpf-next for this one:
>
>    b16904fd9f01 bpf: Fix a few selftest failures due to llvm18 change

Maybe submit a patch to https://github.com/kernel-patches/vmtest/tree/master/ci/diffs?
That is typically the place to have temporary patches to workaround ci failures.

>
> jirka
>
>>
>> ---
>> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
>> index c9fdcc5cdce1..33208eec9361 100644
>> --- a/net/bpf/test_run.c
>> +++ b/net/bpf/test_run.c
>> @@ -543,7 +543,7 @@ struct bpf_fentry_test_t {
>>   int noinline bpf_fentry_test7(struct bpf_fentry_test_t *arg)
>>   {
>>   	asm volatile ("");
>> -	return (long)arg;
>> +	return 0;
>>   }
>>   
>>   int noinline bpf_fentry_test8(struct bpf_fentry_test_t *arg)
>> @@ -668,7 +668,7 @@ int bpf_prog_test_run_tracing(struct bpf_prog *prog,
>>   		    bpf_fentry_test4((void *)7, 8, 9, 10) != 34 ||
>>   		    bpf_fentry_test5(11, (void *)12, 13, 14, 15) != 65 ||
>>   		    bpf_fentry_test6(16, (void *)17, 18, 19, (void *)20, 21) != 111 ||
>> -		    bpf_fentry_test7((struct bpf_fentry_test_t *)0) != 0 ||
>> +		    bpf_fentry_test7(&arg) != 0 ||
>>   		    bpf_fentry_test8(&arg) != 0 ||
>>   		    bpf_fentry_test9(&retval) != 0)
>>   			goto out;
>> diff --git a/tools/testing/selftests/bpf/progs/fentry_test.c b/tools/testing/selftests/bpf/progs/fentry_test.c
>> index 52a550d281d9..95c5c34ccaa8 100644
>> --- a/tools/testing/selftests/bpf/progs/fentry_test.c
>> +++ b/tools/testing/selftests/bpf/progs/fentry_test.c
>> @@ -64,7 +64,7 @@ __u64 test7_result = 0;
>>   SEC("fentry/bpf_fentry_test7")
>>   int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
>>   {
>> -	if (!arg)
>> +	if (arg)
>>   		test7_result = 1;
>>   	return 0;
>>   }
>> diff --git a/tools/testing/selftests/bpf/progs/fexit_test.c b/tools/testing/selftests/bpf/progs/fexit_test.c
>> index 8f1ccb7302e1..ffb30236ca02 100644
>> --- a/tools/testing/selftests/bpf/progs/fexit_test.c
>> +++ b/tools/testing/selftests/bpf/progs/fexit_test.c
>> @@ -65,7 +65,7 @@ __u64 test7_result = 0;
>>   SEC("fexit/bpf_fentry_test7")
>>   int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
>>   {
>> -	if (!arg)
>> +	if (arg)
>>   		test7_result = 1;
>>   	return 0;
>>   }

Andrii Nakryiko Dec. 15, 2023, 9:22 p.m. UTC | #4

On Fri, Dec 15, 2023 at 6:42 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>
>
> On 12/15/23 6:24 AM, Jiri Olsa wrote:
> > On Fri, Dec 15, 2023 at 10:16:27AM +0100, Jiri Olsa wrote:
> >> hi,
> >> The bpf CI is broken due to clang emitting 2 functions for
> >> bpf_fentry_test7:
> >>
> >>    # cat available_filter_functions | grep bpf_fentry_test7
> >>    bpf_fentry_test7
> >>    bpf_fentry_test7.specialized.1
> >>
> >> The tests attach to 'bpf_fentry_test7' while the function with
> >> '.specialized.1' suffix is executed in bpf_prog_test_run_tracing.
> >>
> >> It looks like clang optimalization that comes from passing 0
> >> as argument and returning it directly in bpf_fentry_test7.
> >>
> >> I'm not sure there's a way to disable this, so far I came
> >> up with solution below that passes real pointer, but I think
> >> that was not the original intention for the test.
> >>
> >> We had issue with this function back in august:
> >>    32337c0a2824 bpf: Prevent inlining of bpf_fentry_test7()
> >>
> >> I'm not sure why it started to show now? was clang updated for CI?
> >>
> >> I'll try to find out more, but any clang ideas are welcome ;-)
> >>
> >> thanks,
> >> jirka
> >
> > hm, there seems to be fix in bpf-next for this one:
> >
> >    b16904fd9f01 bpf: Fix a few selftest failures due to llvm18 change
>
> Maybe submit a patch to https://github.com/kernel-patches/vmtest/tree/master/ci/diffs?
> That is typically the place to have temporary patches to workaround ci failures.
>

To get bpf/master back to green CI I did it meanwhile ([0]). Jiri,
please check the PR to be familiar with the process for the future
similar mitigations, thanks.

  [0] https://github.com/kernel-patches/vmtest/pull/258

> >
> > jirka
> >
> >>
> >> ---
> >> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> >> index c9fdcc5cdce1..33208eec9361 100644
> >> --- a/net/bpf/test_run.c
> >> +++ b/net/bpf/test_run.c
> >> @@ -543,7 +543,7 @@ struct bpf_fentry_test_t {
> >>   int noinline bpf_fentry_test7(struct bpf_fentry_test_t *arg)
> >>   {
> >>      asm volatile ("");
> >> -    return (long)arg;
> >> +    return 0;
> >>   }
> >>
> >>   int noinline bpf_fentry_test8(struct bpf_fentry_test_t *arg)
> >> @@ -668,7 +668,7 @@ int bpf_prog_test_run_tracing(struct bpf_prog *prog,
> >>                  bpf_fentry_test4((void *)7, 8, 9, 10) != 34 ||
> >>                  bpf_fentry_test5(11, (void *)12, 13, 14, 15) != 65 ||
> >>                  bpf_fentry_test6(16, (void *)17, 18, 19, (void *)20, 21) != 111 ||
> >> -                bpf_fentry_test7((struct bpf_fentry_test_t *)0) != 0 ||
> >> +                bpf_fentry_test7(&arg) != 0 ||
> >>                  bpf_fentry_test8(&arg) != 0 ||
> >>                  bpf_fentry_test9(&retval) != 0)
> >>                      goto out;
> >> diff --git a/tools/testing/selftests/bpf/progs/fentry_test.c b/tools/testing/selftests/bpf/progs/fentry_test.c
> >> index 52a550d281d9..95c5c34ccaa8 100644
> >> --- a/tools/testing/selftests/bpf/progs/fentry_test.c
> >> +++ b/tools/testing/selftests/bpf/progs/fentry_test.c
> >> @@ -64,7 +64,7 @@ __u64 test7_result = 0;
> >>   SEC("fentry/bpf_fentry_test7")
> >>   int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
> >>   {
> >> -    if (!arg)
> >> +    if (arg)
> >>              test7_result = 1;
> >>      return 0;
> >>   }
> >> diff --git a/tools/testing/selftests/bpf/progs/fexit_test.c b/tools/testing/selftests/bpf/progs/fexit_test.c
> >> index 8f1ccb7302e1..ffb30236ca02 100644
> >> --- a/tools/testing/selftests/bpf/progs/fexit_test.c
> >> +++ b/tools/testing/selftests/bpf/progs/fexit_test.c
> >> @@ -65,7 +65,7 @@ __u64 test7_result = 0;
> >>   SEC("fexit/bpf_fentry_test7")
> >>   int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
> >>   {
> >> -    if (!arg)
> >> +    if (arg)
> >>              test7_result = 1;
> >>      return 0;
> >>   }

Jiri Olsa Dec. 15, 2023, 11:21 p.m. UTC | #5

On Fri, Dec 15, 2023 at 01:22:35PM -0800, Andrii Nakryiko wrote:
> On Fri, Dec 15, 2023 at 6:42 AM Yonghong Song <yonghong.song@linux.dev> wrote:
> >
> >
> > On 12/15/23 6:24 AM, Jiri Olsa wrote:
> > > On Fri, Dec 15, 2023 at 10:16:27AM +0100, Jiri Olsa wrote:
> > >> hi,
> > >> The bpf CI is broken due to clang emitting 2 functions for
> > >> bpf_fentry_test7:
> > >>
> > >>    # cat available_filter_functions | grep bpf_fentry_test7
> > >>    bpf_fentry_test7
> > >>    bpf_fentry_test7.specialized.1
> > >>
> > >> The tests attach to 'bpf_fentry_test7' while the function with
> > >> '.specialized.1' suffix is executed in bpf_prog_test_run_tracing.
> > >>
> > >> It looks like clang optimalization that comes from passing 0
> > >> as argument and returning it directly in bpf_fentry_test7.
> > >>
> > >> I'm not sure there's a way to disable this, so far I came
> > >> up with solution below that passes real pointer, but I think
> > >> that was not the original intention for the test.
> > >>
> > >> We had issue with this function back in august:
> > >>    32337c0a2824 bpf: Prevent inlining of bpf_fentry_test7()
> > >>
> > >> I'm not sure why it started to show now? was clang updated for CI?
> > >>
> > >> I'll try to find out more, but any clang ideas are welcome ;-)
> > >>
> > >> thanks,
> > >> jirka
> > >
> > > hm, there seems to be fix in bpf-next for this one:
> > >
> > >    b16904fd9f01 bpf: Fix a few selftest failures due to llvm18 change
> >
> > Maybe submit a patch to https://github.com/kernel-patches/vmtest/tree/master/ci/diffs?
> > That is typically the place to have temporary patches to workaround ci failures.
> >
> 
> To get bpf/master back to green CI I did it meanwhile ([0]). Jiri,
> please check the PR to be familiar with the process for the future
> similar mitigations, thanks.
> 
>   [0] https://github.com/kernel-patches/vmtest/pull/258

great, thanks

jirka

> 
> > >
> > > jirka
> > >
> > >>
> > >> ---
> > >> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> > >> index c9fdcc5cdce1..33208eec9361 100644
> > >> --- a/net/bpf/test_run.c
> > >> +++ b/net/bpf/test_run.c
> > >> @@ -543,7 +543,7 @@ struct bpf_fentry_test_t {
> > >>   int noinline bpf_fentry_test7(struct bpf_fentry_test_t *arg)
> > >>   {
> > >>      asm volatile ("");
> > >> -    return (long)arg;
> > >> +    return 0;
> > >>   }
> > >>
> > >>   int noinline bpf_fentry_test8(struct bpf_fentry_test_t *arg)
> > >> @@ -668,7 +668,7 @@ int bpf_prog_test_run_tracing(struct bpf_prog *prog,
> > >>                  bpf_fentry_test4((void *)7, 8, 9, 10) != 34 ||
> > >>                  bpf_fentry_test5(11, (void *)12, 13, 14, 15) != 65 ||
> > >>                  bpf_fentry_test6(16, (void *)17, 18, 19, (void *)20, 21) != 111 ||
> > >> -                bpf_fentry_test7((struct bpf_fentry_test_t *)0) != 0 ||
> > >> +                bpf_fentry_test7(&arg) != 0 ||
> > >>                  bpf_fentry_test8(&arg) != 0 ||
> > >>                  bpf_fentry_test9(&retval) != 0)
> > >>                      goto out;
> > >> diff --git a/tools/testing/selftests/bpf/progs/fentry_test.c b/tools/testing/selftests/bpf/progs/fentry_test.c
> > >> index 52a550d281d9..95c5c34ccaa8 100644
> > >> --- a/tools/testing/selftests/bpf/progs/fentry_test.c
> > >> +++ b/tools/testing/selftests/bpf/progs/fentry_test.c
> > >> @@ -64,7 +64,7 @@ __u64 test7_result = 0;
> > >>   SEC("fentry/bpf_fentry_test7")
> > >>   int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
> > >>   {
> > >> -    if (!arg)
> > >> +    if (arg)
> > >>              test7_result = 1;
> > >>      return 0;
> > >>   }
> > >> diff --git a/tools/testing/selftests/bpf/progs/fexit_test.c b/tools/testing/selftests/bpf/progs/fexit_test.c
> > >> index 8f1ccb7302e1..ffb30236ca02 100644
> > >> --- a/tools/testing/selftests/bpf/progs/fexit_test.c
> > >> +++ b/tools/testing/selftests/bpf/progs/fexit_test.c
> > >> @@ -65,7 +65,7 @@ __u64 test7_result = 0;
> > >>   SEC("fexit/bpf_fentry_test7")
> > >>   int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
> > >>   {
> > >> -    if (!arg)
> > >> +    if (arg)
> > >>              test7_result = 1;
> > >>      return 0;
> > >>   }

[RFC] bpf: Issue with bpf_fentry_test7 call

Checks

Commit Message

Comments

Patch