[RFC/PATCH bpf-next 00/20] bpf: Add multi uprobe link

[RFC/PATCH bpf-next 00/20] bpf: Add multi uprobe link

[Jiri Olsa]

hi,
this patchset is adding support to attach multiple uprobes and usdt probes
through new uprobe_multi link.

The current uprobe is attached through the perf event and attaching many
uprobes takes a lot of time because of that.

The main reason is that we need to install perf event for each probed function
and profile shows perf event installation (perf_install_in_context) as culprit.

The new uprobe_multi link just creates raw uprobes and attaches the bpf
program to them without perf event being involved.

In addition to being faster we also save file descriptors. For the current
uprobe attach we use extra perf event fd for each probed function. The new
link just need one fd that covers all the functions we are attaching to.

By dropping perf we lose the ability to attach uprobe to specific pid.
We can workaround that by having pid check directly in the bpf program,
but we might need to check for another solution if that will turn out
to be a problem.


Attaching current bpftrace to 1000 uprobes:

  # BPFTRACE_MAX_PROBES=100000 perf stat -e cycles,instructions \
    ./bpftrace -e 'uprobe:./uprobe_multi:uprobe_multi_func_* { }, i:ms:1 { exit(); }' 
    ...

     126,666,390,509      cycles                                                                
      29,973,207,307      instructions                     #    0.24  insn per cycle            

        85.284833554 seconds time elapsed


Same bpftrace setup with uprobe_multi support:

  # perf stat -e cycles,instructions \
    ./bpftrace -e 'uprobe:./uprobe_multi:uprobe_multi_func_* { }, i:ms:1 { exit(); }' 
    ...

       6,818,470,649      cycles                                                                
      13,275,510,122      instructions                     #    1.95  insn per cycle            

         1.943269451 seconds time elapsed


I'm sending this as RFC because of:
  - I added/exported some new elf_* helper functions in libbpf,
    and I'm not sure that's the best/right way of doing this
  - I'm not completely sure about the usdt integration in bpf_program__attach_usdt,
    I was trying to detect uprobe_multi kernel support first, but ended up with
    just new field for struct bpf_usdt_opts
  - I plan to add more tests for usdt probes defined with refctr


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

There are PRs for tetragon [1] and bpftrace [2] support.

thanks,
jirka


[1] https://github.com/cilium/tetragon/pull/936
[2] https://github.com/olsajiri/bpftrace/tree/uprobe_multi

Cc: Viktor Malik <viktor.malik@gmail.com>
Cc: Daniel Xu <dxu@dxuuu.xyz>
---
Jiri Olsa (20):
      bpf: Add multi uprobe link
      bpf: Add cookies support for uprobe_multi link
      bpf: Add bpf_get_func_ip helper support for uprobe link
      libbpf: Update uapi bpf.h tools header
      libbpf: Add uprobe_multi attach type and link names
      libbpf: Factor elf_for_each_symbol function
      libbpf: Add elf_find_multi_func_offset function
      libbpf: Add elf_find_patern_func_offset function
      libbpf: Add bpf_link_create support for multi uprobes
      libbpf: Add bpf_program__attach_uprobe_multi_opts function
      libbpf: Add support for uprobe.multi/uprobe.multi program sections
      libbpf: Add uprobe multi link support to bpf_program__attach_usdt
      selftests/bpf: Add uprobe_multi skel test
      selftests/bpf: Add uprobe_multi api test
      selftests/bpf: Add uprobe_multi link test
      selftests/bpf: Add uprobe_multi test program
      selftests/bpf: Add uprobe_multi bench test
      selftests/bpf: Add usdt_multi test program
      selftests/bpf: Add usdt_multi bench test
      selftests/bpf: Add uprobe_multi cookie test

 include/linux/trace_events.h                               |   6 +
 include/uapi/linux/bpf.h                                   |  15 +++
 kernel/bpf/syscall.c                                       |  18 ++-
 kernel/trace/bpf_trace.c                                   | 310 +++++++++++++++++++++++++++++++++++++++++++-
 tools/include/uapi/linux/bpf.h                             |  15 +++
 tools/lib/bpf/bpf.c                                        |  10 ++
 tools/lib/bpf/bpf.h                                        |  10 +-
 tools/lib/bpf/libbpf.c                                     | 653 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------
 tools/lib/bpf/libbpf.h                                     |  43 ++++++
 tools/lib/bpf/libbpf.map                                   |   3 +
 tools/lib/bpf/libbpf_internal.h                            |   2 +-
 tools/lib/bpf/usdt.c                                       | 127 +++++++++++++-----
 tools/testing/selftests/bpf/Makefile                       |  10 ++
 tools/testing/selftests/bpf/prog_tests/bpf_cookie.c        |  78 +++++++++++
 tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c | 286 ++++++++++++++++++++++++++++++++++++++++
 tools/testing/selftests/bpf/progs/uprobe_multi.c           |  72 +++++++++++
 tools/testing/selftests/bpf/progs/uprobe_multi_usdt.c      |  16 +++
 tools/testing/selftests/bpf/uprobe_multi.c                 |  53 ++++++++
 tools/testing/selftests/bpf/usdt_multi.c                   |  23 ++++
 19 files changed, 1634 insertions(+), 116 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
 create mode 100644 tools/testing/selftests/bpf/progs/uprobe_multi.c
 create mode 100644 tools/testing/selftests/bpf/progs/uprobe_multi_usdt.c
 create mode 100644 tools/testing/selftests/bpf/uprobe_multi.c
 create mode 100644 tools/testing/selftests/bpf/usdt_multi.c

[RFC/PATCH bpf-next 01/20] bpf: Add multi uprobe link

[Jiri Olsa]

Adding new multi uprobe link that allows to attach bpf program
to multiple uprobes.

Uprobes to attach are specified via new link_create uprobe_multi
union:

  struct {
          __u32           flags;
          __u32           cnt;
          __aligned_u64   paths;
          __aligned_u64   offsets;
          __aligned_u64   ref_ctr_offsets;
  } uprobe_multi;

Uprobes are defined in paths/offsets/ref_ctr_offsets arrays with
the same 'cnt' length. Each uprobe is defined with a single index
in all three arrays:

  paths[idx], offsets[idx] and/or ref_ctr_offsets[idx]

The 'flags' supports single bit for now that marks the uprobe as
return probe.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 include/linux/trace_events.h |   6 +
 include/uapi/linux/bpf.h     |  14 +++
 kernel/bpf/syscall.c         |  16 ++-
 kernel/trace/bpf_trace.c     | 231 +++++++++++++++++++++++++++++++++++
 4 files changed, 265 insertions(+), 2 deletions(-)

diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 0e373222a6df..b0db245fc0f5 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -749,6 +749,7 @@ 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);
+int bpf_uprobe_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)
 {
@@ -795,6 +796,11 @@ bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
 {
 	return -EOPNOTSUPP;
 }
+static inline int
+bpf_uprobe_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 1bb11a6ee667..debc041c6ca5 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1035,6 +1035,7 @@ enum bpf_attach_type {
 	BPF_TRACE_KPROBE_MULTI,
 	BPF_LSM_CGROUP,
 	BPF_STRUCT_OPS,
+	BPF_TRACE_UPROBE_MULTI,
 	__MAX_BPF_ATTACH_TYPE
 };
 
@@ -1052,6 +1053,7 @@ enum bpf_link_type {
 	BPF_LINK_TYPE_KPROBE_MULTI = 8,
 	BPF_LINK_TYPE_STRUCT_OPS = 9,
 	BPF_LINK_TYPE_NETFILTER = 10,
+	BPF_LINK_TYPE_UPROBE_MULTI = 11,
 
 	MAX_BPF_LINK_TYPE,
 };
@@ -1169,6 +1171,11 @@ enum bpf_link_type {
  */
 #define BPF_F_KPROBE_MULTI_RETURN	(1U << 0)
 
+/* link_create.uprobe_multi.flags used in LINK_CREATE command for
+ * BPF_TRACE_UPROBE_MULTI attach type to create return probe.
+ */
+#define BPF_F_UPROBE_MULTI_RETURN	(1U << 0)
+
 /* When BPF ldimm64's insn[0].src_reg != 0 then this can have
  * the following extensions:
  *
@@ -1568,6 +1575,13 @@ union bpf_attr {
 				__s32		priority;
 				__u32		flags;
 			} netfilter;
+			struct {
+				__u32		flags;
+				__u32		cnt;
+				__aligned_u64	paths;
+				__aligned_u64	offsets;
+				__aligned_u64	ref_ctr_offsets;
+			} uprobe_multi;
 		};
 	} link_create;
 
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 14f39c1e573e..0b789a33317b 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -4601,7 +4601,8 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
 		break;
 	case BPF_PROG_TYPE_KPROBE:
 		if (attr->link_create.attach_type != BPF_PERF_EVENT &&
-		    attr->link_create.attach_type != BPF_TRACE_KPROBE_MULTI) {
+		    attr->link_create.attach_type != BPF_TRACE_KPROBE_MULTI &&
+		    attr->link_create.attach_type != BPF_TRACE_UPROBE_MULTI) {
 			ret = -EINVAL;
 			goto out;
 		}
@@ -4666,10 +4667,21 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
 		ret = bpf_perf_link_attach(attr, prog);
 		break;
 	case BPF_PROG_TYPE_KPROBE:
+		/* Ensure that program with eBPF_TRACE_UPROBE_MULTI attach type can
+		 * attach only to uprobe_multi link. It has its own runtime context
+		 * which is specific for get_func_ip/get_attach_cookie helpers.
+		 */
+		if (prog->expected_attach_type == BPF_TRACE_UPROBE_MULTI &&
+		    attr->link_create.attach_type != BPF_TRACE_UPROBE_MULTI) {
+			ret = -EINVAL;
+			goto out;
+		}
 		if (attr->link_create.attach_type == BPF_PERF_EVENT)
 			ret = bpf_perf_link_attach(attr, prog);
-		else
+		else if (attr->link_create.attach_type == BPF_TRACE_KPROBE_MULTI)
 			ret = bpf_kprobe_multi_link_attach(attr, prog);
+		else if (attr->link_create.attach_type == BPF_TRACE_UPROBE_MULTI)
+			ret = bpf_uprobe_multi_link_attach(attr, prog);
 		break;
 	default:
 		ret = -EINVAL;
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index bcf91bc7bf71..b84a7d01abf4 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -23,6 +23,7 @@
 #include <linux/sort.h>
 #include <linux/key.h>
 #include <linux/verification.h>
+#include <linux/namei.h>
 
 #include <net/bpf_sk_storage.h>
 
@@ -2901,3 +2902,233 @@ static u64 bpf_kprobe_multi_entry_ip(struct bpf_run_ctx *ctx)
 	return 0;
 }
 #endif
+
+#ifdef CONFIG_UPROBES
+struct bpf_uprobe_multi_link;
+
+struct bpf_uprobe {
+	struct bpf_uprobe_multi_link *link;
+	struct inode *inode;
+	loff_t offset;
+	loff_t ref_ctr_offset;
+	struct uprobe_consumer consumer;
+};
+
+struct bpf_uprobe_multi_link {
+	struct bpf_link link;
+	u32 cnt;
+	struct bpf_uprobe *uprobes;
+};
+
+struct bpf_uprobe_multi_run_ctx {
+	struct bpf_run_ctx run_ctx;
+	unsigned long entry_ip;
+};
+
+static void bpf_uprobe_unregister(struct bpf_uprobe *uprobes, u32 cnt)
+{
+	u32 i;
+
+	for (i = 0; i < cnt; i++) {
+		uprobe_unregister(uprobes[i].inode, uprobes[i].offset,
+				  &uprobes[i].consumer);
+	}
+}
+
+static void bpf_uprobe_multi_link_release(struct bpf_link *link)
+{
+	struct bpf_uprobe_multi_link *umulti_link;
+
+	umulti_link = container_of(link, struct bpf_uprobe_multi_link, link);
+	bpf_uprobe_unregister(umulti_link->uprobes, umulti_link->cnt);
+}
+
+static void bpf_uprobe_multi_link_dealloc(struct bpf_link *link)
+{
+	struct bpf_uprobe_multi_link *umulti_link;
+
+	umulti_link = container_of(link, struct bpf_uprobe_multi_link, link);
+	kvfree(umulti_link->uprobes);
+	kfree(umulti_link);
+}
+
+static const struct bpf_link_ops bpf_uprobe_multi_link_lops = {
+	.release = bpf_uprobe_multi_link_release,
+	.dealloc = bpf_uprobe_multi_link_dealloc,
+};
+
+static int uprobe_prog_run(struct bpf_uprobe *uprobe,
+			   unsigned long entry_ip,
+			   struct pt_regs *regs)
+{
+	struct bpf_uprobe_multi_link *link = uprobe->link;
+	struct bpf_uprobe_multi_run_ctx run_ctx = {
+		.entry_ip = entry_ip,
+	};
+	struct bpf_run_ctx *old_run_ctx;
+	int err;
+
+	preempt_disable();
+
+	if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) {
+		err = 0;
+		goto out;
+	}
+
+	rcu_read_lock();
+	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();
+
+ out:
+	__this_cpu_dec(bpf_prog_active);
+	preempt_enable();
+	return err;
+}
+
+static int
+uprobe_multi_link_handler(struct uprobe_consumer *con, struct pt_regs *regs)
+{
+	struct bpf_uprobe *uprobe;
+
+	uprobe = container_of(con, struct bpf_uprobe, consumer);
+	return uprobe_prog_run(uprobe, instruction_pointer(regs), regs);
+}
+
+static int
+uprobe_multi_link_ret_handler(struct uprobe_consumer *con, unsigned long func, struct pt_regs *regs)
+{
+	struct bpf_uprobe *uprobe;
+
+	uprobe = container_of(con, struct bpf_uprobe, consumer);
+	return uprobe_prog_run(uprobe, func, regs);
+}
+
+int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
+{
+	void __user **upaths, __user *upath, __user *old_upath = NULL;
+	unsigned long __user *uref_ctr_offsets, ref_ctr_offset = 0;
+	struct bpf_uprobe_multi_link *link = NULL;
+	unsigned long __user *uoffsets, offset;
+	struct bpf_link_primer link_primer;
+	struct bpf_uprobe *uprobes = NULL;
+	struct inode *inode, *old_inode;
+	u32 flags, cnt, i;
+	int err;
+
+	/* no support for 32bit archs yet */
+	if (sizeof(u64) != sizeof(void *))
+		return -EOPNOTSUPP;
+
+	if (prog->expected_attach_type != BPF_TRACE_UPROBE_MULTI)
+		return -EINVAL;
+
+	flags = attr->link_create.uprobe_multi.flags;
+	if (flags & ~BPF_F_UPROBE_MULTI_RETURN)
+		return -EINVAL;
+
+	upaths = u64_to_user_ptr(attr->link_create.uprobe_multi.paths);
+	uoffsets = u64_to_user_ptr(attr->link_create.uprobe_multi.offsets);
+	if (!!upaths != !!uoffsets)
+		return -EINVAL;
+
+	uref_ctr_offsets = u64_to_user_ptr(attr->link_create.uprobe_multi.ref_ctr_offsets);
+
+	cnt = attr->link_create.uprobe_multi.cnt;
+	if (!cnt)
+		return -EINVAL;
+
+	uprobes = kvcalloc(cnt, sizeof(*uprobes), GFP_KERNEL);
+	if (!uprobes)
+		return -ENOMEM;
+
+	link = kzalloc(sizeof(*link), GFP_KERNEL);
+	if (!link) {
+		err = -ENOMEM;
+		goto error;
+	}
+
+	for (i = 0; i < cnt; i++) {
+		if (uref_ctr_offsets && __get_user(ref_ctr_offset, uref_ctr_offsets + i)) {
+			err = -EFAULT;
+			goto error;
+		}
+		if (__get_user(offset, uoffsets + i)) {
+			err = -EFAULT;
+			goto error;
+		}
+		if (__get_user(upath, upaths + i)) {
+			err = -EFAULT;
+			goto error;
+		}
+		if (i && old_upath == upath) {
+			inode = old_inode;
+		} else {
+			struct path path;
+			char *name;
+
+			name = strndup_user(upath, PATH_MAX);
+			if (IS_ERR(name)) {
+				err = PTR_ERR(name);
+				goto error;
+			}
+			err = kern_path(name, LOOKUP_FOLLOW, &path);
+			kfree(name);
+			if (err)
+				goto error;
+			if (!d_is_reg(path.dentry)) {
+				err = -EINVAL;
+				path_put(&path);
+				goto error;
+			}
+			inode = d_real_inode(path.dentry);
+			path_put(&path);
+		}
+		old_upath = upath;
+
+		uprobes[i].inode = inode;
+		uprobes[i].offset = offset;
+		uprobes[i].ref_ctr_offset = ref_ctr_offset;
+		uprobes[i].link = link;
+
+		if (flags & BPF_F_UPROBE_MULTI_RETURN)
+			uprobes[i].consumer.ret_handler = uprobe_multi_link_ret_handler;
+		else
+			uprobes[i].consumer.handler = uprobe_multi_link_handler;
+	}
+
+	link->cnt = cnt;
+	link->uprobes = uprobes;
+
+	bpf_link_init(&link->link, BPF_LINK_TYPE_UPROBE_MULTI,
+		      &bpf_uprobe_multi_link_lops, prog);
+
+	err = bpf_link_prime(&link->link, &link_primer);
+	if (err)
+		goto error;
+
+	for (i = 0; i < cnt; i++) {
+		err = uprobe_register_refctr(uprobes[i].inode, uprobes[i].offset,
+					     uprobes[i].ref_ctr_offset,
+					     &uprobes[i].consumer);
+		if (err) {
+			bpf_uprobe_unregister(uprobes, i);
+			bpf_link_cleanup(&link_primer);
+			return err;
+		}
+	}
+
+	return bpf_link_settle(&link_primer);
+
+error:
+	kfree(uprobes);
+	kfree(link);
+	return err;
+}
+#else /* !CONFIG_UPROBES */
+int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
+{
+	return -EOPNOTSUPP;
+}
+#endif /* CONFIG_UPROBES */
-- 
2.40.0

Re: [RFC/PATCH bpf-next 01/20] bpf: Add multi uprobe link

[Alexei Starovoitov]

On Mon, Apr 24, 2023 at 06:04:28PM +0200, Jiri Olsa wrote:
> +
> +static int uprobe_prog_run(struct bpf_uprobe *uprobe,
> +			   unsigned long entry_ip,
> +			   struct pt_regs *regs)
> +{
> +	struct bpf_uprobe_multi_link *link = uprobe->link;
> +	struct bpf_uprobe_multi_run_ctx run_ctx = {
> +		.entry_ip = entry_ip,
> +	};
> +	struct bpf_run_ctx *old_run_ctx;
> +	int err;
> +
> +	preempt_disable();

preempt_disable? Which year is this? :)
Let's allow sleepable from the start.
See bpf_prog_run_array_sleepable.

Other than this the set looks great.

> +
> +	if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) {
> +		err = 0;
> +		goto out;
> +	}
> +
> +	rcu_read_lock();
> +	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();
> +
> + out:
> +	__this_cpu_dec(bpf_prog_active);
> +	preempt_enable();
> +	return err;
...
> +			struct path path;
> +			char *name;
> +
> +			name = strndup_user(upath, PATH_MAX);
> +			if (IS_ERR(name)) {
> +				err = PTR_ERR(name);
> +				goto error;
> +			}
> +			err = kern_path(name, LOOKUP_FOLLOW, &path);
> +			kfree(name);
> +			if (err)
> +				goto error;
> +			if (!d_is_reg(path.dentry)) {
> +				err = -EINVAL;
> +				path_put(&path);
> +				goto error;
> +			}
> +			inode = d_real_inode(path.dentry);
> +			path_put(&path);

inode won't disappear between here and its use in uprobe_register_refctr ?

> +		}
> +		old_upath = upath;
> +
> +		uprobes[i].inode = inode;
> +		uprobes[i].offset = offset;
> +		uprobes[i].ref_ctr_offset = ref_ctr_offset;
> +		uprobes[i].link = link;
> +
> +		if (flags & BPF_F_UPROBE_MULTI_RETURN)
> +			uprobes[i].consumer.ret_handler = uprobe_multi_link_ret_handler;
> +		else
> +			uprobes[i].consumer.handler = uprobe_multi_link_handler;
> +	}
> +
> +	link->cnt = cnt;
> +	link->uprobes = uprobes;
> +
> +	bpf_link_init(&link->link, BPF_LINK_TYPE_UPROBE_MULTI,
> +		      &bpf_uprobe_multi_link_lops, prog);
> +
> +	err = bpf_link_prime(&link->link, &link_primer);
> +	if (err)
> +		goto error;
> +
> +	for (i = 0; i < cnt; i++) {
> +		err = uprobe_register_refctr(uprobes[i].inode, uprobes[i].offset,
> +					     uprobes[i].ref_ctr_offset,
> +					     &uprobes[i].consumer);

Re: [RFC/PATCH bpf-next 01/20] bpf: Add multi uprobe link

[Jiri Olsa]

On Mon, Apr 24, 2023 at 03:11:20PM -0700, Alexei Starovoitov wrote:
> On Mon, Apr 24, 2023 at 06:04:28PM +0200, Jiri Olsa wrote:
> > +
> > +static int uprobe_prog_run(struct bpf_uprobe *uprobe,
> > +			   unsigned long entry_ip,
> > +			   struct pt_regs *regs)
> > +{
> > +	struct bpf_uprobe_multi_link *link = uprobe->link;
> > +	struct bpf_uprobe_multi_run_ctx run_ctx = {
> > +		.entry_ip = entry_ip,
> > +	};
> > +	struct bpf_run_ctx *old_run_ctx;
> > +	int err;
> > +
> > +	preempt_disable();
> 
> preempt_disable? Which year is this? :)
> Let's allow sleepable from the start.
> See bpf_prog_run_array_sleepable.

ok, we should probably add also 'multi.uprobe.s' section so the program
gets loaded with the BPF_F_SLEEPABLE flag.. or maybe we can enable that
by default for 'multi.uprobe' section

> 
> Other than this the set looks great.
> 
> > +
> > +	if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) {
> > +		err = 0;
> > +		goto out;
> > +	}
> > +
> > +	rcu_read_lock();
> > +	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();
> > +
> > + out:
> > +	__this_cpu_dec(bpf_prog_active);
> > +	preempt_enable();
> > +	return err;
> ...
> > +			struct path path;
> > +			char *name;
> > +
> > +			name = strndup_user(upath, PATH_MAX);
> > +			if (IS_ERR(name)) {
> > +				err = PTR_ERR(name);
> > +				goto error;
> > +			}
> > +			err = kern_path(name, LOOKUP_FOLLOW, &path);
> > +			kfree(name);
> > +			if (err)
> > +				goto error;
> > +			if (!d_is_reg(path.dentry)) {
> > +				err = -EINVAL;
> > +				path_put(&path);
> > +				goto error;
> > +			}
> > +			inode = d_real_inode(path.dentry);
> > +			path_put(&path);
> 
> inode won't disappear between here and its use in uprobe_register_refctr ?

ugh, that's bug.. will fix

thanks,
jirka

> 
> > +		}
> > +		old_upath = upath;
> > +
> > +		uprobes[i].inode = inode;
> > +		uprobes[i].offset = offset;
> > +		uprobes[i].ref_ctr_offset = ref_ctr_offset;
> > +		uprobes[i].link = link;
> > +
> > +		if (flags & BPF_F_UPROBE_MULTI_RETURN)
> > +			uprobes[i].consumer.ret_handler = uprobe_multi_link_ret_handler;
> > +		else
> > +			uprobes[i].consumer.handler = uprobe_multi_link_handler;
> > +	}
> > +
> > +	link->cnt = cnt;
> > +	link->uprobes = uprobes;
> > +
> > +	bpf_link_init(&link->link, BPF_LINK_TYPE_UPROBE_MULTI,
> > +		      &bpf_uprobe_multi_link_lops, prog);
> > +
> > +	err = bpf_link_prime(&link->link, &link_primer);
> > +	if (err)
> > +		goto error;
> > +
> > +	for (i = 0; i < cnt; i++) {
> > +		err = uprobe_register_refctr(uprobes[i].inode, uprobes[i].offset,
> > +					     uprobes[i].ref_ctr_offset,
> > +					     &uprobes[i].consumer);

Re: [RFC/PATCH bpf-next 01/20] bpf: Add multi uprobe link

[Andrii Nakryiko]

On Tue, Apr 25, 2023 at 2:55 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Mon, Apr 24, 2023 at 03:11:20PM -0700, Alexei Starovoitov wrote:
> > On Mon, Apr 24, 2023 at 06:04:28PM +0200, Jiri Olsa wrote:
> > > +
> > > +static int uprobe_prog_run(struct bpf_uprobe *uprobe,
> > > +                      unsigned long entry_ip,
> > > +                      struct pt_regs *regs)
> > > +{
> > > +   struct bpf_uprobe_multi_link *link = uprobe->link;
> > > +   struct bpf_uprobe_multi_run_ctx run_ctx = {
> > > +           .entry_ip = entry_ip,
> > > +   };
> > > +   struct bpf_run_ctx *old_run_ctx;
> > > +   int err;
> > > +
> > > +   preempt_disable();
> >
> > preempt_disable? Which year is this? :)
> > Let's allow sleepable from the start.
> > See bpf_prog_run_array_sleepable.
>
> ok, we should probably add also 'multi.uprobe.s' section so the program
> gets loaded with the BPF_F_SLEEPABLE flag.. or maybe we can enable that
> by default for 'multi.uprobe' section

"uprobe.multi.s" rather, to follow "kprobe.multi.s". But we can't make
it sleepable always/by default. Sleepable BPF programs are not just
better types of programs, they have their own limitations, so it has
to be the user's choice to go with sleepable or non-sleepable.

>
> >
> > Other than this the set looks great.
> >
> > > +
> > > +   if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) {
> > > +           err = 0;
> > > +           goto out;
> > > +   }
> > > +
> > > +   rcu_read_lock();
> > > +   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();
> > > +

[...]

Re: [RFC/PATCH bpf-next 01/20] bpf: Add multi uprobe link

[Jiri Olsa]

On Wed, Apr 26, 2023 at 12:01:47PM -0700, Andrii Nakryiko wrote:
> On Tue, Apr 25, 2023 at 2:55 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Mon, Apr 24, 2023 at 03:11:20PM -0700, Alexei Starovoitov wrote:
> > > On Mon, Apr 24, 2023 at 06:04:28PM +0200, Jiri Olsa wrote:
> > > > +
> > > > +static int uprobe_prog_run(struct bpf_uprobe *uprobe,
> > > > +                      unsigned long entry_ip,
> > > > +                      struct pt_regs *regs)
> > > > +{
> > > > +   struct bpf_uprobe_multi_link *link = uprobe->link;
> > > > +   struct bpf_uprobe_multi_run_ctx run_ctx = {
> > > > +           .entry_ip = entry_ip,
> > > > +   };
> > > > +   struct bpf_run_ctx *old_run_ctx;
> > > > +   int err;
> > > > +
> > > > +   preempt_disable();
> > >
> > > preempt_disable? Which year is this? :)
> > > Let's allow sleepable from the start.
> > > See bpf_prog_run_array_sleepable.
> >
> > ok, we should probably add also 'multi.uprobe.s' section so the program
> > gets loaded with the BPF_F_SLEEPABLE flag.. or maybe we can enable that
> > by default for 'multi.uprobe' section
> 
> "uprobe.multi.s" rather, to follow "kprobe.multi.s". But we can't make
> it sleepable always/by default. Sleepable BPF programs are not just
> better types of programs, they have their own limitations, so it has
> to be the user's choice to go with sleepable or non-sleepable.

ok, will add uprobe.multi.s

thanks,
jirka

> 
> >
> > >
> > > Other than this the set looks great.
> > >
> > > > +
> > > > +   if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) {
> > > > +           err = 0;
> > > > +           goto out;
> > > > +   }
> > > > +
> > > > +   rcu_read_lock();
> > > > +   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();
> > > > +
> 
> [...]

Re: [RFC/PATCH bpf-next 01/20] bpf: Add multi uprobe link

[Yonghong Song]

On 4/24/23 9:04 AM, Jiri Olsa wrote:
> Adding new multi uprobe link that allows to attach bpf program
> to multiple uprobes.
> 
> Uprobes to attach are specified via new link_create uprobe_multi
> union:
> 
>    struct {
>            __u32           flags;
>            __u32           cnt;
>            __aligned_u64   paths;
>            __aligned_u64   offsets;
>            __aligned_u64   ref_ctr_offsets;
>    } uprobe_multi;
> 
> Uprobes are defined in paths/offsets/ref_ctr_offsets arrays with
> the same 'cnt' length. Each uprobe is defined with a single index
> in all three arrays:
> 
>    paths[idx], offsets[idx] and/or ref_ctr_offsets[idx]

paths[idx], offsets[idx] and optional ref_ctr_offsets[idx]?

> 
> The 'flags' supports single bit for now that marks the uprobe as
> return probe.
> 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>   include/linux/trace_events.h |   6 +
>   include/uapi/linux/bpf.h     |  14 +++
>   kernel/bpf/syscall.c         |  16 ++-
>   kernel/trace/bpf_trace.c     | 231 +++++++++++++++++++++++++++++++++++
>   4 files changed, 265 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
> index 0e373222a6df..b0db245fc0f5 100644
> --- a/include/linux/trace_events.h
> +++ b/include/linux/trace_events.h
> @@ -749,6 +749,7 @@ 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);
> +int bpf_uprobe_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)
>   {
> @@ -795,6 +796,11 @@ bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
>   {
>   	return -EOPNOTSUPP;
>   }
> +static inline int
> +bpf_uprobe_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 1bb11a6ee667..debc041c6ca5 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1035,6 +1035,7 @@ enum bpf_attach_type {
>   	BPF_TRACE_KPROBE_MULTI,
>   	BPF_LSM_CGROUP,
>   	BPF_STRUCT_OPS,
> +	BPF_TRACE_UPROBE_MULTI,
>   	__MAX_BPF_ATTACH_TYPE
>   };
>   
> @@ -1052,6 +1053,7 @@ enum bpf_link_type {
>   	BPF_LINK_TYPE_KPROBE_MULTI = 8,
>   	BPF_LINK_TYPE_STRUCT_OPS = 9,
>   	BPF_LINK_TYPE_NETFILTER = 10,
> +	BPF_LINK_TYPE_UPROBE_MULTI = 11,
>   
>   	MAX_BPF_LINK_TYPE,
>   };
> @@ -1169,6 +1171,11 @@ enum bpf_link_type {
>    */
>   #define BPF_F_KPROBE_MULTI_RETURN	(1U << 0)
>   
> +/* link_create.uprobe_multi.flags used in LINK_CREATE command for
> + * BPF_TRACE_UPROBE_MULTI attach type to create return probe.
> + */
> +#define BPF_F_UPROBE_MULTI_RETURN	(1U << 0)
> +
>   /* When BPF ldimm64's insn[0].src_reg != 0 then this can have
>    * the following extensions:
>    *
> @@ -1568,6 +1575,13 @@ union bpf_attr {
>   				__s32		priority;
>   				__u32		flags;
>   			} netfilter;
> +			struct {
> +				__u32		flags;
> +				__u32		cnt;
> +				__aligned_u64	paths;
> +				__aligned_u64	offsets;
> +				__aligned_u64	ref_ctr_offsets;
> +			} uprobe_multi;
>   		};
>   	} link_create;
>   
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 14f39c1e573e..0b789a33317b 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -4601,7 +4601,8 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
>   		break;
>   	case BPF_PROG_TYPE_KPROBE:
>   		if (attr->link_create.attach_type != BPF_PERF_EVENT &&
> -		    attr->link_create.attach_type != BPF_TRACE_KPROBE_MULTI) {
> +		    attr->link_create.attach_type != BPF_TRACE_KPROBE_MULTI &&
> +		    attr->link_create.attach_type != BPF_TRACE_UPROBE_MULTI) {
>   			ret = -EINVAL;
>   			goto out;
>   		}
> @@ -4666,10 +4667,21 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
>   		ret = bpf_perf_link_attach(attr, prog);
>   		break;
>   	case BPF_PROG_TYPE_KPROBE:
> +		/* Ensure that program with eBPF_TRACE_UPROBE_MULTI attach type can
> +		 * attach only to uprobe_multi link. It has its own runtime context
> +		 * which is specific for get_func_ip/get_attach_cookie helpers.
> +		 */
> +		if (prog->expected_attach_type == BPF_TRACE_UPROBE_MULTI &&
> +		    attr->link_create.attach_type != BPF_TRACE_UPROBE_MULTI) {
> +			ret = -EINVAL;
> +			goto out;
> +		}

The above seems redundant since it is checked in 
bpf_uprobe_multi_link_attach().
That is why the BPF_TRACE_KPROBE_MULTI is not checked here since
bpf_kprobe_multi_link_attach() checks it.

>   		if (attr->link_create.attach_type == BPF_PERF_EVENT)
>   			ret = bpf_perf_link_attach(attr, prog);
> -		else
> +		else if (attr->link_create.attach_type == BPF_TRACE_KPROBE_MULTI)
>   			ret = bpf_kprobe_multi_link_attach(attr, prog);
> +		else if (attr->link_create.attach_type == BPF_TRACE_UPROBE_MULTI)
> +			ret = bpf_uprobe_multi_link_attach(attr, prog);
>   		break;
>   	default:
>   		ret = -EINVAL;
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index bcf91bc7bf71..b84a7d01abf4 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -23,6 +23,7 @@
>   #include <linux/sort.h>
>   #include <linux/key.h>
>   #include <linux/verification.h>
> +#include <linux/namei.h>
>   
>   #include <net/bpf_sk_storage.h>
>   
> @@ -2901,3 +2902,233 @@ static u64 bpf_kprobe_multi_entry_ip(struct bpf_run_ctx *ctx)
>   	return 0;
>   }
>   #endif
> +
> +#ifdef CONFIG_UPROBES
> +struct bpf_uprobe_multi_link;
> +
> +struct bpf_uprobe {
> +	struct bpf_uprobe_multi_link *link;
> +	struct inode *inode;
> +	loff_t offset;
> +	loff_t ref_ctr_offset;
> +	struct uprobe_consumer consumer;
> +};
> +
> +struct bpf_uprobe_multi_link {
> +	struct bpf_link link;
> +	u32 cnt;
> +	struct bpf_uprobe *uprobes;
> +};
> +
> +struct bpf_uprobe_multi_run_ctx {
> +	struct bpf_run_ctx run_ctx;
> +	unsigned long entry_ip;
> +};
> +
> +static void bpf_uprobe_unregister(struct bpf_uprobe *uprobes, u32 cnt)
> +{
> +	u32 i;
> +
> +	for (i = 0; i < cnt; i++) {
> +		uprobe_unregister(uprobes[i].inode, uprobes[i].offset,
> +				  &uprobes[i].consumer);
> +	}
> +}
> +
> +static void bpf_uprobe_multi_link_release(struct bpf_link *link)
> +{
> +	struct bpf_uprobe_multi_link *umulti_link;
> +
> +	umulti_link = container_of(link, struct bpf_uprobe_multi_link, link);
> +	bpf_uprobe_unregister(umulti_link->uprobes, umulti_link->cnt);
> +}
> +
> +static void bpf_uprobe_multi_link_dealloc(struct bpf_link *link)
> +{
> +	struct bpf_uprobe_multi_link *umulti_link;
> +
> +	umulti_link = container_of(link, struct bpf_uprobe_multi_link, link);
> +	kvfree(umulti_link->uprobes);
> +	kfree(umulti_link);
> +}
> +
> +static const struct bpf_link_ops bpf_uprobe_multi_link_lops = {
> +	.release = bpf_uprobe_multi_link_release,
> +	.dealloc = bpf_uprobe_multi_link_dealloc,
> +};
> +
> +static int uprobe_prog_run(struct bpf_uprobe *uprobe,
> +			   unsigned long entry_ip,
> +			   struct pt_regs *regs)
> +{
> +	struct bpf_uprobe_multi_link *link = uprobe->link;
> +	struct bpf_uprobe_multi_run_ctx run_ctx = {
> +		.entry_ip = entry_ip,
> +	};
> +	struct bpf_run_ctx *old_run_ctx;
> +	int err;
> +
> +	preempt_disable();

Alexei has pointed out here.
preempt_disable() is not favored.
We should use migrate_disable/enable().
For non sleepable program, the below rcu_read_lock() is okay.
For sleepable program, use rcu_read_lock_trace().
See __bpf_prog_enter_sleepable_recur() in trampoline.c as
an example.

> +
> +	if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) {
> +		err = 0;
> +		goto out;
> +	}
> +
> +	rcu_read_lock();
> +	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();
> +
> + out:
> +	__this_cpu_dec(bpf_prog_active);
> +	preempt_enable();
> +	return err;
> +}
> +
[...]

Re: [RFC/PATCH bpf-next 01/20] bpf: Add multi uprobe link

[Jiri Olsa]

On Tue, Apr 25, 2023 at 04:56:00PM -0700, Yonghong Song wrote:
> 
> 
> On 4/24/23 9:04 AM, Jiri Olsa wrote:
> > Adding new multi uprobe link that allows to attach bpf program
> > to multiple uprobes.
> > 
> > Uprobes to attach are specified via new link_create uprobe_multi
> > union:
> > 
> >    struct {
> >            __u32           flags;
> >            __u32           cnt;
> >            __aligned_u64   paths;
> >            __aligned_u64   offsets;
> >            __aligned_u64   ref_ctr_offsets;
> >    } uprobe_multi;
> > 
> > Uprobes are defined in paths/offsets/ref_ctr_offsets arrays with
> > the same 'cnt' length. Each uprobe is defined with a single index
> > in all three arrays:
> > 
> >    paths[idx], offsets[idx] and/or ref_ctr_offsets[idx]
> 
> paths[idx], offsets[idx] and optional ref_ctr_offsets[idx]?

yes

> 
> > 
> > The 'flags' supports single bit for now that marks the uprobe as
> > return probe.
> > 
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >   include/linux/trace_events.h |   6 +
> >   include/uapi/linux/bpf.h     |  14 +++
> >   kernel/bpf/syscall.c         |  16 ++-
> >   kernel/trace/bpf_trace.c     | 231 +++++++++++++++++++++++++++++++++++
> >   4 files changed, 265 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
> > index 0e373222a6df..b0db245fc0f5 100644
> > --- a/include/linux/trace_events.h
> > +++ b/include/linux/trace_events.h
> > @@ -749,6 +749,7 @@ 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);
> > +int bpf_uprobe_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)
> >   {
> > @@ -795,6 +796,11 @@ bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
> >   {
> >   	return -EOPNOTSUPP;
> >   }
> > +static inline int
> > +bpf_uprobe_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 1bb11a6ee667..debc041c6ca5 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -1035,6 +1035,7 @@ enum bpf_attach_type {
> >   	BPF_TRACE_KPROBE_MULTI,
> >   	BPF_LSM_CGROUP,
> >   	BPF_STRUCT_OPS,
> > +	BPF_TRACE_UPROBE_MULTI,
> >   	__MAX_BPF_ATTACH_TYPE
> >   };
> > @@ -1052,6 +1053,7 @@ enum bpf_link_type {
> >   	BPF_LINK_TYPE_KPROBE_MULTI = 8,
> >   	BPF_LINK_TYPE_STRUCT_OPS = 9,
> >   	BPF_LINK_TYPE_NETFILTER = 10,
> > +	BPF_LINK_TYPE_UPROBE_MULTI = 11,
> >   	MAX_BPF_LINK_TYPE,
> >   };
> > @@ -1169,6 +1171,11 @@ enum bpf_link_type {
> >    */
> >   #define BPF_F_KPROBE_MULTI_RETURN	(1U << 0)
> > +/* link_create.uprobe_multi.flags used in LINK_CREATE command for
> > + * BPF_TRACE_UPROBE_MULTI attach type to create return probe.
> > + */
> > +#define BPF_F_UPROBE_MULTI_RETURN	(1U << 0)
> > +
> >   /* When BPF ldimm64's insn[0].src_reg != 0 then this can have
> >    * the following extensions:
> >    *
> > @@ -1568,6 +1575,13 @@ union bpf_attr {
> >   				__s32		priority;
> >   				__u32		flags;
> >   			} netfilter;
> > +			struct {
> > +				__u32		flags;
> > +				__u32		cnt;
> > +				__aligned_u64	paths;
> > +				__aligned_u64	offsets;
> > +				__aligned_u64	ref_ctr_offsets;
> > +			} uprobe_multi;
> >   		};
> >   	} link_create;
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index 14f39c1e573e..0b789a33317b 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -4601,7 +4601,8 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
> >   		break;
> >   	case BPF_PROG_TYPE_KPROBE:
> >   		if (attr->link_create.attach_type != BPF_PERF_EVENT &&
> > -		    attr->link_create.attach_type != BPF_TRACE_KPROBE_MULTI) {
> > +		    attr->link_create.attach_type != BPF_TRACE_KPROBE_MULTI &&
> > +		    attr->link_create.attach_type != BPF_TRACE_UPROBE_MULTI) {
> >   			ret = -EINVAL;
> >   			goto out;
> >   		}
> > @@ -4666,10 +4667,21 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
> >   		ret = bpf_perf_link_attach(attr, prog);
> >   		break;
> >   	case BPF_PROG_TYPE_KPROBE:
> > +		/* Ensure that program with eBPF_TRACE_UPROBE_MULTI attach type can
> > +		 * attach only to uprobe_multi link. It has its own runtime context
> > +		 * which is specific for get_func_ip/get_attach_cookie helpers.
> > +		 */
> > +		if (prog->expected_attach_type == BPF_TRACE_UPROBE_MULTI &&
> > +		    attr->link_create.attach_type != BPF_TRACE_UPROBE_MULTI) {
> > +			ret = -EINVAL;
> > +			goto out;
> > +		}
> 
> The above seems redundant since it is checked in
> bpf_uprobe_multi_link_attach().
> That is why the BPF_TRACE_KPROBE_MULTI is not checked here since
> bpf_kprobe_multi_link_attach() checks it.

for standard kprobe type program we do not check expected_attach_type,
but get_func_ip/get_attach_cookie helpers functions are picked based on
that:

        case BPF_FUNC_get_attach_cookie:
                if (prog->expected_attach_type == BPF_TRACE_KPROBE_MULTI)
                        return &bpf_get_attach_cookie_proto_kmulti;
                if (prog->expected_attach_type == BPF_TRACE_UPROBE_MULTI)
                        return &bpf_get_attach_cookie_proto_umulti;
                return &bpf_get_attach_cookie_proto_trace;

so standard kprobe attached through BPF_PERF_EVENT would run BPF_TRACE_UPROBE_MULTI
version of the helper and crash, because there's different context used

it's probably a problem for kprobe_multi as well, I'll check and have
separate patch for that

> > +static void bpf_uprobe_multi_link_dealloc(struct bpf_link *link)
> > +{
> > +	struct bpf_uprobe_multi_link *umulti_link;
> > +
> > +	umulti_link = container_of(link, struct bpf_uprobe_multi_link, link);
> > +	kvfree(umulti_link->uprobes);
> > +	kfree(umulti_link);
> > +}
> > +
> > +static const struct bpf_link_ops bpf_uprobe_multi_link_lops = {
> > +	.release = bpf_uprobe_multi_link_release,
> > +	.dealloc = bpf_uprobe_multi_link_dealloc,
> > +};
> > +
> > +static int uprobe_prog_run(struct bpf_uprobe *uprobe,
> > +			   unsigned long entry_ip,
> > +			   struct pt_regs *regs)
> > +{
> > +	struct bpf_uprobe_multi_link *link = uprobe->link;
> > +	struct bpf_uprobe_multi_run_ctx run_ctx = {
> > +		.entry_ip = entry_ip,
> > +	};
> > +	struct bpf_run_ctx *old_run_ctx;
> > +	int err;
> > +
> > +	preempt_disable();
> 
> Alexei has pointed out here.
> preempt_disable() is not favored.
> We should use migrate_disable/enable().
> For non sleepable program, the below rcu_read_lock() is okay.
> For sleepable program, use rcu_read_lock_trace().
> See __bpf_prog_enter_sleepable_recur() in trampoline.c as
> an example.

yes, I'll fix that

thanks,
jirka

Re: [RFC/PATCH bpf-next 01/20] bpf: Add multi uprobe link

[Andrii Nakryiko]

On Mon, Apr 24, 2023 at 9:05 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding new multi uprobe link that allows to attach bpf program
> to multiple uprobes.
>
> Uprobes to attach are specified via new link_create uprobe_multi
> union:
>
>   struct {
>           __u32           flags;
>           __u32           cnt;
>           __aligned_u64   paths;
>           __aligned_u64   offsets;
>           __aligned_u64   ref_ctr_offsets;
>   } uprobe_multi;
>
> Uprobes are defined in paths/offsets/ref_ctr_offsets arrays with
> the same 'cnt' length. Each uprobe is defined with a single index
> in all three arrays:
>
>   paths[idx], offsets[idx] and/or ref_ctr_offsets[idx]
>
> The 'flags' supports single bit for now that marks the uprobe as
> return probe.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  include/linux/trace_events.h |   6 +
>  include/uapi/linux/bpf.h     |  14 +++
>  kernel/bpf/syscall.c         |  16 ++-
>  kernel/trace/bpf_trace.c     | 231 +++++++++++++++++++++++++++++++++++
>  4 files changed, 265 insertions(+), 2 deletions(-)
>

[...]

> @@ -4666,10 +4667,21 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
>                 ret = bpf_perf_link_attach(attr, prog);
>                 break;
>         case BPF_PROG_TYPE_KPROBE:
> +               /* Ensure that program with eBPF_TRACE_UPROBE_MULTI attach type can

eBPF_TRACE_UPROBE_MULTI :)

> +                * attach only to uprobe_multi link. It has its own runtime context
> +                * which is specific for get_func_ip/get_attach_cookie helpers.
> +                */
> +               if (prog->expected_attach_type == BPF_TRACE_UPROBE_MULTI &&
> +                   attr->link_create.attach_type != BPF_TRACE_UPROBE_MULTI) {
> +                       ret = -EINVAL;
> +                       goto out;
> +               }

as Yonghong pointed out, you check this condition in
bpf_uprobe_multi_link_attach() already, so why redundant check?

>                 if (attr->link_create.attach_type == BPF_PERF_EVENT)
>                         ret = bpf_perf_link_attach(attr, prog);
> -               else
> +               else if (attr->link_create.attach_type == BPF_TRACE_KPROBE_MULTI)
>                         ret = bpf_kprobe_multi_link_attach(attr, prog);
> +               else if (attr->link_create.attach_type == BPF_TRACE_UPROBE_MULTI)
> +                       ret = bpf_uprobe_multi_link_attach(attr, prog);
>                 break;
>         default:
>                 ret = -EINVAL;
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index bcf91bc7bf71..b84a7d01abf4 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -23,6 +23,7 @@
>  #include <linux/sort.h>
>  #include <linux/key.h>
>  #include <linux/verification.h>
> +#include <linux/namei.h>
>
>  #include <net/bpf_sk_storage.h>
>
> @@ -2901,3 +2902,233 @@ static u64 bpf_kprobe_multi_entry_ip(struct bpf_run_ctx *ctx)
>         return 0;
>  }
>  #endif
> +
> +#ifdef CONFIG_UPROBES
> +struct bpf_uprobe_multi_link;
> +
> +struct bpf_uprobe {
> +       struct bpf_uprobe_multi_link *link;
> +       struct inode *inode;
> +       loff_t offset;
> +       loff_t ref_ctr_offset;

you seem to need this only during link creation, so we are wasting 8
bytes here per each instance of bpf_uprobe for no good reason? You
should be able to easily move this out of bpf_uprobe into a temporary
array.

> +       struct uprobe_consumer consumer;
> +};
> +
> +struct bpf_uprobe_multi_link {
> +       struct bpf_link link;
> +       u32 cnt;
> +       struct bpf_uprobe *uprobes;
> +};
> +

[...]

> +       if (prog->expected_attach_type != BPF_TRACE_UPROBE_MULTI)
> +               return -EINVAL;
> +
> +       flags = attr->link_create.uprobe_multi.flags;
> +       if (flags & ~BPF_F_UPROBE_MULTI_RETURN)
> +               return -EINVAL;
> +
> +       upaths = u64_to_user_ptr(attr->link_create.uprobe_multi.paths);
> +       uoffsets = u64_to_user_ptr(attr->link_create.uprobe_multi.offsets);
> +       if (!!upaths != !!uoffsets)
> +               return -EINVAL;

when having these as NULL would be ok? cnt == 0? or is there some
valid situation?

> +
> +       uref_ctr_offsets = u64_to_user_ptr(attr->link_create.uprobe_multi.ref_ctr_offsets);

if upaths is NULL, uref_ctr_offsets should be NULL as well?

> +
> +       cnt = attr->link_create.uprobe_multi.cnt;
> +       if (!cnt)
> +               return -EINVAL;
> +
> +       uprobes = kvcalloc(cnt, sizeof(*uprobes), GFP_KERNEL);
> +       if (!uprobes)
> +               return -ENOMEM;
> +

[...]

Re: [RFC/PATCH bpf-next 01/20] bpf: Add multi uprobe link

[Jiri Olsa]

On Wed, Apr 26, 2023 at 12:00:10PM -0700, Andrii Nakryiko wrote:
> On Mon, Apr 24, 2023 at 9:05 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding new multi uprobe link that allows to attach bpf program
> > to multiple uprobes.
> >
> > Uprobes to attach are specified via new link_create uprobe_multi
> > union:
> >
> >   struct {
> >           __u32           flags;
> >           __u32           cnt;
> >           __aligned_u64   paths;
> >           __aligned_u64   offsets;
> >           __aligned_u64   ref_ctr_offsets;
> >   } uprobe_multi;
> >
> > Uprobes are defined in paths/offsets/ref_ctr_offsets arrays with
> > the same 'cnt' length. Each uprobe is defined with a single index
> > in all three arrays:
> >
> >   paths[idx], offsets[idx] and/or ref_ctr_offsets[idx]
> >
> > The 'flags' supports single bit for now that marks the uprobe as
> > return probe.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  include/linux/trace_events.h |   6 +
> >  include/uapi/linux/bpf.h     |  14 +++
> >  kernel/bpf/syscall.c         |  16 ++-
> >  kernel/trace/bpf_trace.c     | 231 +++++++++++++++++++++++++++++++++++
> >  4 files changed, 265 insertions(+), 2 deletions(-)
> >
> 
> [...]
> 
> > @@ -4666,10 +4667,21 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
> >                 ret = bpf_perf_link_attach(attr, prog);
> >                 break;
> >         case BPF_PROG_TYPE_KPROBE:
> > +               /* Ensure that program with eBPF_TRACE_UPROBE_MULTI attach type can
> 
> eBPF_TRACE_UPROBE_MULTI :)

will fix ;-)

> 
> > +                * attach only to uprobe_multi link. It has its own runtime context
> > +                * which is specific for get_func_ip/get_attach_cookie helpers.
> > +                */
> > +               if (prog->expected_attach_type == BPF_TRACE_UPROBE_MULTI &&
> > +                   attr->link_create.attach_type != BPF_TRACE_UPROBE_MULTI) {
> > +                       ret = -EINVAL;
> > +                       goto out;
> > +               }
> 
> as Yonghong pointed out, you check this condition in
> bpf_uprobe_multi_link_attach() already, so why redundant check?

I tried to answer that in here:
  https://lore.kernel.org/bpf/ZEjU0ykZZTHMVlZt@krava/

> 
> >                 if (attr->link_create.attach_type == BPF_PERF_EVENT)
> >                         ret = bpf_perf_link_attach(attr, prog);
> > -               else
> > +               else if (attr->link_create.attach_type == BPF_TRACE_KPROBE_MULTI)
> >                         ret = bpf_kprobe_multi_link_attach(attr, prog);
> > +               else if (attr->link_create.attach_type == BPF_TRACE_UPROBE_MULTI)
> > +                       ret = bpf_uprobe_multi_link_attach(attr, prog);
> >                 break;
> >         default:
> >                 ret = -EINVAL;
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index bcf91bc7bf71..b84a7d01abf4 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -23,6 +23,7 @@
> >  #include <linux/sort.h>
> >  #include <linux/key.h>
> >  #include <linux/verification.h>
> > +#include <linux/namei.h>
> >
> >  #include <net/bpf_sk_storage.h>
> >
> > @@ -2901,3 +2902,233 @@ static u64 bpf_kprobe_multi_entry_ip(struct bpf_run_ctx *ctx)
> >         return 0;
> >  }
> >  #endif
> > +
> > +#ifdef CONFIG_UPROBES
> > +struct bpf_uprobe_multi_link;
> > +
> > +struct bpf_uprobe {
> > +       struct bpf_uprobe_multi_link *link;
> > +       struct inode *inode;
> > +       loff_t offset;
> > +       loff_t ref_ctr_offset;
> 
> you seem to need this only during link creation, so we are wasting 8
> bytes here per each instance of bpf_uprobe for no good reason? You
> should be able to easily move this out of bpf_uprobe into a temporary
> array.

right, we just need offset and inode, good catch, will fix

> 
> > +       struct uprobe_consumer consumer;
> > +};
> > +
> > +struct bpf_uprobe_multi_link {
> > +       struct bpf_link link;
> > +       u32 cnt;
> > +       struct bpf_uprobe *uprobes;
> > +};
> > +
> 
> [...]
> 
> > +       if (prog->expected_attach_type != BPF_TRACE_UPROBE_MULTI)
> > +               return -EINVAL;
> > +
> > +       flags = attr->link_create.uprobe_multi.flags;
> > +       if (flags & ~BPF_F_UPROBE_MULTI_RETURN)
> > +               return -EINVAL;
> > +
> > +       upaths = u64_to_user_ptr(attr->link_create.uprobe_multi.paths);
> > +       uoffsets = u64_to_user_ptr(attr->link_create.uprobe_multi.offsets);
> > +       if (!!upaths != !!uoffsets)
> > +               return -EINVAL;
> 
> when having these as NULL would be ok? cnt == 0? or is there some
> valid situation?

ah nope, that needs to be always != NULL, will fix

> 
> > +
> > +       uref_ctr_offsets = u64_to_user_ptr(attr->link_create.uprobe_multi.ref_ctr_offsets);
> 
> if upaths is NULL, uref_ctr_offsets should be NULL as well?

we need to fail when upaths is NULL, so that should be taken care of

thanks,
jirka

Re: [RFC/PATCH bpf-next 01/20] bpf: Add multi uprobe link

[Andrii Nakryiko]

On Mon, Apr 24, 2023 at 9:05 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding new multi uprobe link that allows to attach bpf program
> to multiple uprobes.
>
> Uprobes to attach are specified via new link_create uprobe_multi
> union:
>
>   struct {
>           __u32           flags;
>           __u32           cnt;
>           __aligned_u64   paths;
>           __aligned_u64   offsets;
>           __aligned_u64   ref_ctr_offsets;
>   } uprobe_multi;
>
> Uprobes are defined in paths/offsets/ref_ctr_offsets arrays with
> the same 'cnt' length. Each uprobe is defined with a single index
> in all three arrays:
>
>   paths[idx], offsets[idx] and/or ref_ctr_offsets[idx]
>
> The 'flags' supports single bit for now that marks the uprobe as
> return probe.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  include/linux/trace_events.h |   6 +
>  include/uapi/linux/bpf.h     |  14 +++
>  kernel/bpf/syscall.c         |  16 ++-
>  kernel/trace/bpf_trace.c     | 231 +++++++++++++++++++++++++++++++++++
>  4 files changed, 265 insertions(+), 2 deletions(-)
>

[...]

> +static void bpf_uprobe_multi_link_dealloc(struct bpf_link *link)
> +{
> +       struct bpf_uprobe_multi_link *umulti_link;
> +
> +       umulti_link = container_of(link, struct bpf_uprobe_multi_link, link);
> +       kvfree(umulti_link->uprobes);
> +       kfree(umulti_link);
> +}
> +
> +static const struct bpf_link_ops bpf_uprobe_multi_link_lops = {
> +       .release = bpf_uprobe_multi_link_release,
> +       .dealloc = bpf_uprobe_multi_link_dealloc,
> +};
> +

let's implement show_fdinfo and fill_link_info as well? At least we
can display how many instances of uprobe are attached for a given
link? And depending on what we decide with single or multi paths per
link, we could output path to the binary, right? And PID as well, if
we agree that it's possible to support it. Thanks!


[...]

Re: [RFC/PATCH bpf-next 01/20] bpf: Add multi uprobe link

[Jiri Olsa]

On Wed, Apr 26, 2023 at 12:17:45PM -0700, Andrii Nakryiko wrote:
> On Mon, Apr 24, 2023 at 9:05 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding new multi uprobe link that allows to attach bpf program
> > to multiple uprobes.
> >
> > Uprobes to attach are specified via new link_create uprobe_multi
> > union:
> >
> >   struct {
> >           __u32           flags;
> >           __u32           cnt;
> >           __aligned_u64   paths;
> >           __aligned_u64   offsets;
> >           __aligned_u64   ref_ctr_offsets;
> >   } uprobe_multi;
> >
> > Uprobes are defined in paths/offsets/ref_ctr_offsets arrays with
> > the same 'cnt' length. Each uprobe is defined with a single index
> > in all three arrays:
> >
> >   paths[idx], offsets[idx] and/or ref_ctr_offsets[idx]
> >
> > The 'flags' supports single bit for now that marks the uprobe as
> > return probe.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  include/linux/trace_events.h |   6 +
> >  include/uapi/linux/bpf.h     |  14 +++
> >  kernel/bpf/syscall.c         |  16 ++-
> >  kernel/trace/bpf_trace.c     | 231 +++++++++++++++++++++++++++++++++++
> >  4 files changed, 265 insertions(+), 2 deletions(-)
> >
> 
> [...]
> 
> > +static void bpf_uprobe_multi_link_dealloc(struct bpf_link *link)
> > +{
> > +       struct bpf_uprobe_multi_link *umulti_link;
> > +
> > +       umulti_link = container_of(link, struct bpf_uprobe_multi_link, link);
> > +       kvfree(umulti_link->uprobes);
> > +       kfree(umulti_link);
> > +}
> > +
> > +static const struct bpf_link_ops bpf_uprobe_multi_link_lops = {
> > +       .release = bpf_uprobe_multi_link_release,
> > +       .dealloc = bpf_uprobe_multi_link_dealloc,
> > +};
> > +
> 
> let's implement show_fdinfo and fill_link_info as well? At least we
> can display how many instances of uprobe are attached for a given
> link? And depending on what we decide with single or multi paths per
> link, we could output path to the binary, right? And PID as well, if
> we agree that it's possible to support it. Thanks!

ok, will check

jirka

[RFC/PATCH bpf-next 02/20] bpf: Add cookies support for uprobe_multi link

[Jiri Olsa]

Adding support to specify cookies array for uprobe_multi link.

The cookies array share indexes and length with other uprobe_multi
arrays (paths/offsets/ref_ctr_offsets).

The cookies[i] value defines cookie for i-the uprobe and will be
returned by bpf_get_attach_cookie helper when called from ebpf
program hooked to that specific uprobe.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 include/uapi/linux/bpf.h |  1 +
 kernel/bpf/syscall.c     |  2 +-
 kernel/trace/bpf_trace.c | 46 +++++++++++++++++++++++++++++++++++++---
 3 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index debc041c6ca5..77ce2159478d 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1581,6 +1581,7 @@ union bpf_attr {
 				__aligned_u64	paths;
 				__aligned_u64	offsets;
 				__aligned_u64	ref_ctr_offsets;
+				__aligned_u64	cookies;
 			} uprobe_multi;
 		};
 	} link_create;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 0b789a33317b..5b2dc7ae8616 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -4566,7 +4566,7 @@ static int bpf_map_do_batch(const union bpf_attr *attr,
 	return err;
 }
 
-#define BPF_LINK_CREATE_LAST_FIELD link_create.kprobe_multi.cookies
+#define BPF_LINK_CREATE_LAST_FIELD link_create.uprobe_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 b84a7d01abf4..f795cfc00e5f 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -87,6 +87,8 @@ static int bpf_btf_printf_prepare(struct btf_ptr *ptr, u32 btf_ptr_size,
 static u64 bpf_kprobe_multi_cookie(struct bpf_run_ctx *ctx);
 static u64 bpf_kprobe_multi_entry_ip(struct bpf_run_ctx *ctx);
 
+static u64 bpf_uprobe_multi_cookie(struct bpf_run_ctx *ctx);
+
 /**
  * trace_call_bpf - invoke BPF program
  * @call: tracepoint event
@@ -1089,6 +1091,18 @@ static const struct bpf_func_proto bpf_get_attach_cookie_proto_kmulti = {
 	.arg1_type	= ARG_PTR_TO_CTX,
 };
 
+BPF_CALL_1(bpf_get_attach_cookie_uprobe_multi, struct pt_regs *, regs)
+{
+	return bpf_uprobe_multi_cookie(current->bpf_ctx);
+}
+
+static const struct bpf_func_proto bpf_get_attach_cookie_proto_umulti = {
+	.func		= bpf_get_attach_cookie_uprobe_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;
@@ -1535,9 +1549,11 @@ 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 prog->expected_attach_type == BPF_TRACE_KPROBE_MULTI ?
-			&bpf_get_attach_cookie_proto_kmulti :
-			&bpf_get_attach_cookie_proto_trace;
+		if (prog->expected_attach_type == BPF_TRACE_KPROBE_MULTI)
+			return &bpf_get_attach_cookie_proto_kmulti;
+		if (prog->expected_attach_type == BPF_TRACE_UPROBE_MULTI)
+			return &bpf_get_attach_cookie_proto_umulti;
+		return &bpf_get_attach_cookie_proto_trace;
 	default:
 		return bpf_tracing_func_proto(func_id, prog);
 	}
@@ -2911,6 +2927,7 @@ struct bpf_uprobe {
 	struct inode *inode;
 	loff_t offset;
 	loff_t ref_ctr_offset;
+	u64 cookie;
 	struct uprobe_consumer consumer;
 };
 
@@ -2923,6 +2940,7 @@ struct bpf_uprobe_multi_link {
 struct bpf_uprobe_multi_run_ctx {
 	struct bpf_run_ctx run_ctx;
 	unsigned long entry_ip;
+	struct bpf_uprobe *uprobe;
 };
 
 static void bpf_uprobe_unregister(struct bpf_uprobe *uprobes, u32 cnt)
@@ -2964,6 +2982,7 @@ static int uprobe_prog_run(struct bpf_uprobe *uprobe,
 	struct bpf_uprobe_multi_link *link = uprobe->link;
 	struct bpf_uprobe_multi_run_ctx run_ctx = {
 		.entry_ip = entry_ip,
+		.uprobe = uprobe,
 	};
 	struct bpf_run_ctx *old_run_ctx;
 	int err;
@@ -3005,6 +3024,16 @@ uprobe_multi_link_ret_handler(struct uprobe_consumer *con, unsigned long func, s
 	return uprobe_prog_run(uprobe, func, regs);
 }
 
+static u64 bpf_uprobe_multi_cookie(struct bpf_run_ctx *ctx)
+{
+	struct bpf_uprobe_multi_run_ctx *run_ctx;
+
+	if (WARN_ON_ONCE(!ctx))
+		return 0;
+	run_ctx = container_of(current->bpf_ctx, struct bpf_uprobe_multi_run_ctx, run_ctx);
+	return run_ctx->uprobe->cookie;
+}
+
 int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
 {
 	void __user **upaths, __user *upath, __user *old_upath = NULL;
@@ -3014,6 +3043,7 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 	struct bpf_link_primer link_primer;
 	struct bpf_uprobe *uprobes = NULL;
 	struct inode *inode, *old_inode;
+	u64 __user *ucookies, cookie = 0;
 	u32 flags, cnt, i;
 	int err;
 
@@ -3034,6 +3064,7 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 		return -EINVAL;
 
 	uref_ctr_offsets = u64_to_user_ptr(attr->link_create.uprobe_multi.ref_ctr_offsets);
+	ucookies = u64_to_user_ptr(attr->link_create.uprobe_multi.cookies);
 
 	cnt = attr->link_create.uprobe_multi.cnt;
 	if (!cnt)
@@ -3050,6 +3081,10 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 	}
 
 	for (i = 0; i < cnt; i++) {
+		if (ucookies && __get_user(cookie, ucookies + i)) {
+			err = -EFAULT;
+			goto error;
+		}
 		if (uref_ctr_offsets && __get_user(ref_ctr_offset, uref_ctr_offsets + i)) {
 			err = -EFAULT;
 			goto error;
@@ -3090,6 +3125,7 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 		uprobes[i].inode = inode;
 		uprobes[i].offset = offset;
 		uprobes[i].ref_ctr_offset = ref_ctr_offset;
+		uprobes[i].cookie = cookie;
 		uprobes[i].link = link;
 
 		if (flags & BPF_F_UPROBE_MULTI_RETURN)
@@ -3131,4 +3167,8 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 {
 	return -EOPNOTSUPP;
 }
+static u64 bpf_uprobe_multi_cookie(struct bpf_run_ctx *ctx)
+{
+	return 0;
+}
 #endif /* CONFIG_UPROBES */
-- 
2.40.0

Re: [RFC/PATCH bpf-next 02/20] bpf: Add cookies support for uprobe_multi link

[Yonghong Song]

On 4/24/23 9:04 AM, Jiri Olsa wrote:
> Adding support to specify cookies array for uprobe_multi link.
> 
> The cookies array share indexes and length with other uprobe_multi
> arrays (paths/offsets/ref_ctr_offsets).
> 
> The cookies[i] value defines cookie for i-the uprobe and will be
> returned by bpf_get_attach_cookie helper when called from ebpf
> program hooked to that specific uprobe.
> 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>   include/uapi/linux/bpf.h |  1 +
>   kernel/bpf/syscall.c     |  2 +-
>   kernel/trace/bpf_trace.c | 46 +++++++++++++++++++++++++++++++++++++---
>   3 files changed, 45 insertions(+), 4 deletions(-)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index debc041c6ca5..77ce2159478d 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1581,6 +1581,7 @@ union bpf_attr {
>   				__aligned_u64	paths;
>   				__aligned_u64	offsets;
>   				__aligned_u64	ref_ctr_offsets;
> +				__aligned_u64	cookies;
>   			} uprobe_multi;
>   		};
>   	} link_create;
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 0b789a33317b..5b2dc7ae8616 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -4566,7 +4566,7 @@ static int bpf_map_do_batch(const union bpf_attr *attr,
>   	return err;
>   }
>   
> -#define BPF_LINK_CREATE_LAST_FIELD link_create.kprobe_multi.cookies
> +#define BPF_LINK_CREATE_LAST_FIELD link_create.uprobe_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 b84a7d01abf4..f795cfc00e5f 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -87,6 +87,8 @@ static int bpf_btf_printf_prepare(struct btf_ptr *ptr, u32 btf_ptr_size,
>   static u64 bpf_kprobe_multi_cookie(struct bpf_run_ctx *ctx);
>   static u64 bpf_kprobe_multi_entry_ip(struct bpf_run_ctx *ctx);
>   
> +static u64 bpf_uprobe_multi_cookie(struct bpf_run_ctx *ctx);
> +
>   /**
>    * trace_call_bpf - invoke BPF program
>    * @call: tracepoint event
> @@ -1089,6 +1091,18 @@ static const struct bpf_func_proto bpf_get_attach_cookie_proto_kmulti = {
>   	.arg1_type	= ARG_PTR_TO_CTX,
>   };
>   
> +BPF_CALL_1(bpf_get_attach_cookie_uprobe_multi, struct pt_regs *, regs)
> +{
> +	return bpf_uprobe_multi_cookie(current->bpf_ctx);
> +}

the argument regs is not used here.
looks like we have the same issue for
   bpf_get_attach_cookie_kprobe_multi
   bpf_get_attach_cookie_trace
   bpf_get_attach_cookie_tracing

I think this probably for preserving uapi. So I am okay with
the above, just want to point it out.

> +
> +static const struct bpf_func_proto bpf_get_attach_cookie_proto_umulti = {
> +	.func		= bpf_get_attach_cookie_uprobe_multi,
> +	.gpl_only	= false,
> +	.ret_type	= RET_INTEGER,
> +	.arg1_type	= ARG_PTR_TO_CTX,
> +};
> +
[...]

Re: [RFC/PATCH bpf-next 02/20] bpf: Add cookies support for uprobe_multi link

[Andrii Nakryiko]

On Mon, Apr 24, 2023 at 9:05 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding support to specify cookies array for uprobe_multi link.
>
> The cookies array share indexes and length with other uprobe_multi
> arrays (paths/offsets/ref_ctr_offsets).
>
> The cookies[i] value defines cookie for i-the uprobe and will be
> returned by bpf_get_attach_cookie helper when called from ebpf
> program hooked to that specific uprobe.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  include/uapi/linux/bpf.h |  1 +
>  kernel/bpf/syscall.c     |  2 +-
>  kernel/trace/bpf_trace.c | 46 +++++++++++++++++++++++++++++++++++++---
>  3 files changed, 45 insertions(+), 4 deletions(-)
>

LGTM, one nit below

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

[...]

>  static void bpf_uprobe_unregister(struct bpf_uprobe *uprobes, u32 cnt)
> @@ -2964,6 +2982,7 @@ static int uprobe_prog_run(struct bpf_uprobe *uprobe,
>         struct bpf_uprobe_multi_link *link = uprobe->link;
>         struct bpf_uprobe_multi_run_ctx run_ctx = {
>                 .entry_ip = entry_ip,
> +               .uprobe = uprobe,
>         };
>         struct bpf_run_ctx *old_run_ctx;
>         int err;
> @@ -3005,6 +3024,16 @@ uprobe_multi_link_ret_handler(struct uprobe_consumer *con, unsigned long func, s
>         return uprobe_prog_run(uprobe, func, regs);
>  }
>
> +static u64 bpf_uprobe_multi_cookie(struct bpf_run_ctx *ctx)
> +{
> +       struct bpf_uprobe_multi_run_ctx *run_ctx;
> +
> +       if (WARN_ON_ONCE(!ctx))
> +               return 0;

do we need this check?... seems redundant, tbh

> +       run_ctx = container_of(current->bpf_ctx, struct bpf_uprobe_multi_run_ctx, run_ctx);
> +       return run_ctx->uprobe->cookie;
> +}
> +

[...]

Re: [RFC/PATCH bpf-next 02/20] bpf: Add cookies support for uprobe_multi link

[Jiri Olsa]

On Wed, Apr 26, 2023 at 12:13:20PM -0700, Andrii Nakryiko wrote:
> On Mon, Apr 24, 2023 at 9:05 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding support to specify cookies array for uprobe_multi link.
> >
> > The cookies array share indexes and length with other uprobe_multi
> > arrays (paths/offsets/ref_ctr_offsets).
> >
> > The cookies[i] value defines cookie for i-the uprobe and will be
> > returned by bpf_get_attach_cookie helper when called from ebpf
> > program hooked to that specific uprobe.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  include/uapi/linux/bpf.h |  1 +
> >  kernel/bpf/syscall.c     |  2 +-
> >  kernel/trace/bpf_trace.c | 46 +++++++++++++++++++++++++++++++++++++---
> >  3 files changed, 45 insertions(+), 4 deletions(-)
> >
> 
> LGTM, one nit below
> 
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
> 
> [...]
> 
> >  static void bpf_uprobe_unregister(struct bpf_uprobe *uprobes, u32 cnt)
> > @@ -2964,6 +2982,7 @@ static int uprobe_prog_run(struct bpf_uprobe *uprobe,
> >         struct bpf_uprobe_multi_link *link = uprobe->link;
> >         struct bpf_uprobe_multi_run_ctx run_ctx = {
> >                 .entry_ip = entry_ip,
> > +               .uprobe = uprobe,
> >         };
> >         struct bpf_run_ctx *old_run_ctx;
> >         int err;
> > @@ -3005,6 +3024,16 @@ uprobe_multi_link_ret_handler(struct uprobe_consumer *con, unsigned long func, s
> >         return uprobe_prog_run(uprobe, func, regs);
> >  }
> >
> > +static u64 bpf_uprobe_multi_cookie(struct bpf_run_ctx *ctx)
> > +{
> > +       struct bpf_uprobe_multi_run_ctx *run_ctx;
> > +
> > +       if (WARN_ON_ONCE(!ctx))
> > +               return 0;
> 
> do we need this check?... seems redundant, tbh

it might be too much.. so this helper is called based on the:

  prog->expected_attach_type == BPF_TRACE_UPROBE_MULTI

so it's just screaming if there's some mismatch with the flag.. but if there is,
we probably would not get to this point or there wil be some pointer != NULL anyway,
yea, I'll remove it

there's similar one for kprobe_multi, I guess it can go as well

thanks,
jirka

> 
> > +       run_ctx = container_of(current->bpf_ctx, struct bpf_uprobe_multi_run_ctx, run_ctx);
> > +       return run_ctx->uprobe->cookie;
> > +}
> > +
> 
> [...]

[RFC/PATCH bpf-next 03/20] bpf: Add bpf_get_func_ip helper support for uprobe link

[Jiri Olsa]

Adding support for bpf_get_func_ip helper being called from
ebpf program attached by uprobe_multi link.

It returns the ip of the uprobe.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/trace/bpf_trace.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index f795cfc00e5f..b9a4a8ff51d7 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -88,6 +88,7 @@ static u64 bpf_kprobe_multi_cookie(struct bpf_run_ctx *ctx);
 static u64 bpf_kprobe_multi_entry_ip(struct bpf_run_ctx *ctx);
 
 static u64 bpf_uprobe_multi_cookie(struct bpf_run_ctx *ctx);
+static u64 bpf_uprobe_multi_entry_ip(struct bpf_run_ctx *ctx);
 
 /**
  * trace_call_bpf - invoke BPF program
@@ -1091,6 +1092,18 @@ static const struct bpf_func_proto bpf_get_attach_cookie_proto_kmulti = {
 	.arg1_type	= ARG_PTR_TO_CTX,
 };
 
+BPF_CALL_1(bpf_get_func_ip_uprobe_multi, struct pt_regs *, regs)
+{
+	return bpf_uprobe_multi_entry_ip(current->bpf_ctx);
+}
+
+static const struct bpf_func_proto bpf_get_func_ip_proto_uprobe_multi = {
+	.func		= bpf_get_func_ip_uprobe_multi,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_CTX,
+};
+
 BPF_CALL_1(bpf_get_attach_cookie_uprobe_multi, struct pt_regs *, regs)
 {
 	return bpf_uprobe_multi_cookie(current->bpf_ctx);
@@ -1545,9 +1558,11 @@ 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 prog->expected_attach_type == BPF_TRACE_KPROBE_MULTI ?
-			&bpf_get_func_ip_proto_kprobe_multi :
-			&bpf_get_func_ip_proto_kprobe;
+		if (prog->expected_attach_type == BPF_TRACE_KPROBE_MULTI)
+			return &bpf_get_func_ip_proto_kprobe_multi;
+		if (prog->expected_attach_type == BPF_TRACE_UPROBE_MULTI)
+			return &bpf_get_func_ip_proto_uprobe_multi;
+		return &bpf_get_func_ip_proto_kprobe;
 	case BPF_FUNC_get_attach_cookie:
 		if (prog->expected_attach_type == BPF_TRACE_KPROBE_MULTI)
 			return &bpf_get_attach_cookie_proto_kmulti;
@@ -3024,6 +3039,14 @@ uprobe_multi_link_ret_handler(struct uprobe_consumer *con, unsigned long func, s
 	return uprobe_prog_run(uprobe, func, regs);
 }
 
+static u64 bpf_uprobe_multi_entry_ip(struct bpf_run_ctx *ctx)
+{
+	struct bpf_uprobe_multi_run_ctx *run_ctx;
+
+	run_ctx = container_of(current->bpf_ctx, struct bpf_uprobe_multi_run_ctx, run_ctx);
+	return run_ctx->entry_ip;
+}
+
 static u64 bpf_uprobe_multi_cookie(struct bpf_run_ctx *ctx)
 {
 	struct bpf_uprobe_multi_run_ctx *run_ctx;
@@ -3171,4 +3194,8 @@ static u64 bpf_uprobe_multi_cookie(struct bpf_run_ctx *ctx)
 {
 	return 0;
 }
+static u64 bpf_uprobe_multi_entry_ip(struct bpf_run_ctx *ctx)
+{
+	return 0;
+}
 #endif /* CONFIG_UPROBES */
-- 
2.40.0

Re: [RFC/PATCH bpf-next 03/20] bpf: Add bpf_get_func_ip helper support for uprobe link

[Andrii Nakryiko]

On Mon, Apr 24, 2023 at 9:05 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding support for bpf_get_func_ip helper being called from
> ebpf program attached by uprobe_multi link.
>
> It returns the ip of the uprobe.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---

LGTM. I assume that IP will be user space address for uprobes, right?
Might be worth calling this out explicitly (it's kind of obvious and
expected, though, so maybe I'm overthinking this).

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

>  kernel/trace/bpf_trace.c | 33 ++++++++++++++++++++++++++++++---
>  1 file changed, 30 insertions(+), 3 deletions(-)
>

[...]

Re: [RFC/PATCH bpf-next 03/20] bpf: Add bpf_get_func_ip helper support for uprobe link

[Jiri Olsa]

On Wed, Apr 26, 2023 at 12:11:41PM -0700, Andrii Nakryiko wrote:
> On Mon, Apr 24, 2023 at 9:05 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding support for bpf_get_func_ip helper being called from
> > ebpf program attached by uprobe_multi link.
> >
> > It returns the ip of the uprobe.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> 
> LGTM. I assume that IP will be user space address for uprobes, right?
> Might be worth calling this out explicitly (it's kind of obvious and
> expected, though, so maybe I'm overthinking this).

yes, it's user space address where the probe is attached,
I'll make it more obvious ;-)

thanks,
jirka

> 
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
> 
> >  kernel/trace/bpf_trace.c | 33 ++++++++++++++++++++++++++++++---
> >  1 file changed, 30 insertions(+), 3 deletions(-)
> >
> 
> [...]

[RFC/PATCH bpf-next 04/20] libbpf: Update uapi bpf.h tools header

[Jiri Olsa]

Updating uapi bpf.h tools header with new uprobe_multi
link interface.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/include/uapi/linux/bpf.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 1bb11a6ee667..77ce2159478d 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1035,6 +1035,7 @@ enum bpf_attach_type {
 	BPF_TRACE_KPROBE_MULTI,
 	BPF_LSM_CGROUP,
 	BPF_STRUCT_OPS,
+	BPF_TRACE_UPROBE_MULTI,
 	__MAX_BPF_ATTACH_TYPE
 };
 
@@ -1052,6 +1053,7 @@ enum bpf_link_type {
 	BPF_LINK_TYPE_KPROBE_MULTI = 8,
 	BPF_LINK_TYPE_STRUCT_OPS = 9,
 	BPF_LINK_TYPE_NETFILTER = 10,
+	BPF_LINK_TYPE_UPROBE_MULTI = 11,
 
 	MAX_BPF_LINK_TYPE,
 };
@@ -1169,6 +1171,11 @@ enum bpf_link_type {
  */
 #define BPF_F_KPROBE_MULTI_RETURN	(1U << 0)
 
+/* link_create.uprobe_multi.flags used in LINK_CREATE command for
+ * BPF_TRACE_UPROBE_MULTI attach type to create return probe.
+ */
+#define BPF_F_UPROBE_MULTI_RETURN	(1U << 0)
+
 /* When BPF ldimm64's insn[0].src_reg != 0 then this can have
  * the following extensions:
  *
@@ -1568,6 +1575,14 @@ union bpf_attr {
 				__s32		priority;
 				__u32		flags;
 			} netfilter;
+			struct {
+				__u32		flags;
+				__u32		cnt;
+				__aligned_u64	paths;
+				__aligned_u64	offsets;
+				__aligned_u64	ref_ctr_offsets;
+				__aligned_u64	cookies;
+			} uprobe_multi;
 		};
 	} link_create;
 
-- 
2.40.0

Re: [RFC/PATCH bpf-next 04/20] libbpf: Update uapi bpf.h tools header

[Andrii Nakryiko]

On Mon, Apr 24, 2023 at 9:05 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Updating uapi bpf.h tools header with new uprobe_multi
> link interface.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---

let's merge this with the original UAPI header update patch? We used
to split this out for libbpf sync purposes, but it is handled easily
with current sync script, so no  need to make this a separate patch
(but up to you, I don't mind either)

>  tools/include/uapi/linux/bpf.h | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 1bb11a6ee667..77ce2159478d 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -1035,6 +1035,7 @@ enum bpf_attach_type {
>         BPF_TRACE_KPROBE_MULTI,
>         BPF_LSM_CGROUP,
>         BPF_STRUCT_OPS,
> +       BPF_TRACE_UPROBE_MULTI,
>         __MAX_BPF_ATTACH_TYPE
>  };
>
> @@ -1052,6 +1053,7 @@ enum bpf_link_type {
>         BPF_LINK_TYPE_KPROBE_MULTI = 8,
>         BPF_LINK_TYPE_STRUCT_OPS = 9,
>         BPF_LINK_TYPE_NETFILTER = 10,
> +       BPF_LINK_TYPE_UPROBE_MULTI = 11,
>
>         MAX_BPF_LINK_TYPE,
>  };
> @@ -1169,6 +1171,11 @@ enum bpf_link_type {
>   */
>  #define BPF_F_KPROBE_MULTI_RETURN      (1U << 0)
>
> +/* link_create.uprobe_multi.flags used in LINK_CREATE command for
> + * BPF_TRACE_UPROBE_MULTI attach type to create return probe.
> + */
> +#define BPF_F_UPROBE_MULTI_RETURN      (1U << 0)
> +
>  /* When BPF ldimm64's insn[0].src_reg != 0 then this can have
>   * the following extensions:
>   *
> @@ -1568,6 +1575,14 @@ union bpf_attr {
>                                 __s32           priority;
>                                 __u32           flags;
>                         } netfilter;
> +                       struct {
> +                               __u32           flags;
> +                               __u32           cnt;
> +                               __aligned_u64   paths;
> +                               __aligned_u64   offsets;
> +                               __aligned_u64   ref_ctr_offsets;
> +                               __aligned_u64   cookies;
> +                       } uprobe_multi;
>                 };
>         } link_create;
>
> --
> 2.40.0
>

Re: [RFC/PATCH bpf-next 04/20] libbpf: Update uapi bpf.h tools header

[Jiri Olsa]

On Wed, Apr 26, 2023 at 12:14:18PM -0700, Andrii Nakryiko wrote:
> On Mon, Apr 24, 2023 at 9:05 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Updating uapi bpf.h tools header with new uprobe_multi
> > link interface.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> 
> let's merge this with the original UAPI header update patch? We used
> to split this out for libbpf sync purposes, but it is handled easily
> with current sync script, so no  need to make this a separate patch
> (but up to you, I don't mind either)

ok, will merge it

thanks,
jirka

> 
> >  tools/include/uapi/linux/bpf.h | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> > index 1bb11a6ee667..77ce2159478d 100644
> > --- a/tools/include/uapi/linux/bpf.h
> > +++ b/tools/include/uapi/linux/bpf.h
> > @@ -1035,6 +1035,7 @@ enum bpf_attach_type {
> >         BPF_TRACE_KPROBE_MULTI,
> >         BPF_LSM_CGROUP,
> >         BPF_STRUCT_OPS,
> > +       BPF_TRACE_UPROBE_MULTI,
> >         __MAX_BPF_ATTACH_TYPE
> >  };
> >
> > @@ -1052,6 +1053,7 @@ enum bpf_link_type {
> >         BPF_LINK_TYPE_KPROBE_MULTI = 8,
> >         BPF_LINK_TYPE_STRUCT_OPS = 9,
> >         BPF_LINK_TYPE_NETFILTER = 10,
> > +       BPF_LINK_TYPE_UPROBE_MULTI = 11,
> >
> >         MAX_BPF_LINK_TYPE,
> >  };
> > @@ -1169,6 +1171,11 @@ enum bpf_link_type {
> >   */
> >  #define BPF_F_KPROBE_MULTI_RETURN      (1U << 0)
> >
> > +/* link_create.uprobe_multi.flags used in LINK_CREATE command for
> > + * BPF_TRACE_UPROBE_MULTI attach type to create return probe.
> > + */
> > +#define BPF_F_UPROBE_MULTI_RETURN      (1U << 0)
> > +
> >  /* When BPF ldimm64's insn[0].src_reg != 0 then this can have
> >   * the following extensions:
> >   *
> > @@ -1568,6 +1575,14 @@ union bpf_attr {
> >                                 __s32           priority;
> >                                 __u32           flags;
> >                         } netfilter;
> > +                       struct {
> > +                               __u32           flags;
> > +                               __u32           cnt;
> > +                               __aligned_u64   paths;
> > +                               __aligned_u64   offsets;
> > +                               __aligned_u64   ref_ctr_offsets;
> > +                               __aligned_u64   cookies;
> > +                       } uprobe_multi;
> >                 };
> >         } link_create;
> >
> > --
> > 2.40.0
> >

[RFC/PATCH bpf-next 05/20] libbpf: Add uprobe_multi attach type and link names

[Jiri Olsa]

Adding new uprobe_multi attach type and link names,
so the functions can resolve the new values.

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

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 1cbacf9e71f3..b5bde1f19831 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -117,6 +117,7 @@ static const char * const attach_type_name[] = {
 	[BPF_PERF_EVENT]		= "perf_event",
 	[BPF_TRACE_KPROBE_MULTI]	= "trace_kprobe_multi",
 	[BPF_STRUCT_OPS]		= "struct_ops",
+	[BPF_TRACE_UPROBE_MULTI]	= "trace_uprobe_multi",
 };
 
 static const char * const link_type_name[] = {
@@ -131,6 +132,7 @@ static const char * const link_type_name[] = {
 	[BPF_LINK_TYPE_KPROBE_MULTI]		= "kprobe_multi",
 	[BPF_LINK_TYPE_STRUCT_OPS]		= "struct_ops",
 	[BPF_LINK_TYPE_NETFILTER]		= "netfilter",
+	[BPF_LINK_TYPE_UPROBE_MULTI]		= "uprobe_multi",
 };
 
 static const char * const map_type_name[] = {
-- 
2.40.0

Re: [RFC/PATCH bpf-next 05/20] libbpf: Add uprobe_multi attach type and link names

[Andrii Nakryiko]

On Mon, Apr 24, 2023 at 9:05 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding new uprobe_multi attach type and link names,
> so the functions can resolve the new values.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---

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

>  tools/lib/bpf/libbpf.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 1cbacf9e71f3..b5bde1f19831 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -117,6 +117,7 @@ static const char * const attach_type_name[] = {
>         [BPF_PERF_EVENT]                = "perf_event",
>         [BPF_TRACE_KPROBE_MULTI]        = "trace_kprobe_multi",
>         [BPF_STRUCT_OPS]                = "struct_ops",
> +       [BPF_TRACE_UPROBE_MULTI]        = "trace_uprobe_multi",
>  };
>
>  static const char * const link_type_name[] = {
> @@ -131,6 +132,7 @@ static const char * const link_type_name[] = {
>         [BPF_LINK_TYPE_KPROBE_MULTI]            = "kprobe_multi",
>         [BPF_LINK_TYPE_STRUCT_OPS]              = "struct_ops",
>         [BPF_LINK_TYPE_NETFILTER]               = "netfilter",
> +       [BPF_LINK_TYPE_UPROBE_MULTI]            = "uprobe_multi",
>  };
>
>  static const char * const map_type_name[] = {
> --
> 2.40.0
>

[RFC/PATCH bpf-next 06/20] libbpf: Factor elf_for_each_symbol function

[Jiri Olsa]

Currently we have elf_find_func_offset function that looks up
symbol in the binary and returns its offset to be used for uprobe
attachment.

For attaching multiple uprobes we will need interface that allows
us to get offsets for multiple symbols specified either by name or
regular expression.

Factoring out elf_for_each_symbol helper function that iterates
all symbols in binary and calls following callbacks:

  fn_match - on each symbol
             if it returns error < 0, we bail out with that error
  fn_done  - when we finish iterating symbol section,
             if it returns true, we don't iterate next section

It will be used in following changes to lookup multiple symbols
and their offsets.

Changing elf_find_func_offset to use elf_for_each_symbol with
single_match callback that's looking to match single function.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/bpf/libbpf.c | 185 +++++++++++++++++++++++++----------------
 1 file changed, 114 insertions(+), 71 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index b5bde1f19831..92c92ed2101f 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10707,30 +10707,87 @@ static Elf_Scn *elf_find_next_scn_by_type(Elf *elf, int sh_type, Elf_Scn *scn)
 	return NULL;
 }
 
-/* Find offset of function name in the provided ELF object. "binary_path" is
- * the path to the ELF binary represented by "elf", and only used for error
- * reporting matters. "name" matches symbol name or name@@LIB for library
- * functions.
- */
-static long elf_find_func_offset(Elf *elf, const char *binary_path, const char *name)
-{
-	int i, sh_types[2] = { SHT_DYNSYM, SHT_SYMTAB };
-	bool is_shared_lib, is_name_qualified;
-	long ret = -ENOENT;
+struct elf_func_offset {
+	const char *name;
+	unsigned long offset;
+	int last_bind;
 	size_t name_len;
-	GElf_Ehdr ehdr;
+	bool is_name_qualified;
+};
 
-	if (!gelf_getehdr(elf, &ehdr)) {
-		pr_warn("elf: failed to get ehdr from %s: %s\n", binary_path, elf_errmsg(-1));
-		ret = -LIBBPF_ERRNO__FORMAT;
-		goto out;
+static int single_done(void *_data)
+{
+	struct elf_func_offset *data = _data;
+
+	return data->offset > 0;
+}
+
+static int single_match(Elf *elf, const char *binary_path, const char *sname,
+			GElf_Sym *sym, void *_data)
+{
+	struct elf_func_offset *data = _data;
+	size_t name_len = data->name_len;
+	const char *name = data->name;
+	Elf_Scn *sym_scn;
+	GElf_Shdr sym_sh;
+	int curr_bind;
+
+	curr_bind = GELF_ST_BIND(sym->st_info);
+
+	/* User can specify func, func@@LIB or func@@LIB_VERSION. */
+	if (strncmp(sname, name, name_len) != 0)
+		return 0;
+	/* ...but we don't want a search for "foo" to match 'foo2" also, so any
+	 * additional characters in sname should be of the form "@@LIB".
+	 */
+	if (!data->is_name_qualified && sname[name_len] != '\0' && sname[name_len] != '@')
+		return 0;
+
+	if (data->offset > 0) {
+		/* handle multiple matches */
+		if (data->last_bind != STB_WEAK && curr_bind != STB_WEAK) {
+			/* Only accept one non-weak bind. */
+			pr_warn("elf: ambiguous match for '%s', '%s' in '%s'\n",
+				sname, name, binary_path);
+			return -LIBBPF_ERRNO__FORMAT;
+		} else if (curr_bind == STB_WEAK) {
+			/* already have a non-weak bind, and
+			 * this is a weak bind, so ignore.
+			 */
+			return 0;
+		}
 	}
-	/* for shared lib case, we do not need to calculate relative offset */
-	is_shared_lib = ehdr.e_type == ET_DYN;
 
-	name_len = strlen(name);
-	/* Does name specify "@@LIB"? */
-	is_name_qualified = strstr(name, "@@") != NULL;
+	/* Transform symbol's virtual address (absolute for
+	 * binaries and relative for shared libs) into file
+	 * offset, which is what kernel is expecting for
+	 * uprobe/uretprobe attachment.
+	 * See Documentation/trace/uprobetracer.rst for more
+	 * details.
+	 * This is done by looking up symbol's containing
+	 * section's header and using it's virtual address
+	 * (sh_addr) and corresponding file offset (sh_offset)
+	 * to transform sym.st_value (virtual address) into
+	 * desired final file offset.
+	 */
+	sym_scn = elf_getscn(elf, sym->st_shndx);
+	if (!sym_scn)
+		return 0;
+	if (!gelf_getshdr(sym_scn, &sym_sh))
+		return 0;
+
+	data->offset = sym->st_value - sym_sh.sh_addr + sym_sh.sh_offset;
+	data->last_bind = curr_bind;
+	return 0;
+}
+
+static int elf_for_each_symbol(Elf *elf, const char *binary_path,
+			       int (*fn_match)(Elf *, const char *, const char *, GElf_Sym *, void *),
+			       int (*fn_done)(void *),
+			       void *data)
+{
+	int i, sh_types[2] = { SHT_DYNSYM, SHT_SYMTAB };
+	int ret = -ENOENT;
 
 	/* Search SHT_DYNSYM, SHT_SYMTAB for symbol. This search order is used because if
 	 * a binary is stripped, it may only have SHT_DYNSYM, and a fully-statically
@@ -10741,7 +10798,6 @@ static long elf_find_func_offset(Elf *elf, const char *binary_path, const char *
 		size_t nr_syms, strtabidx, idx;
 		Elf_Data *symbols = NULL;
 		Elf_Scn *scn = NULL;
-		int last_bind = -1;
 		const char *sname;
 		GElf_Shdr sh;
 
@@ -10764,10 +10820,7 @@ static long elf_find_func_offset(Elf *elf, const char *binary_path, const char *
 		nr_syms = symbols->d_size / sh.sh_entsize;
 
 		for (idx = 0; idx < nr_syms; idx++) {
-			int curr_bind;
 			GElf_Sym sym;
-			Elf_Scn *sym_scn;
-			GElf_Shdr sym_sh;
 
 			if (!gelf_getsym(symbols, idx, &sym))
 				continue;
@@ -10779,58 +10832,48 @@ static long elf_find_func_offset(Elf *elf, const char *binary_path, const char *
 			if (!sname)
 				continue;
 
-			curr_bind = GELF_ST_BIND(sym.st_info);
-
-			/* User can specify func, func@@LIB or func@@LIB_VERSION. */
-			if (strncmp(sname, name, name_len) != 0)
-				continue;
-			/* ...but we don't want a search for "foo" to match 'foo2" also, so any
-			 * additional characters in sname should be of the form "@@LIB".
-			 */
-			if (!is_name_qualified && sname[name_len] != '\0' && sname[name_len] != '@')
-				continue;
+			ret = fn_match(elf, binary_path, sname, &sym, data);
+			if (ret < 0)
+				goto out;
+		}
+		if (fn_done(data))
+			break;
+	}
 
-			if (ret >= 0) {
-				/* handle multiple matches */
-				if (last_bind != STB_WEAK && curr_bind != STB_WEAK) {
-					/* Only accept one non-weak bind. */
-					pr_warn("elf: ambiguous match for '%s', '%s' in '%s'\n",
-						sname, name, binary_path);
-					ret = -LIBBPF_ERRNO__FORMAT;
-					goto out;
-				} else if (curr_bind == STB_WEAK) {
-					/* already have a non-weak bind, and
-					 * this is a weak bind, so ignore.
-					 */
-					continue;
-				}
-			}
+out:
+	return ret;
+}
 
-			/* Transform symbol's virtual address (absolute for
-			 * binaries and relative for shared libs) into file
-			 * offset, which is what kernel is expecting for
-			 * uprobe/uretprobe attachment.
-			 * See Documentation/trace/uprobetracer.rst for more
-			 * details.
-			 * This is done by looking up symbol's containing
-			 * section's header and using it's virtual address
-			 * (sh_addr) and corresponding file offset (sh_offset)
-			 * to transform sym.st_value (virtual address) into
-			 * desired final file offset.
-			 */
-			sym_scn = elf_getscn(elf, sym.st_shndx);
-			if (!sym_scn)
-				continue;
-			if (!gelf_getshdr(sym_scn, &sym_sh))
-				continue;
+/* Find offset of function name in the provided ELF object. "binary_path" is
+ * the path to the ELF binary represented by "elf", and only used for error
+ * reporting matters. "name" matches symbol name or name@@LIB for library
+ * functions.
+ */
+static long elf_find_func_offset(Elf *elf, const char *binary_path, const char *name)
+{
+	struct elf_func_offset data = {
+		.name = name,
+		.last_bind = -1,
+		.name_len = strlen(name),
+		/* Does name specify "@@LIB"? */
+		.is_name_qualified = strstr(name, "@@") != NULL,
+	};
+	bool is_shared_lib;
+	GElf_Ehdr ehdr;
+	long ret;
+	int err;
 
-			ret = sym.st_value - sym_sh.sh_addr + sym_sh.sh_offset;
-			last_bind = curr_bind;
-		}
-		if (ret > 0)
-			break;
+	if (!gelf_getehdr(elf, &ehdr)) {
+		pr_warn("elf: failed to get ehdr from %s: %s\n", binary_path, elf_errmsg(-1));
+		ret = -LIBBPF_ERRNO__FORMAT;
+		goto out;
 	}
 
+	/* for shared lib case, we do not need to calculate relative offset */
+	is_shared_lib = ehdr.e_type == ET_DYN;
+
+	err = elf_for_each_symbol(elf, binary_path, single_match, single_done, &data);
+	ret = err < 0 ? (long) err : data.offset;
 	if (ret > 0) {
 		pr_debug("elf: symbol address match for '%s' in '%s': 0x%lx\n", name, binary_path,
 			 ret);
-- 
2.40.0

Re: [RFC/PATCH bpf-next 06/20] libbpf: Factor elf_for_each_symbol function

[Andrii Nakryiko]

On Mon, Apr 24, 2023 at 9:05 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Currently we have elf_find_func_offset function that looks up
> symbol in the binary and returns its offset to be used for uprobe
> attachment.
>
> For attaching multiple uprobes we will need interface that allows
> us to get offsets for multiple symbols specified either by name or
> regular expression.
>
> Factoring out elf_for_each_symbol helper function that iterates
> all symbols in binary and calls following callbacks:
>
>   fn_match - on each symbol
>              if it returns error < 0, we bail out with that error
>   fn_done  - when we finish iterating symbol section,
>              if it returns true, we don't iterate next section
>
> It will be used in following changes to lookup multiple symbols
> and their offsets.
>
> Changing elf_find_func_offset to use elf_for_each_symbol with
> single_match callback that's looking to match single function.
>

Given we have multiple uses for this for_each_elf_symbol, would it
make sense to implement it as an iterator (following essentially the
same pattern that BPF open-coded iterator is doing, where state is in
a small struct, and then we call next() until we get back NULL?)

This will lead to cleaner code overall, I think. And it does seem func
to implement it this (composable) way.

Also, I think we are at the point where libbpf.c is becoming pretty
bloated, so we should try to split out coherent subsets of
functionality into separate files. ELF helpers seem like a good group
of functionality  to move to a separate file? Maybe as a separate
patch set and/or follow up, but think about whether you can do part of
that during refactoring?

> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/lib/bpf/libbpf.c | 185 +++++++++++++++++++++++++----------------
>  1 file changed, 114 insertions(+), 71 deletions(-)
>

[...]

Re: [RFC/PATCH bpf-next 06/20] libbpf: Factor elf_for_each_symbol function

[Jiri Olsa]

On Wed, Apr 26, 2023 at 12:27:31PM -0700, Andrii Nakryiko wrote:
> On Mon, Apr 24, 2023 at 9:05 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Currently we have elf_find_func_offset function that looks up
> > symbol in the binary and returns its offset to be used for uprobe
> > attachment.
> >
> > For attaching multiple uprobes we will need interface that allows
> > us to get offsets for multiple symbols specified either by name or
> > regular expression.
> >
> > Factoring out elf_for_each_symbol helper function that iterates
> > all symbols in binary and calls following callbacks:
> >
> >   fn_match - on each symbol
> >              if it returns error < 0, we bail out with that error
> >   fn_done  - when we finish iterating symbol section,
> >              if it returns true, we don't iterate next section
> >
> > It will be used in following changes to lookup multiple symbols
> > and their offsets.
> >
> > Changing elf_find_func_offset to use elf_for_each_symbol with
> > single_match callback that's looking to match single function.
> >
> 
> Given we have multiple uses for this for_each_elf_symbol, would it
> make sense to implement it as an iterator (following essentially the
> same pattern that BPF open-coded iterator is doing, where state is in
> a small struct, and then we call next() until we get back NULL?)
> 
> This will lead to cleaner code overall, I think. And it does seem func
> to implement it this (composable) way.

ok, I'll check the open-coded iterator for this

> 
> Also, I think we are at the point where libbpf.c is becoming pretty
> bloated, so we should try to split out coherent subsets of
> functionality into separate files. ELF helpers seem like a good group
> of functionality  to move to a separate file? Maybe as a separate
> patch set and/or follow up, but think about whether you can do part of
> that during refactoring?

right, sounds good, will check

thanks,
jirka

> 
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/lib/bpf/libbpf.c | 185 +++++++++++++++++++++++++----------------
> >  1 file changed, 114 insertions(+), 71 deletions(-)
> >
> 
> [...]

Re: [RFC/PATCH bpf-next 06/20] libbpf: Factor elf_for_each_symbol function

[Andrii Nakryiko]

On Thu, Apr 27, 2023 at 6:24 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Wed, Apr 26, 2023 at 12:27:31PM -0700, Andrii Nakryiko wrote:
> > On Mon, Apr 24, 2023 at 9:05 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > Currently we have elf_find_func_offset function that looks up
> > > symbol in the binary and returns its offset to be used for uprobe
> > > attachment.
> > >
> > > For attaching multiple uprobes we will need interface that allows
> > > us to get offsets for multiple symbols specified either by name or
> > > regular expression.
> > >
> > > Factoring out elf_for_each_symbol helper function that iterates
> > > all symbols in binary and calls following callbacks:
> > >
> > >   fn_match - on each symbol
> > >              if it returns error < 0, we bail out with that error
> > >   fn_done  - when we finish iterating symbol section,
> > >              if it returns true, we don't iterate next section
> > >
> > > It will be used in following changes to lookup multiple symbols
> > > and their offsets.
> > >
> > > Changing elf_find_func_offset to use elf_for_each_symbol with
> > > single_match callback that's looking to match single function.
> > >
> >
> > Given we have multiple uses for this for_each_elf_symbol, would it
> > make sense to implement it as an iterator (following essentially the
> > same pattern that BPF open-coded iterator is doing, where state is in
> > a small struct, and then we call next() until we get back NULL?)
> >
> > This will lead to cleaner code overall, I think. And it does seem func
> > to implement it this (composable) way.
>
> ok, I'll check the open-coded iterator for this

Do check it, as it's a useful thing on BPF side. But tl;dr for libbpf
internal use we could do something like:

struct elf_iter {
    Elf *elf;
    size_t next_sym_idx;
};

and in the code the use will be something like

struct elf_iter;
elf_iter_init(*iter, elf); /* sets next_sym_idx to 0 */

while ((sym = elf_iter_next(&iter))) {
   /* use sym */
}


And we can tune the returned result to have symbol index, etc, of course.



>
> >
> > Also, I think we are at the point where libbpf.c is becoming pretty
> > bloated, so we should try to split out coherent subsets of
> > functionality into separate files. ELF helpers seem like a good group
> > of functionality  to move to a separate file? Maybe as a separate
> > patch set and/or follow up, but think about whether you can do part of
> > that during refactoring?
>
> right, sounds good, will check
>
> thanks,
> jirka
>
> >
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > ---
> > >  tools/lib/bpf/libbpf.c | 185 +++++++++++++++++++++++++----------------
> > >  1 file changed, 114 insertions(+), 71 deletions(-)
> > >
> >
> > [...]

[RFC/PATCH bpf-next 07/20] libbpf: Add elf_find_multi_func_offset function

[Jiri Olsa]

Adding elf_find_multi_func_offset function that looks up
offsets for symbols specified in syms array argument.

Offsets are returned in allocated array with the 'cnt' size,
that needs to be released by the caller.

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

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 92c92ed2101f..0b15609d4573 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10713,6 +10713,7 @@ struct elf_func_offset {
 	int last_bind;
 	size_t name_len;
 	bool is_name_qualified;
+	int idx;
 };
 
 static int single_done(void *_data)
@@ -10891,6 +10892,166 @@ static long elf_find_func_offset(Elf *elf, const char *binary_path, const char *
 	return ret;
 }
 
+struct match_multi_data {
+	int cnt;
+	int cnt_done;
+	struct elf_func_offset *func_offs;
+};
+
+static int cmp_func_offset(const void *_a, const void *_b)
+{
+	const struct elf_func_offset *a = _a;
+	const struct elf_func_offset *b = _b;
+
+	return strcmp(a->name, b->name);
+}
+
+static int multi_done(void *_data)
+{
+	struct match_multi_data *data = _data;
+
+	return data->cnt == data->cnt_done;
+}
+
+static int multi_match(Elf *elf, const char *binary_path, const char *sname,
+		       GElf_Sym *sym, void *_data)
+{
+	struct match_multi_data *data = _data;
+	struct elf_func_offset *fo, func_offs = {
+		.name = sname,
+	};
+	Elf_Scn *sym_scn;
+	GElf_Shdr sym_sh;
+	int curr_bind;
+
+	fo = bsearch(&func_offs, data->func_offs, data->cnt, sizeof(*data->func_offs),
+		     cmp_func_offset);
+	if (!fo)
+		return 0;
+
+	curr_bind = GELF_ST_BIND(sym->st_info);
+
+	if (fo->offset > 0) {
+		/* handle multiple matches */
+		if (fo->last_bind != STB_WEAK && curr_bind != STB_WEAK) {
+			/* Only accept one non-weak bind. */
+			pr_warn("elf: ambiguous match for '%s', '%s' in '%s'\n",
+				sname, fo->name, binary_path);
+			return -LIBBPF_ERRNO__FORMAT;
+		} else if (curr_bind == STB_WEAK) {
+			/* already have a non-weak bind, and
+			 * this is a weak bind, so ignore.
+			 */
+			return 0;
+		}
+	}
+
+	/* Transform symbol's virtual address (absolute for
+	 * binaries and relative for shared libs) into file
+	 * offset, which is what kernel is expecting for
+	 * uprobe/uretprobe attachment.
+	 * See Documentation/trace/uprobetracer.rst for more
+	 * details.
+	 * This is done by looking up symbol's containing
+	 * section's header and using it's virtual address
+	 * (sh_addr) and corresponding file offset (sh_offset)
+	 * to transform sym.st_value (virtual address) into
+	 * desired final file offset.
+	 */
+	sym_scn = elf_getscn(elf, sym->st_shndx);
+	if (!sym_scn)
+		return 0;
+	if (!gelf_getshdr(sym_scn, &sym_sh))
+		return 0;
+
+	if (!fo->offset)
+		data->cnt_done++;
+
+	fo->offset = sym->st_value - sym_sh.sh_addr + sym_sh.sh_offset;
+	fo->last_bind = curr_bind;
+	return 0;
+}
+
+static int
+__elf_find_multi_func_offset(Elf *elf, const char *binary_path, int cnt,
+			     const char **syms, unsigned long **poffsets)
+{
+	struct match_multi_data data = {
+		.cnt = cnt,
+	};
+	struct elf_func_offset *func_offs;
+	unsigned long *offsets = NULL;
+	int err, i, idx;
+
+	data.func_offs = func_offs = calloc(cnt, sizeof(*func_offs));
+	if (!func_offs)
+		return -ENOMEM;
+
+	for (i = 0; i < cnt; i++) {
+		func_offs[i].name = syms[i];
+		func_offs[i].idx = i;
+	}
+
+	qsort(func_offs, cnt, sizeof(*func_offs), cmp_func_offset);
+
+	err = elf_for_each_symbol(elf, binary_path, multi_match, multi_done, &data);
+	if (err)
+		goto out;
+
+	if (cnt != data.cnt_done) {
+		err = -ENOENT;
+		goto out;
+	}
+
+	offsets = calloc(cnt, sizeof(*offsets));
+	if (!offsets) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	for (i = 0; i < cnt; i++) {
+		idx = func_offs[i].idx;
+		offsets[idx] = func_offs[i].offset;
+	}
+
+out:
+	*poffsets = offsets;
+	free(func_offs);
+	return err;
+}
+
+LIBBPF_API int
+elf_find_multi_func_offset(const char *binary_path, int cnt,
+			   const char **syms, unsigned long **poffsets)
+{
+	char errmsg[STRERR_BUFSIZE];
+	long ret = -ENOENT;
+	Elf *elf;
+	int fd;
+
+	fd = open(binary_path, O_RDONLY | O_CLOEXEC);
+	if (fd < 0) {
+		ret = -errno;
+		pr_warn("failed to open %s: %s\n", binary_path,
+			libbpf_strerror_r(ret, errmsg, sizeof(errmsg)));
+		return ret;
+	}
+	if (elf_version(EV_CURRENT) == EV_NONE) {
+		pr_warn("failed to init libelf for %s\n", binary_path);
+		return -LIBBPF_ERRNO__LIBELF;
+	}
+	elf = elf_begin(fd, ELF_C_READ_MMAP, NULL);
+	if (!elf) {
+		pr_warn("elf: could not read elf from %s: %s\n", binary_path, elf_errmsg(-1));
+		close(fd);
+		return -LIBBPF_ERRNO__FORMAT;
+	}
+	ret = __elf_find_multi_func_offset(elf, binary_path, cnt, syms, poffsets);
+	elf_end(elf);
+	close(fd);
+	return ret;
+}
+
 /* Find offset of function name in ELF object specified by path. "name" matches
  * symbol name or name@@LIB for library functions.
  */
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 0b7362397ea3..b1b159263d47 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -1649,6 +1649,12 @@ LIBBPF_API int libbpf_register_prog_handler(const char *sec,
  */
 LIBBPF_API int libbpf_unregister_prog_handler(int handler_id);
 
+/**
+ * @brief *elf_find_multi_func_offset()* return offsets for given *syms*
+ */
+LIBBPF_API int elf_find_multi_func_offset(const char *binary_path, int cnt,
+					  const char **syms, unsigned long **poffsets);
+
 #ifdef __cplusplus
 } /* extern "C" */
 #endif
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index a5aa3a383d69..7b1bf3f9ce4f 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -390,4 +390,5 @@ LIBBPF_1.2.0 {
 		bpf_link_get_info_by_fd;
 		bpf_map_get_info_by_fd;
 		bpf_prog_get_info_by_fd;
+		elf_find_multi_func_offset;
 } LIBBPF_1.1.0;
-- 
2.40.0

[RFC/PATCH bpf-next 08/20] libbpf: Add elf_find_patern_func_offset function

[Jiri Olsa]

Adding elf_find_patern_func_offset function that looks up
offsets for symbols specified by pattern argument.

The 'pattern' argument allows wildcards (*?' supported).

Offsets are returned in allocated array together with its
size and needs to be released by the caller.

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

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 0b15609d4573..7eb7035f7b73 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -11052,6 +11052,127 @@ elf_find_multi_func_offset(const char *binary_path, int cnt,
 	return ret;
 }
 
+struct match_pattern_data {
+	const char *pattern;
+	struct elf_func_offset *func_offs;
+	size_t func_offs_cnt;
+	size_t func_offs_cap;
+};
+
+static int pattern_done(void *_data)
+{
+	struct match_pattern_data *data = _data;
+
+	// If we found anything in the first symbol section, do not search others
+	// to avoid duplicates.
+	return data->func_offs_cnt;
+}
+
+static int pattern_match(Elf *elf, const char *binary_path, const char *sname,
+			 GElf_Sym *sym, void *_data)
+{
+	struct match_pattern_data *data = _data;
+	unsigned long offset;
+	Elf_Scn *sym_scn;
+	GElf_Shdr sym_sh;
+	int err;
+
+	if (!glob_match(sname, data->pattern))
+		return 0;
+	sym_scn = elf_getscn(elf, sym->st_shndx);
+	if (!sym_scn)
+		return 0;
+	if (!gelf_getshdr(sym_scn, &sym_sh))
+		return 0;
+
+	err = libbpf_ensure_mem((void **) &data->func_offs, &data->func_offs_cap,
+				sizeof(*data->func_offs), data->func_offs_cnt + 1);
+	if (err)
+		return err;
+
+	offset = sym->st_value - sym_sh.sh_addr + sym_sh.sh_offset;
+	data->func_offs[data->func_offs_cnt].offset = offset;
+	data->func_offs[data->func_offs_cnt].name = strdup(sname);
+	data->func_offs_cnt++;
+	return 0;
+}
+
+static int
+__elf_find_patern_func_offset(Elf *elf, const char *binary_path, const char *pattern,
+			      const char ***pnames, unsigned long **poffsets, size_t *pcnt)
+{
+	struct match_pattern_data data = {
+		.pattern = pattern,
+	};
+	unsigned long *offsets = NULL;
+	const char **names = NULL;
+	size_t cnt = 0;
+	int err, i;
+
+	err = elf_for_each_symbol(elf, binary_path, pattern_match, pattern_done, &data);
+	if (err)
+		goto out;
+
+	cnt = data.func_offs_cnt;
+	if (!cnt) {
+		err = -ENOENT;
+		goto out;
+	}
+
+	offsets = calloc(cnt, sizeof(*offsets));
+	names = calloc(cnt, sizeof(*names));
+	if (!offsets || !names) {
+		free(offsets);
+		free(names);
+		err = -ENOMEM;
+		goto out;
+	}
+
+	for (i = 0; i < cnt; i++) {
+		offsets[i] = data.func_offs[i].offset;
+		names[i] = data.func_offs[i].name;
+	}
+
+out:
+	*pnames = names;
+	*poffsets = offsets;
+	*pcnt = cnt;
+	free(data.func_offs);
+	return err;
+}
+
+LIBBPF_API int
+elf_find_patern_func_offset(const char *binary_path, const char *pattern,
+			    const char ***pnames, unsigned long **poffsets, size_t *pcnt)
+{
+	char errmsg[STRERR_BUFSIZE];
+	long ret = -ENOENT;
+	Elf *elf;
+	int fd;
+
+	fd = open(binary_path, O_RDONLY | O_CLOEXEC);
+	if (fd < 0) {
+		ret = -errno;
+		pr_warn("failed to open %s: %s\n", binary_path,
+			libbpf_strerror_r(ret, errmsg, sizeof(errmsg)));
+		return ret;
+	}
+	if (elf_version(EV_CURRENT) == EV_NONE) {
+		pr_warn("failed to init libelf for %s\n", binary_path);
+		return -LIBBPF_ERRNO__LIBELF;
+	}
+	elf = elf_begin(fd, ELF_C_READ_MMAP, NULL);
+	if (!elf) {
+		pr_warn("elf: could not read elf from %s: %s\n", binary_path, elf_errmsg(-1));
+		close(fd);
+		return -LIBBPF_ERRNO__FORMAT;
+	}
+	ret = __elf_find_patern_func_offset(elf, binary_path, pattern, pnames, poffsets, pcnt);
+	elf_end(elf);
+	close(fd);
+	return ret;
+}
+
 /* Find offset of function name in ELF object specified by path. "name" matches
  * symbol name or name@@LIB for library functions.
  */
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index b1b159263d47..96ed109ae011 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -1655,6 +1655,13 @@ LIBBPF_API int libbpf_unregister_prog_handler(int handler_id);
 LIBBPF_API int elf_find_multi_func_offset(const char *binary_path, int cnt,
 					  const char **syms, unsigned long **poffsets);
 
+/**
+ * @brief *elf_find_patern_func_offset()* return symbols and offsets for given *pattern*
+ */
+LIBBPF_API int
+elf_find_patern_func_offset(const char *binary_path, const char *pattern,
+			    const char ***pnames, unsigned long **poffsets, size_t *pcnt);
+
 #ifdef __cplusplus
 } /* extern "C" */
 #endif
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 7b1bf3f9ce4f..2f091a0f093b 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -391,4 +391,5 @@ LIBBPF_1.2.0 {
 		bpf_map_get_info_by_fd;
 		bpf_prog_get_info_by_fd;
 		elf_find_multi_func_offset;
+		elf_find_patern_func_offset;
 } LIBBPF_1.1.0;
-- 
2.40.0

Re: [RFC/PATCH bpf-next 08/20] libbpf: Add elf_find_patern_func_offset function

[Andrii Nakryiko]

On Mon, Apr 24, 2023 at 9:06 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding elf_find_patern_func_offset function that looks up
> offsets for symbols specified by pattern argument.
>
> The 'pattern' argument allows wildcards (*?' supported).
>
> Offsets are returned in allocated array together with its
> size and needs to be released by the caller.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---


Why do we need to expose any elf-related helpers as libbpf public API?
Just to use them in selftests? If yes, then selftests can use libbpf
internal helpers just like bpftool due to static linking. In general,
it of course doesn't make sense for libbpf to provide ELF helpers as
part of its API.

Also s/patern/pattern/.


>  tools/lib/bpf/libbpf.c   | 121 +++++++++++++++++++++++++++++++++++++++
>  tools/lib/bpf/libbpf.h   |   7 +++
>  tools/lib/bpf/libbpf.map |   1 +
>  3 files changed, 129 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 0b15609d4573..7eb7035f7b73 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -11052,6 +11052,127 @@ elf_find_multi_func_offset(const char *binary_path, int cnt,
>         return ret;
>  }
>
> +struct match_pattern_data {
> +       const char *pattern;
> +       struct elf_func_offset *func_offs;
> +       size_t func_offs_cnt;
> +       size_t func_offs_cap;
> +};
> +
> +static int pattern_done(void *_data)
> +{
> +       struct match_pattern_data *data = _data;
> +
> +       // If we found anything in the first symbol section, do not search others
> +       // to avoid duplicates.

C++ comment


> +       return data->func_offs_cnt;
> +}
> +

[...]

Re: [RFC/PATCH bpf-next 08/20] libbpf: Add elf_find_patern_func_offset function

[Jiri Olsa]

On Wed, Apr 26, 2023 at 12:24:16PM -0700, Andrii Nakryiko wrote:
> On Mon, Apr 24, 2023 at 9:06 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding elf_find_patern_func_offset function that looks up
> > offsets for symbols specified by pattern argument.
> >
> > The 'pattern' argument allows wildcards (*?' supported).
> >
> > Offsets are returned in allocated array together with its
> > size and needs to be released by the caller.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> 
> 
> Why do we need to expose any elf-related helpers as libbpf public API?
> Just to use them in selftests? If yes, then selftests can use libbpf
> internal helpers just like bpftool due to static linking. In general,
> it of course doesn't make sense for libbpf to provide ELF helpers as
> part of its API.

I use them in bpftrace ;-) I can move the implementation in there,
if we don't want to expose it.. it was just convenient to use libbpf

> 
> Also s/patern/pattern/.

ok

> 
> 
> >  tools/lib/bpf/libbpf.c   | 121 +++++++++++++++++++++++++++++++++++++++
> >  tools/lib/bpf/libbpf.h   |   7 +++
> >  tools/lib/bpf/libbpf.map |   1 +
> >  3 files changed, 129 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 0b15609d4573..7eb7035f7b73 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -11052,6 +11052,127 @@ elf_find_multi_func_offset(const char *binary_path, int cnt,
> >         return ret;
> >  }
> >
> > +struct match_pattern_data {
> > +       const char *pattern;
> > +       struct elf_func_offset *func_offs;
> > +       size_t func_offs_cnt;
> > +       size_t func_offs_cap;
> > +};
> > +
> > +static int pattern_done(void *_data)
> > +{
> > +       struct match_pattern_data *data = _data;
> > +
> > +       // If we found anything in the first symbol section, do not search others
> > +       // to avoid duplicates.
> 
> C++ comment

ok, will fix

thanks,
jirka

Re: [RFC/PATCH bpf-next 08/20] libbpf: Add elf_find_patern_func_offset function

[Andrii Nakryiko]

On Thu, Apr 27, 2023 at 6:21 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Wed, Apr 26, 2023 at 12:24:16PM -0700, Andrii Nakryiko wrote:
> > On Mon, Apr 24, 2023 at 9:06 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > Adding elf_find_patern_func_offset function that looks up
> > > offsets for symbols specified by pattern argument.
> > >
> > > The 'pattern' argument allows wildcards (*?' supported).
> > >
> > > Offsets are returned in allocated array together with its
> > > size and needs to be released by the caller.
> > >
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > ---
> >
> >
> > Why do we need to expose any elf-related helpers as libbpf public API?
> > Just to use them in selftests? If yes, then selftests can use libbpf
> > internal helpers just like bpftool due to static linking. In general,
> > it of course doesn't make sense for libbpf to provide ELF helpers as
> > part of its API.
>
> I use them in bpftrace ;-) I can move the implementation in there,
> if we don't want to expose it.. it was just convenient to use libbpf

yep, let's not expose elf helpers as libbpf UAPI

>
> >
> > Also s/patern/pattern/.
>
> ok
>
> >
> >
> > >  tools/lib/bpf/libbpf.c   | 121 +++++++++++++++++++++++++++++++++++++++
> > >  tools/lib/bpf/libbpf.h   |   7 +++
> > >  tools/lib/bpf/libbpf.map |   1 +
> > >  3 files changed, 129 insertions(+)
> > >
> > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > index 0b15609d4573..7eb7035f7b73 100644
> > > --- a/tools/lib/bpf/libbpf.c
> > > +++ b/tools/lib/bpf/libbpf.c
> > > @@ -11052,6 +11052,127 @@ elf_find_multi_func_offset(const char *binary_path, int cnt,
> > >         return ret;
> > >  }
> > >
> > > +struct match_pattern_data {
> > > +       const char *pattern;
> > > +       struct elf_func_offset *func_offs;
> > > +       size_t func_offs_cnt;
> > > +       size_t func_offs_cap;
> > > +};
> > > +
> > > +static int pattern_done(void *_data)
> > > +{
> > > +       struct match_pattern_data *data = _data;
> > > +
> > > +       // If we found anything in the first symbol section, do not search others
> > > +       // to avoid duplicates.
> >
> > C++ comment
>
> ok, will fix
>
> thanks,
> jirka

[RFC/PATCH bpf-next 09/20] libbpf: Add bpf_link_create support for multi uprobes

[Jiri Olsa]

Adding new uprobe_multi struct to bpf_link_create_opts object
to pass multiple uprobe data to link_create attr uapi.

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

diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 128ac723c4ea..de227846fa3b 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -722,6 +722,16 @@ int bpf_link_create(int prog_fd, int target_fd,
 		if (!OPTS_ZEROED(opts, kprobe_multi))
 			return libbpf_err(-EINVAL);
 		break;
+	case BPF_TRACE_UPROBE_MULTI:
+		attr.link_create.uprobe_multi.flags = OPTS_GET(opts, uprobe_multi.flags, 0);
+		attr.link_create.uprobe_multi.cnt = OPTS_GET(opts, uprobe_multi.cnt, 0);
+		attr.link_create.uprobe_multi.paths = ptr_to_u64(OPTS_GET(opts, uprobe_multi.paths, 0));
+		attr.link_create.uprobe_multi.offsets = ptr_to_u64(OPTS_GET(opts, uprobe_multi.offsets, 0));
+		attr.link_create.uprobe_multi.ref_ctr_offsets = ptr_to_u64(OPTS_GET(opts, uprobe_multi.ref_ctr_offsets, 0));
+		attr.link_create.uprobe_multi.cookies = ptr_to_u64(OPTS_GET(opts, uprobe_multi.cookies, 0));
+		if (!OPTS_ZEROED(opts, uprobe_multi))
+			return libbpf_err(-EINVAL);
+		break;
 	case BPF_TRACE_FENTRY:
 	case BPF_TRACE_FEXIT:
 	case BPF_MODIFY_RETURN:
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index a2c091389b18..9404096b2cf0 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -332,13 +332,21 @@ struct bpf_link_create_opts {
 			const unsigned long *addrs;
 			const __u64 *cookies;
 		} kprobe_multi;
+		struct {
+			__u32 flags;
+			__u32 cnt;
+			const char **paths;
+			const unsigned long *offsets;
+			const unsigned long *ref_ctr_offsets;
+			const __u64 *cookies;
+		} uprobe_multi;
 		struct {
 			__u64 cookie;
 		} tracing;
 	};
 	size_t :0;
 };
-#define bpf_link_create_opts__last_field kprobe_multi.cookies
+#define bpf_link_create_opts__last_field uprobe_multi.cookies
 
 LIBBPF_API int bpf_link_create(int prog_fd, int target_fd,
 			       enum bpf_attach_type attach_type,
-- 
2.40.0

[RFC/PATCH bpf-next 10/20] libbpf: Add bpf_program__attach_uprobe_multi_opts function

[Jiri Olsa]

Adding bpf_program__attach_uprobe_multi_opts function that
allows to attach multiple uprobes with uprobe_multi link.

The user can specify uprobes with direct arguments:

  binary_path/func_pattern

or with struct bpf_uprobe_multi_opts opts argument fields:

  const char **paths;
  const char **syms;
  const unsigned long *offsets;
  const unsigned long *ref_ctr_offsets;

User can specify 3 mutually exclusive set of incputs:

 1) use only binary_path/func_pattern aruments

 2) use only opts argument with allowed combinations of:
    paths/offsets/ref_ctr_offsets/cookies/cnt

 3) use binary_path with allowed combinations of:
    syms/offsets/ref_ctr_offsets/cookies/cnt

Any other usage results in error.

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

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 7eb7035f7b73..c786bc142791 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -11345,6 +11345,143 @@ static int resolve_full_path(const char *file, char *result, size_t result_sz)
 	return -ENOENT;
 }
 
+struct bpf_link *
+bpf_program__attach_uprobe_multi_opts(const struct bpf_program *prog,
+				      const char *binary_path,
+				      const char *func_pattern,
+				      const struct bpf_uprobe_multi_opts *opts)
+{
+	const unsigned long *ref_ctr_offsets = NULL, *offsets = NULL;
+	LIBBPF_OPTS(bpf_link_create_opts, lopts);
+	unsigned long *resolved_offsets = NULL;
+	const char **resolved_symbols = NULL;
+	const char **resolved_paths = NULL;
+	int i, err = 0, link_fd, prog_fd;
+	struct bpf_link *link = NULL;
+	char errmsg[STRERR_BUFSIZE];
+	const char **paths, **syms;
+	char full_path[PATH_MAX];
+	const __u64 *cookies;
+	bool has_pattern;
+	bool retprobe;
+	size_t cnt;
+
+	if (!OPTS_VALID(opts, bpf_uprobe_multi_opts))
+		return libbpf_err_ptr(-EINVAL);
+
+	paths = OPTS_GET(opts, paths, NULL);
+	syms = OPTS_GET(opts, syms, NULL);
+	offsets = OPTS_GET(opts, offsets, NULL);
+	ref_ctr_offsets = OPTS_GET(opts, ref_ctr_offsets, NULL);
+	cookies = OPTS_GET(opts, cookies, NULL);
+	cnt = OPTS_GET(opts, cnt, 0);
+
+	/*
+	 * User can specify 3 mutually exclusive set of incputs:
+	 *
+	 * 1) use only binary_path/func_pattern aruments
+	 *
+	 * 2) use only opts argument with allowed combinations of:
+	 *    paths/offsets/ref_ctr_offsets/cookies/cnt
+	 *
+	 * 3) use binary_path with allowed combinations of:
+	 *    syms/offsets/ref_ctr_offsets/cookies/cnt
+	 *
+	 * Any other usage results in error.
+	 */
+
+	if (!binary_path && !func_pattern && !cnt)
+		return libbpf_err_ptr(-EINVAL);
+	if (func_pattern && !binary_path)
+		return libbpf_err_ptr(-EINVAL);
+
+	has_pattern = binary_path && func_pattern;
+
+	if (has_pattern) {
+		if (paths || syms || offsets || ref_ctr_offsets || cookies || cnt)
+			return libbpf_err_ptr(-EINVAL);
+	} else {
+		if (!cnt)
+			return libbpf_err_ptr(-EINVAL);
+		if (!!paths == !!binary_path)
+			return libbpf_err_ptr(-EINVAL);
+		if (!!syms == !!offsets)
+			return libbpf_err_ptr(-EINVAL);
+		if (paths && syms)
+			return libbpf_err_ptr(-EINVAL);
+	}
+
+	if (has_pattern) {
+		if (!strchr(binary_path, '/')) {
+			err = resolve_full_path(binary_path, full_path, sizeof(full_path));
+			if (err) {
+				pr_warn("prog '%s': failed to resolve full path for '%s': %d\n",
+					prog->name, binary_path, err);
+				return libbpf_err_ptr(err);
+			}
+			binary_path = full_path;
+		}
+
+		err = elf_find_patern_func_offset(binary_path, func_pattern,
+						  &resolved_symbols, &resolved_offsets,
+						  &cnt);
+		if (err < 0)
+			return libbpf_err_ptr(err);
+		offsets = resolved_offsets;
+	} else if (syms) {
+		err = elf_find_multi_func_offset(binary_path, cnt, syms, &resolved_offsets);
+		if (err < 0)
+			return libbpf_err_ptr(err);
+		offsets = resolved_offsets;
+	}
+
+	if (binary_path) {
+		resolved_paths = calloc(cnt, sizeof(*paths));
+		if (!resolved_paths)
+			goto error;
+		for (i = 0; i < cnt; i++)
+			resolved_paths[i] = binary_path;
+		paths = resolved_paths;
+	}
+
+	retprobe = OPTS_GET(opts, retprobe, false);
+
+	lopts.uprobe_multi.paths = paths;
+	lopts.uprobe_multi.offsets = offsets;
+	lopts.uprobe_multi.ref_ctr_offsets = ref_ctr_offsets;
+	lopts.uprobe_multi.cookies = cookies;
+	lopts.uprobe_multi.cnt = cnt;
+	lopts.uprobe_multi.flags = retprobe ? BPF_F_UPROBE_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_UPROBE_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(resolved_offsets);
+	free(resolved_symbols);
+	free(resolved_paths);
+	return link;
+
+error:
+	free(resolved_offsets);
+	free(resolved_symbols);
+	free(resolved_paths);
+	free(link);
+	return libbpf_err_ptr(err);
+}
+
 LIBBPF_API struct bpf_link *
 bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid,
 				const char *binary_path, size_t func_offset,
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 96ed109ae011..921ab2a94cec 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -529,6 +529,34 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
 				      const char *pattern,
 				      const struct bpf_kprobe_multi_opts *opts);
 
+struct bpf_uprobe_multi_opts {
+	/* size of this struct, for forward/backward compatibility */
+	size_t sz;
+	/* array of paths to attach */
+	const char **paths;
+	/* array of function symbols to attach */
+	const char **syms;
+	/* array of function addresses to attach */
+	const unsigned long *offsets;
+	/* array of refctr offsets to attach */
+	const unsigned long *ref_ctr_offsets;
+	/* 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 uprobes */
+	bool retprobe;
+	size_t :0;
+};
+
+#define bpf_uprobe_multi_opts__last_field retprobe
+
+LIBBPF_API struct bpf_link *
+bpf_program__attach_uprobe_multi_opts(const struct bpf_program *prog,
+				      const char *binary_path,
+				      const char *func_pattern,
+				      const struct bpf_uprobe_multi_opts *opts);
+
 struct bpf_ksyscall_opts {
 	/* size of this struct, for forward/backward compatibility */
 	size_t sz;
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 2f091a0f093b..7aae41ff181d 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -392,4 +392,5 @@ LIBBPF_1.2.0 {
 		bpf_prog_get_info_by_fd;
 		elf_find_multi_func_offset;
 		elf_find_patern_func_offset;
+		bpf_program__attach_uprobe_multi_opts;
 } LIBBPF_1.1.0;
-- 
2.40.0

[RFC/PATCH bpf-next 11/20] libbpf: Add support for uprobe.multi/uprobe.multi program sections

[Jiri Olsa]

Adding support for uprobe.multi/uprobe.multi program sections
to allow auto attach of multi_uprobe programs.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/bpf/libbpf.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index c786bc142791..70353aaac86e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -8628,6 +8628,7 @@ static int attach_tp(const struct bpf_program *prog, long cookie, struct bpf_lin
 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_uprobe_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);
 
@@ -8643,6 +8644,8 @@ static const struct bpf_sec_def section_defs[] = {
 	SEC_DEF("uretprobe.s+",		KPROBE, 0, SEC_SLEEPABLE, attach_uprobe),
 	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("uprobe.multi+",	KPROBE,	BPF_TRACE_UPROBE_MULTI, SEC_NONE, attach_uprobe_multi),
+	SEC_DEF("uretprobe.multi+",	KPROBE,	BPF_TRACE_UPROBE_MULTI, SEC_NONE, attach_uprobe_multi),
 	SEC_DEF("ksyscall+",		KPROBE,	0, SEC_NONE, attach_ksyscall),
 	SEC_DEF("kretsyscall+",		KPROBE, 0, SEC_NONE, attach_ksyscall),
 	SEC_DEF("usdt+",		KPROBE,	0, SEC_NONE, attach_usdt),
@@ -10611,6 +10614,41 @@ static int attach_kprobe_multi(const struct bpf_program *prog, long cookie, stru
 	return libbpf_get_error(*link);
 }
 
+static int attach_uprobe_multi(const struct bpf_program *prog, long cookie, struct bpf_link **link)
+{
+	LIBBPF_OPTS(bpf_uprobe_multi_opts, opts);
+	char *probe_type = NULL, *binary_path = NULL, *func_name = NULL;
+	int n, ret = -EINVAL;
+
+	*link = NULL;
+
+	n = sscanf(prog->sec_name, "%m[^/]/%m[^:]:%m[a-zA-Z0-9_.*?]+",
+		   &probe_type, &binary_path, &func_name);
+	switch (n) {
+	case 1:
+		/* handle SEC("u[ret]probe") - format is valid, but auto-attach is impossible. */
+		ret = 0;
+		break;
+	case 2:
+		pr_warn("prog '%s': section '%s' missing ':function[+offset]' specification\n",
+			prog->name, prog->sec_name);
+		break;
+	case 3:
+		opts.retprobe = strcmp(probe_type, "uretprobe.multi");
+		*link = bpf_program__attach_uprobe_multi_opts(prog, binary_path, func_name, &opts);
+		ret = libbpf_get_error(*link);
+		break;
+	default:
+		pr_warn("prog '%s': invalid format of section definition '%s'\n", prog->name,
+			prog->sec_name);
+		break;
+	}
+	free(probe_type);
+	free(binary_path);
+	free(func_name);
+	return ret;
+}
+
 static void gen_uprobe_legacy_event_name(char *buf, size_t buf_sz,
 					 const char *binary_path, uint64_t offset)
 {
-- 
2.40.0

Re: [RFC/PATCH bpf-next 11/20] libbpf: Add support for uprobe.multi/uprobe.multi program sections

[Andrii Nakryiko]

On Mon, Apr 24, 2023 at 9:06 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding support for uprobe.multi/uprobe.multi program sections
> to allow auto attach of multi_uprobe programs.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/lib/bpf/libbpf.c | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index c786bc142791..70353aaac86e 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -8628,6 +8628,7 @@ static int attach_tp(const struct bpf_program *prog, long cookie, struct bpf_lin
>  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_uprobe_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);
>
> @@ -8643,6 +8644,8 @@ static const struct bpf_sec_def section_defs[] = {
>         SEC_DEF("uretprobe.s+",         KPROBE, 0, SEC_SLEEPABLE, attach_uprobe),
>         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("uprobe.multi+",        KPROBE, BPF_TRACE_UPROBE_MULTI, SEC_NONE, attach_uprobe_multi),
> +       SEC_DEF("uretprobe.multi+",     KPROBE, BPF_TRACE_UPROBE_MULTI, SEC_NONE, attach_uprobe_multi),
>         SEC_DEF("ksyscall+",            KPROBE, 0, SEC_NONE, attach_ksyscall),
>         SEC_DEF("kretsyscall+",         KPROBE, 0, SEC_NONE, attach_ksyscall),
>         SEC_DEF("usdt+",                KPROBE, 0, SEC_NONE, attach_usdt),
> @@ -10611,6 +10614,41 @@ static int attach_kprobe_multi(const struct bpf_program *prog, long cookie, stru
>         return libbpf_get_error(*link);
>  }
>
> +static int attach_uprobe_multi(const struct bpf_program *prog, long cookie, struct bpf_link **link)
> +{
> +       LIBBPF_OPTS(bpf_uprobe_multi_opts, opts);
> +       char *probe_type = NULL, *binary_path = NULL, *func_name = NULL;
> +       int n, ret = -EINVAL;
> +
> +       *link = NULL;
> +
> +       n = sscanf(prog->sec_name, "%m[^/]/%m[^:]:%m[a-zA-Z0-9_.*?]+",

Arnaldo recently brought to my attention that Go doesn't do mangling,
so their function names are crazy, e.g.:

"go/doc/comment.(*parseDoc).code"

So we should think about making no assumptions about pattern inside
`%m[a-zA-Z0-9_.*?]`

> +                  &probe_type, &binary_path, &func_name);
> +       switch (n) {
> +       case 1:
> +               /* handle SEC("u[ret]probe") - format is valid, but auto-attach is impossible. */
> +               ret = 0;
> +               break;
> +       case 2:
> +               pr_warn("prog '%s': section '%s' missing ':function[+offset]' specification\n",
> +                       prog->name, prog->sec_name);
> +               break;
> +       case 3:
> +               opts.retprobe = strcmp(probe_type, "uretprobe.multi");
> +               *link = bpf_program__attach_uprobe_multi_opts(prog, binary_path, func_name, &opts);
> +               ret = libbpf_get_error(*link);
> +               break;
> +       default:
> +               pr_warn("prog '%s': invalid format of section definition '%s'\n", prog->name,
> +                       prog->sec_name);
> +               break;
> +       }
> +       free(probe_type);
> +       free(binary_path);
> +       free(func_name);
> +       return ret;
> +}
> +
>  static void gen_uprobe_legacy_event_name(char *buf, size_t buf_sz,
>                                          const char *binary_path, uint64_t offset)
>  {
> --
> 2.40.0
>

[RFC/PATCH bpf-next 12/20] libbpf: Add uprobe multi link support to bpf_program__attach_usdt

[Jiri Olsa]

Adding uprobe_multi bool to struct bpf_usdt_opts. If it's true
the usdt_manager_attach_usdt will use uprobe_multi link to attach
to usdt probes.

The bpf program for usdt probe needs to have BPF_TRACE_UPROBE_MULTI
set as expected_attach_type.

Because current uprobe is implemented through perf event interface,
it allows the pid filter for uprobes. This is not the case for
uprobe_multi link, so the pid filter is not allowed for that.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/bpf/libbpf.c          |   9 ++-
 tools/lib/bpf/libbpf.h          |   2 +
 tools/lib/bpf/libbpf_internal.h |   2 +-
 tools/lib/bpf/usdt.c            | 127 ++++++++++++++++++++++++--------
 4 files changed, 105 insertions(+), 35 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 70353aaac86e..25d32aa605e8 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -6817,7 +6817,6 @@ static int bpf_object_load_prog(struct bpf_object *obj, struct bpf_program *prog
 	if (!insns || !insns_cnt)
 		return -EINVAL;
 
-	load_attr.expected_attach_type = prog->expected_attach_type;
 	if (kernel_supports(obj, FEAT_PROG_NAME))
 		prog_name = prog->name;
 	load_attr.attach_prog_fd = prog->attach_prog_fd;
@@ -6853,6 +6852,9 @@ static int bpf_object_load_prog(struct bpf_object *obj, struct bpf_program *prog
 		insns_cnt = prog->insns_cnt;
 	}
 
+	/* allow prog_prepare_load_fn to change expected_attach_type */
+	load_attr.expected_attach_type = prog->expected_attach_type;
+
 	if (obj->gen_loader) {
 		bpf_gen__prog_load(obj->gen_loader, prog->type, prog->name,
 				   license, insns, insns_cnt, &load_attr,
@@ -11730,6 +11732,7 @@ struct bpf_link *bpf_program__attach_usdt(const struct bpf_program *prog,
 	struct bpf_object *obj = prog->obj;
 	struct bpf_link *link;
 	__u64 usdt_cookie;
+	bool uprobe_multi;
 	int err;
 
 	if (!OPTS_VALID(opts, bpf_uprobe_opts))
@@ -11766,8 +11769,10 @@ struct bpf_link *bpf_program__attach_usdt(const struct bpf_program *prog,
 	}
 
 	usdt_cookie = OPTS_GET(opts, usdt_cookie, 0);
+	uprobe_multi = OPTS_GET(opts, uprobe_multi, 0);
 	link = usdt_manager_attach_usdt(obj->usdt_man, prog, pid, binary_path,
-					usdt_provider, usdt_name, usdt_cookie);
+					usdt_provider, usdt_name, usdt_cookie,
+					uprobe_multi);
 	err = libbpf_get_error(link);
 	if (err)
 		return libbpf_err_ptr(err);
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 921ab2a94cec..025feb21c2ec 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -673,6 +673,8 @@ struct bpf_usdt_opts {
 	size_t sz;
 	/* custom user-provided value accessible through usdt_cookie() */
 	__u64 usdt_cookie;
+	/* use uprobe_multi link */
+	bool uprobe_multi;
 	size_t :0;
 };
 #define bpf_usdt_opts__last_field usdt_cookie
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index e4d05662a96c..5d5f61d0bcfb 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -567,7 +567,7 @@ struct bpf_link * usdt_manager_attach_usdt(struct usdt_manager *man,
 					   const struct bpf_program *prog,
 					   pid_t pid, const char *path,
 					   const char *usdt_provider, const char *usdt_name,
-					   __u64 usdt_cookie);
+					   __u64 usdt_cookie, bool uprobe_multi);
 
 static inline bool is_pow_of_2(size_t x)
 {
diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c
index b8402e3f9eb2..f55dbd47d29e 100644
--- a/tools/lib/bpf/usdt.c
+++ b/tools/lib/bpf/usdt.c
@@ -803,11 +803,20 @@ struct bpf_link_usdt {
 	size_t spec_cnt;
 	int *spec_ids;
 
+	bool has_uprobe_multi;
+
 	size_t uprobe_cnt;
 	struct {
 		long abs_ip;
 		struct bpf_link *link;
 	} *uprobes;
+	struct {
+		char **paths;
+		unsigned long *offsets;
+		unsigned long *ref_ctr_offsets;
+		__u64 *cookies;
+		struct bpf_link *link;
+	} uprobe_multi;
 };
 
 static int bpf_link_usdt_detach(struct bpf_link *link)
@@ -816,19 +825,23 @@ static int bpf_link_usdt_detach(struct bpf_link *link)
 	struct usdt_manager *man = usdt_link->usdt_man;
 	int i;
 
-	for (i = 0; i < usdt_link->uprobe_cnt; i++) {
-		/* detach underlying uprobe link */
-		bpf_link__destroy(usdt_link->uprobes[i].link);
-		/* there is no need to update specs map because it will be
-		 * unconditionally overwritten on subsequent USDT attaches,
-		 * but if BPF cookies are not used we need to remove entry
-		 * from ip_to_spec_id map, otherwise we'll run into false
-		 * conflicting IP errors
-		 */
-		if (!man->has_bpf_cookie) {
-			/* not much we can do about errors here */
-			(void)bpf_map_delete_elem(bpf_map__fd(man->ip_to_spec_id_map),
-						  &usdt_link->uprobes[i].abs_ip);
+	if (usdt_link->has_uprobe_multi) {
+		bpf_link__destroy(usdt_link->uprobe_multi.link);
+	} else {
+		for (i = 0; i < usdt_link->uprobe_cnt; i++) {
+			/* detach underlying uprobe link */
+			bpf_link__destroy(usdt_link->uprobes[i].link);
+			/* there is no need to update specs map because it will be
+			 * unconditionally overwritten on subsequent USDT attaches,
+			 * but if BPF cookies are not used we need to remove entry
+			 * from ip_to_spec_id map, otherwise we'll run into false
+			 * conflicting IP errors
+			 */
+			if (!man->has_bpf_cookie) {
+				/* not much we can do about errors here */
+				(void)bpf_map_delete_elem(bpf_map__fd(man->ip_to_spec_id_map),
+							  &usdt_link->uprobes[i].abs_ip);
+			}
 		}
 	}
 
@@ -868,9 +881,16 @@ static void bpf_link_usdt_dealloc(struct bpf_link *link)
 {
 	struct bpf_link_usdt *usdt_link = container_of(link, struct bpf_link_usdt, link);
 
-	free(usdt_link->spec_ids);
-	free(usdt_link->uprobes);
-	free(usdt_link);
+	if (usdt_link->has_uprobe_multi) {
+		free(usdt_link->uprobe_multi.paths);
+		free(usdt_link->uprobe_multi.offsets);
+		free(usdt_link->uprobe_multi.ref_ctr_offsets);
+		free(usdt_link->uprobe_multi.cookies);
+	} else {
+		free(usdt_link->spec_ids);
+		free(usdt_link->uprobes);
+		free(usdt_link);
+	}
 }
 
 static size_t specs_hash_fn(long key, void *ctx)
@@ -941,16 +961,22 @@ static int allocate_spec_id(struct usdt_manager *man, struct hashmap *specs_hash
 struct bpf_link *usdt_manager_attach_usdt(struct usdt_manager *man, const struct bpf_program *prog,
 					  pid_t pid, const char *path,
 					  const char *usdt_provider, const char *usdt_name,
-					  __u64 usdt_cookie)
+					  __u64 usdt_cookie, bool uprobe_multi)
 {
+	LIBBPF_OPTS(bpf_uprobe_multi_opts, opts_multi);
 	int i, fd, err, spec_map_fd, ip_map_fd;
 	LIBBPF_OPTS(bpf_uprobe_opts, opts);
 	struct hashmap *specs_hash = NULL;
 	struct bpf_link_usdt *link = NULL;
 	struct usdt_target *targets = NULL;
+	struct bpf_link *uprobe_link;
 	size_t target_cnt;
 	Elf *elf;
 
+	/* The uprobe_multi link does not have pid filter. */
+	if (uprobe_multi && pid >= 0)
+		return libbpf_err_ptr(-EINVAL);
+
 	spec_map_fd = bpf_map__fd(man->specs_map);
 	ip_map_fd = bpf_map__fd(man->ip_to_spec_id_map);
 
@@ -1001,19 +1027,32 @@ struct bpf_link *usdt_manager_attach_usdt(struct usdt_manager *man, const struct
 		goto err_out;
 	}
 
+	link->has_uprobe_multi = uprobe_multi;
 	link->usdt_man = man;
 	link->link.detach = &bpf_link_usdt_detach;
 	link->link.dealloc = &bpf_link_usdt_dealloc;
 
-	link->uprobes = calloc(target_cnt, sizeof(*link->uprobes));
-	if (!link->uprobes) {
-		err = -ENOMEM;
-		goto err_out;
+	if (uprobe_multi) {
+		link->uprobe_multi.paths = calloc(target_cnt, sizeof(*link->uprobe_multi.paths));
+		link->uprobe_multi.offsets = calloc(target_cnt, sizeof(*link->uprobe_multi.offsets));
+		link->uprobe_multi.ref_ctr_offsets = calloc(target_cnt, sizeof(*link->uprobe_multi.ref_ctr_offsets));
+		link->uprobe_multi.cookies = calloc(target_cnt, sizeof(*link->uprobe_multi.cookies));
+
+		if (!link->uprobe_multi.paths || !link->uprobe_multi.offsets ||
+		    !link->uprobe_multi.ref_ctr_offsets || !link->uprobe_multi.cookies) {
+			err = -ENOMEM;
+			goto err_out;
+		}
+	} else {
+		link->uprobes = calloc(target_cnt, sizeof(*link->uprobes));
+		if (!link->uprobes) {
+			err = -ENOMEM;
+			goto err_out;
+		}
 	}
 
 	for (i = 0; i < target_cnt; i++) {
 		struct usdt_target *target = &targets[i];
-		struct bpf_link *uprobe_link;
 		bool is_new;
 		int spec_id;
 
@@ -1049,20 +1088,44 @@ struct bpf_link *usdt_manager_attach_usdt(struct usdt_manager *man, const struct
 			goto err_out;
 		}
 
-		opts.ref_ctr_offset = target->sema_off;
-		opts.bpf_cookie = man->has_bpf_cookie ? spec_id : 0;
-		uprobe_link = bpf_program__attach_uprobe_opts(prog, pid, path,
-							      target->rel_ip, &opts);
+		if (uprobe_multi) {
+			link->uprobe_multi.paths[i] = (char *) path;
+			link->uprobe_multi.offsets[i] = target->rel_ip;
+			link->uprobe_multi.ref_ctr_offsets[i] = target->sema_off;
+			link->uprobe_multi.cookies[i] = spec_id;
+		} else {
+			opts.ref_ctr_offset = target->sema_off;
+			opts.bpf_cookie = man->has_bpf_cookie ? spec_id : 0;
+			uprobe_link = bpf_program__attach_uprobe_opts(prog, pid, path,
+								      target->rel_ip, &opts);
+			err = libbpf_get_error(uprobe_link);
+			if (err) {
+				pr_warn("usdt: failed to attach uprobe #%d for '%s:%s' in '%s': %d\n",
+					i, usdt_provider, usdt_name, path, err);
+				goto err_out;
+			}
+
+			link->uprobes[i].link = uprobe_link;
+			link->uprobes[i].abs_ip = target->abs_ip;
+			link->uprobe_cnt++;
+		}
+	}
+
+	if (uprobe_multi) {
+		opts_multi.cnt = target_cnt;
+		opts_multi.paths = (const char **) link->uprobe_multi.paths;
+		opts_multi.offsets = link->uprobe_multi.offsets;
+		opts_multi.ref_ctr_offsets = link->uprobe_multi.ref_ctr_offsets;
+		opts_multi.cookies = link->uprobe_multi.cookies;
+
+		uprobe_link = bpf_program__attach_uprobe_multi_opts(prog, NULL, NULL, &opts_multi);
 		err = libbpf_get_error(uprobe_link);
 		if (err) {
-			pr_warn("usdt: failed to attach uprobe #%d for '%s:%s' in '%s': %d\n",
-				i, usdt_provider, usdt_name, path, err);
+			pr_warn("usdt: failed to attach uprobe multi for '%s:%s' in '%s': %d\n",
+				usdt_provider, usdt_name, path, err);
 			goto err_out;
 		}
-
-		link->uprobes[i].link = uprobe_link;
-		link->uprobes[i].abs_ip = target->abs_ip;
-		link->uprobe_cnt++;
+		link->uprobe_multi.link = uprobe_link;
 	}
 
 	free(targets);
-- 
2.40.0

Re: [RFC/PATCH bpf-next 12/20] libbpf: Add uprobe multi link support to bpf_program__attach_usdt

[Andrii Nakryiko]

On Mon, Apr 24, 2023 at 9:06 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding uprobe_multi bool to struct bpf_usdt_opts. If it's true
> the usdt_manager_attach_usdt will use uprobe_multi link to attach
> to usdt probes.
>
> The bpf program for usdt probe needs to have BPF_TRACE_UPROBE_MULTI
> set as expected_attach_type.
>
> Because current uprobe is implemented through perf event interface,
> it allows the pid filter for uprobes. This is not the case for
> uprobe_multi link, so the pid filter is not allowed for that.

Ok, yep, let's fix that at kernel UAPI level and use multi-uprobe
transparently, if kernel supports it. This seems like a big UX problem
currently with uprobe.multi (and consequently with USDT programs)

>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/lib/bpf/libbpf.c          |   9 ++-
>  tools/lib/bpf/libbpf.h          |   2 +
>  tools/lib/bpf/libbpf_internal.h |   2 +-
>  tools/lib/bpf/usdt.c            | 127 ++++++++++++++++++++++++--------
>  4 files changed, 105 insertions(+), 35 deletions(-)
>

[...]

[RFC/PATCH bpf-next 13/20] selftests/bpf: Add uprobe_multi skel test

[Jiri Olsa]

Adding uprobe_multi test for skeleton load/attach functions,
to test skeleton auto attach for uprobe_multi link.

Test that bpf_get_func_ip works properly for uprobe_multi
attachment.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 .../bpf/prog_tests/uprobe_multi_test.c        | 66 +++++++++++++++++++
 .../selftests/bpf/progs/uprobe_multi.c        | 63 ++++++++++++++++++
 2 files changed, 129 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
 create mode 100644 tools/testing/selftests/bpf/progs/uprobe_multi.c

diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
new file mode 100644
index 000000000000..f68cda122ac2
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <unistd.h>
+#include <test_progs.h>
+#include "uprobe_multi.skel.h"
+
+noinline void uprobe_multi_func_1(void)
+{
+	asm volatile ("");
+}
+
+noinline void uprobe_multi_func_2(void)
+{
+	asm volatile ("");
+}
+
+noinline void uprobe_multi_func_3(void)
+{
+	asm volatile ("");
+}
+
+static void uprobe_multi_test_run(struct uprobe_multi *skel)
+{
+	skel->bss->uprobe_multi_func_1_addr = (u64) uprobe_multi_func_1;
+	skel->bss->uprobe_multi_func_2_addr = (u64) uprobe_multi_func_2;
+	skel->bss->uprobe_multi_func_3_addr = (u64) uprobe_multi_func_3;
+
+	skel->bss->pid = getpid();
+
+	uprobe_multi_func_1();
+	uprobe_multi_func_2();
+	uprobe_multi_func_3();
+
+	ASSERT_EQ(skel->bss->uprobe_multi_func_1_result, 1, "uprobe_multi_func_1_result");
+	ASSERT_EQ(skel->bss->uprobe_multi_func_2_result, 1, "uprobe_multi_func_2_result");
+	ASSERT_EQ(skel->bss->uprobe_multi_func_3_result, 1, "uprobe_multi_func_3_result");
+
+	ASSERT_EQ(skel->bss->uretprobe_multi_func_1_result, 1, "uretprobe_multi_func_1_result");
+	ASSERT_EQ(skel->bss->uretprobe_multi_func_2_result, 1, "uretprobe_multi_func_2_result");
+	ASSERT_EQ(skel->bss->uretprobe_multi_func_3_result, 1, "uretprobe_multi_func_3_result");
+}
+
+static void test_skel_api(void)
+{
+	struct uprobe_multi *skel = NULL;
+	int err;
+
+	skel = uprobe_multi__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "uprobe_multi__open_and_load"))
+		goto cleanup;
+
+	err = uprobe_multi__attach(skel);
+	if (!ASSERT_OK(err, "uprobe_multi__attach"))
+		goto cleanup;
+
+	uprobe_multi_test_run(skel);
+
+cleanup:
+	uprobe_multi__destroy(skel);
+}
+
+void test_uprobe_multi_test(void)
+{
+	if (test__start_subtest("skel_api"))
+		test_skel_api();
+}
diff --git a/tools/testing/selftests/bpf/progs/uprobe_multi.c b/tools/testing/selftests/bpf/progs/uprobe_multi.c
new file mode 100644
index 000000000000..a5dc00938217
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/uprobe_multi.c
@@ -0,0 +1,63 @@
+// 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";
+
+__u64 uprobe_multi_func_1_addr = 0;
+__u64 uprobe_multi_func_2_addr = 0;
+__u64 uprobe_multi_func_3_addr = 0;
+
+__u64 uprobe_multi_func_1_result = 0;
+__u64 uprobe_multi_func_2_result = 0;
+__u64 uprobe_multi_func_3_result = 0;
+
+__u64 uretprobe_multi_func_1_result = 0;
+__u64 uretprobe_multi_func_2_result = 0;
+__u64 uretprobe_multi_func_3_result = 0;
+
+int pid = 0;
+bool test_cookie = false;
+
+static void uprobe_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, __cookie) ({			\
+	if (addr == __addr &&				\
+	   (!test_cookie || (cookie == __cookie)))	\
+		__var = 1;				\
+})
+
+	if (is_return) {
+		SET(uretprobe_multi_func_1_result, uprobe_multi_func_1_addr, 2);
+		SET(uretprobe_multi_func_2_result, uprobe_multi_func_2_addr, 3);
+		SET(uretprobe_multi_func_3_result, uprobe_multi_func_3_addr, 1);
+	} else {
+		SET(uprobe_multi_func_1_result, uprobe_multi_func_1_addr, 3);
+		SET(uprobe_multi_func_2_result, uprobe_multi_func_2_addr, 1);
+		SET(uprobe_multi_func_3_result, uprobe_multi_func_3_addr, 2);
+	}
+
+#undef SET
+}
+
+SEC("uprobe.multi//proc/self/exe:uprobe_multi_func_*")
+int test_uprobe(struct pt_regs *ctx)
+{
+	uprobe_multi_check(ctx, false);
+	return 0;
+}
+
+SEC("uretprobe.multi//proc/self/exe:uprobe_multi_func_*")
+int test_uretprobe(struct pt_regs *ctx)
+{
+	uprobe_multi_check(ctx, true);
+	return 0;
+}
-- 
2.40.0

[RFC/PATCH bpf-next 14/20] selftests/bpf: Add uprobe_multi api test

[Jiri Olsa]

Adding uprobe_multi test for bpf_program__attach_uprobe_multi_opts
attach function.

Testing attachment using glob patterns and via bpf_uprobe_multi_opts
paths/syms fields.

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

diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
index f68cda122ac2..179b78a4b711 100644
--- a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
@@ -59,8 +59,63 @@ static void test_skel_api(void)
 	uprobe_multi__destroy(skel);
 }
 
+static void
+test_attach_api(const char *binary, const char *pattern, struct bpf_uprobe_multi_opts *opts)
+{
+	struct bpf_link *link1 = NULL, *link2 = NULL;
+	struct uprobe_multi *skel = NULL;
+
+	skel = uprobe_multi__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "uprobe_multi"))
+		goto cleanup;
+
+	link1 = bpf_program__attach_uprobe_multi_opts(skel->progs.test_uprobe,
+						      binary, pattern, opts);
+	if (!ASSERT_OK_PTR(link1, "bpf_program__attach_uprobe_multi_opts"))
+		goto cleanup;
+
+	opts->retprobe = true;
+	link2 = bpf_program__attach_uprobe_multi_opts(skel->progs.test_uretprobe,
+						      binary, pattern, opts);
+	if (!ASSERT_OK_PTR(link2, "bpf_program__attach_uprobe_multi_opts_retprobe"))
+		goto cleanup;
+
+	uprobe_multi_test_run(skel);
+
+cleanup:
+	bpf_link__destroy(link2);
+	bpf_link__destroy(link1);
+	uprobe_multi__destroy(skel);
+}
+
+static void test_attach_api_pattern(void)
+{
+	LIBBPF_OPTS(bpf_uprobe_multi_opts, opts);
+
+	test_attach_api("/proc/self/exe", "uprobe_multi_func_*", &opts);
+	test_attach_api("/proc/self/exe", "uprobe_multi_func_?", &opts);
+}
+
+static void test_attach_api_syms(void)
+{
+	LIBBPF_OPTS(bpf_uprobe_multi_opts, opts);
+	const char *syms[3] = {
+		"uprobe_multi_func_1",
+		"uprobe_multi_func_2",
+		"uprobe_multi_func_3",
+	};
+
+	opts.syms = syms;
+	opts.cnt = ARRAY_SIZE(syms);
+	test_attach_api("/proc/self/exe", NULL, &opts);
+}
+
 void test_uprobe_multi_test(void)
 {
 	if (test__start_subtest("skel_api"))
 		test_skel_api();
+	if (test__start_subtest("attach_api_pattern"))
+		test_attach_api_pattern();
+	if (test__start_subtest("attach_api_syms"))
+		test_attach_api_syms();
 }
-- 
2.40.0

[RFC/PATCH bpf-next 15/20] selftests/bpf: Add uprobe_multi link test

[Jiri Olsa]

Adding uprobe_multi test for bpf_link_create attach function.

Testing attachment using the struct bpf_link_create_opts.

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

diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
index 179b78a4b711..abe620d844e7 100644
--- a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
@@ -110,6 +110,59 @@ static void test_attach_api_syms(void)
 	test_attach_api("/proc/self/exe", NULL, &opts);
 }
 
+void test_link_api(void)
+{
+	int prog_fd, link1_fd = -1, link2_fd = -1;
+	LIBBPF_OPTS(bpf_link_create_opts, opts);
+	struct uprobe_multi *skel = NULL;
+	unsigned long *offsets = NULL;
+	const char *syms[3] = {
+		"uprobe_multi_func_1",
+		"uprobe_multi_func_2",
+		"uprobe_multi_func_3",
+	};
+	const char *paths[3] = {
+		"/proc/self/exe",
+		"/proc/self/exe",
+		"/proc/self/exe",
+	};
+	int err;
+
+	err = elf_find_multi_func_offset(paths[0], 3, syms, (unsigned long **) &offsets);
+	if (!ASSERT_OK(err, "elf_find_multi_func_offset"))
+		return;
+
+	opts.uprobe_multi.paths = paths;
+	opts.uprobe_multi.offsets = offsets;
+	opts.uprobe_multi.cnt = ARRAY_SIZE(syms);
+
+	skel = uprobe_multi__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "uprobe_multi"))
+		goto cleanup;
+
+	prog_fd = bpf_program__fd(skel->progs.test_uprobe);
+	link1_fd = bpf_link_create(prog_fd, 0, BPF_TRACE_UPROBE_MULTI, &opts);
+	if (!ASSERT_GE(link1_fd, 0, "link_fd"))
+		goto cleanup;
+
+	opts.kprobe_multi.flags = BPF_F_UPROBE_MULTI_RETURN;
+	prog_fd = bpf_program__fd(skel->progs.test_uretprobe);
+	link2_fd = bpf_link_create(prog_fd, 0, BPF_TRACE_UPROBE_MULTI, &opts);
+	if (!ASSERT_GE(link2_fd, 0, "link_fd"))
+		goto cleanup;
+
+	uprobe_multi_test_run(skel);
+
+cleanup:
+	if (link1_fd != -1)
+		close(link1_fd);
+	if (link2_fd != -1)
+		close(link2_fd);
+
+	uprobe_multi__destroy(skel);
+	free(offsets);
+}
+
 void test_uprobe_multi_test(void)
 {
 	if (test__start_subtest("skel_api"))
@@ -118,4 +171,6 @@ void test_uprobe_multi_test(void)
 		test_attach_api_pattern();
 	if (test__start_subtest("attach_api_syms"))
 		test_attach_api_syms();
+	if (test__start_subtest("link_api"))
+		test_link_api();
 }
-- 
2.40.0

[RFC/PATCH bpf-next 16/20] selftests/bpf: Add uprobe_multi test program

[Jiri Olsa]

Adding uprobe_multi test program that defines 50k uprobe_multi_func_*
functions and will serve as attach point for uprobe_multi bench test
in following patch.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/testing/selftests/bpf/Makefile       |  5 ++
 tools/testing/selftests/bpf/uprobe_multi.c | 53 ++++++++++++++++++++++
 2 files changed, 58 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/uprobe_multi.c

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index c49e5403ad0e..506d92d5505a 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -568,6 +568,7 @@ TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read $(OUTPUT)/bpf_testmod.ko	\
 		       $(OUTPUT)/liburandom_read.so			\
 		       $(OUTPUT)/xdp_synproxy				\
 		       $(OUTPUT)/sign-file				\
+		       $(OUTPUT)/uprobe_multi				\
 		       ima_setup.sh 					\
 		       verify_sig_setup.sh				\
 		       $(wildcard progs/btf_dump_test_case_*.c)		\
@@ -671,6 +672,10 @@ $(OUTPUT)/veristat: $(OUTPUT)/veristat.o
 	$(call msg,BINARY,,$@)
 	$(Q)$(CC) $(CFLAGS) $(LDFLAGS) $(filter %.a %.o,$^) $(LDLIBS) -o $@
 
+$(OUTPUT)/uprobe_multi: uprobe_multi.c
+	$(call msg,BINARY,,$@)
+	$(Q)$(CC) $(CFLAGS) $(LDFLAGS) $^ $(LDLIBS) -o $@
+
 EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(SCRATCH_DIR) $(HOST_SCRATCH_DIR)	\
 	prog_tests/tests.h map_tests/tests.h verifier/tests.h		\
 	feature bpftool							\
diff --git a/tools/testing/selftests/bpf/uprobe_multi.c b/tools/testing/selftests/bpf/uprobe_multi.c
new file mode 100644
index 000000000000..115a7f6cebfa
--- /dev/null
+++ b/tools/testing/selftests/bpf/uprobe_multi.c
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <stdio.h>
+
+#define __PASTE(a, b) a##b
+#define PASTE(a, b) __PASTE(a, b)
+
+#define NAME(name, idx) PASTE(name, idx)
+
+#define DEF(name, idx)  int NAME(name, idx)(void) { return 0; }
+#define CALL(name, idx) NAME(name, idx)();
+
+#define F(body, name, idx) body(name, idx)
+
+#define F10(body, name, idx) \
+	F(body, PASTE(name, idx), 0) F(body, PASTE(name, idx), 1) F(body, PASTE(name, idx), 2) \
+	F(body, PASTE(name, idx), 3) F(body, PASTE(name, idx), 4) F(body, PASTE(name, idx), 5) \
+	F(body, PASTE(name, idx), 6) F(body, PASTE(name, idx), 7) F(body, PASTE(name, idx), 8) \
+	F(body, PASTE(name, idx), 9)
+
+#define F100(body, name, idx) \
+	F10(body, PASTE(name, idx), 0) F10(body, PASTE(name, idx), 1) F10(body, PASTE(name, idx), 2) \
+	F10(body, PASTE(name, idx), 3) F10(body, PASTE(name, idx), 4) F10(body, PASTE(name, idx), 5) \
+	F10(body, PASTE(name, idx), 6) F10(body, PASTE(name, idx), 7) F10(body, PASTE(name, idx), 8) \
+	F10(body, PASTE(name, idx), 9)
+
+#define F1000(body, name, idx) \
+	F100(body, PASTE(name, idx), 0) F100(body, PASTE(name, idx), 1) F100(body, PASTE(name, idx), 2) \
+	F100(body, PASTE(name, idx), 3) F100(body, PASTE(name, idx), 4) F100(body, PASTE(name, idx), 5) \
+	F100(body, PASTE(name, idx), 6) F100(body, PASTE(name, idx), 7) F100(body, PASTE(name, idx), 8) \
+	F100(body, PASTE(name, idx), 9)
+
+#define F10000(body, name, idx) \
+	F1000(body, PASTE(name, idx), 0) F1000(body, PASTE(name, idx), 1) F1000(body, PASTE(name, idx), 2) \
+	F1000(body, PASTE(name, idx), 3) F1000(body, PASTE(name, idx), 4) F1000(body, PASTE(name, idx), 5) \
+	F1000(body, PASTE(name, idx), 6) F1000(body, PASTE(name, idx), 7) F1000(body, PASTE(name, idx), 8) \
+	F1000(body, PASTE(name, idx), 9)
+
+F10000(DEF, uprobe_multi_func_, 0)
+F10000(DEF, uprobe_multi_func_, 1)
+F10000(DEF, uprobe_multi_func_, 2)
+F10000(DEF, uprobe_multi_func_, 3)
+F10000(DEF, uprobe_multi_func_, 4)
+
+int main(void)
+{
+	F10000(CALL, uprobe_multi_func_, 0)
+	F10000(CALL, uprobe_multi_func_, 1)
+	F10000(CALL, uprobe_multi_func_, 2)
+	F10000(CALL, uprobe_multi_func_, 3)
+	F10000(CALL, uprobe_multi_func_, 4)
+	return 0;
+}
-- 
2.40.0

[RFC/PATCH bpf-next 17/20] selftests/bpf: Add uprobe_multi bench test

[Jiri Olsa]

Adding test that attaches 50k uprobes in uprobe_multi binary.

After the attach is done we run the binary and make sure we
get proper amount of hits.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 .../bpf/prog_tests/uprobe_multi_test.c        | 56 +++++++++++++++++++
 .../selftests/bpf/progs/uprobe_multi.c        |  9 +++
 2 files changed, 65 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
index abe620d844e7..9a89c61359a1 100644
--- a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
@@ -163,6 +163,60 @@ void test_link_api(void)
 	free(offsets);
 }
 
+static inline __u64 get_time_ns(void)
+{
+	struct timespec t;
+
+	clock_gettime(CLOCK_MONOTONIC, &t);
+	return (__u64) t.tv_sec * 1000000000 + t.tv_nsec;
+}
+
+void test_bench_attach_uprobe(void)
+{
+	long attach_start_ns, attach_end_ns;
+	long detach_start_ns, detach_end_ns;
+	double attach_delta, detach_delta;
+	struct uprobe_multi *skel = NULL;
+	struct bpf_program *prog;
+	int err;
+
+	skel = uprobe_multi__open();
+	if (!ASSERT_OK_PTR(skel, "uprobe_multi__open"))
+		goto cleanup;
+
+	bpf_object__for_each_program(prog, skel->obj)
+		bpf_program__set_autoload(prog, false);
+
+	bpf_program__set_autoload(skel->progs.test_uprobe_bench, true);
+
+	err = uprobe_multi__load(skel);
+	if (!ASSERT_EQ(err, 0, "strncmp_test load"))
+		goto cleanup;
+
+	attach_start_ns = get_time_ns();
+
+	err = uprobe_multi__attach(skel);
+	if (!ASSERT_OK(err, "uprobe_multi__attach"))
+		goto cleanup;
+
+	attach_end_ns = get_time_ns();
+
+	system("./uprobe_multi");
+
+	ASSERT_EQ(skel->bss->count, 50000, "uprobes_count");
+
+cleanup:
+	detach_start_ns = get_time_ns();
+	uprobe_multi__destroy(skel);
+	detach_end_ns = get_time_ns();
+
+	attach_delta = (attach_end_ns - attach_start_ns) / 1000000000.0;
+	detach_delta = (detach_end_ns - detach_start_ns) / 1000000000.0;
+
+	printf("%s: attached in %7.3lfs\n", __func__, attach_delta);
+	printf("%s: detached in %7.3lfs\n", __func__, detach_delta);
+}
+
 void test_uprobe_multi_test(void)
 {
 	if (test__start_subtest("skel_api"))
@@ -173,4 +227,6 @@ void test_uprobe_multi_test(void)
 		test_attach_api_syms();
 	if (test__start_subtest("link_api"))
 		test_link_api();
+	if (test__start_subtest("bench_uprobe"))
+		test_bench_attach_uprobe();
 }
diff --git a/tools/testing/selftests/bpf/progs/uprobe_multi.c b/tools/testing/selftests/bpf/progs/uprobe_multi.c
index a5dc00938217..126abd5aef89 100644
--- a/tools/testing/selftests/bpf/progs/uprobe_multi.c
+++ b/tools/testing/selftests/bpf/progs/uprobe_multi.c
@@ -61,3 +61,12 @@ int test_uretprobe(struct pt_regs *ctx)
 	uprobe_multi_check(ctx, true);
 	return 0;
 }
+
+int count;
+
+SEC("?uprobe.multi/./uprobe_multi:uprobe_multi_func_*")
+int test_uprobe_bench(struct pt_regs *ctx)
+{
+	count++;
+	return 0;
+}
-- 
2.40.0

[RFC/PATCH bpf-next 18/20] selftests/bpf: Add usdt_multi test program

[Jiri Olsa]

Adding usdt_multi test program that defines 50k usdts and will
serve as attach point for uprobe_multi usdt bench test in
following patch.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/testing/selftests/bpf/Makefile     |  5 +++++
 tools/testing/selftests/bpf/usdt_multi.c | 23 +++++++++++++++++++++++
 2 files changed, 28 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/usdt_multi.c

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 506d92d5505a..33278c21ed9d 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -569,6 +569,7 @@ TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read $(OUTPUT)/bpf_testmod.ko	\
 		       $(OUTPUT)/xdp_synproxy				\
 		       $(OUTPUT)/sign-file				\
 		       $(OUTPUT)/uprobe_multi				\
+		       $(OUTPUT)/usdt_multi				\
 		       ima_setup.sh 					\
 		       verify_sig_setup.sh				\
 		       $(wildcard progs/btf_dump_test_case_*.c)		\
@@ -676,6 +677,10 @@ $(OUTPUT)/uprobe_multi: uprobe_multi.c
 	$(call msg,BINARY,,$@)
 	$(Q)$(CC) $(CFLAGS) $(LDFLAGS) $^ $(LDLIBS) -o $@
 
+$(OUTPUT)/usdt_multi: usdt_multi.c
+	$(call msg,BINARY,,$@)
+	$(Q)$(CC) $(CFLAGS) $(LDFLAGS) $^ $(LDLIBS) -o $@
+
 EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(SCRATCH_DIR) $(HOST_SCRATCH_DIR)	\
 	prog_tests/tests.h map_tests/tests.h verifier/tests.h		\
 	feature bpftool							\
diff --git a/tools/testing/selftests/bpf/usdt_multi.c b/tools/testing/selftests/bpf/usdt_multi.c
new file mode 100644
index 000000000000..3216d15dcd76
--- /dev/null
+++ b/tools/testing/selftests/bpf/usdt_multi.c
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <sdt.h>
+
+#define PROBE STAP_PROBE(test, usdt);
+
+#define PROBE10    PROBE PROBE PROBE PROBE PROBE PROBE PROBE PROBE PROBE PROBE
+#define PROBE100   PROBE10 PROBE10 PROBE10 PROBE10 PROBE10 \
+		   PROBE10 PROBE10 PROBE10 PROBE10 PROBE10
+#define PROBE1000  PROBE100 PROBE100 PROBE100 PROBE100 PROBE100 \
+		   PROBE100 PROBE100 PROBE100 PROBE100 PROBE100
+#define PROBE10000 PROBE1000 PROBE1000 PROBE1000 PROBE1000 PROBE1000 \
+		   PROBE1000 PROBE1000 PROBE1000 PROBE1000 PROBE1000
+
+int main(void)
+{
+	PROBE10000
+	PROBE10000
+	PROBE10000
+	PROBE10000
+	PROBE10000
+	return 0;
+}
-- 
2.40.0

[RFC/PATCH bpf-next 19/20] selftests/bpf: Add usdt_multi bench test

[Jiri Olsa]

Adding test that attaches 50k usdt probes in usdt_multi binary.

After the attach is done we run the binary and make sure we get
proper amount of hits.

With current uprobes:

  # perf stat --null ./test_progs -n 254/6
  #254/6   uprobe_multi_test/bench_usdt:OK
  #254     uprobe_multi_test:OK
  Summary: 1/1 PASSED, 0 SKIPPED, 0 FAILED

   Performance counter stats for './test_progs -n 254/6':

      1353.659680562 seconds time elapsed

With uprobe_multi link:

  # perf stat --null ./test_progs -n 254/6
  #254/6   uprobe_multi_test/bench_usdt:OK
  #254     uprobe_multi_test:OK
  Summary: 1/1 PASSED, 0 SKIPPED, 0 FAILED

   Performance counter stats for './test_progs -n 254/6':

         0.322046364 seconds time elapsed

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 .../bpf/prog_tests/uprobe_multi_test.c        | 54 +++++++++++++++++++
 .../selftests/bpf/progs/uprobe_multi_usdt.c   | 16 ++++++
 2 files changed, 70 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/uprobe_multi_usdt.c

diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
index 9a89c61359a1..ff08d4b8dbfe 100644
--- a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
@@ -3,6 +3,7 @@
 #include <unistd.h>
 #include <test_progs.h>
 #include "uprobe_multi.skel.h"
+#include "uprobe_multi_usdt.skel.h"
 
 noinline void uprobe_multi_func_1(void)
 {
@@ -217,6 +218,57 @@ void test_bench_attach_uprobe(void)
 	printf("%s: detached in %7.3lfs\n", __func__, detach_delta);
 }
 
+void test_bench_attach_usdt(void)
+{
+	struct uprobe_multi_usdt *skel = NULL;
+	long attach_start_ns, attach_end_ns;
+	long detach_start_ns, detach_end_ns;
+	double attach_delta, detach_delta;
+	LIBBPF_OPTS(bpf_usdt_opts, opts,
+		.uprobe_multi = true,
+	);
+	struct bpf_program *prog;
+	int err;
+
+	skel = uprobe_multi_usdt__open();
+	if (!ASSERT_OK_PTR(skel, "uprobe_multi__open"))
+		goto cleanup;
+
+	bpf_object__for_each_program(prog, skel->obj)
+		bpf_program__set_autoload(prog, false);
+
+	bpf_program__set_autoload(skel->progs.usdt0, true);
+	bpf_program__set_expected_attach_type(skel->progs.usdt0, BPF_TRACE_UPROBE_MULTI);
+
+	err = uprobe_multi_usdt__load(skel);
+	if (!ASSERT_EQ(err, 0, "strncmp_test load"))
+		goto cleanup;
+
+	attach_start_ns = get_time_ns();
+
+	skel->links.usdt0 = bpf_program__attach_usdt(skel->progs.usdt0, -1, "./usdt_multi",
+						     "test", "usdt", &opts);
+	if (!ASSERT_OK_PTR(skel->links.usdt0, "usdt_link"))
+		goto cleanup;
+
+	attach_end_ns = get_time_ns();
+
+	system("./usdt_multi");
+
+	ASSERT_EQ(skel->bss->count, 50000, "usdt_count");
+
+cleanup:
+	detach_start_ns = get_time_ns();
+	uprobe_multi_usdt__destroy(skel);
+	detach_end_ns = get_time_ns();
+
+	attach_delta = (attach_end_ns - attach_start_ns) / 1000000000.0;
+	detach_delta = (detach_end_ns - detach_start_ns) / 1000000000.0;
+
+	printf("%s: attached in %7.3lfs\n", __func__, attach_delta);
+	printf("%s: detached in %7.3lfs\n", __func__, detach_delta);
+}
+
 void test_uprobe_multi_test(void)
 {
 	if (test__start_subtest("skel_api"))
@@ -229,4 +281,6 @@ void test_uprobe_multi_test(void)
 		test_link_api();
 	if (test__start_subtest("bench_uprobe"))
 		test_bench_attach_uprobe();
+	if (test__start_subtest("bench_usdt"))
+		test_bench_attach_usdt();
 }
diff --git a/tools/testing/selftests/bpf/progs/uprobe_multi_usdt.c b/tools/testing/selftests/bpf/progs/uprobe_multi_usdt.c
new file mode 100644
index 000000000000..9e1c33d0bd2f
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/uprobe_multi_usdt.c
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/usdt.bpf.h>
+
+char _license[] SEC("license") = "GPL";
+
+int count;
+
+SEC("usdt")
+int usdt0(struct pt_regs *ctx)
+{
+	count++;
+	return 0;
+}
-- 
2.40.0

[RFC/PATCH bpf-next 20/20] selftests/bpf: Add uprobe_multi cookie test

[Jiri Olsa]

Adding test for cookies setup/retrieval in uprobe_link uprobes
and making sure bpf_get_attach_cookie works properly.

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

diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
index 26b2d1bffdfd..0e2d9ebe9258 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
@@ -11,6 +11,7 @@
 #include <bpf/btf.h>
 #include "test_bpf_cookie.skel.h"
 #include "kprobe_multi.skel.h"
+#include "uprobe_multi.skel.h"
 
 /* uprobe attach point */
 static noinline void trigger_func(void)
@@ -239,6 +240,81 @@ static void kprobe_multi_attach_api_subtest(void)
 	bpf_link__destroy(link1);
 	kprobe_multi__destroy(skel);
 }
+
+/* defined in prog_tests/uprobe_multi_test.c */
+void uprobe_multi_func_1(void);
+void uprobe_multi_func_2(void);
+void uprobe_multi_func_3(void);
+
+static void uprobe_multi_test_run(struct uprobe_multi *skel)
+{
+	skel->bss->uprobe_multi_func_1_addr = (u64) uprobe_multi_func_1;
+	skel->bss->uprobe_multi_func_2_addr = (u64) uprobe_multi_func_2;
+	skel->bss->uprobe_multi_func_3_addr = (u64) uprobe_multi_func_3;
+
+	skel->bss->pid = getpid();
+	skel->bss->test_cookie = true;
+
+	uprobe_multi_func_1();
+	uprobe_multi_func_2();
+	uprobe_multi_func_3();
+
+	ASSERT_EQ(skel->bss->uprobe_multi_func_1_result, 1, "uprobe_multi_func_1_result");
+	ASSERT_EQ(skel->bss->uprobe_multi_func_2_result, 1, "uprobe_multi_func_2_result");
+	ASSERT_EQ(skel->bss->uprobe_multi_func_3_result, 1, "uprobe_multi_func_3_result");
+
+	ASSERT_EQ(skel->bss->uretprobe_multi_func_1_result, 1, "uretprobe_multi_func_1_result");
+	ASSERT_EQ(skel->bss->uretprobe_multi_func_2_result, 1, "uretprobe_multi_func_2_result");
+	ASSERT_EQ(skel->bss->uretprobe_multi_func_3_result, 1, "uretprobe_multi_func_3_result");
+}
+
+static void uprobe_multi_attach_api_subtest(void)
+{
+	struct bpf_link *link1 = NULL, *link2 = NULL;
+	struct uprobe_multi *skel = NULL;
+	LIBBPF_OPTS(bpf_uprobe_multi_opts, opts);
+	const char *syms[3] = {
+		"uprobe_multi_func_1",
+		"uprobe_multi_func_2",
+		"uprobe_multi_func_3",
+	};
+	__u64 cookies[3];
+
+	cookies[0] = 3; /* uprobe_multi_func_1 */
+	cookies[1] = 1; /* uprobe_multi_func_2 */
+	cookies[2] = 2; /* uprobe_multi_func_3 */
+
+	opts.syms = syms;
+	opts.cnt = ARRAY_SIZE(syms);
+	opts.cookies = &cookies[0];
+
+	skel = uprobe_multi__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "uprobe_multi"))
+		goto cleanup;
+
+	link1 = bpf_program__attach_uprobe_multi_opts(skel->progs.test_uprobe,
+						      "/proc/self/exe", NULL, &opts);
+	if (!ASSERT_OK_PTR(link1, "bpf_program__attach_uprobe_multi_opts"))
+		goto cleanup;
+
+	cookies[0] = 2; /* uprobe_multi_func_1 */
+	cookies[1] = 3; /* uprobe_multi_func_2 */
+	cookies[2] = 1; /* uprobe_multi_func_3 */
+
+	opts.retprobe = true;
+	link2 = bpf_program__attach_uprobe_multi_opts(skel->progs.test_uretprobe,
+						      "/proc/self/exe", NULL, &opts);
+	if (!ASSERT_OK_PTR(link2, "bpf_program__attach_uprobe_multi_opts_retprobe"))
+		goto cleanup;
+
+	uprobe_multi_test_run(skel);
+
+cleanup:
+	bpf_link__destroy(link2);
+	bpf_link__destroy(link1);
+	uprobe_multi__destroy(skel);
+}
+
 static void uprobe_subtest(struct test_bpf_cookie *skel)
 {
 	DECLARE_LIBBPF_OPTS(bpf_uprobe_opts, opts);
@@ -515,6 +591,8 @@ void test_bpf_cookie(void)
 		kprobe_multi_attach_api_subtest();
 	if (test__start_subtest("uprobe"))
 		uprobe_subtest(skel);
+	if (test__start_subtest("multi_uprobe_attach_api"))
+		uprobe_multi_attach_api_subtest();
 	if (test__start_subtest("tracepoint"))
 		tp_subtest(skel);
 	if (test__start_subtest("perf_event"))
-- 
2.40.0

Re: [RFC/PATCH bpf-next 00/20] bpf: Add multi uprobe link

[Andrii Nakryiko]

On Mon, Apr 24, 2023 at 9:04 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> hi,
> this patchset is adding support to attach multiple uprobes and usdt probes
> through new uprobe_multi link.
>
> The current uprobe is attached through the perf event and attaching many
> uprobes takes a lot of time because of that.
>
> The main reason is that we need to install perf event for each probed function
> and profile shows perf event installation (perf_install_in_context) as culprit.
>
> The new uprobe_multi link just creates raw uprobes and attaches the bpf
> program to them without perf event being involved.
>
> In addition to being faster we also save file descriptors. For the current
> uprobe attach we use extra perf event fd for each probed function. The new
> link just need one fd that covers all the functions we are attaching to.

All of the above are good reasons and thanks for tackling multi-uprobe!

>
> By dropping perf we lose the ability to attach uprobe to specific pid.
> We can workaround that by having pid check directly in the bpf program,
> but we might need to check for another solution if that will turn out
> to be a problem.
>

I think this is a big deal, because it makes multi-uprobe not a
drop-in replacement for normal uprobes even for typical scenarios. It
might be why you couldn't do transparent use of uprobe.multi in USDT?

But I'm not sure why this is a problem? How does perf handle this?
Does it do runtime filtering or something more efficient that prevents
uprobe to be triggered for other PIDs in the first place? If it's the
former, then why can't we do the same simple check ourselves if pid
filter is specified?

I also see that uprobe_consumer has filter callback, not sure if it's
a better solution just for pid filtering, but might be another way to
do this?


Another aspect I wanted to discuss (and I don't know the right answer)
was whether we need to support separate binary path for each offset?
It would simplify (and trim down memory usage significantly) a bunch
of internals if we knew we are dealing with single inode for each
multi-uprobe link. I'm trying to think if it would be limiting in
practice to have to create link per each binary, and so far it seems
like usually user-space code will do symbol resolution per ELF file
anyways, so doesn't seem limiting to have single path + multiple
offsets/cookies within that file. For USDTs use case even ref_ctr is
probably the same, but I'd keep it 1:1 with offset and cookie anyways.
For uniformity and generality.

WDYT?

>
> Attaching current bpftrace to 1000 uprobes:
>
>   # BPFTRACE_MAX_PROBES=100000 perf stat -e cycles,instructions \
>     ./bpftrace -e 'uprobe:./uprobe_multi:uprobe_multi_func_* { }, i:ms:1 { exit(); }'
>     ...
>
>      126,666,390,509      cycles
>       29,973,207,307      instructions                     #    0.24  insn per cycle
>
>         85.284833554 seconds time elapsed
>
>
> Same bpftrace setup with uprobe_multi support:
>
>   # perf stat -e cycles,instructions \
>     ./bpftrace -e 'uprobe:./uprobe_multi:uprobe_multi_func_* { }, i:ms:1 { exit(); }'
>     ...
>
>        6,818,470,649      cycles
>       13,275,510,122      instructions                     #    1.95  insn per cycle
>
>          1.943269451 seconds time elapsed
>
>
> I'm sending this as RFC because of:
>   - I added/exported some new elf_* helper functions in libbpf,
>     and I'm not sure that's the best/right way of doing this

didn't get to that yet, sounds suspicious :)

>   - I'm not completely sure about the usdt integration in bpf_program__attach_usdt,
>     I was trying to detect uprobe_multi kernel support first, but ended up with
>     just new field for struct bpf_usdt_opts

haven't gotten to this yet as well, but it has to be auto-detectable,
not an option (at least I don't see why it wouldn't be, but let me get
to the patch)

>   - I plan to add more tests for usdt probes defined with refctr
>
>
> Also available at:
>   https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
>   uprobe_multi
>
> There are PRs for tetragon [1] and bpftrace [2] support.
>
> thanks,
> jirka
>
>
> [1] https://github.com/cilium/tetragon/pull/936
> [2] https://github.com/olsajiri/bpftrace/tree/uprobe_multi
>
> Cc: Viktor Malik <viktor.malik@gmail.com>
> Cc: Daniel Xu <dxu@dxuuu.xyz>
> ---
> Jiri Olsa (20):
>       bpf: Add multi uprobe link
>       bpf: Add cookies support for uprobe_multi link
>       bpf: Add bpf_get_func_ip helper support for uprobe link
>       libbpf: Update uapi bpf.h tools header
>       libbpf: Add uprobe_multi attach type and link names
>       libbpf: Factor elf_for_each_symbol function
>       libbpf: Add elf_find_multi_func_offset function
>       libbpf: Add elf_find_patern_func_offset function
>       libbpf: Add bpf_link_create support for multi uprobes
>       libbpf: Add bpf_program__attach_uprobe_multi_opts function
>       libbpf: Add support for uprobe.multi/uprobe.multi program sections
>       libbpf: Add uprobe multi link support to bpf_program__attach_usdt
>       selftests/bpf: Add uprobe_multi skel test
>       selftests/bpf: Add uprobe_multi api test
>       selftests/bpf: Add uprobe_multi link test
>       selftests/bpf: Add uprobe_multi test program
>       selftests/bpf: Add uprobe_multi bench test
>       selftests/bpf: Add usdt_multi test program
>       selftests/bpf: Add usdt_multi bench test
>       selftests/bpf: Add uprobe_multi cookie test
>
>  include/linux/trace_events.h                               |   6 +
>  include/uapi/linux/bpf.h                                   |  15 +++
>  kernel/bpf/syscall.c                                       |  18 ++-
>  kernel/trace/bpf_trace.c                                   | 310 +++++++++++++++++++++++++++++++++++++++++++-
>  tools/include/uapi/linux/bpf.h                             |  15 +++
>  tools/lib/bpf/bpf.c                                        |  10 ++
>  tools/lib/bpf/bpf.h                                        |  10 +-
>  tools/lib/bpf/libbpf.c                                     | 653 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------
>  tools/lib/bpf/libbpf.h                                     |  43 ++++++
>  tools/lib/bpf/libbpf.map                                   |   3 +
>  tools/lib/bpf/libbpf_internal.h                            |   2 +-
>  tools/lib/bpf/usdt.c                                       | 127 +++++++++++++-----
>  tools/testing/selftests/bpf/Makefile                       |  10 ++
>  tools/testing/selftests/bpf/prog_tests/bpf_cookie.c        |  78 +++++++++++
>  tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c | 286 ++++++++++++++++++++++++++++++++++++++++
>  tools/testing/selftests/bpf/progs/uprobe_multi.c           |  72 +++++++++++
>  tools/testing/selftests/bpf/progs/uprobe_multi_usdt.c      |  16 +++
>  tools/testing/selftests/bpf/uprobe_multi.c                 |  53 ++++++++
>  tools/testing/selftests/bpf/usdt_multi.c                   |  23 ++++
>  19 files changed, 1634 insertions(+), 116 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
>  create mode 100644 tools/testing/selftests/bpf/progs/uprobe_multi.c
>  create mode 100644 tools/testing/selftests/bpf/progs/uprobe_multi_usdt.c
>  create mode 100644 tools/testing/selftests/bpf/uprobe_multi.c
>  create mode 100644 tools/testing/selftests/bpf/usdt_multi.c

Re: [RFC/PATCH bpf-next 00/20] bpf: Add multi uprobe link

[Jiri Olsa]

On Wed, Apr 26, 2023 at 12:09:59PM -0700, Andrii Nakryiko wrote:
> On Mon, Apr 24, 2023 at 9:04 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > hi,
> > this patchset is adding support to attach multiple uprobes and usdt probes
> > through new uprobe_multi link.
> >
> > The current uprobe is attached through the perf event and attaching many
> > uprobes takes a lot of time because of that.
> >
> > The main reason is that we need to install perf event for each probed function
> > and profile shows perf event installation (perf_install_in_context) as culprit.
> >
> > The new uprobe_multi link just creates raw uprobes and attaches the bpf
> > program to them without perf event being involved.
> >
> > In addition to being faster we also save file descriptors. For the current
> > uprobe attach we use extra perf event fd for each probed function. The new
> > link just need one fd that covers all the functions we are attaching to.
> 
> All of the above are good reasons and thanks for tackling multi-uprobe!
> 
> >
> > By dropping perf we lose the ability to attach uprobe to specific pid.
> > We can workaround that by having pid check directly in the bpf program,
> > but we might need to check for another solution if that will turn out
> > to be a problem.
> >
> 
> I think this is a big deal, because it makes multi-uprobe not a
> drop-in replacement for normal uprobes even for typical scenarios. It
> might be why you couldn't do transparent use of uprobe.multi in USDT?

yes

> 
> But I'm not sure why this is a problem? How does perf handle this?
> Does it do runtime filtering or something more efficient that prevents
> uprobe to be triggered for other PIDs in the first place? If it's the
> former, then why can't we do the same simple check ourselves if pid
> filter is specified?

so the standard uprobe is basically a perf event and as such it can be
created with 'pid' as a target.. and such perf event will get installed
only when the process with that pid is scheduled in and uninstalled
when it's scheduled out

> 
> I also see that uprobe_consumer has filter callback, not sure if it's
> a better solution just for pid filtering, but might be another way to
> do this?

yes, that's probably how we will have to do that, will check

> 
> Another aspect I wanted to discuss (and I don't know the right answer)
> was whether we need to support separate binary path for each offset?
> It would simplify (and trim down memory usage significantly) a bunch
> of internals if we knew we are dealing with single inode for each
> multi-uprobe link. I'm trying to think if it would be limiting in
> practice to have to create link per each binary, and so far it seems
> like usually user-space code will do symbol resolution per ELF file
> anyways, so doesn't seem limiting to have single path + multiple
> offsets/cookies within that file. For USDTs use case even ref_ctr is
> probably the same, but I'd keep it 1:1 with offset and cookie anyways.
> For uniformity and generality.
> 
> WDYT?

right, it's waste for single binary, but I guess it's not a big waste,
because when you have single binary you just repeat the same pointer,
not the path

it's fast enough to be called multiple times for each binary you want
to trace, but it'd be also nice to be able to attach all in once ;-)

maybe we could have a bit in flags saying paths[0] is valid for all

> 
> >
> > Attaching current bpftrace to 1000 uprobes:
> >
> >   # BPFTRACE_MAX_PROBES=100000 perf stat -e cycles,instructions \
> >     ./bpftrace -e 'uprobe:./uprobe_multi:uprobe_multi_func_* { }, i:ms:1 { exit(); }'
> >     ...
> >
> >      126,666,390,509      cycles
> >       29,973,207,307      instructions                     #    0.24  insn per cycle
> >
> >         85.284833554 seconds time elapsed
> >
> >
> > Same bpftrace setup with uprobe_multi support:
> >
> >   # perf stat -e cycles,instructions \
> >     ./bpftrace -e 'uprobe:./uprobe_multi:uprobe_multi_func_* { }, i:ms:1 { exit(); }'
> >     ...
> >
> >        6,818,470,649      cycles
> >       13,275,510,122      instructions                     #    1.95  insn per cycle
> >
> >          1.943269451 seconds time elapsed
> >
> >
> > I'm sending this as RFC because of:
> >   - I added/exported some new elf_* helper functions in libbpf,
> >     and I'm not sure that's the best/right way of doing this
> 
> didn't get to that yet, sounds suspicious :)
> 
> >   - I'm not completely sure about the usdt integration in bpf_program__attach_usdt,
> >     I was trying to detect uprobe_multi kernel support first, but ended up with
> >     just new field for struct bpf_usdt_opts
> 
> haven't gotten to this yet as well, but it has to be auto-detectable,
> not an option (at least I don't see why it wouldn't be, but let me get
> to the patch)

thanks,
jirka

Re: [RFC/PATCH bpf-next 00/20] bpf: Add multi uprobe link

[Andrii Nakryiko]

On Thu, Apr 27, 2023 at 5:44 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Wed, Apr 26, 2023 at 12:09:59PM -0700, Andrii Nakryiko wrote:
> > On Mon, Apr 24, 2023 at 9:04 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > hi,
> > > this patchset is adding support to attach multiple uprobes and usdt probes
> > > through new uprobe_multi link.
> > >
> > > The current uprobe is attached through the perf event and attaching many
> > > uprobes takes a lot of time because of that.
> > >
> > > The main reason is that we need to install perf event for each probed function
> > > and profile shows perf event installation (perf_install_in_context) as culprit.
> > >
> > > The new uprobe_multi link just creates raw uprobes and attaches the bpf
> > > program to them without perf event being involved.
> > >
> > > In addition to being faster we also save file descriptors. For the current
> > > uprobe attach we use extra perf event fd for each probed function. The new
> > > link just need one fd that covers all the functions we are attaching to.
> >
> > All of the above are good reasons and thanks for tackling multi-uprobe!
> >
> > >
> > > By dropping perf we lose the ability to attach uprobe to specific pid.
> > > We can workaround that by having pid check directly in the bpf program,
> > > but we might need to check for another solution if that will turn out
> > > to be a problem.
> > >
> >
> > I think this is a big deal, because it makes multi-uprobe not a
> > drop-in replacement for normal uprobes even for typical scenarios. It
> > might be why you couldn't do transparent use of uprobe.multi in USDT?
>
> yes
>
> >
> > But I'm not sure why this is a problem? How does perf handle this?
> > Does it do runtime filtering or something more efficient that prevents
> > uprobe to be triggered for other PIDs in the first place? If it's the
> > former, then why can't we do the same simple check ourselves if pid
> > filter is specified?
>
> so the standard uprobe is basically a perf event and as such it can be
> created with 'pid' as a target.. and such perf event will get installed
> only when the process with that pid is scheduled in and uninstalled
> when it's scheduled out
>
> >
> > I also see that uprobe_consumer has filter callback, not sure if it's
> > a better solution just for pid filtering, but might be another way to
> > do this?
>
> yes, that's probably how we will have to do that, will check

callback seems like overkill as we'll be paying indirect call price.
So a simple if statement in either uprobe_prog_run or in
uprobe_multi_link_ret_handler/uprobe_multi_link_handler seems like
better solution, IMO.


>
> >
> > Another aspect I wanted to discuss (and I don't know the right answer)
> > was whether we need to support separate binary path for each offset?
> > It would simplify (and trim down memory usage significantly) a bunch
> > of internals if we knew we are dealing with single inode for each
> > multi-uprobe link. I'm trying to think if it would be limiting in
> > practice to have to create link per each binary, and so far it seems
> > like usually user-space code will do symbol resolution per ELF file
> > anyways, so doesn't seem limiting to have single path + multiple
> > offsets/cookies within that file. For USDTs use case even ref_ctr is
> > probably the same, but I'd keep it 1:1 with offset and cookie anyways.
> > For uniformity and generality.
> >
> > WDYT?
>
> right, it's waste for single binary, but I guess it's not a big waste,
> because when you have single binary you just repeat the same pointer,
> not the path
>
> it's fast enough to be called multiple times for each binary you want
> to trace, but it'd be also nice to be able to attach all in once ;-)
>
> maybe we could have a bit in flags saying paths[0] is valid for all

No need for extra flags. I was just thinking about having a simpler
and more straightforward API, where you don't need to create another
array with tons of duplicated string pointers. No big deal, I'm fine
either way.

>
> >
> > >
> > > Attaching current bpftrace to 1000 uprobes:
> > >
> > >   # BPFTRACE_MAX_PROBES=100000 perf stat -e cycles,instructions \
> > >     ./bpftrace -e 'uprobe:./uprobe_multi:uprobe_multi_func_* { }, i:ms:1 { exit(); }'
> > >     ...
> > >
> > >      126,666,390,509      cycles
> > >       29,973,207,307      instructions                     #    0.24  insn per cycle
> > >
> > >         85.284833554 seconds time elapsed
> > >
> > >
> > > Same bpftrace setup with uprobe_multi support:
> > >
> > >   # perf stat -e cycles,instructions \
> > >     ./bpftrace -e 'uprobe:./uprobe_multi:uprobe_multi_func_* { }, i:ms:1 { exit(); }'
> > >     ...
> > >
> > >        6,818,470,649      cycles
> > >       13,275,510,122      instructions                     #    1.95  insn per cycle
> > >
> > >          1.943269451 seconds time elapsed
> > >
> > >
> > > I'm sending this as RFC because of:
> > >   - I added/exported some new elf_* helper functions in libbpf,
> > >     and I'm not sure that's the best/right way of doing this
> >
> > didn't get to that yet, sounds suspicious :)
> >
> > >   - I'm not completely sure about the usdt integration in bpf_program__attach_usdt,
> > >     I was trying to detect uprobe_multi kernel support first, but ended up with
> > >     just new field for struct bpf_usdt_opts
> >
> > haven't gotten to this yet as well, but it has to be auto-detectable,
> > not an option (at least I don't see why it wouldn't be, but let me get
> > to the patch)
>
> thanks,
> jirka

Re: [RFC/PATCH bpf-next 00/20] bpf: Add multi uprobe link

[Jiri Olsa]

On Thu, Apr 27, 2023 at 03:24:25PM -0700, Andrii Nakryiko wrote:
> On Thu, Apr 27, 2023 at 5:44 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Wed, Apr 26, 2023 at 12:09:59PM -0700, Andrii Nakryiko wrote:
> > > On Mon, Apr 24, 2023 at 9:04 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > >
> > > > hi,
> > > > this patchset is adding support to attach multiple uprobes and usdt probes
> > > > through new uprobe_multi link.
> > > >
> > > > The current uprobe is attached through the perf event and attaching many
> > > > uprobes takes a lot of time because of that.
> > > >
> > > > The main reason is that we need to install perf event for each probed function
> > > > and profile shows perf event installation (perf_install_in_context) as culprit.
> > > >
> > > > The new uprobe_multi link just creates raw uprobes and attaches the bpf
> > > > program to them without perf event being involved.
> > > >
> > > > In addition to being faster we also save file descriptors. For the current
> > > > uprobe attach we use extra perf event fd for each probed function. The new
> > > > link just need one fd that covers all the functions we are attaching to.
> > >
> > > All of the above are good reasons and thanks for tackling multi-uprobe!
> > >
> > > >
> > > > By dropping perf we lose the ability to attach uprobe to specific pid.
> > > > We can workaround that by having pid check directly in the bpf program,
> > > > but we might need to check for another solution if that will turn out
> > > > to be a problem.
> > > >
> > >
> > > I think this is a big deal, because it makes multi-uprobe not a
> > > drop-in replacement for normal uprobes even for typical scenarios. It
> > > might be why you couldn't do transparent use of uprobe.multi in USDT?
> >
> > yes
> >
> > >
> > > But I'm not sure why this is a problem? How does perf handle this?
> > > Does it do runtime filtering or something more efficient that prevents
> > > uprobe to be triggered for other PIDs in the first place? If it's the
> > > former, then why can't we do the same simple check ourselves if pid
> > > filter is specified?
> >
> > so the standard uprobe is basically a perf event and as such it can be
> > created with 'pid' as a target.. and such perf event will get installed
> > only when the process with that pid is scheduled in and uninstalled
> > when it's scheduled out
> >
> > >
> > > I also see that uprobe_consumer has filter callback, not sure if it's
> > > a better solution just for pid filtering, but might be another way to
> > > do this?
> >
> > yes, that's probably how we will have to do that, will check
> 
> callback seems like overkill as we'll be paying indirect call price.
> So a simple if statement in either uprobe_prog_run or in
> uprobe_multi_link_ret_handler/uprobe_multi_link_handler seems like
> better solution, IMO.

it looks like the consumer->filter is checked/executed before installing
the breakpoint for uprobe, so it could be actually faster than current
uprobe pid filter.. I'll check and have it there in next version

> 
> 
> >
> > >
> > > Another aspect I wanted to discuss (and I don't know the right answer)
> > > was whether we need to support separate binary path for each offset?
> > > It would simplify (and trim down memory usage significantly) a bunch
> > > of internals if we knew we are dealing with single inode for each
> > > multi-uprobe link. I'm trying to think if it would be limiting in
> > > practice to have to create link per each binary, and so far it seems
> > > like usually user-space code will do symbol resolution per ELF file
> > > anyways, so doesn't seem limiting to have single path + multiple
> > > offsets/cookies within that file. For USDTs use case even ref_ctr is
> > > probably the same, but I'd keep it 1:1 with offset and cookie anyways.
> > > For uniformity and generality.
> > >
> > > WDYT?
> >
> > right, it's waste for single binary, but I guess it's not a big waste,
> > because when you have single binary you just repeat the same pointer,
> > not the path
> >
> > it's fast enough to be called multiple times for each binary you want
> > to trace, but it'd be also nice to be able to attach all in once ;-)
> >
> > maybe we could have a bit in flags saying paths[0] is valid for all
> 
> No need for extra flags. I was just thinking about having a simpler
> and more straightforward API, where you don't need to create another
> array with tons of duplicated string pointers. No big deal, I'm fine
> either way.

ok

thanks,
jirka

Re: [RFC/PATCH bpf-next 00/20] bpf: Add multi uprobe link

[Andrii Nakryiko]

On Fri, Apr 28, 2023 at 3:55 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Thu, Apr 27, 2023 at 03:24:25PM -0700, Andrii Nakryiko wrote:
> > On Thu, Apr 27, 2023 at 5:44 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> > >
> > > On Wed, Apr 26, 2023 at 12:09:59PM -0700, Andrii Nakryiko wrote:
> > > > On Mon, Apr 24, 2023 at 9:04 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > > >
> > > > > hi,
> > > > > this patchset is adding support to attach multiple uprobes and usdt probes
> > > > > through new uprobe_multi link.
> > > > >
> > > > > The current uprobe is attached through the perf event and attaching many
> > > > > uprobes takes a lot of time because of that.
> > > > >
> > > > > The main reason is that we need to install perf event for each probed function
> > > > > and profile shows perf event installation (perf_install_in_context) as culprit.
> > > > >
> > > > > The new uprobe_multi link just creates raw uprobes and attaches the bpf
> > > > > program to them without perf event being involved.
> > > > >
> > > > > In addition to being faster we also save file descriptors. For the current
> > > > > uprobe attach we use extra perf event fd for each probed function. The new
> > > > > link just need one fd that covers all the functions we are attaching to.
> > > >
> > > > All of the above are good reasons and thanks for tackling multi-uprobe!
> > > >
> > > > >
> > > > > By dropping perf we lose the ability to attach uprobe to specific pid.
> > > > > We can workaround that by having pid check directly in the bpf program,
> > > > > but we might need to check for another solution if that will turn out
> > > > > to be a problem.
> > > > >
> > > >
> > > > I think this is a big deal, because it makes multi-uprobe not a
> > > > drop-in replacement for normal uprobes even for typical scenarios. It
> > > > might be why you couldn't do transparent use of uprobe.multi in USDT?
> > >
> > > yes
> > >
> > > >
> > > > But I'm not sure why this is a problem? How does perf handle this?
> > > > Does it do runtime filtering or something more efficient that prevents
> > > > uprobe to be triggered for other PIDs in the first place? If it's the
> > > > former, then why can't we do the same simple check ourselves if pid
> > > > filter is specified?
> > >
> > > so the standard uprobe is basically a perf event and as such it can be
> > > created with 'pid' as a target.. and such perf event will get installed
> > > only when the process with that pid is scheduled in and uninstalled
> > > when it's scheduled out
> > >
> > > >
> > > > I also see that uprobe_consumer has filter callback, not sure if it's
> > > > a better solution just for pid filtering, but might be another way to
> > > > do this?
> > >
> > > yes, that's probably how we will have to do that, will check
> >
> > callback seems like overkill as we'll be paying indirect call price.
> > So a simple if statement in either uprobe_prog_run or in
> > uprobe_multi_link_ret_handler/uprobe_multi_link_handler seems like
> > better solution, IMO.
>
> it looks like the consumer->filter is checked/executed before installing
> the breakpoint for uprobe, so it could be actually faster than current
> uprobe pid filter.. I'll check and have it there in next version

ah, so if it's not executed on each uprobe run, then yeah, that would be best

>
> >
> >
> > >
> > > >
> > > > Another aspect I wanted to discuss (and I don't know the right answer)
> > > > was whether we need to support separate binary path for each offset?
> > > > It would simplify (and trim down memory usage significantly) a bunch
> > > > of internals if we knew we are dealing with single inode for each
> > > > multi-uprobe link. I'm trying to think if it would be limiting in
> > > > practice to have to create link per each binary, and so far it seems
> > > > like usually user-space code will do symbol resolution per ELF file
> > > > anyways, so doesn't seem limiting to have single path + multiple
> > > > offsets/cookies within that file. For USDTs use case even ref_ctr is
> > > > probably the same, but I'd keep it 1:1 with offset and cookie anyways.
> > > > For uniformity and generality.
> > > >
> > > > WDYT?
> > >
> > > right, it's waste for single binary, but I guess it's not a big waste,
> > > because when you have single binary you just repeat the same pointer,
> > > not the path
> > >
> > > it's fast enough to be called multiple times for each binary you want
> > > to trace, but it'd be also nice to be able to attach all in once ;-)
> > >
> > > maybe we could have a bit in flags saying paths[0] is valid for all
> >
> > No need for extra flags. I was just thinking about having a simpler
> > and more straightforward API, where you don't need to create another
> > array with tons of duplicated string pointers. No big deal, I'm fine
> > either way.
>
> ok
>
> thanks,
> jirka