2023-06-12 15:16 UTC+0000 ~ Yafang Shao <laoar.shao@xxxxxxxxx>
> Enhance bpftool to display comprehensive information about exposed
> perf_event links, covering uprobe, kprobe, tracepoint, and generic perf
> event. The resulting output will include the following details:
>
> $ tools/bpf/bpftool/bpftool link show
> 3: perf_event prog 14
> event_type software event_config cpu-clock
> bpf_cookie 0
> pids perf_event(1379330)
> 4: perf_event prog 14
> event_type hw-cache event_config LLC-load-misses
> bpf_cookie 0
> pids perf_event(1379330)
> 5: perf_event prog 14
> event_type hardware event_config cpu-cycles
> bpf_cookie 0
> pids perf_event(1379330)
> 6: perf_event prog 20
> retprobe 0 file_name /home/yafang/bpf/uprobe/a.out offset 0x1338
> bpf_cookie 0
> pids uprobe(1379706)
> 7: perf_event prog 21
> retprobe 1 file_name /home/yafang/bpf/uprobe/a.out offset 0x1338
> bpf_cookie 0
> pids uprobe(1379706)
> 8: perf_event prog 27
> tp_name sched_switch
> bpf_cookie 0
> pids tracepoint(1381734)
> 10: perf_event prog 43
> retprobe 0 func_name kernel_clone addr ffffffffad0a9660
Could we swap the name and the address, for consistency with the
kprobe_multi case?
Also do we really need the "_name" suffix in "func_name" and "file_name"
for plain output? I don't mind in JSON, but I think the result is a bit
long for plain output.
> bpf_cookie 0
> pids kprobe(1384186)
> 11: perf_event prog 41
> retprobe 1 func_name kernel_clone addr ffffffffad0a9660
> bpf_cookie 0
> pids kprobe(1384186)
>
> $ tools/bpf/bpftool/bpftool link show -j
> [{"id":3,"type":"perf_event","prog_id":14,"event_type":"software","event_config":"cpu-clock","bpf_cookie":0,"pids":[{"pid":1379330,"comm":"perf_event"}]},{"id":4,"type":"perf_event","prog_id":14,"event_type":"hw-cache","event_config":"LLC-load-misses","bpf_cookie":0,"pids":[{"pid":1379330,"comm":"perf_event"}]},{"id":5,"type":"perf_event","prog_id":14,"event_type":"hardware","event_config":"cpu-cycles","bpf_cookie":0,"pids":[{"pid":1379330,"comm":"perf_event"}]},{"id":6,"type":"perf_event","prog_id":20,"retprobe":0,"file_name":"/home/yafang/bpf/uprobe/a.out","offset":4920,"bpf_cookie":0,"pids":[{"pid":1379706,"comm":"uprobe"}]},{"id":7,"type":"perf_event","prog_id":21,"retprobe":1,"file_name":"/home/yafang/bpf/uprobe/a.out","offset":4920,"bpf_cookie":0,"pids":[{"pid":1379706,"comm":"uprobe"}]},{"id":8,"type":"perf_event","prog_id":27,"tp_name":"sched_switch","bpf_cookie":0,"pids":[{"pid":1381734,"comm":"tracepoint"}]},{"id":10,"type":"perf_event","prog_id":43,"retprobe":0,"func_name":"kernel_clone","offset":0,"addr":18446744072317736544,"bpf_cookie":0,"pids":[{"pid":1384186,"comm":"kprobe"}]},{"id":11,"type":"perf_event","prog_id":41,"retprobe":1,"func_name":"kernel_clone","offset":0,"addr":18446744072317736544,"bpf_cookie":0,"pids":[{"pid":1384186,"comm":"kprobe"}]}]
>
> For generic perf events, the displayed information in bpftool is limited to
> the type and configuration, while other attributes such as sample_period,
> sample_freq, etc., are not included.
>
> The kernel function address won't be exposed if it is not permitted by
> kptr_restrict. The result as follows when kptr_restrict is 2.
>
> $ tools/bpf/bpftool/bpftool link show
> 3: perf_event prog 14
> event_type software event_config cpu-clock
> 4: perf_event prog 14
> event_type hw-cache event_config LLC-load-misses
> 5: perf_event prog 14
> event_type hardware event_config cpu-cycles
> 6: perf_event prog 20
> retprobe 0 file_name /home/yafang/bpf/uprobe/a.out offset 0x1338
> 7: perf_event prog 21
> retprobe 1 file_name /home/yafang/bpf/uprobe/a.out offset 0x1338
> 8: perf_event prog 27
> tp_name sched_switch
> 10: perf_event prog 43
> retprobe 0 func_name kernel_clone
> 11: perf_event prog 41
> retprobe 1 func_name kernel_clone
>
> Signed-off-by: Yafang Shao <laoar.shao@xxxxxxxxx>
> ---
> tools/bpf/bpftool/link.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 213 insertions(+)
>
> diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
> index 0015582..c16f71d 100644
> --- a/tools/bpf/bpftool/link.c
> +++ b/tools/bpf/bpftool/link.c
> @@ -15,6 +15,7 @@
> #include "json_writer.h"
> #include "main.h"
> #include "xlated_dumper.h"
> +#include "perf.h"
>
> static struct hashmap *link_table;
> static struct dump_data dd = {};
> @@ -207,6 +208,109 @@ static int cmp_u64(const void *A, const void *B)
> jsonw_end_array(json_wtr);
> }
>
> +static void
> +show_perf_event_kprobe_json(struct bpf_link_info *info, json_writer_t *wtr)
> +{
> + jsonw_uint_field(wtr, "retprobe", info->kprobe.flags & 0x1);
"retprobe" should likely be a boolean here too (and below), I don't see
them taking any other values than 0 or 1?
> + jsonw_string_field(wtr, "func_name",
> + u64_to_ptr(info->kprobe.func_name));
> + jsonw_uint_field(wtr, "offset", info->kprobe.offset);
> + jsonw_uint_field(wtr, "addr", info->kprobe.addr);
> +}
> +
> +static void
> +show_perf_event_uprobe_json(struct bpf_link_info *info, json_writer_t *wtr)
> +{
> + jsonw_uint_field(wtr, "retprobe", info->uprobe.flags & 0x1);
> + jsonw_string_field(wtr, "file_name",
> + u64_to_ptr(info->uprobe.file_name));
> + jsonw_uint_field(wtr, "offset", info->uprobe.offset);
> +}
> +
> +static void
> +show_perf_event_tp_json(struct bpf_link_info *info, json_writer_t *wtr)
> +{
> + jsonw_string_field(wtr, "tp_name",
> + u64_to_ptr(info->tracepoint.tp_name));
> +}
> +
> +static const char *perf_config_hw_cache_str(__u64 config)
The returned "str" is not a "const char *"? Why not simply a "char *"
and avoiding the cast when we free() it?
> +{
> +#define PERF_HW_CACHE_LEN 128
Let's move the #define to the top of the file, please.
> + const char *hw_cache, *result, *op;
> + char *str = malloc(PERF_HW_CACHE_LEN);
> +
> + if (!str) {
> + p_err("mem alloc failed");
> + return NULL;
> + }
> + hw_cache = perf_hw_cache_str(config & 0xff);
> + if (hw_cache)
> + snprintf(str, PERF_HW_CACHE_LEN, "%s-", hw_cache);
> + else
> + snprintf(str, PERF_HW_CACHE_LEN, "%lld-", config & 0xff);
> + op = perf_hw_cache_op_str((config >> 8) & 0xff);
> + if (op)
> + snprintf(str + strlen(str), PERF_HW_CACHE_LEN - strlen(str),
> + "%s-", op);
> + else
> + snprintf(str + strlen(str), PERF_HW_CACHE_LEN - strlen(str),
> + "%lld-", (config >> 8) & 0xff);
> + result = perf_hw_cache_op_result_str(config >> 16);
> + if (result)
> + snprintf(str + strlen(str), PERF_HW_CACHE_LEN - strlen(str),
> + "%s", result);
> + else
> + snprintf(str + strlen(str), PERF_HW_CACHE_LEN - strlen(str),
> + "%lld", config >> 16);
> +
> + return str;
> +}
> +
> +static const char *perf_config_str(__u32 type, __u64 config)
> +{
> + const char *perf_config;
> +
> + switch (type) {
> + case PERF_TYPE_HARDWARE:
> + perf_config = perf_hw_str(config);
> + break;
> + case PERF_TYPE_SOFTWARE:
> + perf_config = perf_sw_str(config);
> + break;
> + case PERF_TYPE_HW_CACHE:
> + perf_config = perf_config_hw_cache_str(config);
> + break;
> + default:
> + perf_config = NULL;
> + break;
> + }
> + return perf_config;
> +}
> +
> +static void
> +show_perf_event_event_json(struct bpf_link_info *info, json_writer_t *wtr)
> +{
> + __u64 config = info->perf_event.config;
> + __u32 type = info->perf_event.type;
> + const char *perf_type, *perf_config;
> +
> + perf_type = perf_type_str(type);
> + if (perf_type)
> + jsonw_string_field(wtr, "event_type", perf_type);
> + else
> + jsonw_uint_field(wtr, "event_type", type);
> +
> + perf_config = perf_config_str(type, config);
> + if (perf_config)
> + jsonw_string_field(wtr, "event_config", perf_config);
> + else
> + jsonw_uint_field(wtr, "event_config", config);
> +
> + if (type == PERF_TYPE_HW_CACHE && perf_config)
> + free((void *)perf_config);
> +}
> +
> static int show_link_close_json(int fd, struct bpf_link_info *info)
> {
> struct bpf_prog_info prog_info;
> @@ -262,6 +366,16 @@ static int show_link_close_json(int fd, struct bpf_link_info *info)
> case BPF_LINK_TYPE_KPROBE_MULTI:
> show_kprobe_multi_json(info, json_wtr);
> break;
> + case BPF_LINK_TYPE_PERF_EVENT:
> + if (info->perf_link_type == BPF_PERF_LINK_PERF_EVENT)
> + show_perf_event_event_json(info, json_wtr);
> + else if (info->perf_link_type == BPF_PERF_LINK_TRACEPOINT)
> + show_perf_event_tp_json(info, json_wtr);
> + else if (info->perf_link_type == BPF_PERF_LINK_KPROBE)
> + show_perf_event_kprobe_json(info, json_wtr);
> + else if (info->perf_link_type == BPF_PERF_LINK_UPROBE)
> + show_perf_event_uprobe_json(info, json_wtr);
It would be clearer to me with another switch/case I think (same for
plain output), but I don't mind much.
> + break;
> default:
> break;
> }
> @@ -433,6 +547,71 @@ static void show_kprobe_multi_plain(struct bpf_link_info *info)
> }
> }
>
> +static void show_perf_event_kprobe_plain(struct bpf_link_info *info)
> +{
> + const char *buf;
> + __u32 retprobe;
> +
> + buf = (const char *)u64_to_ptr(info->kprobe.func_name);
> + if (buf[0] == '\0' && !info->kprobe.addr)
> + return;
> +
> + retprobe = info->kprobe.flags & 0x1;
> + printf("\n\tretprobe %u func_name %s ", retprobe, buf);
> + if (info->kprobe.offset)
> + printf("offset %#x ", info->kprobe.offset);
> + if (info->kprobe.addr)
> + printf("addr %llx ", info->kprobe.addr);
> +}
> +
> +static void show_perf_event_uprobe_plain(struct bpf_link_info *info)
> +{
> + const char *buf;
> + __u32 retprobe;
> +
> + buf = (const char *)u64_to_ptr(info->uprobe.file_name);
> + if (buf[0] == '\0')
> + return;
> +
> + retprobe = info->uprobe.flags & 0x1;
> + printf("\n\tretprobe %u file_name %s ", retprobe, buf);
> + if (info->uprobe.offset)
> + printf("offset %#x ", info->kprobe.offset);
> +}
> +
> +static void show_perf_event_tp_plain(struct bpf_link_info *info)
> +{
> + const char *buf;
> +
> + buf = (const char *)u64_to_ptr(info->tracepoint.tp_name);
> + if (buf[0] == '\0')
> + return;
> +
> + printf("\n\ttp_name %s ", buf);
> +}
> +
> +static void show_perf_event_event_plain(struct bpf_link_info *info)
> +{
> + __u64 config = info->perf_event.config;
> + __u32 type = info->perf_event.type;
> + const char *perf_type, *perf_config;
> +
> + perf_type = perf_type_str(type);
> + if (perf_type)
> + printf("\n\tevent_type %s ", perf_type);
> + else
> + printf("\n\tevent_type %u ", type);
> +
> + perf_config = perf_config_str(type, config);
> + if (perf_config)
> + printf("event_config %s ", perf_config);
> + else
> + printf("event_config %llu ", config);
> +
> + if (type == PERF_TYPE_HW_CACHE && perf_config)
> + free((void *)perf_config);
> +}
> +
> static int show_link_close_plain(int fd, struct bpf_link_info *info)
> {
> struct bpf_prog_info prog_info;
> @@ -481,6 +660,16 @@ static int show_link_close_plain(int fd, struct bpf_link_info *info)
> case BPF_LINK_TYPE_KPROBE_MULTI:
> show_kprobe_multi_plain(info);
> break;
> + case BPF_LINK_TYPE_PERF_EVENT:
> + if (info->perf_link_type == BPF_PERF_LINK_PERF_EVENT)
> + show_perf_event_event_plain(info);
> + else if (info->perf_link_type == BPF_PERF_LINK_TRACEPOINT)
> + show_perf_event_tp_plain(info);
> + else if (info->perf_link_type == BPF_PERF_LINK_KPROBE)
> + show_perf_event_kprobe_plain(info);
> + else if (info->perf_link_type == BPF_PERF_LINK_UPROBE)
> + show_perf_event_uprobe_plain(info);
> + break;
> default:
> break;
> }
> @@ -508,6 +697,7 @@ static int do_show_link(int fd)
> int err;
>
> memset(&info, 0, sizeof(info));
> + buf[0] = '\0';
> again:
> err = bpf_link_get_info_by_fd(fd, &info, &len);
> if (err) {
> @@ -542,7 +732,30 @@ static int do_show_link(int fd)
> goto again;
> }
> }
> + if (info.type == BPF_LINK_TYPE_PERF_EVENT) {
> + if (info.perf_link_type == BPF_PERF_LINK_PERF_EVENT)
> + goto out;
> + if (info.perf_link_type == BPF_PERF_LINK_TRACEPOINT &&
> + !info.tracepoint.tp_name) {
> + info.tracepoint.tp_name = (unsigned long)&buf;
> + info.tracepoint.name_len = sizeof(buf);
> + goto again;
> + }
> + if (info.perf_link_type == BPF_PERF_LINK_KPROBE &&
> + !info.kprobe.func_name) {
> + info.kprobe.func_name = (unsigned long)&buf;
> + info.kprobe.name_len = sizeof(buf);
> + goto again;
> + }
> + if (info.perf_link_type == BPF_PERF_LINK_UPROBE &&
> + !info.uprobe.file_name) {
> + info.uprobe.file_name = (unsigned long)&buf;
> + info.uprobe.name_len = sizeof(buf);
Maybe increase the size of buf to accommodate for long paths?
> + goto again;
> + }
> + }
>
> +out:
> if (json_output)
> show_link_close_json(fd, &info);
> else
Thanks for this work!
