Re: [PATCH bpf-next v2] bpftool: Fix missing pids during link show

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 12, 2024 at 8:24 AM Yonghong Song <yonghong.song@xxxxxxxxx> wrote:
>
>
> On 3/12/24 7:01 AM, Quentin Monnet wrote:
> > 2024-03-12 02:32 UTC+0000 ~ Yonghong Song <yonghong.song@xxxxxxxxx>
> >> Current 'bpftool link' command does not show pids, e.g.,
> >>    $ tools/build/bpftool/bpftool link
> >>    ...
> >>    4: tracing  prog 23
> >>          prog_type lsm  attach_type lsm_mac
> >>          target_obj_id 1  target_btf_id 31320
> >>
> >> Hack the following change to enable normal libbpf debug output,
> >>    --- a/tools/bpf/bpftool/pids.c
> >>    +++ b/tools/bpf/bpftool/pids.c
> >>    @@ -121,9 +121,9 @@ int build_obj_refs_table(struct hashmap **map, enum bpf_obj_type type)
> >>            /* we don't want output polluted with libbpf errors if bpf_iter is not
> >>             * supported
> >>             */
> >>    -       default_print = libbpf_set_print(libbpf_print_none);
> >>    +       /* default_print = libbpf_set_print(libbpf_print_none); */
> >>            err = pid_iter_bpf__load(skel);
> >>    -       libbpf_set_print(default_print);
> >>    +       /* libbpf_set_print(default_print); */
> >>
> >> Rerun the above bpftool command:
> >>    $ tools/build/bpftool/bpftool link
> >>    libbpf: prog 'iter': BPF program load failed: Permission denied
> >>    libbpf: prog 'iter': -- BEGIN PROG LOAD LOG --
> >>    0: R1=ctx() R10=fp0
> >>    ; struct task_struct *task = ctx->task; @ pid_iter.bpf.c:69
> >>    0: (79) r6 = *(u64 *)(r1 +8)          ; R1=ctx() R6_w=ptr_or_null_task_struct(id=1)
> >>    ; struct file *file = ctx->file; @ pid_iter.bpf.c:68
> >>    ...
> >>    ; struct bpf_link *link = (struct bpf_link *) file->private_data; @ pid_iter.bpf.c:103
> >>    80: (79) r3 = *(u64 *)(r8 +432)       ; R3_w=scalar() R8=ptr_file()
> >>    ; if (link->type == bpf_core_enum_value(enum bpf_link_type___local, @ pid_iter.bpf.c:105
> >>    81: (61) r1 = *(u32 *)(r3 +12)
> >>    R3 invalid mem access 'scalar'
> >>    processed 39 insns (limit 1000000) max_states_per_insn 0 total_states 3 peak_states 3 mark_read 2
> >>    -- END PROG LOAD LOG --
> >>    libbpf: prog 'iter': failed to load: -13
> >>    ...
> >>
> >> The 'file->private_data' returns a 'void' type and this caused subsequent 'link->type'
> >> (insn #81) failed in verification.
> >>
> >> To fix the issue, restore the previous BPF_CORE_READ so old kernels can also work.
> >> With this patch, the 'bpftool link' runs successfully with 'pids'.
> >>    $ tools/build/bpftool/bpftool link
> >>    ...
> >>    4: tracing  prog 23
> >>          prog_type lsm  attach_type lsm_mac
> >>          target_obj_id 1  target_btf_id 31320
> >>          pids systemd(1)
> >>
> >> Fixes: 44ba7b30e84f ("bpftool: Use a local copy of BPF_LINK_TYPE_PERF_EVENT in pid_iter.bpf.c")
> >> Cc: Quentin Monnet <quentin@xxxxxxxxxxxxx>
> >> Signed-off-by: Yonghong Song <yonghong.song@xxxxxxxxx>
> >> ---
> >>   tools/bpf/bpftool/skeleton/pid_iter.bpf.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
> >> index 26004f0c5a6a..1eb756f8d02e 100644
> >> --- a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
> >> +++ b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c
> >> @@ -102,7 +102,7 @@ int iter(struct bpf_iter__task_file *ctx)
> >>                                     BPF_LINK_TYPE_PERF_EVENT___local)) {
> >>              struct bpf_link *link = (struct bpf_link *) file->private_data;
> >>
> >> -            if (link->type == bpf_core_enum_value(enum bpf_link_type___local,
> >> +            if (BPF_CORE_READ(link, type) == bpf_core_enum_value(enum bpf_link_type___local,
> >>                                                    BPF_LINK_TYPE_PERF_EVENT___local)) {
> > Looks good, thank you! Could you please just realign the
> > BPF_LINK_TYPE_PERF_EVENT___local argument on the next line properly?
>
> Will do. thanks.
>

We can fix it up while applying, so don't send another revision. We
are waiting for trees to converge, so applying patches will be delayed
a bit.

> >
> > With that:
> >
> > Tested-by: Quentin Monnet <quentin@xxxxxxxxxxxxx>
> > Reviewed-by: Quentin Monnet <quentin@xxxxxxxxxxxxx>





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux