All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link
@ 2022-03-16 12:24 Jiri Olsa
  2022-03-16 12:24 ` [PATCHv3 bpf-next 01/13] lib/sort: Add priv pointer to swap function Jiri Olsa
                   ` (14 more replies)
  0 siblings, 15 replies; 26+ messages in thread
From: Jiri Olsa @ 2022-03-16 12:24 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Masami Hiramatsu
  Cc: netdev, bpf, lkml, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Steven Rostedt

hi,
this patchset adds new link type BPF_TRACE_KPROBE_MULTI that attaches
kprobe program through fprobe API [1] instroduced by Masami.

The fprobe API allows to attach probe on multiple functions at once very
fast, because it works on top of ftrace. On the other hand this limits
the probe point to the function entry or return.


With bpftrace support I see following attach speed:

  # perf stat --null -r 5 ./src/bpftrace -e 'kprobe:x* { } i:ms:1 { exit(); } '
  Attaching 2 probes...
  Attaching 3342 functions
  ...

  1.4960 +- 0.0285 seconds time elapsed  ( +-  1.91% )

v3 changes:
  - based on latest fprobe post from Masami [2]
  - add acks
  - add extra comment to kprobe_multi_link_handler wrt entry ip setup [Masami]
  - keep swap_words_64 static and swap values directly in
    bpf_kprobe_multi_cookie_swap [Andrii]
  - rearrange locking/migrate setup in kprobe_multi_link_prog_run [Andrii]
  - move uapi fields [Andrii]
  - add bpf_program__attach_kprobe_multi_opts function [Andrii]
  - many small test changes [Andrii]
  - added tests for bpf_program__attach_kprobe_multi_opts
  - make kallsyms_lookup_name check for empty string [Andrii]

v2 changes:
  - based on latest fprobe changes [1]
  - renaming the uapi interface to kprobe multi
  - adding support for sort_r to pass user pointer for swap functions
    and using that in cookie support to keep just single functions array
  - moving new link to kernel/trace/bpf_trace.c file
  - using single fprobe callback function for entry and exit
  - using kvzalloc, libbpf_ensure_mem functions
  - adding new k[ret]probe.multi sections instead of using current kprobe
  - used glob_match from test_progs.c, added '?' matching
  - move bpf_get_func_ip verifier inline change to seprate change
  - couple of other minor fixes


Also available at:
  https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
  bpf/kprobe_multi

thanks,
jirka


[1] https://lore.kernel.org/bpf/164458044634.586276.3261555265565111183.stgit@devnote2/
[2] https://lore.kernel.org/bpf/164735281449.1084943.12438881786173547153.stgit@devnote2/
---
Jiri Olsa (13):
      lib/sort: Add priv pointer to swap function
      kallsyms: Skip the name search for empty string
      bpf: Add multi kprobe link
      bpf: Add bpf_get_func_ip kprobe helper for multi kprobe link
      bpf: Add support to inline bpf_get_func_ip helper on x86
      bpf: Add cookie support to programs attached with kprobe multi link
      libbpf: Add libbpf_kallsyms_parse function
      libbpf: Add bpf_link_create support for multi kprobes
      libbpf: Add bpf_program__attach_kprobe_multi_opts function
      selftests/bpf: Add kprobe_multi attach test
      selftests/bpf: Add kprobe_multi bpf_cookie test
      selftests/bpf: Add attach test for bpf_program__attach_kprobe_multi_opts
      selftests/bpf: Add cookie test for bpf_program__attach_kprobe_multi_opts

 include/linux/bpf_types.h                                  |   1 +
 include/linux/sort.h                                       |   2 +-
 include/linux/trace_events.h                               |   7 ++
 include/linux/types.h                                      |   1 +
 include/uapi/linux/bpf.h                                   |  14 ++++
 kernel/bpf/syscall.c                                       |  26 +++++--
 kernel/bpf/verifier.c                                      |  21 +++++-
 kernel/kallsyms.c                                          |   4 ++
 kernel/trace/bpf_trace.c                                   | 342 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 lib/sort.c                                                 |  40 ++++++++---
 tools/include/uapi/linux/bpf.h                             |  14 ++++
 tools/lib/bpf/bpf.c                                        |   9 +++
 tools/lib/bpf/bpf.h                                        |   9 ++-
 tools/lib/bpf/libbpf.c                                     | 222 +++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 tools/lib/bpf/libbpf.h                                     |  23 +++++++
 tools/lib/bpf/libbpf.map                                   |   1 +
 tools/lib/bpf/libbpf_internal.h                            |   5 ++
 tools/testing/selftests/bpf/prog_tests/bpf_cookie.c        | 177 ++++++++++++++++++++++++++++++++++++++++++++++++
 tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c | 323 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/testing/selftests/bpf/progs/kprobe_multi.c           |  98 +++++++++++++++++++++++++++
 tools/testing/selftests/bpf/trace_helpers.c                |   7 ++
 21 files changed, 1302 insertions(+), 44 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
 create mode 100644 tools/testing/selftests/bpf/progs/kprobe_multi.c

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCHv3 bpf-next 01/13] lib/sort: Add priv pointer to swap function
  2022-03-16 12:24 [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link Jiri Olsa
@ 2022-03-16 12:24 ` Jiri Olsa
  2022-03-16 12:24 ` [PATCHv3 bpf-next 02/13] kallsyms: Skip the name search for empty string Jiri Olsa
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: Jiri Olsa @ 2022-03-16 12:24 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Masami Hiramatsu
  Cc: Rasmus Villemoes, netdev, bpf, lkml, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Steven Rostedt

Adding support to have priv pointer in swap callback function.

Following the initial change on cmp callback functions [1]
and adding SWAP_WRAPPER macro to identify sort call of sort_r.

Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
[1] 4333fb96ca10 ("media: lib/sort.c: implement sort() variant taking context argument")
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 include/linux/sort.h  |  2 +-
 include/linux/types.h |  1 +
 lib/sort.c            | 40 ++++++++++++++++++++++++++++++----------
 3 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/include/linux/sort.h b/include/linux/sort.h
index b5898725fe9d..e163287ac6c1 100644
--- a/include/linux/sort.h
+++ b/include/linux/sort.h
@@ -6,7 +6,7 @@
 
 void sort_r(void *base, size_t num, size_t size,
 	    cmp_r_func_t cmp_func,
-	    swap_func_t swap_func,
+	    swap_r_func_t swap_func,
 	    const void *priv);
 
 void sort(void *base, size_t num, size_t size,
diff --git a/include/linux/types.h b/include/linux/types.h
index ac825ad90e44..ea8cf60a8a79 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -226,6 +226,7 @@ struct callback_head {
 typedef void (*rcu_callback_t)(struct rcu_head *head);
 typedef void (*call_rcu_func_t)(struct rcu_head *head, rcu_callback_t func);
 
+typedef void (*swap_r_func_t)(void *a, void *b, int size, const void *priv);
 typedef void (*swap_func_t)(void *a, void *b, int size);
 
 typedef int (*cmp_r_func_t)(const void *a, const void *b, const void *priv);
diff --git a/lib/sort.c b/lib/sort.c
index aa18153864d2..b399bf10d675 100644
--- a/lib/sort.c
+++ b/lib/sort.c
@@ -122,16 +122,27 @@ static void swap_bytes(void *a, void *b, size_t n)
  * a pointer, but small integers make for the smallest compare
  * instructions.
  */
-#define SWAP_WORDS_64 (swap_func_t)0
-#define SWAP_WORDS_32 (swap_func_t)1
-#define SWAP_BYTES    (swap_func_t)2
+#define SWAP_WORDS_64 (swap_r_func_t)0
+#define SWAP_WORDS_32 (swap_r_func_t)1
+#define SWAP_BYTES    (swap_r_func_t)2
+#define SWAP_WRAPPER  (swap_r_func_t)3
+
+struct wrapper {
+	cmp_func_t cmp;
+	swap_func_t swap;
+};
 
 /*
  * The function pointer is last to make tail calls most efficient if the
  * compiler decides not to inline this function.
  */
-static void do_swap(void *a, void *b, size_t size, swap_func_t swap_func)
+static void do_swap(void *a, void *b, size_t size, swap_r_func_t swap_func, const void *priv)
 {
+	if (swap_func == SWAP_WRAPPER) {
+		((const struct wrapper *)priv)->swap(a, b, (int)size);
+		return;
+	}
+
 	if (swap_func == SWAP_WORDS_64)
 		swap_words_64(a, b, size);
 	else if (swap_func == SWAP_WORDS_32)
@@ -139,7 +150,7 @@ static void do_swap(void *a, void *b, size_t size, swap_func_t swap_func)
 	else if (swap_func == SWAP_BYTES)
 		swap_bytes(a, b, size);
 	else
-		swap_func(a, b, (int)size);
+		swap_func(a, b, (int)size, priv);
 }
 
 #define _CMP_WRAPPER ((cmp_r_func_t)0L)
@@ -147,7 +158,7 @@ static void do_swap(void *a, void *b, size_t size, swap_func_t swap_func)
 static int do_cmp(const void *a, const void *b, cmp_r_func_t cmp, const void *priv)
 {
 	if (cmp == _CMP_WRAPPER)
-		return ((cmp_func_t)(priv))(a, b);
+		return ((const struct wrapper *)priv)->cmp(a, b);
 	return cmp(a, b, priv);
 }
 
@@ -198,7 +209,7 @@ static size_t parent(size_t i, unsigned int lsbit, size_t size)
  */
 void sort_r(void *base, size_t num, size_t size,
 	    cmp_r_func_t cmp_func,
-	    swap_func_t swap_func,
+	    swap_r_func_t swap_func,
 	    const void *priv)
 {
 	/* pre-scale counters for performance */
@@ -208,6 +219,10 @@ void sort_r(void *base, size_t num, size_t size,
 	if (!a)		/* num < 2 || size == 0 */
 		return;
 
+	/* called from 'sort' without swap function, let's pick the default */
+	if (swap_func == SWAP_WRAPPER && !((struct wrapper *)priv)->swap)
+		swap_func = NULL;
+
 	if (!swap_func) {
 		if (is_aligned(base, size, 8))
 			swap_func = SWAP_WORDS_64;
@@ -230,7 +245,7 @@ void sort_r(void *base, size_t num, size_t size,
 		if (a)			/* Building heap: sift down --a */
 			a -= size;
 		else if (n -= size)	/* Sorting: Extract root to --n */
-			do_swap(base, base + n, size, swap_func);
+			do_swap(base, base + n, size, swap_func, priv);
 		else			/* Sort complete */
 			break;
 
@@ -257,7 +272,7 @@ void sort_r(void *base, size_t num, size_t size,
 		c = b;			/* Where "a" belongs */
 		while (b != a) {	/* Shift it into place */
 			b = parent(b, lsbit, size);
-			do_swap(base + b, base + c, size, swap_func);
+			do_swap(base + b, base + c, size, swap_func, priv);
 		}
 	}
 }
@@ -267,6 +282,11 @@ void sort(void *base, size_t num, size_t size,
 	  cmp_func_t cmp_func,
 	  swap_func_t swap_func)
 {
-	return sort_r(base, num, size, _CMP_WRAPPER, swap_func, cmp_func);
+	struct wrapper w = {
+		.cmp  = cmp_func,
+		.swap = swap_func,
+	};
+
+	return sort_r(base, num, size, _CMP_WRAPPER, SWAP_WRAPPER, &w);
 }
 EXPORT_SYMBOL(sort);
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCHv3 bpf-next 02/13] kallsyms: Skip the name search for empty string
  2022-03-16 12:24 [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link Jiri Olsa
  2022-03-16 12:24 ` [PATCHv3 bpf-next 01/13] lib/sort: Add priv pointer to swap function Jiri Olsa
@ 2022-03-16 12:24 ` Jiri Olsa
  2022-03-16 12:24 ` [PATCHv3 bpf-next 03/13] bpf: Add multi kprobe link Jiri Olsa
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: Jiri Olsa @ 2022-03-16 12:24 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Masami Hiramatsu
  Cc: netdev, bpf, lkml, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Steven Rostedt

When kallsyms_lookup_name is called with empty string,
it will do futile search for it through all the symbols.

Skipping the search for empty string.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/kallsyms.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 951c93216fc4..79f2eb617a62 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -212,6 +212,10 @@ unsigned long kallsyms_lookup_name(const char *name)
 	unsigned long i;
 	unsigned int off;
 
+	/* Skip the search for empty string. */
+	if (!*name)
+		return 0;
+
 	for (i = 0, off = 0; i < kallsyms_num_syms; i++) {
 		off = kallsyms_expand_symbol(off, namebuf, ARRAY_SIZE(namebuf));
 
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCHv3 bpf-next 03/13] bpf: Add multi kprobe link
  2022-03-16 12:24 [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link Jiri Olsa
  2022-03-16 12:24 ` [PATCHv3 bpf-next 01/13] lib/sort: Add priv pointer to swap function Jiri Olsa
  2022-03-16 12:24 ` [PATCHv3 bpf-next 02/13] kallsyms: Skip the name search for empty string Jiri Olsa
@ 2022-03-16 12:24 ` Jiri Olsa
  2022-03-16 18:53   ` Alexei Starovoitov
  2022-03-16 12:24 ` [PATCHv3 bpf-next 04/13] bpf: Add bpf_get_func_ip kprobe helper for " Jiri Olsa
                   ` (11 subsequent siblings)
  14 siblings, 1 reply; 26+ messages in thread
From: Jiri Olsa @ 2022-03-16 12:24 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Masami Hiramatsu
  Cc: netdev, bpf, lkml, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Steven Rostedt

Adding new link type BPF_LINK_TYPE_KPROBE_MULTI that attaches kprobe
program through fprobe API.

The fprobe API allows to attach probe on multiple functions at once
very fast, because it works on top of ftrace. On the other hand this
limits the probe point to the function entry or return.

The kprobe program gets the same pt_regs input ctx as when it's attached
through the perf API.

Adding new attach type BPF_TRACE_KPROBE_MULTI that allows attachment
kprobe to multiple function with new link.

User provides array of addresses or symbols with count to attach the
kprobe program to. The new link_create uapi interface looks like:

  struct {
          __u32           flags;
          __u32           cnt;
          __aligned_u64   syms;
          __aligned_u64   addrs;
  } kprobe_multi;

The flags field allows single BPF_TRACE_KPROBE_MULTI bit to create
return multi kprobe.

Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 include/linux/bpf_types.h      |   1 +
 include/linux/trace_events.h   |   7 ++
 include/uapi/linux/bpf.h       |  13 ++
 kernel/bpf/syscall.c           |  26 +++-
 kernel/trace/bpf_trace.c       | 211 +++++++++++++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h |  13 ++
 6 files changed, 266 insertions(+), 5 deletions(-)

diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index 48a91c51c015..3e24ad0c4b3c 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -140,3 +140,4 @@ BPF_LINK_TYPE(BPF_LINK_TYPE_XDP, xdp)
 #ifdef CONFIG_PERF_EVENTS
 BPF_LINK_TYPE(BPF_LINK_TYPE_PERF_EVENT, perf)
 #endif
+BPF_LINK_TYPE(BPF_LINK_TYPE_KPROBE_MULTI, kprobe_multi)
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index dcea51fb60e2..8f0e9e7cb493 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -15,6 +15,7 @@ struct array_buffer;
 struct tracer;
 struct dentry;
 struct bpf_prog;
+union bpf_attr;
 
 const char *trace_print_flags_seq(struct trace_seq *p, const char *delim,
 				  unsigned long flags,
@@ -738,6 +739,7 @@ void bpf_put_raw_tracepoint(struct bpf_raw_event_map *btp);
 int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
 			    u32 *fd_type, const char **buf,
 			    u64 *probe_offset, u64 *probe_addr);
+int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog);
 #else
 static inline unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx)
 {
@@ -779,6 +781,11 @@ static inline int bpf_get_perf_event_info(const struct perf_event *event,
 {
 	return -EOPNOTSUPP;
 }
+static inline int
+bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
+{
+	return -EOPNOTSUPP;
+}
 #endif
 
 enum {
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 99fab54ae9c0..d77f47af7752 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -997,6 +997,7 @@ enum bpf_attach_type {
 	BPF_SK_REUSEPORT_SELECT,
 	BPF_SK_REUSEPORT_SELECT_OR_MIGRATE,
 	BPF_PERF_EVENT,
+	BPF_TRACE_KPROBE_MULTI,
 	__MAX_BPF_ATTACH_TYPE
 };
 
@@ -1011,6 +1012,7 @@ enum bpf_link_type {
 	BPF_LINK_TYPE_NETNS = 5,
 	BPF_LINK_TYPE_XDP = 6,
 	BPF_LINK_TYPE_PERF_EVENT = 7,
+	BPF_LINK_TYPE_KPROBE_MULTI = 8,
 
 	MAX_BPF_LINK_TYPE,
 };
@@ -1118,6 +1120,11 @@ enum bpf_link_type {
  */
 #define BPF_F_XDP_HAS_FRAGS	(1U << 5)
 
+/* link_create.kprobe_multi.flags used in LINK_CREATE command for
+ * BPF_TRACE_KPROBE_MULTI attach type to create return probe.
+ */
+#define BPF_F_KPROBE_MULTI_RETURN	(1U << 0)
+
 /* When BPF ldimm64's insn[0].src_reg != 0 then this can have
  * the following extensions:
  *
@@ -1475,6 +1482,12 @@ union bpf_attr {
 				 */
 				__u64		bpf_cookie;
 			} perf_event;
+			struct {
+				__u32		flags;
+				__u32		cnt;
+				__aligned_u64	syms;
+				__aligned_u64	addrs;
+			} kprobe_multi;
 		};
 	} link_create;
 
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 9beb585be5a6..b8bb67ee6c57 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -32,6 +32,7 @@
 #include <linux/bpf-netns.h>
 #include <linux/rcupdate_trace.h>
 #include <linux/memcontrol.h>
+#include <linux/trace_events.h>
 
 #define IS_FD_ARRAY(map) ((map)->map_type == BPF_MAP_TYPE_PERF_EVENT_ARRAY || \
 			  (map)->map_type == BPF_MAP_TYPE_CGROUP_ARRAY || \
@@ -3022,6 +3023,11 @@ static int bpf_perf_link_attach(const union bpf_attr *attr, struct bpf_prog *pro
 	fput(perf_file);
 	return err;
 }
+#else
+static int bpf_perf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
+{
+	return -EOPNOTSUPP;
+}
 #endif /* CONFIG_PERF_EVENTS */
 
 #define BPF_RAW_TRACEPOINT_OPEN_LAST_FIELD raw_tracepoint.prog_fd
@@ -4255,7 +4261,7 @@ static int tracing_bpf_link_attach(const union bpf_attr *attr, bpfptr_t uattr,
 	return -EINVAL;
 }
 
-#define BPF_LINK_CREATE_LAST_FIELD link_create.iter_info_len
+#define BPF_LINK_CREATE_LAST_FIELD link_create.kprobe_multi.addrs
 static int link_create(union bpf_attr *attr, bpfptr_t uattr)
 {
 	enum bpf_prog_type ptype;
@@ -4279,7 +4285,6 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
 		ret = tracing_bpf_link_attach(attr, uattr, prog);
 		goto out;
 	case BPF_PROG_TYPE_PERF_EVENT:
-	case BPF_PROG_TYPE_KPROBE:
 	case BPF_PROG_TYPE_TRACEPOINT:
 		if (attr->link_create.attach_type != BPF_PERF_EVENT) {
 			ret = -EINVAL;
@@ -4287,6 +4292,14 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
 		}
 		ptype = prog->type;
 		break;
+	case BPF_PROG_TYPE_KPROBE:
+		if (attr->link_create.attach_type != BPF_PERF_EVENT &&
+		    attr->link_create.attach_type != BPF_TRACE_KPROBE_MULTI) {
+			ret = -EINVAL;
+			goto out;
+		}
+		ptype = prog->type;
+		break;
 	default:
 		ptype = attach_type_to_prog_type(attr->link_create.attach_type);
 		if (ptype == BPF_PROG_TYPE_UNSPEC || ptype != prog->type) {
@@ -4318,13 +4331,16 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
 		ret = bpf_xdp_link_attach(attr, prog);
 		break;
 #endif
-#ifdef CONFIG_PERF_EVENTS
 	case BPF_PROG_TYPE_PERF_EVENT:
 	case BPF_PROG_TYPE_TRACEPOINT:
-	case BPF_PROG_TYPE_KPROBE:
 		ret = bpf_perf_link_attach(attr, prog);
 		break;
-#endif
+	case BPF_PROG_TYPE_KPROBE:
+		if (attr->link_create.attach_type == BPF_PERF_EVENT)
+			ret = bpf_perf_link_attach(attr, prog);
+		else
+			ret = bpf_kprobe_multi_link_attach(attr, prog);
+		break;
 	default:
 		ret = -EINVAL;
 	}
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index a2024ba32a20..fffa2171fae4 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -17,6 +17,7 @@
 #include <linux/error-injection.h>
 #include <linux/btf_ids.h>
 #include <linux/bpf_lsm.h>
+#include <linux/fprobe.h>
 
 #include <net/bpf_sk_storage.h>
 
@@ -2181,3 +2182,213 @@ static int __init bpf_event_init(void)
 
 fs_initcall(bpf_event_init);
 #endif /* CONFIG_MODULES */
+
+#ifdef CONFIG_FPROBE
+struct bpf_kprobe_multi_link {
+	struct bpf_link link;
+	struct fprobe fp;
+	unsigned long *addrs;
+};
+
+static void bpf_kprobe_multi_link_release(struct bpf_link *link)
+{
+	struct bpf_kprobe_multi_link *kmulti_link;
+
+	kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link);
+	unregister_fprobe(&kmulti_link->fp);
+}
+
+static void bpf_kprobe_multi_link_dealloc(struct bpf_link *link)
+{
+	struct bpf_kprobe_multi_link *kmulti_link;
+
+	kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link);
+	kvfree(kmulti_link->addrs);
+	kfree(kmulti_link);
+}
+
+static const struct bpf_link_ops bpf_kprobe_multi_link_lops = {
+	.release = bpf_kprobe_multi_link_release,
+	.dealloc = bpf_kprobe_multi_link_dealloc,
+};
+
+static int
+kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
+			   struct pt_regs *regs)
+{
+	int err;
+
+	if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) {
+		err = 0;
+		goto out;
+	}
+
+	migrate_disable();
+	rcu_read_lock();
+	err = bpf_prog_run(link->link.prog, regs);
+	rcu_read_unlock();
+	migrate_enable();
+
+ out:
+	__this_cpu_dec(bpf_prog_active);
+	return err;
+}
+
+static void
+kprobe_multi_link_handler(struct fprobe *fp, unsigned long entry_ip,
+			  struct pt_regs *regs)
+{
+	unsigned long saved_ip = instruction_pointer(regs);
+	struct bpf_kprobe_multi_link *link;
+
+	/*
+	 * Because fprobe's regs->ip is set to the next instruction of
+	 * dynamic-ftrace instruction, correct entry ip must be set, so
+	 * that the bpf program can access entry address via regs as same
+	 * as kprobes.
+	 *
+	 * Both kprobe and kretprobe see the entry ip of traced function
+	 * as instruction pointer.
+	 */
+	instruction_pointer_set(regs, entry_ip);
+
+	link = container_of(fp, struct bpf_kprobe_multi_link, fp);
+	kprobe_multi_link_prog_run(link, regs);
+
+	instruction_pointer_set(regs, saved_ip);
+}
+
+static int
+kprobe_multi_resolve_syms(const void *usyms, u32 cnt,
+			  unsigned long *addrs)
+{
+	unsigned long addr, size;
+	const char **syms;
+	int err = -ENOMEM;
+	unsigned int i;
+	char *func;
+
+	size = cnt * sizeof(*syms);
+	syms = kvzalloc(size, GFP_KERNEL);
+	if (!syms)
+		return -ENOMEM;
+
+	func = kmalloc(KSYM_NAME_LEN, GFP_KERNEL);
+	if (!func)
+		goto error;
+
+	if (copy_from_user(syms, usyms, size)) {
+		err = -EFAULT;
+		goto error;
+	}
+
+	for (i = 0; i < cnt; i++) {
+		err = strncpy_from_user(func, syms[i], KSYM_NAME_LEN);
+		if (err == KSYM_NAME_LEN)
+			err = -E2BIG;
+		if (err < 0)
+			goto error;
+		err = -EINVAL;
+		addr = kallsyms_lookup_name(func);
+		if (!addr)
+			goto error;
+		if (!kallsyms_lookup_size_offset(addr, &size, NULL))
+			goto error;
+		addr = ftrace_location_range(addr, addr + size - 1);
+		if (!addr)
+			goto error;
+		addrs[i] = addr;
+	}
+
+	err = 0;
+error:
+	kvfree(syms);
+	kfree(func);
+	return err;
+}
+
+int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
+{
+	struct bpf_kprobe_multi_link *link = NULL;
+	struct bpf_link_primer link_primer;
+	unsigned long *addrs;
+	u32 flags, cnt, size;
+	void __user *uaddrs;
+	void __user *usyms;
+	int err;
+
+	/* no support for 32bit archs yet */
+	if (sizeof(u64) != sizeof(void *))
+		return -EOPNOTSUPP;
+
+	if (prog->expected_attach_type != BPF_TRACE_KPROBE_MULTI)
+		return -EINVAL;
+
+	flags = attr->link_create.kprobe_multi.flags;
+	if (flags & ~BPF_F_KPROBE_MULTI_RETURN)
+		return -EINVAL;
+
+	uaddrs = u64_to_user_ptr(attr->link_create.kprobe_multi.addrs);
+	usyms = u64_to_user_ptr(attr->link_create.kprobe_multi.syms);
+	if (!!uaddrs == !!usyms)
+		return -EINVAL;
+
+	cnt = attr->link_create.kprobe_multi.cnt;
+	if (!cnt)
+		return -EINVAL;
+
+	size = cnt * sizeof(*addrs);
+	addrs = kvmalloc(size, GFP_KERNEL);
+	if (!addrs)
+		return -ENOMEM;
+
+	if (uaddrs) {
+		if (copy_from_user(addrs, uaddrs, size)) {
+			err = -EFAULT;
+			goto error;
+		}
+	} else {
+		err = kprobe_multi_resolve_syms(usyms, cnt, addrs);
+		if (err)
+			goto error;
+	}
+
+	link = kzalloc(sizeof(*link), GFP_KERNEL);
+	if (!link) {
+		err = -ENOMEM;
+		goto error;
+	}
+
+	bpf_link_init(&link->link, BPF_LINK_TYPE_KPROBE_MULTI,
+		      &bpf_kprobe_multi_link_lops, prog);
+
+	err = bpf_link_prime(&link->link, &link_primer);
+	if (err)
+		goto error;
+
+	if (flags & BPF_F_KPROBE_MULTI_RETURN)
+		link->fp.exit_handler = kprobe_multi_link_handler;
+	else
+		link->fp.entry_handler = kprobe_multi_link_handler;
+
+	link->addrs = addrs;
+
+	err = register_fprobe_ips(&link->fp, addrs, cnt);
+	if (err) {
+		bpf_link_cleanup(&link_primer);
+		return err;
+	}
+
+	return bpf_link_settle(&link_primer);
+
+error:
+	kfree(link);
+	kvfree(addrs);
+	return err;
+}
+#else /* !CONFIG_FPROBE */
+int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
+{
+	return -EOPNOTSUPP;
+}
+#endif
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 99fab54ae9c0..d77f47af7752 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -997,6 +997,7 @@ enum bpf_attach_type {
 	BPF_SK_REUSEPORT_SELECT,
 	BPF_SK_REUSEPORT_SELECT_OR_MIGRATE,
 	BPF_PERF_EVENT,
+	BPF_TRACE_KPROBE_MULTI,
 	__MAX_BPF_ATTACH_TYPE
 };
 
@@ -1011,6 +1012,7 @@ enum bpf_link_type {
 	BPF_LINK_TYPE_NETNS = 5,
 	BPF_LINK_TYPE_XDP = 6,
 	BPF_LINK_TYPE_PERF_EVENT = 7,
+	BPF_LINK_TYPE_KPROBE_MULTI = 8,
 
 	MAX_BPF_LINK_TYPE,
 };
@@ -1118,6 +1120,11 @@ enum bpf_link_type {
  */
 #define BPF_F_XDP_HAS_FRAGS	(1U << 5)
 
+/* link_create.kprobe_multi.flags used in LINK_CREATE command for
+ * BPF_TRACE_KPROBE_MULTI attach type to create return probe.
+ */
+#define BPF_F_KPROBE_MULTI_RETURN	(1U << 0)
+
 /* When BPF ldimm64's insn[0].src_reg != 0 then this can have
  * the following extensions:
  *
@@ -1475,6 +1482,12 @@ union bpf_attr {
 				 */
 				__u64		bpf_cookie;
 			} perf_event;
+			struct {
+				__u32		flags;
+				__u32		cnt;
+				__aligned_u64	syms;
+				__aligned_u64	addrs;
+			} kprobe_multi;
 		};
 	} link_create;
 
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCHv3 bpf-next 04/13] bpf: Add bpf_get_func_ip kprobe helper for multi kprobe link
  2022-03-16 12:24 [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link Jiri Olsa
                   ` (2 preceding siblings ...)
  2022-03-16 12:24 ` [PATCHv3 bpf-next 03/13] bpf: Add multi kprobe link Jiri Olsa
@ 2022-03-16 12:24 ` Jiri Olsa
  2022-03-16 12:24 ` [PATCHv3 bpf-next 05/13] bpf: Add support to inline bpf_get_func_ip helper on x86 Jiri Olsa
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: Jiri Olsa @ 2022-03-16 12:24 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Masami Hiramatsu
  Cc: netdev, bpf, lkml, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Steven Rostedt

Adding support to call bpf_get_func_ip helper from kprobe
programs attached by multi kprobe link.

Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/trace/bpf_trace.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index fffa2171fae4..250750932228 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1037,6 +1037,18 @@ static const struct bpf_func_proto bpf_get_func_ip_proto_kprobe = {
 	.arg1_type	= ARG_PTR_TO_CTX,
 };
 
+BPF_CALL_1(bpf_get_func_ip_kprobe_multi, struct pt_regs *, regs)
+{
+	return instruction_pointer(regs);
+}
+
+static const struct bpf_func_proto bpf_get_func_ip_proto_kprobe_multi = {
+	.func		= bpf_get_func_ip_kprobe_multi,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_CTX,
+};
+
 BPF_CALL_1(bpf_get_attach_cookie_trace, void *, ctx)
 {
 	struct bpf_trace_run_ctx *run_ctx;
@@ -1280,7 +1292,9 @@ kprobe_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_override_return_proto;
 #endif
 	case BPF_FUNC_get_func_ip:
-		return &bpf_get_func_ip_proto_kprobe;
+		return prog->expected_attach_type == BPF_TRACE_KPROBE_MULTI ?
+			&bpf_get_func_ip_proto_kprobe_multi :
+			&bpf_get_func_ip_proto_kprobe;
 	case BPF_FUNC_get_attach_cookie:
 		return &bpf_get_attach_cookie_proto_trace;
 	default:
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCHv3 bpf-next 05/13] bpf: Add support to inline bpf_get_func_ip helper on x86
  2022-03-16 12:24 [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link Jiri Olsa
                   ` (3 preceding siblings ...)
  2022-03-16 12:24 ` [PATCHv3 bpf-next 04/13] bpf: Add bpf_get_func_ip kprobe helper for " Jiri Olsa
@ 2022-03-16 12:24 ` Jiri Olsa
  2022-03-16 12:24 ` [PATCHv3 bpf-next 06/13] bpf: Add cookie support to programs attached with kprobe multi link Jiri Olsa
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: Jiri Olsa @ 2022-03-16 12:24 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Masami Hiramatsu
  Cc: netdev, bpf, lkml, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Steven Rostedt

Adding support to inline it on x86, because it's single
load instruction.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/bpf/verifier.c    | 21 ++++++++++++++++++++-
 kernel/trace/bpf_trace.c |  1 +
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 0db6cd8dcb35..c40f1aa07979 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13677,7 +13677,7 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
 			continue;
 		}
 
-		/* Implement bpf_get_func_ip inline. */
+		/* Implement tracing bpf_get_func_ip inline. */
 		if (prog_type == BPF_PROG_TYPE_TRACING &&
 		    insn->imm == BPF_FUNC_get_func_ip) {
 			/* Load IP address from ctx - 16 */
@@ -13692,6 +13692,25 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
 			continue;
 		}
 
+#ifdef CONFIG_X86
+		/* Implement kprobe_multi bpf_get_func_ip inline. */
+		if (prog_type == BPF_PROG_TYPE_KPROBE &&
+		    eatype == BPF_TRACE_KPROBE_MULTI &&
+		    insn->imm == BPF_FUNC_get_func_ip) {
+			/* Load IP address from ctx (struct pt_regs) ip */
+			insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1,
+						  offsetof(struct pt_regs, ip));
+
+			new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, 1);
+			if (!new_prog)
+				return -ENOMEM;
+
+			env->prog = prog = new_prog;
+			insn      = new_prog->insnsi + i + delta;
+			continue;
+		}
+#endif
+
 patch_call_imm:
 		fn = env->ops->get_func_proto(insn->imm, env->prog);
 		/* all functions that have prototype and verifier allowed
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 250750932228..0e7f8c9bc756 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1039,6 +1039,7 @@ static const struct bpf_func_proto bpf_get_func_ip_proto_kprobe = {
 
 BPF_CALL_1(bpf_get_func_ip_kprobe_multi, struct pt_regs *, regs)
 {
+	/* This helper call is inlined by verifier on x86. */
 	return instruction_pointer(regs);
 }
 
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCHv3 bpf-next 06/13] bpf: Add cookie support to programs attached with kprobe multi link
  2022-03-16 12:24 [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link Jiri Olsa
                   ` (4 preceding siblings ...)
  2022-03-16 12:24 ` [PATCHv3 bpf-next 05/13] bpf: Add support to inline bpf_get_func_ip helper on x86 Jiri Olsa
@ 2022-03-16 12:24 ` Jiri Olsa
  2022-03-16 12:24 ` [PATCHv3 bpf-next 07/13] libbpf: Add libbpf_kallsyms_parse function Jiri Olsa
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: Jiri Olsa @ 2022-03-16 12:24 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Masami Hiramatsu
  Cc: netdev, bpf, lkml, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Steven Rostedt

Adding support to call bpf_get_attach_cookie helper from
kprobe programs attached with kprobe multi link.

The cookie is provided by array of u64 values, where each
value is paired with provided function address or symbol
with the same array index.

When cookie array is provided it's sorted together with
addresses (check bpf_kprobe_multi_cookie_swap). This way
we can find cookie based on the address in
bpf_get_attach_cookie helper.

Suggested-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 include/uapi/linux/bpf.h       |   1 +
 kernel/bpf/syscall.c           |   2 +-
 kernel/trace/bpf_trace.c       | 114 ++++++++++++++++++++++++++++++++-
 tools/include/uapi/linux/bpf.h |   1 +
 4 files changed, 116 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index d77f47af7752..7604e7d5438f 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1487,6 +1487,7 @@ union bpf_attr {
 				__u32		cnt;
 				__aligned_u64	syms;
 				__aligned_u64	addrs;
+				__aligned_u64	cookies;
 			} kprobe_multi;
 		};
 	} link_create;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index b8bb67ee6c57..cdaa1152436a 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -4261,7 +4261,7 @@ static int tracing_bpf_link_attach(const union bpf_attr *attr, bpfptr_t uattr,
 	return -EINVAL;
 }
 
-#define BPF_LINK_CREATE_LAST_FIELD link_create.kprobe_multi.addrs
+#define BPF_LINK_CREATE_LAST_FIELD link_create.kprobe_multi.cookies
 static int link_create(union bpf_attr *attr, bpfptr_t uattr)
 {
 	enum bpf_prog_type ptype;
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 0e7f8c9bc756..9a7b6be655e4 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -18,6 +18,8 @@
 #include <linux/btf_ids.h>
 #include <linux/bpf_lsm.h>
 #include <linux/fprobe.h>
+#include <linux/bsearch.h>
+#include <linux/sort.h>
 
 #include <net/bpf_sk_storage.h>
 
@@ -78,6 +80,7 @@ u64 bpf_get_stack(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
 static int bpf_btf_printf_prepare(struct btf_ptr *ptr, u32 btf_ptr_size,
 				  u64 flags, const struct btf **btf,
 				  s32 *btf_id);
+static u64 bpf_kprobe_multi_cookie(struct bpf_run_ctx *ctx, u64 ip);
 
 /**
  * trace_call_bpf - invoke BPF program
@@ -1050,6 +1053,18 @@ static const struct bpf_func_proto bpf_get_func_ip_proto_kprobe_multi = {
 	.arg1_type	= ARG_PTR_TO_CTX,
 };
 
+BPF_CALL_1(bpf_get_attach_cookie_kprobe_multi, struct pt_regs *, regs)
+{
+	return bpf_kprobe_multi_cookie(current->bpf_ctx, instruction_pointer(regs));
+}
+
+static const struct bpf_func_proto bpf_get_attach_cookie_proto_kmulti = {
+	.func		= bpf_get_attach_cookie_kprobe_multi,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_CTX,
+};
+
 BPF_CALL_1(bpf_get_attach_cookie_trace, void *, ctx)
 {
 	struct bpf_trace_run_ctx *run_ctx;
@@ -1297,7 +1312,9 @@ kprobe_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 			&bpf_get_func_ip_proto_kprobe_multi :
 			&bpf_get_func_ip_proto_kprobe;
 	case BPF_FUNC_get_attach_cookie:
-		return &bpf_get_attach_cookie_proto_trace;
+		return prog->expected_attach_type == BPF_TRACE_KPROBE_MULTI ?
+			&bpf_get_attach_cookie_proto_kmulti :
+			&bpf_get_attach_cookie_proto_trace;
 	default:
 		return bpf_tracing_func_proto(func_id, prog);
 	}
@@ -2203,6 +2220,13 @@ struct bpf_kprobe_multi_link {
 	struct bpf_link link;
 	struct fprobe fp;
 	unsigned long *addrs;
+	/*
+	 * The run_ctx here is used to get struct bpf_kprobe_multi_link in
+	 * get_attach_cookie helper, so it can't be used to store data.
+	 */
+	struct bpf_run_ctx run_ctx;
+	u64 *cookies;
+	u32 cnt;
 };
 
 static void bpf_kprobe_multi_link_release(struct bpf_link *link)
@@ -2219,6 +2243,7 @@ static void bpf_kprobe_multi_link_dealloc(struct bpf_link *link)
 
 	kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link);
 	kvfree(kmulti_link->addrs);
+	kvfree(kmulti_link->cookies);
 	kfree(kmulti_link);
 }
 
@@ -2227,10 +2252,60 @@ static const struct bpf_link_ops bpf_kprobe_multi_link_lops = {
 	.dealloc = bpf_kprobe_multi_link_dealloc,
 };
 
+static void bpf_kprobe_multi_cookie_swap(void *a, void *b, int size, const void *priv)
+{
+	const struct bpf_kprobe_multi_link *link = priv;
+	unsigned long *addr_a = a, *addr_b = b;
+	u64 *cookie_a, *cookie_b;
+	unsigned long tmp1;
+	u64 tmp2;
+
+	cookie_a = link->cookies + (addr_a - link->addrs);
+	cookie_b = link->cookies + (addr_b - link->addrs);
+
+	/* swap addr_a/addr_b and cookie_a/cookie_b values */
+	tmp1 = *addr_a; *addr_a = *addr_b; *addr_b = tmp1;
+	tmp2 = *cookie_a; *cookie_a = *cookie_b; *cookie_b = tmp2;
+}
+
+static int __bpf_kprobe_multi_cookie_cmp(const void *a, const void *b)
+{
+	const unsigned long *addr_a = a, *addr_b = b;
+
+	if (*addr_a == *addr_b)
+		return 0;
+	return *addr_a < *addr_b ? -1 : 1;
+}
+
+static int bpf_kprobe_multi_cookie_cmp(const void *a, const void *b, const void *priv)
+{
+	return __bpf_kprobe_multi_cookie_cmp(a, b);
+}
+
+static u64 bpf_kprobe_multi_cookie(struct bpf_run_ctx *ctx, u64 ip)
+{
+	struct bpf_kprobe_multi_link *link;
+	unsigned long *addr;
+	u64 *cookie;
+
+	if (WARN_ON_ONCE(!ctx))
+		return 0;
+	link = container_of(ctx, struct bpf_kprobe_multi_link, run_ctx);
+	if (!link->cookies)
+		return 0;
+	addr = bsearch(&ip, link->addrs, link->cnt, sizeof(ip),
+		       __bpf_kprobe_multi_cookie_cmp);
+	if (!addr)
+		return 0;
+	cookie = link->cookies + (addr - link->addrs);
+	return *cookie;
+}
+
 static int
 kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
 			   struct pt_regs *regs)
 {
+	struct bpf_run_ctx *old_run_ctx;
 	int err;
 
 	if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) {
@@ -2240,7 +2315,9 @@ kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
 
 	migrate_disable();
 	rcu_read_lock();
+	old_run_ctx = bpf_set_run_ctx(&link->run_ctx);
 	err = bpf_prog_run(link->link.prog, regs);
+	bpf_reset_run_ctx(old_run_ctx);
 	rcu_read_unlock();
 	migrate_enable();
 
@@ -2326,9 +2403,11 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 {
 	struct bpf_kprobe_multi_link *link = NULL;
 	struct bpf_link_primer link_primer;
+	void __user *ucookies;
 	unsigned long *addrs;
 	u32 flags, cnt, size;
 	void __user *uaddrs;
+	u64 *cookies = NULL;
 	void __user *usyms;
 	int err;
 
@@ -2368,6 +2447,19 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 			goto error;
 	}
 
+	ucookies = u64_to_user_ptr(attr->link_create.kprobe_multi.cookies);
+	if (ucookies) {
+		cookies = kvmalloc(size, GFP_KERNEL);
+		if (!cookies) {
+			err = -ENOMEM;
+			goto error;
+		}
+		if (copy_from_user(cookies, ucookies, size)) {
+			err = -EFAULT;
+			goto error;
+		}
+	}
+
 	link = kzalloc(sizeof(*link), GFP_KERNEL);
 	if (!link) {
 		err = -ENOMEM;
@@ -2387,6 +2479,21 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 		link->fp.entry_handler = kprobe_multi_link_handler;
 
 	link->addrs = addrs;
+	link->cookies = cookies;
+	link->cnt = cnt;
+
+	if (cookies) {
+		/*
+		 * Sorting addresses will trigger sorting cookies as well
+		 * (check bpf_kprobe_multi_cookie_swap). This way we can
+		 * find cookie based on the address in bpf_get_attach_cookie
+		 * helper.
+		 */
+		sort_r(addrs, cnt, sizeof(*addrs),
+		       bpf_kprobe_multi_cookie_cmp,
+		       bpf_kprobe_multi_cookie_swap,
+		       link);
+	}
 
 	err = register_fprobe_ips(&link->fp, addrs, cnt);
 	if (err) {
@@ -2399,6 +2506,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 error:
 	kfree(link);
 	kvfree(addrs);
+	kvfree(cookies);
 	return err;
 }
 #else /* !CONFIG_FPROBE */
@@ -2406,4 +2514,8 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 {
 	return -EOPNOTSUPP;
 }
+static u64 bpf_kprobe_multi_cookie(struct bpf_run_ctx *ctx, u64 ip)
+{
+	return 0;
+}
 #endif
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index d77f47af7752..7604e7d5438f 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1487,6 +1487,7 @@ union bpf_attr {
 				__u32		cnt;
 				__aligned_u64	syms;
 				__aligned_u64	addrs;
+				__aligned_u64	cookies;
 			} kprobe_multi;
 		};
 	} link_create;
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCHv3 bpf-next 07/13] libbpf: Add libbpf_kallsyms_parse function
  2022-03-16 12:24 [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link Jiri Olsa
                   ` (5 preceding siblings ...)
  2022-03-16 12:24 ` [PATCHv3 bpf-next 06/13] bpf: Add cookie support to programs attached with kprobe multi link Jiri Olsa
@ 2022-03-16 12:24 ` Jiri Olsa
  2022-03-16 12:24 ` [PATCHv3 bpf-next 08/13] libbpf: Add bpf_link_create support for multi kprobes Jiri Olsa
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: Jiri Olsa @ 2022-03-16 12:24 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Masami Hiramatsu
  Cc: netdev, bpf, lkml, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Steven Rostedt

Move the kallsyms parsing in internal libbpf_kallsyms_parse
function, so it can be used from other places.

It will be used in following changes.

Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/bpf/libbpf.c          | 62 ++++++++++++++++++++-------------
 tools/lib/bpf/libbpf_internal.h |  5 +++
 2 files changed, 43 insertions(+), 24 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 43161fdd44bb..1ca520a29fdb 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -7171,12 +7171,10 @@ static int bpf_object__sanitize_maps(struct bpf_object *obj)
 	return 0;
 }
 
-static int bpf_object__read_kallsyms_file(struct bpf_object *obj)
+int libbpf_kallsyms_parse(kallsyms_cb_t cb, void *ctx)
 {
 	char sym_type, sym_name[500];
 	unsigned long long sym_addr;
-	const struct btf_type *t;
-	struct extern_desc *ext;
 	int ret, err = 0;
 	FILE *f;
 
@@ -7195,35 +7193,51 @@ static int bpf_object__read_kallsyms_file(struct bpf_object *obj)
 		if (ret != 3) {
 			pr_warn("failed to read kallsyms entry: %d\n", ret);
 			err = -EINVAL;
-			goto out;
+			break;
 		}
 
-		ext = find_extern_by_name(obj, sym_name);
-		if (!ext || ext->type != EXT_KSYM)
-			continue;
-
-		t = btf__type_by_id(obj->btf, ext->btf_id);
-		if (!btf_is_var(t))
-			continue;
-
-		if (ext->is_set && ext->ksym.addr != sym_addr) {
-			pr_warn("extern (ksym) '%s' resolution is ambiguous: 0x%llx or 0x%llx\n",
-				sym_name, ext->ksym.addr, sym_addr);
-			err = -EINVAL;
-			goto out;
-		}
-		if (!ext->is_set) {
-			ext->is_set = true;
-			ext->ksym.addr = sym_addr;
-			pr_debug("extern (ksym) %s=0x%llx\n", sym_name, sym_addr);
-		}
+		err = cb(sym_addr, sym_type, sym_name, ctx);
+		if (err)
+			break;
 	}
 
-out:
 	fclose(f);
 	return err;
 }
 
+static int kallsyms_cb(unsigned long long sym_addr, char sym_type,
+		       const char *sym_name, void *ctx)
+{
+	struct bpf_object *obj = ctx;
+	const struct btf_type *t;
+	struct extern_desc *ext;
+
+	ext = find_extern_by_name(obj, sym_name);
+	if (!ext || ext->type != EXT_KSYM)
+		return 0;
+
+	t = btf__type_by_id(obj->btf, ext->btf_id);
+	if (!btf_is_var(t))
+		return 0;
+
+	if (ext->is_set && ext->ksym.addr != sym_addr) {
+		pr_warn("extern (ksym) '%s' resolution is ambiguous: 0x%llx or 0x%llx\n",
+			sym_name, ext->ksym.addr, sym_addr);
+		return -EINVAL;
+	}
+	if (!ext->is_set) {
+		ext->is_set = true;
+		ext->ksym.addr = sym_addr;
+		pr_debug("extern (ksym) %s=0x%llx\n", sym_name, sym_addr);
+	}
+	return 0;
+}
+
+static int bpf_object__read_kallsyms_file(struct bpf_object *obj)
+{
+	return libbpf_kallsyms_parse(kallsyms_cb, obj);
+}
+
 static int find_ksym_btf_id(struct bpf_object *obj, const char *ksym_name,
 			    __u16 kind, struct btf **res_btf,
 			    struct module_btf **res_mod_btf)
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index 4fda8bdf0a0d..b6247dc7f8eb 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -449,6 +449,11 @@ __s32 btf__find_by_name_kind_own(const struct btf *btf, const char *type_name,
 
 extern enum libbpf_strict_mode libbpf_mode;
 
+typedef int (*kallsyms_cb_t)(unsigned long long sym_addr, char sym_type,
+			     const char *sym_name, void *ctx);
+
+int libbpf_kallsyms_parse(kallsyms_cb_t cb, void *arg);
+
 /* handle direct returned errors */
 static inline int libbpf_err(int ret)
 {
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCHv3 bpf-next 08/13] libbpf: Add bpf_link_create support for multi kprobes
  2022-03-16 12:24 [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link Jiri Olsa
                   ` (6 preceding siblings ...)
  2022-03-16 12:24 ` [PATCHv3 bpf-next 07/13] libbpf: Add libbpf_kallsyms_parse function Jiri Olsa
@ 2022-03-16 12:24 ` Jiri Olsa
  2022-03-16 12:24 ` [PATCHv3 bpf-next 09/13] libbpf: Add bpf_program__attach_kprobe_multi_opts function Jiri Olsa
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: Jiri Olsa @ 2022-03-16 12:24 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Masami Hiramatsu
  Cc: netdev, bpf, lkml, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Steven Rostedt

Adding new kprobe_multi struct to bpf_link_create_opts object
to pass multiple kprobe data to link_create attr uapi.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/bpf/bpf.c | 9 +++++++++
 tools/lib/bpf/bpf.h | 9 ++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index f69ce3a01385..cf27251adb92 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -854,6 +854,15 @@ int bpf_link_create(int prog_fd, int target_fd,
 		if (!OPTS_ZEROED(opts, perf_event))
 			return libbpf_err(-EINVAL);
 		break;
+	case BPF_TRACE_KPROBE_MULTI:
+		attr.link_create.kprobe_multi.flags = OPTS_GET(opts, kprobe_multi.flags, 0);
+		attr.link_create.kprobe_multi.cnt = OPTS_GET(opts, kprobe_multi.cnt, 0);
+		attr.link_create.kprobe_multi.syms = ptr_to_u64(OPTS_GET(opts, kprobe_multi.syms, 0));
+		attr.link_create.kprobe_multi.addrs = ptr_to_u64(OPTS_GET(opts, kprobe_multi.addrs, 0));
+		attr.link_create.kprobe_multi.cookies = ptr_to_u64(OPTS_GET(opts, kprobe_multi.cookies, 0));
+		if (!OPTS_ZEROED(opts, kprobe_multi))
+			return libbpf_err(-EINVAL);
+		break;
 	default:
 		if (!OPTS_ZEROED(opts, flags))
 			return libbpf_err(-EINVAL);
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 5253cb4a4c0a..f4b4afb6d4ba 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -413,10 +413,17 @@ struct bpf_link_create_opts {
 		struct {
 			__u64 bpf_cookie;
 		} perf_event;
+		struct {
+			__u32 flags;
+			__u32 cnt;
+			const char **syms;
+			const unsigned long *addrs;
+			const __u64 *cookies;
+		} kprobe_multi;
 	};
 	size_t :0;
 };
-#define bpf_link_create_opts__last_field perf_event
+#define bpf_link_create_opts__last_field kprobe_multi.cookies
 
 LIBBPF_API int bpf_link_create(int prog_fd, int target_fd,
 			       enum bpf_attach_type attach_type,
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCHv3 bpf-next 09/13] libbpf: Add bpf_program__attach_kprobe_multi_opts function
  2022-03-16 12:24 [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link Jiri Olsa
                   ` (7 preceding siblings ...)
  2022-03-16 12:24 ` [PATCHv3 bpf-next 08/13] libbpf: Add bpf_link_create support for multi kprobes Jiri Olsa
@ 2022-03-16 12:24 ` Jiri Olsa
  2022-03-18  3:53   ` Alexei Starovoitov
  2022-03-16 12:24 ` [PATCHv3 bpf-next 10/13] selftests/bpf: Add kprobe_multi attach test Jiri Olsa
                   ` (5 subsequent siblings)
  14 siblings, 1 reply; 26+ messages in thread
From: Jiri Olsa @ 2022-03-16 12:24 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Masami Hiramatsu
  Cc: netdev, bpf, lkml, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Steven Rostedt

Adding bpf_program__attach_kprobe_multi_opts function for attaching
kprobe program to multiple functions.

  struct bpf_link *
  bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
                                        const char *pattern,
                                        const struct bpf_kprobe_multi_opts *opts);

User can specify functions to attach with 'pattern' argument that
allows wildcards (*?' supported) or provide symbols or addresses
directly through opts argument. These 3 options are mutually
exclusive.

When using symbols or addresses, user can also provide cookie value
for each symbol/address that can be retrieved later in bpf program
with bpf_get_attach_cookie helper.

  struct bpf_kprobe_multi_opts {
          size_t sz;
          const char **syms;
          const unsigned long *addrs;
          const __u64 *cookies;
          size_t cnt;
          bool retprobe;
          size_t :0;
  };

Symbols, addresses and cookies are provided through opts object
(syms/addrs/cookies) as array pointers with specified count (cnt).

Each cookie value is paired with provided function address or symbol
with the same array index.

The program can be also attached as return probe if 'retprobe' is set.

For quick usage with NULL opts argument, like:

  bpf_program__attach_kprobe_multi_opts(prog, "ksys_*", NULL)

the 'prog' will be attached as kprobe to 'ksys_*' functions.

Also adding new program sections for automatic attachment:

  kprobe.multi/<symbol_pattern>
  kretprobe.multi/<symbol_pattern>

The symbol_pattern is used as 'pattern' argument in
bpf_program__attach_kprobe_multi_opts function.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/bpf/libbpf.c   | 160 +++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h   |  23 ++++++
 tools/lib/bpf/libbpf.map |   1 +
 3 files changed, 184 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 1ca520a29fdb..f3a31478e23b 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -8610,6 +8610,7 @@ static int attach_kprobe(const struct bpf_program *prog, long cookie, struct bpf
 static int attach_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link);
 static int attach_raw_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link);
 static int attach_trace(const struct bpf_program *prog, long cookie, struct bpf_link **link);
+static int attach_kprobe_multi(const struct bpf_program *prog, long cookie, struct bpf_link **link);
 static int attach_lsm(const struct bpf_program *prog, long cookie, struct bpf_link **link);
 static int attach_iter(const struct bpf_program *prog, long cookie, struct bpf_link **link);
 
@@ -8621,6 +8622,8 @@ static const struct bpf_sec_def section_defs[] = {
 	SEC_DEF("uprobe/",		KPROBE,	0, SEC_NONE),
 	SEC_DEF("kretprobe/",		KPROBE, 0, SEC_NONE, attach_kprobe),
 	SEC_DEF("uretprobe/",		KPROBE, 0, SEC_NONE),
+	SEC_DEF("kprobe.multi/",	KPROBE,	BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
+	SEC_DEF("kretprobe.multi/",	KPROBE,	BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
 	SEC_DEF("tc",			SCHED_CLS, 0, SEC_NONE),
 	SEC_DEF("classifier",		SCHED_CLS, 0, SEC_NONE | SEC_SLOPPY_PFX | SEC_DEPRECATED),
 	SEC_DEF("action",		SCHED_ACT, 0, SEC_NONE | SEC_SLOPPY_PFX),
@@ -10224,6 +10227,139 @@ struct bpf_link *bpf_program__attach_kprobe(const struct bpf_program *prog,
 	return bpf_program__attach_kprobe_opts(prog, func_name, &opts);
 }
 
+/* Adapted from perf/util/string.c */
+static bool glob_match(const char *str, const char *pat)
+{
+	while (*str && *pat && *pat != '*') {
+		if (*pat == '?') {      /* Matches any single character */
+			str++;
+			pat++;
+			continue;
+		}
+		if (*str != *pat)
+			return false;
+		str++;
+		pat++;
+	}
+	/* Check wild card */
+	if (*pat == '*') {
+		while (*pat == '*')
+			pat++;
+		if (!*pat) /* Tail wild card matches all */
+			return true;
+		while (*str)
+			if (glob_match(str++, pat))
+				return true;
+	}
+	return !*str && !*pat;
+}
+
+struct kprobe_multi_resolve {
+	const char *pattern;
+	unsigned long *addrs;
+	size_t cap;
+	size_t cnt;
+};
+
+static int
+resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
+			const char *sym_name, void *ctx)
+{
+	struct kprobe_multi_resolve *res = ctx;
+	int err;
+
+	if (!glob_match(sym_name, res->pattern))
+		return 0;
+
+	err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(unsigned long),
+				res->cnt + 1);
+	if (err)
+		return err;
+
+	res->addrs[res->cnt++] = (unsigned long) sym_addr;
+	return 0;
+}
+
+struct bpf_link *
+bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
+				      const char *pattern,
+				      const struct bpf_kprobe_multi_opts *opts)
+{
+	LIBBPF_OPTS(bpf_link_create_opts, lopts);
+	struct kprobe_multi_resolve res = {
+		.pattern = pattern,
+	};
+	struct bpf_link *link = NULL;
+	char errmsg[STRERR_BUFSIZE];
+	const unsigned long *addrs;
+	int err, link_fd, prog_fd;
+	const __u64 *cookies;
+	const char **syms;
+	bool retprobe;
+	size_t cnt;
+
+	if (!OPTS_VALID(opts, bpf_kprobe_multi_opts))
+		return libbpf_err_ptr(-EINVAL);
+
+	syms    = OPTS_GET(opts, syms, false);
+	addrs   = OPTS_GET(opts, addrs, false);
+	cnt     = OPTS_GET(opts, cnt, false);
+	cookies = OPTS_GET(opts, cookies, false);
+
+	if (!pattern && !addrs && !syms)
+		return libbpf_err_ptr(-EINVAL);
+	if (pattern && (addrs || syms || cookies || cnt))
+		return libbpf_err_ptr(-EINVAL);
+	if (!pattern && !cnt)
+		return libbpf_err_ptr(-EINVAL);
+	if (addrs && syms)
+		return libbpf_err_ptr(-EINVAL);
+
+	if (pattern) {
+		err = libbpf_kallsyms_parse(resolve_kprobe_multi_cb, &res);
+		if (err)
+			goto error;
+		if (!res.cnt) {
+			err = -ENOENT;
+			goto error;
+		}
+		addrs = res.addrs;
+		cnt = res.cnt;
+	}
+
+	retprobe = OPTS_GET(opts, retprobe, false);
+
+	lopts.kprobe_multi.syms = syms;
+	lopts.kprobe_multi.addrs = addrs;
+	lopts.kprobe_multi.cookies = cookies;
+	lopts.kprobe_multi.cnt = cnt;
+	lopts.kprobe_multi.flags = retprobe ? BPF_F_KPROBE_MULTI_RETURN : 0;
+
+	link = calloc(1, sizeof(*link));
+	if (!link) {
+		err = -ENOMEM;
+		goto error;
+	}
+	link->detach = &bpf_link__detach_fd;
+
+	prog_fd = bpf_program__fd(prog);
+	link_fd = bpf_link_create(prog_fd, 0, BPF_TRACE_KPROBE_MULTI, &lopts);
+	if (link_fd < 0) {
+		err = -errno;
+		pr_warn("prog '%s': failed to attach: %s\n",
+			prog->name, libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
+		goto error;
+	}
+	link->fd = link_fd;
+	free(res.addrs);
+	return link;
+
+error:
+	free(link);
+	free(res.addrs);
+	return libbpf_err_ptr(err);
+}
+
 static int attach_kprobe(const struct bpf_program *prog, long cookie, struct bpf_link **link)
 {
 	DECLARE_LIBBPF_OPTS(bpf_kprobe_opts, opts);
@@ -10255,6 +10391,30 @@ static int attach_kprobe(const struct bpf_program *prog, long cookie, struct bpf
 	return libbpf_get_error(*link);
 }
 
+static int attach_kprobe_multi(const struct bpf_program *prog, long cookie, struct bpf_link **link)
+{
+	LIBBPF_OPTS(bpf_kprobe_multi_opts, opts);
+	const char *spec;
+	char *pattern;
+	int n;
+
+	opts.retprobe = str_has_pfx(prog->sec_name, "kretprobe.multi/");
+	if (opts.retprobe)
+		spec = prog->sec_name + sizeof("kretprobe.multi/") - 1;
+	else
+		spec = prog->sec_name + sizeof("kprobe.multi/") - 1;
+
+	n = sscanf(spec, "%m[a-zA-Z0-9_.*?]", &pattern);
+	if (n < 1) {
+		pr_warn("kprobe multi pattern is invalid: %s\n", pattern);
+		return -EINVAL;
+	}
+
+	*link = bpf_program__attach_kprobe_multi_opts(prog, pattern, &opts);
+	free(pattern);
+	return libbpf_get_error(*link);
+}
+
 static void gen_uprobe_legacy_event_name(char *buf, size_t buf_sz,
 					 const char *binary_path, uint64_t offset)
 {
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index c1b0c2ef14d8..d5239fb4abdc 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -425,6 +425,29 @@ bpf_program__attach_kprobe_opts(const struct bpf_program *prog,
                                 const char *func_name,
                                 const struct bpf_kprobe_opts *opts);
 
+struct bpf_kprobe_multi_opts {
+	/* size of this struct, for forward/backward compatibility */
+	size_t sz;
+	/* array of function symbols to attach */
+	const char **syms;
+	/* array of function addresses to attach */
+	const unsigned long *addrs;
+	/* array of user-provided values fetchable through bpf_get_attach_cookie */
+	const __u64 *cookies;
+	/* number of elements in syms/addrs/cookies arrays */
+	size_t cnt;
+	/* create return kprobes */
+	bool retprobe;
+	size_t :0;
+};
+
+#define bpf_kprobe_multi_opts__last_field retprobe
+
+LIBBPF_API struct bpf_link *
+bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
+				      const char *pattern,
+				      const struct bpf_kprobe_multi_opts *opts);
+
 struct bpf_uprobe_opts {
 	/* size of this struct, for forward/backward compatiblity */
 	size_t sz;
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index df1b947792c8..554c56e6e5d3 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -444,4 +444,5 @@ LIBBPF_0.8.0 {
 	global:
 		libbpf_register_prog_handler;
 		libbpf_unregister_prog_handler;
+		bpf_program__attach_kprobe_multi_opts;
 } LIBBPF_0.7.0;
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCHv3 bpf-next 10/13] selftests/bpf: Add kprobe_multi attach test
  2022-03-16 12:24 [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link Jiri Olsa
                   ` (8 preceding siblings ...)
  2022-03-16 12:24 ` [PATCHv3 bpf-next 09/13] libbpf: Add bpf_program__attach_kprobe_multi_opts function Jiri Olsa
@ 2022-03-16 12:24 ` Jiri Olsa
  2022-03-16 12:24 ` [PATCHv3 bpf-next 11/13] selftests/bpf: Add kprobe_multi bpf_cookie test Jiri Olsa
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: Jiri Olsa @ 2022-03-16 12:24 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Masami Hiramatsu
  Cc: netdev, bpf, lkml, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Steven Rostedt

Adding kprobe_multi attach test that uses new fprobe interface to
attach kprobe program to multiple functions.

The test is attaching programs to bpf_fentry_test* functions and
uses single trampoline program bpf_prog_test_run to trigger
bpf_fentry_test* functions.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 .../bpf/prog_tests/kprobe_multi_test.c        | 141 ++++++++++++++++++
 .../selftests/bpf/progs/kprobe_multi.c        |  95 ++++++++++++
 tools/testing/selftests/bpf/trace_helpers.c   |   7 +
 3 files changed, 243 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
 create mode 100644 tools/testing/selftests/bpf/progs/kprobe_multi.c

diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
new file mode 100644
index 000000000000..ded6b8c8ec05
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include "kprobe_multi.skel.h"
+#include "trace_helpers.h"
+
+static void kprobe_multi_test_run(struct kprobe_multi *skel)
+{
+	LIBBPF_OPTS(bpf_test_run_opts, topts);
+	int err, prog_fd;
+
+	prog_fd = bpf_program__fd(skel->progs.trigger);
+	err = bpf_prog_test_run_opts(prog_fd, &topts);
+	ASSERT_OK(err, "test_run");
+	ASSERT_EQ(topts.retval, 0, "test_run");
+
+	ASSERT_EQ(skel->bss->kprobe_test1_result, 1, "kprobe_test1_result");
+	ASSERT_EQ(skel->bss->kprobe_test2_result, 1, "kprobe_test2_result");
+	ASSERT_EQ(skel->bss->kprobe_test3_result, 1, "kprobe_test3_result");
+	ASSERT_EQ(skel->bss->kprobe_test4_result, 1, "kprobe_test4_result");
+	ASSERT_EQ(skel->bss->kprobe_test5_result, 1, "kprobe_test5_result");
+	ASSERT_EQ(skel->bss->kprobe_test6_result, 1, "kprobe_test6_result");
+	ASSERT_EQ(skel->bss->kprobe_test7_result, 1, "kprobe_test7_result");
+	ASSERT_EQ(skel->bss->kprobe_test8_result, 1, "kprobe_test8_result");
+
+	ASSERT_EQ(skel->bss->kretprobe_test1_result, 1, "kretprobe_test1_result");
+	ASSERT_EQ(skel->bss->kretprobe_test2_result, 1, "kretprobe_test2_result");
+	ASSERT_EQ(skel->bss->kretprobe_test3_result, 1, "kretprobe_test3_result");
+	ASSERT_EQ(skel->bss->kretprobe_test4_result, 1, "kretprobe_test4_result");
+	ASSERT_EQ(skel->bss->kretprobe_test5_result, 1, "kretprobe_test5_result");
+	ASSERT_EQ(skel->bss->kretprobe_test6_result, 1, "kretprobe_test6_result");
+	ASSERT_EQ(skel->bss->kretprobe_test7_result, 1, "kretprobe_test7_result");
+	ASSERT_EQ(skel->bss->kretprobe_test8_result, 1, "kretprobe_test8_result");
+}
+
+static void test_skel_api(void)
+{
+	struct kprobe_multi *skel = NULL;
+	int err;
+
+	skel = kprobe_multi__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "kprobe_multi__open_and_load"))
+		goto cleanup;
+
+	skel->bss->pid = getpid();
+	err = kprobe_multi__attach(skel);
+	if (!ASSERT_OK(err, "kprobe_multi__attach"))
+		goto cleanup;
+
+	kprobe_multi_test_run(skel);
+
+cleanup:
+	kprobe_multi__destroy(skel);
+}
+
+static void test_link_api(struct bpf_link_create_opts *opts)
+{
+	int prog_fd, link1_fd = -1, link2_fd = -1;
+	struct kprobe_multi *skel = NULL;
+
+	skel = kprobe_multi__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "fentry_raw_skel_load"))
+		goto cleanup;
+
+	skel->bss->pid = getpid();
+	prog_fd = bpf_program__fd(skel->progs.test_kprobe);
+	link1_fd = bpf_link_create(prog_fd, 0, BPF_TRACE_KPROBE_MULTI, opts);
+	if (!ASSERT_GE(link1_fd, 0, "link_fd"))
+		goto cleanup;
+
+	opts->kprobe_multi.flags = BPF_F_KPROBE_MULTI_RETURN;
+	prog_fd = bpf_program__fd(skel->progs.test_kretprobe);
+	link2_fd = bpf_link_create(prog_fd, 0, BPF_TRACE_KPROBE_MULTI, opts);
+	if (!ASSERT_GE(link2_fd, 0, "link_fd"))
+		goto cleanup;
+
+	kprobe_multi_test_run(skel);
+
+cleanup:
+	if (link1_fd != -1)
+		close(link1_fd);
+	if (link2_fd != -1)
+		close(link2_fd);
+	kprobe_multi__destroy(skel);
+}
+
+#define GET_ADDR(__sym, __addr) ({					\
+	__addr = ksym_get_addr(__sym);					\
+	if (!ASSERT_NEQ(__addr, 0, "kallsyms load failed for " #__sym))	\
+		return;							\
+})
+
+static void test_link_api_addrs(void)
+{
+	LIBBPF_OPTS(bpf_link_create_opts, opts);
+	unsigned long long addrs[8];
+
+	GET_ADDR("bpf_fentry_test1", addrs[0]);
+	GET_ADDR("bpf_fentry_test2", addrs[1]);
+	GET_ADDR("bpf_fentry_test3", addrs[2]);
+	GET_ADDR("bpf_fentry_test4", addrs[3]);
+	GET_ADDR("bpf_fentry_test5", addrs[4]);
+	GET_ADDR("bpf_fentry_test6", addrs[5]);
+	GET_ADDR("bpf_fentry_test7", addrs[6]);
+	GET_ADDR("bpf_fentry_test8", addrs[7]);
+
+	opts.kprobe_multi.addrs = (const unsigned long*) addrs;
+	opts.kprobe_multi.cnt = ARRAY_SIZE(addrs);
+	test_link_api(&opts);
+}
+
+static void test_link_api_syms(void)
+{
+	LIBBPF_OPTS(bpf_link_create_opts, opts);
+	const char *syms[8] = {
+		"bpf_fentry_test1",
+		"bpf_fentry_test2",
+		"bpf_fentry_test3",
+		"bpf_fentry_test4",
+		"bpf_fentry_test5",
+		"bpf_fentry_test6",
+		"bpf_fentry_test7",
+		"bpf_fentry_test8",
+	};
+
+	opts.kprobe_multi.syms = syms;
+	opts.kprobe_multi.cnt = ARRAY_SIZE(syms);
+	test_link_api(&opts);
+}
+
+void test_kprobe_multi_test(void)
+{
+	if (!ASSERT_OK(load_kallsyms(), "load_kallsyms"))
+		return;
+
+	if (test__start_subtest("skel_api"))
+		test_skel_api();
+	if (test__start_subtest("link_api_addrs"))
+		test_link_api_syms();
+	if (test__start_subtest("link_api_syms"))
+		test_link_api_addrs();
+}
diff --git a/tools/testing/selftests/bpf/progs/kprobe_multi.c b/tools/testing/selftests/bpf/progs/kprobe_multi.c
new file mode 100644
index 000000000000..6616c082c8c2
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/kprobe_multi.c
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include <stdbool.h>
+
+char _license[] SEC("license") = "GPL";
+
+extern const void bpf_fentry_test1 __ksym;
+extern const void bpf_fentry_test2 __ksym;
+extern const void bpf_fentry_test3 __ksym;
+extern const void bpf_fentry_test4 __ksym;
+extern const void bpf_fentry_test5 __ksym;
+extern const void bpf_fentry_test6 __ksym;
+extern const void bpf_fentry_test7 __ksym;
+extern const void bpf_fentry_test8 __ksym;
+
+int pid = 0;
+
+__u64 kprobe_test1_result = 0;
+__u64 kprobe_test2_result = 0;
+__u64 kprobe_test3_result = 0;
+__u64 kprobe_test4_result = 0;
+__u64 kprobe_test5_result = 0;
+__u64 kprobe_test6_result = 0;
+__u64 kprobe_test7_result = 0;
+__u64 kprobe_test8_result = 0;
+
+__u64 kretprobe_test1_result = 0;
+__u64 kretprobe_test2_result = 0;
+__u64 kretprobe_test3_result = 0;
+__u64 kretprobe_test4_result = 0;
+__u64 kretprobe_test5_result = 0;
+__u64 kretprobe_test6_result = 0;
+__u64 kretprobe_test7_result = 0;
+__u64 kretprobe_test8_result = 0;
+
+static void kprobe_multi_check(void *ctx, bool is_return)
+{
+	if (bpf_get_current_pid_tgid() >> 32 != pid)
+		return;
+
+	__u64 addr = bpf_get_func_ip(ctx);
+
+#define SET(__var, __addr) ({			\
+	if ((const void *) addr == __addr) 	\
+		__var = 1;			\
+})
+
+	if (is_return) {
+		SET(kretprobe_test1_result, &bpf_fentry_test1);
+		SET(kretprobe_test2_result, &bpf_fentry_test2);
+		SET(kretprobe_test3_result, &bpf_fentry_test3);
+		SET(kretprobe_test4_result, &bpf_fentry_test4);
+		SET(kretprobe_test5_result, &bpf_fentry_test5);
+		SET(kretprobe_test6_result, &bpf_fentry_test6);
+		SET(kretprobe_test7_result, &bpf_fentry_test7);
+		SET(kretprobe_test8_result, &bpf_fentry_test8);
+	} else {
+		SET(kprobe_test1_result, &bpf_fentry_test1);
+		SET(kprobe_test2_result, &bpf_fentry_test2);
+		SET(kprobe_test3_result, &bpf_fentry_test3);
+		SET(kprobe_test4_result, &bpf_fentry_test4);
+		SET(kprobe_test5_result, &bpf_fentry_test5);
+		SET(kprobe_test6_result, &bpf_fentry_test6);
+		SET(kprobe_test7_result, &bpf_fentry_test7);
+		SET(kprobe_test8_result, &bpf_fentry_test8);
+	}
+
+#undef SET
+}
+
+/*
+ * No tests in here, just to trigger 'bpf_fentry_test*'
+ * through tracing test_run
+ */
+SEC("fentry/bpf_modify_return_test")
+int BPF_PROG(trigger)
+{
+	return 0;
+}
+
+SEC("kprobe.multi/bpf_fentry_tes??")
+int test_kprobe(struct pt_regs *ctx)
+{
+	kprobe_multi_check(ctx, false);
+	return 0;
+}
+
+SEC("kretprobe.multi/bpf_fentry_test*")
+int test_kretprobe(struct pt_regs *ctx)
+{
+	kprobe_multi_check(ctx, true);
+	return 0;
+}
diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c
index ca6abae9b09c..3d6217e3aff7 100644
--- a/tools/testing/selftests/bpf/trace_helpers.c
+++ b/tools/testing/selftests/bpf/trace_helpers.c
@@ -34,6 +34,13 @@ int load_kallsyms(void)
 	if (!f)
 		return -ENOENT;
 
+	/*
+	 * This is called/used from multiplace places,
+	 * load symbols just once.
+	 */
+	if (sym_cnt)
+		return 0;
+
 	while (fgets(buf, sizeof(buf), f)) {
 		if (sscanf(buf, "%p %c %s", &addr, &symbol, func) != 3)
 			break;
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCHv3 bpf-next 11/13] selftests/bpf: Add kprobe_multi bpf_cookie test
  2022-03-16 12:24 [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link Jiri Olsa
                   ` (9 preceding siblings ...)
  2022-03-16 12:24 ` [PATCHv3 bpf-next 10/13] selftests/bpf: Add kprobe_multi attach test Jiri Olsa
@ 2022-03-16 12:24 ` Jiri Olsa
  2022-03-16 12:24 ` [PATCHv3 bpf-next 12/13] selftests/bpf: Add attach test for bpf_program__attach_kprobe_multi_opts Jiri Olsa
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: Jiri Olsa @ 2022-03-16 12:24 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Masami Hiramatsu
  Cc: netdev, bpf, lkml, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Steven Rostedt

Adding bpf_cookie test for programs attached by kprobe_multi links.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 .../selftests/bpf/prog_tests/bpf_cookie.c     | 109 ++++++++++++++++++
 .../selftests/bpf/progs/kprobe_multi.c        |  41 ++++---
 2 files changed, 131 insertions(+), 19 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
index 0612e79a9281..6671d4dc0b5d 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
@@ -7,6 +7,7 @@
 #include <unistd.h>
 #include <test_progs.h>
 #include "test_bpf_cookie.skel.h"
+#include "kprobe_multi.skel.h"
 
 /* uprobe attach point */
 static void trigger_func(void)
@@ -63,6 +64,112 @@ static void kprobe_subtest(struct test_bpf_cookie *skel)
 	bpf_link__destroy(retlink2);
 }
 
+static void kprobe_multi_test_run(struct kprobe_multi *skel)
+{
+	LIBBPF_OPTS(bpf_test_run_opts, topts);
+	int err, prog_fd;
+
+	prog_fd = bpf_program__fd(skel->progs.trigger);
+	err = bpf_prog_test_run_opts(prog_fd, &topts);
+	ASSERT_OK(err, "test_run");
+	ASSERT_EQ(topts.retval, 0, "test_run");
+
+	ASSERT_EQ(skel->bss->kprobe_test1_result, 1, "kprobe_test1_result");
+	ASSERT_EQ(skel->bss->kprobe_test2_result, 1, "kprobe_test2_result");
+	ASSERT_EQ(skel->bss->kprobe_test3_result, 1, "kprobe_test3_result");
+	ASSERT_EQ(skel->bss->kprobe_test4_result, 1, "kprobe_test4_result");
+	ASSERT_EQ(skel->bss->kprobe_test5_result, 1, "kprobe_test5_result");
+	ASSERT_EQ(skel->bss->kprobe_test6_result, 1, "kprobe_test6_result");
+	ASSERT_EQ(skel->bss->kprobe_test7_result, 1, "kprobe_test7_result");
+	ASSERT_EQ(skel->bss->kprobe_test8_result, 1, "kprobe_test8_result");
+
+	ASSERT_EQ(skel->bss->kretprobe_test1_result, 1, "kretprobe_test1_result");
+	ASSERT_EQ(skel->bss->kretprobe_test2_result, 1, "kretprobe_test2_result");
+	ASSERT_EQ(skel->bss->kretprobe_test3_result, 1, "kretprobe_test3_result");
+	ASSERT_EQ(skel->bss->kretprobe_test4_result, 1, "kretprobe_test4_result");
+	ASSERT_EQ(skel->bss->kretprobe_test5_result, 1, "kretprobe_test5_result");
+	ASSERT_EQ(skel->bss->kretprobe_test6_result, 1, "kretprobe_test6_result");
+	ASSERT_EQ(skel->bss->kretprobe_test7_result, 1, "kretprobe_test7_result");
+	ASSERT_EQ(skel->bss->kretprobe_test8_result, 1, "kretprobe_test8_result");
+}
+
+static void kprobe_multi_link_api_subtest(void)
+{
+	int prog_fd, link1_fd = -1, link2_fd = -1;
+	struct kprobe_multi *skel = NULL;
+	LIBBPF_OPTS(bpf_link_create_opts, opts);
+	unsigned long long addrs[8];
+	__u64 cookies[8];
+
+	if (!ASSERT_OK(load_kallsyms(), "load_kallsyms"))
+		goto cleanup;
+
+	skel = kprobe_multi__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "fentry_raw_skel_load"))
+		goto cleanup;
+
+	skel->bss->pid = getpid();
+	skel->bss->test_cookie = true;
+
+#define GET_ADDR(__sym, __addr) ({				\
+	__addr = ksym_get_addr(__sym);				\
+	if (!ASSERT_NEQ(__addr, 0, "ksym_get_addr " #__sym))	\
+		goto cleanup;					\
+})
+
+	GET_ADDR("bpf_fentry_test1", addrs[0]);
+	GET_ADDR("bpf_fentry_test2", addrs[1]);
+	GET_ADDR("bpf_fentry_test3", addrs[2]);
+	GET_ADDR("bpf_fentry_test4", addrs[3]);
+	GET_ADDR("bpf_fentry_test5", addrs[4]);
+	GET_ADDR("bpf_fentry_test6", addrs[5]);
+	GET_ADDR("bpf_fentry_test7", addrs[6]);
+	GET_ADDR("bpf_fentry_test8", addrs[7]);
+
+#undef GET_ADDR
+
+	cookies[0] = 1;
+	cookies[1] = 2;
+	cookies[2] = 3;
+	cookies[3] = 4;
+	cookies[4] = 5;
+	cookies[5] = 6;
+	cookies[6] = 7;
+	cookies[7] = 8;
+
+	opts.kprobe_multi.addrs = (const unsigned long *) &addrs;
+	opts.kprobe_multi.cnt = ARRAY_SIZE(addrs);
+	opts.kprobe_multi.cookies = (const __u64 *) &cookies;
+	prog_fd = bpf_program__fd(skel->progs.test_kprobe);
+
+	link1_fd = bpf_link_create(prog_fd, 0, BPF_TRACE_KPROBE_MULTI, &opts);
+	if (!ASSERT_GE(link1_fd, 0, "link1_fd"))
+		goto cleanup;
+
+	cookies[0] = 8;
+	cookies[1] = 7;
+	cookies[2] = 6;
+	cookies[3] = 5;
+	cookies[4] = 4;
+	cookies[5] = 3;
+	cookies[6] = 2;
+	cookies[7] = 1;
+
+	opts.kprobe_multi.flags = BPF_F_KPROBE_MULTI_RETURN;
+	prog_fd = bpf_program__fd(skel->progs.test_kretprobe);
+
+	link2_fd = bpf_link_create(prog_fd, 0, BPF_TRACE_KPROBE_MULTI, &opts);
+	if (!ASSERT_GE(link2_fd, 0, "link2_fd"))
+		goto cleanup;
+
+	kprobe_multi_test_run(skel);
+
+cleanup:
+	close(link1_fd);
+	close(link2_fd);
+	kprobe_multi__destroy(skel);
+}
+
 static void uprobe_subtest(struct test_bpf_cookie *skel)
 {
 	DECLARE_LIBBPF_OPTS(bpf_uprobe_opts, opts);
@@ -249,6 +356,8 @@ void test_bpf_cookie(void)
 
 	if (test__start_subtest("kprobe"))
 		kprobe_subtest(skel);
+	if (test__start_subtest("multi_kprobe_link_api"))
+		kprobe_multi_link_api_subtest();
 	if (test__start_subtest("uprobe"))
 		uprobe_subtest(skel);
 	if (test__start_subtest("tracepoint"))
diff --git a/tools/testing/selftests/bpf/progs/kprobe_multi.c b/tools/testing/selftests/bpf/progs/kprobe_multi.c
index 6616c082c8c2..af27d2c6fce8 100644
--- a/tools/testing/selftests/bpf/progs/kprobe_multi.c
+++ b/tools/testing/selftests/bpf/progs/kprobe_multi.c
@@ -16,6 +16,7 @@ extern const void bpf_fentry_test7 __ksym;
 extern const void bpf_fentry_test8 __ksym;
 
 int pid = 0;
+bool test_cookie = false;
 
 __u64 kprobe_test1_result = 0;
 __u64 kprobe_test2_result = 0;
@@ -40,31 +41,33 @@ static void kprobe_multi_check(void *ctx, bool is_return)
 	if (bpf_get_current_pid_tgid() >> 32 != pid)
 		return;
 
+	__u64 cookie = test_cookie ? bpf_get_attach_cookie(ctx) : 0;
 	__u64 addr = bpf_get_func_ip(ctx);
 
-#define SET(__var, __addr) ({			\
-	if ((const void *) addr == __addr) 	\
-		__var = 1;			\
+#define SET(__var, __addr, __cookie) ({			\
+	if (((const void *) addr == __addr) &&		\
+	     (!test_cookie || (cookie == __cookie)))	\
+		__var = 1;				\
 })
 
 	if (is_return) {
-		SET(kretprobe_test1_result, &bpf_fentry_test1);
-		SET(kretprobe_test2_result, &bpf_fentry_test2);
-		SET(kretprobe_test3_result, &bpf_fentry_test3);
-		SET(kretprobe_test4_result, &bpf_fentry_test4);
-		SET(kretprobe_test5_result, &bpf_fentry_test5);
-		SET(kretprobe_test6_result, &bpf_fentry_test6);
-		SET(kretprobe_test7_result, &bpf_fentry_test7);
-		SET(kretprobe_test8_result, &bpf_fentry_test8);
+		SET(kretprobe_test1_result, &bpf_fentry_test1, 8);
+		SET(kretprobe_test2_result, &bpf_fentry_test2, 7);
+		SET(kretprobe_test3_result, &bpf_fentry_test3, 6);
+		SET(kretprobe_test4_result, &bpf_fentry_test4, 5);
+		SET(kretprobe_test5_result, &bpf_fentry_test5, 4);
+		SET(kretprobe_test6_result, &bpf_fentry_test6, 3);
+		SET(kretprobe_test7_result, &bpf_fentry_test7, 2);
+		SET(kretprobe_test8_result, &bpf_fentry_test8, 1);
 	} else {
-		SET(kprobe_test1_result, &bpf_fentry_test1);
-		SET(kprobe_test2_result, &bpf_fentry_test2);
-		SET(kprobe_test3_result, &bpf_fentry_test3);
-		SET(kprobe_test4_result, &bpf_fentry_test4);
-		SET(kprobe_test5_result, &bpf_fentry_test5);
-		SET(kprobe_test6_result, &bpf_fentry_test6);
-		SET(kprobe_test7_result, &bpf_fentry_test7);
-		SET(kprobe_test8_result, &bpf_fentry_test8);
+		SET(kprobe_test1_result, &bpf_fentry_test1, 1);
+		SET(kprobe_test2_result, &bpf_fentry_test2, 2);
+		SET(kprobe_test3_result, &bpf_fentry_test3, 3);
+		SET(kprobe_test4_result, &bpf_fentry_test4, 4);
+		SET(kprobe_test5_result, &bpf_fentry_test5, 5);
+		SET(kprobe_test6_result, &bpf_fentry_test6, 6);
+		SET(kprobe_test7_result, &bpf_fentry_test7, 7);
+		SET(kprobe_test8_result, &bpf_fentry_test8, 8);
 	}
 
 #undef SET
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCHv3 bpf-next 12/13] selftests/bpf: Add attach test for bpf_program__attach_kprobe_multi_opts
  2022-03-16 12:24 [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link Jiri Olsa
                   ` (10 preceding siblings ...)
  2022-03-16 12:24 ` [PATCHv3 bpf-next 11/13] selftests/bpf: Add kprobe_multi bpf_cookie test Jiri Olsa
@ 2022-03-16 12:24 ` Jiri Olsa
  2022-03-16 12:24 ` [PATCHv3 bpf-next 13/13] selftests/bpf: Add cookie " Jiri Olsa
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: Jiri Olsa @ 2022-03-16 12:24 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Masami Hiramatsu
  Cc: netdev, bpf, lkml, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Steven Rostedt

Adding tests for bpf_program__attach_kprobe_multi_opts function,
that test attach with pattern, symbols and addrs.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 .../bpf/prog_tests/kprobe_multi_test.c        | 204 +++++++++++++++++-
 1 file changed, 193 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
index ded6b8c8ec05..b9876b55fc0c 100644
--- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
@@ -3,7 +3,7 @@
 #include "kprobe_multi.skel.h"
 #include "trace_helpers.h"
 
-static void kprobe_multi_test_run(struct kprobe_multi *skel)
+static void kprobe_multi_test_run(struct kprobe_multi *skel, bool test_return)
 {
 	LIBBPF_OPTS(bpf_test_run_opts, topts);
 	int err, prog_fd;
@@ -22,14 +22,16 @@ static void kprobe_multi_test_run(struct kprobe_multi *skel)
 	ASSERT_EQ(skel->bss->kprobe_test7_result, 1, "kprobe_test7_result");
 	ASSERT_EQ(skel->bss->kprobe_test8_result, 1, "kprobe_test8_result");
 
-	ASSERT_EQ(skel->bss->kretprobe_test1_result, 1, "kretprobe_test1_result");
-	ASSERT_EQ(skel->bss->kretprobe_test2_result, 1, "kretprobe_test2_result");
-	ASSERT_EQ(skel->bss->kretprobe_test3_result, 1, "kretprobe_test3_result");
-	ASSERT_EQ(skel->bss->kretprobe_test4_result, 1, "kretprobe_test4_result");
-	ASSERT_EQ(skel->bss->kretprobe_test5_result, 1, "kretprobe_test5_result");
-	ASSERT_EQ(skel->bss->kretprobe_test6_result, 1, "kretprobe_test6_result");
-	ASSERT_EQ(skel->bss->kretprobe_test7_result, 1, "kretprobe_test7_result");
-	ASSERT_EQ(skel->bss->kretprobe_test8_result, 1, "kretprobe_test8_result");
+	if (test_return) {
+		ASSERT_EQ(skel->bss->kretprobe_test1_result, 1, "kretprobe_test1_result");
+		ASSERT_EQ(skel->bss->kretprobe_test2_result, 1, "kretprobe_test2_result");
+		ASSERT_EQ(skel->bss->kretprobe_test3_result, 1, "kretprobe_test3_result");
+		ASSERT_EQ(skel->bss->kretprobe_test4_result, 1, "kretprobe_test4_result");
+		ASSERT_EQ(skel->bss->kretprobe_test5_result, 1, "kretprobe_test5_result");
+		ASSERT_EQ(skel->bss->kretprobe_test6_result, 1, "kretprobe_test6_result");
+		ASSERT_EQ(skel->bss->kretprobe_test7_result, 1, "kretprobe_test7_result");
+		ASSERT_EQ(skel->bss->kretprobe_test8_result, 1, "kretprobe_test8_result");
+	}
 }
 
 static void test_skel_api(void)
@@ -46,7 +48,7 @@ static void test_skel_api(void)
 	if (!ASSERT_OK(err, "kprobe_multi__attach"))
 		goto cleanup;
 
-	kprobe_multi_test_run(skel);
+	kprobe_multi_test_run(skel, true);
 
 cleanup:
 	kprobe_multi__destroy(skel);
@@ -73,7 +75,7 @@ static void test_link_api(struct bpf_link_create_opts *opts)
 	if (!ASSERT_GE(link2_fd, 0, "link_fd"))
 		goto cleanup;
 
-	kprobe_multi_test_run(skel);
+	kprobe_multi_test_run(skel, true);
 
 cleanup:
 	if (link1_fd != -1)
@@ -127,6 +129,178 @@ static void test_link_api_syms(void)
 	test_link_api(&opts);
 }
 
+static void
+test_attach_api(const char *pattern, struct bpf_kprobe_multi_opts *opts)
+{
+	struct bpf_link *link1 = NULL, *link2 = NULL;
+	struct kprobe_multi *skel = NULL;
+
+	skel = kprobe_multi__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "fentry_raw_skel_load"))
+		goto cleanup;
+
+	skel->bss->pid = getpid();
+	link1 = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe,
+						      pattern, opts);
+	if (!ASSERT_OK_PTR(link1, "bpf_program__attach_kprobe_multi_opts"))
+		goto cleanup;
+
+	if (opts) {
+		opts->retprobe = true;
+		link2 = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kretprobe,
+							      pattern, opts);
+		if (!ASSERT_OK_PTR(link2, "bpf_program__attach_kprobe_multi_opts"))
+			goto cleanup;
+	}
+
+	kprobe_multi_test_run(skel, !!opts);
+
+cleanup:
+	bpf_link__destroy(link2);
+	bpf_link__destroy(link1);
+	kprobe_multi__destroy(skel);
+}
+
+static void test_attach_api_pattern(void)
+{
+	LIBBPF_OPTS(bpf_kprobe_multi_opts, opts);
+
+	test_attach_api("bpf_fentry_test*", &opts);
+	test_attach_api("bpf_fentry_test?", NULL);
+}
+
+static void test_attach_api_addrs(void)
+{
+	LIBBPF_OPTS(bpf_kprobe_multi_opts, opts);
+	unsigned long long addrs[8];
+
+	GET_ADDR("bpf_fentry_test1", addrs[0]);
+	GET_ADDR("bpf_fentry_test2", addrs[1]);
+	GET_ADDR("bpf_fentry_test3", addrs[2]);
+	GET_ADDR("bpf_fentry_test4", addrs[3]);
+	GET_ADDR("bpf_fentry_test5", addrs[4]);
+	GET_ADDR("bpf_fentry_test6", addrs[5]);
+	GET_ADDR("bpf_fentry_test7", addrs[6]);
+	GET_ADDR("bpf_fentry_test8", addrs[7]);
+
+	opts.addrs = (const unsigned long *) addrs;
+	opts.cnt = ARRAY_SIZE(addrs);
+	test_attach_api(NULL, &opts);
+}
+
+static void test_attach_api_syms(void)
+{
+	LIBBPF_OPTS(bpf_kprobe_multi_opts, opts);
+	const char *syms[8] = {
+		"bpf_fentry_test1",
+		"bpf_fentry_test2",
+		"bpf_fentry_test3",
+		"bpf_fentry_test4",
+		"bpf_fentry_test5",
+		"bpf_fentry_test6",
+		"bpf_fentry_test7",
+		"bpf_fentry_test8",
+	};
+
+	opts.syms = syms;
+	opts.cnt = ARRAY_SIZE(syms);
+	test_attach_api(NULL, &opts);
+}
+
+static void test_attach_api_fails(void)
+{
+	LIBBPF_OPTS(bpf_kprobe_multi_opts, opts);
+	struct kprobe_multi *skel = NULL;
+	struct bpf_link *link = NULL;
+	unsigned long long addrs[2];
+	const char *syms[2] = {
+		"bpf_fentry_test1",
+		"bpf_fentry_test2",
+	};
+	__u64 cookies[2];
+
+	addrs[0] = ksym_get_addr("bpf_fentry_test1");
+	addrs[1] = ksym_get_addr("bpf_fentry_test2");
+
+	if (!ASSERT_FALSE(!addrs[0] || !addrs[1], "ksym_get_addr"))
+		goto cleanup;
+
+	skel = kprobe_multi__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "fentry_raw_skel_load"))
+		goto cleanup;
+
+	skel->bss->pid = getpid();
+
+	/* fail_1 - pattern and opts NULL */
+	link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe,
+						     NULL, NULL);
+	if (!ASSERT_ERR_PTR(link, "fail_1"))
+		goto cleanup;
+
+	if (!ASSERT_EQ(libbpf_get_error(link), -EINVAL, "fail_1_error"))
+		goto cleanup;
+
+	/* fail_2 - both addrs and syms set */
+	opts.addrs = (const unsigned long *) addrs;
+	opts.syms = syms;
+	opts.cnt = ARRAY_SIZE(syms);
+	opts.cookies = NULL;
+
+	link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe,
+						     NULL, &opts);
+	if (!ASSERT_ERR_PTR(link, "fail_2"))
+		goto cleanup;
+
+	if (!ASSERT_EQ(libbpf_get_error(link), -EINVAL, "fail_2_error"))
+		goto cleanup;
+
+	/* fail_3 - pattern and addrs set */
+	opts.addrs = (const unsigned long *) addrs;
+	opts.syms = NULL;
+	opts.cnt = ARRAY_SIZE(syms);
+	opts.cookies = NULL;
+
+	link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe,
+						     "ksys_*", &opts);
+	if (!ASSERT_ERR_PTR(link, "fail_3"))
+		goto cleanup;
+
+	if (!ASSERT_EQ(libbpf_get_error(link), -EINVAL, "fail_3_error"))
+		goto cleanup;
+
+	/* fail_4 - pattern and cnt set */
+	opts.addrs = NULL;
+	opts.syms = NULL;
+	opts.cnt = ARRAY_SIZE(syms);
+	opts.cookies = NULL;
+
+	link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe,
+						     "ksys_*", &opts);
+	if (!ASSERT_ERR_PTR(link, "fail_4"))
+		goto cleanup;
+
+	if (!ASSERT_EQ(libbpf_get_error(link), -EINVAL, "fail_4_error"))
+		goto cleanup;
+
+	/* fail_5 - pattern and cookies */
+	opts.addrs = NULL;
+	opts.syms = NULL;
+	opts.cnt = 0;
+	opts.cookies = cookies;
+
+	link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe,
+						     "ksys_*", &opts);
+	if (!ASSERT_ERR_PTR(link, "fail_5"))
+		goto cleanup;
+
+	if (!ASSERT_EQ(libbpf_get_error(link), -EINVAL, "fail_5_error"))
+		goto cleanup;
+
+cleanup:
+	bpf_link__destroy(link);
+	kprobe_multi__destroy(skel);
+}
+
 void test_kprobe_multi_test(void)
 {
 	if (!ASSERT_OK(load_kallsyms(), "load_kallsyms"))
@@ -138,4 +312,12 @@ void test_kprobe_multi_test(void)
 		test_link_api_syms();
 	if (test__start_subtest("link_api_syms"))
 		test_link_api_addrs();
+	if (test__start_subtest("attach_api_pattern"))
+		test_attach_api_pattern();
+	if (test__start_subtest("attach_api_addrs"))
+		test_attach_api_addrs();
+	if (test__start_subtest("attach_api_syms"))
+		test_attach_api_syms();
+	if (test__start_subtest("attach_api_fails"))
+		test_attach_api_fails();
 }
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCHv3 bpf-next 13/13] selftests/bpf: Add cookie test for bpf_program__attach_kprobe_multi_opts
  2022-03-16 12:24 [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link Jiri Olsa
                   ` (11 preceding siblings ...)
  2022-03-16 12:24 ` [PATCHv3 bpf-next 12/13] selftests/bpf: Add attach test for bpf_program__attach_kprobe_multi_opts Jiri Olsa
@ 2022-03-16 12:24 ` Jiri Olsa
  2022-03-18  3:50 ` [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link patchwork-bot+netdevbpf
  2022-03-19  5:50 ` Andrii Nakryiko
  14 siblings, 0 replies; 26+ messages in thread
From: Jiri Olsa @ 2022-03-16 12:24 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Masami Hiramatsu
  Cc: netdev, bpf, lkml, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Steven Rostedt

Adding bpf_cookie test for programs attached by
bpf_program__attach_kprobe_multi_opts API.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 .../selftests/bpf/prog_tests/bpf_cookie.c     | 68 +++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
index 6671d4dc0b5d..923a6139b2d8 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
@@ -170,6 +170,72 @@ static void kprobe_multi_link_api_subtest(void)
 	kprobe_multi__destroy(skel);
 }
 
+static void kprobe_multi_attach_api_subtest(void)
+{
+	struct bpf_link *link1 = NULL, *link2 = NULL;
+	LIBBPF_OPTS(bpf_kprobe_multi_opts, opts);
+	LIBBPF_OPTS(bpf_test_run_opts, topts);
+	struct kprobe_multi *skel = NULL;
+	const char *syms[8] = {
+		"bpf_fentry_test1",
+		"bpf_fentry_test2",
+		"bpf_fentry_test3",
+		"bpf_fentry_test4",
+		"bpf_fentry_test5",
+		"bpf_fentry_test6",
+		"bpf_fentry_test7",
+		"bpf_fentry_test8",
+	};
+	__u64 cookies[8];
+
+	skel = kprobe_multi__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "fentry_raw_skel_load"))
+		goto cleanup;
+
+	skel->bss->pid = getpid();
+	skel->bss->test_cookie = true;
+
+	cookies[0] = 1;
+	cookies[1] = 2;
+	cookies[2] = 3;
+	cookies[3] = 4;
+	cookies[4] = 5;
+	cookies[5] = 6;
+	cookies[6] = 7;
+	cookies[7] = 8;
+
+	opts.syms = syms;
+	opts.cnt = ARRAY_SIZE(syms);
+	opts.cookies = cookies;
+
+	link1 = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe,
+						      NULL, &opts);
+	if (!ASSERT_OK_PTR(link1, "bpf_program__attach_kprobe_multi_opts"))
+		goto cleanup;
+
+	cookies[0] = 8;
+	cookies[1] = 7;
+	cookies[2] = 6;
+	cookies[3] = 5;
+	cookies[4] = 4;
+	cookies[5] = 3;
+	cookies[6] = 2;
+	cookies[7] = 1;
+
+	opts.retprobe = true;
+
+	link2 = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kretprobe,
+						      NULL, &opts);
+	if (!ASSERT_OK_PTR(link2, "bpf_program__attach_kprobe_multi_opts"))
+		goto cleanup;
+
+	kprobe_multi_test_run(skel);
+
+cleanup:
+	bpf_link__destroy(link2);
+	bpf_link__destroy(link1);
+	kprobe_multi__destroy(skel);
+}
 static void uprobe_subtest(struct test_bpf_cookie *skel)
 {
 	DECLARE_LIBBPF_OPTS(bpf_uprobe_opts, opts);
@@ -358,6 +424,8 @@ void test_bpf_cookie(void)
 		kprobe_subtest(skel);
 	if (test__start_subtest("multi_kprobe_link_api"))
 		kprobe_multi_link_api_subtest();
+	if (test__start_subtest("multi_kprobe_attach_api"))
+		kprobe_multi_attach_api_subtest();
 	if (test__start_subtest("uprobe"))
 		uprobe_subtest(skel);
 	if (test__start_subtest("tracepoint"))
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 bpf-next 03/13] bpf: Add multi kprobe link
  2022-03-16 12:24 ` [PATCHv3 bpf-next 03/13] bpf: Add multi kprobe link Jiri Olsa
@ 2022-03-16 18:53   ` Alexei Starovoitov
  2022-03-17 13:24     ` Jiri Olsa
  0 siblings, 1 reply; 26+ messages in thread
From: Alexei Starovoitov @ 2022-03-16 18:53 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Masami Hiramatsu, netdev, bpf, lkml, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Steven Rostedt

On Wed, Mar 16, 2022 at 01:24:09PM +0100, Jiri Olsa wrote:
> +static int
> +kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
> +			   struct pt_regs *regs)
> +{
> +	int err;
> +
> +	if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) {
> +		err = 0;
> +		goto out;
> +	}

That's a heavy hammer.
It's ok-ish for now, but would be good to switch to
__this_cpu_inc_return(*(prog->active)) at some point.
The way fentry/fexit progs do.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 bpf-next 03/13] bpf: Add multi kprobe link
  2022-03-16 18:53   ` Alexei Starovoitov
@ 2022-03-17 13:24     ` Jiri Olsa
  0 siblings, 0 replies; 26+ messages in thread
From: Jiri Olsa @ 2022-03-17 13:24 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Masami Hiramatsu, netdev, bpf, lkml, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Steven Rostedt

On Wed, Mar 16, 2022 at 11:53:33AM -0700, Alexei Starovoitov wrote:
> On Wed, Mar 16, 2022 at 01:24:09PM +0100, Jiri Olsa wrote:
> > +static int
> > +kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
> > +			   struct pt_regs *regs)
> > +{
> > +	int err;
> > +
> > +	if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) {
> > +		err = 0;
> > +		goto out;
> > +	}
> 
> That's a heavy hammer.
> It's ok-ish for now, but would be good to switch to
> __this_cpu_inc_return(*(prog->active)) at some point.
> The way fentry/fexit progs do.

ok, will check

thanks,
jirka

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link
  2022-03-16 12:24 [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link Jiri Olsa
                   ` (12 preceding siblings ...)
  2022-03-16 12:24 ` [PATCHv3 bpf-next 13/13] selftests/bpf: Add cookie " Jiri Olsa
@ 2022-03-18  3:50 ` patchwork-bot+netdevbpf
  2022-03-19  5:50 ` Andrii Nakryiko
  14 siblings, 0 replies; 26+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-03-18  3:50 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: ast, daniel, andrii, mhiramat, netdev, bpf, linux-kernel, kafai,
	songliubraving, yhs, john.fastabend, kpsingh, rostedt

Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Wed, 16 Mar 2022 13:24:06 +0100 you wrote:
> hi,
> this patchset adds new link type BPF_TRACE_KPROBE_MULTI that attaches
> kprobe program through fprobe API [1] instroduced by Masami.
> 
> The fprobe API allows to attach probe on multiple functions at once very
> fast, because it works on top of ftrace. On the other hand this limits
> the probe point to the function entry or return.
> 
> [...]

Here is the summary with links:
  - [PATCHv3,bpf-next,01/13] lib/sort: Add priv pointer to swap function
    https://git.kernel.org/bpf/bpf-next/c/a0019cd7d41a
  - [PATCHv3,bpf-next,02/13] kallsyms: Skip the name search for empty string
    https://git.kernel.org/bpf/bpf-next/c/aecf489f2ce5
  - [PATCHv3,bpf-next,03/13] bpf: Add multi kprobe link
    https://git.kernel.org/bpf/bpf-next/c/0dcac2725406
  - [PATCHv3,bpf-next,04/13] bpf: Add bpf_get_func_ip kprobe helper for multi kprobe link
    https://git.kernel.org/bpf/bpf-next/c/42a5712094e8
  - [PATCHv3,bpf-next,05/13] bpf: Add support to inline bpf_get_func_ip helper on x86
    https://git.kernel.org/bpf/bpf-next/c/97ee4d20ee67
  - [PATCHv3,bpf-next,06/13] bpf: Add cookie support to programs attached with kprobe multi link
    https://git.kernel.org/bpf/bpf-next/c/ca74823c6e16
  - [PATCHv3,bpf-next,07/13] libbpf: Add libbpf_kallsyms_parse function
    https://git.kernel.org/bpf/bpf-next/c/85153ac06283
  - [PATCHv3,bpf-next,08/13] libbpf: Add bpf_link_create support for multi kprobes
    https://git.kernel.org/bpf/bpf-next/c/5117c26e8773
  - [PATCHv3,bpf-next,09/13] libbpf: Add bpf_program__attach_kprobe_multi_opts function
    https://git.kernel.org/bpf/bpf-next/c/ddc6b04989eb
  - [PATCHv3,bpf-next,10/13] selftests/bpf: Add kprobe_multi attach test
    https://git.kernel.org/bpf/bpf-next/c/f7a11eeccb11
  - [PATCHv3,bpf-next,11/13] selftests/bpf: Add kprobe_multi bpf_cookie test
    https://git.kernel.org/bpf/bpf-next/c/2c6401c966ae
  - [PATCHv3,bpf-next,12/13] selftests/bpf: Add attach test for bpf_program__attach_kprobe_multi_opts
    https://git.kernel.org/bpf/bpf-next/c/9271a0c7ae7a
  - [PATCHv3,bpf-next,13/13] selftests/bpf: Add cookie test for bpf_program__attach_kprobe_multi_opts
    https://git.kernel.org/bpf/bpf-next/c/318c812cebfc

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 bpf-next 09/13] libbpf: Add bpf_program__attach_kprobe_multi_opts function
  2022-03-16 12:24 ` [PATCHv3 bpf-next 09/13] libbpf: Add bpf_program__attach_kprobe_multi_opts function Jiri Olsa
@ 2022-03-18  3:53   ` Alexei Starovoitov
  2022-03-18  5:14     ` Andrii Nakryiko
  2022-03-18  9:08     ` Jiri Olsa
  0 siblings, 2 replies; 26+ messages in thread
From: Alexei Starovoitov @ 2022-03-18  3:53 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Masami Hiramatsu, Network Development, bpf, lkml,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Steven Rostedt

On Wed, Mar 16, 2022 at 5:26 AM Jiri Olsa <jolsa@kernel.org> wrote:
> +
> +struct bpf_link *
> +bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> +                                     const char *pattern,
> +                                     const struct bpf_kprobe_multi_opts *opts)
> +{
> +       LIBBPF_OPTS(bpf_link_create_opts, lopts);
> +       struct kprobe_multi_resolve res = {
> +               .pattern = pattern,
> +       };
> +       struct bpf_link *link = NULL;
> +       char errmsg[STRERR_BUFSIZE];
> +       const unsigned long *addrs;
> +       int err, link_fd, prog_fd;
> +       const __u64 *cookies;
> +       const char **syms;
> +       bool retprobe;
> +       size_t cnt;
> +
> +       if (!OPTS_VALID(opts, bpf_kprobe_multi_opts))
> +               return libbpf_err_ptr(-EINVAL);
> +
> +       syms    = OPTS_GET(opts, syms, false);
> +       addrs   = OPTS_GET(opts, addrs, false);
> +       cnt     = OPTS_GET(opts, cnt, false);
> +       cookies = OPTS_GET(opts, cookies, false);
> +
> +       if (!pattern && !addrs && !syms)
> +               return libbpf_err_ptr(-EINVAL);
> +       if (pattern && (addrs || syms || cookies || cnt))
> +               return libbpf_err_ptr(-EINVAL);
> +       if (!pattern && !cnt)
> +               return libbpf_err_ptr(-EINVAL);
> +       if (addrs && syms)
> +               return libbpf_err_ptr(-EINVAL);
> +
> +       if (pattern) {
> +               err = libbpf_kallsyms_parse(resolve_kprobe_multi_cb, &res);
> +               if (err)
> +                       goto error;
> +               if (!res.cnt) {
> +                       err = -ENOENT;
> +                       goto error;
> +               }
> +               addrs = res.addrs;
> +               cnt = res.cnt;
> +       }

Thanks Jiri.
Great stuff and a major milestone!
I've applied Masami's and your patches to bpf-next.

But the above needs more work.
Currently test_progs -t kprobe_multi
takes 4 seconds on lockdep+debug kernel.
Mainly because of the above loop.

    18.05%  test_progs       [kernel.kallsyms]   [k]
kallsyms_expand_symbol.constprop.4
    12.53%  test_progs       libc-2.28.so        [.] _IO_vfscanf
     6.31%  test_progs       [kernel.kallsyms]   [k] number
     4.66%  test_progs       [kernel.kallsyms]   [k] format_decode
     4.65%  test_progs       [kernel.kallsyms]   [k] string_nocheck

Single test_skel_api() subtest takes almost a second.

A cache inside libbpf probably won't help.
Maybe introduce a bpf iterator for kallsyms?

On the kernel side kprobe_multi_resolve_syms() looks similarly inefficient.
I'm not sure whether it would be a bottle neck though.

Orthogonal to this issue please add a new stress test
to selftest/bpf that attaches to a lot of functions.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 bpf-next 09/13] libbpf: Add bpf_program__attach_kprobe_multi_opts function
  2022-03-18  3:53   ` Alexei Starovoitov
@ 2022-03-18  5:14     ` Andrii Nakryiko
  2022-03-18  9:22       ` Jiri Olsa
  2022-03-18  9:08     ` Jiri Olsa
  1 sibling, 1 reply; 26+ messages in thread
From: Andrii Nakryiko @ 2022-03-18  5:14 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Masami Hiramatsu, Network Development, bpf, lkml,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Steven Rostedt

On Thu, Mar 17, 2022 at 8:53 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Mar 16, 2022 at 5:26 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > +
> > +struct bpf_link *
> > +bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> > +                                     const char *pattern,
> > +                                     const struct bpf_kprobe_multi_opts *opts)
> > +{
> > +       LIBBPF_OPTS(bpf_link_create_opts, lopts);
> > +       struct kprobe_multi_resolve res = {
> > +               .pattern = pattern,
> > +       };
> > +       struct bpf_link *link = NULL;
> > +       char errmsg[STRERR_BUFSIZE];
> > +       const unsigned long *addrs;
> > +       int err, link_fd, prog_fd;
> > +       const __u64 *cookies;
> > +       const char **syms;
> > +       bool retprobe;
> > +       size_t cnt;
> > +
> > +       if (!OPTS_VALID(opts, bpf_kprobe_multi_opts))
> > +               return libbpf_err_ptr(-EINVAL);
> > +
> > +       syms    = OPTS_GET(opts, syms, false);
> > +       addrs   = OPTS_GET(opts, addrs, false);
> > +       cnt     = OPTS_GET(opts, cnt, false);
> > +       cookies = OPTS_GET(opts, cookies, false);
> > +
> > +       if (!pattern && !addrs && !syms)
> > +               return libbpf_err_ptr(-EINVAL);
> > +       if (pattern && (addrs || syms || cookies || cnt))
> > +               return libbpf_err_ptr(-EINVAL);
> > +       if (!pattern && !cnt)
> > +               return libbpf_err_ptr(-EINVAL);
> > +       if (addrs && syms)
> > +               return libbpf_err_ptr(-EINVAL);
> > +
> > +       if (pattern) {
> > +               err = libbpf_kallsyms_parse(resolve_kprobe_multi_cb, &res);
> > +               if (err)
> > +                       goto error;
> > +               if (!res.cnt) {
> > +                       err = -ENOENT;
> > +                       goto error;
> > +               }
> > +               addrs = res.addrs;
> > +               cnt = res.cnt;
> > +       }
>
> Thanks Jiri.
> Great stuff and a major milestone!
> I've applied Masami's and your patches to bpf-next.
>
> But the above needs more work.
> Currently test_progs -t kprobe_multi
> takes 4 seconds on lockdep+debug kernel.
> Mainly because of the above loop.
>
>     18.05%  test_progs       [kernel.kallsyms]   [k]
> kallsyms_expand_symbol.constprop.4
>     12.53%  test_progs       libc-2.28.so        [.] _IO_vfscanf
>      6.31%  test_progs       [kernel.kallsyms]   [k] number
>      4.66%  test_progs       [kernel.kallsyms]   [k] format_decode
>      4.65%  test_progs       [kernel.kallsyms]   [k] string_nocheck
>
> Single test_skel_api() subtest takes almost a second.
>
> A cache inside libbpf probably won't help.
> Maybe introduce a bpf iterator for kallsyms?

BPF iterator for kallsyms is a great idea! So many benefits:
  - it should be significantly more efficient *and* simpler than
parsing /proc/kallsyms;
  - there were some upstream patches recording ksym length (i.e.,
function size), don't remember if that ever landed or not, but besides
that the other complication of even exposing that to user space were
concerns about /proc/kallsyms format being an ABI. With the BPF
iterator we can easily provide that symbol size without any breakage.
This would be great!
  - we can allow parameterizing iterator with options like: skip or
include module symbols, specify a set of types of symbols (function,
variable, etc), etc. This would speed everything up in common cases by
not even decompressing irrelevant names.

In short, kallsyms iterator would be an immensely useful for any sort
of tracing tool that deals with kernel stack traces or kallsyms in
general.

But in this particular case, kprobe_multi_resolve_syms()
implementation is extremely suboptimal. I didn't realize during review
that kallsyms_lookup_name() is a linear scan... If that's not going to
be changed to O(log(N)) some time soon, we need to reimplement
kprobe_multi_resolve_syms(), probably.

One way would be to sort user strings lexicographically and then do a
linear scan over all kallsyms, for each symbol perform binary search
over a sorted array of user strings. Stop once all the positions were
"filled in" (we'd need to keep a bitmap or bool[], probably). This way
it's going to be O(MlogN) instead of O(MN) as it is right now.

BTW, Jiri, libbpf.map is supposed to have an alphabetically ordered
list of functions, it would be good to move
bpf_program__attach_kprobe_multi_opts a bit higher before libbpf_*
functions.



>
> On the kernel side kprobe_multi_resolve_syms() looks similarly inefficient.
> I'm not sure whether it would be a bottle neck though.
>
> Orthogonal to this issue please add a new stress test
> to selftest/bpf that attaches to a lot of functions.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 bpf-next 09/13] libbpf: Add bpf_program__attach_kprobe_multi_opts function
  2022-03-18  3:53   ` Alexei Starovoitov
  2022-03-18  5:14     ` Andrii Nakryiko
@ 2022-03-18  9:08     ` Jiri Olsa
  1 sibling, 0 replies; 26+ messages in thread
From: Jiri Olsa @ 2022-03-18  9:08 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Masami Hiramatsu, Network Development, bpf, lkml,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Steven Rostedt

On Thu, Mar 17, 2022 at 08:53:15PM -0700, Alexei Starovoitov wrote:
> On Wed, Mar 16, 2022 at 5:26 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > +
> > +struct bpf_link *
> > +bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> > +                                     const char *pattern,
> > +                                     const struct bpf_kprobe_multi_opts *opts)
> > +{
> > +       LIBBPF_OPTS(bpf_link_create_opts, lopts);
> > +       struct kprobe_multi_resolve res = {
> > +               .pattern = pattern,
> > +       };
> > +       struct bpf_link *link = NULL;
> > +       char errmsg[STRERR_BUFSIZE];
> > +       const unsigned long *addrs;
> > +       int err, link_fd, prog_fd;
> > +       const __u64 *cookies;
> > +       const char **syms;
> > +       bool retprobe;
> > +       size_t cnt;
> > +
> > +       if (!OPTS_VALID(opts, bpf_kprobe_multi_opts))
> > +               return libbpf_err_ptr(-EINVAL);
> > +
> > +       syms    = OPTS_GET(opts, syms, false);
> > +       addrs   = OPTS_GET(opts, addrs, false);
> > +       cnt     = OPTS_GET(opts, cnt, false);
> > +       cookies = OPTS_GET(opts, cookies, false);
> > +
> > +       if (!pattern && !addrs && !syms)
> > +               return libbpf_err_ptr(-EINVAL);
> > +       if (pattern && (addrs || syms || cookies || cnt))
> > +               return libbpf_err_ptr(-EINVAL);
> > +       if (!pattern && !cnt)
> > +               return libbpf_err_ptr(-EINVAL);
> > +       if (addrs && syms)
> > +               return libbpf_err_ptr(-EINVAL);
> > +
> > +       if (pattern) {
> > +               err = libbpf_kallsyms_parse(resolve_kprobe_multi_cb, &res);
> > +               if (err)
> > +                       goto error;
> > +               if (!res.cnt) {
> > +                       err = -ENOENT;
> > +                       goto error;
> > +               }
> > +               addrs = res.addrs;
> > +               cnt = res.cnt;
> > +       }
> 
> Thanks Jiri.
> Great stuff and a major milestone!
> I've applied Masami's and your patches to bpf-next.

great, thanks

> 
> But the above needs more work.
> Currently test_progs -t kprobe_multi
> takes 4 seconds on lockdep+debug kernel.
> Mainly because of the above loop.
> 
>     18.05%  test_progs       [kernel.kallsyms]   [k]
> kallsyms_expand_symbol.constprop.4
>     12.53%  test_progs       libc-2.28.so        [.] _IO_vfscanf
>      6.31%  test_progs       [kernel.kallsyms]   [k] number
>      4.66%  test_progs       [kernel.kallsyms]   [k] format_decode
>      4.65%  test_progs       [kernel.kallsyms]   [k] string_nocheck
> 
> Single test_skel_api() subtest takes almost a second.

hm, I'll check on lockdep+debug kernel.. I think this test
should be going through kallsyms just once, will check

anyway libbpf_kallsyms_parse could use ksym_get_addr, which is
now cached

> 
> A cache inside libbpf probably won't help.
> Maybe introduce a bpf iterator for kallsyms?
> 
> On the kernel side kprobe_multi_resolve_syms() looks similarly inefficient.
> I'm not sure whether it would be a bottle neck though.
> 
> Orthogonal to this issue please add a new stress test
> to selftest/bpf that attaches to a lot of functions.

ok, will add that

thanks,
jirka

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 bpf-next 09/13] libbpf: Add bpf_program__attach_kprobe_multi_opts function
  2022-03-18  5:14     ` Andrii Nakryiko
@ 2022-03-18  9:22       ` Jiri Olsa
  0 siblings, 0 replies; 26+ messages in thread
From: Jiri Olsa @ 2022-03-18  9:22 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Alexei Starovoitov, Jiri Olsa, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Masami Hiramatsu,
	Network Development, bpf, lkml, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Steven Rostedt,
	Nick Alcock, Arnaldo Carvalho de Melo

On Thu, Mar 17, 2022 at 10:14:28PM -0700, Andrii Nakryiko wrote:

SNIP

> > But the above needs more work.
> > Currently test_progs -t kprobe_multi
> > takes 4 seconds on lockdep+debug kernel.
> > Mainly because of the above loop.
> >
> >     18.05%  test_progs       [kernel.kallsyms]   [k]
> > kallsyms_expand_symbol.constprop.4
> >     12.53%  test_progs       libc-2.28.so        [.] _IO_vfscanf
> >      6.31%  test_progs       [kernel.kallsyms]   [k] number
> >      4.66%  test_progs       [kernel.kallsyms]   [k] format_decode
> >      4.65%  test_progs       [kernel.kallsyms]   [k] string_nocheck
> >
> > Single test_skel_api() subtest takes almost a second.
> >
> > A cache inside libbpf probably won't help.
> > Maybe introduce a bpf iterator for kallsyms?
> 
> BPF iterator for kallsyms is a great idea! So many benefits:

>   - it should be significantly more efficient *and* simpler than
> parsing /proc/kallsyms;
>   - there were some upstream patches recording ksym length (i.e.,
> function size), don't remember if that ever landed or not, but besides
> that the other complication of even exposing that to user space were
> concerns about /proc/kallsyms format being an ABI. With the BPF
> iterator we can easily provide that symbol size without any breakage.
> This would be great!

yes, great idea.. I was cc-ed on patches adding extra stuff to kallsyms:
  https://lore.kernel.org/lkml/20220208184309.148192-7-nick.alcock@oracle.com/

this could be way out ;-) cc-ing Nick

>   - we can allow parameterizing iterator with options like: skip or
> include module symbols, specify a set of types of symbols (function,
> variable, etc), etc. This would speed everything up in common cases by
> not even decompressing irrelevant names.
> 
> In short, kallsyms iterator would be an immensely useful for any sort
> of tracing tool that deals with kernel stack traces or kallsyms in
> general.

I wonder we could make some use of it in perf as well, there's some
guessing wrt symbol sizes when we parse kallsyms, so we could get
rid of it.. I will work on that and try to add this

> 
> But in this particular case, kprobe_multi_resolve_syms()
> implementation is extremely suboptimal. I didn't realize during review
> that kallsyms_lookup_name() is a linear scan... If that's not going to
> be changed to O(log(N)) some time soon, we need to reimplement
> kprobe_multi_resolve_syms(), probably.
> 
> One way would be to sort user strings lexicographically and then do a
> linear scan over all kallsyms, for each symbol perform binary search
> over a sorted array of user strings. Stop once all the positions were
> "filled in" (we'd need to keep a bitmap or bool[], probably). This way
> it's going to be O(MlogN) instead of O(MN) as it is right now.

ok, I did something similar in multi-trampoline patchset that you
suggested, I think that will work here as well

> 
> BTW, Jiri, libbpf.map is supposed to have an alphabetically ordered
> list of functions, it would be good to move
> bpf_program__attach_kprobe_multi_opts a bit higher before libbpf_*
> functions.

ah right, sry.. I'll send fix with follow up changes

thanks,
jirka

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link
  2022-03-16 12:24 [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link Jiri Olsa
                   ` (13 preceding siblings ...)
  2022-03-18  3:50 ` [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link patchwork-bot+netdevbpf
@ 2022-03-19  5:50 ` Andrii Nakryiko
  2022-03-19 12:27   ` Jiri Olsa
  14 siblings, 1 reply; 26+ messages in thread
From: Andrii Nakryiko @ 2022-03-19  5:50 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Masami Hiramatsu, Networking, bpf, lkml, Martin KaFai Lau,
	Song Liu, Yonghong Song, John Fastabend, KP Singh,
	Steven Rostedt

On Wed, Mar 16, 2022 at 5:24 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> hi,
> this patchset adds new link type BPF_TRACE_KPROBE_MULTI that attaches
> kprobe program through fprobe API [1] instroduced by Masami.
>
> The fprobe API allows to attach probe on multiple functions at once very
> fast, because it works on top of ftrace. On the other hand this limits
> the probe point to the function entry or return.
>
>
> With bpftrace support I see following attach speed:
>
>   # perf stat --null -r 5 ./src/bpftrace -e 'kprobe:x* { } i:ms:1 { exit(); } '
>   Attaching 2 probes...
>   Attaching 3342 functions
>   ...
>
>   1.4960 +- 0.0285 seconds time elapsed  ( +-  1.91% )
>
> v3 changes:
>   - based on latest fprobe post from Masami [2]
>   - add acks
>   - add extra comment to kprobe_multi_link_handler wrt entry ip setup [Masami]
>   - keep swap_words_64 static and swap values directly in
>     bpf_kprobe_multi_cookie_swap [Andrii]
>   - rearrange locking/migrate setup in kprobe_multi_link_prog_run [Andrii]
>   - move uapi fields [Andrii]
>   - add bpf_program__attach_kprobe_multi_opts function [Andrii]
>   - many small test changes [Andrii]
>   - added tests for bpf_program__attach_kprobe_multi_opts
>   - make kallsyms_lookup_name check for empty string [Andrii]
>
> v2 changes:
>   - based on latest fprobe changes [1]
>   - renaming the uapi interface to kprobe multi
>   - adding support for sort_r to pass user pointer for swap functions
>     and using that in cookie support to keep just single functions array
>   - moving new link to kernel/trace/bpf_trace.c file
>   - using single fprobe callback function for entry and exit
>   - using kvzalloc, libbpf_ensure_mem functions
>   - adding new k[ret]probe.multi sections instead of using current kprobe
>   - used glob_match from test_progs.c, added '?' matching
>   - move bpf_get_func_ip verifier inline change to seprate change
>   - couple of other minor fixes
>
>
> Also available at:
>   https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
>   bpf/kprobe_multi
>
> thanks,
> jirka
>
>
> [1] https://lore.kernel.org/bpf/164458044634.586276.3261555265565111183.stgit@devnote2/
> [2] https://lore.kernel.org/bpf/164735281449.1084943.12438881786173547153.stgit@devnote2/
> ---
> Jiri Olsa (13):
>       lib/sort: Add priv pointer to swap function
>       kallsyms: Skip the name search for empty string
>       bpf: Add multi kprobe link
>       bpf: Add bpf_get_func_ip kprobe helper for multi kprobe link
>       bpf: Add support to inline bpf_get_func_ip helper on x86
>       bpf: Add cookie support to programs attached with kprobe multi link
>       libbpf: Add libbpf_kallsyms_parse function
>       libbpf: Add bpf_link_create support for multi kprobes
>       libbpf: Add bpf_program__attach_kprobe_multi_opts function
>       selftests/bpf: Add kprobe_multi attach test
>       selftests/bpf: Add kprobe_multi bpf_cookie test
>       selftests/bpf: Add attach test for bpf_program__attach_kprobe_multi_opts
>       selftests/bpf: Add cookie test for bpf_program__attach_kprobe_multi_opts
>

Ok, so I've integrated multi-attach kprobes into retsnoop. It was
pretty straightforward. Here are some numbers for the speed of
attaching and, even more importantly, detaching for a set of almost
400 functions. It's a bit less functions for fentry-based mode due to
more limited BTF information for static functions. Note that retsnoop
attaches two BPF programs for each kernel function, so it's actually
two multi-attach kprobes, each attaching to 397 functions. For
single-attach kprobe, we perform 397 * 2 = 794 attachments.

I've been invoking retsnoop with the following specified set of
functions: -e '*sys_bpf*' -a ':kernel/bpf/syscall.c' -a
':kernel/bpf/verifier.c' -a ':kernel/bpf/btf.c' -a
':kernel/bpf/core.c'. So basically bpf syscall entry functions and all
the discoverable functions from syscall.c, verifier.c, core.c and
btf.c from kernel/bpf subdirectory.

Results:

fentry attach/detach (263 kfuncs): 2133 ms / 31671  ms (33 seconds)
kprobe attach/detach (397 kfuncs): 3121 ms / 13195 ms (16 seconds)
multi-kprobe attach/detach (397 kfuncs): 9 ms / 248 ms (0.25 seconds)

So as you can see, the speed up is tremendous! API is also very
convenient, I didn't have to modify retsnoop internals much to
accommodate multi-attach API. Great job!

Now for the bad news. :(

Stack traces from multi-attach kretprobe are broken, which makes all
this way less useful.

Below, see stack traces captured with multi- and single- kretprobes
for two different use cases. Single kprobe stack traces make much more
sense. Ignore that last function doesn't have actual address
associated with it (i.e. for __sys_bpf and bpf_tracing_prog_attach,
respectively). That's just how retsnoop is doing things, I think. We
actually were capturing stack traces from inside __sys_bpf (first
case) and bpf_tracing_prog_attach (second case).

MULTI KPROBE:
ffffffff81185a80 __sys_bpf+0x0
(kernel/bpf/syscall.c:4622:1)

SINGLE KPROBE:
ffffffff81e0007c entry_SYSCALL_64_after_hwframe+0x44
(arch/x86/entry/entry_64.S:113:0)
ffffffff81cd2b15 do_syscall_64+0x35
(arch/x86/entry/common.c:80:7)
                 . do_syscall_x64
(arch/x86/entry/common.c:50:12)
ffffffff811881aa __x64_sys_bpf+0x1a
(kernel/bpf/syscall.c:4765:1)
                 __sys_bpf


MULTI KPROBE:
ffffffff811851b0 bpf_tracing_prog_attach+0x0
(kernel/bpf/syscall.c:2708:1)

SINGLE KPROBE:
ffffffff81e0007c entry_SYSCALL_64_after_hwframe+0x44
(arch/x86/entry/entry_64.S:113:0)
ffffffff81cd2b15 do_syscall_64+0x35
(arch/x86/entry/common.c:80:7)
                 . do_syscall_x64
(arch/x86/entry/common.c:50:12)
ffffffff811881aa __x64_sys_bpf+0x1a
(kernel/bpf/syscall.c:4765:1)
ffffffff81185e79 __sys_bpf+0x3f9
(kernel/bpf/syscall.c:4705:9)
ffffffff8118583a bpf_raw_tracepoint_open+0x19a
(kernel/bpf/syscall.c:3069:6)
                 bpf_tracing_prog_attach

You can see that in multi-attach kprobe we only get one entry, which
is the very last function in the stack trace. We have no parent
function captured whatsoever. Jiri, Masami, any ideas what's wrong and
how to fix this? Let's try to figure this out and fix it before the
feature makes it into the kernel release. Thanks in advance!


[...]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link
  2022-03-19  5:50 ` Andrii Nakryiko
@ 2022-03-19 12:27   ` Jiri Olsa
  2022-03-19 14:31     ` Jiri Olsa
  0 siblings, 1 reply; 26+ messages in thread
From: Jiri Olsa @ 2022-03-19 12:27 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Masami Hiramatsu, Networking, bpf, lkml, Martin KaFai Lau,
	Song Liu, Yonghong Song, John Fastabend, KP Singh,
	Steven Rostedt

On Fri, Mar 18, 2022 at 10:50:46PM -0700, Andrii Nakryiko wrote:
> On Wed, Mar 16, 2022 at 5:24 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > hi,
> > this patchset adds new link type BPF_TRACE_KPROBE_MULTI that attaches
> > kprobe program through fprobe API [1] instroduced by Masami.
> >
> > The fprobe API allows to attach probe on multiple functions at once very
> > fast, because it works on top of ftrace. On the other hand this limits
> > the probe point to the function entry or return.
> >
> >
> > With bpftrace support I see following attach speed:
> >
> >   # perf stat --null -r 5 ./src/bpftrace -e 'kprobe:x* { } i:ms:1 { exit(); } '
> >   Attaching 2 probes...
> >   Attaching 3342 functions
> >   ...
> >
> >   1.4960 +- 0.0285 seconds time elapsed  ( +-  1.91% )
> >
> > v3 changes:
> >   - based on latest fprobe post from Masami [2]
> >   - add acks
> >   - add extra comment to kprobe_multi_link_handler wrt entry ip setup [Masami]
> >   - keep swap_words_64 static and swap values directly in
> >     bpf_kprobe_multi_cookie_swap [Andrii]
> >   - rearrange locking/migrate setup in kprobe_multi_link_prog_run [Andrii]
> >   - move uapi fields [Andrii]
> >   - add bpf_program__attach_kprobe_multi_opts function [Andrii]
> >   - many small test changes [Andrii]
> >   - added tests for bpf_program__attach_kprobe_multi_opts
> >   - make kallsyms_lookup_name check for empty string [Andrii]
> >
> > v2 changes:
> >   - based on latest fprobe changes [1]
> >   - renaming the uapi interface to kprobe multi
> >   - adding support for sort_r to pass user pointer for swap functions
> >     and using that in cookie support to keep just single functions array
> >   - moving new link to kernel/trace/bpf_trace.c file
> >   - using single fprobe callback function for entry and exit
> >   - using kvzalloc, libbpf_ensure_mem functions
> >   - adding new k[ret]probe.multi sections instead of using current kprobe
> >   - used glob_match from test_progs.c, added '?' matching
> >   - move bpf_get_func_ip verifier inline change to seprate change
> >   - couple of other minor fixes
> >
> >
> > Also available at:
> >   https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> >   bpf/kprobe_multi
> >
> > thanks,
> > jirka
> >
> >
> > [1] https://lore.kernel.org/bpf/164458044634.586276.3261555265565111183.stgit@devnote2/
> > [2] https://lore.kernel.org/bpf/164735281449.1084943.12438881786173547153.stgit@devnote2/
> > ---
> > Jiri Olsa (13):
> >       lib/sort: Add priv pointer to swap function
> >       kallsyms: Skip the name search for empty string
> >       bpf: Add multi kprobe link
> >       bpf: Add bpf_get_func_ip kprobe helper for multi kprobe link
> >       bpf: Add support to inline bpf_get_func_ip helper on x86
> >       bpf: Add cookie support to programs attached with kprobe multi link
> >       libbpf: Add libbpf_kallsyms_parse function
> >       libbpf: Add bpf_link_create support for multi kprobes
> >       libbpf: Add bpf_program__attach_kprobe_multi_opts function
> >       selftests/bpf: Add kprobe_multi attach test
> >       selftests/bpf: Add kprobe_multi bpf_cookie test
> >       selftests/bpf: Add attach test for bpf_program__attach_kprobe_multi_opts
> >       selftests/bpf: Add cookie test for bpf_program__attach_kprobe_multi_opts
> >
> 
> Ok, so I've integrated multi-attach kprobes into retsnoop. It was
> pretty straightforward. Here are some numbers for the speed of
> attaching and, even more importantly, detaching for a set of almost
> 400 functions. It's a bit less functions for fentry-based mode due to
> more limited BTF information for static functions. Note that retsnoop
> attaches two BPF programs for each kernel function, so it's actually
> two multi-attach kprobes, each attaching to 397 functions. For
> single-attach kprobe, we perform 397 * 2 = 794 attachments.
> 
> I've been invoking retsnoop with the following specified set of
> functions: -e '*sys_bpf*' -a ':kernel/bpf/syscall.c' -a
> ':kernel/bpf/verifier.c' -a ':kernel/bpf/btf.c' -a
> ':kernel/bpf/core.c'. So basically bpf syscall entry functions and all
> the discoverable functions from syscall.c, verifier.c, core.c and
> btf.c from kernel/bpf subdirectory.
> 
> Results:
> 
> fentry attach/detach (263 kfuncs): 2133 ms / 31671  ms (33 seconds)
> kprobe attach/detach (397 kfuncs): 3121 ms / 13195 ms (16 seconds)
> multi-kprobe attach/detach (397 kfuncs): 9 ms / 248 ms (0.25 seconds)
> 
> So as you can see, the speed up is tremendous! API is also very
> convenient, I didn't have to modify retsnoop internals much to
> accommodate multi-attach API. Great job!

nice! thanks for doing that so quickly

> 
> Now for the bad news. :(
> 
> Stack traces from multi-attach kretprobe are broken, which makes all
> this way less useful.
> 
> Below, see stack traces captured with multi- and single- kretprobes
> for two different use cases. Single kprobe stack traces make much more
> sense. Ignore that last function doesn't have actual address
> associated with it (i.e. for __sys_bpf and bpf_tracing_prog_attach,
> respectively). That's just how retsnoop is doing things, I think. We
> actually were capturing stack traces from inside __sys_bpf (first
> case) and bpf_tracing_prog_attach (second case).
> 
> MULTI KPROBE:
> ffffffff81185a80 __sys_bpf+0x0
> (kernel/bpf/syscall.c:4622:1)
> 
> SINGLE KPROBE:
> ffffffff81e0007c entry_SYSCALL_64_after_hwframe+0x44
> (arch/x86/entry/entry_64.S:113:0)
> ffffffff81cd2b15 do_syscall_64+0x35
> (arch/x86/entry/common.c:80:7)
>                  . do_syscall_x64
> (arch/x86/entry/common.c:50:12)
> ffffffff811881aa __x64_sys_bpf+0x1a
> (kernel/bpf/syscall.c:4765:1)
>                  __sys_bpf
> 
> 
> MULTI KPROBE:
> ffffffff811851b0 bpf_tracing_prog_attach+0x0
> (kernel/bpf/syscall.c:2708:1)
> 
> SINGLE KPROBE:
> ffffffff81e0007c entry_SYSCALL_64_after_hwframe+0x44
> (arch/x86/entry/entry_64.S:113:0)
> ffffffff81cd2b15 do_syscall_64+0x35
> (arch/x86/entry/common.c:80:7)
>                  . do_syscall_x64
> (arch/x86/entry/common.c:50:12)
> ffffffff811881aa __x64_sys_bpf+0x1a
> (kernel/bpf/syscall.c:4765:1)
> ffffffff81185e79 __sys_bpf+0x3f9
> (kernel/bpf/syscall.c:4705:9)
> ffffffff8118583a bpf_raw_tracepoint_open+0x19a
> (kernel/bpf/syscall.c:3069:6)
>                  bpf_tracing_prog_attach
> 
> You can see that in multi-attach kprobe we only get one entry, which
> is the very last function in the stack trace. We have no parent
> function captured whatsoever. Jiri, Masami, any ideas what's wrong and
> how to fix this? Let's try to figure this out and fix it before the
> feature makes it into the kernel release. Thanks in advance!

oops, I should have tried kstack with the bpftrace's kretprobe, I see the same:

	# ./src/bpftrace -e 'kretprobe:x* { @[kstack] = count(); }'
	Attaching 1 probe...
	Attaching 3340probes
	^C

	@[
	    xfs_trans_apply_dquot_deltas+0
	]: 22
	@[
	    xlog_cil_commit+0
	]: 22
	@[
	    xlog_grant_push_threshold+0
	]: 22
	@[
	    xfs_trans_add_item+0
	]: 22
	@[
	    xfs_log_reserve+0
	]: 22
	@[
	    xlog_space_left+0
	]: 22
	@[
	    xfs_buf_offset+0
	]: 22
	@[
	    xfs_trans_free_dqinfo+0
	]: 22
	@[
	    xlog_ticket_alloc+0
	    xfs_log_reserve+5
	]: 22
	@[
	    xfs_cil_prepare_item+0


I think it's because we set original ip for return probe to have
bpf_get_func_ip working properly, but it breaks backtrace of course 

I'm not sure we could bring along the original regs for return probe,
but I have an idea how to workaround the bpf_get_func_ip issue and
keep the registers intact for other helpers

jirka

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link
  2022-03-19 12:27   ` Jiri Olsa
@ 2022-03-19 14:31     ` Jiri Olsa
  2022-03-19 18:26       ` Andrii Nakryiko
  0 siblings, 1 reply; 26+ messages in thread
From: Jiri Olsa @ 2022-03-19 14:31 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Masami Hiramatsu, Networking, bpf, lkml, Martin KaFai Lau,
	Song Liu, Yonghong Song, John Fastabend, KP Singh,
	Steven Rostedt

On Sat, Mar 19, 2022 at 01:27:56PM +0100, Jiri Olsa wrote:
> On Fri, Mar 18, 2022 at 10:50:46PM -0700, Andrii Nakryiko wrote:
> > On Wed, Mar 16, 2022 at 5:24 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > hi,
> > > this patchset adds new link type BPF_TRACE_KPROBE_MULTI that attaches
> > > kprobe program through fprobe API [1] instroduced by Masami.
> > >
> > > The fprobe API allows to attach probe on multiple functions at once very
> > > fast, because it works on top of ftrace. On the other hand this limits
> > > the probe point to the function entry or return.
> > >
> > >
> > > With bpftrace support I see following attach speed:
> > >
> > >   # perf stat --null -r 5 ./src/bpftrace -e 'kprobe:x* { } i:ms:1 { exit(); } '
> > >   Attaching 2 probes...
> > >   Attaching 3342 functions
> > >   ...
> > >
> > >   1.4960 +- 0.0285 seconds time elapsed  ( +-  1.91% )
> > >
> > > v3 changes:
> > >   - based on latest fprobe post from Masami [2]
> > >   - add acks
> > >   - add extra comment to kprobe_multi_link_handler wrt entry ip setup [Masami]
> > >   - keep swap_words_64 static and swap values directly in
> > >     bpf_kprobe_multi_cookie_swap [Andrii]
> > >   - rearrange locking/migrate setup in kprobe_multi_link_prog_run [Andrii]
> > >   - move uapi fields [Andrii]
> > >   - add bpf_program__attach_kprobe_multi_opts function [Andrii]
> > >   - many small test changes [Andrii]
> > >   - added tests for bpf_program__attach_kprobe_multi_opts
> > >   - make kallsyms_lookup_name check for empty string [Andrii]
> > >
> > > v2 changes:
> > >   - based on latest fprobe changes [1]
> > >   - renaming the uapi interface to kprobe multi
> > >   - adding support for sort_r to pass user pointer for swap functions
> > >     and using that in cookie support to keep just single functions array
> > >   - moving new link to kernel/trace/bpf_trace.c file
> > >   - using single fprobe callback function for entry and exit
> > >   - using kvzalloc, libbpf_ensure_mem functions
> > >   - adding new k[ret]probe.multi sections instead of using current kprobe
> > >   - used glob_match from test_progs.c, added '?' matching
> > >   - move bpf_get_func_ip verifier inline change to seprate change
> > >   - couple of other minor fixes
> > >
> > >
> > > Also available at:
> > >   https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> > >   bpf/kprobe_multi
> > >
> > > thanks,
> > > jirka
> > >
> > >
> > > [1] https://lore.kernel.org/bpf/164458044634.586276.3261555265565111183.stgit@devnote2/
> > > [2] https://lore.kernel.org/bpf/164735281449.1084943.12438881786173547153.stgit@devnote2/
> > > ---
> > > Jiri Olsa (13):
> > >       lib/sort: Add priv pointer to swap function
> > >       kallsyms: Skip the name search for empty string
> > >       bpf: Add multi kprobe link
> > >       bpf: Add bpf_get_func_ip kprobe helper for multi kprobe link
> > >       bpf: Add support to inline bpf_get_func_ip helper on x86
> > >       bpf: Add cookie support to programs attached with kprobe multi link
> > >       libbpf: Add libbpf_kallsyms_parse function
> > >       libbpf: Add bpf_link_create support for multi kprobes
> > >       libbpf: Add bpf_program__attach_kprobe_multi_opts function
> > >       selftests/bpf: Add kprobe_multi attach test
> > >       selftests/bpf: Add kprobe_multi bpf_cookie test
> > >       selftests/bpf: Add attach test for bpf_program__attach_kprobe_multi_opts
> > >       selftests/bpf: Add cookie test for bpf_program__attach_kprobe_multi_opts
> > >
> > 
> > Ok, so I've integrated multi-attach kprobes into retsnoop. It was
> > pretty straightforward. Here are some numbers for the speed of
> > attaching and, even more importantly, detaching for a set of almost
> > 400 functions. It's a bit less functions for fentry-based mode due to
> > more limited BTF information for static functions. Note that retsnoop
> > attaches two BPF programs for each kernel function, so it's actually
> > two multi-attach kprobes, each attaching to 397 functions. For
> > single-attach kprobe, we perform 397 * 2 = 794 attachments.
> > 
> > I've been invoking retsnoop with the following specified set of
> > functions: -e '*sys_bpf*' -a ':kernel/bpf/syscall.c' -a
> > ':kernel/bpf/verifier.c' -a ':kernel/bpf/btf.c' -a
> > ':kernel/bpf/core.c'. So basically bpf syscall entry functions and all
> > the discoverable functions from syscall.c, verifier.c, core.c and
> > btf.c from kernel/bpf subdirectory.
> > 
> > Results:
> > 
> > fentry attach/detach (263 kfuncs): 2133 ms / 31671  ms (33 seconds)
> > kprobe attach/detach (397 kfuncs): 3121 ms / 13195 ms (16 seconds)
> > multi-kprobe attach/detach (397 kfuncs): 9 ms / 248 ms (0.25 seconds)
> > 
> > So as you can see, the speed up is tremendous! API is also very
> > convenient, I didn't have to modify retsnoop internals much to
> > accommodate multi-attach API. Great job!
> 
> nice! thanks for doing that so quickly
> 
> > 
> > Now for the bad news. :(
> > 
> > Stack traces from multi-attach kretprobe are broken, which makes all
> > this way less useful.
> > 
> > Below, see stack traces captured with multi- and single- kretprobes
> > for two different use cases. Single kprobe stack traces make much more
> > sense. Ignore that last function doesn't have actual address
> > associated with it (i.e. for __sys_bpf and bpf_tracing_prog_attach,
> > respectively). That's just how retsnoop is doing things, I think. We
> > actually were capturing stack traces from inside __sys_bpf (first
> > case) and bpf_tracing_prog_attach (second case).
> > 
> > MULTI KPROBE:
> > ffffffff81185a80 __sys_bpf+0x0
> > (kernel/bpf/syscall.c:4622:1)
> > 
> > SINGLE KPROBE:
> > ffffffff81e0007c entry_SYSCALL_64_after_hwframe+0x44
> > (arch/x86/entry/entry_64.S:113:0)
> > ffffffff81cd2b15 do_syscall_64+0x35
> > (arch/x86/entry/common.c:80:7)
> >                  . do_syscall_x64
> > (arch/x86/entry/common.c:50:12)
> > ffffffff811881aa __x64_sys_bpf+0x1a
> > (kernel/bpf/syscall.c:4765:1)
> >                  __sys_bpf
> > 
> > 
> > MULTI KPROBE:
> > ffffffff811851b0 bpf_tracing_prog_attach+0x0
> > (kernel/bpf/syscall.c:2708:1)
> > 
> > SINGLE KPROBE:
> > ffffffff81e0007c entry_SYSCALL_64_after_hwframe+0x44
> > (arch/x86/entry/entry_64.S:113:0)
> > ffffffff81cd2b15 do_syscall_64+0x35
> > (arch/x86/entry/common.c:80:7)
> >                  . do_syscall_x64
> > (arch/x86/entry/common.c:50:12)
> > ffffffff811881aa __x64_sys_bpf+0x1a
> > (kernel/bpf/syscall.c:4765:1)
> > ffffffff81185e79 __sys_bpf+0x3f9
> > (kernel/bpf/syscall.c:4705:9)
> > ffffffff8118583a bpf_raw_tracepoint_open+0x19a
> > (kernel/bpf/syscall.c:3069:6)
> >                  bpf_tracing_prog_attach
> > 
> > You can see that in multi-attach kprobe we only get one entry, which
> > is the very last function in the stack trace. We have no parent
> > function captured whatsoever. Jiri, Masami, any ideas what's wrong and
> > how to fix this? Let's try to figure this out and fix it before the
> > feature makes it into the kernel release. Thanks in advance!
> 
> oops, I should have tried kstack with the bpftrace's kretprobe, I see the same:
> 
> 	# ./src/bpftrace -e 'kretprobe:x* { @[kstack] = count(); }'
> 	Attaching 1 probe...
> 	Attaching 3340probes
> 	^C
> 
> 	@[
> 	    xfs_trans_apply_dquot_deltas+0
> 	]: 22
> 	@[
> 	    xlog_cil_commit+0
> 	]: 22
> 	@[
> 	    xlog_grant_push_threshold+0
> 	]: 22
> 	@[
> 	    xfs_trans_add_item+0
> 	]: 22
> 	@[
> 	    xfs_log_reserve+0
> 	]: 22
> 	@[
> 	    xlog_space_left+0
> 	]: 22
> 	@[
> 	    xfs_buf_offset+0
> 	]: 22
> 	@[
> 	    xfs_trans_free_dqinfo+0
> 	]: 22
> 	@[
> 	    xlog_ticket_alloc+0
> 	    xfs_log_reserve+5
> 	]: 22
> 	@[
> 	    xfs_cil_prepare_item+0
> 
> 
> I think it's because we set original ip for return probe to have
> bpf_get_func_ip working properly, but it breaks backtrace of course 
> 
> I'm not sure we could bring along the original regs for return probe,
> but I have an idea how to workaround the bpf_get_func_ip issue and
> keep the registers intact for other helpers

change below is using bpf_run_ctx to store link and entry ip on stack,
where helpers can find it.. it fixed the retprobe backtrace for me

I had to revert the get_func_ip inline.. it's squashed in the change
below for quick testing.. I'll send revert in separate patch with the
formal change

could you please test?

thanks,
jirka


---
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 0287176bfe9a..cf92f9c01556 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13678,7 +13678,7 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
 			continue;
 		}
 
-		/* Implement tracing bpf_get_func_ip inline. */
+		/* Implement bpf_get_func_ip inline. */
 		if (prog_type == BPF_PROG_TYPE_TRACING &&
 		    insn->imm == BPF_FUNC_get_func_ip) {
 			/* Load IP address from ctx - 16 */
@@ -13693,25 +13693,6 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
 			continue;
 		}
 
-#ifdef CONFIG_X86
-		/* Implement kprobe_multi bpf_get_func_ip inline. */
-		if (prog_type == BPF_PROG_TYPE_KPROBE &&
-		    eatype == BPF_TRACE_KPROBE_MULTI &&
-		    insn->imm == BPF_FUNC_get_func_ip) {
-			/* Load IP address from ctx (struct pt_regs) ip */
-			insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1,
-						  offsetof(struct pt_regs, ip));
-
-			new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, 1);
-			if (!new_prog)
-				return -ENOMEM;
-
-			env->prog = prog = new_prog;
-			insn      = new_prog->insnsi + i + delta;
-			continue;
-		}
-#endif
-
 patch_call_imm:
 		fn = env->ops->get_func_proto(insn->imm, env->prog);
 		/* all functions that have prototype and verifier allowed
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 9a7b6be655e4..aefe33c08296 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -80,7 +80,8 @@ u64 bpf_get_stack(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
 static int bpf_btf_printf_prepare(struct btf_ptr *ptr, u32 btf_ptr_size,
 				  u64 flags, const struct btf **btf,
 				  s32 *btf_id);
-static u64 bpf_kprobe_multi_cookie(struct bpf_run_ctx *ctx, u64 ip);
+static u64 bpf_kprobe_multi_cookie(struct bpf_run_ctx *ctx);
+static u64 bpf_kprobe_multi_entry_ip(struct bpf_run_ctx *ctx);
 
 /**
  * trace_call_bpf - invoke BPF program
@@ -1042,8 +1043,7 @@ static const struct bpf_func_proto bpf_get_func_ip_proto_kprobe = {
 
 BPF_CALL_1(bpf_get_func_ip_kprobe_multi, struct pt_regs *, regs)
 {
-	/* This helper call is inlined by verifier on x86. */
-	return instruction_pointer(regs);
+	return bpf_kprobe_multi_entry_ip(current->bpf_ctx);
 }
 
 static const struct bpf_func_proto bpf_get_func_ip_proto_kprobe_multi = {
@@ -1055,7 +1055,7 @@ static const struct bpf_func_proto bpf_get_func_ip_proto_kprobe_multi = {
 
 BPF_CALL_1(bpf_get_attach_cookie_kprobe_multi, struct pt_regs *, regs)
 {
-	return bpf_kprobe_multi_cookie(current->bpf_ctx, instruction_pointer(regs));
+	return bpf_kprobe_multi_cookie(current->bpf_ctx);
 }
 
 static const struct bpf_func_proto bpf_get_attach_cookie_proto_kmulti = {
@@ -2220,15 +2220,16 @@ struct bpf_kprobe_multi_link {
 	struct bpf_link link;
 	struct fprobe fp;
 	unsigned long *addrs;
-	/*
-	 * The run_ctx here is used to get struct bpf_kprobe_multi_link in
-	 * get_attach_cookie helper, so it can't be used to store data.
-	 */
-	struct bpf_run_ctx run_ctx;
 	u64 *cookies;
 	u32 cnt;
 };
 
+struct bpf_kprobe_multi_run_ctx {
+	struct bpf_run_ctx run_ctx;
+	struct bpf_kprobe_multi_link *link;
+	unsigned long entry_ip;
+};
+
 static void bpf_kprobe_multi_link_release(struct bpf_link *link)
 {
 	struct bpf_kprobe_multi_link *kmulti_link;
@@ -2282,18 +2283,21 @@ static int bpf_kprobe_multi_cookie_cmp(const void *a, const void *b, const void
 	return __bpf_kprobe_multi_cookie_cmp(a, b);
 }
 
-static u64 bpf_kprobe_multi_cookie(struct bpf_run_ctx *ctx, u64 ip)
+static u64 bpf_kprobe_multi_cookie(struct bpf_run_ctx *ctx)
 {
+	struct bpf_kprobe_multi_run_ctx *run_ctx;
 	struct bpf_kprobe_multi_link *link;
+	u64 *cookie, entry_ip;
 	unsigned long *addr;
-	u64 *cookie;
 
 	if (WARN_ON_ONCE(!ctx))
 		return 0;
-	link = container_of(ctx, struct bpf_kprobe_multi_link, run_ctx);
+	run_ctx = container_of(current->bpf_ctx, struct bpf_kprobe_multi_run_ctx, run_ctx);
+	link = run_ctx->link;
+	entry_ip = run_ctx->entry_ip;
 	if (!link->cookies)
 		return 0;
-	addr = bsearch(&ip, link->addrs, link->cnt, sizeof(ip),
+	addr = bsearch(&entry_ip, link->addrs, link->cnt, sizeof(entry_ip),
 		       __bpf_kprobe_multi_cookie_cmp);
 	if (!addr)
 		return 0;
@@ -2301,10 +2305,22 @@ static u64 bpf_kprobe_multi_cookie(struct bpf_run_ctx *ctx, u64 ip)
 	return *cookie;
 }
 
+static u64 bpf_kprobe_multi_entry_ip(struct bpf_run_ctx *ctx)
+{
+	struct bpf_kprobe_multi_run_ctx *run_ctx;
+
+	run_ctx = container_of(current->bpf_ctx, struct bpf_kprobe_multi_run_ctx, run_ctx);
+	return run_ctx->entry_ip;
+}
+
 static int
 kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
-			   struct pt_regs *regs)
+			   unsigned long entry_ip, struct pt_regs *regs)
 {
+	struct bpf_kprobe_multi_run_ctx run_ctx = {
+		.link = link,
+		.entry_ip = entry_ip,
+	};
 	struct bpf_run_ctx *old_run_ctx;
 	int err;
 
@@ -2315,7 +2331,7 @@ kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
 
 	migrate_disable();
 	rcu_read_lock();
-	old_run_ctx = bpf_set_run_ctx(&link->run_ctx);
+	old_run_ctx = bpf_set_run_ctx(&run_ctx.run_ctx);
 	err = bpf_prog_run(link->link.prog, regs);
 	bpf_reset_run_ctx(old_run_ctx);
 	rcu_read_unlock();
@@ -2330,24 +2346,10 @@ static void
 kprobe_multi_link_handler(struct fprobe *fp, unsigned long entry_ip,
 			  struct pt_regs *regs)
 {
-	unsigned long saved_ip = instruction_pointer(regs);
 	struct bpf_kprobe_multi_link *link;
 
-	/*
-	 * Because fprobe's regs->ip is set to the next instruction of
-	 * dynamic-ftrace instruction, correct entry ip must be set, so
-	 * that the bpf program can access entry address via regs as same
-	 * as kprobes.
-	 *
-	 * Both kprobe and kretprobe see the entry ip of traced function
-	 * as instruction pointer.
-	 */
-	instruction_pointer_set(regs, entry_ip);
-
 	link = container_of(fp, struct bpf_kprobe_multi_link, fp);
-	kprobe_multi_link_prog_run(link, regs);
-
-	instruction_pointer_set(regs, saved_ip);
+	kprobe_multi_link_prog_run(link, entry_ip, regs);
 }
 
 static int
@@ -2514,7 +2516,11 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 {
 	return -EOPNOTSUPP;
 }
-static u64 bpf_kprobe_multi_cookie(struct bpf_run_ctx *ctx, u64 ip)
+static u64 bpf_kprobe_multi_cookie(struct bpf_run_ctx *ctx)
+{
+	return 0;
+}
+static u64 bpf_kprobe_multi_entry_ip(struct bpf_run_ctx *ctx)
 {
 	return 0;
 }

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link
  2022-03-19 14:31     ` Jiri Olsa
@ 2022-03-19 18:26       ` Andrii Nakryiko
  2022-03-19 19:01         ` Andrii Nakryiko
  0 siblings, 1 reply; 26+ messages in thread
From: Andrii Nakryiko @ 2022-03-19 18:26 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Masami Hiramatsu, Networking, bpf, lkml, Martin KaFai Lau,
	Song Liu, Yonghong Song, John Fastabend, KP Singh,
	Steven Rostedt

On Sat, Mar 19, 2022 at 7:31 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Sat, Mar 19, 2022 at 01:27:56PM +0100, Jiri Olsa wrote:
> > On Fri, Mar 18, 2022 at 10:50:46PM -0700, Andrii Nakryiko wrote:
> > > On Wed, Mar 16, 2022 at 5:24 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > >
> > > > hi,
> > > > this patchset adds new link type BPF_TRACE_KPROBE_MULTI that attaches
> > > > kprobe program through fprobe API [1] instroduced by Masami.
> > > >
> > > > The fprobe API allows to attach probe on multiple functions at once very
> > > > fast, because it works on top of ftrace. On the other hand this limits
> > > > the probe point to the function entry or return.
> > > >
> > > >
> > > > With bpftrace support I see following attach speed:
> > > >
> > > >   # perf stat --null -r 5 ./src/bpftrace -e 'kprobe:x* { } i:ms:1 { exit(); } '
> > > >   Attaching 2 probes...
> > > >   Attaching 3342 functions
> > > >   ...
> > > >
> > > >   1.4960 +- 0.0285 seconds time elapsed  ( +-  1.91% )
> > > >
> > > > v3 changes:
> > > >   - based on latest fprobe post from Masami [2]
> > > >   - add acks
> > > >   - add extra comment to kprobe_multi_link_handler wrt entry ip setup [Masami]
> > > >   - keep swap_words_64 static and swap values directly in
> > > >     bpf_kprobe_multi_cookie_swap [Andrii]
> > > >   - rearrange locking/migrate setup in kprobe_multi_link_prog_run [Andrii]
> > > >   - move uapi fields [Andrii]
> > > >   - add bpf_program__attach_kprobe_multi_opts function [Andrii]
> > > >   - many small test changes [Andrii]
> > > >   - added tests for bpf_program__attach_kprobe_multi_opts
> > > >   - make kallsyms_lookup_name check for empty string [Andrii]
> > > >
> > > > v2 changes:
> > > >   - based on latest fprobe changes [1]
> > > >   - renaming the uapi interface to kprobe multi
> > > >   - adding support for sort_r to pass user pointer for swap functions
> > > >     and using that in cookie support to keep just single functions array
> > > >   - moving new link to kernel/trace/bpf_trace.c file
> > > >   - using single fprobe callback function for entry and exit
> > > >   - using kvzalloc, libbpf_ensure_mem functions
> > > >   - adding new k[ret]probe.multi sections instead of using current kprobe
> > > >   - used glob_match from test_progs.c, added '?' matching
> > > >   - move bpf_get_func_ip verifier inline change to seprate change
> > > >   - couple of other minor fixes
> > > >
> > > >
> > > > Also available at:
> > > >   https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> > > >   bpf/kprobe_multi
> > > >
> > > > thanks,
> > > > jirka
> > > >
> > > >
> > > > [1] https://lore.kernel.org/bpf/164458044634.586276.3261555265565111183.stgit@devnote2/
> > > > [2] https://lore.kernel.org/bpf/164735281449.1084943.12438881786173547153.stgit@devnote2/
> > > > ---
> > > > Jiri Olsa (13):
> > > >       lib/sort: Add priv pointer to swap function
> > > >       kallsyms: Skip the name search for empty string
> > > >       bpf: Add multi kprobe link
> > > >       bpf: Add bpf_get_func_ip kprobe helper for multi kprobe link
> > > >       bpf: Add support to inline bpf_get_func_ip helper on x86
> > > >       bpf: Add cookie support to programs attached with kprobe multi link
> > > >       libbpf: Add libbpf_kallsyms_parse function
> > > >       libbpf: Add bpf_link_create support for multi kprobes
> > > >       libbpf: Add bpf_program__attach_kprobe_multi_opts function
> > > >       selftests/bpf: Add kprobe_multi attach test
> > > >       selftests/bpf: Add kprobe_multi bpf_cookie test
> > > >       selftests/bpf: Add attach test for bpf_program__attach_kprobe_multi_opts
> > > >       selftests/bpf: Add cookie test for bpf_program__attach_kprobe_multi_opts
> > > >
> > >
> > > Ok, so I've integrated multi-attach kprobes into retsnoop. It was
> > > pretty straightforward. Here are some numbers for the speed of
> > > attaching and, even more importantly, detaching for a set of almost
> > > 400 functions. It's a bit less functions for fentry-based mode due to
> > > more limited BTF information for static functions. Note that retsnoop
> > > attaches two BPF programs for each kernel function, so it's actually
> > > two multi-attach kprobes, each attaching to 397 functions. For
> > > single-attach kprobe, we perform 397 * 2 = 794 attachments.
> > >
> > > I've been invoking retsnoop with the following specified set of
> > > functions: -e '*sys_bpf*' -a ':kernel/bpf/syscall.c' -a
> > > ':kernel/bpf/verifier.c' -a ':kernel/bpf/btf.c' -a
> > > ':kernel/bpf/core.c'. So basically bpf syscall entry functions and all
> > > the discoverable functions from syscall.c, verifier.c, core.c and
> > > btf.c from kernel/bpf subdirectory.
> > >
> > > Results:
> > >
> > > fentry attach/detach (263 kfuncs): 2133 ms / 31671  ms (33 seconds)
> > > kprobe attach/detach (397 kfuncs): 3121 ms / 13195 ms (16 seconds)
> > > multi-kprobe attach/detach (397 kfuncs): 9 ms / 248 ms (0.25 seconds)
> > >
> > > So as you can see, the speed up is tremendous! API is also very
> > > convenient, I didn't have to modify retsnoop internals much to
> > > accommodate multi-attach API. Great job!
> >
> > nice! thanks for doing that so quickly
> >
> > >
> > > Now for the bad news. :(
> > >
> > > Stack traces from multi-attach kretprobe are broken, which makes all
> > > this way less useful.
> > >
> > > Below, see stack traces captured with multi- and single- kretprobes
> > > for two different use cases. Single kprobe stack traces make much more
> > > sense. Ignore that last function doesn't have actual address
> > > associated with it (i.e. for __sys_bpf and bpf_tracing_prog_attach,
> > > respectively). That's just how retsnoop is doing things, I think. We
> > > actually were capturing stack traces from inside __sys_bpf (first
> > > case) and bpf_tracing_prog_attach (second case).
> > >
> > > MULTI KPROBE:
> > > ffffffff81185a80 __sys_bpf+0x0
> > > (kernel/bpf/syscall.c:4622:1)
> > >
> > > SINGLE KPROBE:
> > > ffffffff81e0007c entry_SYSCALL_64_after_hwframe+0x44
> > > (arch/x86/entry/entry_64.S:113:0)
> > > ffffffff81cd2b15 do_syscall_64+0x35
> > > (arch/x86/entry/common.c:80:7)
> > >                  . do_syscall_x64
> > > (arch/x86/entry/common.c:50:12)
> > > ffffffff811881aa __x64_sys_bpf+0x1a
> > > (kernel/bpf/syscall.c:4765:1)
> > >                  __sys_bpf
> > >
> > >
> > > MULTI KPROBE:
> > > ffffffff811851b0 bpf_tracing_prog_attach+0x0
> > > (kernel/bpf/syscall.c:2708:1)
> > >
> > > SINGLE KPROBE:
> > > ffffffff81e0007c entry_SYSCALL_64_after_hwframe+0x44
> > > (arch/x86/entry/entry_64.S:113:0)
> > > ffffffff81cd2b15 do_syscall_64+0x35
> > > (arch/x86/entry/common.c:80:7)
> > >                  . do_syscall_x64
> > > (arch/x86/entry/common.c:50:12)
> > > ffffffff811881aa __x64_sys_bpf+0x1a
> > > (kernel/bpf/syscall.c:4765:1)
> > > ffffffff81185e79 __sys_bpf+0x3f9
> > > (kernel/bpf/syscall.c:4705:9)
> > > ffffffff8118583a bpf_raw_tracepoint_open+0x19a
> > > (kernel/bpf/syscall.c:3069:6)
> > >                  bpf_tracing_prog_attach
> > >
> > > You can see that in multi-attach kprobe we only get one entry, which
> > > is the very last function in the stack trace. We have no parent
> > > function captured whatsoever. Jiri, Masami, any ideas what's wrong and
> > > how to fix this? Let's try to figure this out and fix it before the
> > > feature makes it into the kernel release. Thanks in advance!
> >
> > oops, I should have tried kstack with the bpftrace's kretprobe, I see the same:
> >
> >       # ./src/bpftrace -e 'kretprobe:x* { @[kstack] = count(); }'
> >       Attaching 1 probe...
> >       Attaching 3340probes
> >       ^C
> >
> >       @[
> >           xfs_trans_apply_dquot_deltas+0
> >       ]: 22
> >       @[
> >           xlog_cil_commit+0
> >       ]: 22
> >       @[
> >           xlog_grant_push_threshold+0
> >       ]: 22
> >       @[
> >           xfs_trans_add_item+0
> >       ]: 22
> >       @[
> >           xfs_log_reserve+0
> >       ]: 22
> >       @[
> >           xlog_space_left+0
> >       ]: 22
> >       @[
> >           xfs_buf_offset+0
> >       ]: 22
> >       @[
> >           xfs_trans_free_dqinfo+0
> >       ]: 22
> >       @[
> >           xlog_ticket_alloc+0
> >           xfs_log_reserve+5
> >       ]: 22
> >       @[
> >           xfs_cil_prepare_item+0
> >
> >
> > I think it's because we set original ip for return probe to have
> > bpf_get_func_ip working properly, but it breaks backtrace of course
> >
> > I'm not sure we could bring along the original regs for return probe,
> > but I have an idea how to workaround the bpf_get_func_ip issue and
> > keep the registers intact for other helpers
>
> change below is using bpf_run_ctx to store link and entry ip on stack,
> where helpers can find it.. it fixed the retprobe backtrace for me
>
> I had to revert the get_func_ip inline.. it's squashed in the change
> below for quick testing.. I'll send revert in separate patch with the
> formal change
>
> could you please test?
>

Yep, tried locally and now stack traces work as expected. Thanks!
Please resubmit as a proper patch and add my ack:

Acked-by: Andrii Nakryiko <andrii@kernel.org>

> thanks,
> jirka
>
>
> ---

[...]

> -static u64 bpf_kprobe_multi_cookie(struct bpf_run_ctx *ctx, u64 ip)
> +static u64 bpf_kprobe_multi_cookie(struct bpf_run_ctx *ctx)
>  {
> +       struct bpf_kprobe_multi_run_ctx *run_ctx;
>         struct bpf_kprobe_multi_link *link;
> +       u64 *cookie, entry_ip;
>         unsigned long *addr;
> -       u64 *cookie;
>
>         if (WARN_ON_ONCE(!ctx))
>                 return 0;
> -       link = container_of(ctx, struct bpf_kprobe_multi_link, run_ctx);
> +       run_ctx = container_of(current->bpf_ctx, struct bpf_kprobe_multi_run_ctx, run_ctx);
> +       link = run_ctx->link;
> +       entry_ip = run_ctx->entry_ip;

nit: this can be assigned after we checked that we have link->cookies

>         if (!link->cookies)
>                 return 0;
> -       addr = bsearch(&ip, link->addrs, link->cnt, sizeof(ip),
> +       addr = bsearch(&entry_ip, link->addrs, link->cnt, sizeof(entry_ip),
>                        __bpf_kprobe_multi_cookie_cmp);
>         if (!addr)
>                 return 0;

[...]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link
  2022-03-19 18:26       ` Andrii Nakryiko
@ 2022-03-19 19:01         ` Andrii Nakryiko
  0 siblings, 0 replies; 26+ messages in thread
From: Andrii Nakryiko @ 2022-03-19 19:01 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Masami Hiramatsu, Networking, bpf, lkml, Martin KaFai Lau,
	Song Liu, Yonghong Song, John Fastabend, KP Singh,
	Steven Rostedt

On Sat, Mar 19, 2022 at 11:26 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Sat, Mar 19, 2022 at 7:31 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Sat, Mar 19, 2022 at 01:27:56PM +0100, Jiri Olsa wrote:
> > > On Fri, Mar 18, 2022 at 10:50:46PM -0700, Andrii Nakryiko wrote:
> > > > On Wed, Mar 16, 2022 at 5:24 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > > >
> > > > > hi,
> > > > > this patchset adds new link type BPF_TRACE_KPROBE_MULTI that attaches
> > > > > kprobe program through fprobe API [1] instroduced by Masami.
> > > > >
> > > > > The fprobe API allows to attach probe on multiple functions at once very
> > > > > fast, because it works on top of ftrace. On the other hand this limits
> > > > > the probe point to the function entry or return.
> > > > >
> > > > >
> > > > > With bpftrace support I see following attach speed:
> > > > >
> > > > >   # perf stat --null -r 5 ./src/bpftrace -e 'kprobe:x* { } i:ms:1 { exit(); } '
> > > > >   Attaching 2 probes...
> > > > >   Attaching 3342 functions
> > > > >   ...
> > > > >
> > > > >   1.4960 +- 0.0285 seconds time elapsed  ( +-  1.91% )
> > > > >
> > > > > v3 changes:
> > > > >   - based on latest fprobe post from Masami [2]
> > > > >   - add acks
> > > > >   - add extra comment to kprobe_multi_link_handler wrt entry ip setup [Masami]
> > > > >   - keep swap_words_64 static and swap values directly in
> > > > >     bpf_kprobe_multi_cookie_swap [Andrii]
> > > > >   - rearrange locking/migrate setup in kprobe_multi_link_prog_run [Andrii]
> > > > >   - move uapi fields [Andrii]
> > > > >   - add bpf_program__attach_kprobe_multi_opts function [Andrii]
> > > > >   - many small test changes [Andrii]
> > > > >   - added tests for bpf_program__attach_kprobe_multi_opts
> > > > >   - make kallsyms_lookup_name check for empty string [Andrii]
> > > > >
> > > > > v2 changes:
> > > > >   - based on latest fprobe changes [1]
> > > > >   - renaming the uapi interface to kprobe multi
> > > > >   - adding support for sort_r to pass user pointer for swap functions
> > > > >     and using that in cookie support to keep just single functions array
> > > > >   - moving new link to kernel/trace/bpf_trace.c file
> > > > >   - using single fprobe callback function for entry and exit
> > > > >   - using kvzalloc, libbpf_ensure_mem functions
> > > > >   - adding new k[ret]probe.multi sections instead of using current kprobe
> > > > >   - used glob_match from test_progs.c, added '?' matching
> > > > >   - move bpf_get_func_ip verifier inline change to seprate change
> > > > >   - couple of other minor fixes
> > > > >
> > > > >
> > > > > Also available at:
> > > > >   https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> > > > >   bpf/kprobe_multi
> > > > >
> > > > > thanks,
> > > > > jirka
> > > > >
> > > > >
> > > > > [1] https://lore.kernel.org/bpf/164458044634.586276.3261555265565111183.stgit@devnote2/
> > > > > [2] https://lore.kernel.org/bpf/164735281449.1084943.12438881786173547153.stgit@devnote2/
> > > > > ---
> > > > > Jiri Olsa (13):
> > > > >       lib/sort: Add priv pointer to swap function
> > > > >       kallsyms: Skip the name search for empty string
> > > > >       bpf: Add multi kprobe link
> > > > >       bpf: Add bpf_get_func_ip kprobe helper for multi kprobe link
> > > > >       bpf: Add support to inline bpf_get_func_ip helper on x86
> > > > >       bpf: Add cookie support to programs attached with kprobe multi link
> > > > >       libbpf: Add libbpf_kallsyms_parse function
> > > > >       libbpf: Add bpf_link_create support for multi kprobes
> > > > >       libbpf: Add bpf_program__attach_kprobe_multi_opts function
> > > > >       selftests/bpf: Add kprobe_multi attach test
> > > > >       selftests/bpf: Add kprobe_multi bpf_cookie test
> > > > >       selftests/bpf: Add attach test for bpf_program__attach_kprobe_multi_opts
> > > > >       selftests/bpf: Add cookie test for bpf_program__attach_kprobe_multi_opts
> > > > >
> > > >
> > > > Ok, so I've integrated multi-attach kprobes into retsnoop. It was
> > > > pretty straightforward. Here are some numbers for the speed of
> > > > attaching and, even more importantly, detaching for a set of almost
> > > > 400 functions. It's a bit less functions for fentry-based mode due to
> > > > more limited BTF information for static functions. Note that retsnoop
> > > > attaches two BPF programs for each kernel function, so it's actually
> > > > two multi-attach kprobes, each attaching to 397 functions. For
> > > > single-attach kprobe, we perform 397 * 2 = 794 attachments.
> > > >
> > > > I've been invoking retsnoop with the following specified set of
> > > > functions: -e '*sys_bpf*' -a ':kernel/bpf/syscall.c' -a
> > > > ':kernel/bpf/verifier.c' -a ':kernel/bpf/btf.c' -a
> > > > ':kernel/bpf/core.c'. So basically bpf syscall entry functions and all
> > > > the discoverable functions from syscall.c, verifier.c, core.c and
> > > > btf.c from kernel/bpf subdirectory.
> > > >
> > > > Results:
> > > >
> > > > fentry attach/detach (263 kfuncs): 2133 ms / 31671  ms (33 seconds)
> > > > kprobe attach/detach (397 kfuncs): 3121 ms / 13195 ms (16 seconds)
> > > > multi-kprobe attach/detach (397 kfuncs): 9 ms / 248 ms (0.25 seconds)
> > > >
> > > > So as you can see, the speed up is tremendous! API is also very
> > > > convenient, I didn't have to modify retsnoop internals much to
> > > > accommodate multi-attach API. Great job!
> > >
> > > nice! thanks for doing that so quickly
> > >
> > > >
> > > > Now for the bad news. :(
> > > >
> > > > Stack traces from multi-attach kretprobe are broken, which makes all
> > > > this way less useful.
> > > >
> > > > Below, see stack traces captured with multi- and single- kretprobes
> > > > for two different use cases. Single kprobe stack traces make much more
> > > > sense. Ignore that last function doesn't have actual address
> > > > associated with it (i.e. for __sys_bpf and bpf_tracing_prog_attach,
> > > > respectively). That's just how retsnoop is doing things, I think. We
> > > > actually were capturing stack traces from inside __sys_bpf (first
> > > > case) and bpf_tracing_prog_attach (second case).
> > > >
> > > > MULTI KPROBE:
> > > > ffffffff81185a80 __sys_bpf+0x0
> > > > (kernel/bpf/syscall.c:4622:1)
> > > >
> > > > SINGLE KPROBE:
> > > > ffffffff81e0007c entry_SYSCALL_64_after_hwframe+0x44
> > > > (arch/x86/entry/entry_64.S:113:0)
> > > > ffffffff81cd2b15 do_syscall_64+0x35
> > > > (arch/x86/entry/common.c:80:7)
> > > >                  . do_syscall_x64
> > > > (arch/x86/entry/common.c:50:12)
> > > > ffffffff811881aa __x64_sys_bpf+0x1a
> > > > (kernel/bpf/syscall.c:4765:1)
> > > >                  __sys_bpf
> > > >
> > > >
> > > > MULTI KPROBE:
> > > > ffffffff811851b0 bpf_tracing_prog_attach+0x0
> > > > (kernel/bpf/syscall.c:2708:1)
> > > >
> > > > SINGLE KPROBE:
> > > > ffffffff81e0007c entry_SYSCALL_64_after_hwframe+0x44
> > > > (arch/x86/entry/entry_64.S:113:0)
> > > > ffffffff81cd2b15 do_syscall_64+0x35
> > > > (arch/x86/entry/common.c:80:7)
> > > >                  . do_syscall_x64
> > > > (arch/x86/entry/common.c:50:12)
> > > > ffffffff811881aa __x64_sys_bpf+0x1a
> > > > (kernel/bpf/syscall.c:4765:1)
> > > > ffffffff81185e79 __sys_bpf+0x3f9
> > > > (kernel/bpf/syscall.c:4705:9)
> > > > ffffffff8118583a bpf_raw_tracepoint_open+0x19a
> > > > (kernel/bpf/syscall.c:3069:6)
> > > >                  bpf_tracing_prog_attach
> > > >
> > > > You can see that in multi-attach kprobe we only get one entry, which
> > > > is the very last function in the stack trace. We have no parent
> > > > function captured whatsoever. Jiri, Masami, any ideas what's wrong and
> > > > how to fix this? Let's try to figure this out and fix it before the
> > > > feature makes it into the kernel release. Thanks in advance!
> > >
> > > oops, I should have tried kstack with the bpftrace's kretprobe, I see the same:
> > >
> > >       # ./src/bpftrace -e 'kretprobe:x* { @[kstack] = count(); }'
> > >       Attaching 1 probe...
> > >       Attaching 3340probes
> > >       ^C
> > >
> > >       @[
> > >           xfs_trans_apply_dquot_deltas+0
> > >       ]: 22
> > >       @[
> > >           xlog_cil_commit+0
> > >       ]: 22
> > >       @[
> > >           xlog_grant_push_threshold+0
> > >       ]: 22
> > >       @[
> > >           xfs_trans_add_item+0
> > >       ]: 22
> > >       @[
> > >           xfs_log_reserve+0
> > >       ]: 22
> > >       @[
> > >           xlog_space_left+0
> > >       ]: 22
> > >       @[
> > >           xfs_buf_offset+0
> > >       ]: 22
> > >       @[
> > >           xfs_trans_free_dqinfo+0
> > >       ]: 22
> > >       @[
> > >           xlog_ticket_alloc+0
> > >           xfs_log_reserve+5
> > >       ]: 22
> > >       @[
> > >           xfs_cil_prepare_item+0
> > >
> > >
> > > I think it's because we set original ip for return probe to have
> > > bpf_get_func_ip working properly, but it breaks backtrace of course
> > >
> > > I'm not sure we could bring along the original regs for return probe,
> > > but I have an idea how to workaround the bpf_get_func_ip issue and
> > > keep the registers intact for other helpers
> >
> > change below is using bpf_run_ctx to store link and entry ip on stack,
> > where helpers can find it.. it fixed the retprobe backtrace for me
> >
> > I had to revert the get_func_ip inline.. it's squashed in the change
> > below for quick testing.. I'll send revert in separate patch with the
> > formal change
> >
> > could you please test?
> >
>
> Yep, tried locally and now stack traces work as expected. Thanks!

BTW, there is a separate and unrelated problem, forgot to mention it yesterday.

It is possible to attach to some functions through using .syms
(resolve by name), but it is rejected when specifying .addrs (which
retsnoop derives directly from kallsyms). One such example is
"__bpf_tramp_exit". What's interesting, this function is marked
notrace, but is still attachable by name (including using
single-attach kprobe), while I'd assume it shouldn't.

It is all kind of broken, tbh, this notrace function is present in
/sys/kernel/debug/tracing/available_filter_functions, it's not in
/sys/kernel/debug/kprobes/blacklist, so the whole notrace and
blacklisting doesn't seem to work well.

So anyways, worth looking into this as well and figuring out why this
is inconsistent between .syms and .addrs approaches. Please take a
look when you get a chance. And I'll try to figure out if there is any
way to determine notrace functions from outside the kernel.


> Please resubmit as a proper patch and add my ack:
>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
>
> > thanks,
> > jirka
> >
> >
> > ---
>
> [...]
>
> > -static u64 bpf_kprobe_multi_cookie(struct bpf_run_ctx *ctx, u64 ip)
> > +static u64 bpf_kprobe_multi_cookie(struct bpf_run_ctx *ctx)
> >  {
> > +       struct bpf_kprobe_multi_run_ctx *run_ctx;
> >         struct bpf_kprobe_multi_link *link;
> > +       u64 *cookie, entry_ip;
> >         unsigned long *addr;
> > -       u64 *cookie;
> >
> >         if (WARN_ON_ONCE(!ctx))
> >                 return 0;
> > -       link = container_of(ctx, struct bpf_kprobe_multi_link, run_ctx);
> > +       run_ctx = container_of(current->bpf_ctx, struct bpf_kprobe_multi_run_ctx, run_ctx);
> > +       link = run_ctx->link;
> > +       entry_ip = run_ctx->entry_ip;
>
> nit: this can be assigned after we checked that we have link->cookies
>
> >         if (!link->cookies)
> >                 return 0;
> > -       addr = bsearch(&ip, link->addrs, link->cnt, sizeof(ip),
> > +       addr = bsearch(&entry_ip, link->addrs, link->cnt, sizeof(entry_ip),
> >                        __bpf_kprobe_multi_cookie_cmp);
> >         if (!addr)
> >                 return 0;
>
> [...]

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2022-03-19 19:02 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-16 12:24 [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link Jiri Olsa
2022-03-16 12:24 ` [PATCHv3 bpf-next 01/13] lib/sort: Add priv pointer to swap function Jiri Olsa
2022-03-16 12:24 ` [PATCHv3 bpf-next 02/13] kallsyms: Skip the name search for empty string Jiri Olsa
2022-03-16 12:24 ` [PATCHv3 bpf-next 03/13] bpf: Add multi kprobe link Jiri Olsa
2022-03-16 18:53   ` Alexei Starovoitov
2022-03-17 13:24     ` Jiri Olsa
2022-03-16 12:24 ` [PATCHv3 bpf-next 04/13] bpf: Add bpf_get_func_ip kprobe helper for " Jiri Olsa
2022-03-16 12:24 ` [PATCHv3 bpf-next 05/13] bpf: Add support to inline bpf_get_func_ip helper on x86 Jiri Olsa
2022-03-16 12:24 ` [PATCHv3 bpf-next 06/13] bpf: Add cookie support to programs attached with kprobe multi link Jiri Olsa
2022-03-16 12:24 ` [PATCHv3 bpf-next 07/13] libbpf: Add libbpf_kallsyms_parse function Jiri Olsa
2022-03-16 12:24 ` [PATCHv3 bpf-next 08/13] libbpf: Add bpf_link_create support for multi kprobes Jiri Olsa
2022-03-16 12:24 ` [PATCHv3 bpf-next 09/13] libbpf: Add bpf_program__attach_kprobe_multi_opts function Jiri Olsa
2022-03-18  3:53   ` Alexei Starovoitov
2022-03-18  5:14     ` Andrii Nakryiko
2022-03-18  9:22       ` Jiri Olsa
2022-03-18  9:08     ` Jiri Olsa
2022-03-16 12:24 ` [PATCHv3 bpf-next 10/13] selftests/bpf: Add kprobe_multi attach test Jiri Olsa
2022-03-16 12:24 ` [PATCHv3 bpf-next 11/13] selftests/bpf: Add kprobe_multi bpf_cookie test Jiri Olsa
2022-03-16 12:24 ` [PATCHv3 bpf-next 12/13] selftests/bpf: Add attach test for bpf_program__attach_kprobe_multi_opts Jiri Olsa
2022-03-16 12:24 ` [PATCHv3 bpf-next 13/13] selftests/bpf: Add cookie " Jiri Olsa
2022-03-18  3:50 ` [PATCHv3 bpf-next 00/13] bpf: Add kprobe multi link patchwork-bot+netdevbpf
2022-03-19  5:50 ` Andrii Nakryiko
2022-03-19 12:27   ` Jiri Olsa
2022-03-19 14:31     ` Jiri Olsa
2022-03-19 18:26       ` Andrii Nakryiko
2022-03-19 19:01         ` Andrii Nakryiko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.