| Message ID | 20230528142027.5585-8-laoar.shao@gmail.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf: Support ->show_fdinfo and ->fill_link_info for kprobe prog | expand |
On Sun, May 28, 2023 at 02:20:26PM +0000, Yafang Shao wrote: > By adding support for ->fill_link_info to the perf_event link, users will > be able to inspect it using `bpftool link show`. While users can currently > access this information via `bpftool perf show`, consolidating the link > information for all link types in one place would be more convenient. > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > --- > include/uapi/linux/bpf.h | 6 ++++++ > kernel/bpf/syscall.c | 46 ++++++++++++++++++++++++++++++++++++++++++ > tools/include/uapi/linux/bpf.h | 6 ++++++ > 3 files changed, 58 insertions(+) > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index 6be9b1d..1f2be1d 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h > @@ -6438,6 +6438,12 @@ struct bpf_link_info { > __aligned_u64 addrs; > __u32 count; > } kprobe_multi; > + struct { > + __aligned_u64 name; > + __aligned_u64 addr; __aligned_u64 ? what is the reason? > + __u32 name_len; > + __u32 offset; > + } perf_event; > }; > } __attribute__((aligned(8))); > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > index 33a72ec..b12707e 100644 > --- a/kernel/bpf/syscall.c > +++ b/kernel/bpf/syscall.c > @@ -3329,10 +3329,56 @@ static void bpf_perf_link_show_fdinfo(const struct bpf_link *link, > seq_printf(seq, "offset:\t%llu\n", probe_offset); > } > > +static int bpf_perf_link_fill_link_info(const struct bpf_link *link, > + struct bpf_link_info *info) > +{ > + struct bpf_perf_link *perf_link = container_of(link, struct bpf_perf_link, link); > + char __user *ubuf = u64_to_user_ptr(info->perf_event.name); > + u32 ulen = info->perf_event.name_len; > + const struct perf_event *event; > + u64 probe_offset, probe_addr; > + u32 prog_id, fd_type; > + const char *buf; > + size_t len; > + int err; > + > + if (!ulen ^ !ubuf) > + return -EINVAL; > + if (!ubuf) > + return 0; > + > + event = perf_get_event(perf_link->perf_file); > + if (IS_ERR(event)) > + return PTR_ERR(event); > + > + err = bpf_get_perf_event_info(event, &prog_id, &fd_type, > + &buf, &probe_offset, > + &probe_addr); > + if (err) > + return err; > + > + len = strlen(buf); > + info->perf_event.name_len = len + 1; this use of name_len contradicts with patch 8. Is it 'in' or 'out' field? > + if (buf) { > + err = bpf_copy_to_user(ubuf, buf, ulen, len); > + if (err) > + return err; > + } else { > + char zero = '\0'; > + > + if (put_user(zero, ubuf)) > + return -EFAULT; > + } > + info->perf_event.addr = probe_addr; > + info->perf_event.offset = probe_offset; > + return 0; > +} > + > static const struct bpf_link_ops bpf_perf_link_lops = { > .release = bpf_perf_link_release, > .dealloc = bpf_perf_link_dealloc, > .show_fdinfo = bpf_perf_link_show_fdinfo, > + .fill_link_info = bpf_perf_link_fill_link_info, > }; > > static int bpf_perf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog) > diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h > index 6be9b1d..1f2be1d 100644 > --- a/tools/include/uapi/linux/bpf.h > +++ b/tools/include/uapi/linux/bpf.h > @@ -6438,6 +6438,12 @@ struct bpf_link_info { > __aligned_u64 addrs; > __u32 count; > } kprobe_multi; > + struct { > + __aligned_u64 name; > + __aligned_u64 addr; > + __u32 name_len; > + __u32 offset; > + } perf_event; > }; > } __attribute__((aligned(8))); > > -- > 1.8.3.1 >
On Wed, May 31, 2023 at 8:37 AM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Sun, May 28, 2023 at 02:20:26PM +0000, Yafang Shao wrote: > > By adding support for ->fill_link_info to the perf_event link, users will > > be able to inspect it using `bpftool link show`. While users can currently > > access this information via `bpftool perf show`, consolidating the link > > information for all link types in one place would be more convenient. > > > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > > --- > > include/uapi/linux/bpf.h | 6 ++++++ > > kernel/bpf/syscall.c | 46 ++++++++++++++++++++++++++++++++++++++++++ > > tools/include/uapi/linux/bpf.h | 6 ++++++ > > 3 files changed, 58 insertions(+) > > > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > > index 6be9b1d..1f2be1d 100644 > > --- a/include/uapi/linux/bpf.h > > +++ b/include/uapi/linux/bpf.h > > @@ -6438,6 +6438,12 @@ struct bpf_link_info { > > __aligned_u64 addrs; > > __u32 count; > > } kprobe_multi; > > + struct { > > + __aligned_u64 name; > > + __aligned_u64 addr; > > __aligned_u64 ? what is the reason? It is because of the copy-and-paste. Will use _u64 instead. > > > + __u32 name_len; > > + __u32 offset; > > + } perf_event; > > }; > > } __attribute__((aligned(8))); > > > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > > index 33a72ec..b12707e 100644 > > --- a/kernel/bpf/syscall.c > > +++ b/kernel/bpf/syscall.c > > @@ -3329,10 +3329,56 @@ static void bpf_perf_link_show_fdinfo(const struct bpf_link *link, > > seq_printf(seq, "offset:\t%llu\n", probe_offset); > > } > > > > +static int bpf_perf_link_fill_link_info(const struct bpf_link *link, > > + struct bpf_link_info *info) > > +{ > > + struct bpf_perf_link *perf_link = container_of(link, struct bpf_perf_link, link); > > + char __user *ubuf = u64_to_user_ptr(info->perf_event.name); > > + u32 ulen = info->perf_event.name_len; > > + const struct perf_event *event; > > + u64 probe_offset, probe_addr; > > + u32 prog_id, fd_type; > > + const char *buf; > > + size_t len; > > + int err; > > + > > + if (!ulen ^ !ubuf) > > + return -EINVAL; > > + if (!ubuf) > > + return 0; > > + > > + event = perf_get_event(perf_link->perf_file); > > + if (IS_ERR(event)) > > + return PTR_ERR(event); > > + > > + err = bpf_get_perf_event_info(event, &prog_id, &fd_type, > > + &buf, &probe_offset, > > + &probe_addr); > > + if (err) > > + return err; > > + > > + len = strlen(buf); > > + info->perf_event.name_len = len + 1; > > this use of name_len contradicts with patch 8. > Is it 'in' or 'out' field? My mistake. I should remove this sentence. The reason I didn't do it the same with bpf_raw_tp_link_fill_link_info() is that if we return the buf length to the userspace when the ubuf is NULL, we have to call bpf_get_perf_event_info() multiple times.
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index 6be9b1d..1f2be1d 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -6438,6 +6438,12 @@ struct bpf_link_info { __aligned_u64 addrs; __u32 count; } kprobe_multi; + struct { + __aligned_u64 name; + __aligned_u64 addr; + __u32 name_len; + __u32 offset; + } perf_event; }; } __attribute__((aligned(8))); diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 33a72ec..b12707e 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -3329,10 +3329,56 @@ static void bpf_perf_link_show_fdinfo(const struct bpf_link *link, seq_printf(seq, "offset:\t%llu\n", probe_offset); } +static int bpf_perf_link_fill_link_info(const struct bpf_link *link, + struct bpf_link_info *info) +{ + struct bpf_perf_link *perf_link = container_of(link, struct bpf_perf_link, link); + char __user *ubuf = u64_to_user_ptr(info->perf_event.name); + u32 ulen = info->perf_event.name_len; + const struct perf_event *event; + u64 probe_offset, probe_addr; + u32 prog_id, fd_type; + const char *buf; + size_t len; + int err; + + if (!ulen ^ !ubuf) + return -EINVAL; + if (!ubuf) + return 0; + + event = perf_get_event(perf_link->perf_file); + if (IS_ERR(event)) + return PTR_ERR(event); + + err = bpf_get_perf_event_info(event, &prog_id, &fd_type, + &buf, &probe_offset, + &probe_addr); + if (err) + return err; + + len = strlen(buf); + info->perf_event.name_len = len + 1; + if (buf) { + err = bpf_copy_to_user(ubuf, buf, ulen, len); + if (err) + return err; + } else { + char zero = '\0'; + + if (put_user(zero, ubuf)) + return -EFAULT; + } + info->perf_event.addr = probe_addr; + info->perf_event.offset = probe_offset; + return 0; +} + static const struct bpf_link_ops bpf_perf_link_lops = { .release = bpf_perf_link_release, .dealloc = bpf_perf_link_dealloc, .show_fdinfo = bpf_perf_link_show_fdinfo, + .fill_link_info = bpf_perf_link_fill_link_info, }; static int bpf_perf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog) diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index 6be9b1d..1f2be1d 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -6438,6 +6438,12 @@ struct bpf_link_info { __aligned_u64 addrs; __u32 count; } kprobe_multi; + struct { + __aligned_u64 name; + __aligned_u64 addr; + __u32 name_len; + __u32 offset; + } perf_event; }; } __attribute__((aligned(8)));
By adding support for ->fill_link_info to the perf_event link, users will be able to inspect it using `bpftool link show`. While users can currently access this information via `bpftool perf show`, consolidating the link information for all link types in one place would be more convenient. Signed-off-by: Yafang Shao <laoar.shao@gmail.com> --- include/uapi/linux/bpf.h | 6 ++++++ kernel/bpf/syscall.c | 46 ++++++++++++++++++++++++++++++++++++++++++ tools/include/uapi/linux/bpf.h | 6 ++++++ 3 files changed, 58 insertions(+)