| Message ID | 20240311214116.2123875-1-yonghong.song@linux.dev (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next] bpftool: Fix missing pids during link show | expand |
On Mon, Mar 11, 2024 at 2:41 PM Yonghong Song <yonghong.song@linux.dev> wrote: > > 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, do a type cast from 'void *' to 'struct bpf_link *' for file->private_data > as in this patch, and 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@isovalent.com> > Signed-off-by: Yonghong Song <yonghong.song@linux.dev> > --- > 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..96ffcc4f0e67 100644 > --- a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c > +++ b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c > @@ -100,7 +100,7 @@ int iter(struct bpf_iter__task_file *ctx) > if (obj_type == BPF_OBJ_LINK && > bpf_core_enum_value_exists(enum bpf_link_type___local, > BPF_LINK_TYPE_PERF_EVENT___local)) { > - struct bpf_link *link = (struct bpf_link *) file->private_data; > + struct bpf_link *link = bpf_core_cast(file->private_data, struct bpf_link); bpf_core_cast() (i.e., bpf_rdonly_cast()) is newer feature, so relying on it in bpftool will make PID printing not work on older kernels that generally do support iter/task_file iterators. > > if (link->type == bpf_core_enum_value(enum bpf_link_type___local, the problem is that we dropped BPF_CORE_READ() here and went with `link->type`. Should we restore BPF_CORE_READ(link, type) here and solve the problem, while also supporting this feature on wider set of kernels? Quentin? > BPF_LINK_TYPE_PERF_EVENT___local)) { > -- > 2.43.0 >
On 3/11/24 4:13 PM, Andrii Nakryiko wrote: > On Mon, Mar 11, 2024 at 2:41 PM Yonghong Song <yonghong.song@linux.dev> wrote: >> 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, do a type cast from 'void *' to 'struct bpf_link *' for file->private_data >> as in this patch, and 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@isovalent.com> >> Signed-off-by: Yonghong Song <yonghong.song@linux.dev> >> --- >> 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..96ffcc4f0e67 100644 >> --- a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c >> +++ b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c >> @@ -100,7 +100,7 @@ int iter(struct bpf_iter__task_file *ctx) >> if (obj_type == BPF_OBJ_LINK && >> bpf_core_enum_value_exists(enum bpf_link_type___local, >> BPF_LINK_TYPE_PERF_EVENT___local)) { >> - struct bpf_link *link = (struct bpf_link *) file->private_data; >> + struct bpf_link *link = bpf_core_cast(file->private_data, struct bpf_link); > bpf_core_cast() (i.e., bpf_rdonly_cast()) is newer feature, so relying > on it in bpftool will make PID printing not work on older kernels that > generally do support iter/task_file iterators. > >> if (link->type == bpf_core_enum_value(enum bpf_link_type___local, > the problem is that we dropped BPF_CORE_READ() here and went with > `link->type`. Should we restore BPF_CORE_READ(link, type) here and > solve the problem, while also supporting this feature on wider set of > kernels? Quentin? Good point. Using the latest fancy feature might not be the best way as old kernels won't work and this is a strong argument. I will change to use BPF_CORE_READ in v2. > >> BPF_LINK_TYPE_PERF_EVENT___local)) { >> -- >> 2.43.0 >>
2024-03-11 23:30 UTC+0000 ~ Yonghong Song <yonghong.song@linux.dev> > > On 3/11/24 4:13 PM, Andrii Nakryiko wrote: >> On Mon, Mar 11, 2024 at 2:41 PM Yonghong Song >> <yonghong.song@linux.dev> wrote: >>> 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); */ I'm taking note to make these logs available when users pass the --debug flag (https://github.com/libbpf/bpftool/issues/137), there's no reason to hide them in that case. >>> >>> 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, do a type cast from 'void *' to 'struct bpf_link *' >>> for file->private_data >>> as in this patch, and 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@isovalent.com> >>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev> >>> --- >>> 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..96ffcc4f0e67 100644 >>> --- a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c >>> +++ b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c >>> @@ -100,7 +100,7 @@ int iter(struct bpf_iter__task_file *ctx) >>> if (obj_type == BPF_OBJ_LINK && >>> bpf_core_enum_value_exists(enum bpf_link_type___local, >>> >>> BPF_LINK_TYPE_PERF_EVENT___local)) { >>> - struct bpf_link *link = (struct bpf_link *) >>> file->private_data; >>> + struct bpf_link *link = >>> bpf_core_cast(file->private_data, struct bpf_link); >> bpf_core_cast() (i.e., bpf_rdonly_cast()) is newer feature, so relying >> on it in bpftool will make PID printing not work on older kernels that >> generally do support iter/task_file iterators. >> >>> if (link->type == bpf_core_enum_value(enum >>> bpf_link_type___local, >> the problem is that we dropped BPF_CORE_READ() here and went with >> `link->type`. Should we restore BPF_CORE_READ(link, type) here and >> solve the problem, while also supporting this feature on wider set of >> kernels? Quentin? > > Good point. Using the latest fancy feature might not be the best way as old > kernels won't work and this is a strong argument. I will change > to use BPF_CORE_READ in v2. Agreed, the whole point of commit 44ba7b30e84f that introduced the issue was to make bpftool work with older kernels. BPF_CORE_READ sounds like the right approach, thank you!
On 3/12/24 7:00 AM, Quentin Monnet wrote: > 2024-03-11 23:30 UTC+0000 ~ Yonghong Song <yonghong.song@linux.dev> >> On 3/11/24 4:13 PM, Andrii Nakryiko wrote: >>> On Mon, Mar 11, 2024 at 2:41 PM Yonghong Song >>> <yonghong.song@linux.dev> wrote: >>>> 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); */ > I'm taking note to make these logs available when users pass the --debug > flag (https://github.com/libbpf/bpftool/issues/137), there's no reason > to hide them in that case. Totally make sense as most kernels used in production should already have bpf_link and bpf_iter support already. [...]
diff --git a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c index 26004f0c5a6a..96ffcc4f0e67 100644 --- a/tools/bpf/bpftool/skeleton/pid_iter.bpf.c +++ b/tools/bpf/bpftool/skeleton/pid_iter.bpf.c @@ -100,7 +100,7 @@ int iter(struct bpf_iter__task_file *ctx) if (obj_type == BPF_OBJ_LINK && bpf_core_enum_value_exists(enum bpf_link_type___local, BPF_LINK_TYPE_PERF_EVENT___local)) { - struct bpf_link *link = (struct bpf_link *) file->private_data; + struct bpf_link *link = bpf_core_cast(file->private_data, struct bpf_link); if (link->type == bpf_core_enum_value(enum bpf_link_type___local, BPF_LINK_TYPE_PERF_EVENT___local)) {
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, do a type cast from 'void *' to 'struct bpf_link *' for file->private_data as in this patch, and 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@isovalent.com> Signed-off-by: Yonghong Song <yonghong.song@linux.dev> --- tools/bpf/bpftool/skeleton/pid_iter.bpf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)