Skip to contentSkip to conversation

bpf: Add cookies retrieval for perf/kprobe multi links

PATCH Superseded Jiri Olsa 4 participants 19 messages Jan 18, 2024 – Jan 19, 2024

Subsystems (8)
Kselftest BPF Tracing BPF selftests +4 more subsystems

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Jiri Olsa

author reviewer
#0 lore raw

cover bpf: Add cookies retrieval for perf/kprobe multi links

hi,
this patchset adds support to retrieve cookies from existing tracing links that still did not support it plus changes to bpftool to display them. It's leftover we discussed some time ago [1].

thanks,
jirka

[1] https://lore.kernel.org/bpf/CALOAHbAZ6=A9j3VFCLoAC_WhgQKU7injMf06=cM2sU4Hi4Sx+Q@mail.gmail.com/
---
Jiri Olsa (8):
bpf: Add cookie to perf_event bpf_link_info records
bpf: Store cookies in kprobe_multi bpf_link_info data
bpftool: Fix wrong free call in do_show_link
selftests/bpf: Add cookies check for kprobe_multi fill_link_info test selftests/bpf: Add cookies check for perf_event fill_link_info test selftests/bpf: Add fill_link_info test for perf event
bpftool: Display cookie for perf event link probes
bpftool: Display cookie for kprobe multi link

 include/uapi/linux/bpf.h                                |   5 +++++
 kernel/bpf/syscall.c                                    |   4 ++++
 kernel/trace/bpf_trace.c                                |  15 +++++++++++++
 tools/bpf/bpftool/link.c                                |  87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------
 tools/include/uapi/linux/bpf.h                          |   5 +++++
 tools/testing/selftests/bpf/prog_tests/fill_link_info.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------
 tools/testing/selftests/bpf/progs/test_fill_link_info.c |   6 +++++
 7 files changed, 204 insertions(+), 32 deletions(-)
author reviewer
#1 lore raw

PATCH 1/8 bpf: Add cookie to perf_event bpf_link_info records

At the moment we don't store cookie for perf_event probes, while we do that for the rest of the probes.

Adding cookie fields to struct bpf_link_info perf event probe records:

perf_event.uprobe
perf_event.kprobe
perf_event.tracepoint
perf_event.perf_event

And the code to store that in bpf_link_info struct.

---
 include/uapi/linux/bpf.h       | 4 ++++
 kernel/bpf/syscall.c           | 4 ++++
 tools/include/uapi/linux/bpf.h | 4 ++++
 3 files changed, 12 insertions(+)
include/uapi/linux/bpf.h +4 -0
index a00f8a5623e1..b823d367a83c 100644
@@ -6582,6 +6582,7 @@ struct bpf_link_info {
 					__aligned_u64 file_name; /* in/out */
 					__u32 name_len;
 					__u32 offset; /* offset from file_name */
+					__u64 cookie;
 				} uprobe; /* BPF_PERF_EVENT_UPROBE, BPF_PERF_EVENT_URETPROBE */
 				struct {
 					__aligned_u64 func_name; /* in/out */
@@ -6589,14 +6590,17 @@ struct bpf_link_info {
 					__u32 offset; /* offset from func_name */
 					__u64 addr;
 					__u64 missed;
+					__u64 cookie;
 				} kprobe; /* BPF_PERF_EVENT_KPROBE, BPF_PERF_EVENT_KRETPROBE */
 				struct {
 					__aligned_u64 tp_name;   /* in/out */
 					__u32 name_len;
+					__u64 cookie;
 				} tracepoint; /* BPF_PERF_EVENT_TRACEPOINT */
 				struct {
 					__u64 config;
 					__u32 type;
+					__u64 cookie;
 				} event; /* BPF_PERF_EVENT_EVENT */
 			};
 		} perf_event;
kernel/bpf/syscall.c +4 -0
index a1f18681721c..13193aaafb64 100644
@@ -3501,6 +3501,7 @@ static int bpf_perf_link_fill_kprobe(const struct perf_event *event,
 	if (!kallsyms_show_value(current_cred()))
 		addr = 0;
 	info->perf_event.kprobe.addr = addr;
+	info->perf_event.kprobe.cookie = event->bpf_cookie;
 	return 0;
 }
 #endif
@@ -3526,6 +3527,7 @@ static int bpf_perf_link_fill_uprobe(const struct perf_event *event,
 	else
 		info->perf_event.type = BPF_PERF_EVENT_UPROBE;
 	info->perf_event.uprobe.offset = offset;
+	info->perf_event.uprobe.cookie = event->bpf_cookie;
 	return 0;
 }
 #endif
@@ -3553,6 +3555,7 @@ static int bpf_perf_link_fill_tracepoint(const struct perf_event *event,
 	uname = u64_to_user_ptr(info->perf_event.tracepoint.tp_name);
 	ulen = info->perf_event.tracepoint.name_len;
 	info->perf_event.type = BPF_PERF_EVENT_TRACEPOINT;
+	info->perf_event.tracepoint.cookie = event->bpf_cookie;
 	return bpf_perf_link_fill_common(event, uname, ulen, NULL, NULL, NULL, NULL);
 }
 
@@ -3561,6 +3564,7 @@ static int bpf_perf_link_fill_perf_event(const struct perf_event *event,
 {
 	info->perf_event.event.type = event->attr.type;
 	info->perf_event.event.config = event->attr.config;
+	info->perf_event.event.cookie = event->bpf_cookie;
 	info->perf_event.type = BPF_PERF_EVENT_EVENT;
 	return 0;
 }
tools/include/uapi/linux/bpf.h +4 -0
index a00f8a5623e1..b823d367a83c 100644
@@ -6582,6 +6582,7 @@ struct bpf_link_info {
 					__aligned_u64 file_name; /* in/out */
 					__u32 name_len;
 					__u32 offset; /* offset from file_name */
+					__u64 cookie;
 				} uprobe; /* BPF_PERF_EVENT_UPROBE, BPF_PERF_EVENT_URETPROBE */
 				struct {
 					__aligned_u64 func_name; /* in/out */
@@ -6589,14 +6590,17 @@ struct bpf_link_info {
 					__u32 offset; /* offset from func_name */
 					__u64 addr;
 					__u64 missed;
+					__u64 cookie;
 				} kprobe; /* BPF_PERF_EVENT_KPROBE, BPF_PERF_EVENT_KRETPROBE */
 				struct {
 					__aligned_u64 tp_name;   /* in/out */
 					__u32 name_len;
+					__u64 cookie;
 				} tracepoint; /* BPF_PERF_EVENT_TRACEPOINT */
 				struct {
 					__u64 config;
 					__u32 type;
+					__u64 cookie;
 				} event; /* BPF_PERF_EVENT_EVENT */
 			};
 		} perf_event;
Signature · 1 line
2.43.0
#2 lore raw

Re: [PATCH bpf-next 1/8] bpf: Add cookie to perf_event bpf_link_info records

Quoted 34 lines · Jiri Olsa

At the moment we don't store cookie for perf_event probes, while we do that for the rest of the probes.

Adding cookie fields to struct bpf_link_info perf event probe records:

perf_event.uprobe
perf_event.kprobe
perf_event.tracepoint
perf_event.perf_event

And the code to store that in bpf_link_info struct.

---
 include/uapi/linux/bpf.h       | 4 ++++
 kernel/bpf/syscall.c           | 4 ++++
 tools/include/uapi/linux/bpf.h | 4 ++++
 3 files changed, 12 insertions(+)
include/uapi/linux/bpf.h +1 -0
index a00f8a5623e1..b823d367a83c 100644
@@ -6582,6 +6582,7 @@ struct bpf_link_info {
                                        __aligned_u64 file_name; /* in/out */
                                        __u32 name_len;
                                        __u32 offset; /* offset from file_name */
+                                       __u64 cookie;
                                } uprobe; /* BPF_PERF_EVENT_UPROBE, BPF_PERF_EVENT_URETPROBE */
                                struct {
                                        __aligned_u64 func_name; /* in/out */

include/uapi/linux/bpf.h struct bpf_link_info { · line 6597

Quoted 8 lines
include/uapi/linux/bpf.h +1 -0
@@ -6589,14 +6590,17 @@ struct bpf_link_info {
                                        __u32 offset; /* offset from func_name */
                                        __u64 addr;
                                        __u64 missed;
+                                       __u64 cookie;
                                } kprobe; /* BPF_PERF_EVENT_KPROBE, BPF_PERF_EVENT_KRETPROBE */
                                struct {
                                        __aligned_u64 tp_name;   /* in/out */
                                        __u32 name_len;

It might be beneficial to include an alignment pad '__u32 :32;' here, following the pattern used in other instances within this file.

include/uapi/linux/bpf.h struct bpf_link_info { · line 6602

Quoted 5 lines
include/uapi/linux/bpf.h +1 -0
+                                       __u64 cookie;
                                } tracepoint; /* BPF_PERF_EVENT_TRACEPOINT */
                                struct {
                                        __u64 config;
                                        __u32 type;

Same here.

Quoted 73 lines
include/uapi/linux/bpf.h +1 -0
+                                       __u64 cookie;
                                } event; /* BPF_PERF_EVENT_EVENT */
                        };
                } perf_event;
kernel/bpf/syscall.c +4 -0
index a1f18681721c..13193aaafb64 100644
@@ -3501,6 +3501,7 @@ static int bpf_perf_link_fill_kprobe(const struct perf_event *event,
        if (!kallsyms_show_value(current_cred()))
                addr = 0;
        info->perf_event.kprobe.addr = addr;
+       info->perf_event.kprobe.cookie = event->bpf_cookie;
        return 0;
 }
 #endif
@@ -3526,6 +3527,7 @@ static int bpf_perf_link_fill_uprobe(const struct perf_event *event,
        else
                info->perf_event.type = BPF_PERF_EVENT_UPROBE;
        info->perf_event.uprobe.offset = offset;
+       info->perf_event.uprobe.cookie = event->bpf_cookie;
        return 0;
 }
 #endif
@@ -3553,6 +3555,7 @@ static int bpf_perf_link_fill_tracepoint(const struct perf_event *event,
        uname = u64_to_user_ptr(info->perf_event.tracepoint.tp_name);
        ulen = info->perf_event.tracepoint.name_len;
        info->perf_event.type = BPF_PERF_EVENT_TRACEPOINT;
+       info->perf_event.tracepoint.cookie = event->bpf_cookie;
        return bpf_perf_link_fill_common(event, uname, ulen, NULL, NULL, NULL, NULL);
 }
 
@@ -3561,6 +3564,7 @@ static int bpf_perf_link_fill_perf_event(const struct perf_event *event,
 {
        info->perf_event.event.type = event->attr.type;
        info->perf_event.event.config = event->attr.config;
+       info->perf_event.event.cookie = event->bpf_cookie;
        info->perf_event.type = BPF_PERF_EVENT_EVENT;
        return 0;
 }
tools/include/uapi/linux/bpf.h +4 -1
index a00f8a5623e1..b823d367a83c 100644
@@ -6582,6 +6582,7 @@ struct bpf_link_info {
                                        __aligned_u64 file_name; /* in/out */
                                        __u32 name_len;
                                        __u32 offset; /* offset from file_name */
+                                       __u64 cookie;
                                } uprobe; /* BPF_PERF_EVENT_UPROBE, BPF_PERF_EVENT_URETPROBE */
                                struct {
                                        __aligned_u64 func_name; /* in/out */
@@ -6589,14 +6590,17 @@ struct bpf_link_info {
                                        __u32 offset; /* offset from func_name */
                                        __u64 addr;
                                        __u64 missed;
+                                       __u64 cookie;
                                } kprobe; /* BPF_PERF_EVENT_KPROBE, BPF_PERF_EVENT_KRETPROBE */
                                struct {
                                        __aligned_u64 tp_name;   /* in/out */
                                        __u32 name_len;
+                                       __u64 cookie;
                                } tracepoint; /* BPF_PERF_EVENT_TRACEPOINT */
                                struct {
                                        __u64 config;
                                        __u32 type;
+                                       __u64 cookie;
                                } event; /* BPF_PERF_EVENT_EVENT */
                        };
                } perf_event;
--

2.43.0

Signature · 2 lines
Regards
Yafang
#3 lore raw

Re: [PATCH bpf-next 1/8] bpf: Add cookie to perf_event bpf_link_info records

Quoted 38 lines · Yafang Shao
Quoted 37 lines · Jiri Olsa

At the moment we don't store cookie for perf_event probes, while we do that for the rest of the probes.

Adding cookie fields to struct bpf_link_info perf event probe records:

perf_event.uprobe
perf_event.kprobe
perf_event.tracepoint
perf_event.perf_event

And the code to store that in bpf_link_info struct.

---
 include/uapi/linux/bpf.h       | 4 ++++
 kernel/bpf/syscall.c           | 4 ++++
 tools/include/uapi/linux/bpf.h | 4 ++++
 3 files changed, 12 insertions(+)
include/uapi/linux/bpf.h +1 -0
index a00f8a5623e1..b823d367a83c 100644
@@ -6582,6 +6582,7 @@ struct bpf_link_info {
                                        __aligned_u64 file_name; /* in/out */
                                        __u32 name_len;
                                        __u32 offset; /* offset from file_name */
+                                       __u64 cookie;
                                } uprobe; /* BPF_PERF_EVENT_UPROBE, BPF_PERF_EVENT_URETPROBE */
                                struct {
                                        __aligned_u64 func_name; /* in/out */
@@ -6589,14 +6590,17 @@ struct bpf_link_info {
                                        __u32 offset; /* offset from func_name */
                                        __u64 addr;
                                        __u64 missed;
Quoted 8 lines

include/uapi/linux/bpf.h struct bpf_link_info { · line 6597

Quoted 5 lines
include/uapi/linux/bpf.h +1 -0
+                                       __u64 cookie;
                                } kprobe; /* BPF_PERF_EVENT_KPROBE, BPF_PERF_EVENT_KRETPROBE */
                                struct {
                                        __aligned_u64 tp_name;   /* in/out */
                                        __u32 name_len;

It might be beneficial to include an alignment pad '__u32 :32;' here, following the pattern used in other instances within this file.

good cactch, thanks

jirka

Quoted 87 lines

include/uapi/linux/bpf.h struct bpf_link_info { · line 6602

Quoted 6 lines
include/uapi/linux/bpf.h +1 -0
+                                       __u64 cookie;
                                } tracepoint; /* BPF_PERF_EVENT_TRACEPOINT */
                                struct {
                                        __u64 config;
                                        __u32 type;

Same here.

Quoted 73 lines
include/uapi/linux/bpf.h +1 -0
+                                       __u64 cookie;
                                } event; /* BPF_PERF_EVENT_EVENT */
                        };
                } perf_event;
kernel/bpf/syscall.c +4 -0
index a1f18681721c..13193aaafb64 100644
@@ -3501,6 +3501,7 @@ static int bpf_perf_link_fill_kprobe(const struct perf_event *event,
        if (!kallsyms_show_value(current_cred()))
                addr = 0;
        info->perf_event.kprobe.addr = addr;
+       info->perf_event.kprobe.cookie = event->bpf_cookie;
        return 0;
 }
 #endif
@@ -3526,6 +3527,7 @@ static int bpf_perf_link_fill_uprobe(const struct perf_event *event,
        else
                info->perf_event.type = BPF_PERF_EVENT_UPROBE;
        info->perf_event.uprobe.offset = offset;
+       info->perf_event.uprobe.cookie = event->bpf_cookie;
        return 0;
 }
 #endif
@@ -3553,6 +3555,7 @@ static int bpf_perf_link_fill_tracepoint(const struct perf_event *event,
        uname = u64_to_user_ptr(info->perf_event.tracepoint.tp_name);
        ulen = info->perf_event.tracepoint.name_len;
        info->perf_event.type = BPF_PERF_EVENT_TRACEPOINT;
+       info->perf_event.tracepoint.cookie = event->bpf_cookie;
        return bpf_perf_link_fill_common(event, uname, ulen, NULL, NULL, NULL, NULL);
 }
 
@@ -3561,6 +3564,7 @@ static int bpf_perf_link_fill_perf_event(const struct perf_event *event,
 {
        info->perf_event.event.type = event->attr.type;
        info->perf_event.event.config = event->attr.config;
+       info->perf_event.event.cookie = event->bpf_cookie;
        info->perf_event.type = BPF_PERF_EVENT_EVENT;
        return 0;
 }
tools/include/uapi/linux/bpf.h +4 -1
index a00f8a5623e1..b823d367a83c 100644
@@ -6582,6 +6582,7 @@ struct bpf_link_info {
                                        __aligned_u64 file_name; /* in/out */
                                        __u32 name_len;
                                        __u32 offset; /* offset from file_name */
+                                       __u64 cookie;
                                } uprobe; /* BPF_PERF_EVENT_UPROBE, BPF_PERF_EVENT_URETPROBE */
                                struct {
                                        __aligned_u64 func_name; /* in/out */
@@ -6589,14 +6590,17 @@ struct bpf_link_info {
                                        __u32 offset; /* offset from func_name */
                                        __u64 addr;
                                        __u64 missed;
+                                       __u64 cookie;
                                } kprobe; /* BPF_PERF_EVENT_KPROBE, BPF_PERF_EVENT_KRETPROBE */
                                struct {
                                        __aligned_u64 tp_name;   /* in/out */
                                        __u32 name_len;
+                                       __u64 cookie;
                                } tracepoint; /* BPF_PERF_EVENT_TRACEPOINT */
                                struct {
                                        __u64 config;
                                        __u32 type;
+                                       __u64 cookie;
                                } event; /* BPF_PERF_EVENT_EVENT */
                        };
                } perf_event;
--

2.43.0

Signature · 2 lines
Regards
Yafang
author reviewer
#4 lore raw

PATCH 2/8 bpf: Store cookies in kprobe_multi bpf_link_info data

In reply to Jiri Olsa

Storing cookies in kprobe_multi bpf_link_info data. The cookies field is optional and if provided it needs to be an array of __u64 with kprobe_multi.count length.

---
 include/uapi/linux/bpf.h       |  1 +
 kernel/trace/bpf_trace.c       | 15 +++++++++++++++
 tools/include/uapi/linux/bpf.h |  1 +
 3 files changed, 17 insertions(+)
include/uapi/linux/bpf.h +1 -0
index b823d367a83c..199cb93dca7f 100644
@@ -6563,6 +6563,7 @@ struct bpf_link_info {
 			__u32 count; /* in/out: kprobe_multi function count */
 			__u32 flags;
 			__u64 missed;
+			__aligned_u64 cookies;
 		} kprobe_multi;
 		struct {
 			__aligned_u64 path;
kernel/trace/bpf_trace.c +15 -0
index 7ac6c52b25eb..c98c20abaf99 100644
@@ -2679,6 +2679,7 @@ static void bpf_kprobe_multi_link_dealloc(struct bpf_link *link)
 static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link,
 						struct bpf_link_info *info)
 {
+	u64 __user *ucookies = u64_to_user_ptr(info->kprobe_multi.cookies);
 	u64 __user *uaddrs = u64_to_user_ptr(info->kprobe_multi.addrs);
 	struct bpf_kprobe_multi_link *kmulti_link;
 	u32 ucount = info->kprobe_multi.count;
@@ -2686,6 +2687,8 @@ static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link,
 
 	if (!uaddrs ^ !ucount)
 		return -EINVAL;
+	if (ucookies && !ucount)
+		return -EINVAL;
 
 	kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link);
 	info->kprobe_multi.count = kmulti_link->cnt;
@@ -2699,6 +2702,18 @@ static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link,
 	else
 		ucount = kmulti_link->cnt;
 
+	if (ucookies) {
+		if (kmulti_link->cookies) {
+			if (copy_to_user(ucookies, kmulti_link->cookies, ucount * sizeof(u64)))
+				return -EFAULT;
+		} else {
+			for (i = 0; i < ucount; i++) {
+				if (put_user(0, ucookies + i))
+					return -EFAULT;
+			}
+		}
+	}
+
 	if (kallsyms_show_value(current_cred())) {
 		if (copy_to_user(uaddrs, kmulti_link->addrs, ucount * sizeof(u64)))
 			return -EFAULT;
tools/include/uapi/linux/bpf.h +1 -0
index b823d367a83c..199cb93dca7f 100644
@@ -6563,6 +6563,7 @@ struct bpf_link_info {
 			__u32 count; /* in/out: kprobe_multi function count */
 			__u32 flags;
 			__u64 missed;
+			__aligned_u64 cookies;
 		} kprobe_multi;
 		struct {
 			__aligned_u64 path;
Signature · 1 line
2.43.0
Acked-by on v1 2/8
#5 lore raw

Re: [PATCH bpf-next 2/8] bpf: Store cookies in kprobe_multi bpf_link_info data

Quoted 6 lines · Jiri Olsa

Storing cookies in kprobe_multi bpf_link_info data. The cookies field is optional and if provided it needs to be an array of __u64 with kprobe_multi.count length.

LGTM.

Quoted 73 lines
---
 include/uapi/linux/bpf.h       |  1 +
 kernel/trace/bpf_trace.c       | 15 +++++++++++++++
 tools/include/uapi/linux/bpf.h |  1 +
 3 files changed, 17 insertions(+)
include/uapi/linux/bpf.h +1 -0
index b823d367a83c..199cb93dca7f 100644
@@ -6563,6 +6563,7 @@ struct bpf_link_info {
                        __u32 count; /* in/out: kprobe_multi function count */
                        __u32 flags;
                        __u64 missed;
+                       __aligned_u64 cookies;
                } kprobe_multi;
                struct {
                        __aligned_u64 path;
kernel/trace/bpf_trace.c +15 -0
index 7ac6c52b25eb..c98c20abaf99 100644
@@ -2679,6 +2679,7 @@ static void bpf_kprobe_multi_link_dealloc(struct bpf_link *link)
 static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link,
                                                struct bpf_link_info *info)
 {
+       u64 __user *ucookies = u64_to_user_ptr(info->kprobe_multi.cookies);
        u64 __user *uaddrs = u64_to_user_ptr(info->kprobe_multi.addrs);
        struct bpf_kprobe_multi_link *kmulti_link;
        u32 ucount = info->kprobe_multi.count;
@@ -2686,6 +2687,8 @@ static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link,
 
        if (!uaddrs ^ !ucount)
                return -EINVAL;
+       if (ucookies && !ucount)
+               return -EINVAL;
 
        kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link);
        info->kprobe_multi.count = kmulti_link->cnt;
@@ -2699,6 +2702,18 @@ static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link,
        else
                ucount = kmulti_link->cnt;
 
+       if (ucookies) {
+               if (kmulti_link->cookies) {
+                       if (copy_to_user(ucookies, kmulti_link->cookies, ucount * sizeof(u64)))
+                               return -EFAULT;
+               } else {
+                       for (i = 0; i < ucount; i++) {
+                               if (put_user(0, ucookies + i))
+                                       return -EFAULT;
+                       }
+               }
+       }
+
        if (kallsyms_show_value(current_cred())) {
                if (copy_to_user(uaddrs, kmulti_link->addrs, ucount * sizeof(u64)))
                        return -EFAULT;
tools/include/uapi/linux/bpf.h +1 -1
index b823d367a83c..199cb93dca7f 100644
@@ -6563,6 +6563,7 @@ struct bpf_link_info {
                        __u32 count; /* in/out: kprobe_multi function count */
                        __u32 flags;
                        __u64 missed;
+                       __aligned_u64 cookies;
                } kprobe_multi;
                struct {
                        __aligned_u64 path;
--

2.43.0

Signature · 2 lines
Regards
Yafang
author reviewer
#6 lore raw

PATCH 3/8 bpftool: Fix wrong free call in do_show_link

In reply to Jiri Olsa

The error path frees wrong array, it should be ref_ctr_offsets.

Fixes a7795698f8b6 ("bpftool: Add support to display uprobe_multi links")
---
 tools/bpf/bpftool/link.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/bpf/bpftool/link.c +1 -1
index cb46667a6b2e..35b6859dd7c3 100644
@@ -977,7 +977,7 @@ static int do_show_link(int fd)
 			cookies = calloc(count, sizeof(__u64));
 			if (!cookies) {
 				p_err("mem alloc failed");
-				free(cookies);
+				free(ref_ctr_offsets);
 				free(offsets);
 				close(fd);
 				return -ENOMEM;
Signature · 1 line
2.43.0
Acked-by on v1 3/8
#7 lore raw

Re: [PATCH bpf-next 3/8] bpftool: Fix wrong free call in do_show_link

Quoted 5 lines · Jiri Olsa

The error path frees wrong array, it should be ref_ctr_offsets.

Fixes a7795698f8b6 ("bpftool: Add support to display uprobe_multi links")
Quoted 20 lines
---
 tools/bpf/bpftool/link.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/bpf/bpftool/link.c +1 -2
index cb46667a6b2e..35b6859dd7c3 100644
@@ -977,7 +977,7 @@ static int do_show_link(int fd)
                        cookies = calloc(count, sizeof(__u64));
                        if (!cookies) {
                                p_err("mem alloc failed");
-                               free(cookies);
+                               free(ref_ctr_offsets);
                                free(offsets);
                                close(fd);
                                return -ENOMEM;
--

2.43.0

Signature · 2 lines
Regards
Yafang
Reviewed-by
#8 lore raw

Re: [PATCH bpf-next 3/8] bpftool: Fix wrong free call in do_show_link

2024-01-18 12:30 UTC+0000 ~ Yafang Shao <[email protected]>

Quoted 8 lines · Jiri Olsa
Quoted 5 lines

The error path frees wrong array, it should be ref_ctr_offsets.

Fixes a7795698f8b6 ("bpftool: Add support to display uprobe_multi links")
author reviewer
#9 lore raw

PATCH 4/8 selftests/bpf: Add cookies check for kprobe_multi fill_link_info test

In reply to Jiri Olsa

Adding cookies check for kprobe_multi fill_link_info test, plus tests for invalid values related to cookies.

---
 .../selftests/bpf/prog_tests/fill_link_info.c | 48 ++++++++++++++-----
 1 file changed, 36 insertions(+), 12 deletions(-)
tools/testing/selftests/bpf/prog_tests/fill_link_info.c +36 -12
index d4b1901f7879..2c2a02010c0b 100644
@@ -19,6 +19,7 @@ static const char *kmulti_syms[] = {
 };
 #define KMULTI_CNT ARRAY_SIZE(kmulti_syms)
 static __u64 kmulti_addrs[KMULTI_CNT];
+static __u64 kmulti_cookies[] = { 3, 1, 2 };
 
 #define KPROBE_FUNC "bpf_fentry_test1"
 static __u64 kprobe_addr;
@@ -195,11 +196,11 @@ static void test_uprobe_fill_link_info(struct test_fill_link_info *skel,
 	bpf_link__destroy(link);
 }
 
-static int verify_kmulti_link_info(int fd, bool retprobe)
+static int verify_kmulti_link_info(int fd, bool retprobe, bool has_cookies)
 {
+	__u64 addrs[KMULTI_CNT], cookies[KMULTI_CNT];
 	struct bpf_link_info info;
 	__u32 len = sizeof(info);
-	__u64 addrs[KMULTI_CNT];
 	int flags, i, err;
 
 	memset(&info, 0, sizeof(info));
@@ -221,18 +222,22 @@ static int verify_kmulti_link_info(int fd, bool retprobe)
 
 	if (!info.kprobe_multi.addrs) {
 		info.kprobe_multi.addrs = ptr_to_u64(addrs);
+		info.kprobe_multi.cookies = ptr_to_u64(cookies);
 		goto again;
 	}
-	for (i = 0; i < KMULTI_CNT; i++)
+	for (i = 0; i < KMULTI_CNT; i++) {
 		ASSERT_EQ(addrs[i], kmulti_addrs[i], "kmulti_addrs");
+		ASSERT_EQ(cookies[i], has_cookies ? kmulti_cookies[i] : 0,
+			  "kmulti_cookies_value");
+	}
 	return 0;
 }
 
 static void verify_kmulti_invalid_user_buffer(int fd)
 {
+	__u64 addrs[KMULTI_CNT], cookies[KMULTI_CNT];
 	struct bpf_link_info info;
 	__u32 len = sizeof(info);
-	__u64 addrs[KMULTI_CNT];
 	int err, i;
 
 	memset(&info, 0, sizeof(info));
@@ -266,7 +271,20 @@ static void verify_kmulti_invalid_user_buffer(int fd)
 	info.kprobe_multi.count = KMULTI_CNT;
 	info.kprobe_multi.addrs = 0x1; /* invalid addr */
 	err = bpf_link_get_info_by_fd(fd, &info, &len);
-	ASSERT_EQ(err, -EFAULT, "invalid_buff");
+	ASSERT_EQ(err, -EFAULT, "invalid_buff_addrs");
+
+	info.kprobe_multi.count = KMULTI_CNT;
+	info.kprobe_multi.addrs = ptr_to_u64(addrs);
+	info.kprobe_multi.cookies = 0x1; /* invalid addr */
+	err = bpf_link_get_info_by_fd(fd, &info, &len);
+	ASSERT_EQ(err, -EFAULT, "invalid_buff_cookies");
+
+	/* cookies && !count */
+	info.kprobe_multi.count = 0;
+	info.kprobe_multi.addrs = ptr_to_u64(NULL);
+	info.kprobe_multi.cookies = ptr_to_u64(cookies);
+	err = bpf_link_get_info_by_fd(fd, &info, &len);
+	ASSERT_EQ(err, -EINVAL, "invalid_cookies_count");
 }
 
 static int symbols_cmp_r(const void *a, const void *b)
@@ -278,13 +296,15 @@ static int symbols_cmp_r(const void *a, const void *b)
 }
 
 static void test_kprobe_multi_fill_link_info(struct test_fill_link_info *skel,
-					     bool retprobe, bool invalid)
+					     bool retprobe, bool cookies,
+					     bool invalid)
 {
 	LIBBPF_OPTS(bpf_kprobe_multi_opts, opts);
 	struct bpf_link *link;
 	int link_fd, err;
 
 	opts.syms = kmulti_syms;
+	opts.cookies = cookies ? kmulti_cookies : NULL;
 	opts.cnt = KMULTI_CNT;
 	opts.retprobe = retprobe;
 	link = bpf_program__attach_kprobe_multi_opts(skel->progs.kmulti_run, NULL, &opts);
@@ -293,7 +313,7 @@ static void test_kprobe_multi_fill_link_info(struct test_fill_link_info *skel,
 
 	link_fd = bpf_link__fd(link);
 	if (!invalid) {
-		err = verify_kmulti_link_info(link_fd, retprobe);
+		err = verify_kmulti_link_info(link_fd, retprobe, cookies);
 		ASSERT_OK(err, "verify_kmulti_link_info");
 	} else {
 		verify_kmulti_invalid_user_buffer(link_fd);
@@ -523,12 +543,16 @@ void test_fill_link_info(void)
 	qsort(kmulti_syms, KMULTI_CNT, sizeof(kmulti_syms[0]), symbols_cmp_r);
 	for (i = 0; i < KMULTI_CNT; i++)
 		kmulti_addrs[i] = ksym_get_addr(kmulti_syms[i]);
-	if (test__start_subtest("kprobe_multi_link_info"))
-		test_kprobe_multi_fill_link_info(skel, false, false);
-	if (test__start_subtest("kretprobe_multi_link_info"))
-		test_kprobe_multi_fill_link_info(skel, true, false);
+	if (test__start_subtest("kprobe_multi_link_info")) {
+		test_kprobe_multi_fill_link_info(skel, false, false, false);
+		test_kprobe_multi_fill_link_info(skel, false, true, false);
+	}
+	if (test__start_subtest("kretprobe_multi_link_info")) {
+		test_kprobe_multi_fill_link_info(skel, true, false, false);
+		test_kprobe_multi_fill_link_info(skel, true, true, false);
+	}
 	if (test__start_subtest("kprobe_multi_invalid_ubuff"))
-		test_kprobe_multi_fill_link_info(skel, true, true);
+		test_kprobe_multi_fill_link_info(skel, true, true, true);
 
 	if (test__start_subtest("uprobe_multi_link_info"))
 		test_uprobe_multi_fill_link_info(skel, false, false);
Signature · 1 line
2.43.0
author reviewer
#10 lore raw

PATCH 5/8 selftests/bpf: Add cookies check for perf_event fill_link_info test

In reply to Jiri Olsa

Now that we get cookies for perf_event probes, adding tests for cookie for kprobe/uprobe/tracepoint.

The perf_event test needs to be added completely and is coming in following change.

---
 .../selftests/bpf/prog_tests/fill_link_info.c | 26 +++++++++++++++----
 1 file changed, 21 insertions(+), 5 deletions(-)
tools/testing/selftests/bpf/prog_tests/fill_link_info.c +21 -5
index 2c2a02010c0b..20e105001bc3 100644
@@ -32,6 +32,8 @@ static noinline void uprobe_func(void)
 	asm volatile ("");
 }
 
+#define PERF_EVENT_COOKIE 0xdeadbeef
+
 static int verify_perf_link_info(int fd, enum bpf_perf_event_type type, long addr,
 				 ssize_t offset, ssize_t entry_offset)
 {
@@ -63,6 +65,8 @@ static int verify_perf_link_info(int fd, enum bpf_perf_event_type type, long add
 			ASSERT_EQ(info.perf_event.kprobe.addr, addr + entry_offset,
 				  "kprobe_addr");
 
+		ASSERT_EQ(info.perf_event.kprobe.cookie, PERF_EVENT_COOKIE, "kprobe_cookie");
+
 		if (!info.perf_event.kprobe.func_name) {
 			ASSERT_EQ(info.perf_event.kprobe.name_len, 0, "name_len");
 			info.perf_event.kprobe.func_name = ptr_to_u64(&buf);
@@ -82,6 +86,8 @@ static int verify_perf_link_info(int fd, enum bpf_perf_event_type type, long add
 			goto again;
 		}
 
+		ASSERT_EQ(info.perf_event.tracepoint.cookie, PERF_EVENT_COOKIE, "tracepoint_cookie");
+
 		err = strncmp(u64_to_ptr(info.perf_event.tracepoint.tp_name), TP_NAME,
 			      strlen(TP_NAME));
 		ASSERT_EQ(err, 0, "cmp_tp_name");
@@ -97,6 +103,8 @@ static int verify_perf_link_info(int fd, enum bpf_perf_event_type type, long add
 			goto again;
 		}
 
+		ASSERT_EQ(info.perf_event.uprobe.cookie, PERF_EVENT_COOKIE, "uprobe_cookie");
+
 		err = strncmp(u64_to_ptr(info.perf_event.uprobe.file_name), UPROBE_FILE,
 			      strlen(UPROBE_FILE));
 			ASSERT_EQ(err, 0, "cmp_file_name");
@@ -140,6 +148,7 @@ static void test_kprobe_fill_link_info(struct test_fill_link_info *skel,
 	DECLARE_LIBBPF_OPTS(bpf_kprobe_opts, opts,
 		.attach_mode = PROBE_ATTACH_MODE_LINK,
 		.retprobe = type == BPF_PERF_EVENT_KRETPROBE,
+		.bpf_cookie = PERF_EVENT_COOKIE,
 	);
 	ssize_t entry_offset = 0;
 	struct bpf_link *link;
@@ -164,10 +173,13 @@ static void test_kprobe_fill_link_info(struct test_fill_link_info *skel,
 
 static void test_tp_fill_link_info(struct test_fill_link_info *skel)
 {
+	DECLARE_LIBBPF_OPTS(bpf_tracepoint_opts, opts,
+		.bpf_cookie = PERF_EVENT_COOKIE,
+	);
 	struct bpf_link *link;
 	int link_fd, err;
 
-	link = bpf_program__attach_tracepoint(skel->progs.tp_run, TP_CAT, TP_NAME);
+	link = bpf_program__attach_tracepoint_opts(skel->progs.tp_run, TP_CAT, TP_NAME, &opts);
 	if (!ASSERT_OK_PTR(link, "attach_tp"))
 		return;
 
@@ -180,13 +192,17 @@ static void test_tp_fill_link_info(struct test_fill_link_info *skel)
 static void test_uprobe_fill_link_info(struct test_fill_link_info *skel,
 				       enum bpf_perf_event_type type)
 {
+	DECLARE_LIBBPF_OPTS(bpf_uprobe_opts, opts,
+		.retprobe = type == BPF_PERF_EVENT_URETPROBE,
+		.bpf_cookie = PERF_EVENT_COOKIE,
+	);
 	struct bpf_link *link;
 	int link_fd, err;
 
-	link = bpf_program__attach_uprobe(skel->progs.uprobe_run,
-					  type == BPF_PERF_EVENT_URETPROBE,
-					  0, /* self pid */
-					  UPROBE_FILE, uprobe_offset);
+	link = bpf_program__attach_uprobe_opts(skel->progs.uprobe_run,
+					       0, /* self pid */
+					       UPROBE_FILE, uprobe_offset,
+					       &opts);
 	if (!ASSERT_OK_PTR(link, "attach_uprobe"))
 		return;
 
Signature · 1 line
2.43.0
author reviewer
#11 lore raw

PATCH 6/8 selftests/bpf: Add fill_link_info test for perf event

In reply to Jiri Olsa

Adding fill_link_info test for perf event and testing we get its values back through the bpf_link_info interface.

---
 .../selftests/bpf/prog_tests/fill_link_info.c | 40 +++++++++++++++++++
 .../selftests/bpf/progs/test_fill_link_info.c |  6 +++
 2 files changed, 46 insertions(+)
tools/testing/selftests/bpf/prog_tests/fill_link_info.c +40 -0
index 20e105001bc3..f3932941bbaa 100644
@@ -109,6 +109,11 @@ static int verify_perf_link_info(int fd, enum bpf_perf_event_type type, long add
 			      strlen(UPROBE_FILE));
 			ASSERT_EQ(err, 0, "cmp_file_name");
 		break;
+	case BPF_PERF_EVENT_EVENT:
+		ASSERT_EQ(info.perf_event.event.type, PERF_TYPE_SOFTWARE, "event_type");
+		ASSERT_EQ(info.perf_event.event.config, PERF_COUNT_SW_PAGE_FAULTS, "event_config");
+		ASSERT_EQ(info.perf_event.event.cookie, PERF_EVENT_COOKIE, "event_cookie");
+		break;
 	default:
 		err = -1;
 		break;
@@ -189,6 +194,39 @@ static void test_tp_fill_link_info(struct test_fill_link_info *skel)
 	bpf_link__destroy(link);
 }
 
+static void test_event_fill_link_info(struct test_fill_link_info *skel)
+{
+	DECLARE_LIBBPF_OPTS(bpf_perf_event_opts, opts,
+		.bpf_cookie = PERF_EVENT_COOKIE,
+	);
+	struct bpf_link *link;
+	int link_fd, err, pfd;
+	struct perf_event_attr attr = {
+		.type = PERF_TYPE_SOFTWARE,
+		.config = PERF_COUNT_SW_PAGE_FAULTS,
+		.freq = 1,
+		.sample_freq = 1,
+		.size = sizeof(struct perf_event_attr),
+	};
+
+	pfd = syscall(__NR_perf_event_open, &attr, -1 /* pid */, 0 /* cpu 0 */,
+		      -1 /* group id */, 0 /* flags */);
+	if (!ASSERT_GE(pfd, 0, "perf_event_open"))
+		return;
+
+	link = bpf_program__attach_perf_event_opts(skel->progs.event_run, pfd, &opts);
+	if (!ASSERT_OK_PTR(link, "attach_event"))
+		goto error;
+
+	link_fd = bpf_link__fd(link);
+	err = verify_perf_link_info(link_fd, BPF_PERF_EVENT_EVENT, 0, 0, 0);
+	ASSERT_OK(err, "verify_perf_link_info");
+	bpf_link__destroy(link);
+
+error:
+	close(pfd);
+}
+
 static void test_uprobe_fill_link_info(struct test_fill_link_info *skel,
 				       enum bpf_perf_event_type type)
 {
@@ -549,6 +587,8 @@ void test_fill_link_info(void)
 		test_kprobe_fill_link_info(skel, BPF_PERF_EVENT_KPROBE, true);
 	if (test__start_subtest("tracepoint_link_info"))
 		test_tp_fill_link_info(skel);
+	if (test__start_subtest("event_link_info"))
+		test_event_fill_link_info(skel);
 
 	uprobe_offset = get_uprobe_offset(&uprobe_func);
 	if (test__start_subtest("uprobe_link_info"))
tools/testing/selftests/bpf/progs/test_fill_link_info.c +6 -0
index 69509f8bb680..6afa834756e9 100644
@@ -33,6 +33,12 @@ int BPF_PROG(tp_run)
 	return 0;
 }
 
+SEC("perf_event")
+int event_run(void *ctx)
+{
+	return 0;
+}
+
 SEC("kprobe.multi")
 int BPF_PROG(kmulti_run)
 {
Signature · 1 line
2.43.0
author reviewer
#12 lore raw

PATCH 7/8 bpftool: Display cookie for perf event link probes

In reply to Jiri Olsa

Displaying cookie for perf event link probes, in plain mode:

  # bpftool link
  17: perf_event  prog 90
          kprobe ffffffff82b1c2b0 bpf_fentry_test1  cookie 3735928559
  18: perf_event  prog 90
          kretprobe ffffffff82b1c2b0 bpf_fentry_test1  cookie 3735928559
  20: perf_event  prog 92
          tracepoint sched_switch  cookie 3735928559
  21: perf_event  prog 93
          event software:page-faults  cookie 3735928559
  22: perf_event  prog 91
          uprobe /proc/self/exe+0xd703c  cookie 3735928559

And in json mode:

  # bpftool link -j | jq
  {
    "id": 30,
    "type": "perf_event",
    "prog_id": 160,
    "retprobe": false,
    "addr": 18446744071607272112,
    "func": "bpf_fentry_test1",
    "offset": 0,
    "missed": 0,
    "cookie": 3735928559
  }

  {
    "id": 33,
    "type": "perf_event",
    "prog_id": 162,
    "tracepoint": "sched_switch",
    "cookie": 3735928559
  }

  {
    "id": 34,
    "type": "perf_event",
    "prog_id": 163,
    "event_type": "software",
    "event_config": "page-faults",
    "cookie": 3735928559
  }

  {
    "id": 35,
    "type": "perf_event",
    "prog_id": 161,
    "retprobe": false,
    "file": "/proc/self/exe",
    "offset": 880700,
    "cookie": 3735928559
  }
---
 tools/bpf/bpftool/link.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
tools/bpf/bpftool/link.c +14 -0
index 35b6859dd7c3..b66a1598b87c 100644
@@ -334,6 +334,7 @@ show_perf_event_kprobe_json(struct bpf_link_info *info, json_writer_t *wtr)
 			   u64_to_ptr(info->perf_event.kprobe.func_name));
 	jsonw_uint_field(wtr, "offset", info->perf_event.kprobe.offset);
 	jsonw_uint_field(wtr, "missed", info->perf_event.kprobe.missed);
+	jsonw_uint_field(wtr, "cookie", info->perf_event.kprobe.cookie);
 }
 
 static void
@@ -343,6 +344,7 @@ show_perf_event_uprobe_json(struct bpf_link_info *info, json_writer_t *wtr)
 	jsonw_string_field(wtr, "file",
 			   u64_to_ptr(info->perf_event.uprobe.file_name));
 	jsonw_uint_field(wtr, "offset", info->perf_event.uprobe.offset);
+	jsonw_uint_field(wtr, "cookie", info->perf_event.uprobe.cookie);
 }
 
 static void
@@ -350,6 +352,7 @@ show_perf_event_tracepoint_json(struct bpf_link_info *info, json_writer_t *wtr)
 {
 	jsonw_string_field(wtr, "tracepoint",
 			   u64_to_ptr(info->perf_event.tracepoint.tp_name));
+	jsonw_uint_field(wtr, "cookie", info->perf_event.tracepoint.cookie);
 }
 
 static char *perf_config_hw_cache_str(__u64 config)
@@ -426,6 +429,8 @@ show_perf_event_event_json(struct bpf_link_info *info, json_writer_t *wtr)
 	else
 		jsonw_uint_field(wtr, "event_config", config);
 
+	jsonw_uint_field(wtr, "cookie", info->perf_event.event.cookie);
+
 	if (type == PERF_TYPE_HW_CACHE && perf_config)
 		free((void *)perf_config);
 }
@@ -754,6 +759,8 @@ static void show_perf_event_kprobe_plain(struct bpf_link_info *info)
 		printf("+%#x", info->perf_event.kprobe.offset);
 	if (info->perf_event.kprobe.missed)
 		printf("  missed %llu", info->perf_event.kprobe.missed);
+	if (info->perf_event.kprobe.cookie)
+		printf("  cookie %llu", info->perf_event.kprobe.cookie);
 	printf("  ");
 }
 
@@ -770,6 +777,8 @@ static void show_perf_event_uprobe_plain(struct bpf_link_info *info)
 	else
 		printf("\n\tuprobe ");
 	printf("%s+%#x  ", buf, info->perf_event.uprobe.offset);
+	if (info->perf_event.uprobe.cookie)
+		printf("cookie %llu  ", info->perf_event.uprobe.cookie);
 }
 
 static void show_perf_event_tracepoint_plain(struct bpf_link_info *info)
@@ -781,6 +790,8 @@ static void show_perf_event_tracepoint_plain(struct bpf_link_info *info)
 		return;
 
 	printf("\n\ttracepoint %s  ", buf);
+	if (info->perf_event.tracepoint.cookie)
+		printf("cookie %llu  ", info->perf_event.tracepoint.cookie);
 }
 
 static void show_perf_event_event_plain(struct bpf_link_info *info)
@@ -802,6 +813,9 @@ static void show_perf_event_event_plain(struct bpf_link_info *info)
 	else
 		printf("%llu  ", config);
 
+	if (info->perf_event.event.cookie)
+		printf("cookie %llu  ", info->perf_event.event.cookie);
+
 	if (type == PERF_TYPE_HW_CACHE && perf_config)
 		free((void *)perf_config);
 }
Signature · 1 line
2.43.0
Reviewed-by on v1 7/8
#13 lore raw

Re: [PATCH bpf-next 7/8] bpftool: Display cookie for perf event link probes

2024-01-18 09:55 UTC+0000 ~ Jiri Olsa <[email protected]>

Quoted 57 lines

Displaying cookie for perf event link probes, in plain mode:

  # bpftool link
  17: perf_event  prog 90
          kprobe ffffffff82b1c2b0 bpf_fentry_test1  cookie 3735928559
  18: perf_event  prog 90
          kretprobe ffffffff82b1c2b0 bpf_fentry_test1  cookie 3735928559
  20: perf_event  prog 92
          tracepoint sched_switch  cookie 3735928559
  21: perf_event  prog 93
          event software:page-faults  cookie 3735928559
  22: perf_event  prog 91
          uprobe /proc/self/exe+0xd703c  cookie 3735928559

And in json mode:

  # bpftool link -j | jq
  {
    "id": 30,
    "type": "perf_event",
    "prog_id": 160,
    "retprobe": false,
    "addr": 18446744071607272112,
    "func": "bpf_fentry_test1",
    "offset": 0,
    "missed": 0,
    "cookie": 3735928559
  }

  {
    "id": 33,
    "type": "perf_event",
    "prog_id": 162,
    "tracepoint": "sched_switch",
    "cookie": 3735928559
  }

  {
    "id": 34,
    "type": "perf_event",
    "prog_id": 163,
    "event_type": "software",
    "event_config": "page-faults",
    "cookie": 3735928559
  }

  {
    "id": 35,
    "type": "perf_event",
    "prog_id": 161,
    "retprobe": false,
    "file": "/proc/self/exe",
    "offset": 880700,
    "cookie": 3735928559
  }
Quoted 1 line

Thanks!

author reviewer
#14 lore raw

PATCH 8/8 bpftool: Display cookie for kprobe multi link

In reply to Jiri Olsa

Displaying cookies for kprobe multi link, in plain mode:

  # bpftool link
  ...
  1397: kprobe_multi  prog 47532
          kretprobe.multi  func_cnt 3
          addr             cookie           func [module]
          ffffffff82b370c0 3                bpf_fentry_test1
          ffffffff82b39780 1                bpf_fentry_test2
          ffffffff82b397a0 2                bpf_fentry_test3

And in json mode:

  # bpftool link -j | jq
  ...
    {
      "id": 1397,
      "type": "kprobe_multi",
      "prog_id": 47532,
      "retprobe": true,
      "func_cnt": 3,
      "missed": 0,
      "funcs": [
        {
          "addr": 18446744071607382208,
          "func": "bpf_fentry_test1",
          "module": null,
          "cookie": 3
        },
        {
          "addr": 18446744071607392128,
          "func": "bpf_fentry_test2",
          "module": null,
          "cookie": 1
        },
        {
          "addr": 18446744071607392160,
          "func": "bpf_fentry_test3",
          "module": null,
          "cookie": 2
        }
      ]
    }

Cookie is attached to specific address, and because we sort addresses before printing, we need to sort cookies the same way, hence adding the struct addr_cookie to keep and sort them together.

---
 tools/bpf/bpftool/link.c | 71 ++++++++++++++++++++++++++++++++--------
 1 file changed, 57 insertions(+), 14 deletions(-)
tools/bpf/bpftool/link.c +57 -14
index b66a1598b87c..fd862afe6c6f 100644
@@ -249,18 +249,44 @@ 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)
+struct addr_cookie {
+	__u64 addr;
+	__u64 cookie;
+};
+
+static int cmp_addr_cookie(const void *A, const void *B)
+{
+	const struct addr_cookie *a = A, *b = B;
+
+	if (a->addr == b->addr)
+		return 0;
+	return a->addr < b->addr ? -1 : 1;
+}
+
+static struct addr_cookie *
+get_addr_cookie_array(__u64 *addrs, __u64 *cookies, __u32 count)
 {
-	const __u64 *a = A, *b = B;
+	struct addr_cookie *data;
+	__u32 i;
 
-	return *a - *b;
+	data = calloc(count, sizeof(data[0]));
+	if (!data) {
+		p_err("mem alloc failed");
+		return NULL;
+	}
+	for (i = 0; i < count; i++) {
+		data[i].addr = addrs[i];
+		data[i].cookie = cookies[i];
+	}
+	qsort(data, count, sizeof(data[0]), cmp_addr_cookie);
+	return data;
 }
 
 static void
 show_kprobe_multi_json(struct bpf_link_info *info, json_writer_t *wtr)
 {
+	struct addr_cookie *data;
 	__u32 i, j = 0;
-	__u64 *addrs;
 
 	jsonw_bool_field(json_wtr, "retprobe",
 			 info->kprobe_multi.flags & BPF_F_KPROBE_MULTI_RETURN);
@@ -268,14 +294,17 @@ show_kprobe_multi_json(struct bpf_link_info *info, json_writer_t *wtr)
 	jsonw_uint_field(json_wtr, "missed", info->kprobe_multi.missed);
 	jsonw_name(json_wtr, "funcs");
 	jsonw_start_array(json_wtr);
-	addrs = u64_to_ptr(info->kprobe_multi.addrs);
-	qsort(addrs, info->kprobe_multi.count, sizeof(addrs[0]), cmp_u64);
+	data = get_addr_cookie_array(u64_to_ptr(info->kprobe_multi.addrs),
+				     u64_to_ptr(info->kprobe_multi.cookies),
+				     info->kprobe_multi.count);
+	if (!data)
+		return;
 
 	/* 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])
+		if (dd.sym_mapping[i].address != data[j].addr)
 			continue;
 		jsonw_start_object(json_wtr);
 		jsonw_uint_field(json_wtr, "addr", dd.sym_mapping[i].address);
@@ -287,11 +316,13 @@ show_kprobe_multi_json(struct bpf_link_info *info, json_writer_t *wtr)
 		} else {
 			jsonw_string_field(json_wtr, "module", dd.sym_mapping[i].module);
 		}
+		jsonw_uint_field(json_wtr, "cookie", data[j].cookie);
 		jsonw_end_object(json_wtr);
 		if (j++ == info->kprobe_multi.count)
 			break;
 	}
 	jsonw_end_array(json_wtr);
+	free(data);
 }
 
 static __u64 *u64_to_arr(__u64 val)
@@ -675,8 +706,8 @@ void netfilter_dump_plain(const struct bpf_link_info *info)
 
 static void show_kprobe_multi_plain(struct bpf_link_info *info)
 {
+	struct addr_cookie *data;
 	__u32 i, j = 0;
-	__u64 *addrs;
 
 	if (!info->kprobe_multi.count)
 		return;
@@ -688,8 +719,11 @@ static void show_kprobe_multi_plain(struct bpf_link_info *info)
 	printf("func_cnt %u  ", info->kprobe_multi.count);
 	if (info->kprobe_multi.missed)
 		printf("missed %llu  ", info->kprobe_multi.missed);
-	addrs = (__u64 *)u64_to_ptr(info->kprobe_multi.addrs);
-	qsort(addrs, info->kprobe_multi.count, sizeof(__u64), cmp_u64);
+	data = get_addr_cookie_array(u64_to_ptr(info->kprobe_multi.addrs),
+				     u64_to_ptr(info->kprobe_multi.cookies),
+				     info->kprobe_multi.count);
+	if (!data)
+		return;
 
 	/* Load it once for all. */
 	if (!dd.sym_count)
@@ -697,12 +731,12 @@ static void show_kprobe_multi_plain(struct bpf_link_info *info)
 	if (!dd.sym_count)
 		return;
 
-	printf("\n\t%-16s %s", "addr", "func [module]");
+	printf("\n\t%-16s %-16s %s", "addr", "cookie", "func [module]");
 	for (i = 0; i < dd.sym_count; i++) {
-		if (dd.sym_mapping[i].address != addrs[j])
+		if (dd.sym_mapping[i].address != data[j].addr)
 			continue;
-		printf("\n\t%016lx %s",
-		       dd.sym_mapping[i].address, dd.sym_mapping[i].name);
+		printf("\n\t%016lx %-16llx %s",
+		       dd.sym_mapping[i].address, data[j].cookie, dd.sym_mapping[i].name);
 		if (dd.sym_mapping[i].module[0] != '\0')
 			printf(" [%s]  ", dd.sym_mapping[i].module);
 		else
@@ -711,6 +745,7 @@ static void show_kprobe_multi_plain(struct bpf_link_info *info)
 		if (j++ == info->kprobe_multi.count)
 			break;
 	}
+	free(data);
 }
 
 static void show_uprobe_multi_plain(struct bpf_link_info *info)
@@ -966,6 +1001,14 @@ static int do_show_link(int fd)
 				return -ENOMEM;
 			}
 			info.kprobe_multi.addrs = ptr_to_u64(addrs);
+			cookies = calloc(count, sizeof(__u64));
+			if (!cookies) {
+				p_err("mem alloc failed");
+				free(addrs);
+				close(fd);
+				return -ENOMEM;
+			}
+			info.kprobe_multi.cookies = ptr_to_u64(cookies);
 			goto again;
 		}
 	}
Signature · 1 line
2.43.0
#15 lore raw

Re: [PATCH bpf-next 8/8] bpftool: Display cookie for kprobe multi link

2024-01-18 09:55 UTC+0000 ~ Jiri Olsa <[email protected]>

Quoted 166 lines

Displaying cookies for kprobe multi link, in plain mode:

  # bpftool link
  ...
  1397: kprobe_multi  prog 47532
          kretprobe.multi  func_cnt 3
          addr             cookie           func [module]
          ffffffff82b370c0 3                bpf_fentry_test1
          ffffffff82b39780 1                bpf_fentry_test2
          ffffffff82b397a0 2                bpf_fentry_test3

And in json mode:

  # bpftool link -j | jq
  ...
    {
      "id": 1397,
      "type": "kprobe_multi",
      "prog_id": 47532,
      "retprobe": true,
      "func_cnt": 3,
      "missed": 0,
      "funcs": [
        {
          "addr": 18446744071607382208,
          "func": "bpf_fentry_test1",
          "module": null,
          "cookie": 3
        },
        {
          "addr": 18446744071607392128,
          "func": "bpf_fentry_test2",
          "module": null,
          "cookie": 1
        },
        {
          "addr": 18446744071607392160,
          "func": "bpf_fentry_test3",
          "module": null,
          "cookie": 2
        }
      ]
    }

Cookie is attached to specific address, and because we sort addresses before printing, we need to sort cookies the same way, hence adding the struct addr_cookie to keep and sort them together.

---
 tools/bpf/bpftool/link.c | 71 ++++++++++++++++++++++++++++++++--------
 1 file changed, 57 insertions(+), 14 deletions(-)
tools/bpf/bpftool/link.c +44 -10
index b66a1598b87c..fd862afe6c6f 100644
@@ -249,18 +249,44 @@ 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)
+struct addr_cookie {
+	__u64 addr;
+	__u64 cookie;
+};
+
+static int cmp_addr_cookie(const void *A, const void *B)
+{
+	const struct addr_cookie *a = A, *b = B;
+
+	if (a->addr == b->addr)
+		return 0;
+	return a->addr < b->addr ? -1 : 1;
+}
+
+static struct addr_cookie *
+get_addr_cookie_array(__u64 *addrs, __u64 *cookies, __u32 count)
 {
-	const __u64 *a = A, *b = B;
+	struct addr_cookie *data;
+	__u32 i;
 
-	return *a - *b;
+	data = calloc(count, sizeof(data[0]));
+	if (!data) {
+		p_err("mem alloc failed");
+		return NULL;
+	}
+	for (i = 0; i < count; i++) {
+		data[i].addr = addrs[i];
+		data[i].cookie = cookies[i];
+	}
+	qsort(data, count, sizeof(data[0]), cmp_addr_cookie);
+	return data;
 }
 
 static void
 show_kprobe_multi_json(struct bpf_link_info *info, json_writer_t *wtr)
 {
+	struct addr_cookie *data;
 	__u32 i, j = 0;
-	__u64 *addrs;
 
 	jsonw_bool_field(json_wtr, "retprobe",
 			 info->kprobe_multi.flags & BPF_F_KPROBE_MULTI_RETURN);
@@ -268,14 +294,17 @@ show_kprobe_multi_json(struct bpf_link_info *info, json_writer_t *wtr)
 	jsonw_uint_field(json_wtr, "missed", info->kprobe_multi.missed);
 	jsonw_name(json_wtr, "funcs");
 	jsonw_start_array(json_wtr);
-	addrs = u64_to_ptr(info->kprobe_multi.addrs);
-	qsort(addrs, info->kprobe_multi.count, sizeof(addrs[0]), cmp_u64);
+	data = get_addr_cookie_array(u64_to_ptr(info->kprobe_multi.addrs),
+				     u64_to_ptr(info->kprobe_multi.cookies),
+				     info->kprobe_multi.count);
+	if (!data)
+		return;
 
 	/* 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])
+		if (dd.sym_mapping[i].address != data[j].addr)
 			continue;
 		jsonw_start_object(json_wtr);
 		jsonw_uint_field(json_wtr, "addr", dd.sym_mapping[i].address);
@@ -287,11 +316,13 @@ show_kprobe_multi_json(struct bpf_link_info *info, json_writer_t *wtr)
 		} else {
 			jsonw_string_field(json_wtr, "module", dd.sym_mapping[i].module);
 		}
+		jsonw_uint_field(json_wtr, "cookie", data[j].cookie);
 		jsonw_end_object(json_wtr);
 		if (j++ == info->kprobe_multi.count)
 			break;
 	}
 	jsonw_end_array(json_wtr);
+	free(data);
 }
 
 static __u64 *u64_to_arr(__u64 val)
@@ -675,8 +706,8 @@ void netfilter_dump_plain(const struct bpf_link_info *info)
 
 static void show_kprobe_multi_plain(struct bpf_link_info *info)
 {
+	struct addr_cookie *data;
 	__u32 i, j = 0;
-	__u64 *addrs;
 
 	if (!info->kprobe_multi.count)
 		return;
@@ -688,8 +719,11 @@ static void show_kprobe_multi_plain(struct bpf_link_info *info)
 	printf("func_cnt %u  ", info->kprobe_multi.count);
 	if (info->kprobe_multi.missed)
 		printf("missed %llu  ", info->kprobe_multi.missed);
-	addrs = (__u64 *)u64_to_ptr(info->kprobe_multi.addrs);
-	qsort(addrs, info->kprobe_multi.count, sizeof(__u64), cmp_u64);
+	data = get_addr_cookie_array(u64_to_ptr(info->kprobe_multi.addrs),
+				     u64_to_ptr(info->kprobe_multi.cookies),
+				     info->kprobe_multi.count);
+	if (!data)
+		return;
 
 	/* Load it once for all. */
 	if (!dd.sym_count)

tools/bpf/bpftool/link.c static void show_kprobe_multi_plain(struct bpf_link_info *info) · line 732

Quoted 2 lines
tools/bpf/bpftool/link.c +0 -0
@@ -697,12 +731,12 @@ static void show_kprobe_multi_plain(struct bpf_link_info *info)
 	if (!dd.sym_count)
 		return;

Don't we need to free(data) if we return here?

Quoted 37 lines
tools/bpf/bpftool/link.c +13 -4
-	printf("\n\t%-16s %s", "addr", "func [module]");
+	printf("\n\t%-16s %-16s %s", "addr", "cookie", "func [module]");
 	for (i = 0; i < dd.sym_count; i++) {
-		if (dd.sym_mapping[i].address != addrs[j])
+		if (dd.sym_mapping[i].address != data[j].addr)
 			continue;
-		printf("\n\t%016lx %s",
-		       dd.sym_mapping[i].address, dd.sym_mapping[i].name);
+		printf("\n\t%016lx %-16llx %s",
+		       dd.sym_mapping[i].address, data[j].cookie, dd.sym_mapping[i].name);
 		if (dd.sym_mapping[i].module[0] != '\0')
 			printf(" [%s]  ", dd.sym_mapping[i].module);
 		else
@@ -711,6 +745,7 @@ static void show_kprobe_multi_plain(struct bpf_link_info *info)
 		if (j++ == info->kprobe_multi.count)
 			break;
 	}
+	free(data);
 }
 
 static void show_uprobe_multi_plain(struct bpf_link_info *info)
@@ -966,6 +1001,14 @@ static int do_show_link(int fd)
 				return -ENOMEM;
 			}
 			info.kprobe_multi.addrs = ptr_to_u64(addrs);
+			cookies = calloc(count, sizeof(__u64));
+			if (!cookies) {
+				p_err("mem alloc failed");
+				free(addrs);
+				close(fd);
+				return -ENOMEM;
+			}
+			info.kprobe_multi.cookies = ptr_to_u64(cookies);
 			goto again;
 		}
 	}
#16 lore raw

Re: [PATCH bpf-next 8/8] bpftool: Display cookie for kprobe multi link

On Thu, Jan 18, 2024 at 05:51:17PM +0000, Quentin Monnet wrote:

SNIP

Quoted 23 lines
Quoted 23 lines

static __u64 *u64_to_arr(__u64 val)

tools/bpf/bpftool/link.c +6 -3
@@ -675,8 +706,8 @@ void netfilter_dump_plain(const struct bpf_link_info *info)
 
 static void show_kprobe_multi_plain(struct bpf_link_info *info)
 {
+	struct addr_cookie *data;
 	__u32 i, j = 0;
-	__u64 *addrs;
 
 	if (!info->kprobe_multi.count)
 		return;
@@ -688,8 +719,11 @@ static void show_kprobe_multi_plain(struct bpf_link_info *info)
 	printf("func_cnt %u  ", info->kprobe_multi.count);
 	if (info->kprobe_multi.missed)
 		printf("missed %llu  ", info->kprobe_multi.missed);
-	addrs = (__u64 *)u64_to_ptr(info->kprobe_multi.addrs);
-	qsort(addrs, info->kprobe_multi.count, sizeof(__u64), cmp_u64);
+	data = get_addr_cookie_array(u64_to_ptr(info->kprobe_multi.addrs),
+				     u64_to_ptr(info->kprobe_multi.cookies),
+				     info->kprobe_multi.count);
+	if (!data)
+		return;
 
Quoted 7 lines

tools/bpf/bpftool/link.c static void show_kprobe_multi_plain(struct bpf_link_info *info) · line 732

Quoted 5 lines
tools/bpf/bpftool/link.c +0 -0
 	/* Load it once for all. */
 	if (!dd.sym_count)
@@ -697,12 +731,12 @@ static void show_kprobe_multi_plain(struct bpf_link_info *info)
 	if (!dd.sym_count)
 		return;

Don't we need to free(data) if we return here?

good catch! I guess I got distracted by show_kprobe_multi_plain being similar to show_kprobe_multi_json, which does not check dd.sym_count and does not return, which it should :-\ I'll include that fix as well

thanks,
jirka

Quoted 39 lines
Quoted 38 lines
tools/bpf/bpftool/link.c +13 -4
-	printf("\n\t%-16s %s", "addr", "func [module]");
+	printf("\n\t%-16s %-16s %s", "addr", "cookie", "func [module]");
 	for (i = 0; i < dd.sym_count; i++) {
-		if (dd.sym_mapping[i].address != addrs[j])
+		if (dd.sym_mapping[i].address != data[j].addr)
 			continue;
-		printf("\n\t%016lx %s",
-		       dd.sym_mapping[i].address, dd.sym_mapping[i].name);
+		printf("\n\t%016lx %-16llx %s",
+		       dd.sym_mapping[i].address, data[j].cookie, dd.sym_mapping[i].name);
 		if (dd.sym_mapping[i].module[0] != '\0')
 			printf(" [%s]  ", dd.sym_mapping[i].module);
 		else
@@ -711,6 +745,7 @@ static void show_kprobe_multi_plain(struct bpf_link_info *info)
 		if (j++ == info->kprobe_multi.count)
 			break;
 	}
+	free(data);
 }
 
 static void show_uprobe_multi_plain(struct bpf_link_info *info)
@@ -966,6 +1001,14 @@ static int do_show_link(int fd)
 				return -ENOMEM;
 			}
 			info.kprobe_multi.addrs = ptr_to_u64(addrs);
+			cookies = calloc(count, sizeof(__u64));
+			if (!cookies) {
+				p_err("mem alloc failed");
+				free(addrs);
+				close(fd);
+				return -ENOMEM;
+			}
+			info.kprobe_multi.cookies = ptr_to_u64(cookies);
 			goto again;
 		}
 	}
#17 lore raw
In reply to Jiri Olsa
Quoted 5 lines · Jiri Olsa

hi,
this patchset adds support to retrieve cookies from existing tracing links that still did not support it plus changes to bpftool to display them. It's leftover we discussed some time ago [1].

Thanks for your work.

bpf cookie is also displayed in the pid_iter [0]. After we add support for the cookie for other progs like kprobe_multi and uprobe_multi, I think we should update this file as well.

[0]. tools/bpf/bpftool/skeleton/pid_iter.bpf.c

Quoted 25 lines

thanks,
jirka

[1] https://lore.kernel.org/bpf/CALOAHbAZ6=A9j3VFCLoAC_WhgQKU7injMf06=cM2sU4Hi4Sx+Q@mail.gmail.com/
---
Jiri Olsa (8):
bpf: Add cookie to perf_event bpf_link_info records
bpf: Store cookies in kprobe_multi bpf_link_info data
bpftool: Fix wrong free call in do_show_link
selftests/bpf: Add cookies check for kprobe_multi fill_link_info test selftests/bpf: Add cookies check for perf_event fill_link_info test selftests/bpf: Add fill_link_info test for perf event
bpftool: Display cookie for perf event link probes
bpftool: Display cookie for kprobe multi link

 include/uapi/linux/bpf.h                                |   5 +++++
 kernel/bpf/syscall.c                                    |   4 ++++
 kernel/trace/bpf_trace.c                                |  15 +++++++++++++
 tools/bpf/bpftool/link.c                                |  87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------
 tools/include/uapi/linux/bpf.h                          |   5 +++++
 tools/testing/selftests/bpf/prog_tests/fill_link_info.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------
 tools/testing/selftests/bpf/progs/test_fill_link_info.c |   6 +++++
 7 files changed, 204 insertions(+), 32 deletions(-)
Signature · 2 lines
Regards
Yafang
#18 lore raw
Quoted 7 lines · Yafang Shao
Quoted 5 lines · Jiri Olsa

hi,
this patchset adds support to retrieve cookies from existing tracing links that still did not support it plus changes to bpftool to display them. It's leftover we discussed some time ago [1].

Quoted 5 lines

Thanks for your work.

bpf cookie is also displayed in the pid_iter [0]. After we add support for the cookie for other progs like kprobe_multi and uprobe_multi, I think we should update this file as well.

ok, I'll check

thanks,
jirka

Quoted 34 lines

[0]. tools/bpf/bpftool/skeleton/pid_iter.bpf.c

Quoted 25 lines

thanks,
jirka

[1] https://lore.kernel.org/bpf/CALOAHbAZ6=A9j3VFCLoAC_WhgQKU7injMf06=cM2sU4Hi4Sx+Q@mail.gmail.com/
---
Jiri Olsa (8):
bpf: Add cookie to perf_event bpf_link_info records
bpf: Store cookies in kprobe_multi bpf_link_info data
bpftool: Fix wrong free call in do_show_link
selftests/bpf: Add cookies check for kprobe_multi fill_link_info test selftests/bpf: Add cookies check for perf_event fill_link_info test selftests/bpf: Add fill_link_info test for perf event
bpftool: Display cookie for perf event link probes
bpftool: Display cookie for kprobe multi link

 include/uapi/linux/bpf.h                                |   5 +++++
 kernel/bpf/syscall.c                                    |   4 ++++
 kernel/trace/bpf_trace.c                                |  15 +++++++++++++
 tools/bpf/bpftool/link.c                                |  87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------
 tools/include/uapi/linux/bpf.h                          |   5 +++++
 tools/testing/selftests/bpf/prog_tests/fill_link_info.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------
 tools/testing/selftests/bpf/progs/test_fill_link_info.c |   6 +++++
 7 files changed, 204 insertions(+), 32 deletions(-)
Signature · 2 lines
Regards
Yafang