Re: [PATCH v3 bpf-next 08/10] bpf: Support ->fill_link_info for perf_event

[Yonghong Song <yhs@meta.com>]

On 6/12/23 8:16 AM, Yafang Shao 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 not permitted, the
> probed address will not be exposed, maintaining security measures.
> 
> A new enum bpf_link_perf_event_type is introduced to help the user
> understand which struct is relevant.
> 
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
>   include/uapi/linux/bpf.h       |  32 +++++++++++
>   kernel/bpf/syscall.c           | 124 +++++++++++++++++++++++++++++++++++++++++
>   tools/include/uapi/linux/bpf.h |  32 +++++++++++
>   3 files changed, 188 insertions(+)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 23691ea..8d4556e 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1056,6 +1056,16 @@ enum bpf_link_type {
>   	MAX_BPF_LINK_TYPE,
>   };
>   
> +enum bpf_perf_link_type {
> +	BPF_PERF_LINK_UNSPEC = 0,
> +	BPF_PERF_LINK_UPROBE = 1,
> +	BPF_PERF_LINK_KPROBE = 2,
> +	BPF_PERF_LINK_TRACEPOINT = 3,
> +	BPF_PERF_LINK_PERF_EVENT = 4,
> +
> +	MAX_BPF_LINK_PERF_EVENT_TYPE,
> +};
> +
>   /* cgroup-bpf attach flags used in BPF_PROG_ATTACH command
>    *
>    * NONE(default): No further bpf programs allowed in the subtree.
> @@ -6443,7 +6453,29 @@ struct bpf_link_info {
>   			__u32 count;
>   			__u32 flags;
>   		} kprobe_multi;
> +		struct {
> +			__u64 config;
> +			__u32 type;
> +		} perf_event; /* BPF_LINK_PERF_EVENT_PERF_EVENT */
> +		struct {
> +			__aligned_u64 file_name; /* in/out: buff ptr */
> +			__u32 name_len;
> +			__u32 offset;            /* offset from name */
> +			__u32 flags;
> +		} uprobe; /* BPF_LINK_PERF_EVENT_UPROBE */
> +		struct {
> +			__aligned_u64 func_name; /* in/out: buff ptr */
> +			__u32 name_len;
> +			__u32 offset;            /* offset from name */
> +			__u64 addr;
> +			__u32 flags;
> +		} kprobe; /* BPF_LINK_PERF_EVENT_KPROBE */
> +		struct {
> +			__aligned_u64 tp_name;   /* in/out: buff ptr */
> +			__u32 name_len;
> +		} tracepoint; /* BPF_LINK_PERF_EVENT_TRACEPOINT */
>   	};
> +	__u32 perf_link_type; /* enum bpf_perf_link_type */

I think put perf_link_type into each indivual struct is better.
It won't increase the bpf_link_info struct size. It will allow
extensions for all structs in the big union (raw_tracepoint,
tracing, cgroup, iter, ..., kprobe_multi, ...) etc.

>   } __attribute__((aligned(8)));
>   
>   /* User bpf_sock_addr struct to access socket fields and sockaddr struct passed
[...]