All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] selftests:bpf:Fix repeated initialization
@ 2023-07-05 12:33 Wang Ming
  2023-07-05 13:32 ` Daniel Borkmann
  0 siblings, 1 reply; 3+ messages in thread
From: Wang Ming @ 2023-07-05 12:33 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Mykola Lysenko,
	Shuah Khan, Wang Ming, bpf, linux-kselftest, linux-kernel
  Cc: opensource.kernel

In use_missing_map function, value is
initialized twice.There is no
connection between the two assignment.
This patch could fix this bug.

Signed-off-by: Wang Ming <machel@vivo.com>
---
 tools/testing/selftests/bpf/progs/test_log_fixup.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/test_log_fixup.c b/tools/testing/selftests/bpf/progs/test_log_fixup.c
index 1bd48feaaa42..1c49b2f9be6c 100644
--- a/tools/testing/selftests/bpf/progs/test_log_fixup.c
+++ b/tools/testing/selftests/bpf/progs/test_log_fixup.c
@@ -52,13 +52,9 @@ struct {
 SEC("?raw_tp/sys_enter")
 int use_missing_map(const void *ctx)
 {
-	int zero = 0, *value;
+	int zero = 0;
 
-	value = bpf_map_lookup_elem(&existing_map, &zero);
-
-	value = bpf_map_lookup_elem(&missing_map, &zero);
-
-	return value != NULL;
+	return bpf_map_lookup_elem(&missing_map, &zero) != NULL;
 }
 
 extern int bpf_nonexistent_kfunc(void) __ksym __weak;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH v1] selftests:bpf:Fix repeated initialization
  2023-07-05 12:33 [PATCH v1] selftests:bpf:Fix repeated initialization Wang Ming
@ 2023-07-05 13:32 ` Daniel Borkmann
  2023-07-05 13:38   ` 回复: " 王明-软件底层技术部
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Borkmann @ 2023-07-05 13:32 UTC (permalink / raw)
  To: Wang Ming, Alexei Starovoitov, Andrii Nakryiko, Martin KaFai Lau,
	Song Liu, Yonghong Song, John Fastabend, KP Singh,
	Stanislav Fomichev, Hao Luo, Jiri Olsa, Mykola Lysenko,
	Shuah Khan, bpf, linux-kselftest, linux-kernel
  Cc: opensource.kernel

On 7/5/23 2:33 PM, Wang Ming wrote:
> In use_missing_map function, value is
> initialized twice.There is no
> connection between the two assignment.
> This patch could fix this bug.

Please never submit patches where you are just speculating and did not even
bother to run BPF selftests !

Otherwise you would have seen that your change is breaking it :

Error: #126 log_fixup
Error: #126/5 log_fixup/missing_map
   Error: #126/5 log_fixup/missing_map
   missing_map:PASS:skel_open 0 nsec
   libbpf: prog 'use_missing_map': BPF program load failed: Invalid argument
   libbpf: prog 'use_missing_map': failed to load: -22
   libbpf: failed to load object 'test_log_fixup'
   libbpf: failed to load BPF skeleton 'test_log_fixup': -22
   missing_map:PASS:load_fail 0 nsec
   missing_map:PASS:existing_map_autocreate 0 nsec
   missing_map:PASS:missing_map_autocreate 0 nsec
   missing_map:FAIL:log_buf unexpected log_buf: '8: <invalid BPF map reference>
   BPF map 'missing_map' is referenced but wasn't created
   ' is not a substring of 'reg type unsupported for arg#0 function use_missing_map#20
   0: R1=ctx(off=0,imm=0) R10=fp0
   ; int use_missing_map(const void *ctx)
   0: (b4) w1 = 0                        ; R1_w=0
   ; int zero = 0;
   1: (63) *(u32 *)(r10 -4) = r1         ; R1_w=0 R10=fp0 fp-8=0000????
   2: (bf) r2 = r10                      ; R2_w=fp0 R10=fp0
   ;
   3: (07) r2 += -4                      ; R2_w=fp-4
   ; return bpf_map_lookup_elem(&missing_map, &zero) != NULL;
   4: <invalid BPF map reference>
   BPF map 'missing_map' is referenced but wasn't created
   processed 5 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0

> Signed-off-by: Wang Ming <machel@vivo.com>
> ---
>   tools/testing/selftests/bpf/progs/test_log_fixup.c | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/progs/test_log_fixup.c b/tools/testing/selftests/bpf/progs/test_log_fixup.c
> index 1bd48feaaa42..1c49b2f9be6c 100644
> --- a/tools/testing/selftests/bpf/progs/test_log_fixup.c
> +++ b/tools/testing/selftests/bpf/progs/test_log_fixup.c
> @@ -52,13 +52,9 @@ struct {
>   SEC("?raw_tp/sys_enter")
>   int use_missing_map(const void *ctx)
>   {
> -	int zero = 0, *value;
> +	int zero = 0;
>   
> -	value = bpf_map_lookup_elem(&existing_map, &zero);
> -
> -	value = bpf_map_lookup_elem(&missing_map, &zero);
> -
> -	return value != NULL;
> +	return bpf_map_lookup_elem(&missing_map, &zero) != NULL;
>   }
>   
>   extern int bpf_nonexistent_kfunc(void) __ksym __weak;
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread
* 回复: [PATCH v1] selftests:bpf:Fix repeated initialization
  2023-07-05 13:32 ` Daniel Borkmann
@ 2023-07-05 13:38   ` 王明-软件底层技术部
  0 siblings, 0 replies; 3+ messages in thread
From: 王明-软件底层技术部 @ 2023-07-05 13:38 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Alexei Starovoitov, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Mykola Lysenko, Shuah Khan, bpf,
	linux-kselftest, linux-kernel

Thank you, but I don't understand the error.

-----邮件原件-----
发件人: Daniel Borkmann <daniel@iogearbox.net> 
发送时间: 2023年7月5日 21:32
收件人: 王明-软件底层技术部 <machel@vivo.com>; Alexei Starovoitov <ast@kernel.org>; Andrii Nakryiko <andrii@kernel.org>; Martin KaFai Lau <martin.lau@linux.dev>; Song Liu <song@kernel.org>; Yonghong Song <yhs@fb.com>; John Fastabend <john.fastabend@gmail.com>; KP Singh <kpsingh@kernel.org>; Stanislav Fomichev <sdf@google.com>; Hao Luo <haoluo@google.com>; Jiri Olsa <jolsa@kernel.org>; Mykola Lysenko <mykolal@fb.com>; Shuah Khan <shuah@kernel.org>; bpf@vger.kernel.org; linux-kselftest@vger.kernel.org; linux-kernel@vger.kernel.org
抄送: opensource.kernel <opensource.kernel@vivo.com>
主题: Re: [PATCH v1] selftests:bpf:Fix repeated initialization

On 7/5/23 2:33 PM, Wang Ming wrote:
> In use_missing_map function, value is
> initialized twice.There is no
> connection between the two assignment.
> This patch could fix this bug.

Please never submit patches where you are just speculating and did not even bother to run BPF selftests !

Otherwise you would have seen that your change is breaking it :

Error: #126 log_fixup
Error: #126/5 log_fixup/missing_map
   Error: #126/5 log_fixup/missing_map
   missing_map:PASS:skel_open 0 nsec
   libbpf: prog 'use_missing_map': BPF program load failed: Invalid argument
   libbpf: prog 'use_missing_map': failed to load: -22
   libbpf: failed to load object 'test_log_fixup'
   libbpf: failed to load BPF skeleton 'test_log_fixup': -22
   missing_map:PASS:load_fail 0 nsec
   missing_map:PASS:existing_map_autocreate 0 nsec
   missing_map:PASS:missing_map_autocreate 0 nsec
   missing_map:FAIL:log_buf unexpected log_buf: '8: <invalid BPF map reference>
   BPF map 'missing_map' is referenced but wasn't created
   ' is not a substring of 'reg type unsupported for arg#0 function use_missing_map#20
   0: R1=ctx(off=0,imm=0) R10=fp0
   ; int use_missing_map(const void *ctx)
   0: (b4) w1 = 0                        ; R1_w=0
   ; int zero = 0;
   1: (63) *(u32 *)(r10 -4) = r1         ; R1_w=0 R10=fp0 fp-8=0000????
   2: (bf) r2 = r10                      ; R2_w=fp0 R10=fp0
   ;
   3: (07) r2 += -4                      ; R2_w=fp-4
   ; return bpf_map_lookup_elem(&missing_map, &zero) != NULL;
   4: <invalid BPF map reference>
   BPF map 'missing_map' is referenced but wasn't created
   processed 5 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0

> Signed-off-by: Wang Ming <machel@vivo.com>
> ---
>   tools/testing/selftests/bpf/progs/test_log_fixup.c | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/progs/test_log_fixup.c 
> b/tools/testing/selftests/bpf/progs/test_log_fixup.c
> index 1bd48feaaa42..1c49b2f9be6c 100644
> --- a/tools/testing/selftests/bpf/progs/test_log_fixup.c
> +++ b/tools/testing/selftests/bpf/progs/test_log_fixup.c
> @@ -52,13 +52,9 @@ struct {
>   SEC("?raw_tp/sys_enter")
>   int use_missing_map(const void *ctx)
>   {
> -	int zero = 0, *value;
> +	int zero = 0;
>   
> -	value = bpf_map_lookup_elem(&existing_map, &zero);
> -
> -	value = bpf_map_lookup_elem(&missing_map, &zero);
> -
> -	return value != NULL;
> +	return bpf_map_lookup_elem(&missing_map, &zero) != NULL;
>   }
>   
>   extern int bpf_nonexistent_kfunc(void) __ksym __weak;
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-07-05 13:38 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-05 12:33 [PATCH v1] selftests:bpf:Fix repeated initialization Wang Ming
2023-07-05 13:32 ` Daniel Borkmann
2023-07-05 13:38   ` 回复: " 王明-软件底层技术部

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.