On Wed, Aug 26, 2020 at 12:13 AM Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > > On Tue, Aug 25, 2020 at 2:05 PM Alexei Starovoitov > <alexei.starovoitov@xxxxxxxxx> wrote: > > > > On Tue, Aug 25, 2020 at 11:29 AM KP Singh <kpsingh@xxxxxxxxxxxx> wrote: > > > > > > From: KP Singh <kpsingh@xxxxxxxxxx> > > > > > > # v9 -> v10 > > > > > > - Added NULL check for inode_storage_ptr before calling > > > bpf_local_storage_update > > > - Removed an extraneous include > > > - Rebased and added Acks / Signoff. > > > > Hmm. Though it looks good I cannot apply it, because > > test_progs -t map_ptr > > is broken: > > 2225: (18) r2 = 0xffffc900004e5004 > > 2227: (b4) w1 = 58 > > 2228: (63) *(u32 *)(r2 +0) = r1 > > R0=map_value(id=0,off=0,ks=4,vs=4,imm=0) R1_w=inv58 > > R2_w=map_value(id=0,off=4,ks=4,vs=8,imm=0) R3=inv49 R4=inv63 > > R5=inv(id=0,umax_value=4294967295,var_off=(0x0; 0xffffffff)) R6_w=inv0 > > R7=invP8 R8=map_ptr(id=0,off=0,ks=4,vs=4,imm=0) R10=? > > ; VERIFY_TYPE(BPF_MAP_TYPE_SK_STORAGE, check_sk_storage); > > 2229: (18) r1 = 0xffffc900004e5000 > > 2231: (b4) w3 = 24 > > 2232: (63) *(u32 *)(r1 +0) = r3 > > R0=map_value(id=0,off=0,ks=4,vs=4,imm=0) > > R1_w=map_value(id=0,off=0,ks=4,vs=8,imm=0) > > R2_w=map_value(id=0,off=4,ks=4,vs=8,imm=0) R3_w=inv24 R4=inv63 > > R5=inv(id=0,umax_value=4294967295,var_off=(0x0; 0xffffffff)) R6_w=inv0 > > R7=invP8 R8=map_pt? > > 2233: (18) r3 = 0xffff8881f03f7000 > > ; VERIFY(indirect->map_type == direct->map_type); > > 2235: (85) call unknown#195896080 > > invalid func unknown#195896080 > > processed 4678 insns (limit 1000000) max_states_per_insn 9 > > total_states 240 peak_states 178 mark_read 11 > > > > libbpf: -- END LOG -- > > libbpf: failed to load program 'cgroup_skb/egress' > > libbpf: failed to load object 'map_ptr_kern' > > libbpf: failed to load BPF skeleton 'map_ptr_kern': -4007 > > test_map_ptr:FAIL:skel_open_load open_load failed > > #43 map_ptr:FAIL > > > > Above 'invalid func unknown#195896080' happens > > when libbpf fails to do a relocation at runtime. > > Please debug. > > It's certainly caused by this set, but not sure why. > > So I've ended up bisecting and debugging it. > It turned out that the patch 1 was responsible. > I've added the following hunk to fix it: Thanks for fixing and debugging it. > diff --git a/tools/testing/selftests/bpf/progs/map_ptr_kern.c > b/tools/testing/selftests/bpf/progs/map_ptr_kern.c > index 473665cac67e..982a2d8aa844 100644 > --- a/tools/testing/selftests/bpf/progs/map_ptr_kern.c > +++ b/tools/testing/selftests/bpf/progs/map_ptr_kern.c > @@ -589,7 +589,7 @@ static inline int check_stack(void) > return 1; [...] > and pushed the whole set. > In the future please always run test_progs and test_progs-no_alu32 Noted, I do run them but this test gave me a different error and I always ended up ignoring this: ./test_progs -t map_ptr libbpf: Error in bpf_create_map_xattr(m_array_of_maps):ERROR: strerror_r(-524)=22(-524). Retrying without BTF. libbpf: Error in bpf_create_map_xattr(m_hash_of_maps):ERROR: strerror_r(-524)=22(-524). Retrying without BTF. libbpf: Error in bpf_create_map_xattr(m_perf_event_array):ERROR: strerror_r(-524)=22(-524). Retrying without BTF. libbpf: Error in bpf_create_map_xattr(m_stack_trace):ERROR: strerror_r(-524)=22(-524). Retrying without BTF. libbpf: Error in bpf_create_map_xattr(m_cgroup_array):ERROR: strerror_r(-524)=22(-524). Retrying without BTF. libbpf: Error in bpf_create_map_xattr(m_devmap):ERROR: strerror_r(-524)=22(-524). Retrying without BTF. libbpf: Error in bpf_create_map_xattr(m_sockmap):Invalid argument(-22). Retrying without BTF. libbpf: map 'm_sockmap': failed to create: Invalid argument(-22) libbpf: failed to load object 'map_ptr_kern' libbpf: failed to load BPF skeleton 'map_ptr_kern': -22 test_map_ptr:FAIL:skel_open_load open_load failed I now realized that I was not sourcing tools/testing/selftests/bpf/config correctly and CONFIG_BPF_STREAM_PARSER was not enabled in my configuration. Nonetheless, no excuses and will ensure these tests pass in the future. - KP > for every patch and submit patches only if _all_ tests are passing. > Do not assume that your change is not responsible for breakage.
