[RFC PATCH bpf-next 0/8] bpf: Support ->show_fdinfo and ->fill_link_info for kprobe prog

[RFC PATCH bpf-next 0/8] bpf: Support ->show_fdinfo and ->fill_link_info for kprobe prog

[Yafang Shao]

Currently, it is not easy to determine which functions are probed by a
kprobe_multi program. This patchset supports ->show_fdinfo and
->fill_link_info for it, allowing the user to easily obtain the probed
functions.

Although the user can retrieve the functions probed by a perf_event
program using `bpftool perf show`, it would be beneficial to also support
->show_fdinfo and ->fill_link_info. This way, the user can obtain it in the
same manner as other bpf links.

It would be preferable to expose the address directly rather than the symbol
name, as multiple functions may share the same name.

Yafang Shao (8):
  bpf: Support ->show_fdinfo for kprobe_multi
  bpf: Support ->fill_link_info for kprobe_multi
  bpftool: Show probed function in kprobe_multi link info
  bpf: Always expose the probed address
  bpf: Support ->show_fdinfo for perf_event
  bpf: Add a common helper bpf_copy_to_user()
  bpf: Support ->fill_link_info for perf_event
  bpftool: Show probed function in perf_event link info

 include/uapi/linux/bpf.h       |  10 ++++
 kernel/bpf/syscall.c           | 107 +++++++++++++++++++++++++++++++++++------
 kernel/trace/bpf_trace.c       |  48 ++++++++++++++++++
 kernel/trace/trace_kprobe.c    |   2 +-
 tools/bpf/bpftool/link.c       |  71 ++++++++++++++++++++++++++-
 tools/include/uapi/linux/bpf.h |  10 ++++
 6 files changed, 232 insertions(+), 16 deletions(-)

-- 
1.8.3.1

[RFC PATCH bpf-next 1/8] bpf: Support ->show_fdinfo for kprobe_multi

[Yafang Shao]

Currently, there is no way to check which functions are attached to a
kprobe_multi link, causing confusion for users. It is important that we
provide a means to expose these functions. The expected result is as follows,

$ cat /proc/10936/fdinfo/9
pos:    0
flags:  02000000
mnt_id: 15
ino:    2094
link_type:      kprobe_multi
link_id:        2
prog_tag:       a04f5eef06a7f555
prog_id:        11
func_count:     4
func_addrs:     ffffffffaad475c0
                ffffffffaad47600
                ffffffffaad47640
                ffffffffaad47680

$ cat /proc/10936/fdinfo/9 | grep "func_addrs" -A 4 | \
  awk '{ if (NR ==1) {print $2} else {print $1}}' | \
  awk '{"grep " $1 " /proc/kallsyms"| getline f; print f}'
ffffffffaad475c0 T schedule_timeout_interruptible
ffffffffaad47600 T schedule_timeout_killable
ffffffffaad47640 T schedule_timeout_uninterruptible
ffffffffaad47680 T schedule_timeout_idle

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 kernel/trace/bpf_trace.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 2bc41e6..0d84a7a 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2548,9 +2548,26 @@ static void bpf_kprobe_multi_link_dealloc(struct bpf_link *link)
 	kfree(kmulti_link);
 }
 
+static void bpf_kprobe_multi_link_show_fdinfo(const struct bpf_link *link,
+				      struct seq_file *seq)
+{
+	struct bpf_kprobe_multi_link *kmulti_link;
+	int i;
+
+	kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link);
+	seq_printf(seq, "func_count:\t%d\n", kmulti_link->cnt);
+	for (i = 0; i < kmulti_link->cnt; i++) {
+		if (i == 0)
+			seq_printf(seq, "func_addrs:\t%lx\n", kmulti_link->addrs[i]);
+		else
+			seq_printf(seq, "           \t%lx\n", kmulti_link->addrs[i]);
+	}
+}
+
 static const struct bpf_link_ops bpf_kprobe_multi_link_lops = {
 	.release = bpf_kprobe_multi_link_release,
 	.dealloc = bpf_kprobe_multi_link_dealloc,
+	.show_fdinfo = bpf_kprobe_multi_link_show_fdinfo,
 };
 
 static void bpf_kprobe_multi_cookie_swap(void *a, void *b, int size, const void *priv)
-- 
1.8.3.1

[RFC PATCH bpf-next 2/8] bpf: Support ->fill_link_info for kprobe_multi

[Yafang Shao]

By adding support for ->fill_link_info to the kprobe_multi link, users will
be able to inspect it using `bpftool link show`. This enhancement will
expose both the count of probed functions and their respective addresses to
the user.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 include/uapi/linux/bpf.h       |  4 ++++
 kernel/trace/bpf_trace.c       | 31 +++++++++++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h |  4 ++++
 3 files changed, 39 insertions(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 9273c65..6be9b1d 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -6434,6 +6434,10 @@ struct bpf_link_info {
 			__s32 priority;
 			__u32 flags;
 		} netfilter;
+		struct {
+			__aligned_u64 addrs;
+			__u32 count;
+		} kprobe_multi;
 	};
 } __attribute__((aligned(8)));
 
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 0d84a7a..00a0009 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2564,10 +2564,41 @@ static void bpf_kprobe_multi_link_show_fdinfo(const struct bpf_link *link,
 	}
 }
 
+static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link,
+						struct bpf_link_info *info)
+{
+	struct bpf_kprobe_multi_link *kmulti_link;
+	u64 *uaddrs = u64_to_user_ptr(info->kprobe_multi.addrs);
+	u32 ucount = info->kprobe_multi.count;
+	int i;
+
+	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)
+		return 0;
+
+	if (ucount != kmulti_link->cnt)
+		return -EINVAL;
+
+	for (i = 0; i < ucount; i++)
+		if (copy_to_user(uaddrs + i, kmulti_link->addrs + i,
+				 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,
 	.show_fdinfo = bpf_kprobe_multi_link_show_fdinfo,
+	.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)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 9273c65..6be9b1d 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -6434,6 +6434,10 @@ struct bpf_link_info {
 			__s32 priority;
 			__u32 flags;
 		} netfilter;
+		struct {
+			__aligned_u64 addrs;
+			__u32 count;
+		} kprobe_multi;
 	};
 } __attribute__((aligned(8)));
 
-- 
1.8.3.1

[RFC PATCH bpf-next 3/8] bpftool: Show probed function in kprobe_multi link info

[Yafang Shao]

Show the already expose kprobe_multi link info in bpftool. The result as
follows,

$ bpftool link show
2: kprobe_multi  prog 11
        func_cnt 4  addrs ffffffffaad475c0 ffffffffaad47600
                          ffffffffaad47640 ffffffffaad47680
        pids trace(10936)

$ bpftool link show -j
[{"id":1,"type":"perf_event","prog_id":5,"bpf_cookie":0,"pids":[{"pid":10658,"comm":"trace"}]},{"id":2,"type":"kprobe_multi","prog_id":11,"func_cnt":4,"addrs":[18446744072280634816,18446744072280634880,18446744072280634944,18446744072280635008],"pids":[{"pid":10936,"comm":"trace"}]},{"id":120,"type":"iter","prog_id":266,"target_name":"bpf_map"},{"id":121,"type":"iter","prog_id":267,"target_name":"bpf_prog"}]

$ bpftool link show  | grep -A 1 "func_cnt" | \
  awk '{if (NR == 1) {print $4; print $5;} else {print $1; print $2} }' | \
  awk '{"grep " $1 " /proc/kallsyms" | getline f; print f;}'
ffffffffaad475c0 T schedule_timeout_interruptible
ffffffffaad47600 T schedule_timeout_killable
ffffffffaad47640 T schedule_timeout_uninterruptible
ffffffffaad47680 T schedule_timeout_idle

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 tools/bpf/bpftool/link.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index 2d78607..76f1bb2 100644
--- a/tools/bpf/bpftool/link.c
+++ b/tools/bpf/bpftool/link.c
@@ -218,6 +218,20 @@ 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:
+		const __u64 *addrs;
+		__u32 i;
+
+		jsonw_uint_field(json_wtr, "func_cnt", info->kprobe_multi.count);
+		if (!info->kprobe_multi.count)
+			break;
+		jsonw_name(json_wtr, "addrs");
+		jsonw_start_array(json_wtr);
+		addrs = (const __u64 *)u64_to_ptr(info->kprobe_multi.addrs);
+		for (i = 0; i < info->kprobe_multi.count; i++)
+			jsonw_lluint(json_wtr, addrs[i]);
+		jsonw_end_array(json_wtr);
+		break;
 	default:
 		break;
 	}
@@ -396,6 +410,24 @@ 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:
+		__u32 indent, cnt, i;
+		const __u64 *addrs;
+
+		cnt = info->kprobe_multi.count;
+		if (!cnt)
+			break;
+		printf("\n\tfunc_cnt %d  addrs", cnt);
+		for (i = 0; cnt; i++)
+			cnt /= 10;
+		indent = strlen("func_cnt ") + i + strlen("  addrs");
+		addrs = (const __u64 *)u64_to_ptr(info->kprobe_multi.addrs);
+		for (i = 0; i < info->kprobe_multi.count; i++) {
+			if (i && !(i & 0x1))
+				printf("\n\t%*s", indent, "");
+			printf(" %0*llx", 16, addrs[i]);
+		}
+		break;
 	default:
 		break;
 	}
@@ -417,7 +449,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 +475,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 = malloc(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;
 }
-- 
1.8.3.1

[RFC PATCH bpf-next 4/8] bpf: Always expose the probed address

[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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 59cda19..a7a905a 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1547,7 +1547,7 @@ 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;
+		*probe_addr = (unsigned long)tk->rp.kp.addr;
 	} else {
 		*symbol = NULL;
 		*probe_offset = 0;
-- 
1.8.3.1

[RFC PATCH bpf-next 5/8] bpf: Support ->show_fdinfo for perf_event

[Yafang Shao]

By adding support for ->show_fdinfo to the perf_event link, users will be
able to examine it through the task's fdinfo. The expected result is as
follows:

$ cat /proc/9637/fdinfo/11
pos:    0
flags:  02000000
mnt_id: 15
ino:    2094
link_type:      perf
link_id:        1
prog_tag:       a04f5eef06a7f555
prog_id:        5
func:   kernel_clone
addr:   ffffffff8d0bc310
offset: 0

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 kernel/bpf/syscall.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 92a57ef..e6b5127 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3297,9 +3297,36 @@ static void bpf_perf_link_dealloc(struct bpf_link *link)
 	kfree(perf_link);
 }
 
+static void bpf_perf_link_show_fdinfo(const struct bpf_link *link,
+				      struct seq_file *seq)
+{
+	struct bpf_perf_link *perf_link = container_of(link, struct bpf_perf_link, link);
+	const struct perf_event *event;
+	u64 probe_offset, probe_addr;
+	u32 prog_id, fd_type;
+	const char *buf;
+	int err;
+
+	event = perf_get_event(perf_link->perf_file);
+	if (IS_ERR(event))
+		return;
+
+	err = bpf_get_perf_event_info(event, &prog_id, &fd_type,
+				      &buf, &probe_offset,
+				      &probe_addr);
+	if (err)
+		return;
+
+	if (buf)
+		seq_printf(seq, "func:\t%s\n", buf);
+	seq_printf(seq, "addr:\t%llx\n", probe_addr);
+	seq_printf(seq, "offset:\t%llu\n", probe_offset);
+}
+
 static const struct bpf_link_ops bpf_perf_link_lops = {
 	.release = bpf_perf_link_release,
 	.dealloc = bpf_perf_link_dealloc,
+	.show_fdinfo = bpf_perf_link_show_fdinfo,
 };
 
 static int bpf_perf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
-- 
1.8.3.1

[RFC PATCH bpf-next 6/8] bpf: Add a common helper bpf_copy_to_user()

[Yafang Shao]

Add a common helper bpf_copy_to_user(), which will be used at multiple
places.

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 e6b5127..33a72ec 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

[RFC PATCH bpf-next 7/8] bpf: Support ->fill_link_info for perf_event

[Yafang Shao]

By adding support for ->fill_link_info to the perf_event link, users will
be able to inspect it using `bpftool link show`. While users can currently
access this information via `bpftool perf show`, consolidating the link
information for all link types in one place would be more convenient.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 include/uapi/linux/bpf.h       |  6 ++++++
 kernel/bpf/syscall.c           | 46 ++++++++++++++++++++++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h |  6 ++++++
 3 files changed, 58 insertions(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 6be9b1d..1f2be1d 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -6438,6 +6438,12 @@ struct bpf_link_info {
 			__aligned_u64 addrs;
 			__u32 count;
 		} kprobe_multi;
+		struct {
+			__aligned_u64 name;
+			__aligned_u64 addr;
+			__u32 name_len;
+			__u32 offset;
+		} perf_event;
 	};
 } __attribute__((aligned(8)));
 
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 33a72ec..b12707e 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3329,10 +3329,56 @@ static void bpf_perf_link_show_fdinfo(const struct bpf_link *link,
 	seq_printf(seq, "offset:\t%llu\n", probe_offset);
 }
 
+static int bpf_perf_link_fill_link_info(const struct bpf_link *link,
+					struct bpf_link_info *info)
+{
+	struct bpf_perf_link *perf_link = container_of(link, struct bpf_perf_link, link);
+	char __user *ubuf = u64_to_user_ptr(info->perf_event.name);
+	u32 ulen = info->perf_event.name_len;
+	const struct perf_event *event;
+	u64 probe_offset, probe_addr;
+	u32 prog_id, fd_type;
+	const char *buf;
+	size_t len;
+	int err;
+
+	if (!ulen ^ !ubuf)
+		return -EINVAL;
+	if (!ubuf)
+		return 0;
+
+	event = perf_get_event(perf_link->perf_file);
+	if (IS_ERR(event))
+		return PTR_ERR(event);
+
+	err = bpf_get_perf_event_info(event, &prog_id, &fd_type,
+				      &buf, &probe_offset,
+				      &probe_addr);
+	if (err)
+		return err;
+
+	len = strlen(buf);
+	info->perf_event.name_len = len + 1;
+	if (buf) {
+		err = bpf_copy_to_user(ubuf, buf, ulen, len);
+		if (err)
+			return err;
+	} else {
+		char zero = '\0';
+
+		if (put_user(zero, ubuf))
+			return -EFAULT;
+	}
+	info->perf_event.addr = probe_addr;
+	info->perf_event.offset = probe_offset;
+	return 0;
+}
+
 static const struct bpf_link_ops bpf_perf_link_lops = {
 	.release = bpf_perf_link_release,
 	.dealloc = bpf_perf_link_dealloc,
 	.show_fdinfo = bpf_perf_link_show_fdinfo,
+	.fill_link_info = bpf_perf_link_fill_link_info,
 };
 
 static int bpf_perf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 6be9b1d..1f2be1d 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -6438,6 +6438,12 @@ struct bpf_link_info {
 			__aligned_u64 addrs;
 			__u32 count;
 		} kprobe_multi;
+		struct {
+			__aligned_u64 name;
+			__aligned_u64 addr;
+			__u32 name_len;
+			__u32 offset;
+		} perf_event;
 	};
 } __attribute__((aligned(8)));
 
-- 
1.8.3.1

[RFC PATCH bpf-next 8/8] bpftool: Show probed function in perf_event link info

[Yafang Shao]

Show the exposed perf_event link info in bpftool. The result as follows,

$ bpftool link show
1: perf_event  prog 5
        func kernel_clone  addr ffffffffb40bc310  offset 0
        bpf_cookie 0
        pids trace(9726)
$ bpftool link show -j
[{"id":1,"type":"perf_event","prog_id":5,"func":"kernel_clone","addr":18446744072435254032,"offset":0,"bpf_cookie":0,"pids":[{"pid":9726,"comm":"trace"}]}]

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 tools/bpf/bpftool/link.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index 76f1bb2..8493a05 100644
--- a/tools/bpf/bpftool/link.c
+++ b/tools/bpf/bpftool/link.c
@@ -232,6 +232,12 @@ static int show_link_close_json(int fd, struct bpf_link_info *info)
 			jsonw_lluint(json_wtr, addrs[i]);
 		jsonw_end_array(json_wtr);
 		break;
+	case BPF_LINK_TYPE_PERF_EVENT:
+		jsonw_string_field(json_wtr, "func",
+				   u64_to_ptr(info->perf_event.name));
+		jsonw_uint_field(json_wtr, "addr", info->perf_event.addr);
+		jsonw_uint_field(json_wtr, "offset", info->perf_event.offset);
+		break;
 	default:
 		break;
 	}
@@ -368,7 +374,7 @@ void netfilter_dump_plain(const struct bpf_link_info *info)
 static int show_link_close_plain(int fd, struct bpf_link_info *info)
 {
 	struct bpf_prog_info prog_info;
-	const char *prog_type_str;
+	const char *prog_type_str, *buf;
 	int err;
 
 	show_link_header_plain(info);
@@ -428,6 +434,12 @@ static int show_link_close_plain(int fd, struct bpf_link_info *info)
 			printf(" %0*llx", 16, addrs[i]);
 		}
 		break;
+	case BPF_LINK_TYPE_PERF_EVENT:
+		buf = (const char *)u64_to_ptr(info->perf_event.name);
+		if (buf[0] != '\0' || info->perf_event.addr)
+			printf("\n\tfunc %s  addr %llx  offset %d  ", buf,
+			       info->perf_event.addr, info->perf_event.offset);
+		break;
 	default:
 		break;
 	}
@@ -454,6 +466,7 @@ static int do_show_link(int fd)
 	int count;
 	int err;
 
+	buf[0] = '\0';
 	memset(&info, 0, sizeof(info));
 again:
 	err = bpf_link_get_info_by_fd(fd, &info, &len);
@@ -489,6 +502,12 @@ static int do_show_link(int fd)
 			goto again;
 		}
 	}
+	if (info.type == BPF_LINK_TYPE_PERF_EVENT &&
+	    !info.perf_event.name) {
+		info.perf_event.name = (unsigned long)&buf;
+		info.perf_event.name_len = sizeof(buf);
+		goto again;
+	}
 
 	if (json_output)
 		show_link_close_json(fd, &info);
-- 
1.8.3.1

Re: [RFC PATCH bpf-next 1/8] bpf: Support ->show_fdinfo for kprobe_multi

[Jiri Olsa]

On Sun, May 28, 2023 at 02:20:20PM +0000, Yafang Shao wrote:
> Currently, there is no way to check which functions are attached to a
> kprobe_multi link, causing confusion for users. It is important that we
> provide a means to expose these functions. The expected result is as follows,
> 
> $ cat /proc/10936/fdinfo/9
> pos:    0
> flags:  02000000
> mnt_id: 15
> ino:    2094
> link_type:      kprobe_multi
> link_id:        2
> prog_tag:       a04f5eef06a7f555
> prog_id:        11
> func_count:     4
> func_addrs:     ffffffffaad475c0
>                 ffffffffaad47600
>                 ffffffffaad47640
>                 ffffffffaad47680

I like the idea of exposing this through the link_info syscall,
but I'm bit concerned of potentially dumping thousands of addresses
through fdinfo file, because I always thought of fdinfo as brief
file info, but that might be just my problem ;-)

jirka

> 
> $ cat /proc/10936/fdinfo/9 | grep "func_addrs" -A 4 | \
>   awk '{ if (NR ==1) {print $2} else {print $1}}' | \
>   awk '{"grep " $1 " /proc/kallsyms"| getline f; print f}'
> ffffffffaad475c0 T schedule_timeout_interruptible
> ffffffffaad47600 T schedule_timeout_killable
> ffffffffaad47640 T schedule_timeout_uninterruptible
> ffffffffaad47680 T schedule_timeout_idle
> 
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
>  kernel/trace/bpf_trace.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 2bc41e6..0d84a7a 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2548,9 +2548,26 @@ static void bpf_kprobe_multi_link_dealloc(struct bpf_link *link)
>  	kfree(kmulti_link);
>  }
>  
> +static void bpf_kprobe_multi_link_show_fdinfo(const struct bpf_link *link,
> +				      struct seq_file *seq)
> +{
> +	struct bpf_kprobe_multi_link *kmulti_link;
> +	int i;
> +
> +	kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link);
> +	seq_printf(seq, "func_count:\t%d\n", kmulti_link->cnt);
> +	for (i = 0; i < kmulti_link->cnt; i++) {
> +		if (i == 0)
> +			seq_printf(seq, "func_addrs:\t%lx\n", kmulti_link->addrs[i]);
> +		else
> +			seq_printf(seq, "           \t%lx\n", kmulti_link->addrs[i]);
> +	}
> +}
> +
>  static const struct bpf_link_ops bpf_kprobe_multi_link_lops = {
>  	.release = bpf_kprobe_multi_link_release,
>  	.dealloc = bpf_kprobe_multi_link_dealloc,
> +	.show_fdinfo = bpf_kprobe_multi_link_show_fdinfo,
>  };
>  
>  static void bpf_kprobe_multi_cookie_swap(void *a, void *b, int size, const void *priv)
> -- 
> 1.8.3.1
> 

Re: [RFC PATCH bpf-next 2/8] bpf: Support ->fill_link_info for kprobe_multi

[Jiri Olsa]

On Sun, May 28, 2023 at 02:20:21PM +0000, Yafang Shao wrote:

SNIP

> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 0d84a7a..00a0009 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2564,10 +2564,41 @@ static void bpf_kprobe_multi_link_show_fdinfo(const struct bpf_link *link,
>  	}
>  }
>  
> +static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link,
> +						struct bpf_link_info *info)
> +{
> +	struct bpf_kprobe_multi_link *kmulti_link;
> +	u64 *uaddrs = u64_to_user_ptr(info->kprobe_multi.addrs);
> +	u32 ucount = info->kprobe_multi.count;
> +	int i;
> +
> +	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)
> +		return 0;
> +
> +	if (ucount != kmulti_link->cnt)
> +		return -EINVAL;
> +
> +	for (i = 0; i < ucount; i++)
> +		if (copy_to_user(uaddrs + i, kmulti_link->addrs + i,
> +				 sizeof(u64)))
> +			return -EFAULT;

let's use put_user instead copy_to_user? or even better why not
copy that with single copy_to_user from kmulti_link->addrs

jirka

> +	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,
>  	.show_fdinfo = bpf_kprobe_multi_link_show_fdinfo,
> +	.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)
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 9273c65..6be9b1d 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -6434,6 +6434,10 @@ struct bpf_link_info {
>  			__s32 priority;
>  			__u32 flags;
>  		} netfilter;
> +		struct {
> +			__aligned_u64 addrs;
> +			__u32 count;
> +		} kprobe_multi;
>  	};
>  } __attribute__((aligned(8)));
>  
> -- 
> 1.8.3.1
> 

Re: [RFC PATCH bpf-next 1/8] bpf: Support ->show_fdinfo for kprobe_multi

[Yafang Shao]

On Mon, May 29, 2023 at 8:06 PM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Sun, May 28, 2023 at 02:20:20PM +0000, Yafang Shao wrote:
> > Currently, there is no way to check which functions are attached to a
> > kprobe_multi link, causing confusion for users. It is important that we
> > provide a means to expose these functions. The expected result is as follows,
> >
> > $ cat /proc/10936/fdinfo/9
> > pos:    0
> > flags:  02000000
> > mnt_id: 15
> > ino:    2094
> > link_type:      kprobe_multi
> > link_id:        2
> > prog_tag:       a04f5eef06a7f555
> > prog_id:        11
> > func_count:     4
> > func_addrs:     ffffffffaad475c0
> >                 ffffffffaad47600
> >                 ffffffffaad47640
> >                 ffffffffaad47680
>
> I like the idea of exposing this through the link_info syscall,
> but I'm bit concerned of potentially dumping thousands of addresses
> through fdinfo file, because I always thought of fdinfo as brief
> file info, but that might be just my problem ;-)

In most cases, there are only a few addresses, and it is uncommon to
have thousands of addresses. To handle this, what about displaying a
maximum of 16 addresses? For cases where the number of addresses
exceeds 16, we can use '...' to represent the remaining addresses.

-- 
Regards
Yafang

Re: [RFC PATCH bpf-next 2/8] bpf: Support ->fill_link_info for kprobe_multi

[Yafang Shao]

On Mon, May 29, 2023 at 8:49 PM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Sun, May 28, 2023 at 02:20:21PM +0000, Yafang Shao wrote:
>
> SNIP
>
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index 0d84a7a..00a0009 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -2564,10 +2564,41 @@ static void bpf_kprobe_multi_link_show_fdinfo(const struct bpf_link *link,
> >       }
> >  }
> >
> > +static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link,
> > +                                             struct bpf_link_info *info)
> > +{
> > +     struct bpf_kprobe_multi_link *kmulti_link;
> > +     u64 *uaddrs = u64_to_user_ptr(info->kprobe_multi.addrs);
> > +     u32 ucount = info->kprobe_multi.count;
> > +     int i;
> > +
> > +     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)
> > +             return 0;
> > +
> > +     if (ucount != kmulti_link->cnt)
> > +             return -EINVAL;
> > +
> > +     for (i = 0; i < ucount; i++)
> > +             if (copy_to_user(uaddrs + i, kmulti_link->addrs + i,
> > +                              sizeof(u64)))
> > +                     return -EFAULT;
>
> let's use put_user instead copy_to_user? or even better why not
> copy that with single copy_to_user from kmulti_link->addrs
>

Good point. I will utilize a single copy_to_user instead.

-- 
Regards
Yafang

Re: [RFC PATCH bpf-next 3/8] bpftool: Show probed function in kprobe_multi link info

[Quentin Monnet]

2023-05-28 14:20 UTC+0000 ~ Yafang Shao <laoar.shao@gmail.com>
> Show the already expose kprobe_multi link info in bpftool. The result as
> follows,
> 
> $ bpftool link show
> 2: kprobe_multi  prog 11
>         func_cnt 4  addrs ffffffffaad475c0 ffffffffaad47600
>                           ffffffffaad47640 ffffffffaad47680
>         pids trace(10936)
> 
> $ bpftool link show -j
> [{"id":1,"type":"perf_event","prog_id":5,"bpf_cookie":0,"pids":[{"pid":10658,"comm":"trace"}]},{"id":2,"type":"kprobe_multi","prog_id":11,"func_cnt":4,"addrs":[18446744072280634816,18446744072280634880,18446744072280634944,18446744072280635008],"pids":[{"pid":10936,"comm":"trace"}]},{"id":120,"type":"iter","prog_id":266,"target_name":"bpf_map"},{"id":121,"type":"iter","prog_id":267,"target_name":"bpf_prog"}]
> 
> $ bpftool link show  | grep -A 1 "func_cnt" | \
>   awk '{if (NR == 1) {print $4; print $5;} else {print $1; print $2} }' | \
>   awk '{"grep " $1 " /proc/kallsyms" | getline f; print f;}'
> ffffffffaad475c0 T schedule_timeout_interruptible
> ffffffffaad47600 T schedule_timeout_killable
> ffffffffaad47640 T schedule_timeout_uninterruptible
> ffffffffaad47680 T schedule_timeout_idle

Looks nice, thank you!

The address is a useful addition, but I feel like most of the time, this
is the actual function name that we'd like to see. We could maybe print
it directly in bpftool, what do you think? We already parse
/proc/kallsyms elsewhere (to get the address of __bpf_call_base()). If
we can parse the file only once for all func_cnt function, the overhead
is maybe acceptable?

> 
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
>  tools/bpf/bpftool/link.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
> index 2d78607..76f1bb2 100644
> --- a/tools/bpf/bpftool/link.c
> +++ b/tools/bpf/bpftool/link.c
> @@ -218,6 +218,20 @@ 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:
> +		const __u64 *addrs;
> +		__u32 i;
> +
> +		jsonw_uint_field(json_wtr, "func_cnt", info->kprobe_multi.count);
> +		if (!info->kprobe_multi.count)
> +			break;

I'd as well avoid having conditional entries in the JSON output. Let's
just keep 0 and empty array in this case?

> +		jsonw_name(json_wtr, "addrs");
> +		jsonw_start_array(json_wtr);
> +		addrs = (const __u64 *)u64_to_ptr(info->kprobe_multi.addrs);
> +		for (i = 0; i < info->kprobe_multi.count; i++)
> +			jsonw_lluint(json_wtr, addrs[i]);
> +		jsonw_end_array(json_wtr);
> +		break;
>  	default:
>  		break;
>  	}
> @@ -396,6 +410,24 @@ 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:
> +		__u32 indent, cnt, i;
> +		const __u64 *addrs;
> +
> +		cnt = info->kprobe_multi.count;
> +		if (!cnt)
> +			break;
> +		printf("\n\tfunc_cnt %d  addrs", cnt);
> +		for (i = 0; cnt; i++)
> +			cnt /= 10;
> +		indent = strlen("func_cnt ") + i + strlen("  addrs");
> +		addrs = (const __u64 *)u64_to_ptr(info->kprobe_multi.addrs);
> +		for (i = 0; i < info->kprobe_multi.count; i++) {
> +			if (i && !(i & 0x1))
> +				printf("\n\t%*s", indent, "");
> +			printf(" %0*llx", 16, addrs[i]);
> +		}
> +		break;
>  	default:
>  		break;
>  	}
> @@ -417,7 +449,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 +475,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 = malloc(count * sizeof(__u64));

Nit: calloc() instead?

> +			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;
>  }

The other bpftool patch (perf_event link) looks good to me.

Thanks,
Quentin

Re: [RFC PATCH bpf-next 1/8] bpf: Support ->show_fdinfo for kprobe_multi

[Alexei Starovoitov]

On Tue, May 30, 2023 at 09:39:01AM +0800, Yafang Shao wrote:
> On Mon, May 29, 2023 at 8:06 PM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Sun, May 28, 2023 at 02:20:20PM +0000, Yafang Shao wrote:
> > > Currently, there is no way to check which functions are attached to a
> > > kprobe_multi link, causing confusion for users. It is important that we
> > > provide a means to expose these functions. The expected result is as follows,
> > >
> > > $ cat /proc/10936/fdinfo/9
> > > pos:    0
> > > flags:  02000000
> > > mnt_id: 15
> > > ino:    2094
> > > link_type:      kprobe_multi
> > > link_id:        2
> > > prog_tag:       a04f5eef06a7f555
> > > prog_id:        11
> > > func_count:     4
> > > func_addrs:     ffffffffaad475c0
> > >                 ffffffffaad47600
> > >                 ffffffffaad47640
> > >                 ffffffffaad47680
> >
> > I like the idea of exposing this through the link_info syscall,
> > but I'm bit concerned of potentially dumping thousands of addresses
> > through fdinfo file, because I always thought of fdinfo as brief
> > file info, but that might be just my problem ;-)
> 
> In most cases, there are only a few addresses, and it is uncommon to

I doubt you have data to prove that kprobe_multi is "few addresses in most cases",
so please don't throw such arguments without proof.

> have thousands of addresses. To handle this, what about displaying a
> maximum of 16 addresses? For cases where the number of addresses
> exceeds 16, we can use '...' to represent the remaining addresses.

at this point the kernel can pick random 16 kernel funcs and it won't be
much worse.

Asking users to do
$ cat /proc/10936/fdinfo/9 | grep "func_addrs" -A 4 | \
  awk '{ if (NR ==1) {print $2} else {print $1}}' | \
  awk '{"grep " $1 " /proc/kallsyms"| getline f; print f}'
ffffffffaad475c0 T schedule_timeout_interruptible
ffffffffaad47600 T schedule_timeout_killable

isn't a great interface either.

The proper interface through fill_link_info and bpftool is good to have,
but fdinfo shouldn't partially duplicate it. So drop this patch and others.

Re: [RFC PATCH bpf-next 3/8] bpftool: Show probed function in kprobe_multi link info

[Alexei Starovoitov]

On Sun, May 28, 2023 at 02:20:22PM +0000, Yafang Shao wrote:
> Show the already expose kprobe_multi link info in bpftool. The result as
> follows,
> 
> $ bpftool link show
> 2: kprobe_multi  prog 11
>         func_cnt 4  addrs ffffffffaad475c0 ffffffffaad47600
>                           ffffffffaad47640 ffffffffaad47680
>         pids trace(10936)
> 
> $ bpftool link show -j
> [{"id":1,"type":"perf_event","prog_id":5,"bpf_cookie":0,"pids":[{"pid":10658,"comm":"trace"}]},{"id":2,"type":"kprobe_multi","prog_id":11,"func_cnt":4,"addrs":[18446744072280634816,18446744072280634880,18446744072280634944,18446744072280635008],"pids":[{"pid":10936,"comm":"trace"}]},{"id":120,"type":"iter","prog_id":266,"target_name":"bpf_map"},{"id":121,"type":"iter","prog_id":267,"target_name":"bpf_prog"}]
> 
> $ bpftool link show  | grep -A 1 "func_cnt" | \
>   awk '{if (NR == 1) {print $4; print $5;} else {print $1; print $2} }' | \
>   awk '{"grep " $1 " /proc/kallsyms" | getline f; print f;}'

This is not human friendly.
Either make bpftool print complete info or don't do it all.

> ffffffffaad475c0 T schedule_timeout_interruptible
> ffffffffaad47600 T schedule_timeout_killable
> ffffffffaad47640 T schedule_timeout_uninterruptible
> ffffffffaad47680 T schedule_timeout_idle
> 
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
>  tools/bpf/bpftool/link.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
> index 2d78607..76f1bb2 100644
> --- a/tools/bpf/bpftool/link.c
> +++ b/tools/bpf/bpftool/link.c
> @@ -218,6 +218,20 @@ 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:
> +		const __u64 *addrs;
> +		__u32 i;
> +
> +		jsonw_uint_field(json_wtr, "func_cnt", info->kprobe_multi.count);
> +		if (!info->kprobe_multi.count)
> +			break;
> +		jsonw_name(json_wtr, "addrs");
> +		jsonw_start_array(json_wtr);
> +		addrs = (const __u64 *)u64_to_ptr(info->kprobe_multi.addrs);
> +		for (i = 0; i < info->kprobe_multi.count; i++)
> +			jsonw_lluint(json_wtr, addrs[i]);
> +		jsonw_end_array(json_wtr);
> +		break;
>  	default:
>  		break;
>  	}
> @@ -396,6 +410,24 @@ 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:
> +		__u32 indent, cnt, i;
> +		const __u64 *addrs;
> +
> +		cnt = info->kprobe_multi.count;
> +		if (!cnt)
> +			break;
> +		printf("\n\tfunc_cnt %d  addrs", cnt);
> +		for (i = 0; cnt; i++)
> +			cnt /= 10;
> +		indent = strlen("func_cnt ") + i + strlen("  addrs");
> +		addrs = (const __u64 *)u64_to_ptr(info->kprobe_multi.addrs);
> +		for (i = 0; i < info->kprobe_multi.count; i++) {
> +			if (i && !(i & 0x1))
> +				printf("\n\t%*s", indent, "");
> +			printf(" %0*llx", 16, addrs[i]);
> +		}
> +		break;
>  	default:
>  		break;
>  	}
> @@ -417,7 +449,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 +475,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 = malloc(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;
>  }
> -- 
> 1.8.3.1
> 

Re: [RFC PATCH bpf-next 7/8] bpf: Support ->fill_link_info for perf_event

[Alexei Starovoitov]

On Sun, May 28, 2023 at 02:20:26PM +0000, Yafang Shao wrote:
> By adding support for ->fill_link_info to the perf_event link, users will
> be able to inspect it using `bpftool link show`. While users can currently
> access this information via `bpftool perf show`, consolidating the link
> information for all link types in one place would be more convenient.
> 
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
>  include/uapi/linux/bpf.h       |  6 ++++++
>  kernel/bpf/syscall.c           | 46 ++++++++++++++++++++++++++++++++++++++++++
>  tools/include/uapi/linux/bpf.h |  6 ++++++
>  3 files changed, 58 insertions(+)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 6be9b1d..1f2be1d 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -6438,6 +6438,12 @@ struct bpf_link_info {
>  			__aligned_u64 addrs;
>  			__u32 count;
>  		} kprobe_multi;
> +		struct {
> +			__aligned_u64 name;
> +			__aligned_u64 addr;

__aligned_u64 ? what is the reason?

> +			__u32 name_len;
> +			__u32 offset;
> +		} perf_event;
>  	};
>  } __attribute__((aligned(8)));
>  
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 33a72ec..b12707e 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -3329,10 +3329,56 @@ static void bpf_perf_link_show_fdinfo(const struct bpf_link *link,
>  	seq_printf(seq, "offset:\t%llu\n", probe_offset);
>  }
>  
> +static int bpf_perf_link_fill_link_info(const struct bpf_link *link,
> +					struct bpf_link_info *info)
> +{
> +	struct bpf_perf_link *perf_link = container_of(link, struct bpf_perf_link, link);
> +	char __user *ubuf = u64_to_user_ptr(info->perf_event.name);
> +	u32 ulen = info->perf_event.name_len;
> +	const struct perf_event *event;
> +	u64 probe_offset, probe_addr;
> +	u32 prog_id, fd_type;
> +	const char *buf;
> +	size_t len;
> +	int err;
> +
> +	if (!ulen ^ !ubuf)
> +		return -EINVAL;
> +	if (!ubuf)
> +		return 0;
> +
> +	event = perf_get_event(perf_link->perf_file);
> +	if (IS_ERR(event))
> +		return PTR_ERR(event);
> +
> +	err = bpf_get_perf_event_info(event, &prog_id, &fd_type,
> +				      &buf, &probe_offset,
> +				      &probe_addr);
> +	if (err)
> +		return err;
> +
> +	len = strlen(buf);
> +	info->perf_event.name_len = len + 1;

this use of name_len contradicts with patch 8.
Is it 'in' or 'out' field?

> +	if (buf) {
> +		err = bpf_copy_to_user(ubuf, buf, ulen, len);
> +		if (err)
> +			return err;
> +	} else {
> +		char zero = '\0';
> +
> +		if (put_user(zero, ubuf))
> +			return -EFAULT;
> +	}
> +	info->perf_event.addr = probe_addr;
> +	info->perf_event.offset = probe_offset;
> +	return 0;
> +}
> +
>  static const struct bpf_link_ops bpf_perf_link_lops = {
>  	.release = bpf_perf_link_release,
>  	.dealloc = bpf_perf_link_dealloc,
>  	.show_fdinfo = bpf_perf_link_show_fdinfo,
> +	.fill_link_info = bpf_perf_link_fill_link_info,
>  };
>  
>  static int bpf_perf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 6be9b1d..1f2be1d 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -6438,6 +6438,12 @@ struct bpf_link_info {
>  			__aligned_u64 addrs;
>  			__u32 count;
>  		} kprobe_multi;
> +		struct {
> +			__aligned_u64 name;
> +			__aligned_u64 addr;
> +			__u32 name_len;
> +			__u32 offset;
> +		} perf_event;
>  	};
>  } __attribute__((aligned(8)));
>  
> -- 
> 1.8.3.1
> 

Re: [RFC PATCH bpf-next 1/8] bpf: Support ->show_fdinfo for kprobe_multi

[Yafang Shao]

On Wed, May 31, 2023 at 8:29 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue, May 30, 2023 at 09:39:01AM +0800, Yafang Shao wrote:
> > On Mon, May 29, 2023 at 8:06 PM Jiri Olsa <olsajiri@gmail.com> wrote:
> > >
> > > On Sun, May 28, 2023 at 02:20:20PM +0000, Yafang Shao wrote:
> > > > Currently, there is no way to check which functions are attached to a
> > > > kprobe_multi link, causing confusion for users. It is important that we
> > > > provide a means to expose these functions. The expected result is as follows,
> > > >
> > > > $ cat /proc/10936/fdinfo/9
> > > > pos:    0
> > > > flags:  02000000
> > > > mnt_id: 15
> > > > ino:    2094
> > > > link_type:      kprobe_multi
> > > > link_id:        2
> > > > prog_tag:       a04f5eef06a7f555
> > > > prog_id:        11
> > > > func_count:     4
> > > > func_addrs:     ffffffffaad475c0
> > > >                 ffffffffaad47600
> > > >                 ffffffffaad47640
> > > >                 ffffffffaad47680
> > >
> > > I like the idea of exposing this through the link_info syscall,
> > > but I'm bit concerned of potentially dumping thousands of addresses
> > > through fdinfo file, because I always thought of fdinfo as brief
> > > file info, but that might be just my problem ;-)
> >
> > In most cases, there are only a few addresses, and it is uncommon to
>
> I doubt you have data to prove that kprobe_multi is "few addresses in most cases",
> so please don't throw such arguments without proof.
>
> > have thousands of addresses. To handle this, what about displaying a
> > maximum of 16 addresses? For cases where the number of addresses
> > exceeds 16, we can use '...' to represent the remaining addresses.
>
> at this point the kernel can pick random 16 kernel funcs and it won't be
> much worse.
>
> Asking users to do
> $ cat /proc/10936/fdinfo/9 | grep "func_addrs" -A 4 | \
>   awk '{ if (NR ==1) {print $2} else {print $1}}' | \
>   awk '{"grep " $1 " /proc/kallsyms"| getline f; print f}'
> ffffffffaad475c0 T schedule_timeout_interruptible
> ffffffffaad47600 T schedule_timeout_killable
>
> isn't a great interface either.
>
> The proper interface through fill_link_info and bpftool is good to have,
> but fdinfo shouldn't partially duplicate it. So drop this patch and others.

Sure, I will drop the ->show_fdinfo patches.

-- 
Regards
Yafang

Re: [RFC PATCH bpf-next 3/8] bpftool: Show probed function in kprobe_multi link info

[Yafang Shao]

On Tue, May 30, 2023 at 7:16 PM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2023-05-28 14:20 UTC+0000 ~ Yafang Shao <laoar.shao@gmail.com>
> > Show the already expose kprobe_multi link info in bpftool. The result as
> > follows,
> >
> > $ bpftool link show
> > 2: kprobe_multi  prog 11
> >         func_cnt 4  addrs ffffffffaad475c0 ffffffffaad47600
> >                           ffffffffaad47640 ffffffffaad47680
> >         pids trace(10936)
> >
> > $ bpftool link show -j
> > [{"id":1,"type":"perf_event","prog_id":5,"bpf_cookie":0,"pids":[{"pid":10658,"comm":"trace"}]},{"id":2,"type":"kprobe_multi","prog_id":11,"func_cnt":4,"addrs":[18446744072280634816,18446744072280634880,18446744072280634944,18446744072280635008],"pids":[{"pid":10936,"comm":"trace"}]},{"id":120,"type":"iter","prog_id":266,"target_name":"bpf_map"},{"id":121,"type":"iter","prog_id":267,"target_name":"bpf_prog"}]
> >
> > $ bpftool link show  | grep -A 1 "func_cnt" | \
> >   awk '{if (NR == 1) {print $4; print $5;} else {print $1; print $2} }' | \
> >   awk '{"grep " $1 " /proc/kallsyms" | getline f; print f;}'
> > ffffffffaad475c0 T schedule_timeout_interruptible
> > ffffffffaad47600 T schedule_timeout_killable
> > ffffffffaad47640 T schedule_timeout_uninterruptible
> > ffffffffaad47680 T schedule_timeout_idle
>
> Looks nice, thank you!
>
> The address is a useful addition, but I feel like most of the time, this
> is the actual function name that we'd like to see. We could maybe print
> it directly in bpftool, what do you think? We already parse
> /proc/kallsyms elsewhere (to get the address of __bpf_call_base()). If
> we can parse the file only once for all func_cnt function, the overhead
> is maybe acceptable?
>

Thanks for your suggestion. Will change it.

> >
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > ---
> >  tools/bpf/bpftool/link.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 50 insertions(+)
> >
> > diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
> > index 2d78607..76f1bb2 100644
> > --- a/tools/bpf/bpftool/link.c
> > +++ b/tools/bpf/bpftool/link.c
> > @@ -218,6 +218,20 @@ 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:
> > +             const __u64 *addrs;
> > +             __u32 i;
> > +
> > +             jsonw_uint_field(json_wtr, "func_cnt", info->kprobe_multi.count);
> > +             if (!info->kprobe_multi.count)
> > +                     break;
>
> I'd as well avoid having conditional entries in the JSON output. Let's
> just keep 0 and empty array in this case?
>

Will do it.

> > +             jsonw_name(json_wtr, "addrs");
> > +             jsonw_start_array(json_wtr);
> > +             addrs = (const __u64 *)u64_to_ptr(info->kprobe_multi.addrs);
> > +             for (i = 0; i < info->kprobe_multi.count; i++)
> > +                     jsonw_lluint(json_wtr, addrs[i]);
> > +             jsonw_end_array(json_wtr);
> > +             break;
> >       default:
> >               break;
> >       }
> > @@ -396,6 +410,24 @@ 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:
> > +             __u32 indent, cnt, i;
> > +             const __u64 *addrs;
> > +
> > +             cnt = info->kprobe_multi.count;
> > +             if (!cnt)
> > +                     break;
> > +             printf("\n\tfunc_cnt %d  addrs", cnt);
> > +             for (i = 0; cnt; i++)
> > +                     cnt /= 10;
> > +             indent = strlen("func_cnt ") + i + strlen("  addrs");
> > +             addrs = (const __u64 *)u64_to_ptr(info->kprobe_multi.addrs);
> > +             for (i = 0; i < info->kprobe_multi.count; i++) {
> > +                     if (i && !(i & 0x1))
> > +                             printf("\n\t%*s", indent, "");
> > +                     printf(" %0*llx", 16, addrs[i]);
> > +             }
> > +             break;
> >       default:
> >               break;
> >       }
> > @@ -417,7 +449,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 +475,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 = malloc(count * sizeof(__u64));
>
> Nit: calloc() instead?

Good point. Will do it.

>
> > +                     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;
> >  }
>
> The other bpftool patch (perf_event link) looks good to me.
>

Thanks for your review.

-- 
Regards
Yafang

Re: [RFC PATCH bpf-next 3/8] bpftool: Show probed function in kprobe_multi link info

[Yafang Shao]

On Wed, May 31, 2023 at 8:31 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Sun, May 28, 2023 at 02:20:22PM +0000, Yafang Shao wrote:
> > Show the already expose kprobe_multi link info in bpftool. The result as
> > follows,
> >
> > $ bpftool link show
> > 2: kprobe_multi  prog 11
> >         func_cnt 4  addrs ffffffffaad475c0 ffffffffaad47600
> >                           ffffffffaad47640 ffffffffaad47680
> >         pids trace(10936)
> >
> > $ bpftool link show -j
> > [{"id":1,"type":"perf_event","prog_id":5,"bpf_cookie":0,"pids":[{"pid":10658,"comm":"trace"}]},{"id":2,"type":"kprobe_multi","prog_id":11,"func_cnt":4,"addrs":[18446744072280634816,18446744072280634880,18446744072280634944,18446744072280635008],"pids":[{"pid":10936,"comm":"trace"}]},{"id":120,"type":"iter","prog_id":266,"target_name":"bpf_map"},{"id":121,"type":"iter","prog_id":267,"target_name":"bpf_prog"}]
> >
> > $ bpftool link show  | grep -A 1 "func_cnt" | \
> >   awk '{if (NR == 1) {print $4; print $5;} else {print $1; print $2} }' | \
> >   awk '{"grep " $1 " /proc/kallsyms" | getline f; print f;}'
>
> This is not human friendly.
> Either make bpftool print complete info or don't do it all.
>

I will make bpftool print complete info in the next version.

-- 
Regards
Yafang

Re: [RFC PATCH bpf-next 7/8] bpf: Support ->fill_link_info for perf_event

[Yafang Shao]

On Wed, May 31, 2023 at 8:37 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Sun, May 28, 2023 at 02:20:26PM +0000, Yafang Shao wrote:
> > By adding support for ->fill_link_info to the perf_event link, users will
> > be able to inspect it using `bpftool link show`. While users can currently
> > access this information via `bpftool perf show`, consolidating the link
> > information for all link types in one place would be more convenient.
> >
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > ---
> >  include/uapi/linux/bpf.h       |  6 ++++++
> >  kernel/bpf/syscall.c           | 46 ++++++++++++++++++++++++++++++++++++++++++
> >  tools/include/uapi/linux/bpf.h |  6 ++++++
> >  3 files changed, 58 insertions(+)
> >
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 6be9b1d..1f2be1d 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -6438,6 +6438,12 @@ struct bpf_link_info {
> >                       __aligned_u64 addrs;
> >                       __u32 count;
> >               } kprobe_multi;
> > +             struct {
> > +                     __aligned_u64 name;
> > +                     __aligned_u64 addr;
>
> __aligned_u64 ? what is the reason?

It is because of the copy-and-paste.  Will use _u64 instead.

>
> > +                     __u32 name_len;
> > +                     __u32 offset;
> > +             } perf_event;
> >       };
> >  } __attribute__((aligned(8)));
> >
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index 33a72ec..b12707e 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -3329,10 +3329,56 @@ static void bpf_perf_link_show_fdinfo(const struct bpf_link *link,
> >       seq_printf(seq, "offset:\t%llu\n", probe_offset);
> >  }
> >
> > +static int bpf_perf_link_fill_link_info(const struct bpf_link *link,
> > +                                     struct bpf_link_info *info)
> > +{
> > +     struct bpf_perf_link *perf_link = container_of(link, struct bpf_perf_link, link);
> > +     char __user *ubuf = u64_to_user_ptr(info->perf_event.name);
> > +     u32 ulen = info->perf_event.name_len;
> > +     const struct perf_event *event;
> > +     u64 probe_offset, probe_addr;
> > +     u32 prog_id, fd_type;
> > +     const char *buf;
> > +     size_t len;
> > +     int err;
> > +
> > +     if (!ulen ^ !ubuf)
> > +             return -EINVAL;
> > +     if (!ubuf)
> > +             return 0;
> > +
> > +     event = perf_get_event(perf_link->perf_file);
> > +     if (IS_ERR(event))
> > +             return PTR_ERR(event);
> > +
> > +     err = bpf_get_perf_event_info(event, &prog_id, &fd_type,
> > +                                   &buf, &probe_offset,
> > +                                   &probe_addr);
> > +     if (err)
> > +             return err;
> > +
> > +     len = strlen(buf);
> > +     info->perf_event.name_len = len + 1;
>
> this use of name_len contradicts with patch 8.
> Is it 'in' or 'out' field?

My mistake. I should remove this sentence.
The reason I didn't do it the same with
bpf_raw_tp_link_fill_link_info() is that if we return the buf length
to the userspace when the ubuf is NULL, we have to call
bpf_get_perf_event_info() multiple times.

-- 
Regards
Yafang