On Fri, Jun 9, 2023 at 2:57 AM Yafang Shao <laoar.shao@xxxxxxxxx> wrote:
>
> On Fri, Jun 9, 2023 at 5:53 PM Yafang Shao <laoar.shao@xxxxxxxxx> wrote:
> >
> > On Fri, Jun 9, 2023 at 7:12 AM Andrii Nakryiko
> > <andrii.nakryiko@xxxxxxxxx> wrote:
> > >
> > > On Thu, Jun 8, 2023 at 3:35 AM Yafang Shao <laoar.shao@xxxxxxxxx> wrote:
> > > >
> > > > By introducing support for ->fill_link_info to the perf_event link, users
> > > > gain the ability to inspect it using `bpftool link show`. While the current
> > > > approach involves accessing this information via `bpftool perf show`,
> > > > consolidating link information for all link types in one place offers
> > > > greater convenience. Additionally, this patch extends support to the
> > > > generic perf event, which is not currently accommodated by
> > > > `bpftool perf show`. While only the perf type and config are exposed to
> > > > userspace, other attributes such as sample_period and sample_freq are
> > > > ignored. It's important to note that if kptr_restrict is set to 2, the
> > > > probed address will not be exposed, maintaining security measures.
> > > >
> > > > Signed-off-by: Yafang Shao <laoar.shao@xxxxxxxxx>
> > > > ---
> > > > include/uapi/linux/bpf.h | 22 ++++++++++
> > > > kernel/bpf/syscall.c | 98 ++++++++++++++++++++++++++++++++++++++++++
> > > > tools/include/uapi/linux/bpf.h | 22 ++++++++++
> > > > 3 files changed, 142 insertions(+)
> > > >
> > > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > > > index d99cc16..c3b821d 100644
> > > > --- a/include/uapi/linux/bpf.h
> > > > +++ b/include/uapi/linux/bpf.h
> > > > @@ -6443,6 +6443,28 @@ struct bpf_link_info {
> > > > __u32 count;
> > > > __u8 retprobe;
> > > > } kprobe_multi;
> > > > + union {
> > > > + struct {
> > > > + /* The name is:
> > > > + * a) uprobe: file name
> > > > + * b) kprobe: kernel function
> > > > + */
> > > > + __aligned_u64 name; /* in/out: name buffer ptr */
> > > > + __u32 name_len;
> > > > + __u32 offset; /* offset from the name */
> > > > + __u64 addr;
> > > > + __u8 retprobe;
> > > > + } probe; /* uprobe, kprobe */
> > > > + struct {
> > > > + /* in/out: tracepoint name buffer ptr */
> > > > + __aligned_u64 tp_name;
> > > > + __u32 name_len;
> > > > + } tp; /* tracepoint */
> > > > + struct {
> > > > + __u64 config;
> > > > + __u32 type;
> > > > + } event; /* generic perf event */
> > >
> > > how should the user know which of those structs are relevant? we need
> > > some enum to specify what kind of perf_event link it is?
> > >
> >
> > Do you mean that we add a new field 'type' into the union perf_event,
> > as follows ?
> > union {
> > __u32 type;
> > struct {} probe; /* BPF_LINK_TYPE_PERF_EVENT_PROBE */
> > struct {} tp; /* BPF_LINK_TYPE_PERF_EVENT_TP */
> > struct {} event; /* BPF_LINK_TYPE_PERF_EVENT_EVENT */
> > };
> >
>
> Correct it:
>
> struct {
> __u32 type;
> union {
> struct {} probe; /* BPF_LINK_TYPE_PERF_EVENT_PROBE */
> struct {} tp; /* BPF_LINK_TYPE_PERF_EVENT_TP */
> struct {} event; /* BPF_LINK_TYPE_PERF_EVENT_EVENT */
> };
> } perf_event;
yes, something like this. Unless we want to leave perf_event {} to
mean really perf event only, while kprobe/uprobe/tracepoint should be
their own separate sections at the same level of nestedness as
perf_Event and other cases. Not sure.
>
> --
> Regards
> Yafang
