Re: [PATCH] tools/perf: Fix bpf__probe to set bpf_prog_type type only if differs from the desired one
On Thu, Aug 17, 2023 at 10:35 AM Athira Rajeev wrote: > > > > > On 07-Aug-2023, at 11:07 AM, Sachin Sant wrote: > > > > > > > >> On 07-Aug-2023, at 10:22 AM, Athira Rajeev > >> wrote: > >> > >> The test "BPF prologue generation" fails as below: > >> > >> Writing event: p:perf_bpf_probe/func _text+10423200 f_mode=+20(%gpr3):x32 > >> offset=%gpr4:s64 orig=%gpr5:s32 > >> In map_prologue, ntevs=1 > >> mapping[0]=0 > >> libbpf: prog 'bpf_func__null_lseek': BPF program load failed: Permission > >> denied > >> libbpf: prog 'bpf_func__null_lseek': -- BEGIN PROG LOAD LOG -- > >> btf_vmlinux is malformed > >> reg type unsupported for arg#0 function bpf_func__null_lseek#5 > >> 0: R1=ctx(off=0,imm=0) R10=fp0 > >> ; > >> 0: (57) r3 &= 2 > >> R3 !read_ok > >> processed 1 insns (limit 100) max_states_per_insn 0 total_states 0 > >> peak_states 0 mark_read 0 > >> -- END PROG LOAD LOG -- > >> libbpf: prog 'bpf_func__null_lseek': failed to load: -13 > >> libbpf: failed to load object '[bpf_prologue_test]' > >> bpf: load objects failed: err=-13: (Permission denied) > >> Failed to add events selected by BPF > >> > >> This fails occurs after this commit: > >> commit d6e6286a12e7 ("libbpf: disassociate section handler > >> on explicit bpf_program__set_type() call")' > >> > >> With this change, SEC_DEF handler libbpf which is determined > >> initially based on program's SEC() is set to NULL. The change > >> is made because sec_def is not valid when user sets the program > >> type with bpf_program__set_type function. This commit also fixed > >> bpf_prog_test_load() helper in selftests/bpf to force-set program > >> type only if it differs from the desired one. > >> > >> The "bpf__probe" function in util/bpf-loader.c, also calls > >> bpf_program__set_type to set bpf_prog_type. Add similar fix in > >> here as well to avoid setting sec_def to NULL. > >> > >> Reported-by: Sachin Sant > >> Signed-off-by: Athira Rajeev > >> --- > > > > Thanks Athira for the fix. > > With this patch applied perf BPF prologue sub test works correctly. > > > > 42: BPF filter : > > 42.1: Basic BPF filtering: Ok > > 42.2: BPF pinning : Ok > > 42.3: BPF prologue generation : Ok > > > > Tested-by: Sachin Sant > > > > Can you please use the above mentioned id(without vnet) in the reported-by ? > > > > - Sachin > > Hi All, > > Looking for review comments on this patch > > Athira Hi, the patch set: https://lore.kernel.org/lkml/20230810184853.2860737-1-irog...@google.com/ removes the affected code/test. Thanks, Ian
Re: [PATCH] tools/perf: Fix bpf__probe to set bpf_prog_type type only if differs from the desired one
> On 07-Aug-2023, at 11:07 AM, Sachin Sant wrote: > > > >> On 07-Aug-2023, at 10:22 AM, Athira Rajeev >> wrote: >> >> The test "BPF prologue generation" fails as below: >> >> Writing event: p:perf_bpf_probe/func _text+10423200 f_mode=+20(%gpr3):x32 >> offset=%gpr4:s64 orig=%gpr5:s32 >> In map_prologue, ntevs=1 >> mapping[0]=0 >> libbpf: prog 'bpf_func__null_lseek': BPF program load failed: Permission >> denied >> libbpf: prog 'bpf_func__null_lseek': -- BEGIN PROG LOAD LOG -- >> btf_vmlinux is malformed >> reg type unsupported for arg#0 function bpf_func__null_lseek#5 >> 0: R1=ctx(off=0,imm=0) R10=fp0 >> ; >> 0: (57) r3 &= 2 >> R3 !read_ok >> processed 1 insns (limit 100) max_states_per_insn 0 total_states 0 >> peak_states 0 mark_read 0 >> -- END PROG LOAD LOG -- >> libbpf: prog 'bpf_func__null_lseek': failed to load: -13 >> libbpf: failed to load object '[bpf_prologue_test]' >> bpf: load objects failed: err=-13: (Permission denied) >> Failed to add events selected by BPF >> >> This fails occurs after this commit: >> commit d6e6286a12e7 ("libbpf: disassociate section handler >> on explicit bpf_program__set_type() call")' >> >> With this change, SEC_DEF handler libbpf which is determined >> initially based on program's SEC() is set to NULL. The change >> is made because sec_def is not valid when user sets the program >> type with bpf_program__set_type function. This commit also fixed >> bpf_prog_test_load() helper in selftests/bpf to force-set program >> type only if it differs from the desired one. >> >> The "bpf__probe" function in util/bpf-loader.c, also calls >> bpf_program__set_type to set bpf_prog_type. Add similar fix in >> here as well to avoid setting sec_def to NULL. >> >> Reported-by: Sachin Sant >> Signed-off-by: Athira Rajeev >> --- > > Thanks Athira for the fix. > With this patch applied perf BPF prologue sub test works correctly. > > 42: BPF filter : > 42.1: Basic BPF filtering: Ok > 42.2: BPF pinning : Ok > 42.3: BPF prologue generation : Ok > > Tested-by: Sachin Sant > > Can you please use the above mentioned id(without vnet) in the reported-by ? > > - Sachin Hi All, Looking for review comments on this patch Athira
Re: [PATCH] tools/perf: Fix bpf__probe to set bpf_prog_type type only if differs from the desired one
> On 07-Aug-2023, at 10:22 AM, Athira Rajeev > wrote: > > The test "BPF prologue generation" fails as below: > > Writing event: p:perf_bpf_probe/func _text+10423200 f_mode=+20(%gpr3):x32 > offset=%gpr4:s64 orig=%gpr5:s32 > In map_prologue, ntevs=1 > mapping[0]=0 > libbpf: prog 'bpf_func__null_lseek': BPF program load failed: Permission > denied > libbpf: prog 'bpf_func__null_lseek': -- BEGIN PROG LOAD LOG -- > btf_vmlinux is malformed > reg type unsupported for arg#0 function bpf_func__null_lseek#5 > 0: R1=ctx(off=0,imm=0) R10=fp0 > ; > 0: (57) r3 &= 2 > R3 !read_ok > processed 1 insns (limit 100) max_states_per_insn 0 total_states 0 > peak_states 0 mark_read 0 > -- END PROG LOAD LOG -- > libbpf: prog 'bpf_func__null_lseek': failed to load: -13 > libbpf: failed to load object '[bpf_prologue_test]' > bpf: load objects failed: err=-13: (Permission denied) > Failed to add events selected by BPF > > This fails occurs after this commit: > commit d6e6286a12e7 ("libbpf: disassociate section handler > on explicit bpf_program__set_type() call")' > > With this change, SEC_DEF handler libbpf which is determined > initially based on program's SEC() is set to NULL. The change > is made because sec_def is not valid when user sets the program > type with bpf_program__set_type function. This commit also fixed > bpf_prog_test_load() helper in selftests/bpf to force-set program > type only if it differs from the desired one. > > The "bpf__probe" function in util/bpf-loader.c, also calls > bpf_program__set_type to set bpf_prog_type. Add similar fix in > here as well to avoid setting sec_def to NULL. > > Reported-by: Sachin Sant > Signed-off-by: Athira Rajeev > --- Thanks Athira for the fix. With this patch applied perf BPF prologue sub test works correctly. 42: BPF filter : 42.1: Basic BPF filtering: Ok 42.2: BPF pinning : Ok 42.3: BPF prologue generation : Ok Tested-by: Sachin Sant Can you please use the above mentioned id(without vnet) in the reported-by ? - Sachin
[PATCH] tools/perf: Fix bpf__probe to set bpf_prog_type type only if differs from the desired one
The test "BPF prologue generation" fails as below: Writing event: p:perf_bpf_probe/func _text+10423200 f_mode=+20(%gpr3):x32 offset=%gpr4:s64 orig=%gpr5:s32 In map_prologue, ntevs=1 mapping[0]=0 libbpf: prog 'bpf_func__null_lseek': BPF program load failed: Permission denied libbpf: prog 'bpf_func__null_lseek': -- BEGIN PROG LOAD LOG -- btf_vmlinux is malformed reg type unsupported for arg#0 function bpf_func__null_lseek#5 0: R1=ctx(off=0,imm=0) R10=fp0 ; 0: (57) r3 &= 2 R3 !read_ok processed 1 insns (limit 100) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0 -- END PROG LOAD LOG -- libbpf: prog 'bpf_func__null_lseek': failed to load: -13 libbpf: failed to load object '[bpf_prologue_test]' bpf: load objects failed: err=-13: (Permission denied) Failed to add events selected by BPF This fails occurs after this commit: commit d6e6286a12e7 ("libbpf: disassociate section handler on explicit bpf_program__set_type() call")' With this change, SEC_DEF handler libbpf which is determined initially based on program's SEC() is set to NULL. The change is made because sec_def is not valid when user sets the program type with bpf_program__set_type function. This commit also fixed bpf_prog_test_load() helper in selftests/bpf to force-set program type only if it differs from the desired one. The "bpf__probe" function in util/bpf-loader.c, also calls bpf_program__set_type to set bpf_prog_type. Add similar fix in here as well to avoid setting sec_def to NULL. Reported-by: Sachin Sant Signed-off-by: Athira Rajeev --- tools/perf/util/bpf-loader.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c index 44cde27d6389..b8e0b430e302 100644 --- a/tools/perf/util/bpf-loader.c +++ b/tools/perf/util/bpf-loader.c @@ -870,12 +870,14 @@ int bpf__probe(struct bpf_object *obj) goto out; } - if (priv->is_tp) { + if (priv->is_tp && bpf_program__type(prog) != BPF_PROG_TYPE_TRACEPOINT) { bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT); continue; } - bpf_program__set_type(prog, BPF_PROG_TYPE_KPROBE); + if (bpf_program__type(prog) != BPF_PROG_TYPE_KPROBE) + bpf_program__set_type(prog, BPF_PROG_TYPE_KPROBE); + pev = >pev; err = convert_perf_probe_events(pev, 1); -- 2.39.3