* [PATCH v3 bpf-next 00/10] bpf: Support ->fill_link_info for kprobe_multi and perf_event links
@ 2023-06-12 15:15 Yafang Shao
2023-06-12 15:15 ` [PATCH v3 bpf-next 01/10] bpf: Support ->fill_link_info for kprobe_multi Yafang Shao
` (10 more replies)
0 siblings, 11 replies; 47+ messages in thread
From: Yafang Shao @ 2023-06-12 15:15 UTC (permalink / raw)
To: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat
Cc: bpf, linux-trace-kernel, Yafang Shao
This patchset enhances the usability of kprobe_multi programs by introducing
support for ->fill_link_info. This allows users to easily determine the
probed functions associated with a kprobe_multi program. While
`bpftool perf show` already provides information about functions probed by
perf_event programs, supporting ->fill_link_info ensures consistent access to
this information across all bpf links.
In addition, this patch extends support to generic perf events, which are
currently not covered by `bpftool perf show`. While userspace is exposed to
only the perf type and config, other attributes such as sample_period and
sample_freq are disregarded.
To ensure accurate identification of probed functions, it is preferable to
expose the address directly rather than relying solely on the symbol name.
However, this implementation respects the kptr_restrict setting and avoids
exposing the address if it is not permitted.
v2->v3:
- Expose flags instead of retporbe (Andrii)
- Simplify the check on kmulti_link->cnt (Andrii)
- Use kallsyms_show_value() instead (Andrii)
- Show also the module name for kprobe_multi (Andrii)
- Add new enum bpf_perf_link_type (Andrii)
- Move perf event names into bpftool (Andrii, Quentin, Jiri)
- Keep perf event names in sync with perf tools (Jiri)
v1->v2:
- Fix sparse warning (Stanislav, lkp@intel.com)
- Fix BPF CI build error
- Reuse kernel_syms_load() (Alexei)
- Print 'name' instead of 'func' (Alexei)
- Show whether the probe is retprobe or not (Andrii)
- Add comment for the meaning of perf_event name (Andrii)
- Add support for generic perf event
- Adhere to the kptr_restrict setting
RFC->v1:
- Use a single copy_to_user() instead (Jiri)
- Show also the symbol name in bpftool (Quentin, Alexei)
- Use calloc() instead of malloc() in bpftool (Quentin)
- Avoid having conditional entries in the JSON output (Quentin)
- Drop ->show_fdinfo (Alexei)
- Use __u64 instead of __aligned_u64 for the field addr (Alexei)
- Avoid the contradiction in perf_event name length (Alexei)
- Address a build warning reported by kernel test robot <lkp@intel.com>
Yafang Shao (10):
bpf: Support ->fill_link_info for kprobe_multi
bpftool: Dump the kernel symbol's module name
bpftool: Show probed function in kprobe_multi link info
bpf: Protect probed address based on kptr_restrict setting
bpf: Clear the probe_addr for uprobe
bpf: Expose symbol's respective address
bpf: Add a common helper bpf_copy_to_user()
bpf: Support ->fill_link_info for perf_event
bpftool: Add perf event names
bpftool: Show probed function in perf_event link info
include/uapi/linux/bpf.h | 37 +++++
kernel/bpf/syscall.c | 158 +++++++++++++++++--
kernel/trace/bpf_trace.c | 32 +++-
kernel/trace/trace_kprobe.c | 7 +-
tools/bpf/bpftool/link.c | 322 +++++++++++++++++++++++++++++++++++++-
tools/bpf/bpftool/perf.c | 107 +++++++++++++
tools/bpf/bpftool/perf.h | 11 ++
tools/bpf/bpftool/xlated_dumper.c | 6 +-
tools/bpf/bpftool/xlated_dumper.h | 2 +
tools/include/uapi/linux/bpf.h | 37 +++++
10 files changed, 700 insertions(+), 19 deletions(-)
create mode 100644 tools/bpf/bpftool/perf.h
--
1.8.3.1
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v3 bpf-next 01/10] bpf: Support ->fill_link_info for kprobe_multi
2023-06-12 15:15 [PATCH v3 bpf-next 00/10] bpf: Support ->fill_link_info for kprobe_multi and perf_event links Yafang Shao
@ 2023-06-12 15:15 ` Yafang Shao
2023-06-15 8:29 ` Jiri Olsa
2023-06-16 17:24 ` Andrii Nakryiko
2023-06-12 15:16 ` [PATCH v3 bpf-next 02/10] bpftool: Dump the kernel symbol's module name Yafang Shao
` (9 subsequent siblings)
10 siblings, 2 replies; 47+ messages in thread
From: Yafang Shao @ 2023-06-12 15:15 UTC (permalink / raw)
To: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat
Cc: bpf, linux-trace-kernel, Yafang Shao
With the addition of support for fill_link_info to the kprobe_multi link,
users will gain the ability to inspect it conveniently using the
`bpftool link show`. This enhancement provides valuable information to the
user, including the count of probed functions and their respective
addresses. It's important to note that if the kptr_restrict setting is not
permitted, the probed address will not be exposed, ensuring security.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
include/uapi/linux/bpf.h | 5 +++++
kernel/trace/bpf_trace.c | 28 ++++++++++++++++++++++++++++
tools/include/uapi/linux/bpf.h | 5 +++++
3 files changed, 38 insertions(+)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index a7b5e91..23691ea 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -6438,6 +6438,11 @@ struct bpf_link_info {
__s32 priority;
__u32 flags;
} netfilter;
+ struct {
+ __aligned_u64 addrs; /* in/out: addresses buffer ptr */
+ __u32 count;
+ __u32 flags;
+ } kprobe_multi;
};
} __attribute__((aligned(8)));
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 2bc41e6..742047c 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2548,9 +2548,36 @@ static void bpf_kprobe_multi_link_dealloc(struct bpf_link *link)
kfree(kmulti_link);
}
+static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link,
+ struct bpf_link_info *info)
+{
+ u64 __user *uaddrs = u64_to_user_ptr(info->kprobe_multi.addrs);
+ struct bpf_kprobe_multi_link *kmulti_link;
+ u32 ucount = info->kprobe_multi.count;
+
+ if (!uaddrs ^ !ucount)
+ return -EINVAL;
+
+ kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link);
+ if (!uaddrs) {
+ info->kprobe_multi.count = kmulti_link->cnt;
+ return 0;
+ }
+
+ if (ucount < kmulti_link->cnt)
+ return -EINVAL;
+ info->kprobe_multi.flags = kmulti_link->fp.flags;
+ if (!kallsyms_show_value(current_cred()))
+ return 0;
+ if (copy_to_user(uaddrs, kmulti_link->addrs, ucount * sizeof(u64)))
+ return -EFAULT;
+ return 0;
+}
+
static const struct bpf_link_ops bpf_kprobe_multi_link_lops = {
.release = bpf_kprobe_multi_link_release,
.dealloc = bpf_kprobe_multi_link_dealloc,
+ .fill_link_info = bpf_kprobe_multi_link_fill_link_info,
};
static void bpf_kprobe_multi_cookie_swap(void *a, void *b, int size, const void *priv)
@@ -2890,6 +2917,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
return err;
}
+ link->fp.flags = flags;
return bpf_link_settle(&link_primer);
error:
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index a7b5e91..23691ea 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -6438,6 +6438,11 @@ struct bpf_link_info {
__s32 priority;
__u32 flags;
} netfilter;
+ struct {
+ __aligned_u64 addrs; /* in/out: addresses buffer ptr */
+ __u32 count;
+ __u32 flags;
+ } kprobe_multi;
};
} __attribute__((aligned(8)));
--
1.8.3.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v3 bpf-next 02/10] bpftool: Dump the kernel symbol's module name
2023-06-12 15:15 [PATCH v3 bpf-next 00/10] bpf: Support ->fill_link_info for kprobe_multi and perf_event links Yafang Shao
2023-06-12 15:15 ` [PATCH v3 bpf-next 01/10] bpf: Support ->fill_link_info for kprobe_multi Yafang Shao
@ 2023-06-12 15:16 ` Yafang Shao
2023-06-13 13:41 ` Quentin Monnet
2023-06-16 17:25 ` Andrii Nakryiko
2023-06-12 15:16 ` [PATCH v3 bpf-next 03/10] bpftool: Show probed function in kprobe_multi link info Yafang Shao
` (8 subsequent siblings)
10 siblings, 2 replies; 47+ messages in thread
From: Yafang Shao @ 2023-06-12 15:16 UTC (permalink / raw)
To: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat
Cc: bpf, linux-trace-kernel, Yafang Shao
If the kernel symbol is in a module, we will dump the module name as
well.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
tools/bpf/bpftool/xlated_dumper.c | 6 +++++-
tools/bpf/bpftool/xlated_dumper.h | 2 ++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/xlated_dumper.c b/tools/bpf/bpftool/xlated_dumper.c
index da608e1..dd917f3 100644
--- a/tools/bpf/bpftool/xlated_dumper.c
+++ b/tools/bpf/bpftool/xlated_dumper.c
@@ -46,7 +46,11 @@ void kernel_syms_load(struct dump_data *dd)
}
dd->sym_mapping = tmp;
sym = &dd->sym_mapping[dd->sym_count];
- if (sscanf(buff, "%p %*c %s", &address, sym->name) != 2)
+
+ /* module is optional */
+ sym->module[0] = '\0';
+ if (sscanf(buff, "%p %*c %s %s", &address, sym->name,
+ sym->module) < 2)
continue;
sym->address = (unsigned long)address;
if (!strcmp(sym->name, "__bpf_call_base")) {
diff --git a/tools/bpf/bpftool/xlated_dumper.h b/tools/bpf/bpftool/xlated_dumper.h
index 9a94637..5df8025 100644
--- a/tools/bpf/bpftool/xlated_dumper.h
+++ b/tools/bpf/bpftool/xlated_dumper.h
@@ -5,12 +5,14 @@
#define __BPF_TOOL_XLATED_DUMPER_H
#define SYM_MAX_NAME 256
+#define MODULE_NAME_LEN 64
struct bpf_prog_linfo;
struct kernel_sym {
unsigned long address;
char name[SYM_MAX_NAME];
+ char module[MODULE_NAME_LEN];
};
struct dump_data {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v3 bpf-next 03/10] bpftool: Show probed function in kprobe_multi link info
2023-06-12 15:15 [PATCH v3 bpf-next 00/10] bpf: Support ->fill_link_info for kprobe_multi and perf_event links Yafang Shao
2023-06-12 15:15 ` [PATCH v3 bpf-next 01/10] bpf: Support ->fill_link_info for kprobe_multi Yafang Shao
2023-06-12 15:16 ` [PATCH v3 bpf-next 02/10] bpftool: Dump the kernel symbol's module name Yafang Shao
@ 2023-06-12 15:16 ` Yafang Shao
2023-06-13 13:41 ` Quentin Monnet
` (2 more replies)
2023-06-12 15:16 ` [PATCH v3 bpf-next 04/10] bpf: Protect probed address based on kptr_restrict setting Yafang Shao
` (7 subsequent siblings)
10 siblings, 3 replies; 47+ messages in thread
From: Yafang Shao @ 2023-06-12 15:16 UTC (permalink / raw)
To: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat
Cc: bpf, linux-trace-kernel, Yafang Shao
Show the already expose kprobe_multi link info in bpftool. The result as
follows,
52: kprobe_multi prog 381
retprobe 0 func_cnt 7
addrs ffffffff9ec44f20 funcs schedule_timeout_interruptible
ffffffff9ec44f60 schedule_timeout_killable
ffffffff9ec44fa0 schedule_timeout_uninterruptible
ffffffff9ec44fe0 schedule_timeout_idle
ffffffffc09468d0 xfs_trans_get_efd [xfs]
ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
pids kprobe_multi(559862)
53: kprobe_multi prog 381
retprobe 1 func_cnt 7
addrs ffffffff9ec44f20 funcs schedule_timeout_interruptible
ffffffff9ec44f60 schedule_timeout_killable
ffffffff9ec44fa0 schedule_timeout_uninterruptible
ffffffff9ec44fe0 schedule_timeout_idle
ffffffffc09468d0 xfs_trans_get_efd [xfs]
ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
pids kprobe_multi(559862)
$ tools/bpf/bpftool/bpftool link show -j
[{"id":52,"type":"kprobe_multi","prog_id":381,"retprobe":0,"func_cnt":7,"funcs":[{"addr":18446744072078249760,"func":"schedule_timeout_interruptible","module":""},{"addr":18446744072078249824,"func":"schedule_timeout_killable","module":""},{"addr":18446744072078249888,"func":"schedule_timeout_uninterruptible","module":""},{"addr":18446744072078249952,"func":"schedule_timeout_idle","module":""},{"addr":18446744072645535952,"func":"xfs_trans_get_efd","module":"[xfs]"},{"addr":18446744072645589520,"func":"xfs_trans_get_buf_map","module":"[xfs]"},{"addr":18446744072645604128,"func":"xfs_trans_get_dqtrx","module":"[xfs]"}],"pids":[{"pid":559862,"comm":"kprobe_multi"}]},{"id":53,"type":"kprobe_multi","prog_id":381,"retprobe":1,"func_cnt":7,"funcs":[{"addr":18446744072078249760,"func":"schedule_timeout_interruptible","module":""},{"addr":18446744072078249824,"func":"schedule_timeout_killable","module":""},{"addr":18446744072078249888,"func":"schedule_timeout_uninterruptible","module":""},{"addr":18446744072078249952,"func":"schedule_timeout_idle","module":""},{"addr":18446744072645535952,"func":"xfs_trans_get_efd","module":"[xfs]"},{"addr":18446744072645589520,"func":"xfs_trans_get_buf_map","module":"[xfs]"},{"addr":18446744072645604128,"func":"xfs_trans_get_dqtrx","module":"[xfs]"}],"pids":[{"pid":559862,"comm":"kprobe_multi"}]}]
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
tools/bpf/bpftool/link.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 108 insertions(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index 2d78607..0015582 100644
--- a/tools/bpf/bpftool/link.c
+++ b/tools/bpf/bpftool/link.c
@@ -14,8 +14,10 @@
#include "json_writer.h"
#include "main.h"
+#include "xlated_dumper.h"
static struct hashmap *link_table;
+static struct dump_data dd = {};
static int link_parse_fd(int *argc, char ***argv)
{
@@ -166,6 +168,45 @@ static int get_prog_info(int prog_id, struct bpf_prog_info *info)
return err;
}
+static int cmp_u64(const void *A, const void *B)
+{
+ const __u64 *a = A, *b = B;
+
+ return *a - *b;
+}
+
+static void
+show_kprobe_multi_json(struct bpf_link_info *info, json_writer_t *wtr)
+{
+ __u32 i, j = 0;
+ __u64 *addrs;
+
+ jsonw_uint_field(json_wtr, "retprobe",
+ info->kprobe_multi.flags & BPF_F_KPROBE_MULTI_RETURN);
+ jsonw_uint_field(json_wtr, "func_cnt", info->kprobe_multi.count);
+ jsonw_name(json_wtr, "funcs");
+ jsonw_start_array(json_wtr);
+ addrs = (__u64 *)u64_to_ptr(info->kprobe_multi.addrs);
+ qsort((void *)addrs, info->kprobe_multi.count, sizeof(__u64), cmp_u64);
+
+ /* Load it once for all. */
+ if (!dd.sym_count)
+ kernel_syms_load(&dd);
+ for (i = 0; i < dd.sym_count; i++) {
+ if (dd.sym_mapping[i].address != addrs[j])
+ continue;
+ jsonw_start_object(json_wtr);
+ jsonw_uint_field(json_wtr, "addr", dd.sym_mapping[i].address);
+ jsonw_string_field(json_wtr, "func", dd.sym_mapping[i].name);
+ /* Print none if it is vmlinux */
+ jsonw_string_field(json_wtr, "module", dd.sym_mapping[i].module);
+ jsonw_end_object(json_wtr);
+ if (j++ == info->kprobe_multi.count)
+ break;
+ }
+ jsonw_end_array(json_wtr);
+}
+
static int show_link_close_json(int fd, struct bpf_link_info *info)
{
struct bpf_prog_info prog_info;
@@ -218,6 +259,9 @@ static int show_link_close_json(int fd, struct bpf_link_info *info)
jsonw_uint_field(json_wtr, "map_id",
info->struct_ops.map_id);
break;
+ case BPF_LINK_TYPE_KPROBE_MULTI:
+ show_kprobe_multi_json(info, json_wtr);
+ break;
default:
break;
}
@@ -351,6 +395,44 @@ void netfilter_dump_plain(const struct bpf_link_info *info)
printf(" flags 0x%x", info->netfilter.flags);
}
+static void show_kprobe_multi_plain(struct bpf_link_info *info)
+{
+ __u32 i, j = 0;
+ __u64 *addrs;
+
+ if (!info->kprobe_multi.count)
+ return;
+
+ printf("\n\tretprobe %d func_cnt %u ",
+ info->kprobe_multi.flags & BPF_F_KPROBE_MULTI_RETURN,
+ info->kprobe_multi.count);
+ addrs = (__u64 *)u64_to_ptr(info->kprobe_multi.addrs);
+ qsort((void *)addrs, info->kprobe_multi.count, sizeof(__u64), cmp_u64);
+
+ /* Load it once for all. */
+ if (!dd.sym_count)
+ kernel_syms_load(&dd);
+ for (i = 0; i < dd.sym_count; i++) {
+ if (dd.sym_mapping[i].address != addrs[j])
+ continue;
+ if (!j)
+ printf("\n\taddrs %016lx funcs %s",
+ dd.sym_mapping[i].address,
+ dd.sym_mapping[i].name);
+ else
+ printf("\n\t %016lx %s",
+ dd.sym_mapping[i].address,
+ dd.sym_mapping[i].name);
+ if (dd.sym_mapping[i].module[0] != '\0')
+ printf(" %s ", dd.sym_mapping[i].module);
+ else
+ printf(" ");
+
+ if (j++ == info->kprobe_multi.count)
+ break;
+ }
+}
+
static int show_link_close_plain(int fd, struct bpf_link_info *info)
{
struct bpf_prog_info prog_info;
@@ -396,6 +478,9 @@ static int show_link_close_plain(int fd, struct bpf_link_info *info)
case BPF_LINK_TYPE_NETFILTER:
netfilter_dump_plain(info);
break;
+ case BPF_LINK_TYPE_KPROBE_MULTI:
+ show_kprobe_multi_plain(info);
+ break;
default:
break;
}
@@ -417,7 +502,9 @@ static int do_show_link(int fd)
{
struct bpf_link_info info;
__u32 len = sizeof(info);
+ __u64 *addrs = NULL;
char buf[256];
+ int count;
int err;
memset(&info, 0, sizeof(info));
@@ -441,12 +528,28 @@ static int do_show_link(int fd)
info.iter.target_name_len = sizeof(buf);
goto again;
}
+ if (info.type == BPF_LINK_TYPE_KPROBE_MULTI &&
+ !info.kprobe_multi.addrs) {
+ count = info.kprobe_multi.count;
+ if (count) {
+ addrs = calloc(count, sizeof(__u64));
+ if (!addrs) {
+ p_err("mem alloc failed");
+ close(fd);
+ return -1;
+ }
+ info.kprobe_multi.addrs = (unsigned long)addrs;
+ goto again;
+ }
+ }
if (json_output)
show_link_close_json(fd, &info);
else
show_link_close_plain(fd, &info);
+ if (addrs)
+ free(addrs);
close(fd);
return 0;
}
@@ -471,7 +574,8 @@ static int do_show(int argc, char **argv)
fd = link_parse_fd(&argc, &argv);
if (fd < 0)
return fd;
- return do_show_link(fd);
+ do_show_link(fd);
+ goto out;
}
if (argc)
@@ -510,6 +614,9 @@ static int do_show(int argc, char **argv)
if (show_pinned)
delete_pinned_obj_table(link_table);
+out:
+ if (dd.sym_count)
+ kernel_syms_destroy(&dd);
return errno == ENOENT ? 0 : -1;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v3 bpf-next 04/10] bpf: Protect probed address based on kptr_restrict setting
2023-06-12 15:15 [PATCH v3 bpf-next 00/10] bpf: Support ->fill_link_info for kprobe_multi and perf_event links Yafang Shao
` (2 preceding siblings ...)
2023-06-12 15:16 ` [PATCH v3 bpf-next 03/10] bpftool: Show probed function in kprobe_multi link info Yafang Shao
@ 2023-06-12 15:16 ` Yafang Shao
2023-06-12 15:16 ` [PATCH v3 bpf-next 05/10] bpf: Clear the probe_addr for uprobe Yafang Shao
` (6 subsequent siblings)
10 siblings, 0 replies; 47+ messages in thread
From: Yafang Shao @ 2023-06-12 15:16 UTC (permalink / raw)
To: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat
Cc: bpf, linux-trace-kernel, Yafang Shao
The probed address can be accessed by userspace through querying the task
file descriptor (fd). However, it is crucial to adhere to the kptr_restrict
setting and refrain from exposing the address if it is not permitted.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
kernel/trace/trace_kprobe.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 59cda19..e4554db 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1551,7 +1551,10 @@ int bpf_get_kprobe_info(const struct perf_event *event, u32 *fd_type,
} else {
*symbol = NULL;
*probe_offset = 0;
- *probe_addr = (unsigned long)tk->rp.kp.addr;
+ if (kallsyms_show_value(current_cred()))
+ *probe_addr = (unsigned long)tk->rp.kp.addr;
+ else
+ *probe_addr = 0;
}
return 0;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v3 bpf-next 05/10] bpf: Clear the probe_addr for uprobe
2023-06-12 15:15 [PATCH v3 bpf-next 00/10] bpf: Support ->fill_link_info for kprobe_multi and perf_event links Yafang Shao
` (3 preceding siblings ...)
2023-06-12 15:16 ` [PATCH v3 bpf-next 04/10] bpf: Protect probed address based on kptr_restrict setting Yafang Shao
@ 2023-06-12 15:16 ` Yafang Shao
2023-06-12 17:22 ` Yonghong Song
2023-06-12 15:16 ` [PATCH v3 bpf-next 06/10] bpf: Expose symbol's respective address Yafang Shao
` (5 subsequent siblings)
10 siblings, 1 reply; 47+ messages in thread
From: Yafang Shao @ 2023-06-12 15:16 UTC (permalink / raw)
To: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat
Cc: bpf, linux-trace-kernel, Yafang Shao
To avoid returning uninitialized or random values when querying the file
descriptor (fd) and accessing probe_addr, it is necessary to clear the
variable prior to its use.
Fixes: 41bdc4b40ed6 ("bpf: introduce bpf subcommand BPF_TASK_FD_QUERY")
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Cc: Yonghong Song <yhs@fb.com>
---
kernel/trace/bpf_trace.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 742047c..97a5235 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2372,10 +2372,12 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
event->attr.type == PERF_TYPE_TRACEPOINT);
#endif
#ifdef CONFIG_UPROBE_EVENTS
- if (flags & TRACE_EVENT_FL_UPROBE)
+ if (flags & TRACE_EVENT_FL_UPROBE) {
err = bpf_get_uprobe_info(event, fd_type, buf,
probe_offset,
event->attr.type == PERF_TYPE_TRACEPOINT);
+ *probe_addr = 0x0;
+ }
#endif
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v3 bpf-next 06/10] bpf: Expose symbol's respective address
2023-06-12 15:15 [PATCH v3 bpf-next 00/10] bpf: Support ->fill_link_info for kprobe_multi and perf_event links Yafang Shao
` (4 preceding siblings ...)
2023-06-12 15:16 ` [PATCH v3 bpf-next 05/10] bpf: Clear the probe_addr for uprobe Yafang Shao
@ 2023-06-12 15:16 ` Yafang Shao
2023-06-12 15:16 ` [PATCH v3 bpf-next 07/10] bpf: Add a common helper bpf_copy_to_user() Yafang Shao
` (4 subsequent siblings)
10 siblings, 0 replies; 47+ messages in thread
From: Yafang Shao @ 2023-06-12 15:16 UTC (permalink / raw)
To: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat
Cc: bpf, linux-trace-kernel, Yafang Shao
Since different symbols can share the same name, it is insufficient to only
expose the symbol name. It is essential to also expose the symbol address
so that users can accurately identify which one is being probed.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
kernel/trace/trace_kprobe.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index e4554db..17e1729 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1547,15 +1547,15 @@ int bpf_get_kprobe_info(const struct perf_event *event, u32 *fd_type,
if (tk->symbol) {
*symbol = tk->symbol;
*probe_offset = tk->rp.kp.offset;
- *probe_addr = 0;
} else {
*symbol = NULL;
*probe_offset = 0;
- if (kallsyms_show_value(current_cred()))
- *probe_addr = (unsigned long)tk->rp.kp.addr;
- else
- *probe_addr = 0;
}
+
+ if (kallsyms_show_value(current_cred()))
+ *probe_addr = (unsigned long)tk->rp.kp.addr;
+ else
+ *probe_addr = 0;
return 0;
}
#endif /* CONFIG_PERF_EVENTS */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v3 bpf-next 07/10] bpf: Add a common helper bpf_copy_to_user()
2023-06-12 15:15 [PATCH v3 bpf-next 00/10] bpf: Support ->fill_link_info for kprobe_multi and perf_event links Yafang Shao
` (5 preceding siblings ...)
2023-06-12 15:16 ` [PATCH v3 bpf-next 06/10] bpf: Expose symbol's respective address Yafang Shao
@ 2023-06-12 15:16 ` Yafang Shao
2023-06-12 15:16 ` [PATCH v3 bpf-next 08/10] bpf: Support ->fill_link_info for perf_event Yafang Shao
` (3 subsequent siblings)
10 siblings, 0 replies; 47+ messages in thread
From: Yafang Shao @ 2023-06-12 15:16 UTC (permalink / raw)
To: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat
Cc: bpf, linux-trace-kernel, Yafang Shao
Add a common helper bpf_copy_to_user(), which will be used at multiple
places.
No functional change.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
kernel/bpf/syscall.c | 34 ++++++++++++++++++++--------------
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 92a57ef..80c9ec0 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3234,6 +3234,25 @@ static void bpf_raw_tp_link_show_fdinfo(const struct bpf_link *link,
raw_tp_link->btp->tp->name);
}
+static int bpf_copy_to_user(char __user *ubuf, const char *buf, u32 ulen,
+ u32 len)
+{
+ if (ulen >= len + 1) {
+ if (copy_to_user(ubuf, buf, len + 1))
+ return -EFAULT;
+ } else {
+ char zero = '\0';
+
+ if (copy_to_user(ubuf, buf, ulen - 1))
+ return -EFAULT;
+ if (put_user(zero, ubuf + ulen - 1))
+ return -EFAULT;
+ return -ENOSPC;
+ }
+
+ return 0;
+}
+
static int bpf_raw_tp_link_fill_link_info(const struct bpf_link *link,
struct bpf_link_info *info)
{
@@ -3252,20 +3271,7 @@ static int bpf_raw_tp_link_fill_link_info(const struct bpf_link *link,
if (!ubuf)
return 0;
- if (ulen >= tp_len + 1) {
- if (copy_to_user(ubuf, tp_name, tp_len + 1))
- return -EFAULT;
- } else {
- char zero = '\0';
-
- if (copy_to_user(ubuf, tp_name, ulen - 1))
- return -EFAULT;
- if (put_user(zero, ubuf + ulen - 1))
- return -EFAULT;
- return -ENOSPC;
- }
-
- return 0;
+ return bpf_copy_to_user(ubuf, tp_name, ulen, tp_len);
}
static const struct bpf_link_ops bpf_raw_tp_link_lops = {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v3 bpf-next 08/10] bpf: Support ->fill_link_info for perf_event
2023-06-12 15:15 [PATCH v3 bpf-next 00/10] bpf: Support ->fill_link_info for kprobe_multi and perf_event links Yafang Shao
` (6 preceding siblings ...)
2023-06-12 15:16 ` [PATCH v3 bpf-next 07/10] bpf: Add a common helper bpf_copy_to_user() Yafang Shao
@ 2023-06-12 15:16 ` Yafang Shao
2023-06-12 17:36 ` Yonghong Song
2023-06-15 10:21 ` Jiri Olsa
2023-06-12 15:16 ` [PATCH v3 bpf-next 09/10] bpftool: Add perf event names Yafang Shao
` (2 subsequent siblings)
10 siblings, 2 replies; 47+ messages in thread
From: Yafang Shao @ 2023-06-12 15:16 UTC (permalink / raw)
To: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat
Cc: bpf, linux-trace-kernel, Yafang Shao
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 */
} __attribute__((aligned(8)));
/* User bpf_sock_addr struct to access socket fields and sockaddr struct passed
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 80c9ec0..fe354d5 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3303,9 +3303,133 @@ static void bpf_perf_link_dealloc(struct bpf_link *link)
kfree(perf_link);
}
+static int bpf_perf_link_fill_name(const struct perf_event *event,
+ char __user *uname, u32 ulen,
+ u64 *probe_offset, u64 *probe_addr,
+ u32 *fd_type)
+{
+ const char *buf;
+ u32 prog_id;
+ size_t len;
+ int err;
+
+ if (!ulen ^ !uname)
+ return -EINVAL;
+ if (!uname)
+ return 0;
+
+ err = bpf_get_perf_event_info(event, &prog_id, fd_type, &buf,
+ probe_offset, probe_addr);
+ if (err)
+ return err;
+
+ len = strlen(buf);
+ if (buf) {
+ err = bpf_copy_to_user(uname, buf, ulen, len);
+ if (err)
+ return err;
+ } else {
+ char zero = '\0';
+
+ if (put_user(zero, uname))
+ return -EFAULT;
+ }
+ return 0;
+}
+
+static int bpf_perf_link_fill_probe(const struct perf_event *event,
+ struct bpf_link_info *info)
+{
+ char __user *uname;
+ u64 addr, offset;
+ u32 ulen, type;
+ int err;
+
+#ifdef CONFIG_KPROBE_EVENTS
+ if (event->tp_event->flags & TRACE_EVENT_FL_KPROBE) {
+ uname = u64_to_user_ptr(info->kprobe.func_name);
+ ulen = info->kprobe.name_len;
+ info->perf_link_type = BPF_PERF_LINK_KPROBE;
+ err = bpf_perf_link_fill_name(event, uname, ulen, &offset,
+ &addr, &type);
+ if (err)
+ return err;
+
+ info->kprobe.offset = offset;
+ if (type == BPF_FD_TYPE_KRETPROBE)
+ info->kprobe.flags = 1;
+ if (!kallsyms_show_value(current_cred()))
+ return 0;
+ info->kprobe.addr = addr;
+ return 0;
+ }
+#endif
+
+#ifdef CONFIG_UPROBE_EVENTS
+ if (event->tp_event->flags & TRACE_EVENT_FL_UPROBE) {
+ uname = u64_to_user_ptr(info->uprobe.file_name);
+ ulen = info->uprobe.name_len;
+ info->perf_link_type = BPF_PERF_LINK_UPROBE;
+ err = bpf_perf_link_fill_name(event, uname, ulen, &offset,
+ &addr, &type);
+ if (err)
+ return err;
+
+ info->uprobe.offset = offset;
+ if (type == BPF_FD_TYPE_URETPROBE)
+ info->uprobe.flags = 1;
+ return 0;
+ }
+#endif
+
+ return -EOPNOTSUPP;
+}
+
+static int bpf_perf_link_fill_tracepoint(const struct perf_event *event,
+ struct bpf_link_info *info)
+{
+ char __user *uname = u64_to_user_ptr(info->tracepoint.tp_name);
+ u32 ulen = info->tracepoint.name_len;
+ u64 addr, off;
+ u32 type;
+
+ info->perf_link_type = BPF_PERF_LINK_TRACEPOINT;
+ return bpf_perf_link_fill_name(event, uname, ulen, &off, &addr, &type);
+}
+
+static int bpf_perf_link_fill_perf_event(const struct perf_event *event,
+ struct bpf_link_info *info)
+{
+ info->perf_event.type = event->attr.type;
+ info->perf_event.config = event->attr.config;
+ info->perf_link_type = BPF_PERF_LINK_PERF_EVENT;
+ return 0;
+}
+
+static int bpf_perf_link_fill_link_info(const struct bpf_link *link,
+ struct bpf_link_info *info)
+{
+ struct bpf_perf_link *perf_link;
+ const struct perf_event *event;
+
+ perf_link = container_of(link, struct bpf_perf_link, link);
+ event = perf_get_event(perf_link->perf_file);
+ if (IS_ERR(event))
+ return PTR_ERR(event);
+
+ if (!event->prog)
+ return -EINVAL;
+ if (event->prog->type == BPF_PROG_TYPE_PERF_EVENT)
+ return bpf_perf_link_fill_perf_event(event, info);
+ if (event->prog->type == BPF_PROG_TYPE_TRACEPOINT)
+ return bpf_perf_link_fill_tracepoint(event, info);
+ return bpf_perf_link_fill_probe(event, info);
+}
+
static const struct bpf_link_ops bpf_perf_link_lops = {
.release = bpf_perf_link_release,
.dealloc = bpf_perf_link_dealloc,
+ .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 23691ea..8d4556e 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/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 */
} __attribute__((aligned(8)));
/* User bpf_sock_addr struct to access socket fields and sockaddr struct passed
--
1.8.3.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v3 bpf-next 09/10] bpftool: Add perf event names
2023-06-12 15:15 [PATCH v3 bpf-next 00/10] bpf: Support ->fill_link_info for kprobe_multi and perf_event links Yafang Shao
` (7 preceding siblings ...)
2023-06-12 15:16 ` [PATCH v3 bpf-next 08/10] bpf: Support ->fill_link_info for perf_event Yafang Shao
@ 2023-06-12 15:16 ` Yafang Shao
2023-06-13 13:41 ` Quentin Monnet
2023-06-15 10:23 ` Jiri Olsa
2023-06-12 15:16 ` [PATCH v3 bpf-next 10/10] bpftool: Show probed function in perf_event link info Yafang Shao
2023-06-15 10:04 ` [PATCH v3 bpf-next 00/10] bpf: Support ->fill_link_info for kprobe_multi and perf_event links Jiri Olsa
10 siblings, 2 replies; 47+ messages in thread
From: Yafang Shao @ 2023-06-12 15:16 UTC (permalink / raw)
To: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat
Cc: bpf, linux-trace-kernel, Yafang Shao, Jiri Olsa
Add new functions and macros to get perf event names. These names are
copied from tool/perf/util/{parse-events,evsel}.c, so that in the future we
will have a good chance to use the same code.
Suggested-by: Jiri Olsa <olsajiri@gmail.com>
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
tools/bpf/bpftool/perf.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++
tools/bpf/bpftool/perf.h | 11 +++++
2 files changed, 118 insertions(+)
create mode 100644 tools/bpf/bpftool/perf.h
diff --git a/tools/bpf/bpftool/perf.c b/tools/bpf/bpftool/perf.c
index 9174344..fbdf88c 100644
--- a/tools/bpf/bpftool/perf.c
+++ b/tools/bpf/bpftool/perf.c
@@ -18,6 +18,113 @@
#include <bpf/bpf.h>
#include "main.h"
+#include "perf.h"
+
+static const char *perf_type_name[PERF_TYPE_MAX] = {
+ [PERF_TYPE_HARDWARE] = "hardware",
+ [PERF_TYPE_SOFTWARE] = "software",
+ [PERF_TYPE_TRACEPOINT] = "tracepoint",
+ [PERF_TYPE_HW_CACHE] = "hw-cache",
+ [PERF_TYPE_RAW] = "raw",
+ [PERF_TYPE_BREAKPOINT] = "breakpoint",
+};
+
+const char *event_symbols_hw[PERF_COUNT_HW_MAX] = {
+ [PERF_COUNT_HW_CPU_CYCLES] = "cpu-cycles",
+ [PERF_COUNT_HW_INSTRUCTIONS] = "instructions",
+ [PERF_COUNT_HW_CACHE_REFERENCES] = "cache-references",
+ [PERF_COUNT_HW_CACHE_MISSES] = "cache-misses",
+ [PERF_COUNT_HW_BRANCH_INSTRUCTIONS] = "branch-instructions",
+ [PERF_COUNT_HW_BRANCH_MISSES] = "branch-misses",
+ [PERF_COUNT_HW_BUS_CYCLES] = "bus-cycles",
+ [PERF_COUNT_HW_STALLED_CYCLES_FRONTEND] = "stalled-cycles-frontend",
+ [PERF_COUNT_HW_STALLED_CYCLES_BACKEND] = "stalled-cycles-backend",
+ [PERF_COUNT_HW_REF_CPU_CYCLES] = "ref-cycles",
+};
+
+const char *event_symbols_sw[PERF_COUNT_SW_MAX] = {
+ [PERF_COUNT_SW_CPU_CLOCK] = "cpu-clock",
+ [PERF_COUNT_SW_TASK_CLOCK] = "task-clock",
+ [PERF_COUNT_SW_PAGE_FAULTS] = "page-faults",
+ [PERF_COUNT_SW_CONTEXT_SWITCHES] = "context-switches",
+ [PERF_COUNT_SW_CPU_MIGRATIONS] = "cpu-migrations",
+ [PERF_COUNT_SW_PAGE_FAULTS_MIN] = "minor-faults",
+ [PERF_COUNT_SW_PAGE_FAULTS_MAJ] = "major-faults",
+ [PERF_COUNT_SW_ALIGNMENT_FAULTS] = "alignment-faults",
+ [PERF_COUNT_SW_EMULATION_FAULTS] = "emulation-faults",
+ [PERF_COUNT_SW_DUMMY] = "dummy",
+ [PERF_COUNT_SW_BPF_OUTPUT] = "bpf-output",
+ [PERF_COUNT_SW_CGROUP_SWITCHES] = "cgroup-switches",
+};
+
+const char *evsel__hw_cache[PERF_COUNT_HW_CACHE_MAX] = {
+ [PERF_COUNT_HW_CACHE_L1D] = "L1-dcache",
+ [PERF_COUNT_HW_CACHE_L1I] = "L1-icache",
+ [PERF_COUNT_HW_CACHE_LL] = "LLC",
+ [PERF_COUNT_HW_CACHE_DTLB] = "dTLB",
+ [PERF_COUNT_HW_CACHE_ITLB] = "iTLB",
+ [PERF_COUNT_HW_CACHE_BPU] = "branch",
+ [PERF_COUNT_HW_CACHE_NODE] = "node",
+};
+
+const char *evsel__hw_cache_op[PERF_COUNT_HW_CACHE_OP_MAX] = {
+ [PERF_COUNT_HW_CACHE_OP_READ] = "load",
+ [PERF_COUNT_HW_CACHE_OP_WRITE] = "store",
+ [PERF_COUNT_HW_CACHE_OP_PREFETCH] = "prefetch",
+};
+
+const char *evsel__hw_cache_result[PERF_COUNT_HW_CACHE_RESULT_MAX] = {
+ [PERF_COUNT_HW_CACHE_RESULT_ACCESS] = "refs",
+ [PERF_COUNT_HW_CACHE_RESULT_MISS] = "misses",
+};
+
+const char *perf_type_str(enum perf_type_id t)
+{
+ if (t < 0 || t >= ARRAY_SIZE(perf_type_name))
+ return NULL;
+
+ return perf_type_name[t];
+}
+
+const char *perf_hw_str(enum perf_hw_id t)
+{
+ if (t < 0 || t >= ARRAY_SIZE(event_symbols_hw))
+ return NULL;
+
+ return event_symbols_hw[t];
+}
+
+const char *perf_hw_cache_str(enum perf_hw_cache_id t)
+{
+ if (t < 0 || t >= ARRAY_SIZE(evsel__hw_cache))
+ return NULL;
+
+ return evsel__hw_cache[t];
+}
+
+const char *perf_hw_cache_op_str(enum perf_hw_cache_op_id t)
+{
+ if (t < 0 || t >= ARRAY_SIZE(evsel__hw_cache_op))
+ return NULL;
+
+ return evsel__hw_cache_op[t];
+}
+
+const char *perf_hw_cache_op_result_str(enum perf_hw_cache_op_result_id t)
+{
+ if (t < 0 || t >= ARRAY_SIZE(evsel__hw_cache_result))
+ return NULL;
+
+ return evsel__hw_cache_result[t];
+}
+
+const char *perf_sw_str(enum perf_sw_ids t)
+{
+ if (t < 0 || t >= ARRAY_SIZE(event_symbols_sw))
+ return NULL;
+
+ return event_symbols_sw[t];
+}
/* 0: undecided, 1: supported, 2: not supported */
static int perf_query_supported;
diff --git a/tools/bpf/bpftool/perf.h b/tools/bpf/bpftool/perf.h
new file mode 100644
index 0000000..3fd7e42
--- /dev/null
+++ b/tools/bpf/bpftool/perf.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/* Copyright (C) 2023 Yafang Shao <laoar.shao@gmail.com> */
+
+#include <linux/perf_event.h>
+
+const char *perf_type_str(enum perf_type_id t);
+const char *perf_hw_str(enum perf_hw_id t);
+const char *perf_hw_cache_str(enum perf_hw_cache_id t);
+const char *perf_hw_cache_op_str(enum perf_hw_cache_op_id t);
+const char *perf_hw_cache_op_result_str(enum perf_hw_cache_op_result_id t);
+const char *perf_sw_str(enum perf_sw_ids t);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v3 bpf-next 10/10] bpftool: Show probed function in perf_event link info
2023-06-12 15:15 [PATCH v3 bpf-next 00/10] bpf: Support ->fill_link_info for kprobe_multi and perf_event links Yafang Shao
` (8 preceding siblings ...)
2023-06-12 15:16 ` [PATCH v3 bpf-next 09/10] bpftool: Add perf event names Yafang Shao
@ 2023-06-12 15:16 ` Yafang Shao
2023-06-13 13:42 ` Quentin Monnet
2023-06-16 20:41 ` Andrii Nakryiko
2023-06-15 10:04 ` [PATCH v3 bpf-next 00/10] bpf: Support ->fill_link_info for kprobe_multi and perf_event links Jiri Olsa
10 siblings, 2 replies; 47+ messages in thread
From: Yafang Shao @ 2023-06-12 15:16 UTC (permalink / raw)
To: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat
Cc: bpf, linux-trace-kernel, Yafang Shao
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
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@gmail.com>
---
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);
+ 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)
+{
+#define PERF_HW_CACHE_LEN 128
+ 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);
+ 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);
+ goto again;
+ }
+ }
+out:
if (json_output)
show_link_close_json(fd, &info);
else
--
1.8.3.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 05/10] bpf: Clear the probe_addr for uprobe
2023-06-12 15:16 ` [PATCH v3 bpf-next 05/10] bpf: Clear the probe_addr for uprobe Yafang Shao
@ 2023-06-12 17:22 ` Yonghong Song
0 siblings, 0 replies; 47+ messages in thread
From: Yonghong Song @ 2023-06-12 17:22 UTC (permalink / raw)
To: Yafang Shao, ast, daniel, john.fastabend, andrii, martin.lau,
song, yhs, kpsingh, sdf, haoluo, jolsa, quentin, rostedt,
mhiramat
Cc: bpf, linux-trace-kernel
On 6/12/23 8:16 AM, Yafang Shao wrote:
> To avoid returning uninitialized or random values when querying the file
> descriptor (fd) and accessing probe_addr, it is necessary to clear the
> variable prior to its use.
>
> Fixes: 41bdc4b40ed6 ("bpf: introduce bpf subcommand BPF_TASK_FD_QUERY")
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> Cc: Yonghong Song <yhs@fb.com>
Thanks for the fix! LGTM.
Acked-by: Yonghong Song <yhs@fb.com>
> ---
> kernel/trace/bpf_trace.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 742047c..97a5235 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2372,10 +2372,12 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
> event->attr.type == PERF_TYPE_TRACEPOINT);
> #endif
> #ifdef CONFIG_UPROBE_EVENTS
> - if (flags & TRACE_EVENT_FL_UPROBE)
> + if (flags & TRACE_EVENT_FL_UPROBE) {
> err = bpf_get_uprobe_info(event, fd_type, buf,
> probe_offset,
> event->attr.type == PERF_TYPE_TRACEPOINT);
> + *probe_addr = 0x0;
> + }
> #endif
> }
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 08/10] bpf: Support ->fill_link_info for perf_event
2023-06-12 15:16 ` [PATCH v3 bpf-next 08/10] bpf: Support ->fill_link_info for perf_event Yafang Shao
@ 2023-06-12 17:36 ` Yonghong Song
2023-06-13 2:47 ` Yafang Shao
2023-06-15 10:21 ` Jiri Olsa
1 sibling, 1 reply; 47+ messages in thread
From: Yonghong Song @ 2023-06-12 17:36 UTC (permalink / raw)
To: Yafang Shao, ast, daniel, john.fastabend, andrii, martin.lau,
song, yhs, kpsingh, sdf, haoluo, jolsa, quentin, rostedt,
mhiramat
Cc: bpf, linux-trace-kernel
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
[...]
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 08/10] bpf: Support ->fill_link_info for perf_event
2023-06-12 17:36 ` Yonghong Song
@ 2023-06-13 2:47 ` Yafang Shao
2023-06-14 2:34 ` Kui-Feng Lee
0 siblings, 1 reply; 47+ messages in thread
From: Yafang Shao @ 2023-06-13 2:47 UTC (permalink / raw)
To: Yonghong Song
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat, bpf,
linux-trace-kernel
On Tue, Jun 13, 2023 at 1:36 AM Yonghong Song <yhs@meta.com> wrote:
>
>
>
> 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.
If we put it into each individual struct, we have to choose one
specific struct to get the type before we use the real struct, for
example,
if (info.perf_event.type == BPF_PERF_LINK_PERF_EVENT)
goto out;
if (info.perf_event.type == BPF_PERF_LINK_TRACEPOINT &&
!info.tracepoint.tp_name) {
info.tracepoint.tp_name = (unsigned long)&buf;
info.tracepoint.name_len = sizeof(buf);
goto again;
}
...
That doesn't look perfect.
However I agree with you that the perf_link_type may disallow the
extensions for the big union. I will think about it.
--
Regards
Yafang
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 02/10] bpftool: Dump the kernel symbol's module name
2023-06-12 15:16 ` [PATCH v3 bpf-next 02/10] bpftool: Dump the kernel symbol's module name Yafang Shao
@ 2023-06-13 13:41 ` Quentin Monnet
2023-06-13 14:56 ` Yafang Shao
2023-06-16 17:25 ` Andrii Nakryiko
1 sibling, 1 reply; 47+ messages in thread
From: Quentin Monnet @ 2023-06-13 13:41 UTC (permalink / raw)
To: Yafang Shao, ast, daniel, john.fastabend, andrii, martin.lau,
song, yhs, kpsingh, sdf, haoluo, jolsa, rostedt, mhiramat
Cc: bpf, linux-trace-kernel
2023-06-12 15:16 UTC+0000 ~ Yafang Shao <laoar.shao@gmail.com>
> If the kernel symbol is in a module, we will dump the module name as
> well.
>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
> tools/bpf/bpftool/xlated_dumper.c | 6 +++++-
> tools/bpf/bpftool/xlated_dumper.h | 2 ++
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/xlated_dumper.c b/tools/bpf/bpftool/xlated_dumper.c
> index da608e1..dd917f3 100644
> --- a/tools/bpf/bpftool/xlated_dumper.c
> +++ b/tools/bpf/bpftool/xlated_dumper.c
> @@ -46,7 +46,11 @@ void kernel_syms_load(struct dump_data *dd)
> }
> dd->sym_mapping = tmp;
> sym = &dd->sym_mapping[dd->sym_count];
> - if (sscanf(buff, "%p %*c %s", &address, sym->name) != 2)
> +
> + /* module is optional */
> + sym->module[0] = '\0';
> + if (sscanf(buff, "%p %*c %s %s", &address, sym->name,
> + sym->module) < 2)
> continue;
> sym->address = (unsigned long)address;
> if (!strcmp(sym->name, "__bpf_call_base")) {
> diff --git a/tools/bpf/bpftool/xlated_dumper.h b/tools/bpf/bpftool/xlated_dumper.h
> index 9a94637..5df8025 100644
> --- a/tools/bpf/bpftool/xlated_dumper.h
> +++ b/tools/bpf/bpftool/xlated_dumper.h
> @@ -5,12 +5,14 @@
> #define __BPF_TOOL_XLATED_DUMPER_H
>
> #define SYM_MAX_NAME 256
> +#define MODULE_NAME_LEN 64
>
> struct bpf_prog_linfo;
>
> struct kernel_sym {
> unsigned long address;
> char name[SYM_MAX_NAME];
> + char module[MODULE_NAME_LEN];
Nit: MODULE_MAX_NAME would be more consistent and would make more sense
to me? And it would avoid confusion with MODULE_NAME_LEN from kernel,
which doesn't have the same value.
> };
>
> struct dump_data {
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 03/10] bpftool: Show probed function in kprobe_multi link info
2023-06-12 15:16 ` [PATCH v3 bpf-next 03/10] bpftool: Show probed function in kprobe_multi link info Yafang Shao
@ 2023-06-13 13:41 ` Quentin Monnet
2023-06-13 14:59 ` Yafang Shao
2023-06-13 22:36 ` Kui-Feng Lee
2023-06-16 17:30 ` Andrii Nakryiko
2 siblings, 1 reply; 47+ messages in thread
From: Quentin Monnet @ 2023-06-13 13:41 UTC (permalink / raw)
To: Yafang Shao, ast, daniel, john.fastabend, andrii, martin.lau,
song, yhs, kpsingh, sdf, haoluo, jolsa, rostedt, mhiramat
Cc: bpf, linux-trace-kernel
2023-06-12 15:16 UTC+0000 ~ Yafang Shao <laoar.shao@gmail.com>
> Show the already expose kprobe_multi link info in bpftool. The result as
> follows,
>
> 52: kprobe_multi prog 381
> retprobe 0 func_cnt 7
> addrs ffffffff9ec44f20 funcs schedule_timeout_interruptible
> ffffffff9ec44f60 schedule_timeout_killable
> ffffffff9ec44fa0 schedule_timeout_uninterruptible
> ffffffff9ec44fe0 schedule_timeout_idle
> ffffffffc09468d0 xfs_trans_get_efd [xfs]
> ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
> ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
> pids kprobe_multi(559862)
> 53: kprobe_multi prog 381
> retprobe 1 func_cnt 7
> addrs ffffffff9ec44f20 funcs schedule_timeout_interruptible
> ffffffff9ec44f60 schedule_timeout_killable
> ffffffff9ec44fa0 schedule_timeout_uninterruptible
> ffffffff9ec44fe0 schedule_timeout_idle
> ffffffffc09468d0 xfs_trans_get_efd [xfs]
> ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
> ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
> pids kprobe_multi(559862)
>
> $ tools/bpf/bpftool/bpftool link show -j
> [{"id":52,"type":"kprobe_multi","prog_id":381,"retprobe":0,"func_cnt":7,"funcs":[{"addr":18446744072078249760,"func":"schedule_timeout_interruptible","module":""},{"addr":18446744072078249824,"func":"schedule_timeout_killable","module":""},{"addr":18446744072078249888,"func":"schedule_timeout_uninterruptible","module":""},{"addr":18446744072078249952,"func":"schedule_timeout_idle","module":""},{"addr":18446744072645535952,"func":"xfs_trans_get_efd","module":"[xfs]"},{"addr":18446744072645589520,"func":"xfs_trans_get_buf_map","module":"[xfs]"},{"addr":18446744072645604128,"func":"xfs_trans_get_dqtrx","module":"[xfs]"}],"pids":[{"pid":559862,"comm":"kprobe_multi"}]},{"id":53,"type":"kprobe_multi","prog_id":381,"retprobe":1,"func_cnt":7,"funcs":[{"addr":18446744072078249760,"func":"schedule_timeout_interruptible","module":""},{"addr":18446744072078249824,"func":"schedule_timeout_killable","module":""},{"addr":18446744072078249888,"func":"schedule_timeout_uninterruptible","module":""},{"addr":18446744072078249952,"func":"schedule_timeout_idle","module":""},{"addr":18446744072645535952,"func":"xfs_trans_get_efd","module":"[xfs]"},{"addr":18446744072645589520,"func":"xfs_trans_get_buf_map","module":"[xfs]"},{"addr":18446744072645604128,"func":"xfs_trans_get_dqtrx","module":"[xfs]"}],"pids":[{"pid":559862,"comm":"kprobe_multi"}]}]
>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
> tools/bpf/bpftool/link.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 108 insertions(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
> index 2d78607..0015582 100644
> --- a/tools/bpf/bpftool/link.c
> +++ b/tools/bpf/bpftool/link.c
> @@ -14,8 +14,10 @@
>
> #include "json_writer.h"
> #include "main.h"
> +#include "xlated_dumper.h"
>
> static struct hashmap *link_table;
> +static struct dump_data dd = {};
>
> static int link_parse_fd(int *argc, char ***argv)
> {
> @@ -166,6 +168,45 @@ static int get_prog_info(int prog_id, struct bpf_prog_info *info)
> return err;
> }
>
> +static int cmp_u64(const void *A, const void *B)
> +{
> + const __u64 *a = A, *b = B;
> +
> + return *a - *b;
> +}
> +
> +static void
> +show_kprobe_multi_json(struct bpf_link_info *info, json_writer_t *wtr)
> +{
> + __u32 i, j = 0;
> + __u64 *addrs;
> +
> + jsonw_uint_field(json_wtr, "retprobe",
> + info->kprobe_multi.flags & BPF_F_KPROBE_MULTI_RETURN);
The "retprobe" field could maybe be a boolean rather than an int.
> + jsonw_uint_field(json_wtr, "func_cnt", info->kprobe_multi.count);
> + jsonw_name(json_wtr, "funcs");
> + jsonw_start_array(json_wtr);
> + addrs = (__u64 *)u64_to_ptr(info->kprobe_multi.addrs);
> + qsort((void *)addrs, info->kprobe_multi.count, sizeof(__u64), cmp_u64);
> +
> + /* Load it once for all. */
> + if (!dd.sym_count)
> + kernel_syms_load(&dd);
> + for (i = 0; i < dd.sym_count; i++) {
> + if (dd.sym_mapping[i].address != addrs[j])
> + continue;
> + jsonw_start_object(json_wtr);
> + jsonw_uint_field(json_wtr, "addr", dd.sym_mapping[i].address);
> + jsonw_string_field(json_wtr, "func", dd.sym_mapping[i].name);
> + /* Print none if it is vmlinux */
> + jsonw_string_field(json_wtr, "module", dd.sym_mapping[i].module);
Can we trim the square brackets around module names for the JSON output,
please? They make entries look like arrays; but mostly, if we keep them,
we're forcing every consumer to trim them on their side before being
able to reuse the value.
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 09/10] bpftool: Add perf event names
2023-06-12 15:16 ` [PATCH v3 bpf-next 09/10] bpftool: Add perf event names Yafang Shao
@ 2023-06-13 13:41 ` Quentin Monnet
2023-06-13 15:01 ` Yafang Shao
2023-06-15 10:23 ` Jiri Olsa
1 sibling, 1 reply; 47+ messages in thread
From: Quentin Monnet @ 2023-06-13 13:41 UTC (permalink / raw)
To: Yafang Shao, ast, daniel, john.fastabend, andrii, martin.lau,
song, yhs, kpsingh, sdf, haoluo, jolsa, rostedt, mhiramat
Cc: bpf, linux-trace-kernel, Jiri Olsa
2023-06-12 15:16 UTC+0000 ~ Yafang Shao <laoar.shao@gmail.com>
> Add new functions and macros to get perf event names. These names are
> copied from tool/perf/util/{parse-events,evsel}.c, so that in the future we
> will have a good chance to use the same code.
>
> Suggested-by: Jiri Olsa <olsajiri@gmail.com>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
> tools/bpf/bpftool/perf.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++
> tools/bpf/bpftool/perf.h | 11 +++++
Although the names are deceiving, I think these should all be moved to
link.c and link.h, where we'll actually use them, or to some other file
with a new name. File perf.c is for implementing "bpftool perf ...".
> 2 files changed, 118 insertions(+)
> create mode 100644 tools/bpf/bpftool/perf.h
>
> diff --git a/tools/bpf/bpftool/perf.c b/tools/bpf/bpftool/perf.c
> index 9174344..fbdf88c 100644
> --- a/tools/bpf/bpftool/perf.c
> +++ b/tools/bpf/bpftool/perf.c
> @@ -18,6 +18,113 @@
> #include <bpf/bpf.h>
>
> #include "main.h"
> +#include "perf.h"
> +
> +static const char *perf_type_name[PERF_TYPE_MAX] = {
> + [PERF_TYPE_HARDWARE] = "hardware",
> + [PERF_TYPE_SOFTWARE] = "software",
> + [PERF_TYPE_TRACEPOINT] = "tracepoint",
> + [PERF_TYPE_HW_CACHE] = "hw-cache",
> + [PERF_TYPE_RAW] = "raw",
> + [PERF_TYPE_BREAKPOINT] = "breakpoint",
> +};
> +
> +const char *event_symbols_hw[PERF_COUNT_HW_MAX] = {
> + [PERF_COUNT_HW_CPU_CYCLES] = "cpu-cycles",
> + [PERF_COUNT_HW_INSTRUCTIONS] = "instructions",
> + [PERF_COUNT_HW_CACHE_REFERENCES] = "cache-references",
> + [PERF_COUNT_HW_CACHE_MISSES] = "cache-misses",
> + [PERF_COUNT_HW_BRANCH_INSTRUCTIONS] = "branch-instructions",
> + [PERF_COUNT_HW_BRANCH_MISSES] = "branch-misses",
> + [PERF_COUNT_HW_BUS_CYCLES] = "bus-cycles",
> + [PERF_COUNT_HW_STALLED_CYCLES_FRONTEND] = "stalled-cycles-frontend",
> + [PERF_COUNT_HW_STALLED_CYCLES_BACKEND] = "stalled-cycles-backend",
> + [PERF_COUNT_HW_REF_CPU_CYCLES] = "ref-cycles",
> +};
> +
> +const char *event_symbols_sw[PERF_COUNT_SW_MAX] = {
> + [PERF_COUNT_SW_CPU_CLOCK] = "cpu-clock",
> + [PERF_COUNT_SW_TASK_CLOCK] = "task-clock",
> + [PERF_COUNT_SW_PAGE_FAULTS] = "page-faults",
> + [PERF_COUNT_SW_CONTEXT_SWITCHES] = "context-switches",
> + [PERF_COUNT_SW_CPU_MIGRATIONS] = "cpu-migrations",
> + [PERF_COUNT_SW_PAGE_FAULTS_MIN] = "minor-faults",
> + [PERF_COUNT_SW_PAGE_FAULTS_MAJ] = "major-faults",
> + [PERF_COUNT_SW_ALIGNMENT_FAULTS] = "alignment-faults",
> + [PERF_COUNT_SW_EMULATION_FAULTS] = "emulation-faults",
> + [PERF_COUNT_SW_DUMMY] = "dummy",
> + [PERF_COUNT_SW_BPF_OUTPUT] = "bpf-output",
> + [PERF_COUNT_SW_CGROUP_SWITCHES] = "cgroup-switches",
> +};
> +
> +const char *evsel__hw_cache[PERF_COUNT_HW_CACHE_MAX] = {
> + [PERF_COUNT_HW_CACHE_L1D] = "L1-dcache",
> + [PERF_COUNT_HW_CACHE_L1I] = "L1-icache",
> + [PERF_COUNT_HW_CACHE_LL] = "LLC",
> + [PERF_COUNT_HW_CACHE_DTLB] = "dTLB",
> + [PERF_COUNT_HW_CACHE_ITLB] = "iTLB",
> + [PERF_COUNT_HW_CACHE_BPU] = "branch",
> + [PERF_COUNT_HW_CACHE_NODE] = "node",
> +};
> +
> +const char *evsel__hw_cache_op[PERF_COUNT_HW_CACHE_OP_MAX] = {
> + [PERF_COUNT_HW_CACHE_OP_READ] = "load",
> + [PERF_COUNT_HW_CACHE_OP_WRITE] = "store",
> + [PERF_COUNT_HW_CACHE_OP_PREFETCH] = "prefetch",
> +};
> +
> +const char *evsel__hw_cache_result[PERF_COUNT_HW_CACHE_RESULT_MAX] = {
> + [PERF_COUNT_HW_CACHE_RESULT_ACCESS] = "refs",
> + [PERF_COUNT_HW_CACHE_RESULT_MISS] = "misses",
> +};
> +
> +const char *perf_type_str(enum perf_type_id t)
> +{
> + if (t < 0 || t >= ARRAY_SIZE(perf_type_name))
> + return NULL;
> +
> + return perf_type_name[t];
> +}
> +
> +const char *perf_hw_str(enum perf_hw_id t)
> +{
> + if (t < 0 || t >= ARRAY_SIZE(event_symbols_hw))
> + return NULL;
> +
> + return event_symbols_hw[t];
> +}
> +
> +const char *perf_hw_cache_str(enum perf_hw_cache_id t)
> +{
> + if (t < 0 || t >= ARRAY_SIZE(evsel__hw_cache))
> + return NULL;
> +
> + return evsel__hw_cache[t];
> +}
> +
> +const char *perf_hw_cache_op_str(enum perf_hw_cache_op_id t)
> +{
> + if (t < 0 || t >= ARRAY_SIZE(evsel__hw_cache_op))
> + return NULL;
> +
> + return evsel__hw_cache_op[t];
> +}
> +
> +const char *perf_hw_cache_op_result_str(enum perf_hw_cache_op_result_id t)
> +{
> + if (t < 0 || t >= ARRAY_SIZE(evsel__hw_cache_result))
> + return NULL;
> +
> + return evsel__hw_cache_result[t];
> +}
> +
> +const char *perf_sw_str(enum perf_sw_ids t)
> +{
> + if (t < 0 || t >= ARRAY_SIZE(event_symbols_sw))
> + return NULL;
> +
> + return event_symbols_sw[t];
> +}
>
> /* 0: undecided, 1: supported, 2: not supported */
> static int perf_query_supported;
> diff --git a/tools/bpf/bpftool/perf.h b/tools/bpf/bpftool/perf.h
> new file mode 100644
> index 0000000..3fd7e42
> --- /dev/null
> +++ b/tools/bpf/bpftool/perf.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/* Copyright (C) 2023 Yafang Shao <laoar.shao@gmail.com> */
> +
> +#include <linux/perf_event.h>
> +
> +const char *perf_type_str(enum perf_type_id t);
> +const char *perf_hw_str(enum perf_hw_id t);
> +const char *perf_hw_cache_str(enum perf_hw_cache_id t);
> +const char *perf_hw_cache_op_str(enum perf_hw_cache_op_id t);
> +const char *perf_hw_cache_op_result_str(enum perf_hw_cache_op_result_id t);
> +const char *perf_sw_str(enum perf_sw_ids t);
I'm not sure we need all these API functions if we keep the arrays in
bpftool. I'd probably have just a generic one and pass it the name of
the relevant array in argument. Although I've got no objection with the
current form if it helps unifying the code with perf in the future.
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 10/10] bpftool: Show probed function in perf_event link info
2023-06-12 15:16 ` [PATCH v3 bpf-next 10/10] bpftool: Show probed function in perf_event link info Yafang Shao
@ 2023-06-13 13:42 ` Quentin Monnet
2023-06-13 15:11 ` Yafang Shao
2023-06-16 20:41 ` Andrii Nakryiko
1 sibling, 1 reply; 47+ messages in thread
From: Quentin Monnet @ 2023-06-13 13:42 UTC (permalink / raw)
To: Yafang Shao, ast, daniel, john.fastabend, andrii, martin.lau,
song, yhs, kpsingh, sdf, haoluo, jolsa, rostedt, mhiramat
Cc: bpf, linux-trace-kernel
2023-06-12 15:16 UTC+0000 ~ Yafang Shao <laoar.shao@gmail.com>
> 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@gmail.com>
> ---
> 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!
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 02/10] bpftool: Dump the kernel symbol's module name
2023-06-13 13:41 ` Quentin Monnet
@ 2023-06-13 14:56 ` Yafang Shao
0 siblings, 0 replies; 47+ messages in thread
From: Yafang Shao @ 2023-06-13 14:56 UTC (permalink / raw)
To: Quentin Monnet
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, rostedt, mhiramat, bpf,
linux-trace-kernel
On Tue, Jun 13, 2023 at 9:41 PM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2023-06-12 15:16 UTC+0000 ~ Yafang Shao <laoar.shao@gmail.com>
> > If the kernel symbol is in a module, we will dump the module name as
> > well.
> >
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > ---
> > tools/bpf/bpftool/xlated_dumper.c | 6 +++++-
> > tools/bpf/bpftool/xlated_dumper.h | 2 ++
> > 2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/bpf/bpftool/xlated_dumper.c b/tools/bpf/bpftool/xlated_dumper.c
> > index da608e1..dd917f3 100644
> > --- a/tools/bpf/bpftool/xlated_dumper.c
> > +++ b/tools/bpf/bpftool/xlated_dumper.c
> > @@ -46,7 +46,11 @@ void kernel_syms_load(struct dump_data *dd)
> > }
> > dd->sym_mapping = tmp;
> > sym = &dd->sym_mapping[dd->sym_count];
> > - if (sscanf(buff, "%p %*c %s", &address, sym->name) != 2)
> > +
> > + /* module is optional */
> > + sym->module[0] = '\0';
> > + if (sscanf(buff, "%p %*c %s %s", &address, sym->name,
> > + sym->module) < 2)
> > continue;
> > sym->address = (unsigned long)address;
> > if (!strcmp(sym->name, "__bpf_call_base")) {
> > diff --git a/tools/bpf/bpftool/xlated_dumper.h b/tools/bpf/bpftool/xlated_dumper.h
> > index 9a94637..5df8025 100644
> > --- a/tools/bpf/bpftool/xlated_dumper.h
> > +++ b/tools/bpf/bpftool/xlated_dumper.h
> > @@ -5,12 +5,14 @@
> > #define __BPF_TOOL_XLATED_DUMPER_H
> >
> > #define SYM_MAX_NAME 256
> > +#define MODULE_NAME_LEN 64
> >
> > struct bpf_prog_linfo;
> >
> > struct kernel_sym {
> > unsigned long address;
> > char name[SYM_MAX_NAME];
> > + char module[MODULE_NAME_LEN];
>
> Nit: MODULE_MAX_NAME would be more consistent and would make more sense
> to me? And it would avoid confusion with MODULE_NAME_LEN from kernel,
> which doesn't have the same value.
Will use MODULE_MAX_NAME instead.
--
Regards
Yafang
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 03/10] bpftool: Show probed function in kprobe_multi link info
2023-06-13 13:41 ` Quentin Monnet
@ 2023-06-13 14:59 ` Yafang Shao
0 siblings, 0 replies; 47+ messages in thread
From: Yafang Shao @ 2023-06-13 14:59 UTC (permalink / raw)
To: Quentin Monnet
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, rostedt, mhiramat, bpf,
linux-trace-kernel
On Tue, Jun 13, 2023 at 9:41 PM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2023-06-12 15:16 UTC+0000 ~ Yafang Shao <laoar.shao@gmail.com>
> > Show the already expose kprobe_multi link info in bpftool. The result as
> > follows,
> >
> > 52: kprobe_multi prog 381
> > retprobe 0 func_cnt 7
> > addrs ffffffff9ec44f20 funcs schedule_timeout_interruptible
> > ffffffff9ec44f60 schedule_timeout_killable
> > ffffffff9ec44fa0 schedule_timeout_uninterruptible
> > ffffffff9ec44fe0 schedule_timeout_idle
> > ffffffffc09468d0 xfs_trans_get_efd [xfs]
> > ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
> > ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
> > pids kprobe_multi(559862)
> > 53: kprobe_multi prog 381
> > retprobe 1 func_cnt 7
> > addrs ffffffff9ec44f20 funcs schedule_timeout_interruptible
> > ffffffff9ec44f60 schedule_timeout_killable
> > ffffffff9ec44fa0 schedule_timeout_uninterruptible
> > ffffffff9ec44fe0 schedule_timeout_idle
> > ffffffffc09468d0 xfs_trans_get_efd [xfs]
> > ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
> > ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
> > pids kprobe_multi(559862)
> >
> > $ tools/bpf/bpftool/bpftool link show -j
> > [{"id":52,"type":"kprobe_multi","prog_id":381,"retprobe":0,"func_cnt":7,"funcs":[{"addr":18446744072078249760,"func":"schedule_timeout_interruptible","module":""},{"addr":18446744072078249824,"func":"schedule_timeout_killable","module":""},{"addr":18446744072078249888,"func":"schedule_timeout_uninterruptible","module":""},{"addr":18446744072078249952,"func":"schedule_timeout_idle","module":""},{"addr":18446744072645535952,"func":"xfs_trans_get_efd","module":"[xfs]"},{"addr":18446744072645589520,"func":"xfs_trans_get_buf_map","module":"[xfs]"},{"addr":18446744072645604128,"func":"xfs_trans_get_dqtrx","module":"[xfs]"}],"pids":[{"pid":559862,"comm":"kprobe_multi"}]},{"id":53,"type":"kprobe_multi","prog_id":381,"retprobe":1,"func_cnt":7,"funcs":[{"addr":18446744072078249760,"func":"schedule_timeout_interruptible","module":""},{"addr":18446744072078249824,"func":"schedule_timeout_killable","module":""},{"addr":18446744072078249888,"func":"schedule_timeout_uninterruptible","module":""},{"addr":18446744072078249952,"func":"schedule_timeout_idle","module":""},{"addr":18446744072645535952,"func":"xfs_trans_get_efd","module":"[xfs]"},{"addr":18446744072645589520,"func":"xfs_trans_get_buf_map","module":"[xfs]"},{"addr":18446744072645604128,"func":"xfs_trans_get_dqtrx","module":"[xfs]"}],"pids":[{"pid":559862,"comm":"kprobe_multi"}]}]
> >
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > ---
> > tools/bpf/bpftool/link.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 108 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
> > index 2d78607..0015582 100644
> > --- a/tools/bpf/bpftool/link.c
> > +++ b/tools/bpf/bpftool/link.c
> > @@ -14,8 +14,10 @@
> >
> > #include "json_writer.h"
> > #include "main.h"
> > +#include "xlated_dumper.h"
> >
> > static struct hashmap *link_table;
> > +static struct dump_data dd = {};
> >
> > static int link_parse_fd(int *argc, char ***argv)
> > {
> > @@ -166,6 +168,45 @@ static int get_prog_info(int prog_id, struct bpf_prog_info *info)
> > return err;
> > }
> >
> > +static int cmp_u64(const void *A, const void *B)
> > +{
> > + const __u64 *a = A, *b = B;
> > +
> > + return *a - *b;
> > +}
> > +
> > +static void
> > +show_kprobe_multi_json(struct bpf_link_info *info, json_writer_t *wtr)
> > +{
> > + __u32 i, j = 0;
> > + __u64 *addrs;
> > +
> > + jsonw_uint_field(json_wtr, "retprobe",
> > + info->kprobe_multi.flags & BPF_F_KPROBE_MULTI_RETURN);
>
> The "retprobe" field could maybe be a boolean rather than an int.
Will change it.
>
> > + jsonw_uint_field(json_wtr, "func_cnt", info->kprobe_multi.count);
> > + jsonw_name(json_wtr, "funcs");
> > + jsonw_start_array(json_wtr);
> > + addrs = (__u64 *)u64_to_ptr(info->kprobe_multi.addrs);
> > + qsort((void *)addrs, info->kprobe_multi.count, sizeof(__u64), cmp_u64);
> > +
> > + /* Load it once for all. */
> > + if (!dd.sym_count)
> > + kernel_syms_load(&dd);
> > + for (i = 0; i < dd.sym_count; i++) {
> > + if (dd.sym_mapping[i].address != addrs[j])
> > + continue;
> > + jsonw_start_object(json_wtr);
> > + jsonw_uint_field(json_wtr, "addr", dd.sym_mapping[i].address);
> > + jsonw_string_field(json_wtr, "func", dd.sym_mapping[i].name);
> > + /* Print none if it is vmlinux */
> > + jsonw_string_field(json_wtr, "module", dd.sym_mapping[i].module);
>
> Can we trim the square brackets around module names for the JSON output,
> please? They make entries look like arrays; but mostly, if we keep them,
> we're forcing every consumer to trim them on their side before being
> able to reuse the value.
Agree, will trim it.
--
Regards
Yafang
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 09/10] bpftool: Add perf event names
2023-06-13 13:41 ` Quentin Monnet
@ 2023-06-13 15:01 ` Yafang Shao
0 siblings, 0 replies; 47+ messages in thread
From: Yafang Shao @ 2023-06-13 15:01 UTC (permalink / raw)
To: Quentin Monnet
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, rostedt, mhiramat, bpf,
linux-trace-kernel, Jiri Olsa
On Tue, Jun 13, 2023 at 9:42 PM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2023-06-12 15:16 UTC+0000 ~ Yafang Shao <laoar.shao@gmail.com>
> > Add new functions and macros to get perf event names. These names are
> > copied from tool/perf/util/{parse-events,evsel}.c, so that in the future we
> > will have a good chance to use the same code.
> >
> > Suggested-by: Jiri Olsa <olsajiri@gmail.com>
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > ---
> > tools/bpf/bpftool/perf.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++
> > tools/bpf/bpftool/perf.h | 11 +++++
>
> Although the names are deceiving, I think these should all be moved to
> link.c and link.h, where we'll actually use them, or to some other file
> with a new name. File perf.c is for implementing "bpftool perf ...".
Got it. Will move them into link.c.
>
> > 2 files changed, 118 insertions(+)
> > create mode 100644 tools/bpf/bpftool/perf.h
> >
> > diff --git a/tools/bpf/bpftool/perf.c b/tools/bpf/bpftool/perf.c
> > index 9174344..fbdf88c 100644
> > --- a/tools/bpf/bpftool/perf.c
> > +++ b/tools/bpf/bpftool/perf.c
> > @@ -18,6 +18,113 @@
> > #include <bpf/bpf.h>
> >
> > #include "main.h"
> > +#include "perf.h"
> > +
> > +static const char *perf_type_name[PERF_TYPE_MAX] = {
> > + [PERF_TYPE_HARDWARE] = "hardware",
> > + [PERF_TYPE_SOFTWARE] = "software",
> > + [PERF_TYPE_TRACEPOINT] = "tracepoint",
> > + [PERF_TYPE_HW_CACHE] = "hw-cache",
> > + [PERF_TYPE_RAW] = "raw",
> > + [PERF_TYPE_BREAKPOINT] = "breakpoint",
> > +};
> > +
> > +const char *event_symbols_hw[PERF_COUNT_HW_MAX] = {
> > + [PERF_COUNT_HW_CPU_CYCLES] = "cpu-cycles",
> > + [PERF_COUNT_HW_INSTRUCTIONS] = "instructions",
> > + [PERF_COUNT_HW_CACHE_REFERENCES] = "cache-references",
> > + [PERF_COUNT_HW_CACHE_MISSES] = "cache-misses",
> > + [PERF_COUNT_HW_BRANCH_INSTRUCTIONS] = "branch-instructions",
> > + [PERF_COUNT_HW_BRANCH_MISSES] = "branch-misses",
> > + [PERF_COUNT_HW_BUS_CYCLES] = "bus-cycles",
> > + [PERF_COUNT_HW_STALLED_CYCLES_FRONTEND] = "stalled-cycles-frontend",
> > + [PERF_COUNT_HW_STALLED_CYCLES_BACKEND] = "stalled-cycles-backend",
> > + [PERF_COUNT_HW_REF_CPU_CYCLES] = "ref-cycles",
> > +};
> > +
> > +const char *event_symbols_sw[PERF_COUNT_SW_MAX] = {
> > + [PERF_COUNT_SW_CPU_CLOCK] = "cpu-clock",
> > + [PERF_COUNT_SW_TASK_CLOCK] = "task-clock",
> > + [PERF_COUNT_SW_PAGE_FAULTS] = "page-faults",
> > + [PERF_COUNT_SW_CONTEXT_SWITCHES] = "context-switches",
> > + [PERF_COUNT_SW_CPU_MIGRATIONS] = "cpu-migrations",
> > + [PERF_COUNT_SW_PAGE_FAULTS_MIN] = "minor-faults",
> > + [PERF_COUNT_SW_PAGE_FAULTS_MAJ] = "major-faults",
> > + [PERF_COUNT_SW_ALIGNMENT_FAULTS] = "alignment-faults",
> > + [PERF_COUNT_SW_EMULATION_FAULTS] = "emulation-faults",
> > + [PERF_COUNT_SW_DUMMY] = "dummy",
> > + [PERF_COUNT_SW_BPF_OUTPUT] = "bpf-output",
> > + [PERF_COUNT_SW_CGROUP_SWITCHES] = "cgroup-switches",
> > +};
> > +
> > +const char *evsel__hw_cache[PERF_COUNT_HW_CACHE_MAX] = {
> > + [PERF_COUNT_HW_CACHE_L1D] = "L1-dcache",
> > + [PERF_COUNT_HW_CACHE_L1I] = "L1-icache",
> > + [PERF_COUNT_HW_CACHE_LL] = "LLC",
> > + [PERF_COUNT_HW_CACHE_DTLB] = "dTLB",
> > + [PERF_COUNT_HW_CACHE_ITLB] = "iTLB",
> > + [PERF_COUNT_HW_CACHE_BPU] = "branch",
> > + [PERF_COUNT_HW_CACHE_NODE] = "node",
> > +};
> > +
> > +const char *evsel__hw_cache_op[PERF_COUNT_HW_CACHE_OP_MAX] = {
> > + [PERF_COUNT_HW_CACHE_OP_READ] = "load",
> > + [PERF_COUNT_HW_CACHE_OP_WRITE] = "store",
> > + [PERF_COUNT_HW_CACHE_OP_PREFETCH] = "prefetch",
> > +};
> > +
> > +const char *evsel__hw_cache_result[PERF_COUNT_HW_CACHE_RESULT_MAX] = {
> > + [PERF_COUNT_HW_CACHE_RESULT_ACCESS] = "refs",
> > + [PERF_COUNT_HW_CACHE_RESULT_MISS] = "misses",
> > +};
> > +
> > +const char *perf_type_str(enum perf_type_id t)
> > +{
> > + if (t < 0 || t >= ARRAY_SIZE(perf_type_name))
> > + return NULL;
> > +
> > + return perf_type_name[t];
> > +}
> > +
> > +const char *perf_hw_str(enum perf_hw_id t)
> > +{
> > + if (t < 0 || t >= ARRAY_SIZE(event_symbols_hw))
> > + return NULL;
> > +
> > + return event_symbols_hw[t];
> > +}
> > +
> > +const char *perf_hw_cache_str(enum perf_hw_cache_id t)
> > +{
> > + if (t < 0 || t >= ARRAY_SIZE(evsel__hw_cache))
> > + return NULL;
> > +
> > + return evsel__hw_cache[t];
> > +}
> > +
> > +const char *perf_hw_cache_op_str(enum perf_hw_cache_op_id t)
> > +{
> > + if (t < 0 || t >= ARRAY_SIZE(evsel__hw_cache_op))
> > + return NULL;
> > +
> > + return evsel__hw_cache_op[t];
> > +}
> > +
> > +const char *perf_hw_cache_op_result_str(enum perf_hw_cache_op_result_id t)
> > +{
> > + if (t < 0 || t >= ARRAY_SIZE(evsel__hw_cache_result))
> > + return NULL;
> > +
> > + return evsel__hw_cache_result[t];
> > +}
> > +
> > +const char *perf_sw_str(enum perf_sw_ids t)
> > +{
> > + if (t < 0 || t >= ARRAY_SIZE(event_symbols_sw))
> > + return NULL;
> > +
> > + return event_symbols_sw[t];
> > +}
> >
> > /* 0: undecided, 1: supported, 2: not supported */
> > static int perf_query_supported;
> > diff --git a/tools/bpf/bpftool/perf.h b/tools/bpf/bpftool/perf.h
> > new file mode 100644
> > index 0000000..3fd7e42
> > --- /dev/null
> > +++ b/tools/bpf/bpftool/perf.h
> > @@ -0,0 +1,11 @@
> > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> > +/* Copyright (C) 2023 Yafang Shao <laoar.shao@gmail.com> */
> > +
> > +#include <linux/perf_event.h>
> > +
> > +const char *perf_type_str(enum perf_type_id t);
> > +const char *perf_hw_str(enum perf_hw_id t);
> > +const char *perf_hw_cache_str(enum perf_hw_cache_id t);
> > +const char *perf_hw_cache_op_str(enum perf_hw_cache_op_id t);
> > +const char *perf_hw_cache_op_result_str(enum perf_hw_cache_op_result_id t);
> > +const char *perf_sw_str(enum perf_sw_ids t);
>
> I'm not sure we need all these API functions if we keep the arrays in
> bpftool. I'd probably have just a generic one and pass it the name of
> the relevant array in argument. Although I've got no objection with the
> current form if it helps unifying the code with perf in the future.
>
Sure, I will use a generic one instead.
--
Regards
Yafang
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 10/10] bpftool: Show probed function in perf_event link info
2023-06-13 13:42 ` Quentin Monnet
@ 2023-06-13 15:11 ` Yafang Shao
0 siblings, 0 replies; 47+ messages in thread
From: Yafang Shao @ 2023-06-13 15:11 UTC (permalink / raw)
To: Quentin Monnet
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, rostedt, mhiramat, bpf,
linux-trace-kernel
On Tue, Jun 13, 2023 at 9:42 PM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2023-06-12 15:16 UTC+0000 ~ Yafang Shao <laoar.shao@gmail.com>
> > 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?
Agree. Will change it.
>
> 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.
They are really a bit long. Will remove the "_name" suffix.
>
> > 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@gmail.com>
> > ---
> > 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?
Right. Should use boolean instead.
>
> > + 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?
Good point. Will change it.
>
> > +{
> > +#define PERF_HW_CACHE_LEN 128
>
> Let's move the #define to the top of the file, please.
Agree.
>
> > + 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.
Agree. Will use switch-case instead.
>
> > + 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?
Agree. Should increase it to PATH_MAX.
>
> > + goto again;
> > + }
> > + }
> >
> > +out:
> > if (json_output)
> > show_link_close_json(fd, &info);
> > else
>
> Thanks for this work!
Many thanks for your review.
--
Regards
Yafang
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 03/10] bpftool: Show probed function in kprobe_multi link info
2023-06-12 15:16 ` [PATCH v3 bpf-next 03/10] bpftool: Show probed function in kprobe_multi link info Yafang Shao
2023-06-13 13:41 ` Quentin Monnet
@ 2023-06-13 22:36 ` Kui-Feng Lee
2023-06-14 2:42 ` Yafang Shao
2023-06-16 17:30 ` Andrii Nakryiko
2 siblings, 1 reply; 47+ messages in thread
From: Kui-Feng Lee @ 2023-06-13 22:36 UTC (permalink / raw)
To: Yafang Shao, ast, daniel, john.fastabend, andrii, martin.lau,
song, yhs, kpsingh, sdf, haoluo, jolsa, quentin, rostedt,
mhiramat
Cc: bpf, linux-trace-kernel
On 6/12/23 08:16, Yafang Shao wrote:
> Show the already expose kprobe_multi link info in bpftool. The result as
> follows,
>
> 52: kprobe_multi prog 381
> retprobe 0 func_cnt 7
> addrs ffffffff9ec44f20 funcs schedule_timeout_interruptible
> ffffffff9ec44f60 schedule_timeout_killable
> ffffffff9ec44fa0 schedule_timeout_uninterruptible
> ffffffff9ec44fe0 schedule_timeout_idle
> ffffffffc09468d0 xfs_trans_get_efd [xfs]
> ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
> ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
> pids kprobe_multi(559862)
> 53: kprobe_multi prog 381
> retprobe 1 func_cnt 7
> addrs ffffffff9ec44f20 funcs schedule_timeout_interruptible
> ffffffff9ec44f60 schedule_timeout_killable
> ffffffff9ec44fa0 schedule_timeout_uninterruptible
> ffffffff9ec44fe0 schedule_timeout_idle
> ffffffffc09468d0 xfs_trans_get_efd [xfs]
> ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
> ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
> pids kprobe_multi(559862)
>
> $ tools/bpf/bpftool/bpftool link show -j
> [{"id":52,"type":"kprobe_multi","prog_id":381,"retprobe":0,"func_cnt":7,"funcs":[{"addr":18446744072078249760,"func":"schedule_timeout_interruptible","module":""},{"addr":18446744072078249824,"func":"schedule_timeout_killable","module":""},{"addr":18446744072078249888,"func":"schedule_timeout_uninterruptible","module":""},{"addr":18446744072078249952,"func":"schedule_timeout_idle","module":""},{"addr":18446744072645535952,"func":"xfs_trans_get_efd","module":"[xfs]"},{"addr":18446744072645589520,"func":"xfs_trans_get_buf_map","module":"[xfs]"},{"addr":18446744072645604128,"func":"xfs_trans_get_dqtrx","module":"[xfs]"}],"pids":[{"pid":559862,"comm":"kprobe_multi"}]},{"id":53,"type":"kprobe_multi","prog_id":381,"retprobe":1,"func_cnt":7,"funcs":[{"addr":18446744072078249760,"func":"schedule_timeout_interruptible","module":""},{"addr":18446744072078249824,"func":"schedule_timeout_killable","module":""},{"addr":18446744072078249888,"func":"schedule_timeout_uninterruptible","module":""},{"addr":18446744072078249952,"func":"schedule_timeout_idle","module":""},{"addr":18446744072645535952,"func":"xfs_trans_get_efd","module":"[xfs]"},{"addr":18446744072645589520,"func":"xfs_trans_get_buf_map","module":"[xfs]"},{"addr":18446744072645604128,"func":"xfs_trans_get_dqtrx","module":"[xfs]"}],"pids":[{"pid":559862,"comm":"kprobe_multi"}]}]
>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
> tools/bpf/bpftool/link.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 108 insertions(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
> index 2d78607..0015582 100644
> --- a/tools/bpf/bpftool/link.c
> +++ b/tools/bpf/bpftool/link.c
> @@ -14,8 +14,10 @@
>
> #include "json_writer.h"
> #include "main.h"
> +#include "xlated_dumper.h"
>
> static struct hashmap *link_table;
> +static struct dump_data dd = {};
>
> static int link_parse_fd(int *argc, char ***argv)
> {
> @@ -166,6 +168,45 @@ static int get_prog_info(int prog_id, struct bpf_prog_info *info)
> return err;
> }
>
> +static int cmp_u64(const void *A, const void *B)
> +{
> + const __u64 *a = A, *b = B;
> +
> + return *a - *b;
> +}
> +
> +static void
> +show_kprobe_multi_json(struct bpf_link_info *info, json_writer_t *wtr)
> +{
> + __u32 i, j = 0;
> + __u64 *addrs;
> +
> + jsonw_uint_field(json_wtr, "retprobe",
> + info->kprobe_multi.flags & BPF_F_KPROBE_MULTI_RETURN);
> + jsonw_uint_field(json_wtr, "func_cnt", info->kprobe_multi.count);
> + jsonw_name(json_wtr, "funcs");
> + jsonw_start_array(json_wtr);
> + addrs = (__u64 *)u64_to_ptr(info->kprobe_multi.addrs);
> + qsort((void *)addrs, info->kprobe_multi.count, sizeof(__u64), cmp_u64);
> +
> + /* Load it once for all. */
> + if (!dd.sym_count)
> + kernel_syms_load(&dd);
> + for (i = 0; i < dd.sym_count; i++) {
> + if (dd.sym_mapping[i].address != addrs[j])
> + continue;
> + jsonw_start_object(json_wtr);
> + jsonw_uint_field(json_wtr, "addr", dd.sym_mapping[i].address);
> + jsonw_string_field(json_wtr, "func", dd.sym_mapping[i].name);
> + /* Print none if it is vmlinux */
> + jsonw_string_field(json_wtr, "module", dd.sym_mapping[i].module);
> + jsonw_end_object(json_wtr);
> + if (j++ == info->kprobe_multi.count)
> + break;
> + }
> + jsonw_end_array(json_wtr);
> +}
> +
> static int show_link_close_json(int fd, struct bpf_link_info *info)
> {
> struct bpf_prog_info prog_info;
> @@ -218,6 +259,9 @@ static int show_link_close_json(int fd, struct bpf_link_info *info)
> jsonw_uint_field(json_wtr, "map_id",
> info->struct_ops.map_id);
> break;
> + case BPF_LINK_TYPE_KPROBE_MULTI:
> + show_kprobe_multi_json(info, json_wtr);
> + break;
> default:
> break;
> }
> @@ -351,6 +395,44 @@ void netfilter_dump_plain(const struct bpf_link_info *info)
> printf(" flags 0x%x", info->netfilter.flags);
> }
>
> +static void show_kprobe_multi_plain(struct bpf_link_info *info)
> +{
> + __u32 i, j = 0;
> + __u64 *addrs;
> +
> + if (!info->kprobe_multi.count)
> + return;
> +
> + printf("\n\tretprobe %d func_cnt %u ",
> + info->kprobe_multi.flags & BPF_F_KPROBE_MULTI_RETURN,
> + info->kprobe_multi.count);
> + addrs = (__u64 *)u64_to_ptr(info->kprobe_multi.addrs);
> + qsort((void *)addrs, info->kprobe_multi.count, sizeof(__u64), cmp_u64);
> +
> + /* Load it once for all. */
> + if (!dd.sym_count)
> + kernel_syms_load(&dd);
> + for (i = 0; i < dd.sym_count; i++) {
> + if (dd.sym_mapping[i].address != addrs[j])
> + continue;
> + if (!j)
> + printf("\n\taddrs %016lx funcs %s",
> + dd.sym_mapping[i].address,
> + dd.sym_mapping[i].name);
> + else
> + printf("\n\t %016lx %s",
> + dd.sym_mapping[i].address,
> + dd.sym_mapping[i].name);
> + if (dd.sym_mapping[i].module[0] != '\0')
> + printf(" %s ", dd.sym_mapping[i].module);
> + else
> + printf(" ");
Could you explain what these extra spaces after module names are for?
> +
> + if (j++ == info->kprobe_multi.count)
> + break;
> + }
> +}
> +
> static int show_link_close_plain(int fd, struct bpf_link_info *info)
> {
> struct bpf_prog_info prog_info;
> @@ -396,6 +478,9 @@ static int show_link_close_plain(int fd, struct bpf_link_info *info)
> case BPF_LINK_TYPE_NETFILTER:
> netfilter_dump_plain(info);
> break;
> + case BPF_LINK_TYPE_KPROBE_MULTI:
> + show_kprobe_multi_plain(info);
> + break;
> default:
> break;
> }
> @@ -417,7 +502,9 @@ static int do_show_link(int fd)
> {
> struct bpf_link_info info;
> __u32 len = sizeof(info);
> + __u64 *addrs = NULL;
> char buf[256];
> + int count;
> int err;
>
> memset(&info, 0, sizeof(info));
> @@ -441,12 +528,28 @@ static int do_show_link(int fd)
> info.iter.target_name_len = sizeof(buf);
> goto again;
> }
> + if (info.type == BPF_LINK_TYPE_KPROBE_MULTI &&
> + !info.kprobe_multi.addrs) {
> + count = info.kprobe_multi.count;
> + if (count) {
> + addrs = calloc(count, sizeof(__u64));
> + if (!addrs) {
> + p_err("mem alloc failed");
> + close(fd);
> + return -1;
> + }
> + info.kprobe_multi.addrs = (unsigned long)addrs;
> + goto again;
> + }
> + }
>
> if (json_output)
> show_link_close_json(fd, &info);
> else
> show_link_close_plain(fd, &info);
>
> + if (addrs)
> + free(addrs);
> close(fd);
> return 0;
> }
> @@ -471,7 +574,8 @@ static int do_show(int argc, char **argv)
> fd = link_parse_fd(&argc, &argv);
> if (fd < 0)
> return fd;
> - return do_show_link(fd);
> + do_show_link(fd);
> + goto out;
> }
>
> if (argc)
> @@ -510,6 +614,9 @@ static int do_show(int argc, char **argv)
> if (show_pinned)
> delete_pinned_obj_table(link_table);
>
> +out:
> + if (dd.sym_count)
> + kernel_syms_destroy(&dd);
> return errno == ENOENT ? 0 : -1;
> }
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 08/10] bpf: Support ->fill_link_info for perf_event
2023-06-13 2:47 ` Yafang Shao
@ 2023-06-14 2:34 ` Kui-Feng Lee
2023-06-14 2:45 ` Yafang Shao
0 siblings, 1 reply; 47+ messages in thread
From: Kui-Feng Lee @ 2023-06-14 2:34 UTC (permalink / raw)
To: Yafang Shao, Yonghong Song
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat, bpf,
linux-trace-kernel
On 6/12/23 19:47, Yafang Shao wrote:
> On Tue, Jun 13, 2023 at 1:36 AM Yonghong Song <yhs@meta.com> wrote:
>>
>>
>>
>> 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.
>
> If we put it into each individual struct, we have to choose one
> specific struct to get the type before we use the real struct, for
> example,
> if (info.perf_event.type == BPF_PERF_LINK_PERF_EVENT)
> goto out;
> if (info.perf_event.type == BPF_PERF_LINK_TRACEPOINT &&
> !info.tracepoint.tp_name) {
> info.tracepoint.tp_name = (unsigned long)&buf;
> info.tracepoint.name_len = sizeof(buf);
> goto again;
> }
> ...
>
> That doesn't look perfect.
How about adding a common struct?
struct {
__u32 type;
} perf_common;
Then you check info.perf_common.type.
>
> However I agree with you that the perf_link_type may disallow the
> extensions for the big union. I will think about it.
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 03/10] bpftool: Show probed function in kprobe_multi link info
2023-06-13 22:36 ` Kui-Feng Lee
@ 2023-06-14 2:42 ` Yafang Shao
2023-06-14 8:33 ` Quentin Monnet
0 siblings, 1 reply; 47+ messages in thread
From: Yafang Shao @ 2023-06-14 2:42 UTC (permalink / raw)
To: Kui-Feng Lee
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat, bpf,
linux-trace-kernel
On Wed, Jun 14, 2023 at 6:36 AM Kui-Feng Lee <sinquersw@gmail.com> wrote:
>
>
>
> On 6/12/23 08:16, Yafang Shao wrote:
> > Show the already expose kprobe_multi link info in bpftool. The result as
> > follows,
> >
> > 52: kprobe_multi prog 381
> > retprobe 0 func_cnt 7
> > addrs ffffffff9ec44f20 funcs schedule_timeout_interruptible
> > ffffffff9ec44f60 schedule_timeout_killable
> > ffffffff9ec44fa0 schedule_timeout_uninterruptible
> > ffffffff9ec44fe0 schedule_timeout_idle
> > ffffffffc09468d0 xfs_trans_get_efd [xfs]
> > ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
> > ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
> > pids kprobe_multi(559862)
> > 53: kprobe_multi prog 381
> > retprobe 1 func_cnt 7
> > addrs ffffffff9ec44f20 funcs schedule_timeout_interruptible
> > ffffffff9ec44f60 schedule_timeout_killable
> > ffffffff9ec44fa0 schedule_timeout_uninterruptible
> > ffffffff9ec44fe0 schedule_timeout_idle
> > ffffffffc09468d0 xfs_trans_get_efd [xfs]
> > ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
> > ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
> > pids kprobe_multi(559862)
> >
> > $ tools/bpf/bpftool/bpftool link show -j
> > [{"id":52,"type":"kprobe_multi","prog_id":381,"retprobe":0,"func_cnt":7,"funcs":[{"addr":18446744072078249760,"func":"schedule_timeout_interruptible","module":""},{"addr":18446744072078249824,"func":"schedule_timeout_killable","module":""},{"addr":18446744072078249888,"func":"schedule_timeout_uninterruptible","module":""},{"addr":18446744072078249952,"func":"schedule_timeout_idle","module":""},{"addr":18446744072645535952,"func":"xfs_trans_get_efd","module":"[xfs]"},{"addr":18446744072645589520,"func":"xfs_trans_get_buf_map","module":"[xfs]"},{"addr":18446744072645604128,"func":"xfs_trans_get_dqtrx","module":"[xfs]"}],"pids":[{"pid":559862,"comm":"kprobe_multi"}]},{"id":53,"type":"kprobe_multi","prog_id":381,"retprobe":1,"func_cnt":7,"funcs":[{"addr":18446744072078249760,"func":"schedule_timeout_interruptible","module":""},{"addr":18446744072078249824,"func":"schedule_timeout_killable","module":""},{"addr":18446744072078249888,"func":"schedule_timeout_uninterruptible","module":""},{"addr":18446744072078249952,"func":"schedule_timeout_idle","module":""},{"addr":18446744072645535952,"func":"xfs_trans_get_efd","module":"[xfs]"},{"addr":18446744072645589520,"func":"xfs_trans_get_buf_map","module":"[xfs]"},{"addr":18446744072645604128,"func":"xfs_trans_get_dqtrx","module":"[xfs]"}],"pids":[{"pid":559862,"comm":"kprobe_multi"}]}]
> >
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > ---
> > tools/bpf/bpftool/link.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 108 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
> > index 2d78607..0015582 100644
> > --- a/tools/bpf/bpftool/link.c
> > +++ b/tools/bpf/bpftool/link.c
> > @@ -14,8 +14,10 @@
> >
> > #include "json_writer.h"
> > #include "main.h"
> > +#include "xlated_dumper.h"
> >
> > static struct hashmap *link_table;
> > +static struct dump_data dd = {};
> >
> > static int link_parse_fd(int *argc, char ***argv)
> > {
> > @@ -166,6 +168,45 @@ static int get_prog_info(int prog_id, struct bpf_prog_info *info)
> > return err;
> > }
> >
> > +static int cmp_u64(const void *A, const void *B)
> > +{
> > + const __u64 *a = A, *b = B;
> > +
> > + return *a - *b;
> > +}
> > +
> > +static void
> > +show_kprobe_multi_json(struct bpf_link_info *info, json_writer_t *wtr)
> > +{
> > + __u32 i, j = 0;
> > + __u64 *addrs;
> > +
> > + jsonw_uint_field(json_wtr, "retprobe",
> > + info->kprobe_multi.flags & BPF_F_KPROBE_MULTI_RETURN);
> > + jsonw_uint_field(json_wtr, "func_cnt", info->kprobe_multi.count);
> > + jsonw_name(json_wtr, "funcs");
> > + jsonw_start_array(json_wtr);
> > + addrs = (__u64 *)u64_to_ptr(info->kprobe_multi.addrs);
> > + qsort((void *)addrs, info->kprobe_multi.count, sizeof(__u64), cmp_u64);
> > +
> > + /* Load it once for all. */
> > + if (!dd.sym_count)
> > + kernel_syms_load(&dd);
> > + for (i = 0; i < dd.sym_count; i++) {
> > + if (dd.sym_mapping[i].address != addrs[j])
> > + continue;
> > + jsonw_start_object(json_wtr);
> > + jsonw_uint_field(json_wtr, "addr", dd.sym_mapping[i].address);
> > + jsonw_string_field(json_wtr, "func", dd.sym_mapping[i].name);
> > + /* Print none if it is vmlinux */
> > + jsonw_string_field(json_wtr, "module", dd.sym_mapping[i].module);
> > + jsonw_end_object(json_wtr);
> > + if (j++ == info->kprobe_multi.count)
> > + break;
> > + }
> > + jsonw_end_array(json_wtr);
> > +}
> > +
> > static int show_link_close_json(int fd, struct bpf_link_info *info)
> > {
> > struct bpf_prog_info prog_info;
> > @@ -218,6 +259,9 @@ static int show_link_close_json(int fd, struct bpf_link_info *info)
> > jsonw_uint_field(json_wtr, "map_id",
> > info->struct_ops.map_id);
> > break;
> > + case BPF_LINK_TYPE_KPROBE_MULTI:
> > + show_kprobe_multi_json(info, json_wtr);
> > + break;
> > default:
> > break;
> > }
> > @@ -351,6 +395,44 @@ void netfilter_dump_plain(const struct bpf_link_info *info)
> > printf(" flags 0x%x", info->netfilter.flags);
> > }
> >
> > +static void show_kprobe_multi_plain(struct bpf_link_info *info)
> > +{
> > + __u32 i, j = 0;
> > + __u64 *addrs;
> > +
> > + if (!info->kprobe_multi.count)
> > + return;
> > +
> > + printf("\n\tretprobe %d func_cnt %u ",
> > + info->kprobe_multi.flags & BPF_F_KPROBE_MULTI_RETURN,
> > + info->kprobe_multi.count);
> > + addrs = (__u64 *)u64_to_ptr(info->kprobe_multi.addrs);
> > + qsort((void *)addrs, info->kprobe_multi.count, sizeof(__u64), cmp_u64);
> > +
> > + /* Load it once for all. */
> > + if (!dd.sym_count)
> > + kernel_syms_load(&dd);
> > + for (i = 0; i < dd.sym_count; i++) {
> > + if (dd.sym_mapping[i].address != addrs[j])
> > + continue;
> > + if (!j)
> > + printf("\n\taddrs %016lx funcs %s",
> > + dd.sym_mapping[i].address,
> > + dd.sym_mapping[i].name);
> > + else
> > + printf("\n\t %016lx %s",
> > + dd.sym_mapping[i].address,
> > + dd.sym_mapping[i].name);
> > + if (dd.sym_mapping[i].module[0] != '\0')
> > + printf(" %s ", dd.sym_mapping[i].module);
> > + else
> > + printf(" ");
>
> Could you explain what these extra spaces after module names are for?
There are two spaces. We use two spaces to seperate different items
printed in bpftool. For example,
"4: kprobe_multi prog 16"
There are two spaces between the "type" and the "prog".
We always print these two spaces after one item is printed:
printf("type %u ", info->type);
printf("prog %u ", info->prog_id);
That way, we can add new item easily and consistently.
--
Regards
Yafang
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 08/10] bpf: Support ->fill_link_info for perf_event
2023-06-14 2:34 ` Kui-Feng Lee
@ 2023-06-14 2:45 ` Yafang Shao
2023-06-16 20:36 ` Andrii Nakryiko
0 siblings, 1 reply; 47+ messages in thread
From: Yafang Shao @ 2023-06-14 2:45 UTC (permalink / raw)
To: Kui-Feng Lee
Cc: Yonghong Song, ast, daniel, john.fastabend, andrii, martin.lau,
song, yhs, kpsingh, sdf, haoluo, jolsa, quentin, rostedt,
mhiramat, bpf, linux-trace-kernel
On Wed, Jun 14, 2023 at 10:34 AM Kui-Feng Lee <sinquersw@gmail.com> wrote:
>
>
>
> On 6/12/23 19:47, Yafang Shao wrote:
> > On Tue, Jun 13, 2023 at 1:36 AM Yonghong Song <yhs@meta.com> wrote:
> >>
> >>
> >>
> >> 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.
> >
> > If we put it into each individual struct, we have to choose one
> > specific struct to get the type before we use the real struct, for
> > example,
> > if (info.perf_event.type == BPF_PERF_LINK_PERF_EVENT)
> > goto out;
> > if (info.perf_event.type == BPF_PERF_LINK_TRACEPOINT &&
> > !info.tracepoint.tp_name) {
> > info.tracepoint.tp_name = (unsigned long)&buf;
> > info.tracepoint.name_len = sizeof(buf);
> > goto again;
> > }
> > ...
> >
> > That doesn't look perfect.
>
> How about adding a common struct?
>
> struct {
> __u32 type;
> } perf_common;
>
> Then you check info.perf_common.type.
I perfer below struct,
struct {
__u32 type; /* enum bpf_perf_link_type */
union {
struct {
__u64 config;
__u32 type;
} perf_event; /* BPF_PERF_LINK_PERF_EVENT */
struct {
__aligned_u64 file_name; /* in/out */
__u32 name_len;
__u32 offset;/* offset from file_name */
__u32 flags;
} uprobe; /* BPF_PERF_LINK_UPROBE */
struct {
__aligned_u64 func_name; /* in/out */
__u32 name_len;
__u32 offset;/* offset from func_name */
__u64 addr;
__u32 flags;
} kprobe; /* BPF_PERF_LINK_KPROBE */
struct {
__aligned_u64 tp_name; /* in/out */
__u32 name_len;
} tracepoint; /* BPF_PERF_LINK_TRACEPOINT */
};
} perf_link;
I think that would be more clear.
--
Regards
Yafang
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 03/10] bpftool: Show probed function in kprobe_multi link info
2023-06-14 2:42 ` Yafang Shao
@ 2023-06-14 8:33 ` Quentin Monnet
0 siblings, 0 replies; 47+ messages in thread
From: Quentin Monnet @ 2023-06-14 8:33 UTC (permalink / raw)
To: Yafang Shao, Kui-Feng Lee
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, rostedt, mhiramat, bpf,
linux-trace-kernel
2023-06-14 10:42 UTC+0800 ~ Yafang Shao <laoar.shao@gmail.com>
> On Wed, Jun 14, 2023 at 6:36 AM Kui-Feng Lee <sinquersw@gmail.com> wrote:
>>
>>
>>
>> On 6/12/23 08:16, Yafang Shao wrote:
>>> Show the already expose kprobe_multi link info in bpftool. The result as
>>> follows,
>>>
>>> 52: kprobe_multi prog 381
>>> retprobe 0 func_cnt 7
>>> addrs ffffffff9ec44f20 funcs schedule_timeout_interruptible
>>> ffffffff9ec44f60 schedule_timeout_killable
>>> ffffffff9ec44fa0 schedule_timeout_uninterruptible
>>> ffffffff9ec44fe0 schedule_timeout_idle
>>> ffffffffc09468d0 xfs_trans_get_efd [xfs]
>>> ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
>>> ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
>>> pids kprobe_multi(559862)
>>> 53: kprobe_multi prog 381
>>> retprobe 1 func_cnt 7
>>> addrs ffffffff9ec44f20 funcs schedule_timeout_interruptible
>>> ffffffff9ec44f60 schedule_timeout_killable
>>> ffffffff9ec44fa0 schedule_timeout_uninterruptible
>>> ffffffff9ec44fe0 schedule_timeout_idle
>>> ffffffffc09468d0 xfs_trans_get_efd [xfs]
>>> ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
>>> ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
>>> pids kprobe_multi(559862)
>>>
>>> $ tools/bpf/bpftool/bpftool link show -j
>>> [{"id":52,"type":"kprobe_multi","prog_id":381,"retprobe":0,"func_cnt":7,"funcs":[{"addr":18446744072078249760,"func":"schedule_timeout_interruptible","module":""},{"addr":18446744072078249824,"func":"schedule_timeout_killable","module":""},{"addr":18446744072078249888,"func":"schedule_timeout_uninterruptible","module":""},{"addr":18446744072078249952,"func":"schedule_timeout_idle","module":""},{"addr":18446744072645535952,"func":"xfs_trans_get_efd","module":"[xfs]"},{"addr":18446744072645589520,"func":"xfs_trans_get_buf_map","module":"[xfs]"},{"addr":18446744072645604128,"func":"xfs_trans_get_dqtrx","module":"[xfs]"}],"pids":[{"pid":559862,"comm":"kprobe_multi"}]},{"id":53,"type":"kprobe_multi","prog_id":381,"retprobe":1,"func_cnt":7,"funcs":[{"addr":18446744072078249760,"func":"schedule_timeout_interruptible","module":""},{"addr":18446744072078249824,"func":"schedule_timeout_killable","module":""},{"addr":18446744072078249888,"func":"schedule_timeout_uninterruptible","module":""},{"addr":18446744072078249952,"func":"schedule_timeout_idle","module":""},{"addr":18446744072645535952,"func":"xfs_trans_get_efd","module":"[xfs]"},{"addr":18446744072645589520,"func":"xfs_trans_get_buf_map","module":"[xfs]"},{"addr":18446744072645604128,"func":"xfs_trans_get_dqtrx","module":"[xfs]"}],"pids":[{"pid":559862,"comm":"kprobe_multi"}]}]
>>>
>>> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
>>> ---
>>> tools/bpf/bpftool/link.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++-
>>> 1 file changed, 108 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
>>> index 2d78607..0015582 100644
>>> --- a/tools/bpf/bpftool/link.c
>>> +++ b/tools/bpf/bpftool/link.c
>>> @@ -14,8 +14,10 @@
>>>
>>> #include "json_writer.h"
>>> #include "main.h"
>>> +#include "xlated_dumper.h"
>>>
>>> static struct hashmap *link_table;
>>> +static struct dump_data dd = {};
>>>
>>> static int link_parse_fd(int *argc, char ***argv)
>>> {
>>> @@ -166,6 +168,45 @@ static int get_prog_info(int prog_id, struct bpf_prog_info *info)
>>> return err;
>>> }
>>>
>>> +static int cmp_u64(const void *A, const void *B)
>>> +{
>>> + const __u64 *a = A, *b = B;
>>> +
>>> + return *a - *b;
>>> +}
>>> +
>>> +static void
>>> +show_kprobe_multi_json(struct bpf_link_info *info, json_writer_t *wtr)
>>> +{
>>> + __u32 i, j = 0;
>>> + __u64 *addrs;
>>> +
>>> + jsonw_uint_field(json_wtr, "retprobe",
>>> + info->kprobe_multi.flags & BPF_F_KPROBE_MULTI_RETURN);
>>> + jsonw_uint_field(json_wtr, "func_cnt", info->kprobe_multi.count);
>>> + jsonw_name(json_wtr, "funcs");
>>> + jsonw_start_array(json_wtr);
>>> + addrs = (__u64 *)u64_to_ptr(info->kprobe_multi.addrs);
>>> + qsort((void *)addrs, info->kprobe_multi.count, sizeof(__u64), cmp_u64);
>>> +
>>> + /* Load it once for all. */
>>> + if (!dd.sym_count)
>>> + kernel_syms_load(&dd);
>>> + for (i = 0; i < dd.sym_count; i++) {
>>> + if (dd.sym_mapping[i].address != addrs[j])
>>> + continue;
>>> + jsonw_start_object(json_wtr);
>>> + jsonw_uint_field(json_wtr, "addr", dd.sym_mapping[i].address);
>>> + jsonw_string_field(json_wtr, "func", dd.sym_mapping[i].name);
>>> + /* Print none if it is vmlinux */
>>> + jsonw_string_field(json_wtr, "module", dd.sym_mapping[i].module);
>>> + jsonw_end_object(json_wtr);
>>> + if (j++ == info->kprobe_multi.count)
>>> + break;
>>> + }
>>> + jsonw_end_array(json_wtr);
>>> +}
>>> +
>>> static int show_link_close_json(int fd, struct bpf_link_info *info)
>>> {
>>> struct bpf_prog_info prog_info;
>>> @@ -218,6 +259,9 @@ static int show_link_close_json(int fd, struct bpf_link_info *info)
>>> jsonw_uint_field(json_wtr, "map_id",
>>> info->struct_ops.map_id);
>>> break;
>>> + case BPF_LINK_TYPE_KPROBE_MULTI:
>>> + show_kprobe_multi_json(info, json_wtr);
>>> + break;
>>> default:
>>> break;
>>> }
>>> @@ -351,6 +395,44 @@ void netfilter_dump_plain(const struct bpf_link_info *info)
>>> printf(" flags 0x%x", info->netfilter.flags);
>>> }
>>>
>>> +static void show_kprobe_multi_plain(struct bpf_link_info *info)
>>> +{
>>> + __u32 i, j = 0;
>>> + __u64 *addrs;
>>> +
>>> + if (!info->kprobe_multi.count)
>>> + return;
>>> +
>>> + printf("\n\tretprobe %d func_cnt %u ",
>>> + info->kprobe_multi.flags & BPF_F_KPROBE_MULTI_RETURN,
>>> + info->kprobe_multi.count);
>>> + addrs = (__u64 *)u64_to_ptr(info->kprobe_multi.addrs);
>>> + qsort((void *)addrs, info->kprobe_multi.count, sizeof(__u64), cmp_u64);
>>> +
>>> + /* Load it once for all. */
>>> + if (!dd.sym_count)
>>> + kernel_syms_load(&dd);
>>> + for (i = 0; i < dd.sym_count; i++) {
>>> + if (dd.sym_mapping[i].address != addrs[j])
>>> + continue;
>>> + if (!j)
>>> + printf("\n\taddrs %016lx funcs %s",
>>> + dd.sym_mapping[i].address,
>>> + dd.sym_mapping[i].name);
>>> + else
>>> + printf("\n\t %016lx %s",
>>> + dd.sym_mapping[i].address,
>>> + dd.sym_mapping[i].name);
>>> + if (dd.sym_mapping[i].module[0] != '\0')
>>> + printf(" %s ", dd.sym_mapping[i].module);
>>> + else
>>> + printf(" ");
>>
>> Could you explain what these extra spaces after module names are for?
>
> There are two spaces. We use two spaces to seperate different items
> printed in bpftool. For example,
> "4: kprobe_multi prog 16"
> There are two spaces between the "type" and the "prog".
>
> We always print these two spaces after one item is printed:
> printf("type %u ", info->type);
> printf("prog %u ", info->prog_id);
> That way, we can add new item easily and consistently.
>
I'm not sure we _always_ do it at the end of lines, but it seems to be
rather consistent in link.c at least. For my part I don't know if this
printf(" ") is really necessary - I'd rather avoid spaces at the end of
lines, but then there would be several other locations to update. So I
don't have any objection, either.
Quentin
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 01/10] bpf: Support ->fill_link_info for kprobe_multi
2023-06-12 15:15 ` [PATCH v3 bpf-next 01/10] bpf: Support ->fill_link_info for kprobe_multi Yafang Shao
@ 2023-06-15 8:29 ` Jiri Olsa
2023-06-15 12:09 ` Yafang Shao
2023-06-16 17:24 ` Andrii Nakryiko
1 sibling, 1 reply; 47+ messages in thread
From: Jiri Olsa @ 2023-06-15 8:29 UTC (permalink / raw)
To: Yafang Shao
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, quentin, rostedt, mhiramat, bpf,
linux-trace-kernel
On Mon, Jun 12, 2023 at 03:15:59PM +0000, Yafang Shao wrote:
SNIP
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 2bc41e6..742047c 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2548,9 +2548,36 @@ static void bpf_kprobe_multi_link_dealloc(struct bpf_link *link)
> kfree(kmulti_link);
> }
>
> +static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link,
> + struct bpf_link_info *info)
> +{
> + u64 __user *uaddrs = u64_to_user_ptr(info->kprobe_multi.addrs);
> + struct bpf_kprobe_multi_link *kmulti_link;
> + u32 ucount = info->kprobe_multi.count;
> +
> + if (!uaddrs ^ !ucount)
> + return -EINVAL;
> +
> + kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link);
> + if (!uaddrs) {
> + info->kprobe_multi.count = kmulti_link->cnt;
> + return 0;
> + }
> +
> + if (ucount < kmulti_link->cnt)
> + return -EINVAL;
> + info->kprobe_multi.flags = kmulti_link->fp.flags;
> + if (!kallsyms_show_value(current_cred()))
> + return 0;
> + if (copy_to_user(uaddrs, kmulti_link->addrs, ucount * sizeof(u64)))
> + return -EFAULT;
> + return 0;
> +}
> +
> static const struct bpf_link_ops bpf_kprobe_multi_link_lops = {
> .release = bpf_kprobe_multi_link_release,
> .dealloc = bpf_kprobe_multi_link_dealloc,
> + .fill_link_info = bpf_kprobe_multi_link_fill_link_info,
> };
>
> static void bpf_kprobe_multi_cookie_swap(void *a, void *b, int size, const void *priv)
> @@ -2890,6 +2917,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
> return err;
> }
>
> + link->fp.flags = flags;
hum this looks wrong, we can't use fprobe flags to store our flags
you should add flags to bpf_kprobe_multi_link
jirka
> return bpf_link_settle(&link_primer);
>
> error:
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index a7b5e91..23691ea 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -6438,6 +6438,11 @@ struct bpf_link_info {
> __s32 priority;
> __u32 flags;
> } netfilter;
> + struct {
> + __aligned_u64 addrs; /* in/out: addresses buffer ptr */
> + __u32 count;
> + __u32 flags;
> + } kprobe_multi;
> };
> } __attribute__((aligned(8)));
>
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 00/10] bpf: Support ->fill_link_info for kprobe_multi and perf_event links
2023-06-12 15:15 [PATCH v3 bpf-next 00/10] bpf: Support ->fill_link_info for kprobe_multi and perf_event links Yafang Shao
` (9 preceding siblings ...)
2023-06-12 15:16 ` [PATCH v3 bpf-next 10/10] bpftool: Show probed function in perf_event link info Yafang Shao
@ 2023-06-15 10:04 ` Jiri Olsa
2023-06-15 12:09 ` Yafang Shao
10 siblings, 1 reply; 47+ messages in thread
From: Jiri Olsa @ 2023-06-15 10:04 UTC (permalink / raw)
To: Yafang Shao
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, quentin, rostedt, mhiramat, bpf,
linux-trace-kernel
On Mon, Jun 12, 2023 at 03:15:58PM +0000, Yafang Shao wrote:
> This patchset enhances the usability of kprobe_multi programs by introducing
> support for ->fill_link_info. This allows users to easily determine the
> probed functions associated with a kprobe_multi program. While
> `bpftool perf show` already provides information about functions probed by
> perf_event programs, supporting ->fill_link_info ensures consistent access to
> this information across all bpf links.
>
> In addition, this patch extends support to generic perf events, which are
> currently not covered by `bpftool perf show`. While userspace is exposed to
> only the perf type and config, other attributes such as sample_period and
> sample_freq are disregarded.
>
> To ensure accurate identification of probed functions, it is preferable to
> expose the address directly rather than relying solely on the symbol name.
> However, this implementation respects the kptr_restrict setting and avoids
> exposing the address if it is not permitted.
>
> v2->v3:
> - Expose flags instead of retporbe (Andrii)
> - Simplify the check on kmulti_link->cnt (Andrii)
> - Use kallsyms_show_value() instead (Andrii)
> - Show also the module name for kprobe_multi (Andrii)
> - Add new enum bpf_perf_link_type (Andrii)
> - Move perf event names into bpftool (Andrii, Quentin, Jiri)
> - Keep perf event names in sync with perf tools (Jiri)
hi,
I'm getting some failing tests with this version:
#11/2 bpf_cookie/multi_kprobe_link_api:FAIL
#11/3 bpf_cookie/multi_kprobe_attach_api:FAIL
#11 bpf_cookie:FAIL
#104/1 kprobe_multi_test/skel_api:FAIL
#104/2 kprobe_multi_test/link_api_addrs:FAIL
#104/3 kprobe_multi_test/link_api_syms:FAIL
#104/4 kprobe_multi_test/attach_api_pattern:FAIL
#104/5 kprobe_multi_test/attach_api_addrs:FAIL
#104/6 kprobe_multi_test/attach_api_syms:FAIL
#104 kprobe_multi_test:FAIL
#105/1 kprobe_multi_testmod_test/testmod_attach_api_syms:FAIL
#105/2 kprobe_multi_testmod_test/testmod_attach_api_addrs:FAIL
#105 kprobe_multi_testmod_test:FAIL
jirka
>
> v1->v2:
> - Fix sparse warning (Stanislav, lkp@intel.com)
> - Fix BPF CI build error
> - Reuse kernel_syms_load() (Alexei)
> - Print 'name' instead of 'func' (Alexei)
> - Show whether the probe is retprobe or not (Andrii)
> - Add comment for the meaning of perf_event name (Andrii)
> - Add support for generic perf event
> - Adhere to the kptr_restrict setting
>
> RFC->v1:
> - Use a single copy_to_user() instead (Jiri)
> - Show also the symbol name in bpftool (Quentin, Alexei)
> - Use calloc() instead of malloc() in bpftool (Quentin)
> - Avoid having conditional entries in the JSON output (Quentin)
> - Drop ->show_fdinfo (Alexei)
> - Use __u64 instead of __aligned_u64 for the field addr (Alexei)
> - Avoid the contradiction in perf_event name length (Alexei)
> - Address a build warning reported by kernel test robot <lkp@intel.com>
>
> Yafang Shao (10):
> bpf: Support ->fill_link_info for kprobe_multi
> bpftool: Dump the kernel symbol's module name
> bpftool: Show probed function in kprobe_multi link info
> bpf: Protect probed address based on kptr_restrict setting
> bpf: Clear the probe_addr for uprobe
> bpf: Expose symbol's respective address
> bpf: Add a common helper bpf_copy_to_user()
> bpf: Support ->fill_link_info for perf_event
> bpftool: Add perf event names
> bpftool: Show probed function in perf_event link info
>
> include/uapi/linux/bpf.h | 37 +++++
> kernel/bpf/syscall.c | 158 +++++++++++++++++--
> kernel/trace/bpf_trace.c | 32 +++-
> kernel/trace/trace_kprobe.c | 7 +-
> tools/bpf/bpftool/link.c | 322 +++++++++++++++++++++++++++++++++++++-
> tools/bpf/bpftool/perf.c | 107 +++++++++++++
> tools/bpf/bpftool/perf.h | 11 ++
> tools/bpf/bpftool/xlated_dumper.c | 6 +-
> tools/bpf/bpftool/xlated_dumper.h | 2 +
> tools/include/uapi/linux/bpf.h | 37 +++++
> 10 files changed, 700 insertions(+), 19 deletions(-)
> create mode 100644 tools/bpf/bpftool/perf.h
>
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 08/10] bpf: Support ->fill_link_info for perf_event
2023-06-12 15:16 ` [PATCH v3 bpf-next 08/10] bpf: Support ->fill_link_info for perf_event Yafang Shao
2023-06-12 17:36 ` Yonghong Song
@ 2023-06-15 10:21 ` Jiri Olsa
2023-06-15 12:10 ` Yafang Shao
1 sibling, 1 reply; 47+ messages in thread
From: Jiri Olsa @ 2023-06-15 10:21 UTC (permalink / raw)
To: Yafang Shao
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, quentin, rostedt, mhiramat, bpf,
linux-trace-kernel
On Mon, Jun 12, 2023 at 03:16:06PM +0000, Yafang Shao wrote:
SNIP
>
> /* User bpf_sock_addr struct to access socket fields and sockaddr struct passed
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 80c9ec0..fe354d5 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -3303,9 +3303,133 @@ static void bpf_perf_link_dealloc(struct bpf_link *link)
> kfree(perf_link);
> }
>
> +static int bpf_perf_link_fill_name(const struct perf_event *event,
> + char __user *uname, u32 ulen,
> + u64 *probe_offset, u64 *probe_addr,
> + u32 *fd_type)
> +{
this function name sounds misleading, it does query all the link data
plus copying the name.. seems like this should be renamed and separated
> + const char *buf;
> + u32 prog_id;
> + size_t len;
> + int err;
> +
> + if (!ulen ^ !uname)
> + return -EINVAL;
> + if (!uname)
> + return 0;
> +
> + err = bpf_get_perf_event_info(event, &prog_id, fd_type, &buf,
> + probe_offset, probe_addr);
> + if (err)
> + return err;
> +
> + len = strlen(buf);
> + if (buf) {
> + err = bpf_copy_to_user(uname, buf, ulen, len);
> + if (err)
> + return err;
> + } else {
> + char zero = '\0';
> +
> + if (put_user(zero, uname))
> + return -EFAULT;
> + }
> + return 0;
> +}
> +
> +static int bpf_perf_link_fill_probe(const struct perf_event *event,
> + struct bpf_link_info *info)
> +{
> + char __user *uname;
> + u64 addr, offset;
> + u32 ulen, type;
> + int err;
> +
> +#ifdef CONFIG_KPROBE_EVENTS
this will break compilation when CONFIG_KPROBE_EVENTS or CONFIG_UPROBE_EVENTS
options are not defined
jirka
> + if (event->tp_event->flags & TRACE_EVENT_FL_KPROBE) {
> + uname = u64_to_user_ptr(info->kprobe.func_name);
> + ulen = info->kprobe.name_len;
> + info->perf_link_type = BPF_PERF_LINK_KPROBE;
> + err = bpf_perf_link_fill_name(event, uname, ulen, &offset,
> + &addr, &type);
> + if (err)
> + return err;
> +
> + info->kprobe.offset = offset;
> + if (type == BPF_FD_TYPE_KRETPROBE)
> + info->kprobe.flags = 1;
> + if (!kallsyms_show_value(current_cred()))
> + return 0;
> + info->kprobe.addr = addr;
> + return 0;
> + }
> +#endif
> +
> +#ifdef CONFIG_UPROBE_EVENTS
> + if (event->tp_event->flags & TRACE_EVENT_FL_UPROBE) {
> + uname = u64_to_user_ptr(info->uprobe.file_name);
> + ulen = info->uprobe.name_len;
> + info->perf_link_type = BPF_PERF_LINK_UPROBE;
> + err = bpf_perf_link_fill_name(event, uname, ulen, &offset,
> + &addr, &type);
> + if (err)
> + return err;
> +
> + info->uprobe.offset = offset;
> + if (type == BPF_FD_TYPE_URETPROBE)
> + info->uprobe.flags = 1;
> + return 0;
> + }
> +#endif
> +
> + return -EOPNOTSUPP;
> +}
> +
SNIP
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 09/10] bpftool: Add perf event names
2023-06-12 15:16 ` [PATCH v3 bpf-next 09/10] bpftool: Add perf event names Yafang Shao
2023-06-13 13:41 ` Quentin Monnet
@ 2023-06-15 10:23 ` Jiri Olsa
1 sibling, 0 replies; 47+ messages in thread
From: Jiri Olsa @ 2023-06-15 10:23 UTC (permalink / raw)
To: Yafang Shao
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, quentin, rostedt, mhiramat, bpf,
linux-trace-kernel, Jiri Olsa
On Mon, Jun 12, 2023 at 03:16:07PM +0000, Yafang Shao wrote:
> Add new functions and macros to get perf event names. These names are
> copied from tool/perf/util/{parse-events,evsel}.c, so that in the future we
> will have a good chance to use the same code.
>
> Suggested-by: Jiri Olsa <olsajiri@gmail.com>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
> tools/bpf/bpftool/perf.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++
> tools/bpf/bpftool/perf.h | 11 +++++
> 2 files changed, 118 insertions(+)
> create mode 100644 tools/bpf/bpftool/perf.h
>
> diff --git a/tools/bpf/bpftool/perf.c b/tools/bpf/bpftool/perf.c
> index 9174344..fbdf88c 100644
> --- a/tools/bpf/bpftool/perf.c
> +++ b/tools/bpf/bpftool/perf.c
> @@ -18,6 +18,113 @@
> #include <bpf/bpf.h>
>
> #include "main.h"
> +#include "perf.h"
> +
> +static const char *perf_type_name[PERF_TYPE_MAX] = {
> + [PERF_TYPE_HARDWARE] = "hardware",
> + [PERF_TYPE_SOFTWARE] = "software",
> + [PERF_TYPE_TRACEPOINT] = "tracepoint",
> + [PERF_TYPE_HW_CACHE] = "hw-cache",
> + [PERF_TYPE_RAW] = "raw",
> + [PERF_TYPE_BREAKPOINT] = "breakpoint",
> +};
> +
> +const char *event_symbols_hw[PERF_COUNT_HW_MAX] = {
> + [PERF_COUNT_HW_CPU_CYCLES] = "cpu-cycles",
> + [PERF_COUNT_HW_INSTRUCTIONS] = "instructions",
> + [PERF_COUNT_HW_CACHE_REFERENCES] = "cache-references",
> + [PERF_COUNT_HW_CACHE_MISSES] = "cache-misses",
> + [PERF_COUNT_HW_BRANCH_INSTRUCTIONS] = "branch-instructions",
> + [PERF_COUNT_HW_BRANCH_MISSES] = "branch-misses",
> + [PERF_COUNT_HW_BUS_CYCLES] = "bus-cycles",
> + [PERF_COUNT_HW_STALLED_CYCLES_FRONTEND] = "stalled-cycles-frontend",
> + [PERF_COUNT_HW_STALLED_CYCLES_BACKEND] = "stalled-cycles-backend",
> + [PERF_COUNT_HW_REF_CPU_CYCLES] = "ref-cycles",
> +};
> +
> +const char *event_symbols_sw[PERF_COUNT_SW_MAX] = {
> + [PERF_COUNT_SW_CPU_CLOCK] = "cpu-clock",
> + [PERF_COUNT_SW_TASK_CLOCK] = "task-clock",
> + [PERF_COUNT_SW_PAGE_FAULTS] = "page-faults",
> + [PERF_COUNT_SW_CONTEXT_SWITCHES] = "context-switches",
> + [PERF_COUNT_SW_CPU_MIGRATIONS] = "cpu-migrations",
> + [PERF_COUNT_SW_PAGE_FAULTS_MIN] = "minor-faults",
> + [PERF_COUNT_SW_PAGE_FAULTS_MAJ] = "major-faults",
> + [PERF_COUNT_SW_ALIGNMENT_FAULTS] = "alignment-faults",
> + [PERF_COUNT_SW_EMULATION_FAULTS] = "emulation-faults",
> + [PERF_COUNT_SW_DUMMY] = "dummy",
> + [PERF_COUNT_SW_BPF_OUTPUT] = "bpf-output",
> + [PERF_COUNT_SW_CGROUP_SWITCHES] = "cgroup-switches",
> +};
> +
> +const char *evsel__hw_cache[PERF_COUNT_HW_CACHE_MAX] = {
> + [PERF_COUNT_HW_CACHE_L1D] = "L1-dcache",
> + [PERF_COUNT_HW_CACHE_L1I] = "L1-icache",
> + [PERF_COUNT_HW_CACHE_LL] = "LLC",
> + [PERF_COUNT_HW_CACHE_DTLB] = "dTLB",
> + [PERF_COUNT_HW_CACHE_ITLB] = "iTLB",
> + [PERF_COUNT_HW_CACHE_BPU] = "branch",
> + [PERF_COUNT_HW_CACHE_NODE] = "node",
> +};
> +
> +const char *evsel__hw_cache_op[PERF_COUNT_HW_CACHE_OP_MAX] = {
> + [PERF_COUNT_HW_CACHE_OP_READ] = "load",
> + [PERF_COUNT_HW_CACHE_OP_WRITE] = "store",
> + [PERF_COUNT_HW_CACHE_OP_PREFETCH] = "prefetch",
> +};
> +
> +const char *evsel__hw_cache_result[PERF_COUNT_HW_CACHE_RESULT_MAX] = {
> + [PERF_COUNT_HW_CACHE_RESULT_ACCESS] = "refs",
> + [PERF_COUNT_HW_CACHE_RESULT_MISS] = "misses",
> +};
names lok good to me, thanks
jirka
> +
> +const char *perf_type_str(enum perf_type_id t)
> +{
> + if (t < 0 || t >= ARRAY_SIZE(perf_type_name))
> + return NULL;
> +
> + return perf_type_name[t];
> +}
> +
> +const char *perf_hw_str(enum perf_hw_id t)
> +{
> + if (t < 0 || t >= ARRAY_SIZE(event_symbols_hw))
> + return NULL;
> +
> + return event_symbols_hw[t];
> +}
> +
> +const char *perf_hw_cache_str(enum perf_hw_cache_id t)
> +{
> + if (t < 0 || t >= ARRAY_SIZE(evsel__hw_cache))
> + return NULL;
> +
> + return evsel__hw_cache[t];
> +}
> +
> +const char *perf_hw_cache_op_str(enum perf_hw_cache_op_id t)
> +{
> + if (t < 0 || t >= ARRAY_SIZE(evsel__hw_cache_op))
> + return NULL;
> +
> + return evsel__hw_cache_op[t];
> +}
> +
> +const char *perf_hw_cache_op_result_str(enum perf_hw_cache_op_result_id t)
> +{
> + if (t < 0 || t >= ARRAY_SIZE(evsel__hw_cache_result))
> + return NULL;
> +
> + return evsel__hw_cache_result[t];
> +}
> +
> +const char *perf_sw_str(enum perf_sw_ids t)
> +{
> + if (t < 0 || t >= ARRAY_SIZE(event_symbols_sw))
> + return NULL;
> +
> + return event_symbols_sw[t];
> +}
>
> /* 0: undecided, 1: supported, 2: not supported */
> static int perf_query_supported;
> diff --git a/tools/bpf/bpftool/perf.h b/tools/bpf/bpftool/perf.h
> new file mode 100644
> index 0000000..3fd7e42
> --- /dev/null
> +++ b/tools/bpf/bpftool/perf.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/* Copyright (C) 2023 Yafang Shao <laoar.shao@gmail.com> */
> +
> +#include <linux/perf_event.h>
> +
> +const char *perf_type_str(enum perf_type_id t);
> +const char *perf_hw_str(enum perf_hw_id t);
> +const char *perf_hw_cache_str(enum perf_hw_cache_id t);
> +const char *perf_hw_cache_op_str(enum perf_hw_cache_op_id t);
> +const char *perf_hw_cache_op_result_str(enum perf_hw_cache_op_result_id t);
> +const char *perf_sw_str(enum perf_sw_ids t);
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 00/10] bpf: Support ->fill_link_info for kprobe_multi and perf_event links
2023-06-15 10:04 ` [PATCH v3 bpf-next 00/10] bpf: Support ->fill_link_info for kprobe_multi and perf_event links Jiri Olsa
@ 2023-06-15 12:09 ` Yafang Shao
0 siblings, 0 replies; 47+ messages in thread
From: Yafang Shao @ 2023-06-15 12:09 UTC (permalink / raw)
To: Jiri Olsa
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, quentin, rostedt, mhiramat, bpf,
linux-trace-kernel
On Thu, Jun 15, 2023 at 6:04 PM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Mon, Jun 12, 2023 at 03:15:58PM +0000, Yafang Shao wrote:
> > This patchset enhances the usability of kprobe_multi programs by introducing
> > support for ->fill_link_info. This allows users to easily determine the
> > probed functions associated with a kprobe_multi program. While
> > `bpftool perf show` already provides information about functions probed by
> > perf_event programs, supporting ->fill_link_info ensures consistent access to
> > this information across all bpf links.
> >
> > In addition, this patch extends support to generic perf events, which are
> > currently not covered by `bpftool perf show`. While userspace is exposed to
> > only the perf type and config, other attributes such as sample_period and
> > sample_freq are disregarded.
> >
> > To ensure accurate identification of probed functions, it is preferable to
> > expose the address directly rather than relying solely on the symbol name.
> > However, this implementation respects the kptr_restrict setting and avoids
> > exposing the address if it is not permitted.
> >
> > v2->v3:
> > - Expose flags instead of retporbe (Andrii)
> > - Simplify the check on kmulti_link->cnt (Andrii)
> > - Use kallsyms_show_value() instead (Andrii)
> > - Show also the module name for kprobe_multi (Andrii)
> > - Add new enum bpf_perf_link_type (Andrii)
> > - Move perf event names into bpftool (Andrii, Quentin, Jiri)
> > - Keep perf event names in sync with perf tools (Jiri)
>
> hi,
> I'm getting some failing tests with this version:
>
> #11/2 bpf_cookie/multi_kprobe_link_api:FAIL
> #11/3 bpf_cookie/multi_kprobe_attach_api:FAIL
> #11 bpf_cookie:FAIL
>
> #104/1 kprobe_multi_test/skel_api:FAIL
> #104/2 kprobe_multi_test/link_api_addrs:FAIL
> #104/3 kprobe_multi_test/link_api_syms:FAIL
> #104/4 kprobe_multi_test/attach_api_pattern:FAIL
> #104/5 kprobe_multi_test/attach_api_addrs:FAIL
> #104/6 kprobe_multi_test/attach_api_syms:FAIL
> #104 kprobe_multi_test:FAIL
> #105/1 kprobe_multi_testmod_test/testmod_attach_api_syms:FAIL
> #105/2 kprobe_multi_testmod_test/testmod_attach_api_addrs:FAIL
> #105 kprobe_multi_testmod_test:FAIL
Thanks for your report.
BPF CI catched these errors as well.
That is caused by setting link->fp.flags, which has been pointed out
by you in patch #1.
I will fix it in the next version.
--
Regards
Yafang
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 01/10] bpf: Support ->fill_link_info for kprobe_multi
2023-06-15 8:29 ` Jiri Olsa
@ 2023-06-15 12:09 ` Yafang Shao
0 siblings, 0 replies; 47+ messages in thread
From: Yafang Shao @ 2023-06-15 12:09 UTC (permalink / raw)
To: Jiri Olsa
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, quentin, rostedt, mhiramat, bpf,
linux-trace-kernel
On Thu, Jun 15, 2023 at 4:29 PM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Mon, Jun 12, 2023 at 03:15:59PM +0000, Yafang Shao wrote:
>
> SNIP
>
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index 2bc41e6..742047c 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -2548,9 +2548,36 @@ static void bpf_kprobe_multi_link_dealloc(struct bpf_link *link)
> > kfree(kmulti_link);
> > }
> >
> > +static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link,
> > + struct bpf_link_info *info)
> > +{
> > + u64 __user *uaddrs = u64_to_user_ptr(info->kprobe_multi.addrs);
> > + struct bpf_kprobe_multi_link *kmulti_link;
> > + u32 ucount = info->kprobe_multi.count;
> > +
> > + if (!uaddrs ^ !ucount)
> > + return -EINVAL;
> > +
> > + kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link);
> > + if (!uaddrs) {
> > + info->kprobe_multi.count = kmulti_link->cnt;
> > + return 0;
> > + }
> > +
> > + if (ucount < kmulti_link->cnt)
> > + return -EINVAL;
> > + info->kprobe_multi.flags = kmulti_link->fp.flags;
> > + if (!kallsyms_show_value(current_cred()))
> > + return 0;
> > + if (copy_to_user(uaddrs, kmulti_link->addrs, ucount * sizeof(u64)))
> > + return -EFAULT;
> > + return 0;
> > +}
> > +
> > static const struct bpf_link_ops bpf_kprobe_multi_link_lops = {
> > .release = bpf_kprobe_multi_link_release,
> > .dealloc = bpf_kprobe_multi_link_dealloc,
> > + .fill_link_info = bpf_kprobe_multi_link_fill_link_info,
> > };
> >
> > static void bpf_kprobe_multi_cookie_swap(void *a, void *b, int size, const void *priv)
> > @@ -2890,6 +2917,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
> > return err;
> > }
> >
> > + link->fp.flags = flags;
>
> hum this looks wrong, we can't use fprobe flags to store our flags
> you should add flags to bpf_kprobe_multi_link
Will fix it. Thanks for pointing it out.
--
Regards
Yafang
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 08/10] bpf: Support ->fill_link_info for perf_event
2023-06-15 10:21 ` Jiri Olsa
@ 2023-06-15 12:10 ` Yafang Shao
0 siblings, 0 replies; 47+ messages in thread
From: Yafang Shao @ 2023-06-15 12:10 UTC (permalink / raw)
To: Jiri Olsa
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, quentin, rostedt, mhiramat, bpf,
linux-trace-kernel
On Thu, Jun 15, 2023 at 6:21 PM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Mon, Jun 12, 2023 at 03:16:06PM +0000, Yafang Shao wrote:
>
> SNIP
>
> >
> > /* User bpf_sock_addr struct to access socket fields and sockaddr struct passed
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index 80c9ec0..fe354d5 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -3303,9 +3303,133 @@ static void bpf_perf_link_dealloc(struct bpf_link *link)
> > kfree(perf_link);
> > }
> >
> > +static int bpf_perf_link_fill_name(const struct perf_event *event,
> > + char __user *uname, u32 ulen,
> > + u64 *probe_offset, u64 *probe_addr,
> > + u32 *fd_type)
> > +{
>
> this function name sounds misleading, it does query all the link data
> plus copying the name.. seems like this should be renamed and separated
Will do it.
>
>
> > + const char *buf;
> > + u32 prog_id;
> > + size_t len;
> > + int err;
> > +
> > + if (!ulen ^ !uname)
> > + return -EINVAL;
> > + if (!uname)
> > + return 0;
> > +
> > + err = bpf_get_perf_event_info(event, &prog_id, fd_type, &buf,
> > + probe_offset, probe_addr);
> > + if (err)
> > + return err;
> > +
> > + len = strlen(buf);
> > + if (buf) {
> > + err = bpf_copy_to_user(uname, buf, ulen, len);
> > + if (err)
> > + return err;
> > + } else {
> > + char zero = '\0';
> > +
> > + if (put_user(zero, uname))
> > + return -EFAULT;
> > + }
> > + return 0;
> > +}
> > +
> > +static int bpf_perf_link_fill_probe(const struct perf_event *event,
> > + struct bpf_link_info *info)
> > +{
> > + char __user *uname;
> > + u64 addr, offset;
> > + u32 ulen, type;
> > + int err;
> > +
> > +#ifdef CONFIG_KPROBE_EVENTS
>
> this will break compilation when CONFIG_KPROBE_EVENTS or CONFIG_UPROBE_EVENTS
> options are not defined
Indeed. Will improve it.
--
Regards
Yafang
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 01/10] bpf: Support ->fill_link_info for kprobe_multi
2023-06-12 15:15 ` [PATCH v3 bpf-next 01/10] bpf: Support ->fill_link_info for kprobe_multi Yafang Shao
2023-06-15 8:29 ` Jiri Olsa
@ 2023-06-16 17:24 ` Andrii Nakryiko
2023-06-17 2:48 ` Yafang Shao
1 sibling, 1 reply; 47+ messages in thread
From: Andrii Nakryiko @ 2023-06-16 17:24 UTC (permalink / raw)
To: Yafang Shao
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat, bpf,
linux-trace-kernel
On Mon, Jun 12, 2023 at 8:16 AM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> With the addition of support for fill_link_info to the kprobe_multi link,
> users will gain the ability to inspect it conveniently using the
> `bpftool link show`. This enhancement provides valuable information to the
> user, including the count of probed functions and their respective
> addresses. It's important to note that if the kptr_restrict setting is not
> permitted, the probed address will not be exposed, ensuring security.
>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
> include/uapi/linux/bpf.h | 5 +++++
> kernel/trace/bpf_trace.c | 28 ++++++++++++++++++++++++++++
> tools/include/uapi/linux/bpf.h | 5 +++++
> 3 files changed, 38 insertions(+)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index a7b5e91..23691ea 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -6438,6 +6438,11 @@ struct bpf_link_info {
> __s32 priority;
> __u32 flags;
> } netfilter;
> + struct {
> + __aligned_u64 addrs; /* in/out: addresses buffer ptr */
> + __u32 count;
> + __u32 flags;
> + } kprobe_multi;
> };
> } __attribute__((aligned(8)));
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 2bc41e6..742047c 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2548,9 +2548,36 @@ static void bpf_kprobe_multi_link_dealloc(struct bpf_link *link)
> kfree(kmulti_link);
> }
>
> +static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link,
> + struct bpf_link_info *info)
> +{
> + u64 __user *uaddrs = u64_to_user_ptr(info->kprobe_multi.addrs);
> + struct bpf_kprobe_multi_link *kmulti_link;
> + u32 ucount = info->kprobe_multi.count;
> +
> + if (!uaddrs ^ !ucount)
> + return -EINVAL;
> +
> + kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link);
> + if (!uaddrs) {
> + info->kprobe_multi.count = kmulti_link->cnt;
> + return 0;
> + }
> +
> + if (ucount < kmulti_link->cnt)
> + return -EINVAL;
> + info->kprobe_multi.flags = kmulti_link->fp.flags;
besides what Jiri said, flags should always be returned, just like
cnt. So structure code instead around uaddrs being optional, that will
everything more straightforward (i.e., fill out everything but uaddrs
and then at the end fill out addrs if uaddrs is not zero)
> + if (!kallsyms_show_value(current_cred()))
> + return 0;
> + if (copy_to_user(uaddrs, kmulti_link->addrs, ucount * sizeof(u64)))
> + return -EFAULT;
> + return 0;
> +}
> +
> static const struct bpf_link_ops bpf_kprobe_multi_link_lops = {
> .release = bpf_kprobe_multi_link_release,
> .dealloc = bpf_kprobe_multi_link_dealloc,
> + .fill_link_info = bpf_kprobe_multi_link_fill_link_info,
> };
>
> static void bpf_kprobe_multi_cookie_swap(void *a, void *b, int size, const void *priv)
> @@ -2890,6 +2917,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
> return err;
> }
>
> + link->fp.flags = flags;
> return bpf_link_settle(&link_primer);
>
> error:
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index a7b5e91..23691ea 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -6438,6 +6438,11 @@ struct bpf_link_info {
> __s32 priority;
> __u32 flags;
> } netfilter;
> + struct {
> + __aligned_u64 addrs; /* in/out: addresses buffer ptr */
> + __u32 count;
> + __u32 flags;
> + } kprobe_multi;
> };
> } __attribute__((aligned(8)));
>
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 02/10] bpftool: Dump the kernel symbol's module name
2023-06-12 15:16 ` [PATCH v3 bpf-next 02/10] bpftool: Dump the kernel symbol's module name Yafang Shao
2023-06-13 13:41 ` Quentin Monnet
@ 2023-06-16 17:25 ` Andrii Nakryiko
2023-06-17 2:55 ` Yafang Shao
1 sibling, 1 reply; 47+ messages in thread
From: Andrii Nakryiko @ 2023-06-16 17:25 UTC (permalink / raw)
To: Yafang Shao
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat, bpf,
linux-trace-kernel
On Mon, Jun 12, 2023 at 8:16 AM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> If the kernel symbol is in a module, we will dump the module name as
> well.
>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
> tools/bpf/bpftool/xlated_dumper.c | 6 +++++-
> tools/bpf/bpftool/xlated_dumper.h | 2 ++
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/xlated_dumper.c b/tools/bpf/bpftool/xlated_dumper.c
> index da608e1..dd917f3 100644
> --- a/tools/bpf/bpftool/xlated_dumper.c
> +++ b/tools/bpf/bpftool/xlated_dumper.c
> @@ -46,7 +46,11 @@ void kernel_syms_load(struct dump_data *dd)
> }
> dd->sym_mapping = tmp;
> sym = &dd->sym_mapping[dd->sym_count];
> - if (sscanf(buff, "%p %*c %s", &address, sym->name) != 2)
> +
> + /* module is optional */
> + sym->module[0] = '\0';
> + if (sscanf(buff, "%p %*c %s %s", &address, sym->name,
> + sym->module) < 2)
nit: please keep it single line if it fits in under 100 characters
> continue;
> sym->address = (unsigned long)address;
> if (!strcmp(sym->name, "__bpf_call_base")) {
> diff --git a/tools/bpf/bpftool/xlated_dumper.h b/tools/bpf/bpftool/xlated_dumper.h
> index 9a94637..5df8025 100644
> --- a/tools/bpf/bpftool/xlated_dumper.h
> +++ b/tools/bpf/bpftool/xlated_dumper.h
> @@ -5,12 +5,14 @@
> #define __BPF_TOOL_XLATED_DUMPER_H
>
> #define SYM_MAX_NAME 256
> +#define MODULE_NAME_LEN 64
>
> struct bpf_prog_linfo;
>
> struct kernel_sym {
> unsigned long address;
> char name[SYM_MAX_NAME];
> + char module[MODULE_NAME_LEN];
> };
>
> struct dump_data {
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 03/10] bpftool: Show probed function in kprobe_multi link info
2023-06-12 15:16 ` [PATCH v3 bpf-next 03/10] bpftool: Show probed function in kprobe_multi link info Yafang Shao
2023-06-13 13:41 ` Quentin Monnet
2023-06-13 22:36 ` Kui-Feng Lee
@ 2023-06-16 17:30 ` Andrii Nakryiko
2023-06-17 3:08 ` Yafang Shao
2 siblings, 1 reply; 47+ messages in thread
From: Andrii Nakryiko @ 2023-06-16 17:30 UTC (permalink / raw)
To: Yafang Shao
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat, bpf,
linux-trace-kernel
On Mon, Jun 12, 2023 at 8:16 AM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> Show the already expose kprobe_multi link info in bpftool. The result as
> follows,
>
> 52: kprobe_multi prog 381
> retprobe 0 func_cnt 7
> addrs ffffffff9ec44f20 funcs schedule_timeout_interruptible
> ffffffff9ec44f60 schedule_timeout_killable
> ffffffff9ec44fa0 schedule_timeout_uninterruptible
> ffffffff9ec44fe0 schedule_timeout_idle
> ffffffffc09468d0 xfs_trans_get_efd [xfs]
> ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
> ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
> pids kprobe_multi(559862)
> 53: kprobe_multi prog 381
> retprobe 1 func_cnt 7
> addrs ffffffff9ec44f20 funcs schedule_timeout_interruptible
> ffffffff9ec44f60 schedule_timeout_killable
> ffffffff9ec44fa0 schedule_timeout_uninterruptible
> ffffffff9ec44fe0 schedule_timeout_idle
> ffffffffc09468d0 xfs_trans_get_efd [xfs]
> ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
> ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
it all subjective, but this format is a bit weird where "addrs" and
"funcs" is in first row to the left. Just makes everything wider. Why
not something like
addr func
ffffffff9ec44f20 schedule_timeout_interruptible
ffffffff9ec44f60 schedule_timeout_killable
ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
Not it's singular (addr and func) because it's column names,
basically. Can also do "addr func [module]".
> pids kprobe_multi(559862)
>
> $ tools/bpf/bpftool/bpftool link show -j
> [{"id":52,"type":"kprobe_multi","prog_id":381,"retprobe":0,"func_cnt":7,"funcs":[{"addr":18446744072078249760,"func":"schedule_timeout_interruptible","module":""},{"addr":18446744072078249824,"func":"schedule_timeout_killable","module":""},{"addr":18446744072078249888,"func":"schedule_timeout_uninterruptible","module":""},{"addr":18446744072078249952,"func":"schedule_timeout_idle","module":""},{"addr":18446744072645535952,"func":"xfs_trans_get_efd","module":"[xfs]"},{"addr":18446744072645589520,"func":"xfs_trans_get_buf_map","module":"[xfs]"},{"addr":18446744072645604128,"func":"xfs_trans_get_dqtrx","module":"[xfs]"}],"pids":[{"pid":559862,"comm":"kprobe_multi"}]},{"id":53,"type":"kprobe_multi","prog_id":381,"retprobe":1,"func_cnt":7,"funcs":[{"addr":18446744072078249760,"func":"schedule_timeout_interruptible","module":""},{"addr":18446744072078249824,"func":"schedule_timeout_killable","module":""},{"addr":18446744072078249888,"func":"schedule_timeout_uninterruptible","module":""},{"addr":18446744072078249952,"func":"schedule_timeout_idle","module":""},{"addr":18446744072645535952,"func":"xfs_trans_get_efd","module":"[xfs]"},{"addr":18446744072645589520,"func":"xfs_trans_get_buf_map","module":"[xfs]"},{"addr":18446744072645604128,"func":"xfs_trans_get_dqtrx","module":"[xfs]"}],"pids":[{"pid":559862,"comm":"kprobe_multi"}]}]
>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
> tools/bpf/bpftool/link.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 108 insertions(+), 1 deletion(-)
>
[...]
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 08/10] bpf: Support ->fill_link_info for perf_event
2023-06-14 2:45 ` Yafang Shao
@ 2023-06-16 20:36 ` Andrii Nakryiko
2023-06-17 3:13 ` Yafang Shao
0 siblings, 1 reply; 47+ messages in thread
From: Andrii Nakryiko @ 2023-06-16 20:36 UTC (permalink / raw)
To: Yafang Shao
Cc: Kui-Feng Lee, Yonghong Song, ast, daniel, john.fastabend, andrii,
martin.lau, song, yhs, kpsingh, sdf, haoluo, jolsa, quentin,
rostedt, mhiramat, bpf, linux-trace-kernel
On Tue, Jun 13, 2023 at 7:46 PM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> On Wed, Jun 14, 2023 at 10:34 AM Kui-Feng Lee <sinquersw@gmail.com> wrote:
> >
> >
> >
> > On 6/12/23 19:47, Yafang Shao wrote:
> > > On Tue, Jun 13, 2023 at 1:36 AM Yonghong Song <yhs@meta.com> wrote:
> > >>
> > >>
> > >>
> > >> 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.
> > >
> > > If we put it into each individual struct, we have to choose one
> > > specific struct to get the type before we use the real struct, for
> > > example,
> > > if (info.perf_event.type == BPF_PERF_LINK_PERF_EVENT)
> > > goto out;
> > > if (info.perf_event.type == BPF_PERF_LINK_TRACEPOINT &&
> > > !info.tracepoint.tp_name) {
> > > info.tracepoint.tp_name = (unsigned long)&buf;
> > > info.tracepoint.name_len = sizeof(buf);
> > > goto again;
> > > }
> > > ...
> > >
> > > That doesn't look perfect.
> >
> > How about adding a common struct?
> >
> > struct {
> > __u32 type;
> > } perf_common;
> >
> > Then you check info.perf_common.type.
>
> I perfer below struct,
+1, we should do it this way
> struct {
> __u32 type; /* enum bpf_perf_link_type */
> union {
> struct {
> __u64 config;
> __u32 type;
> } perf_event; /* BPF_PERF_LINK_PERF_EVENT */
> struct {
> __aligned_u64 file_name; /* in/out */
> __u32 name_len;
> __u32 offset;/* offset from file_name */
> __u32 flags;
> } uprobe; /* BPF_PERF_LINK_UPROBE */
> struct {
> __aligned_u64 func_name; /* in/out */
> __u32 name_len;
> __u32 offset;/* offset from func_name */
> __u64 addr;
> __u32 flags;
> } kprobe; /* BPF_PERF_LINK_KPROBE */
> struct {
> __aligned_u64 tp_name; /* in/out */
> __u32 name_len;
> } tracepoint; /* BPF_PERF_LINK_TRACEPOINT */
> };
> } perf_link;
this should be named "perf_event" to match BPF_LINK_TYPE_PERF_EVENT
and "perf_event" above probably could be just "event" then? Similarly
we can s/BPF_PERF_LINK_PERF_EVENT/BPF_PERF_LINK_EVENT/?
>
> I think that would be more clear.
>
> --
> Regards
> Yafang
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 10/10] bpftool: Show probed function in perf_event link info
2023-06-12 15:16 ` [PATCH v3 bpf-next 10/10] bpftool: Show probed function in perf_event link info Yafang Shao
2023-06-13 13:42 ` Quentin Monnet
@ 2023-06-16 20:41 ` Andrii Nakryiko
2023-06-17 3:20 ` Yafang Shao
1 sibling, 1 reply; 47+ messages in thread
From: Andrii Nakryiko @ 2023-06-16 20:41 UTC (permalink / raw)
To: Yafang Shao
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat, bpf,
linux-trace-kernel
On Mon, Jun 12, 2023 at 8:16 AM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> 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
how about
"event hardware:cpu-cycles" for events
> bpf_cookie 0
> pids perf_event(1379330)
> 6: perf_event prog 20
> retprobe 0 file_name /home/yafang/bpf/uprobe/a.out offset 0x1338
for uprobes: "uprobe /home/yafang/bpf/uprobe/a.out+0x1338"
for retprobes: "uretprobe /home/yafang/bpf/uprobe/a.out+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
"tracepoint sched_switch" ?
> bpf_cookie 0
> pids tracepoint(1381734)
> 10: perf_event prog 43
> retprobe 0 func_name kernel_clone addr ffffffffad0a9660
similar to uprobes:
"kprobe kernel_clone 0xffffffffad0a9660"
"kretprobe kernel_clone 0xffffffffad0a9660"
That is, make this more human readable instead of mechanically
translated from kernel info? retprobe 1/0 is quite cumbersome,
"uprobe" vs "uretprobe" makes more sense?
JSON is where it could be completely mechanically translated, IMO.
> 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@gmail.com>
> ---
> tools/bpf/bpftool/link.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 213 insertions(+)
>
[...]
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 01/10] bpf: Support ->fill_link_info for kprobe_multi
2023-06-16 17:24 ` Andrii Nakryiko
@ 2023-06-17 2:48 ` Yafang Shao
0 siblings, 0 replies; 47+ messages in thread
From: Yafang Shao @ 2023-06-17 2:48 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat, bpf,
linux-trace-kernel
On Sat, Jun 17, 2023 at 1:24 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Mon, Jun 12, 2023 at 8:16 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> >
> > With the addition of support for fill_link_info to the kprobe_multi link,
> > users will gain the ability to inspect it conveniently using the
> > `bpftool link show`. This enhancement provides valuable information to the
> > user, including the count of probed functions and their respective
> > addresses. It's important to note that if the kptr_restrict setting is not
> > permitted, the probed address will not be exposed, ensuring security.
> >
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > ---
> > include/uapi/linux/bpf.h | 5 +++++
> > kernel/trace/bpf_trace.c | 28 ++++++++++++++++++++++++++++
> > tools/include/uapi/linux/bpf.h | 5 +++++
> > 3 files changed, 38 insertions(+)
> >
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index a7b5e91..23691ea 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -6438,6 +6438,11 @@ struct bpf_link_info {
> > __s32 priority;
> > __u32 flags;
> > } netfilter;
> > + struct {
> > + __aligned_u64 addrs; /* in/out: addresses buffer ptr */
> > + __u32 count;
> > + __u32 flags;
> > + } kprobe_multi;
> > };
> > } __attribute__((aligned(8)));
> >
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index 2bc41e6..742047c 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -2548,9 +2548,36 @@ static void bpf_kprobe_multi_link_dealloc(struct bpf_link *link)
> > kfree(kmulti_link);
> > }
> >
> > +static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link,
> > + struct bpf_link_info *info)
> > +{
> > + u64 __user *uaddrs = u64_to_user_ptr(info->kprobe_multi.addrs);
> > + struct bpf_kprobe_multi_link *kmulti_link;
> > + u32 ucount = info->kprobe_multi.count;
> > +
> > + if (!uaddrs ^ !ucount)
> > + return -EINVAL;
> > +
> > + kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link);
> > + if (!uaddrs) {
> > + info->kprobe_multi.count = kmulti_link->cnt;
> > + return 0;
> > + }
> > +
> > + if (ucount < kmulti_link->cnt)
> > + return -EINVAL;
> > + info->kprobe_multi.flags = kmulti_link->fp.flags;
>
> besides what Jiri said, flags should always be returned, just like
> cnt. So structure code instead around uaddrs being optional, that will
> everything more straightforward (i.e., fill out everything but uaddrs
> and then at the end fill out addrs if uaddrs is not zero)
Agree. That will be more straightforward. Will change it.
--
Regards
Yafang
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 02/10] bpftool: Dump the kernel symbol's module name
2023-06-16 17:25 ` Andrii Nakryiko
@ 2023-06-17 2:55 ` Yafang Shao
0 siblings, 0 replies; 47+ messages in thread
From: Yafang Shao @ 2023-06-17 2:55 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat, bpf,
linux-trace-kernel
On Sat, Jun 17, 2023 at 1:25 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Mon, Jun 12, 2023 at 8:16 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> >
> > If the kernel symbol is in a module, we will dump the module name as
> > well.
> >
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > ---
> > tools/bpf/bpftool/xlated_dumper.c | 6 +++++-
> > tools/bpf/bpftool/xlated_dumper.h | 2 ++
> > 2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/bpf/bpftool/xlated_dumper.c b/tools/bpf/bpftool/xlated_dumper.c
> > index da608e1..dd917f3 100644
> > --- a/tools/bpf/bpftool/xlated_dumper.c
> > +++ b/tools/bpf/bpftool/xlated_dumper.c
> > @@ -46,7 +46,11 @@ void kernel_syms_load(struct dump_data *dd)
> > }
> > dd->sym_mapping = tmp;
> > sym = &dd->sym_mapping[dd->sym_count];
> > - if (sscanf(buff, "%p %*c %s", &address, sym->name) != 2)
> > +
> > + /* module is optional */
> > + sym->module[0] = '\0';
> > + if (sscanf(buff, "%p %*c %s %s", &address, sym->name,
> > + sym->module) < 2)
>
> nit: please keep it single line if it fits in under 100 characters
Will change it.
I thought it is 80 characters.
--
Regards
Yafang
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 03/10] bpftool: Show probed function in kprobe_multi link info
2023-06-16 17:30 ` Andrii Nakryiko
@ 2023-06-17 3:08 ` Yafang Shao
2023-06-20 17:17 ` Andrii Nakryiko
0 siblings, 1 reply; 47+ messages in thread
From: Yafang Shao @ 2023-06-17 3:08 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat, bpf,
linux-trace-kernel
On Sat, Jun 17, 2023 at 1:30 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Mon, Jun 12, 2023 at 8:16 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> >
> > Show the already expose kprobe_multi link info in bpftool. The result as
> > follows,
> >
> > 52: kprobe_multi prog 381
> > retprobe 0 func_cnt 7
> > addrs ffffffff9ec44f20 funcs schedule_timeout_interruptible
> > ffffffff9ec44f60 schedule_timeout_killable
> > ffffffff9ec44fa0 schedule_timeout_uninterruptible
> > ffffffff9ec44fe0 schedule_timeout_idle
> > ffffffffc09468d0 xfs_trans_get_efd [xfs]
> > ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
> > ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
> > pids kprobe_multi(559862)
> > 53: kprobe_multi prog 381
> > retprobe 1 func_cnt 7
> > addrs ffffffff9ec44f20 funcs schedule_timeout_interruptible
> > ffffffff9ec44f60 schedule_timeout_killable
> > ffffffff9ec44fa0 schedule_timeout_uninterruptible
> > ffffffff9ec44fe0 schedule_timeout_idle
> > ffffffffc09468d0 xfs_trans_get_efd [xfs]
> > ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
> > ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
>
> it all subjective, but this format is a bit weird where "addrs" and
> "funcs" is in first row to the left. Just makes everything wider. Why
> not something like
>
> addr func
> ffffffff9ec44f20 schedule_timeout_interruptible
> ffffffff9ec44f60 schedule_timeout_killable
> ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
> ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
It may be a little strange if there's only one function, but I don't
mind doing it as you suggested.
>
> Not it's singular (addr and func) because it's column names,
> basically. Can also do "addr func [module]".
The length of the function name is variable, so it is not easy to
determine where to put the "[module]". So I prefer to not show the
"[module]".
--
Regards
Yafang
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 08/10] bpf: Support ->fill_link_info for perf_event
2023-06-16 20:36 ` Andrii Nakryiko
@ 2023-06-17 3:13 ` Yafang Shao
0 siblings, 0 replies; 47+ messages in thread
From: Yafang Shao @ 2023-06-17 3:13 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Kui-Feng Lee, Yonghong Song, ast, daniel, john.fastabend, andrii,
martin.lau, song, yhs, kpsingh, sdf, haoluo, jolsa, quentin,
rostedt, mhiramat, bpf, linux-trace-kernel
On Sat, Jun 17, 2023 at 4:36 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Tue, Jun 13, 2023 at 7:46 PM Yafang Shao <laoar.shao@gmail.com> wrote:
> >
> > On Wed, Jun 14, 2023 at 10:34 AM Kui-Feng Lee <sinquersw@gmail.com> wrote:
> > >
> > >
> > >
> > > On 6/12/23 19:47, Yafang Shao wrote:
> > > > On Tue, Jun 13, 2023 at 1:36 AM Yonghong Song <yhs@meta.com> wrote:
> > > >>
> > > >>
> > > >>
> > > >> 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.
> > > >
> > > > If we put it into each individual struct, we have to choose one
> > > > specific struct to get the type before we use the real struct, for
> > > > example,
> > > > if (info.perf_event.type == BPF_PERF_LINK_PERF_EVENT)
> > > > goto out;
> > > > if (info.perf_event.type == BPF_PERF_LINK_TRACEPOINT &&
> > > > !info.tracepoint.tp_name) {
> > > > info.tracepoint.tp_name = (unsigned long)&buf;
> > > > info.tracepoint.name_len = sizeof(buf);
> > > > goto again;
> > > > }
> > > > ...
> > > >
> > > > That doesn't look perfect.
> > >
> > > How about adding a common struct?
> > >
> > > struct {
> > > __u32 type;
> > > } perf_common;
> > >
> > > Then you check info.perf_common.type.
> >
> > I perfer below struct,
>
> +1, we should do it this way
>
> > struct {
> > __u32 type; /* enum bpf_perf_link_type */
> > union {
> > struct {
> > __u64 config;
> > __u32 type;
> > } perf_event; /* BPF_PERF_LINK_PERF_EVENT */
> > struct {
> > __aligned_u64 file_name; /* in/out */
> > __u32 name_len;
> > __u32 offset;/* offset from file_name */
> > __u32 flags;
> > } uprobe; /* BPF_PERF_LINK_UPROBE */
> > struct {
> > __aligned_u64 func_name; /* in/out */
> > __u32 name_len;
> > __u32 offset;/* offset from func_name */
> > __u64 addr;
> > __u32 flags;
> > } kprobe; /* BPF_PERF_LINK_KPROBE */
> > struct {
> > __aligned_u64 tp_name; /* in/out */
> > __u32 name_len;
> > } tracepoint; /* BPF_PERF_LINK_TRACEPOINT */
> > };
> > } perf_link;
>
> this should be named "perf_event" to match BPF_LINK_TYPE_PERF_EVENT
>
> and "perf_event" above probably could be just "event" then? Similarly
> we can s/BPF_PERF_LINK_PERF_EVENT/BPF_PERF_LINK_EVENT/?
Agree. Will change it.
--
Regards
Yafang
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 10/10] bpftool: Show probed function in perf_event link info
2023-06-16 20:41 ` Andrii Nakryiko
@ 2023-06-17 3:20 ` Yafang Shao
2023-06-17 3:29 ` Yafang Shao
0 siblings, 1 reply; 47+ messages in thread
From: Yafang Shao @ 2023-06-17 3:20 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat, bpf,
linux-trace-kernel
On Sat, Jun 17, 2023 at 4:41 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Mon, Jun 12, 2023 at 8:16 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> >
> > 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
>
> how about
>
> "event hardware:cpu-cycles" for events
Agree. That is better.
>
> > bpf_cookie 0
> > pids perf_event(1379330)
> > 6: perf_event prog 20
> > retprobe 0 file_name /home/yafang/bpf/uprobe/a.out offset 0x1338
>
> for uprobes: "uprobe /home/yafang/bpf/uprobe/a.out+0x1338"
> for retprobes: "uretprobe /home/yafang/bpf/uprobe/a.out+0x1338"
Agree.
>
> > 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
>
> "tracepoint sched_switch" ?
Agree.
>
> > bpf_cookie 0
> > pids tracepoint(1381734)
> > 10: perf_event prog 43
> > retprobe 0 func_name kernel_clone addr ffffffffad0a9660
>
> similar to uprobes:
>
> "kprobe kernel_clone 0xffffffffad0a9660"
> "kretprobe kernel_clone 0xffffffffad0a9660"
>
>
> That is, make this more human readable instead of mechanically
> translated from kernel info? retprobe 1/0 is quite cumbersome,
> "uprobe" vs "uretprobe" makes more sense?
Agree. Will do it.
>
> JSON is where it could be completely mechanically translated, IMO.
Agree.
--
Regards
Yafang
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 10/10] bpftool: Show probed function in perf_event link info
2023-06-17 3:20 ` Yafang Shao
@ 2023-06-17 3:29 ` Yafang Shao
0 siblings, 0 replies; 47+ messages in thread
From: Yafang Shao @ 2023-06-17 3:29 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat, bpf,
linux-trace-kernel
On Sat, Jun 17, 2023 at 11:20 AM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> On Sat, Jun 17, 2023 at 4:41 AM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Mon, Jun 12, 2023 at 8:16 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> > >
> > > 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
> >
> > how about
> >
> > "event hardware:cpu-cycles" for events
>
> Agree. That is better.
>
> >
> > > bpf_cookie 0
> > > pids perf_event(1379330)
> > > 6: perf_event prog 20
> > > retprobe 0 file_name /home/yafang/bpf/uprobe/a.out offset 0x1338
> >
> > for uprobes: "uprobe /home/yafang/bpf/uprobe/a.out+0x1338"
> > for retprobes: "uretprobe /home/yafang/bpf/uprobe/a.out+0x1338"
>
> Agree.
BTW, should we also change the output of `bpftool perf show` that way?
- Old
$ bpftool perf show
pid 11898 fd 7: prog_id 6 kprobe func kernel_clone offset 0
pid 11898 fd 9: prog_id 5 kretprobe func kernel_clone offset 0
pid 11966 fd 9: prog_id 13 uprobe filename
/home/dev/waken/bpf/uprobe/a.out offset 4920
pid 11966 fd 11: prog_id 14 uretprobe filename
/home/dev/waken/bpf/uprobe/a.out offset 4920
- New ?
$ bpftool perf show
pid 11898 fd 7: prog_id 6 kprobe kernel_clone
pid 11898 fd 9: prog_id 5 kretprobe kernel_clone
pid 11966 fd 9: prog_id 13 uprobe /home/dev/waken/bpf/uprobe/a.out+0x1338
pid 11966 fd 11: prog_id 14 uretprobe /home/dev/waken/bpf/uprobe/a.out+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
> >
> > "tracepoint sched_switch" ?
>
> Agree.
>
> >
> > > bpf_cookie 0
> > > pids tracepoint(1381734)
> > > 10: perf_event prog 43
> > > retprobe 0 func_name kernel_clone addr ffffffffad0a9660
> >
> > similar to uprobes:
> >
> > "kprobe kernel_clone 0xffffffffad0a9660"
> > "kretprobe kernel_clone 0xffffffffad0a9660"
> >
> >
> > That is, make this more human readable instead of mechanically
> > translated from kernel info? retprobe 1/0 is quite cumbersome,
> > "uprobe" vs "uretprobe" makes more sense?
>
> Agree. Will do it.
>
> >
> > JSON is where it could be completely mechanically translated, IMO.
>
> Agree.
>
> --
> Regards
> Yafang
--
Regards
Yafang
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 03/10] bpftool: Show probed function in kprobe_multi link info
2023-06-17 3:08 ` Yafang Shao
@ 2023-06-20 17:17 ` Andrii Nakryiko
2023-06-21 1:29 ` Yafang Shao
0 siblings, 1 reply; 47+ messages in thread
From: Andrii Nakryiko @ 2023-06-20 17:17 UTC (permalink / raw)
To: Yafang Shao
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat, bpf,
linux-trace-kernel
On Fri, Jun 16, 2023 at 8:09 PM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> On Sat, Jun 17, 2023 at 1:30 AM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Mon, Jun 12, 2023 at 8:16 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> > >
> > > Show the already expose kprobe_multi link info in bpftool. The result as
> > > follows,
> > >
> > > 52: kprobe_multi prog 381
> > > retprobe 0 func_cnt 7
> > > addrs ffffffff9ec44f20 funcs schedule_timeout_interruptible
> > > ffffffff9ec44f60 schedule_timeout_killable
> > > ffffffff9ec44fa0 schedule_timeout_uninterruptible
> > > ffffffff9ec44fe0 schedule_timeout_idle
> > > ffffffffc09468d0 xfs_trans_get_efd [xfs]
> > > ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
> > > ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
> > > pids kprobe_multi(559862)
> > > 53: kprobe_multi prog 381
> > > retprobe 1 func_cnt 7
> > > addrs ffffffff9ec44f20 funcs schedule_timeout_interruptible
> > > ffffffff9ec44f60 schedule_timeout_killable
> > > ffffffff9ec44fa0 schedule_timeout_uninterruptible
> > > ffffffff9ec44fe0 schedule_timeout_idle
> > > ffffffffc09468d0 xfs_trans_get_efd [xfs]
> > > ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
> > > ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
> >
> > it all subjective, but this format is a bit weird where "addrs" and
> > "funcs" is in first row to the left. Just makes everything wider. Why
> > not something like
> >
> > addr func
> > ffffffff9ec44f20 schedule_timeout_interruptible
> > ffffffff9ec44f60 schedule_timeout_killable
> > ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
> > ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
>
> It may be a little strange if there's only one function, but I don't
> mind doing it as you suggested.
>
> >
> > Not it's singular (addr and func) because it's column names,
> > basically. Can also do "addr func [module]".
>
> The length of the function name is variable, so it is not easy to
> determine where to put the "[module]". So I prefer to not show the
> "[module]".
"func [module]" in the header will give a hint of what is that value
in square brackets. I didn't mean to align it into a third column
>
> --
> Regards
> Yafang
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 bpf-next 03/10] bpftool: Show probed function in kprobe_multi link info
2023-06-20 17:17 ` Andrii Nakryiko
@ 2023-06-21 1:29 ` Yafang Shao
0 siblings, 0 replies; 47+ messages in thread
From: Yafang Shao @ 2023-06-21 1:29 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, quentin, rostedt, mhiramat, bpf,
linux-trace-kernel
On Wed, Jun 21, 2023 at 1:17 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Fri, Jun 16, 2023 at 8:09 PM Yafang Shao <laoar.shao@gmail.com> wrote:
> >
> > On Sat, Jun 17, 2023 at 1:30 AM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Mon, Jun 12, 2023 at 8:16 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> > > >
> > > > Show the already expose kprobe_multi link info in bpftool. The result as
> > > > follows,
> > > >
> > > > 52: kprobe_multi prog 381
> > > > retprobe 0 func_cnt 7
> > > > addrs ffffffff9ec44f20 funcs schedule_timeout_interruptible
> > > > ffffffff9ec44f60 schedule_timeout_killable
> > > > ffffffff9ec44fa0 schedule_timeout_uninterruptible
> > > > ffffffff9ec44fe0 schedule_timeout_idle
> > > > ffffffffc09468d0 xfs_trans_get_efd [xfs]
> > > > ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
> > > > ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
> > > > pids kprobe_multi(559862)
> > > > 53: kprobe_multi prog 381
> > > > retprobe 1 func_cnt 7
> > > > addrs ffffffff9ec44f20 funcs schedule_timeout_interruptible
> > > > ffffffff9ec44f60 schedule_timeout_killable
> > > > ffffffff9ec44fa0 schedule_timeout_uninterruptible
> > > > ffffffff9ec44fe0 schedule_timeout_idle
> > > > ffffffffc09468d0 xfs_trans_get_efd [xfs]
> > > > ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
> > > > ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
> > >
> > > it all subjective, but this format is a bit weird where "addrs" and
> > > "funcs" is in first row to the left. Just makes everything wider. Why
> > > not something like
> > >
> > > addr func
> > > ffffffff9ec44f20 schedule_timeout_interruptible
> > > ffffffff9ec44f60 schedule_timeout_killable
> > > ffffffffc0953a10 xfs_trans_get_buf_map [xfs]
> > > ffffffffc0957320 xfs_trans_get_dqtrx [xfs]
> >
> > It may be a little strange if there's only one function, but I don't
> > mind doing it as you suggested.
> >
> > >
> > > Not it's singular (addr and func) because it's column names,
> > > basically. Can also do "addr func [module]".
> >
> > The length of the function name is variable, so it is not easy to
> > determine where to put the "[module]". So I prefer to not show the
> > "[module]".
>
> "func [module]" in the header will give a hint of what is that value
> in square brackets. I didn't mean to align it into a third column
Thanks for the clarification. Will change it.
--
Regards
Yafang
^ permalink raw reply [flat|nested] 47+ messages in thread
end of thread, other threads:[~2023-06-21 1:29 UTC | newest]
Thread overview: 47+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-12 15:15 [PATCH v3 bpf-next 00/10] bpf: Support ->fill_link_info for kprobe_multi and perf_event links Yafang Shao
2023-06-12 15:15 ` [PATCH v3 bpf-next 01/10] bpf: Support ->fill_link_info for kprobe_multi Yafang Shao
2023-06-15 8:29 ` Jiri Olsa
2023-06-15 12:09 ` Yafang Shao
2023-06-16 17:24 ` Andrii Nakryiko
2023-06-17 2:48 ` Yafang Shao
2023-06-12 15:16 ` [PATCH v3 bpf-next 02/10] bpftool: Dump the kernel symbol's module name Yafang Shao
2023-06-13 13:41 ` Quentin Monnet
2023-06-13 14:56 ` Yafang Shao
2023-06-16 17:25 ` Andrii Nakryiko
2023-06-17 2:55 ` Yafang Shao
2023-06-12 15:16 ` [PATCH v3 bpf-next 03/10] bpftool: Show probed function in kprobe_multi link info Yafang Shao
2023-06-13 13:41 ` Quentin Monnet
2023-06-13 14:59 ` Yafang Shao
2023-06-13 22:36 ` Kui-Feng Lee
2023-06-14 2:42 ` Yafang Shao
2023-06-14 8:33 ` Quentin Monnet
2023-06-16 17:30 ` Andrii Nakryiko
2023-06-17 3:08 ` Yafang Shao
2023-06-20 17:17 ` Andrii Nakryiko
2023-06-21 1:29 ` Yafang Shao
2023-06-12 15:16 ` [PATCH v3 bpf-next 04/10] bpf: Protect probed address based on kptr_restrict setting Yafang Shao
2023-06-12 15:16 ` [PATCH v3 bpf-next 05/10] bpf: Clear the probe_addr for uprobe Yafang Shao
2023-06-12 17:22 ` Yonghong Song
2023-06-12 15:16 ` [PATCH v3 bpf-next 06/10] bpf: Expose symbol's respective address Yafang Shao
2023-06-12 15:16 ` [PATCH v3 bpf-next 07/10] bpf: Add a common helper bpf_copy_to_user() Yafang Shao
2023-06-12 15:16 ` [PATCH v3 bpf-next 08/10] bpf: Support ->fill_link_info for perf_event Yafang Shao
2023-06-12 17:36 ` Yonghong Song
2023-06-13 2:47 ` Yafang Shao
2023-06-14 2:34 ` Kui-Feng Lee
2023-06-14 2:45 ` Yafang Shao
2023-06-16 20:36 ` Andrii Nakryiko
2023-06-17 3:13 ` Yafang Shao
2023-06-15 10:21 ` Jiri Olsa
2023-06-15 12:10 ` Yafang Shao
2023-06-12 15:16 ` [PATCH v3 bpf-next 09/10] bpftool: Add perf event names Yafang Shao
2023-06-13 13:41 ` Quentin Monnet
2023-06-13 15:01 ` Yafang Shao
2023-06-15 10:23 ` Jiri Olsa
2023-06-12 15:16 ` [PATCH v3 bpf-next 10/10] bpftool: Show probed function in perf_event link info Yafang Shao
2023-06-13 13:42 ` Quentin Monnet
2023-06-13 15:11 ` Yafang Shao
2023-06-16 20:41 ` Andrii Nakryiko
2023-06-17 3:20 ` Yafang Shao
2023-06-17 3:29 ` Yafang Shao
2023-06-15 10:04 ` [PATCH v3 bpf-next 00/10] bpf: Support ->fill_link_info for kprobe_multi and perf_event links Jiri Olsa
2023-06-15 12:09 ` Yafang Shao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).