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;
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;
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;
linux-kselftest-mirror@lists.linaro.org