bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC bpf-next 0/4] libbpf: userspace attach by name
@ 2022-01-12 16:18 Alan Maguire
  2022-01-12 16:18 ` [RFC bpf-next 1/4] libbpf: support function name-based attach for uprobes Alan Maguire
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Alan Maguire @ 2022-01-12 16:18 UTC (permalink / raw)
  To: ast, daniel, andrii
  Cc: kafai, songliubraving, yhs, john.fastabend, kpsingh, jolsa,
	sunyucong, netdev, bpf, Alan Maguire

This patch series is a rough attempt to support attach by name for
uprobes and USDT (Userland Static Defined Tracing) probes.
Currently attach for such probes is done by determining the offset
manually, so the aim is to try and mimic the simplicity of kprobe
attach, making use of uprobe opts.

One restriction applies: uprobe attach supports system-wide probing
by specifying "-1" for the pid.  That functionality is not supported,
since we need a running process to determine the base address to
subtract to get the uprobe-friendly offset.  There may be a way
to do this without a running process, so any suggestions would
be greatly appreciated.

There are probably a bunch of subtleties missing here; the aim
is to see if this is useful and if so hopefully we can refine
it to deal with more complex cases.  I tried to handle one case
that came to mind - weak library symbols - but there are probably
other issues when determining which address to use I haven't
thought of.

Alan Maguire (4):
  libbpf: support function name-based attach for uprobes
  libbpf: support usdt provider/probe name-based attach for uprobes
  selftests/bpf: add tests for u[ret]probe attach by name
  selftests/bpf: add test for USDT uprobe attach by name

 tools/lib/bpf/libbpf.c                             | 244 +++++++++++++++++++++
 tools/lib/bpf/libbpf.h                             |  17 +-
 tools/testing/selftests/bpf/Makefile               |  34 +++
 .../selftests/bpf/prog_tests/attach_probe.c        |  74 ++++++-
 .../selftests/bpf/progs/test_attach_probe.c        |  24 ++
 5 files changed, 391 insertions(+), 2 deletions(-)

-- 
1.8.3.1


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

* [RFC bpf-next 1/4] libbpf: support function name-based attach for uprobes
  2022-01-12 16:18 [RFC bpf-next 0/4] libbpf: userspace attach by name Alan Maguire
@ 2022-01-12 16:18 ` Alan Maguire
  2022-01-12 16:18 ` [RFC bpf-next 2/4] libbpf: support usdt provider/probe " Alan Maguire
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Alan Maguire @ 2022-01-12 16:18 UTC (permalink / raw)
  To: ast, daniel, andrii
  Cc: kafai, songliubraving, yhs, john.fastabend, kpsingh, jolsa,
	sunyucong, netdev, bpf, Alan Maguire

kprobe attach is name-based, using lookups of kallsyms to translate
a function name to an address.  Currently uprobe attach is done
via an offset value as described in [1].  Extend uprobe opts
for attach to include a function name which can then be converted
into a uprobe-friendly offset.  The calcualation is done in two
steps:

- first, determine the symbol address using libelf; this gives us
  the offset as reported by objdump; then
- subtract the base address associated with the object.

The resultant value is then added to the func_offset value passed
in to specify the uprobe attach address.  So specifying a func_offset
of 0 along with a function name "printf" will attach to printf entry.

[1] https://www.kernel.org/doc/html/latest/trace/uprobetracer.html

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 tools/lib/bpf/libbpf.c | 172 +++++++++++++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h |  10 ++-
 2 files changed, 181 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index cf862a1..bccc26a 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10155,6 +10155,126 @@ static int perf_event_uprobe_open_legacy(const char *probe_name, bool retprobe,
 	return pfd;
 }
 
+/* uprobes deal in relative offsets; subtract the base address associated with
+ * the mapped binary.  See Documentation/trace/uprobetracer.rst for more
+ * details.
+ */
+static ssize_t get_rel_offset(pid_t pid, uintptr_t addr)
+{
+	size_t start, end, offset;
+	char msg[STRERR_BUFSIZE];
+	char maps[64];
+	char buf[256];
+	FILE *f;
+	int err;
+
+	/* pid 0 implies "this process" */
+	snprintf(maps, sizeof(maps), "/proc/%d/maps", pid ? pid : getpid());
+	f = fopen(maps, "r");
+	if (!f) {
+		err = -errno;
+		pr_warn("could not open %s: %s\n",
+			maps, libbpf_strerror_r(err, msg, sizeof(msg)));
+		return err;
+	}
+
+	while (fscanf(f, "%zx-%zx %s %zx %*[^\n]\n", &start, &end, buf, &offset) == 4) {
+		if (addr >= start && addr < end) {
+			fclose(f);
+			return (size_t)addr - start + offset;
+		}
+	}
+	fclose(f);
+	return -ENOENT;
+}
+
+/* find next ELF section of sh_type, returning fd and setting elfp and scnp to
+ * point at Elf and next Elf_Scn.
+ */
+static Elf_Scn *find_elfscn(Elf *elf, int sh_type, Elf_Scn *scn)
+{
+	Elf64_Shdr *sh;
+
+	while ((scn = elf_nextscn(elf, scn)) != NULL) {
+		sh = elf64_getshdr(scn);
+		if (sh && sh->sh_type == sh_type)
+			break;
+	}
+	return scn;
+}
+
+/* Find offset of function name in object specified by path.  "name" matches
+ * symbol name or name@@LIB for library functions.
+ */
+static ssize_t find_elf_func_offset(Elf *elf, const char *name)
+{
+	size_t si, strtabidx, nr_syms;
+	Elf_Data *symbols = NULL;
+	ssize_t ret = -ENOENT;
+	Elf_Scn *scn = NULL;
+	const char *sname;
+	Elf64_Shdr *sh;
+	int bind;
+
+	scn = find_elfscn(elf, SHT_SYMTAB, NULL);
+	if (!scn) {
+		pr_debug("elf: failed to find symbol table ELF section\n");
+		return -ENOENT;
+	}
+
+	sh = elf64_getshdr(scn);
+	strtabidx = sh->sh_link;
+	symbols = elf_getdata(scn, 0);
+	if (!symbols) {
+		pr_debug("elf: failed to get symtab section: %s\n", elf_errmsg(-1));
+		return -LIBBPF_ERRNO__FORMAT;
+	}
+
+	nr_syms = symbols->d_size / sizeof(Elf64_Sym);
+	for (si = 0; si < nr_syms; si++) {
+		Elf64_Sym *sym = (Elf64_Sym *)symbols->d_buf + si;
+		size_t matchlen;
+		int currbind;
+
+		if (ELF64_ST_TYPE(sym->st_info) != STT_FUNC)
+			continue;
+
+		sname = elf_strptr(elf, strtabidx, sym->st_name);
+		if (!sname) {
+			pr_debug("failed to get sym name string for var %s\n", name);
+			return -EIO;
+		}
+		currbind = ELF64_ST_BIND(sym->st_info);
+
+		/* If matching on func@@LIB, match on everything prior to
+		 * the '@@'; otherwise match on full string.
+		 */
+		matchlen = strstr(sname, "@@") ? strstr(sname, "@@") - sname :
+						 strlen(sname);
+
+		if (strlen(name) == matchlen &&
+		    strncmp(sname, name, matchlen) == 0) {
+			if (ret >= 0) {
+				/* handle multiple matches */
+				if (bind != STB_WEAK && currbind != STB_WEAK) {
+					/* Only accept one non-weak bind. */
+					pr_debug("got additional match for symbol %s: %s\n",
+						 sname, name);
+					return -LIBBPF_ERRNO__FORMAT;
+				} else if (currbind == STB_WEAK) {
+					/* already have a non-weak bind, and
+					 * this is a weak bind, so ignore.
+					 */
+					continue;
+				}
+			}
+			ret = sym->st_value;
+			bind = currbind;
+		}
+	}
+	return ret;
+}
+
 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,
@@ -10166,6 +10286,7 @@ static int perf_event_uprobe_open_legacy(const char *probe_name, bool retprobe,
 	size_t ref_ctr_off;
 	int pfd, err;
 	bool retprobe, legacy;
+	const char *func_name;
 
 	if (!OPTS_VALID(opts, bpf_uprobe_opts))
 		return libbpf_err_ptr(-EINVAL);
@@ -10174,6 +10295,57 @@ static int perf_event_uprobe_open_legacy(const char *probe_name, bool retprobe,
 	ref_ctr_off = OPTS_GET(opts, ref_ctr_offset, 0);
 	pe_opts.bpf_cookie = OPTS_GET(opts, bpf_cookie, 0);
 
+	func_name = OPTS_GET(opts, func_name, NULL);
+	if (func_name) {
+		ssize_t sym_off, rel_off;
+		Elf *elf;
+		int fd;
+
+		if (pid == -1) {
+			/* system-wide probing is not supported; we need
+			 * a running process to determine offsets.
+			 */
+			pr_warn("name-based attach does not work for pid -1 (all processes)\n");
+			return libbpf_err_ptr(-EINVAL);
+		}
+		if (!binary_path) {
+			pr_warn("name-based attach requires binary_path\n");
+			return libbpf_err_ptr(-EINVAL);
+		}
+		if (elf_version(EV_CURRENT) == EV_NONE) {
+			pr_debug("failed to init libelf for %s\n", binary_path);
+			return libbpf_err_ptr(-LIBBPF_ERRNO__LIBELF);
+		}
+		fd = open(binary_path, O_RDONLY | O_CLOEXEC);
+		if (fd < 0) {
+			err = -errno;
+			pr_debug("failed to open %s: %s\n", binary_path,
+				 libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
+			return libbpf_err_ptr(err);
+		}
+		elf = elf_begin(fd, ELF_C_READ_MMAP, NULL);
+		if (!elf) {
+			pr_debug("could not read elf from %s: %s\n",
+				 binary_path, elf_errmsg(-1));
+			close(fd);
+			return libbpf_err_ptr(-LIBBPF_ERRNO__FORMAT);
+		}
+		sym_off = find_elf_func_offset(elf, func_name);
+		close(fd);
+		elf_end(elf);
+		if (sym_off < 0) {
+			pr_debug("could not find sym offset for %s\n", func_name);
+			return libbpf_err_ptr(sym_off);
+		}
+		rel_off = get_rel_offset(pid, sym_off);
+		if (rel_off < 0) {
+			pr_debug("could not find relative offset for %s at 0x%lx\n",
+				 func_name, sym_off);
+			return libbpf_err_ptr(rel_off);
+		}
+		func_offset += (size_t)rel_off;
+	}
+
 	legacy = determine_uprobe_perf_type() < 0;
 	if (!legacy) {
 		pfd = perf_event_open_probe(true /* uprobe */, retprobe, binary_path,
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 85dfef8..40cb5ae 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -431,9 +431,17 @@ struct bpf_uprobe_opts {
 	__u64 bpf_cookie;
 	/* uprobe is return probe, invoked at function return time */
 	bool retprobe;
+	/* name of function name or function@@LIBRARY.  Partial matches
+	 * work for library name, such as printf, printf@@GLIBC.
+	 * To specify function entry, func_offset argument should be 0 and
+	 * func_name should specify function to trace.  To trace an offset
+	 * within the function, specify func_name and use func_offset
+	 * argument to specify argument _within_ the function.
+	 */
+	const char *func_name;
 	size_t :0;
 };
-#define bpf_uprobe_opts__last_field retprobe
+#define bpf_uprobe_opts__last_field func_name
 
 /**
  * @brief **bpf_program__attach_uprobe()** attaches a BPF program
-- 
1.8.3.1


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

* [RFC bpf-next 2/4] libbpf: support usdt provider/probe name-based attach for uprobes
  2022-01-12 16:18 [RFC bpf-next 0/4] libbpf: userspace attach by name Alan Maguire
  2022-01-12 16:18 ` [RFC bpf-next 1/4] libbpf: support function name-based attach for uprobes Alan Maguire
@ 2022-01-12 16:18 ` Alan Maguire
  2022-01-12 16:18 ` [RFC bpf-next 3/4] selftests/bpf: add tests for u[ret]probe attach by name Alan Maguire
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Alan Maguire @ 2022-01-12 16:18 UTC (permalink / raw)
  To: ast, daniel, andrii
  Cc: kafai, songliubraving, yhs, john.fastabend, kpsingh, jolsa,
	sunyucong, netdev, bpf, Alan Maguire

Add support for name-based attach to Userland Static-Defined Tracing
(USDT) probes via lookup of ELF notes associated with probe definition.
ELF notes are consulted for probe offset, and - if "is-enabled" style
of probing is in use - semaphore offset.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 tools/lib/bpf/libbpf.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++----
 tools/lib/bpf/libbpf.h |  9 +++++-
 2 files changed, 86 insertions(+), 7 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index bccc26a..cdcd799 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10275,6 +10275,52 @@ static ssize_t find_elf_func_offset(Elf *elf, const char *name)
 	return ret;
 }
 
+struct sdt_note {
+	uint64_t	pc;
+	uint64_t	base_addr;
+	uint64_t	semaphore;
+	const char	provider_probe[256];
+};
+
+/* Find offset of USDT probe in object specified in ELF notes.
+ * Note may also specify semaphore offset, record value in *semaphore_offp.
+ */
+static ssize_t find_elf_usdt_offset(Elf *elf, const char *provider,
+				    const char *name, ssize_t *semaphore_offp)
+{
+	Elf_Data *data = NULL;
+	Elf_Scn *scn = NULL;
+
+	while ((scn = find_elfscn(elf, SHT_NOTE, scn)) > 0) {
+		while ((data = elf_getdata(scn, data)) != 0) {
+			size_t name_off, desc_off, off;
+			GElf_Nhdr nhdr;
+
+			while ((off = gelf_getnote(data, off, &nhdr,
+						   &name_off, &desc_off)) != 0) {
+				struct sdt_note *sdt_note;
+				const char *probe;
+
+				if (nhdr.n_namesz != 8 ||
+				    memcmp((char *)data->d_buf + name_off, "stapsdt", 8) != 0)
+					continue;
+				sdt_note = (struct sdt_note *)(data->d_buf + desc_off);
+				if (strcmp(provider, sdt_note->provider_probe) != 0)
+					continue;
+				/* probe is after null-terminated provider */
+				probe = sdt_note->provider_probe +
+					strlen(sdt_note->provider_probe) + 1;
+				if (strcmp(probe, name) != 0)
+					continue;
+
+				*semaphore_offp = (ssize_t)sdt_note->semaphore;
+				return (ssize_t)sdt_note->pc;
+			}
+		}
+	}
+	return -ENOENT;
+}
+
 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,
@@ -10286,7 +10332,7 @@ static ssize_t find_elf_func_offset(Elf *elf, const char *name)
 	size_t ref_ctr_off;
 	int pfd, err;
 	bool retprobe, legacy;
-	const char *func_name;
+	const char *func_name, *usdt_name, *usdt_provider;
 
 	if (!OPTS_VALID(opts, bpf_uprobe_opts))
 		return libbpf_err_ptr(-EINVAL);
@@ -10296,11 +10342,25 @@ static ssize_t find_elf_func_offset(Elf *elf, const char *name)
 	pe_opts.bpf_cookie = OPTS_GET(opts, bpf_cookie, 0);
 
 	func_name = OPTS_GET(opts, func_name, NULL);
-	if (func_name) {
-		ssize_t sym_off, rel_off;
+	usdt_provider = OPTS_GET(opts, usdt_provider, NULL);
+	usdt_name = OPTS_GET(opts, usdt_name, NULL);
+	if (func_name || usdt_name) {
+		ssize_t sym_off, rel_off, semaphore_off = 0;
 		Elf *elf;
 		int fd;
 
+		if (func_name && usdt_name) {
+			pr_warn("both func_name and usdt_name cannot be specified\n");
+			return libbpf_err_ptr(-EINVAL);
+		}
+		if (usdt_name && (func_offset || ref_ctr_off)) {
+			pr_warn("func_offset argument, ref_ctr_off option should be 0 when usdt_name is used\n");
+			return libbpf_err_ptr(-EINVAL);
+		}
+		if (usdt_name && !usdt_provider) {
+			pr_warn("usdt_provider and usdt_name must be supplied\n");
+			return libbpf_err_ptr(-EINVAL);
+		}
 		if (pid == -1) {
 			/* system-wide probing is not supported; we need
 			 * a running process to determine offsets.
@@ -10330,20 +10390,32 @@ static ssize_t find_elf_func_offset(Elf *elf, const char *name)
 			close(fd);
 			return libbpf_err_ptr(-LIBBPF_ERRNO__FORMAT);
 		}
-		sym_off = find_elf_func_offset(elf, func_name);
+		if (func_name)
+			sym_off = find_elf_func_offset(elf, func_name);
+		else
+			sym_off = find_elf_usdt_offset(elf, usdt_provider, usdt_name,
+						       &semaphore_off);
 		close(fd);
 		elf_end(elf);
 		if (sym_off < 0) {
-			pr_debug("could not find sym offset for %s\n", func_name);
+			pr_debug("could not find sym offset for %s\n", func_name ?: usdt_name);
 			return libbpf_err_ptr(sym_off);
 		}
 		rel_off = get_rel_offset(pid, sym_off);
 		if (rel_off < 0) {
 			pr_debug("could not find relative offset for %s at 0x%lx\n",
-				 func_name, sym_off);
+				 func_name ?: usdt_name, sym_off);
 			return libbpf_err_ptr(rel_off);
 		}
 		func_offset += (size_t)rel_off;
+		if (semaphore_off) {
+			ref_ctr_off = get_rel_offset(pid, semaphore_off);
+			if (ref_ctr_off < 0) {
+				pr_debug("could not find relative offset for semaphore for %s at 0x%lx\n",
+					 usdt_name, semaphore_off);
+				return libbpf_err_ptr(ref_ctr_off);
+			}
+		}
 	}
 
 	legacy = determine_uprobe_perf_type() < 0;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 40cb5ae..fcad6b1 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -439,9 +439,16 @@ struct bpf_uprobe_opts {
 	 * argument to specify argument _within_ the function.
 	 */
 	const char *func_name;
+	/* name of USDT (Userland Static-Defined Tracing) provider/probe.
+	 * Offsets of USDT probe and associated semaphore (if any) are found
+	 * in ELF notes.  Note that if usdt_name is specified, func_offset
+	 * argument and ref_ctr_offset values should be zero.
+	 */
+	const char *usdt_provider;
+	const char *usdt_name;
 	size_t :0;
 };
-#define bpf_uprobe_opts__last_field func_name
+#define bpf_uprobe_opts__last_field usdt_name
 
 /**
  * @brief **bpf_program__attach_uprobe()** attaches a BPF program
-- 
1.8.3.1


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

* [RFC bpf-next 3/4] selftests/bpf: add tests for u[ret]probe attach by name
  2022-01-12 16:18 [RFC bpf-next 0/4] libbpf: userspace attach by name Alan Maguire
  2022-01-12 16:18 ` [RFC bpf-next 1/4] libbpf: support function name-based attach for uprobes Alan Maguire
  2022-01-12 16:18 ` [RFC bpf-next 2/4] libbpf: support usdt provider/probe " Alan Maguire
@ 2022-01-12 16:18 ` Alan Maguire
  2022-01-12 16:18 ` [RFC bpf-next 4/4] selftests/bpf: add test for USDT uprobe " Alan Maguire
  2022-01-12 17:04 ` [RFC bpf-next 0/4] libbpf: userspace " Andrii Nakryiko
  4 siblings, 0 replies; 10+ messages in thread
From: Alan Maguire @ 2022-01-12 16:18 UTC (permalink / raw)
  To: ast, daniel, andrii
  Cc: kafai, songliubraving, yhs, john.fastabend, kpsingh, jolsa,
	sunyucong, netdev, bpf, Alan Maguire

add tests that verify attaching by name for a local and library
function succeed for uprobe and uretprobe using new "func_name"
option for bpf_program__attach_uprobe_opts().

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 .../selftests/bpf/prog_tests/attach_probe.c        | 41 +++++++++++++++++++++-
 .../selftests/bpf/progs/test_attach_probe.c        | 16 +++++++++
 2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
index d0bd51e..521d7bd 100644
--- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c
+++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
@@ -10,12 +10,18 @@ static void method(void) {
 	return ;
 }
 
+/* attach point for byname uprobe */
+static void method2(void) {
+	return;
+}
+
 void test_attach_probe(void)
 {
 	DECLARE_LIBBPF_OPTS(bpf_uprobe_opts, uprobe_opts);
 	int duration = 0;
 	struct bpf_link *kprobe_link, *kretprobe_link;
 	struct bpf_link *uprobe_link, *uretprobe_link;
+	struct bpf_link *uprobe_byname_link, *uretprobe_byname_link;
 	struct test_attach_probe* skel;
 	size_t uprobe_offset;
 	ssize_t base_addr, ref_ctr_offset;
@@ -92,7 +98,30 @@ void test_attach_probe(void)
 		goto cleanup;
 	skel->links.handle_uretprobe = uretprobe_link;
 
-	/* trigger & validate kprobe && kretprobe */
+	uprobe_opts.func_name = "method2";
+	uprobe_opts.retprobe = false;
+	uprobe_opts.ref_ctr_offset = 0;
+	uprobe_byname_link = bpf_program__attach_uprobe_opts(skel->progs.handle_uprobe_byname,
+							     0 /* this pid */,
+							     "/proc/self/exe",
+							     0, &uprobe_opts);
+	if (!ASSERT_OK_PTR(uprobe_byname_link, "attach_uprobe_byname"))
+		goto cleanup;
+	skel->links.handle_uprobe_byname = uprobe_byname_link;
+
+	/* test attach by name for a library function */
+	uprobe_opts.func_name = "usleep";
+	uprobe_opts.retprobe = true;
+	uprobe_opts.ref_ctr_offset = 0;
+	uretprobe_byname_link = bpf_program__attach_uprobe_opts(skel->progs.handle_uretprobe_byname,
+								0 /* this pid */,
+								"/proc/self/exe",
+								0, &uprobe_opts);
+	if (!ASSERT_OK_PTR(uretprobe_byname_link, "attach_uretprobe_byname"))
+		goto cleanup;
+	skel->links.handle_uretprobe_byname = uretprobe_byname_link;
+
+	/* trigger & validate kprobe && kretprobe && uretprobe by name */
 	usleep(1);
 
 	if (CHECK(skel->bss->kprobe_res != 1, "check_kprobe_res",
@@ -105,6 +134,9 @@ void test_attach_probe(void)
 	/* trigger & validate uprobe & uretprobe */
 	method();
 
+	/* trigger & validate uprobe attached by name */
+	method2();
+
 	if (CHECK(skel->bss->uprobe_res != 3, "check_uprobe_res",
 		  "wrong uprobe res: %d\n", skel->bss->uprobe_res))
 		goto cleanup;
@@ -112,6 +144,13 @@ void test_attach_probe(void)
 		  "wrong uretprobe res: %d\n", skel->bss->uretprobe_res))
 		goto cleanup;
 
+	if (CHECK(skel->bss->uprobe_byname_res != 5, "check_uprobe_byname_res",
+		  "wrong uprobe byname res: %d\n", skel->bss->uprobe_byname_res))
+		goto cleanup;
+	if (CHECK(skel->bss->uretprobe_byname_res != 6, "check_uretprobe_byname_res",
+		  "wrong uretprobe byname res: %d\n", skel->bss->uretprobe_byname_res))
+		goto cleanup;
+
 cleanup:
 	test_attach_probe__destroy(skel);
 	ASSERT_EQ(uprobe_ref_ctr, 0, "uprobe_ref_ctr_cleanup");
diff --git a/tools/testing/selftests/bpf/progs/test_attach_probe.c b/tools/testing/selftests/bpf/progs/test_attach_probe.c
index 8056a4c..efa56bd 100644
--- a/tools/testing/selftests/bpf/progs/test_attach_probe.c
+++ b/tools/testing/selftests/bpf/progs/test_attach_probe.c
@@ -10,6 +10,8 @@
 int kretprobe_res = 0;
 int uprobe_res = 0;
 int uretprobe_res = 0;
+int uprobe_byname_res = 0;
+int uretprobe_byname_res = 0;
 
 SEC("kprobe/sys_nanosleep")
 int handle_kprobe(struct pt_regs *ctx)
@@ -39,4 +41,18 @@ int handle_uretprobe(struct pt_regs *ctx)
 	return 0;
 }
 
+SEC("uprobe/trigger_func_byname")
+int handle_uprobe_byname(struct pt_regs *ctx)
+{
+	uprobe_byname_res = 5;
+	return 0;
+}
+
+SEC("uretprobe/trigger_func_byname")
+int handle_uretprobe_byname(struct pt_regs *ctx)
+{
+	uretprobe_byname_res = 6;
+	return 0;
+}
+
 char _license[] SEC("license") = "GPL";
-- 
1.8.3.1


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

* [RFC bpf-next 4/4] selftests/bpf: add test for USDT uprobe attach by name
  2022-01-12 16:18 [RFC bpf-next 0/4] libbpf: userspace attach by name Alan Maguire
                   ` (2 preceding siblings ...)
  2022-01-12 16:18 ` [RFC bpf-next 3/4] selftests/bpf: add tests for u[ret]probe attach by name Alan Maguire
@ 2022-01-12 16:18 ` Alan Maguire
  2022-01-12 17:04 ` [RFC bpf-next 0/4] libbpf: userspace " Andrii Nakryiko
  4 siblings, 0 replies; 10+ messages in thread
From: Alan Maguire @ 2022-01-12 16:18 UTC (permalink / raw)
  To: ast, daniel, andrii
  Cc: kafai, songliubraving, yhs, john.fastabend, kpsingh, jolsa,
	sunyucong, netdev, bpf, Alan Maguire

add test to verify attaching to USDT probe via specification of
provider/name succeeds and probe fires.

Depends on presence of <sys/sdt.h> as verified via feature-sdt

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 tools/testing/selftests/bpf/Makefile               | 34 ++++++++++++++++++++++
 .../selftests/bpf/prog_tests/attach_probe.c        | 33 +++++++++++++++++++++
 .../selftests/bpf/progs/test_attach_probe.c        |  8 +++++
 3 files changed, 75 insertions(+)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 42ffc24..3a7e3f2 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -3,6 +3,14 @@ include ../../../build/Build.include
 include ../../../scripts/Makefile.arch
 include ../../../scripts/Makefile.include
 
+# needed for Makile.feature
+ifeq ($(srctree),)
+srctree := $(patsubst %/,%,$(dir $(CURDIR)))
+srctree := $(patsubst %/,%,$(dir $(srctree)))
+srctree := $(patsubst %/,%,$(dir $(srctree)))
+srctree := $(patsubst %/,%,$(dir $(srctree)))
+endif
+
 CXX ?= $(CROSS_COMPILE)g++
 
 CURDIR := $(abspath .)
@@ -32,6 +40,26 @@ ifneq ($(LLVM),)
 CFLAGS += -Wno-unused-command-line-argument
 endif
 
+FEATURE_USER = .bpftest
+FEATURE_TESTS = sdt
+FEATURE_DISPLAY = sdt
+
+check_feat := 1
+NON_CHECK_FEAT_TARGETS := clean docs docs-clean
+ifdef MAKECMDGOALS
+ifeq ($(filter-out $(NON_CHECK_FEAT_TARGETS),$(MAKECMDGOALS)),)
+  check_feat := 0
+endif
+endif
+
+ifeq ($(check_feat),1)
+ifeq ($(FEATURES_DUMP),)
+include $(srctree)/tools/build/Makefile.feature
+else
+include $(FEATURES_DUMP)
+endif
+endif
+
 # Order correspond to 'make run_tests' order
 TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \
 	test_verifier_log test_dev_cgroup \
@@ -108,6 +136,7 @@ override define CLEAN
 	$(Q)$(RM) -r $(TEST_GEN_PROGS)
 	$(Q)$(RM) -r $(TEST_GEN_PROGS_EXTENDED)
 	$(Q)$(RM) -r $(TEST_GEN_FILES)
+	$(Q)$(RM) -r $(OUTPUT)FEATURE-DUMP.bpf
 	$(Q)$(RM) -r $(EXTRA_CLEAN)
 	$(Q)$(MAKE) -C bpf_testmod clean
 	$(Q)$(MAKE) docs-clean
@@ -434,6 +463,11 @@ $(TRUNNER_TESTS_HDR): $(TRUNNER_TESTS_DIR)/*.c
 		 ) > $$@)
 endif
 
+# support for adding USDT probes?
+ifeq ($(feature-sdt), 1)
+CFLAGS += -DHAVE_SDT_EVENT
+endif
+
 # compile individual test files
 # Note: we cd into output directory to ensure embedded BPF object is found
 $(TRUNNER_TEST_OBJS): $(TRUNNER_OUTPUT)/%.test.o:			\
diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
index 521d7bd..5bb24927 100644
--- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c
+++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
@@ -2,6 +2,17 @@
 #include <test_progs.h>
 #include "test_attach_probe.skel.h"
 
+#if defined(HAVE_SDT_EVENT)
+#include <sys/sdt.h>
+
+static void usdt_method(void)
+{
+	DTRACE_PROBE(bpftest, probe1);
+	return;
+}
+
+#endif /* HAVE_SDT_EVENT */
+
 /* this is how USDT semaphore is actually defined, except volatile modifier */
 volatile unsigned short uprobe_ref_ctr __attribute__((unused)) __attribute((section(".probes")));
 
@@ -22,6 +33,7 @@ void test_attach_probe(void)
 	struct bpf_link *kprobe_link, *kretprobe_link;
 	struct bpf_link *uprobe_link, *uretprobe_link;
 	struct bpf_link *uprobe_byname_link, *uretprobe_byname_link;
+	struct bpf_link *usdtprobe_byname_link;
 	struct test_attach_probe* skel;
 	size_t uprobe_offset;
 	ssize_t base_addr, ref_ctr_offset;
@@ -121,6 +133,27 @@ void test_attach_probe(void)
 		goto cleanup;
 	skel->links.handle_uretprobe_byname = uretprobe_byname_link;
 
+#if defined(HAVE_SDT_EVENT)
+	uprobe_opts.usdt_provider = "bpftest";
+	uprobe_opts.usdt_name = "probe1";
+	uprobe_opts.func_name = NULL;
+	uprobe_opts.retprobe = false;
+	usdtprobe_byname_link = bpf_program__attach_uprobe_opts(skel->progs.handle_usdtprobe_byname,
+								0 /* this pid */,
+								"/proc/self/exe",
+								0, &uprobe_opts);
+	if (!ASSERT_OK_PTR(usdtprobe_byname_link, "attach_usdtprobe_byname"))
+		goto cleanup;
+	skel->links.handle_usdtprobe_byname = usdtprobe_byname_link;
+
+	/* trigger and validate usdt probe */
+	usdt_method();
+
+	if (CHECK(skel->bss->usdtprobe_byname_res != 7, "check_usdtprobe_byname_res",
+		  "wrong usdtprobe_byname res: %d\n", skel->bss->usdtprobe_byname_res))
+		goto cleanup;
+#endif /* HAVE_SDT_EVENT */
+
 	/* trigger & validate kprobe && kretprobe && uretprobe by name */
 	usleep(1);
 
diff --git a/tools/testing/selftests/bpf/progs/test_attach_probe.c b/tools/testing/selftests/bpf/progs/test_attach_probe.c
index efa56bd..0c93191 100644
--- a/tools/testing/selftests/bpf/progs/test_attach_probe.c
+++ b/tools/testing/selftests/bpf/progs/test_attach_probe.c
@@ -12,6 +12,7 @@
 int uretprobe_res = 0;
 int uprobe_byname_res = 0;
 int uretprobe_byname_res = 0;
+int usdtprobe_byname_res = 0;
 
 SEC("kprobe/sys_nanosleep")
 int handle_kprobe(struct pt_regs *ctx)
@@ -55,4 +56,11 @@ int handle_uretprobe_byname(struct pt_regs *ctx)
 	return 0;
 }
 
+SEC("uprobe/trigger_usdt_byname")
+int handle_usdtprobe_byname(struct pt_regs *ctx)
+{
+	usdtprobe_byname_res = 7;
+	return 0;
+}
+
 char _license[] SEC("license") = "GPL";
-- 
1.8.3.1


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

* Re: [RFC bpf-next 0/4] libbpf: userspace attach by name
  2022-01-12 16:18 [RFC bpf-next 0/4] libbpf: userspace attach by name Alan Maguire
                   ` (3 preceding siblings ...)
  2022-01-12 16:18 ` [RFC bpf-next 4/4] selftests/bpf: add test for USDT uprobe " Alan Maguire
@ 2022-01-12 17:04 ` Andrii Nakryiko
  2022-01-13 10:29   ` Alan Maguire
  4 siblings, 1 reply; 10+ messages in thread
From: Andrii Nakryiko @ 2022-01-12 17:04 UTC (permalink / raw)
  To: Alan Maguire
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
	Song Liu, Yonghong Song, john fastabend, KP Singh, Jiri Olsa,
	Yucong Sun, Networking, bpf

On Wed, Jan 12, 2022 at 8:19 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> This patch series is a rough attempt to support attach by name for
> uprobes and USDT (Userland Static Defined Tracing) probes.
> Currently attach for such probes is done by determining the offset
> manually, so the aim is to try and mimic the simplicity of kprobe
> attach, making use of uprobe opts.
>
> One restriction applies: uprobe attach supports system-wide probing
> by specifying "-1" for the pid.  That functionality is not supported,
> since we need a running process to determine the base address to
> subtract to get the uprobe-friendly offset.  There may be a way
> to do this without a running process, so any suggestions would
> be greatly appreciated.
>
> There are probably a bunch of subtleties missing here; the aim
> is to see if this is useful and if so hopefully we can refine
> it to deal with more complex cases.  I tried to handle one case
> that came to mind - weak library symbols - but there are probably
> other issues when determining which address to use I haven't
> thought of.
>
> Alan Maguire (4):
>   libbpf: support function name-based attach for uprobes
>   libbpf: support usdt provider/probe name-based attach for uprobes
>   selftests/bpf: add tests for u[ret]probe attach by name
>   selftests/bpf: add test for USDT uprobe attach by name
>

Hey Alan,

I've been working on USDT support last year. It's considerably more
code than in this RFC, but it handles not just finding a location of
USDT probe(s), but also fetching its arguments based on argument
location specification and more usability focused BPF-side APIs to
work with USDTs.

I don't remember how up to date it is, but the last "open source"
version of it can be found at [0]. I currently have the latest
debugged and tested version internally in the process of being
integrated into our profiling solution here at Meta. So far it seems
to be working fine and covers our production use cases well.

The plan is to open source it as a separate companion library to
libbpf some time in the next few months. Hopefully that would work for
you. Once it is available, I hope we can also utilize it to convert
some more BCC-based tools (that rely on USDT) to libbpf ([1]).

  [0] https://github.com/anakryiko/linux/commit/d473d042c8058da0a9e6c0353d97aeaf574925c6
  [1] https://github.com/iovisor/bcc/tree/master/libbpf-tools

>  tools/lib/bpf/libbpf.c                             | 244 +++++++++++++++++++++
>  tools/lib/bpf/libbpf.h                             |  17 +-
>  tools/testing/selftests/bpf/Makefile               |  34 +++
>  .../selftests/bpf/prog_tests/attach_probe.c        |  74 ++++++-
>  .../selftests/bpf/progs/test_attach_probe.c        |  24 ++
>  5 files changed, 391 insertions(+), 2 deletions(-)
>
> --
> 1.8.3.1
>

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

* Re: [RFC bpf-next 0/4] libbpf: userspace attach by name
  2022-01-12 17:04 ` [RFC bpf-next 0/4] libbpf: userspace " Andrii Nakryiko
@ 2022-01-13 10:29   ` Alan Maguire
  2022-01-14 20:48     ` Andrii Nakryiko
  0 siblings, 1 reply; 10+ messages in thread
From: Alan Maguire @ 2022-01-13 10:29 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Alan Maguire, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin Lau, Song Liu, Yonghong Song,
	john fastabend, KP Singh, Jiri Olsa, Yucong Sun, Networking, bpf

On Wed, 12 Jan 2022, Andrii Nakryiko wrote:

> On Wed, Jan 12, 2022 at 8:19 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> >
> > This patch series is a rough attempt to support attach by name for
> > uprobes and USDT (Userland Static Defined Tracing) probes.
> > Currently attach for such probes is done by determining the offset
> > manually, so the aim is to try and mimic the simplicity of kprobe
> > attach, making use of uprobe opts.
> >
> > One restriction applies: uprobe attach supports system-wide probing
> > by specifying "-1" for the pid.  That functionality is not supported,
> > since we need a running process to determine the base address to
> > subtract to get the uprobe-friendly offset.  There may be a way
> > to do this without a running process, so any suggestions would
> > be greatly appreciated.
> >
> > There are probably a bunch of subtleties missing here; the aim
> > is to see if this is useful and if so hopefully we can refine
> > it to deal with more complex cases.  I tried to handle one case
> > that came to mind - weak library symbols - but there are probably
> > other issues when determining which address to use I haven't
> > thought of.
> >
> > Alan Maguire (4):
> >   libbpf: support function name-based attach for uprobes
> >   libbpf: support usdt provider/probe name-based attach for uprobes
> >   selftests/bpf: add tests for u[ret]probe attach by name
> >   selftests/bpf: add test for USDT uprobe attach by name
> >
> 
> Hey Alan,
> 
> I've been working on USDT support last year. It's considerably more
> code than in this RFC, but it handles not just finding a location of
> USDT probe(s), but also fetching its arguments based on argument
> location specification and more usability focused BPF-side APIs to
> work with USDTs.
> 
> I don't remember how up to date it is, but the last "open source"
> version of it can be found at [0]. I currently have the latest
> debugged and tested version internally in the process of being
> integrated into our profiling solution here at Meta. So far it seems
> to be working fine and covers our production use cases well.
> 

This looks great Andrii! I really like the argument access work, and the
global tracing part is solved too by using the ELF segment info instead
of the process maps to get the relative offset, with (I think?) use of
BPF cookies to disambiguate between different user attachments.

The one piece that seems to be missing from my perspective - and this may 
be in more recent versions - is uprobe function attachment by name. Most of 
the work is  already done in libusdt so it's reasonably doable I think - at a 
minimum  it would require an equivalent to the find_elf_func_offset() 
function in my  patch 1. Now the name of the library libusdt suggests its 
focus is on USDT of course, but I think having userspace function attach 
by name too would be great. Is that part of your plans for this work?

Thanks!

Alan

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

* Re: [RFC bpf-next 0/4] libbpf: userspace attach by name
  2022-01-13 10:29   ` Alan Maguire
@ 2022-01-14 20:48     ` Andrii Nakryiko
  2022-01-19 14:03       ` Alan Maguire
  0 siblings, 1 reply; 10+ messages in thread
From: Andrii Nakryiko @ 2022-01-14 20:48 UTC (permalink / raw)
  To: Alan Maguire
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
	Song Liu, Yonghong Song, john fastabend, KP Singh, Jiri Olsa,
	Yucong Sun, Networking, bpf

On Thu, Jan 13, 2022 at 2:30 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On Wed, 12 Jan 2022, Andrii Nakryiko wrote:
>
> > On Wed, Jan 12, 2022 at 8:19 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> > >
> > > This patch series is a rough attempt to support attach by name for
> > > uprobes and USDT (Userland Static Defined Tracing) probes.
> > > Currently attach for such probes is done by determining the offset
> > > manually, so the aim is to try and mimic the simplicity of kprobe
> > > attach, making use of uprobe opts.
> > >
> > > One restriction applies: uprobe attach supports system-wide probing
> > > by specifying "-1" for the pid.  That functionality is not supported,
> > > since we need a running process to determine the base address to
> > > subtract to get the uprobe-friendly offset.  There may be a way
> > > to do this without a running process, so any suggestions would
> > > be greatly appreciated.
> > >
> > > There are probably a bunch of subtleties missing here; the aim
> > > is to see if this is useful and if so hopefully we can refine
> > > it to deal with more complex cases.  I tried to handle one case
> > > that came to mind - weak library symbols - but there are probably
> > > other issues when determining which address to use I haven't
> > > thought of.
> > >
> > > Alan Maguire (4):
> > >   libbpf: support function name-based attach for uprobes
> > >   libbpf: support usdt provider/probe name-based attach for uprobes
> > >   selftests/bpf: add tests for u[ret]probe attach by name
> > >   selftests/bpf: add test for USDT uprobe attach by name
> > >
> >
> > Hey Alan,
> >
> > I've been working on USDT support last year. It's considerably more
> > code than in this RFC, but it handles not just finding a location of
> > USDT probe(s), but also fetching its arguments based on argument
> > location specification and more usability focused BPF-side APIs to
> > work with USDTs.
> >
> > I don't remember how up to date it is, but the last "open source"
> > version of it can be found at [0]. I currently have the latest
> > debugged and tested version internally in the process of being
> > integrated into our profiling solution here at Meta. So far it seems
> > to be working fine and covers our production use cases well.
> >
>
> This looks great Andrii! I really like the argument access work, and the
> global tracing part is solved too by using the ELF segment info instead
> of the process maps to get the relative offset, with (I think?) use of
> BPF cookies to disambiguate between different user attachments.

BPF cookies are mandatory for when attaching to a shared library *and*
NOT specifying PID. This is actually the mode that BCC doesn't seem to
support. In all other cases BPF cookie shouldn't be mandatory.

>
> The one piece that seems to be missing from my perspective - and this may
> be in more recent versions - is uprobe function attachment by name. Most of
> the work is  already done in libusdt so it's reasonably doable I think - at a
> minimum  it would require an equivalent to the find_elf_func_offset()
> function in my  patch 1. Now the name of the library libusdt suggests its
> focus is on USDT of course, but I think having userspace function attach
> by name too would be great. Is that part of your plans for this work?

True, uprobes don't supprot attaching by function name, which is quite
annoying. It's certainly not a focus for libusdt (or whatever it will
end up being called when open-sources). But if it's not much code and
complexity we should probably just add that to libbpf directly for
uprobes.


>
> Thanks!
>
> Alan

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

* Re: [RFC bpf-next 0/4] libbpf: userspace attach by name
  2022-01-14 20:48     ` Andrii Nakryiko
@ 2022-01-19 14:03       ` Alan Maguire
  2022-01-19 18:49         ` Andrii Nakryiko
  0 siblings, 1 reply; 10+ messages in thread
From: Alan Maguire @ 2022-01-19 14:03 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Alan Maguire, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin Lau, Song Liu, Yonghong Song,
	john fastabend, KP Singh, Jiri Olsa, Yucong Sun, Networking, bpf

On Fri, 14 Jan 2022, Andrii Nakryiko wrote:

> > The one piece that seems to be missing from my perspective - and this may
> > be in more recent versions - is uprobe function attachment by name. Most of
> > the work is  already done in libusdt so it's reasonably doable I think - at a
> > minimum  it would require an equivalent to the find_elf_func_offset()
> > function in my  patch 1. Now the name of the library libusdt suggests its
> > focus is on USDT of course, but I think having userspace function attach
> > by name too would be great. Is that part of your plans for this work?
> 
> True, uprobes don't supprot attaching by function name, which is quite
> annoying. It's certainly not a focus for libusdt (or whatever it will
> end up being called when open-sources). But if it's not much code and
> complexity we should probably just add that to libbpf directly for
> uprobes.
>

I've been looking at this, and I've got the following cases working:

- local symbols in a binary. This involves symbol table lookup and 
  relative offset calcuation.
- shared object symbols in a shared object.  In this case, the symbol 
  table values suffice, no adjustment needed.

The former works using the program headers (instead of /proc/pid/maps for
offset computation), so can be run for all processes, lifting the 
limitation in the RFC which only supported name lookup for a specific 
process. Around a hundred lines for this makes it worthwhile I think.

There is one more case, which is a shared library function in a binary -
where I specify "malloc" as the function and /usr/bin/foo as the binary
path.  In this case, for dynamic symbols we can't just look up the symbol 
table in the binary, since the associated values are 0.  Ideally it would 
be nice if the user could just specify "malloc" and not need to use libc 
as the binary path argument, but getting this working is proving to be 
trickier. I've tried making use of PLT section information but no luck 
yet (the idea being we try to use the trampoline address of malloc@@PLT
instead, but I'm still trying to figure out how to extract that).

So I'm wondering if we just fail lookup for that case, assuming the user 
will specify the shared library path if they want to trace a shared library 
function. What do you think? Thanks!

Alan

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

* Re: [RFC bpf-next 0/4] libbpf: userspace attach by name
  2022-01-19 14:03       ` Alan Maguire
@ 2022-01-19 18:49         ` Andrii Nakryiko
  0 siblings, 0 replies; 10+ messages in thread
From: Andrii Nakryiko @ 2022-01-19 18:49 UTC (permalink / raw)
  To: Alan Maguire
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
	Song Liu, Yonghong Song, john fastabend, KP Singh, Jiri Olsa,
	Yucong Sun, Networking, bpf

On Wed, Jan 19, 2022 at 6:04 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On Fri, 14 Jan 2022, Andrii Nakryiko wrote:
>
> > > The one piece that seems to be missing from my perspective - and this may
> > > be in more recent versions - is uprobe function attachment by name. Most of
> > > the work is  already done in libusdt so it's reasonably doable I think - at a
> > > minimum  it would require an equivalent to the find_elf_func_offset()
> > > function in my  patch 1. Now the name of the library libusdt suggests its
> > > focus is on USDT of course, but I think having userspace function attach
> > > by name too would be great. Is that part of your plans for this work?
> >
> > True, uprobes don't supprot attaching by function name, which is quite
> > annoying. It's certainly not a focus for libusdt (or whatever it will
> > end up being called when open-sources). But if it's not much code and
> > complexity we should probably just add that to libbpf directly for
> > uprobes.
> >
>
> I've been looking at this, and I've got the following cases working:
>
> - local symbols in a binary. This involves symbol table lookup and
>   relative offset calcuation.
> - shared object symbols in a shared object.  In this case, the symbol
>   table values suffice, no adjustment needed.
>
> The former works using the program headers (instead of /proc/pid/maps for
> offset computation), so can be run for all processes, lifting the
> limitation in the RFC which only supported name lookup for a specific
> process. Around a hundred lines for this makes it worthwhile I think.
>
> There is one more case, which is a shared library function in a binary -
> where I specify "malloc" as the function and /usr/bin/foo as the binary
> path.  In this case, for dynamic symbols we can't just look up the symbol
> table in the binary, since the associated values are 0.  Ideally it would
> be nice if the user could just specify "malloc" and not need to use libc
> as the binary path argument, but getting this working is proving to be
> trickier. I've tried making use of PLT section information but no luck
> yet (the idea being we try to use the trampoline address of malloc@@PLT
> instead, but I'm still trying to figure out how to extract that).
>
> So I'm wondering if we just fail lookup for that case, assuming the user
> will specify the shared library path if they want to trace a shared library
> function. What do you think? Thanks!

I think it all makes sense (but let's see the code as well ;) ). For
the latter, can you please double-check what sort of functionality BCC
provides? Also make sure that you support specifying absolute address
instead of function name as well (func+0x123 probably as well, just
like for kprobes?).

The annoying bit is libbpf's convention to use '/' as a separator in
SEC() definitions. I think bpftrace/dtrace's ':' makes more sense, but
it seems to disruptive to switch it now. Because of this, specifying
absolute path to the binary would look weird:

SEC("uprobe//usr/bin/bash/readline")

or something like that would consistent with current convention, but
super weird.

Did you run into this issue during your experiments?

I can see two improvements, more and less radical (short of switching
from / to : completely):

1. less radical is to use "custom" format for uprobe after the "uprobe/" part:

SEC("uprobe//usr/bin/bash:readline")

2. a bit more radical (but probably better long term) is to support
'/' and ':' interchangeably (but only one of them in any given SEC()
definition).  For existing definitions, we can say that both forms are
supported now:

SEC("kprobe/some_func") and SEC("kprobe:some_func")

For uprobe I'd probably combine #1 and #2 and say that these two forms
are supported:

SEC("uprobe//usr/bin/bash:readline") (so function separator is always ':')

and

SEC("uprobe:/usr/bin/bash:readline") (nicer and more consistent).


Thoughts?

BTW, as much as I like consistency, the proposal to switch to ':'
exclusively in libbpf 1.0 is a no-go, IMO, it's too much of a
disruption for tons of users.


>
> Alan

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

end of thread, other threads:[~2022-01-19 18:49 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-12 16:18 [RFC bpf-next 0/4] libbpf: userspace attach by name Alan Maguire
2022-01-12 16:18 ` [RFC bpf-next 1/4] libbpf: support function name-based attach for uprobes Alan Maguire
2022-01-12 16:18 ` [RFC bpf-next 2/4] libbpf: support usdt provider/probe " Alan Maguire
2022-01-12 16:18 ` [RFC bpf-next 3/4] selftests/bpf: add tests for u[ret]probe attach by name Alan Maguire
2022-01-12 16:18 ` [RFC bpf-next 4/4] selftests/bpf: add test for USDT uprobe " Alan Maguire
2022-01-12 17:04 ` [RFC bpf-next 0/4] libbpf: userspace " Andrii Nakryiko
2022-01-13 10:29   ` Alan Maguire
2022-01-14 20:48     ` Andrii Nakryiko
2022-01-19 14:03       ` Alan Maguire
2022-01-19 18:49         ` Andrii Nakryiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).