2024-03-11 23:30 UTC+0000 ~ Yonghong Song <yonghong.song@xxxxxxxxx> > > On 3/11/24 4:13 PM, Andrii Nakryiko wrote: >> On Mon, Mar 11, 2024 at 2:41 PM Yonghong Song >> <yonghong.song@xxxxxxxxx> 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@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..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!
