[PATCH v4 bpf-next 0/9] libbpf: add bpf_link and tracing attach APIs

[PATCH v4 bpf-next 0/9] libbpf: add bpf_link and tracing attach APIs

[Andrii Nakryiko]

This patchset adds the following APIs to allow attaching BPF programs to
tracing entities:
- bpf_program__attach_perf_event for attaching to any opened perf event FD,
  allowing users full control;
- bpf_program__attach_kprobe for attaching to kernel probes (both entry and
  return probes);
- bpf_program__attach_uprobe for attaching to user probes (both entry/return);
- bpf_program__attach_tracepoint for attaching to kernel tracepoints;
- bpf_program__attach_raw_tracepoint for attaching to raw kernel tracepoint
  (wrapper around bpf_raw_tracepoint_open);

This set of APIs makes libbpf more useful for tracing applications.

All attach APIs return abstract struct bpf_link that encapsulates logic of
detaching BPF program. See patch #2 for details. bpf_assoc was considered as
an alternative name for this opaque "handle", but bpf_link seems to be
appropriate semantically and is nice and short.

Pre-patch #1 makes internal libbpf_strerror_r helper function work w/ negative
error codes, lifting the burder off callers to keep track of error sign.
Patch #2 adds bpf_link abstraction.
Patch #3 adds attach_perf_event, which is the base for all other APIs.
Patch #4 adds kprobe/uprobe APIs.
Patch #5 adds tracepoint API.
Patch #6 adds raw_tracepoint API.
Patch #7 converts one existing test to use attach_perf_event.
Patch #8 adds new kprobe/uprobe tests.
Patch #9 converts some selftests currently using tracepoint to new APIs.

v3->v4:
- proper errno handling (Stanislav);
- bpf_fd -> prog_fd (Stanislav);
- switch to fprintf (Song);
v2->v3:
- added bpf_link concept (Daniel);
- didn't add generic bpf_link__attach_program for reasons described in [0];
- dropped Stanislav's Reviewed-by from patches #2-#6, in case he doesn't like
  the change;
v1->v2:
- preserve errno before close() call (Stanislav);
- use libbpf_perf_event_disable_and_close in selftest (Stanislav);
- remove unnecessary memset (Stanislav);

[0] https://lore.kernel.org/bpf/CAEf4BzZ7EM5eP2eaZn7T2Yb5QgVRiwAs+epeLR1g01TTx-6m6Q@mail.gmail.com/

Andrii Nakryiko (9):
  libbpf: make libbpf_strerror_r agnostic to sign of error
  libbpf: introduce concept of bpf_link
  libbpf: add ability to attach/detach BPF program to perf event
  libbpf: add kprobe/uprobe attach API
  libbpf: add tracepoint attach API
  libbpf: add raw tracepoint attach API
  selftests/bpf: switch test to new attach_perf_event API
  selftests/bpf: add kprobe/uprobe selftests
  selftests/bpf: convert existing tracepoint tests to new APIs

 tools/lib/bpf/libbpf.c                        | 359 ++++++++++++++++++
 tools/lib/bpf/libbpf.h                        |  21 +
 tools/lib/bpf/libbpf.map                      |   8 +-
 tools/lib/bpf/str_error.c                     |   2 +-
 .../selftests/bpf/prog_tests/attach_probe.c   | 155 ++++++++
 .../bpf/prog_tests/stacktrace_build_id.c      |  50 +--
 .../bpf/prog_tests/stacktrace_build_id_nmi.c  |  31 +-
 .../selftests/bpf/prog_tests/stacktrace_map.c |  43 +--
 .../bpf/prog_tests/stacktrace_map_raw_tp.c    |  15 +-
 .../selftests/bpf/progs/test_attach_probe.c   |  55 +++
 10 files changed, 644 insertions(+), 95 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/attach_probe.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_attach_probe.c

-- 
2.17.1

[PATCH v4 bpf-next 1/9] libbpf: make libbpf_strerror_r agnostic to sign of error

[Andrii Nakryiko]

It's often inconvenient to switch sign of error when passing it into
libbpf_strerror_r. It's better for it to handle that automatically.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Reviewed-by: Stanislav Fomichev <sdf@google.com>
Acked-by: Song Liu <songliubraving@fb.com>
---
 tools/lib/bpf/str_error.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/bpf/str_error.c b/tools/lib/bpf/str_error.c
index 00e48ac5b806..b8064eedc177 100644
--- a/tools/lib/bpf/str_error.c
+++ b/tools/lib/bpf/str_error.c
@@ -11,7 +11,7 @@
  */
 char *libbpf_strerror_r(int err, char *dst, int len)
 {
-	int ret = strerror_r(err, dst, len);
+	int ret = strerror_r(err < 0 ? -err : err, dst, len);
 	if (ret)
 		snprintf(dst, len, "ERROR: strerror_r(%d)=%d", err, ret);
 	return dst;
-- 
2.17.1

[PATCH v4 bpf-next 2/9] libbpf: introduce concept of bpf_link

[Andrii Nakryiko]

bpf_link is and abstraction of an association of a BPF program and one
of many possible BPF attachment points (hooks). This allows to have
uniform interface for detaching BPF programs regardless of the nature of
link and how it was created. Details of creation and setting up of
a specific bpf_link is handled by corresponding attachment methods
(bpf_program__attach_xxx) added in subsequent commits. Once successfully
created, bpf_link has to be eventually destroyed with
bpf_link__destroy(), at which point BPF program is disassociated from
a hook and all the relevant resources are freed.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>
---
 tools/lib/bpf/libbpf.c   | 17 +++++++++++++++++
 tools/lib/bpf/libbpf.h   |  4 ++++
 tools/lib/bpf/libbpf.map |  3 ++-
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 6e6ebef11ba3..455795e6f8af 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -3941,6 +3941,23 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
 	return 0;
 }
 
+struct bpf_link {
+	int (*destroy)(struct bpf_link *link);
+};
+
+int bpf_link__destroy(struct bpf_link *link)
+{
+	int err;
+
+	if (!link)
+		return 0;
+
+	err = link->destroy(link);
+	free(link);
+
+	return err;
+}
+
 enum bpf_perf_event_ret
 bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
 			   void **copy_mem, size_t *copy_size,
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index d639f47e3110..5082a5ebb0c2 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -165,6 +165,10 @@ LIBBPF_API int bpf_program__pin(struct bpf_program *prog, const char *path);
 LIBBPF_API int bpf_program__unpin(struct bpf_program *prog, const char *path);
 LIBBPF_API void bpf_program__unload(struct bpf_program *prog);
 
+struct bpf_link;
+
+LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
+
 struct bpf_insn;
 
 /*
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 2c6d835620d2..3cde850fc8da 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -167,10 +167,11 @@ LIBBPF_0.0.3 {
 
 LIBBPF_0.0.4 {
 	global:
+		bpf_link__destroy;
+		bpf_object__load_xattr;
 		btf_dump__dump_type;
 		btf_dump__free;
 		btf_dump__new;
 		btf__parse_elf;
-		bpf_object__load_xattr;
 		libbpf_num_possible_cpus;
 } LIBBPF_0.0.3;
-- 
2.17.1

[PATCH v4 bpf-next 3/9] libbpf: add ability to attach/detach BPF program to perf event

[Andrii Nakryiko]

bpf_program__attach_perf_event allows to attach BPF program to existing
perf event hook, providing most generic and most low-level way to attach BPF
programs. It returns struct bpf_link, which should be passed to
bpf_link__destroy to detach and free resources, associated with a link.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/libbpf.c   | 61 ++++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h   |  3 ++
 tools/lib/bpf/libbpf.map |  1 +
 3 files changed, 65 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 455795e6f8af..98c155ec3bfa 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -32,6 +32,7 @@
 #include <linux/limits.h>
 #include <linux/perf_event.h>
 #include <linux/ring_buffer.h>
+#include <sys/ioctl.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/vfs.h>
@@ -3958,6 +3959,66 @@ int bpf_link__destroy(struct bpf_link *link)
 	return err;
 }
 
+struct bpf_link_fd {
+	struct bpf_link link; /* has to be at the top of struct */
+	int fd; /* hook FD */
+};
+
+static int bpf_link__destroy_perf_event(struct bpf_link *link)
+{
+	struct bpf_link_fd *l = (void *)link;
+	int err;
+
+	if (l->fd < 0)
+		return 0;
+
+	err = ioctl(l->fd, PERF_EVENT_IOC_DISABLE, 0);
+	if (err)
+		err = -errno;
+
+	close(l->fd);
+	return err;
+}
+
+struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
+						int pfd)
+{
+	char errmsg[STRERR_BUFSIZE];
+	struct bpf_link_fd *link;
+	int prog_fd, err;
+
+	prog_fd = bpf_program__fd(prog);
+	if (prog_fd < 0) {
+		pr_warning("program '%s': can't attach before loaded\n",
+			   bpf_program__title(prog, false));
+		return ERR_PTR(-EINVAL);
+	}
+
+	link = malloc(sizeof(*link));
+	if (!link)
+		return ERR_PTR(-ENOMEM);
+	link->link.destroy = &bpf_link__destroy_perf_event;
+	link->fd = pfd;
+
+	if (ioctl(pfd, PERF_EVENT_IOC_SET_BPF, prog_fd) < 0) {
+		err = -errno;
+		free(link);
+		pr_warning("program '%s': failed to attach to pfd %d: %s\n",
+			   bpf_program__title(prog, false), pfd,
+			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
+		return ERR_PTR(err);
+	}
+	if (ioctl(pfd, PERF_EVENT_IOC_ENABLE, 0) < 0) {
+		err = -errno;
+		free(link);
+		pr_warning("program '%s': failed to enable pfd %d: %s\n",
+			   bpf_program__title(prog, false), pfd,
+			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
+		return ERR_PTR(err);
+	}
+	return (struct bpf_link *)link;
+}
+
 enum bpf_perf_event_ret
 bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
 			   void **copy_mem, size_t *copy_size,
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 5082a5ebb0c2..1bf66c4a9330 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -169,6 +169,9 @@ struct bpf_link;
 
 LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
 
+LIBBPF_API struct bpf_link *
+bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
+
 struct bpf_insn;
 
 /*
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 3cde850fc8da..756f5aa802e9 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -169,6 +169,7 @@ LIBBPF_0.0.4 {
 	global:
 		bpf_link__destroy;
 		bpf_object__load_xattr;
+		bpf_program__attach_perf_event;
 		btf_dump__dump_type;
 		btf_dump__free;
 		btf_dump__new;
-- 
2.17.1

[PATCH v4 bpf-next 4/9] libbpf: add kprobe/uprobe attach API

[Andrii Nakryiko]

Add ability to attach to kernel and user probes and retprobes.
Implementation depends on perf event support for kprobes/uprobes.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/libbpf.c   | 165 +++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h   |   7 ++
 tools/lib/bpf/libbpf.map |   2 +
 3 files changed, 174 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 98c155ec3bfa..2f79e9563db9 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4019,6 +4019,171 @@ struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
 	return (struct bpf_link *)link;
 }
 
+static int parse_value_from_file(const char *file, const char *fmt)
+{
+	char buf[STRERR_BUFSIZE];
+	int err, ret;
+	FILE *f;
+
+	f = fopen(file, "r");
+	if (!f) {
+		err = -errno;
+		pr_debug("failed to open '%s': %s\n", file,
+			 libbpf_strerror_r(err, buf, sizeof(buf)));
+		fclose(f);
+		return err;
+	}
+	err = fscanf(f, fmt, &ret);
+	if (err != 1) {
+		err = err == EOF ? -EIO : -errno;
+		pr_debug("failed to parse '%s': %s\n", file,
+			libbpf_strerror_r(err, buf, sizeof(buf)));
+		fclose(f);
+		return err;
+	}
+	fclose(f);
+	return ret;
+}
+
+static int determine_kprobe_perf_type(void)
+{
+	const char *file = "/sys/bus/event_source/devices/kprobe/type";
+
+	return parse_value_from_file(file, "%d\n");
+}
+
+static int determine_uprobe_perf_type(void)
+{
+	const char *file = "/sys/bus/event_source/devices/uprobe/type";
+
+	return parse_value_from_file(file, "%d\n");
+}
+
+static int determine_kprobe_retprobe_bit(void)
+{
+	const char *file = "/sys/bus/event_source/devices/kprobe/format/retprobe";
+
+	return parse_value_from_file(file, "config:%d\n");
+}
+
+static int determine_uprobe_retprobe_bit(void)
+{
+	const char *file = "/sys/bus/event_source/devices/uprobe/format/retprobe";
+
+	return parse_value_from_file(file, "config:%d\n");
+}
+
+static int perf_event_open_probe(bool uprobe, bool retprobe, const char *name,
+				 uint64_t offset, int pid)
+{
+	struct perf_event_attr attr = {};
+	char errmsg[STRERR_BUFSIZE];
+	int type, pfd, err;
+
+	type = uprobe ? determine_uprobe_perf_type()
+		      : determine_kprobe_perf_type();
+	if (type < 0) {
+		pr_warning("failed to determine %s perf type: %s\n",
+			   uprobe ? "uprobe" : "kprobe",
+			   libbpf_strerror_r(type, errmsg, sizeof(errmsg)));
+		return type;
+	}
+	if (retprobe) {
+		int bit = uprobe ? determine_uprobe_retprobe_bit()
+				 : determine_kprobe_retprobe_bit();
+
+		if (bit < 0) {
+			pr_warning("failed to determine %s retprobe bit: %s\n",
+				   uprobe ? "uprobe" : "kprobe",
+				   libbpf_strerror_r(bit, errmsg,
+						     sizeof(errmsg)));
+			return bit;
+		}
+		attr.config |= 1 << bit;
+	}
+	attr.size = sizeof(attr);
+	attr.type = type;
+	attr.config1 = (uint64_t)(void *)name; /* kprobe_func or uprobe_path */
+	attr.config2 = offset;		       /* kprobe_addr or probe_offset */
+
+	/* pid filter is meaningful only for uprobes */
+	pfd = syscall(__NR_perf_event_open, &attr,
+		      pid < 0 ? -1 : pid /* pid */,
+		      pid == -1 ? 0 : -1 /* cpu */,
+		      -1 /* group_fd */, PERF_FLAG_FD_CLOEXEC);
+	if (pfd < 0) {
+		err = -errno;
+		pr_warning("%s perf_event_open() failed: %s\n",
+			   uprobe ? "uprobe" : "kprobe",
+			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
+		return err;
+	}
+	return pfd;
+}
+
+struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog,
+					    bool retprobe,
+					    const char *func_name)
+{
+	char errmsg[STRERR_BUFSIZE];
+	struct bpf_link *link;
+	int pfd, err;
+
+	pfd = perf_event_open_probe(false /* uprobe */, retprobe, func_name,
+				    0 /* offset */, -1 /* pid */);
+	if (pfd < 0) {
+		pr_warning("program '%s': failed to create %s '%s' perf event: %s\n",
+			   bpf_program__title(prog, false),
+			   retprobe ? "kretprobe" : "kprobe", func_name,
+			   libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
+		return ERR_PTR(pfd);
+	}
+	link = bpf_program__attach_perf_event(prog, pfd);
+	if (IS_ERR(link)) {
+		close(pfd);
+		err = PTR_ERR(link);
+		pr_warning("program '%s': failed to attach to %s '%s': %s\n",
+			   bpf_program__title(prog, false),
+			   retprobe ? "kretprobe" : "kprobe", func_name,
+			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
+		return link;
+	}
+	return link;
+}
+
+struct bpf_link *bpf_program__attach_uprobe(struct bpf_program *prog,
+					    bool retprobe, pid_t pid,
+					    const char *binary_path,
+					    size_t func_offset)
+{
+	char errmsg[STRERR_BUFSIZE];
+	struct bpf_link *link;
+	int pfd, err;
+
+	pfd = perf_event_open_probe(true /* uprobe */, retprobe,
+				    binary_path, func_offset, pid);
+	if (pfd < 0) {
+		pr_warning("program '%s': failed to create %s '%s:0x%zx' perf event: %s\n",
+			   bpf_program__title(prog, false),
+			   retprobe ? "uretprobe" : "uprobe",
+			   binary_path, func_offset,
+			   libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
+		return ERR_PTR(pfd);
+	}
+	link = bpf_program__attach_perf_event(prog, pfd);
+	if (IS_ERR(link)) {
+		close(pfd);
+		err = PTR_ERR(link);
+		pr_warning("program '%s': failed to attach to %s '%s:0x%zx': %s\n",
+			   bpf_program__title(prog, false),
+			   retprobe ? "uretprobe" : "uprobe",
+			   binary_path, func_offset,
+			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
+		return link;
+	}
+	return link;
+}
+
 enum bpf_perf_event_ret
 bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
 			   void **copy_mem, size_t *copy_size,
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 1bf66c4a9330..bd767cc11967 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -171,6 +171,13 @@ LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
 
 LIBBPF_API struct bpf_link *
 bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
+LIBBPF_API struct bpf_link *
+bpf_program__attach_kprobe(struct bpf_program *prog, bool retprobe,
+			   const char *func_name);
+LIBBPF_API struct bpf_link *
+bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
+			   pid_t pid, const char *binary_path,
+			   size_t func_offset);
 
 struct bpf_insn;
 
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 756f5aa802e9..57a40fb60718 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -169,7 +169,9 @@ LIBBPF_0.0.4 {
 	global:
 		bpf_link__destroy;
 		bpf_object__load_xattr;
+		bpf_program__attach_kprobe;
 		bpf_program__attach_perf_event;
+		bpf_program__attach_uprobe;
 		btf_dump__dump_type;
 		btf_dump__free;
 		btf_dump__new;
-- 
2.17.1

[PATCH v4 bpf-next 5/9] libbpf: add tracepoint attach API

[Andrii Nakryiko]

Allow attaching BPF programs to kernel tracepoint BPF hooks specified by
category and name.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>
---
 tools/lib/bpf/libbpf.c   | 79 ++++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h   |  4 ++
 tools/lib/bpf/libbpf.map |  1 +
 3 files changed, 84 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 2f79e9563db9..8ad4f915df38 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4184,6 +4184,85 @@ struct bpf_link *bpf_program__attach_uprobe(struct bpf_program *prog,
 	return link;
 }
 
+static int determine_tracepoint_id(const char *tp_category,
+				   const char *tp_name)
+{
+	char file[PATH_MAX];
+	int ret;
+
+	ret = snprintf(file, sizeof(file),
+		       "/sys/kernel/debug/tracing/events/%s/%s/id",
+		       tp_category, tp_name);
+	if (ret < 0)
+		return -errno;
+	if (ret >= sizeof(file)) {
+		pr_debug("tracepoint %s/%s path is too long\n",
+			 tp_category, tp_name);
+		return -E2BIG;
+	}
+	return parse_value_from_file(file, "%d\n");
+}
+
+static int perf_event_open_tracepoint(const char *tp_category,
+				      const char *tp_name)
+{
+	struct perf_event_attr attr = {};
+	char errmsg[STRERR_BUFSIZE];
+	int tp_id, pfd, err;
+
+	tp_id = determine_tracepoint_id(tp_category, tp_name);
+	if (tp_id < 0) {
+		pr_warning("failed to determine tracepoint '%s/%s' perf ID: %s\n",
+			   tp_category, tp_name,
+			   libbpf_strerror_r(tp_id, errmsg, sizeof(errmsg)));
+		return tp_id;
+	}
+
+	attr.type = PERF_TYPE_TRACEPOINT;
+	attr.size = sizeof(attr);
+	attr.config = tp_id;
+
+	pfd = syscall(__NR_perf_event_open, &attr, -1 /* pid */, 0 /* cpu */,
+		      -1 /* group_fd */, PERF_FLAG_FD_CLOEXEC);
+	if (pfd < 0) {
+		err = -errno;
+		pr_warning("tracepoint '%s/%s' perf_event_open() failed: %s\n",
+			   tp_category, tp_name,
+			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
+		return err;
+	}
+	return pfd;
+}
+
+struct bpf_link *bpf_program__attach_tracepoint(struct bpf_program *prog,
+						const char *tp_category,
+						const char *tp_name)
+{
+	char errmsg[STRERR_BUFSIZE];
+	struct bpf_link *link;
+	int pfd, err;
+
+	pfd = perf_event_open_tracepoint(tp_category, tp_name);
+	if (pfd < 0) {
+		pr_warning("program '%s': failed to create tracepoint '%s/%s' perf event: %s\n",
+			   bpf_program__title(prog, false),
+			   tp_category, tp_name,
+			   libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
+		return ERR_PTR(pfd);
+	}
+	link = bpf_program__attach_perf_event(prog, pfd);
+	if (IS_ERR(link)) {
+		close(pfd);
+		err = PTR_ERR(link);
+		pr_warning("program '%s': failed to attach to tracepoint '%s/%s': %s\n",
+			   bpf_program__title(prog, false),
+			   tp_category, tp_name,
+			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
+		return link;
+	}
+	return link;
+}
+
 enum bpf_perf_event_ret
 bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
 			   void **copy_mem, size_t *copy_size,
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index bd767cc11967..60611f4b4e1d 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -178,6 +178,10 @@ LIBBPF_API struct bpf_link *
 bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
 			   pid_t pid, const char *binary_path,
 			   size_t func_offset);
+LIBBPF_API struct bpf_link *
+bpf_program__attach_tracepoint(struct bpf_program *prog,
+			       const char *tp_category,
+			       const char *tp_name);
 
 struct bpf_insn;
 
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 57a40fb60718..3c618b75ef65 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -171,6 +171,7 @@ LIBBPF_0.0.4 {
 		bpf_object__load_xattr;
 		bpf_program__attach_kprobe;
 		bpf_program__attach_perf_event;
+		bpf_program__attach_tracepoint;
 		bpf_program__attach_uprobe;
 		btf_dump__dump_type;
 		btf_dump__free;
-- 
2.17.1

[PATCH v4 bpf-next 6/9] libbpf: add raw tracepoint attach API

[Andrii Nakryiko]

Add a wrapper utilizing bpf_link "infrastructure" to allow attaching BPF
programs to raw tracepoints.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>
---
 tools/lib/bpf/libbpf.c   | 37 +++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h   |  3 +++
 tools/lib/bpf/libbpf.map |  1 +
 3 files changed, 41 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 8ad4f915df38..f8c7a7ecb35e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4263,6 +4263,43 @@ struct bpf_link *bpf_program__attach_tracepoint(struct bpf_program *prog,
 	return link;
 }
 
+static int bpf_link__destroy_fd(struct bpf_link *link)
+{
+	struct bpf_link_fd *l = (void *)link;
+
+	return close(l->fd);
+}
+
+struct bpf_link *bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
+						    const char *tp_name)
+{
+	char errmsg[STRERR_BUFSIZE];
+	struct bpf_link_fd *link;
+	int prog_fd, pfd;
+
+	prog_fd = bpf_program__fd(prog);
+	if (prog_fd < 0) {
+		pr_warning("program '%s': can't attach before loaded\n",
+			   bpf_program__title(prog, false));
+		return ERR_PTR(-EINVAL);
+	}
+
+	link = malloc(sizeof(*link));
+	link->link.destroy = &bpf_link__destroy_fd;
+
+	pfd = bpf_raw_tracepoint_open(tp_name, prog_fd);
+	if (pfd < 0) {
+		pfd = -errno;
+		free(link);
+		pr_warning("program '%s': failed to attach to raw tracepoint '%s': %s\n",
+			   bpf_program__title(prog, false), tp_name,
+			   libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
+		return ERR_PTR(pfd);
+	}
+	link->fd = pfd;
+	return (struct bpf_link *)link;
+}
+
 enum bpf_perf_event_ret
 bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
 			   void **copy_mem, size_t *copy_size,
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 60611f4b4e1d..f55933784f95 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -182,6 +182,9 @@ LIBBPF_API struct bpf_link *
 bpf_program__attach_tracepoint(struct bpf_program *prog,
 			       const char *tp_category,
 			       const char *tp_name);
+LIBBPF_API struct bpf_link *
+bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
+				   const char *tp_name);
 
 struct bpf_insn;
 
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 3c618b75ef65..e6b7d4edbc93 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -171,6 +171,7 @@ LIBBPF_0.0.4 {
 		bpf_object__load_xattr;
 		bpf_program__attach_kprobe;
 		bpf_program__attach_perf_event;
+		bpf_program__attach_raw_tracepoint;
 		bpf_program__attach_tracepoint;
 		bpf_program__attach_uprobe;
 		btf_dump__dump_type;
-- 
2.17.1

[PATCH v4 bpf-next 7/9] selftests/bpf: switch test to new attach_perf_event API

[Andrii Nakryiko]

Use new bpf_program__attach_perf_event() in test previously relying on
direct ioctl manipulations.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Reviewed-by: Stanislav Fomichev <sdf@google.com>
Acked-by: Song Liu <songliubraving@fb.com>
---
 .../bpf/prog_tests/stacktrace_build_id_nmi.c  | 31 +++++++++----------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
index 1c1a2f75f3d8..9557b7dfb782 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
@@ -17,6 +17,7 @@ static __u64 read_perf_max_sample_freq(void)
 void test_stacktrace_build_id_nmi(void)
 {
 	int control_map_fd, stackid_hmap_fd, stackmap_fd, stack_amap_fd;
+	const char *prog_name = "tracepoint/random/urandom_read";
 	const char *file = "./test_stacktrace_build_id.o";
 	int err, pmu_fd, prog_fd;
 	struct perf_event_attr attr = {
@@ -25,7 +26,9 @@ void test_stacktrace_build_id_nmi(void)
 		.config = PERF_COUNT_HW_CPU_CYCLES,
 	};
 	__u32 key, previous_key, val, duration = 0;
+	struct bpf_program *prog;
 	struct bpf_object *obj;
+	struct bpf_link *link;
 	char buf[256];
 	int i, j;
 	struct bpf_stack_build_id id_offs[PERF_MAX_STACK_DEPTH];
@@ -39,6 +42,10 @@ void test_stacktrace_build_id_nmi(void)
 	if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno))
 		return;
 
+	prog = bpf_object__find_program_by_title(obj, prog_name);
+	if (CHECK(!prog, "find_prog", "prog '%s' not found\n", prog_name))
+		goto close_prog;
+
 	pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */,
 			 0 /* cpu 0 */, -1 /* group id */,
 			 0 /* flags */);
@@ -47,15 +54,12 @@ void test_stacktrace_build_id_nmi(void)
 		  pmu_fd, errno))
 		goto close_prog;
 
-	err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
-	if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n",
-		  err, errno))
-		goto close_pmu;
-
-	err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
-	if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n",
-		  err, errno))
-		goto disable_pmu;
+	link = bpf_program__attach_perf_event(prog, pmu_fd);
+	if (CHECK(IS_ERR(link), "attach_perf_event",
+		  "err %ld\n", PTR_ERR(link))) {
+		close(pmu_fd);
+		goto close_prog;
+	}
 
 	/* find map fds */
 	control_map_fd = bpf_find_map(__func__, obj, "control_map");
@@ -134,8 +138,7 @@ void test_stacktrace_build_id_nmi(void)
 	 * try it one more time.
 	 */
 	if (build_id_matches < 1 && retry--) {
-		ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
-		close(pmu_fd);
+		bpf_link__destroy(link);
 		bpf_object__close(obj);
 		printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
 		       __func__);
@@ -154,11 +157,7 @@ void test_stacktrace_build_id_nmi(void)
 	 */
 
 disable_pmu:
-	ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
-
-close_pmu:
-	close(pmu_fd);
-
+	bpf_link__destroy(link);
 close_prog:
 	bpf_object__close(obj);
 }
-- 
2.17.1

[PATCH v4 bpf-next 8/9] selftests/bpf: add kprobe/uprobe selftests

[Andrii Nakryiko]

Add tests verifying kprobe/kretprobe/uprobe/uretprobe APIs work as
expected.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Reviewed-by: Stanislav Fomichev <sdf@google.com>
Acked-by: Song Liu <songliubraving@fb.com>
---
 .../selftests/bpf/prog_tests/attach_probe.c   | 155 ++++++++++++++++++
 .../selftests/bpf/progs/test_attach_probe.c   |  55 +++++++
 2 files changed, 210 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/attach_probe.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_attach_probe.c

diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
new file mode 100644
index 000000000000..f22929063c58
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
@@ -0,0 +1,155 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+
+ssize_t get_base_addr() {
+	size_t start;
+	char buf[256];
+	FILE *f;
+
+	f = fopen("/proc/self/maps", "r");
+	if (!f)
+		return -errno;
+
+	while (fscanf(f, "%zx-%*x %s %*s\n", &start, buf) == 2) {
+		if (strcmp(buf, "r-xp") == 0) {
+			fclose(f);
+			return start;
+		}
+	}
+
+	fclose(f);
+	return -EINVAL;
+}
+
+void test_attach_probe(void)
+{
+	const char *kprobe_name = "kprobe/sys_nanosleep";
+	const char *kretprobe_name = "kretprobe/sys_nanosleep";
+	const char *uprobe_name = "uprobe/trigger_func";
+	const char *uretprobe_name = "uretprobe/trigger_func";
+	const int kprobe_idx = 0, kretprobe_idx = 1;
+	const int uprobe_idx = 2, uretprobe_idx = 3;
+	const char *file = "./test_attach_probe.o";
+	struct bpf_program *kprobe_prog, *kretprobe_prog;
+	struct bpf_program *uprobe_prog, *uretprobe_prog;
+	struct bpf_object *obj;
+	int err, prog_fd, duration = 0, res;
+	struct bpf_link *kprobe_link = NULL;
+	struct bpf_link *kretprobe_link = NULL;
+	struct bpf_link *uprobe_link = NULL;
+	struct bpf_link *uretprobe_link = NULL;
+	int results_map_fd;
+	size_t uprobe_offset;
+	ssize_t base_addr;
+
+	base_addr = get_base_addr();
+	if (CHECK(base_addr < 0, "get_base_addr",
+		  "failed to find base addr: %zd", base_addr))
+		return;
+	uprobe_offset = (size_t)&get_base_addr - base_addr;
+
+	/* load programs */
+	err = bpf_prog_load(file, BPF_PROG_TYPE_KPROBE, &obj, &prog_fd);
+	if (CHECK(err, "obj_load", "err %d errno %d\n", err, errno))
+		return;
+
+	kprobe_prog = bpf_object__find_program_by_title(obj, kprobe_name);
+	if (CHECK(!kprobe_prog, "find_probe",
+		  "prog '%s' not found\n", kprobe_name))
+		goto cleanup;
+	kretprobe_prog = bpf_object__find_program_by_title(obj, kretprobe_name);
+	if (CHECK(!kretprobe_prog, "find_probe",
+		  "prog '%s' not found\n", kretprobe_name))
+		goto cleanup;
+	uprobe_prog = bpf_object__find_program_by_title(obj, uprobe_name);
+	if (CHECK(!uprobe_prog, "find_probe",
+		  "prog '%s' not found\n", uprobe_name))
+		goto cleanup;
+	uretprobe_prog = bpf_object__find_program_by_title(obj, uretprobe_name);
+	if (CHECK(!uretprobe_prog, "find_probe",
+		  "prog '%s' not found\n", uretprobe_name))
+		goto cleanup;
+
+	/* load maps */
+	results_map_fd = bpf_find_map(__func__, obj, "results_map");
+	if (CHECK(results_map_fd < 0, "find_results_map",
+		  "err %d\n", results_map_fd))
+		goto cleanup;
+
+	kprobe_link = bpf_program__attach_kprobe(kprobe_prog,
+						 false /* retprobe */,
+						 "sys_nanosleep");
+	if (CHECK(IS_ERR(kprobe_link), "attach_kprobe",
+		  "err %ld\n", PTR_ERR(kprobe_link)))
+		goto cleanup;
+
+	kretprobe_link = bpf_program__attach_kprobe(kretprobe_prog,
+						    true /* retprobe */,
+						    "sys_nanosleep");
+	if (CHECK(IS_ERR(kretprobe_link), "attach_kretprobe",
+		  "err %ld\n", PTR_ERR(kretprobe_link)))
+		goto cleanup;
+
+	uprobe_link = bpf_program__attach_uprobe(uprobe_prog,
+						 false /* retprobe */,
+						 0 /* self pid */,
+						 "/proc/self/exe",
+						 uprobe_offset);
+	if (CHECK(IS_ERR(uprobe_link), "attach_uprobe",
+		  "err %ld\n", PTR_ERR(uprobe_link)))
+		goto cleanup;
+
+	uretprobe_link = bpf_program__attach_uprobe(uretprobe_prog,
+						    true /* retprobe */,
+						    -1 /* any pid */,
+						    "/proc/self/exe",
+						    uprobe_offset);
+	if (CHECK(IS_ERR(uretprobe_link), "attach_uretprobe",
+		  "err %ld\n", PTR_ERR(uretprobe_link)))
+		goto cleanup;
+
+	/* trigger & validate kprobe && kretprobe */
+	usleep(1);
+
+	err = bpf_map_lookup_elem(results_map_fd, &kprobe_idx, &res);
+	if (CHECK(err, "get_kprobe_res",
+		  "failed to get kprobe res: %d\n", err))
+		goto cleanup;
+	if (CHECK(res != kprobe_idx + 1, "check_kprobe_res",
+		  "wrong kprobe res: %d\n", res))
+		goto cleanup;
+
+	err = bpf_map_lookup_elem(results_map_fd, &kretprobe_idx, &res);
+	if (CHECK(err, "get_kretprobe_res",
+		  "failed to get kretprobe res: %d\n", err))
+		goto cleanup;
+	if (CHECK(res != kretprobe_idx + 1, "check_kretprobe_res",
+		  "wrong kretprobe res: %d\n", res))
+		goto cleanup;
+
+	/* trigger & validate uprobe & uretprobe */
+	get_base_addr();
+
+	err = bpf_map_lookup_elem(results_map_fd, &uprobe_idx, &res);
+	if (CHECK(err, "get_uprobe_res",
+		  "failed to get uprobe res: %d\n", err))
+		goto cleanup;
+	if (CHECK(res != uprobe_idx + 1, "check_uprobe_res",
+		  "wrong uprobe res: %d\n", res))
+		goto cleanup;
+
+	err = bpf_map_lookup_elem(results_map_fd, &uretprobe_idx, &res);
+	if (CHECK(err, "get_uretprobe_res",
+		  "failed to get uretprobe res: %d\n", err))
+		goto cleanup;
+	if (CHECK(res != uretprobe_idx + 1, "check_uretprobe_res",
+		  "wrong uretprobe res: %d\n", res))
+		goto cleanup;
+
+cleanup:
+	bpf_link__destroy(kprobe_link);
+	bpf_link__destroy(kretprobe_link);
+	bpf_link__destroy(uprobe_link);
+	bpf_link__destroy(uretprobe_link);
+	bpf_object__close(obj);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_attach_probe.c b/tools/testing/selftests/bpf/progs/test_attach_probe.c
new file mode 100644
index 000000000000..7a7c5cd728c8
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_attach_probe.c
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2017 Facebook
+
+#include <linux/ptrace.h>
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+struct {
+	int type;
+	int max_entries;
+	int *key;
+	int *value;
+} results_map SEC(".maps") = {
+	.type = BPF_MAP_TYPE_ARRAY,
+	.max_entries = 4,
+};
+
+SEC("kprobe/sys_nanosleep")
+int handle_sys_nanosleep_entry(struct pt_regs *ctx)
+{
+	const int key = 0, value = 1;
+
+	bpf_map_update_elem(&results_map, &key, &value, 0);
+	return 0;
+}
+
+SEC("kretprobe/sys_nanosleep")
+int handle_sys_getpid_return(struct pt_regs *ctx)
+{
+	const int key = 1, value = 2;
+
+	bpf_map_update_elem(&results_map, &key, &value, 0);
+	return 0;
+}
+
+SEC("uprobe/trigger_func")
+int handle_uprobe_entry(struct pt_regs *ctx)
+{
+	const int key = 2, value = 3;
+
+	bpf_map_update_elem(&results_map, &key, &value, 0);
+	return 0;
+}
+
+SEC("uretprobe/trigger_func")
+int handle_uprobe_return(struct pt_regs *ctx)
+{
+	const int key = 3, value = 4;
+
+	bpf_map_update_elem(&results_map, &key, &value, 0);
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
+__u32 _version SEC("version") = 1;
-- 
2.17.1

[PATCH v4 bpf-next 9/9] selftests/bpf: convert existing tracepoint tests to new APIs

[Andrii Nakryiko]

Convert some existing tests that attach to tracepoints to use
bpf_program__attach_tracepoint API instead.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Reviewed-by: Stanislav Fomichev <sdf@google.com>
Acked-by: Song Liu <songliubraving@fb.com>
---
 .../bpf/prog_tests/stacktrace_build_id.c      | 50 ++++---------------
 .../selftests/bpf/prog_tests/stacktrace_map.c | 43 ++++------------
 .../bpf/prog_tests/stacktrace_map_raw_tp.c    | 15 ++++--
 3 files changed, 31 insertions(+), 77 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
index 3aab2b083c71..768883d838ea 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
@@ -4,11 +4,13 @@
 void test_stacktrace_build_id(void)
 {
 	int control_map_fd, stackid_hmap_fd, stackmap_fd, stack_amap_fd;
+	const char *prog_name = "tracepoint/random/urandom_read";
 	const char *file = "./test_stacktrace_build_id.o";
-	int bytes, efd, err, pmu_fd, prog_fd, stack_trace_len;
-	struct perf_event_attr attr = {};
+	int err, prog_fd, stack_trace_len;
 	__u32 key, previous_key, val, duration = 0;
+	struct bpf_program *prog;
 	struct bpf_object *obj;
+	struct bpf_link *link = NULL;
 	char buf[256];
 	int i, j;
 	struct bpf_stack_build_id id_offs[PERF_MAX_STACK_DEPTH];
@@ -20,42 +22,14 @@ void test_stacktrace_build_id(void)
 	if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno))
 		goto out;
 
-	/* Get the ID for the sched/sched_switch tracepoint */
-	snprintf(buf, sizeof(buf),
-		 "/sys/kernel/debug/tracing/events/random/urandom_read/id");
-	efd = open(buf, O_RDONLY, 0);
-	if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
+	prog = bpf_object__find_program_by_title(obj, prog_name);
+	if (CHECK(!prog, "find_prog", "prog '%s' not found\n", prog_name))
 		goto close_prog;
 
-	bytes = read(efd, buf, sizeof(buf));
-	close(efd);
-	if (CHECK(bytes <= 0 || bytes >= sizeof(buf),
-		  "read", "bytes %d errno %d\n", bytes, errno))
+	link = bpf_program__attach_tracepoint(prog, "random", "urandom_read");
+	if (CHECK(IS_ERR(link), "attach_tp", "err %ld\n", PTR_ERR(link)))
 		goto close_prog;
 
-	/* Open the perf event and attach bpf progrram */
-	attr.config = strtol(buf, NULL, 0);
-	attr.type = PERF_TYPE_TRACEPOINT;
-	attr.sample_type = PERF_SAMPLE_RAW | PERF_SAMPLE_CALLCHAIN;
-	attr.sample_period = 1;
-	attr.wakeup_events = 1;
-	pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */,
-			 0 /* cpu 0 */, -1 /* group id */,
-			 0 /* flags */);
-	if (CHECK(pmu_fd < 0, "perf_event_open", "err %d errno %d\n",
-		  pmu_fd, errno))
-		goto close_prog;
-
-	err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
-	if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n",
-		  err, errno))
-		goto close_pmu;
-
-	err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
-	if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n",
-		  err, errno))
-		goto disable_pmu;
-
 	/* find map fds */
 	control_map_fd = bpf_find_map(__func__, obj, "control_map");
 	if (CHECK(control_map_fd < 0, "bpf_find_map control_map",
@@ -133,8 +107,7 @@ void test_stacktrace_build_id(void)
 	 * try it one more time.
 	 */
 	if (build_id_matches < 1 && retry--) {
-		ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
-		close(pmu_fd);
+		bpf_link__destroy(link);
 		bpf_object__close(obj);
 		printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
 		       __func__);
@@ -152,10 +125,7 @@ void test_stacktrace_build_id(void)
 	      "err %d errno %d\n", err, errno);
 
 disable_pmu:
-	ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
-
-close_pmu:
-	close(pmu_fd);
+	bpf_link__destroy(link);
 
 close_prog:
 	bpf_object__close(obj);
diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
index 2bfd50a0d6d1..fc539335c5b3 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
@@ -4,50 +4,26 @@
 void test_stacktrace_map(void)
 {
 	int control_map_fd, stackid_hmap_fd, stackmap_fd, stack_amap_fd;
+	const char *prog_name = "tracepoint/sched/sched_switch";
+	int err, prog_fd, stack_trace_len;
 	const char *file = "./test_stacktrace_map.o";
-	int bytes, efd, err, pmu_fd, prog_fd, stack_trace_len;
-	struct perf_event_attr attr = {};
 	__u32 key, val, duration = 0;
+	struct bpf_program *prog;
 	struct bpf_object *obj;
-	char buf[256];
+	struct bpf_link *link;
 
 	err = bpf_prog_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj, &prog_fd);
 	if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno))
 		return;
 
-	/* Get the ID for the sched/sched_switch tracepoint */
-	snprintf(buf, sizeof(buf),
-		 "/sys/kernel/debug/tracing/events/sched/sched_switch/id");
-	efd = open(buf, O_RDONLY, 0);
-	if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
+	prog = bpf_object__find_program_by_title(obj, prog_name);
+	if (CHECK(!prog, "find_prog", "prog '%s' not found\n", prog_name))
 		goto close_prog;
 
-	bytes = read(efd, buf, sizeof(buf));
-	close(efd);
-	if (bytes <= 0 || bytes >= sizeof(buf))
+	link = bpf_program__attach_tracepoint(prog, "sched", "sched_switch");
+	if (CHECK(IS_ERR(link), "attach_tp", "err %ld\n", PTR_ERR(link)))
 		goto close_prog;
 
-	/* Open the perf event and attach bpf progrram */
-	attr.config = strtol(buf, NULL, 0);
-	attr.type = PERF_TYPE_TRACEPOINT;
-	attr.sample_type = PERF_SAMPLE_RAW | PERF_SAMPLE_CALLCHAIN;
-	attr.sample_period = 1;
-	attr.wakeup_events = 1;
-	pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */,
-			 0 /* cpu 0 */, -1 /* group id */,
-			 0 /* flags */);
-	if (CHECK(pmu_fd < 0, "perf_event_open", "err %d errno %d\n",
-		  pmu_fd, errno))
-		goto close_prog;
-
-	err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
-	if (err)
-		goto disable_pmu;
-
-	err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
-	if (err)
-		goto disable_pmu;
-
 	/* find map fds */
 	control_map_fd = bpf_find_map(__func__, obj, "control_map");
 	if (control_map_fd < 0)
@@ -96,8 +72,7 @@ void test_stacktrace_map(void)
 disable_pmu:
 	error_cnt++;
 disable_pmu_noerr:
-	ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
-	close(pmu_fd);
+	bpf_link__destroy(link);
 close_prog:
 	bpf_object__close(obj);
 }
diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c
index 1f8387d80fd7..fbfa8e76cf63 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c
@@ -3,18 +3,25 @@
 
 void test_stacktrace_map_raw_tp(void)
 {
+	const char *prog_name = "tracepoint/sched/sched_switch";
 	int control_map_fd, stackid_hmap_fd, stackmap_fd;
 	const char *file = "./test_stacktrace_map.o";
-	int efd, err, prog_fd;
 	__u32 key, val, duration = 0;
+	int err, prog_fd;
+	struct bpf_program *prog;
 	struct bpf_object *obj;
+	struct bpf_link *link = NULL;
 
 	err = bpf_prog_load(file, BPF_PROG_TYPE_RAW_TRACEPOINT, &obj, &prog_fd);
 	if (CHECK(err, "prog_load raw tp", "err %d errno %d\n", err, errno))
 		return;
 
-	efd = bpf_raw_tracepoint_open("sched_switch", prog_fd);
-	if (CHECK(efd < 0, "raw_tp_open", "err %d errno %d\n", efd, errno))
+	prog = bpf_object__find_program_by_title(obj, prog_name);
+	if (CHECK(!prog, "find_prog", "prog '%s' not found\n", prog_name))
+		goto close_prog;
+
+	link = bpf_program__attach_raw_tracepoint(prog, "sched_switch");
+	if (CHECK(IS_ERR(link), "attach_raw_tp", "err %ld\n", PTR_ERR(link)))
 		goto close_prog;
 
 	/* find map fds */
@@ -55,5 +62,7 @@ void test_stacktrace_map_raw_tp(void)
 close_prog:
 	error_cnt++;
 close_prog_noerr:
+	if (!IS_ERR_OR_NULL(link))
+		bpf_link__destroy(link);
 	bpf_object__close(obj);
 }
-- 
2.17.1

Re: [PATCH v4 bpf-next 3/9] libbpf: add ability to attach/detach BPF program to perf event

[Song Liu]

On Fri, Jun 28, 2019 at 8:49 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> bpf_program__attach_perf_event allows to attach BPF program to existing
> perf event hook, providing most generic and most low-level way to attach BPF
> programs. It returns struct bpf_link, which should be passed to
> bpf_link__destroy to detach and free resources, associated with a link.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Acked-by: Song Liu <songliubraving@fb.com>

With one nit below.

> ---
>  tools/lib/bpf/libbpf.c   | 61 ++++++++++++++++++++++++++++++++++++++++
>  tools/lib/bpf/libbpf.h   |  3 ++
>  tools/lib/bpf/libbpf.map |  1 +
>  3 files changed, 65 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 455795e6f8af..98c155ec3bfa 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -32,6 +32,7 @@
>  #include <linux/limits.h>
>  #include <linux/perf_event.h>
>  #include <linux/ring_buffer.h>
> +#include <sys/ioctl.h>
>  #include <sys/stat.h>
>  #include <sys/types.h>
>  #include <sys/vfs.h>
> @@ -3958,6 +3959,66 @@ int bpf_link__destroy(struct bpf_link *link)
>         return err;
>  }
>
> +struct bpf_link_fd {
> +       struct bpf_link link; /* has to be at the top of struct */
> +       int fd; /* hook FD */
> +};
> +
> +static int bpf_link__destroy_perf_event(struct bpf_link *link)
> +{
> +       struct bpf_link_fd *l = (void *)link;
> +       int err;
> +
> +       if (l->fd < 0)
> +               return 0;
> +
> +       err = ioctl(l->fd, PERF_EVENT_IOC_DISABLE, 0);
> +       if (err)
> +               err = -errno;
> +
> +       close(l->fd);
> +       return err;
> +}
> +
> +struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
> +                                               int pfd)
> +{
> +       char errmsg[STRERR_BUFSIZE];
> +       struct bpf_link_fd *link;
> +       int prog_fd, err;
> +
> +       prog_fd = bpf_program__fd(prog);
> +       if (prog_fd < 0) {
> +               pr_warning("program '%s': can't attach before loaded\n",
> +                          bpf_program__title(prog, false));

This warning message is not very easy to follow.

> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       link = malloc(sizeof(*link));
> +       if (!link)
> +               return ERR_PTR(-ENOMEM);
> +       link->link.destroy = &bpf_link__destroy_perf_event;
> +       link->fd = pfd;
> +
> +       if (ioctl(pfd, PERF_EVENT_IOC_SET_BPF, prog_fd) < 0) {
> +               err = -errno;
> +               free(link);
> +               pr_warning("program '%s': failed to attach to pfd %d: %s\n",
> +                          bpf_program__title(prog, false), pfd,
> +                          libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> +               return ERR_PTR(err);
> +       }
> +       if (ioctl(pfd, PERF_EVENT_IOC_ENABLE, 0) < 0) {
> +               err = -errno;
> +               free(link);
> +               pr_warning("program '%s': failed to enable pfd %d: %s\n",
> +                          bpf_program__title(prog, false), pfd,
> +                          libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> +               return ERR_PTR(err);
> +       }
> +       return (struct bpf_link *)link;
> +}
> +
>  enum bpf_perf_event_ret
>  bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
>                            void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 5082a5ebb0c2..1bf66c4a9330 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -169,6 +169,9 @@ struct bpf_link;
>
>  LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
>
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
> +
>  struct bpf_insn;
>
>  /*
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 3cde850fc8da..756f5aa802e9 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -169,6 +169,7 @@ LIBBPF_0.0.4 {
>         global:
>                 bpf_link__destroy;
>                 bpf_object__load_xattr;
> +               bpf_program__attach_perf_event;
>                 btf_dump__dump_type;
>                 btf_dump__free;
>                 btf_dump__new;
> --
> 2.17.1
>

Re: [PATCH v4 bpf-next 4/9] libbpf: add kprobe/uprobe attach API

[Song Liu]

On Fri, Jun 28, 2019 at 8:49 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> Add ability to attach to kernel and user probes and retprobes.
> Implementation depends on perf event support for kprobes/uprobes.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Acked-by: Song Liu <songliubraving@fb.com>

Re: [PATCH v4 bpf-next 0/9] libbpf: add bpf_link and tracing attach APIs

[Stanislav Fomichev]

On 06/28, Andrii Nakryiko wrote:
> This patchset adds the following APIs to allow attaching BPF programs to
> tracing entities:
> - bpf_program__attach_perf_event for attaching to any opened perf event FD,
>   allowing users full control;
> - bpf_program__attach_kprobe for attaching to kernel probes (both entry and
>   return probes);
> - bpf_program__attach_uprobe for attaching to user probes (both entry/return);
> - bpf_program__attach_tracepoint for attaching to kernel tracepoints;
> - bpf_program__attach_raw_tracepoint for attaching to raw kernel tracepoint
>   (wrapper around bpf_raw_tracepoint_open);
> 
> This set of APIs makes libbpf more useful for tracing applications.
> 
> All attach APIs return abstract struct bpf_link that encapsulates logic of
> detaching BPF program. See patch #2 for details. bpf_assoc was considered as
> an alternative name for this opaque "handle", but bpf_link seems to be
> appropriate semantically and is nice and short.
> 
> Pre-patch #1 makes internal libbpf_strerror_r helper function work w/ negative
> error codes, lifting the burder off callers to keep track of error sign.
> Patch #2 adds bpf_link abstraction.
> Patch #3 adds attach_perf_event, which is the base for all other APIs.
> Patch #4 adds kprobe/uprobe APIs.
> Patch #5 adds tracepoint API.
> Patch #6 adds raw_tracepoint API.
> Patch #7 converts one existing test to use attach_perf_event.
> Patch #8 adds new kprobe/uprobe tests.
> Patch #9 converts some selftests currently using tracepoint to new APIs.
> 
> v3->v4:
> - proper errno handling (Stanislav);
> - bpf_fd -> prog_fd (Stanislav);
> - switch to fprintf (Song);
Reviewed-by: Stanislav Fomichev <sdf@google.com>

Thanks!

> v2->v3:
> - added bpf_link concept (Daniel);
> - didn't add generic bpf_link__attach_program for reasons described in [0];
> - dropped Stanislav's Reviewed-by from patches #2-#6, in case he doesn't like
>   the change;
> v1->v2:
> - preserve errno before close() call (Stanislav);
> - use libbpf_perf_event_disable_and_close in selftest (Stanislav);
> - remove unnecessary memset (Stanislav);
> 
> [0] https://lore.kernel.org/bpf/CAEf4BzZ7EM5eP2eaZn7T2Yb5QgVRiwAs+epeLR1g01TTx-6m6Q@mail.gmail.com/
> 
> Andrii Nakryiko (9):
>   libbpf: make libbpf_strerror_r agnostic to sign of error
>   libbpf: introduce concept of bpf_link
>   libbpf: add ability to attach/detach BPF program to perf event
>   libbpf: add kprobe/uprobe attach API
>   libbpf: add tracepoint attach API
>   libbpf: add raw tracepoint attach API
>   selftests/bpf: switch test to new attach_perf_event API
>   selftests/bpf: add kprobe/uprobe selftests
>   selftests/bpf: convert existing tracepoint tests to new APIs
> 
>  tools/lib/bpf/libbpf.c                        | 359 ++++++++++++++++++
>  tools/lib/bpf/libbpf.h                        |  21 +
>  tools/lib/bpf/libbpf.map                      |   8 +-
>  tools/lib/bpf/str_error.c                     |   2 +-
>  .../selftests/bpf/prog_tests/attach_probe.c   | 155 ++++++++
>  .../bpf/prog_tests/stacktrace_build_id.c      |  50 +--
>  .../bpf/prog_tests/stacktrace_build_id_nmi.c  |  31 +-
>  .../selftests/bpf/prog_tests/stacktrace_map.c |  43 +--
>  .../bpf/prog_tests/stacktrace_map_raw_tp.c    |  15 +-
>  .../selftests/bpf/progs/test_attach_probe.c   |  55 +++
>  10 files changed, 644 insertions(+), 95 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/attach_probe.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_attach_probe.c
> 
> -- 
> 2.17.1
> 

Re: [PATCH v4 bpf-next 2/9] libbpf: introduce concept of bpf_link

[Yonghong Song]

On 6/28/19 8:48 PM, Andrii Nakryiko wrote:
> bpf_link is and abstraction of an association of a BPF program and one

"is and" => "is an".

> of many possible BPF attachment points (hooks). This allows to have
> uniform interface for detaching BPF programs regardless of the nature of
> link and how it was created. Details of creation and setting up of
> a specific bpf_link is handled by corresponding attachment methods
> (bpf_program__attach_xxx) added in subsequent commits. Once successfully
> created, bpf_link has to be eventually destroyed with
> bpf_link__destroy(), at which point BPF program is disassociated from
> a hook and all the relevant resources are freed.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> ---
>   tools/lib/bpf/libbpf.c   | 17 +++++++++++++++++
>   tools/lib/bpf/libbpf.h   |  4 ++++
>   tools/lib/bpf/libbpf.map |  3 ++-
>   3 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 6e6ebef11ba3..455795e6f8af 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -3941,6 +3941,23 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
>   	return 0;
>   }
>   
> +struct bpf_link {
> +	int (*destroy)(struct bpf_link *link);
> +};
> +
> +int bpf_link__destroy(struct bpf_link *link)
> +{
> +	int err;
> +
> +	if (!link)
> +		return 0;
> +
> +	err = link->destroy(link);
> +	free(link);
> +
> +	return err;
> +}
> +
>   enum bpf_perf_event_ret
>   bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
>   			   void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index d639f47e3110..5082a5ebb0c2 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -165,6 +165,10 @@ LIBBPF_API int bpf_program__pin(struct bpf_program *prog, const char *path);
>   LIBBPF_API int bpf_program__unpin(struct bpf_program *prog, const char *path);
>   LIBBPF_API void bpf_program__unload(struct bpf_program *prog);
>   
> +struct bpf_link;
> +
> +LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
> +
>   struct bpf_insn;
>   
>   /*
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 2c6d835620d2..3cde850fc8da 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -167,10 +167,11 @@ LIBBPF_0.0.3 {
>   
>   LIBBPF_0.0.4 {
>   	global:
> +		bpf_link__destroy;
> +		bpf_object__load_xattr;
>   		btf_dump__dump_type;
>   		btf_dump__free;
>   		btf_dump__new;
>   		btf__parse_elf;
> -		bpf_object__load_xattr;
>   		libbpf_num_possible_cpus;
>   } LIBBPF_0.0.3;
> 

Re: [PATCH v4 bpf-next 3/9] libbpf: add ability to attach/detach BPF program to perf event

[Yonghong Song]

On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> bpf_program__attach_perf_event allows to attach BPF program to existing
> perf event hook, providing most generic and most low-level way to attach BPF
> programs. It returns struct bpf_link, which should be passed to
> bpf_link__destroy to detach and free resources, associated with a link.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>   tools/lib/bpf/libbpf.c   | 61 ++++++++++++++++++++++++++++++++++++++++
>   tools/lib/bpf/libbpf.h   |  3 ++
>   tools/lib/bpf/libbpf.map |  1 +
>   3 files changed, 65 insertions(+)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 455795e6f8af..98c155ec3bfa 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -32,6 +32,7 @@
>   #include <linux/limits.h>
>   #include <linux/perf_event.h>
>   #include <linux/ring_buffer.h>
> +#include <sys/ioctl.h>
>   #include <sys/stat.h>
>   #include <sys/types.h>
>   #include <sys/vfs.h>
> @@ -3958,6 +3959,66 @@ int bpf_link__destroy(struct bpf_link *link)
>   	return err;
>   }
>   
> +struct bpf_link_fd {
> +	struct bpf_link link; /* has to be at the top of struct */
> +	int fd; /* hook FD */
> +};
> +
> +static int bpf_link__destroy_perf_event(struct bpf_link *link)
> +{
> +	struct bpf_link_fd *l = (void *)link;
> +	int err;
> +
> +	if (l->fd < 0)
> +		return 0;
> +
> +	err = ioctl(l->fd, PERF_EVENT_IOC_DISABLE, 0);
> +	if (err)
> +		err = -errno;
> +
> +	close(l->fd);
> +	return err;
> +}
> +
> +struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
> +						int pfd)
> +{
> +	char errmsg[STRERR_BUFSIZE];
> +	struct bpf_link_fd *link;
> +	int prog_fd, err;
> +
> +	prog_fd = bpf_program__fd(prog);
> +	if (prog_fd < 0) {
> +		pr_warning("program '%s': can't attach before loaded\n",
> +			   bpf_program__title(prog, false));
> +		return ERR_PTR(-EINVAL);
> +	}

should we check validity of pfd here?
If pfd < 0, we just return ERR_PTR(-EINVAL)?
This way, in bpf_link__destroy_perf_event(), we do not need to check
l->fd < 0 since it will be always nonnegative.

> +
> +	link = malloc(sizeof(*link));
> +	if (!link)
> +		return ERR_PTR(-ENOMEM);
> +	link->link.destroy = &bpf_link__destroy_perf_event;
> +	link->fd = pfd;
> +
> +	if (ioctl(pfd, PERF_EVENT_IOC_SET_BPF, prog_fd) < 0) {
> +		err = -errno;
> +		free(link);
> +		pr_warning("program '%s': failed to attach to pfd %d: %s\n",
> +			   bpf_program__title(prog, false), pfd,
> +			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> +		return ERR_PTR(err);
> +	}
> +	if (ioctl(pfd, PERF_EVENT_IOC_ENABLE, 0) < 0) {
> +		err = -errno;
> +		free(link);
> +		pr_warning("program '%s': failed to enable pfd %d: %s\n",
> +			   bpf_program__title(prog, false), pfd,
> +			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> +		return ERR_PTR(err);
> +	}
> +	return (struct bpf_link *)link;
> +}
> +
>   enum bpf_perf_event_ret
>   bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
>   			   void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 5082a5ebb0c2..1bf66c4a9330 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -169,6 +169,9 @@ struct bpf_link;
>   
>   LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
>   
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
> +
>   struct bpf_insn;
>   
>   /*
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 3cde850fc8da..756f5aa802e9 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -169,6 +169,7 @@ LIBBPF_0.0.4 {
>   	global:
>   		bpf_link__destroy;
>   		bpf_object__load_xattr;
> +		bpf_program__attach_perf_event;
>   		btf_dump__dump_type;
>   		btf_dump__free;
>   		btf_dump__new;
> 

Re: [PATCH v4 bpf-next 4/9] libbpf: add kprobe/uprobe attach API

[Yonghong Song]

On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> Add ability to attach to kernel and user probes and retprobes.
> Implementation depends on perf event support for kprobes/uprobes.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>   tools/lib/bpf/libbpf.c   | 165 +++++++++++++++++++++++++++++++++++++++
>   tools/lib/bpf/libbpf.h   |   7 ++
>   tools/lib/bpf/libbpf.map |   2 +
>   3 files changed, 174 insertions(+)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 98c155ec3bfa..2f79e9563db9 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4019,6 +4019,171 @@ struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
>   	return (struct bpf_link *)link;
>   }
>   
> +static int parse_value_from_file(const char *file, const char *fmt)

Here, the value from the file must be positive int values to avoid
confusion between valid value vs. error code.
Could you add a comment to state this fact for the current 
uprobe/kprobe/tracepoint support?

> +{
> +	char buf[STRERR_BUFSIZE];
> +	int err, ret;
> +	FILE *f;
> +
> +	f = fopen(file, "r");
> +	if (!f) {
> +		err = -errno;
> +		pr_debug("failed to open '%s': %s\n", file,
> +			 libbpf_strerror_r(err, buf, sizeof(buf)));
> +		fclose(f);

fclose(f) is not needed. fopen has failed.

> +		return err;
> +	}
> +	err = fscanf(f, fmt, &ret);
> +	if (err != 1) {
> +		err = err == EOF ? -EIO : -errno;
> +		pr_debug("failed to parse '%s': %s\n", file,
> +			libbpf_strerror_r(err, buf, sizeof(buf)));
> +		fclose(f);
> +		return err;
> +	}
> +	fclose(f);
> +	return ret;
> +}
> +
> +static int determine_kprobe_perf_type(void)
> +{
> +	const char *file = "/sys/bus/event_source/devices/kprobe/type";
> +
> +	return parse_value_from_file(file, "%d\n");
> +}
> +
> +static int determine_uprobe_perf_type(void)
> +{
> +	const char *file = "/sys/bus/event_source/devices/uprobe/type";
> +
> +	return parse_value_from_file(file, "%d\n");
> +}
> +
> +static int determine_kprobe_retprobe_bit(void)
> +{
> +	const char *file = "/sys/bus/event_source/devices/kprobe/format/retprobe";
> +
> +	return parse_value_from_file(file, "config:%d\n");
> +}
> +
> +static int determine_uprobe_retprobe_bit(void)
> +{
> +	const char *file = "/sys/bus/event_source/devices/uprobe/format/retprobe";
> +
> +	return parse_value_from_file(file, "config:%d\n");
> +}
> +
> +static int perf_event_open_probe(bool uprobe, bool retprobe, const char *name,
> +				 uint64_t offset, int pid)
> +{
> +	struct perf_event_attr attr = {};
> +	char errmsg[STRERR_BUFSIZE];
> +	int type, pfd, err;
> +
> +	type = uprobe ? determine_uprobe_perf_type()
> +		      : determine_kprobe_perf_type();
> +	if (type < 0) {
> +		pr_warning("failed to determine %s perf type: %s\n",
> +			   uprobe ? "uprobe" : "kprobe",
> +			   libbpf_strerror_r(type, errmsg, sizeof(errmsg)));
> +		return type;
> +	}
> +	if (retprobe) {
> +		int bit = uprobe ? determine_uprobe_retprobe_bit()
> +				 : determine_kprobe_retprobe_bit();
> +
> +		if (bit < 0) {
> +			pr_warning("failed to determine %s retprobe bit: %s\n",
> +				   uprobe ? "uprobe" : "kprobe",
> +				   libbpf_strerror_r(bit, errmsg,
> +						     sizeof(errmsg)));
> +			return bit;
> +		}
> +		attr.config |= 1 << bit;
> +	}
> +	attr.size = sizeof(attr);
> +	attr.type = type;
> +	attr.config1 = (uint64_t)(void *)name; /* kprobe_func or uprobe_path */
> +	attr.config2 = offset;		       /* kprobe_addr or probe_offset */
> +
> +	/* pid filter is meaningful only for uprobes */
> +	pfd = syscall(__NR_perf_event_open, &attr,
> +		      pid < 0 ? -1 : pid /* pid */,
> +		      pid == -1 ? 0 : -1 /* cpu */,
> +		      -1 /* group_fd */, PERF_FLAG_FD_CLOEXEC);
> +	if (pfd < 0) {
> +		err = -errno;
> +		pr_warning("%s perf_event_open() failed: %s\n",
> +			   uprobe ? "uprobe" : "kprobe",
> +			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> +		return err;
> +	}
> +	return pfd;
> +}
> +
> +struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog,
> +					    bool retprobe,
> +					    const char *func_name)
> +{
> +	char errmsg[STRERR_BUFSIZE];
> +	struct bpf_link *link;
> +	int pfd, err;
> +
> +	pfd = perf_event_open_probe(false /* uprobe */, retprobe, func_name,
> +				    0 /* offset */, -1 /* pid */);
> +	if (pfd < 0) {
> +		pr_warning("program '%s': failed to create %s '%s' perf event: %s\n",
> +			   bpf_program__title(prog, false),
> +			   retprobe ? "kretprobe" : "kprobe", func_name,
> +			   libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> +		return ERR_PTR(pfd);
> +	}
> +	link = bpf_program__attach_perf_event(prog, pfd);
> +	if (IS_ERR(link)) {
> +		close(pfd);
> +		err = PTR_ERR(link);
> +		pr_warning("program '%s': failed to attach to %s '%s': %s\n",
> +			   bpf_program__title(prog, false),
> +			   retprobe ? "kretprobe" : "kprobe", func_name,
> +			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> +		return link;
> +	}
> +	return link;
> +}
> +
> +struct bpf_link *bpf_program__attach_uprobe(struct bpf_program *prog,
> +					    bool retprobe, pid_t pid,
> +					    const char *binary_path,
> +					    size_t func_offset)
> +{
> +	char errmsg[STRERR_BUFSIZE];
> +	struct bpf_link *link;
> +	int pfd, err;
> +
> +	pfd = perf_event_open_probe(true /* uprobe */, retprobe,
> +				    binary_path, func_offset, pid);
> +	if (pfd < 0) {
> +		pr_warning("program '%s': failed to create %s '%s:0x%zx' perf event: %s\n",
> +			   bpf_program__title(prog, false),
> +			   retprobe ? "uretprobe" : "uprobe",
> +			   binary_path, func_offset,
> +			   libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> +		return ERR_PTR(pfd);
> +	}
> +	link = bpf_program__attach_perf_event(prog, pfd);
> +	if (IS_ERR(link)) {
> +		close(pfd);
> +		err = PTR_ERR(link);
> +		pr_warning("program '%s': failed to attach to %s '%s:0x%zx': %s\n",
> +			   bpf_program__title(prog, false),
> +			   retprobe ? "uretprobe" : "uprobe",
> +			   binary_path, func_offset,
> +			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> +		return link;
> +	}
> +	return link;
> +}
> +
>   enum bpf_perf_event_ret
>   bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
>   			   void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 1bf66c4a9330..bd767cc11967 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -171,6 +171,13 @@ LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
>   
>   LIBBPF_API struct bpf_link *
>   bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_kprobe(struct bpf_program *prog, bool retprobe,
> +			   const char *func_name);
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
> +			   pid_t pid, const char *binary_path,
> +			   size_t func_offset);
>   
>   struct bpf_insn;
>   
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 756f5aa802e9..57a40fb60718 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -169,7 +169,9 @@ LIBBPF_0.0.4 {
>   	global:
>   		bpf_link__destroy;
>   		bpf_object__load_xattr;
> +		bpf_program__attach_kprobe;
>   		bpf_program__attach_perf_event;
> +		bpf_program__attach_uprobe;
>   		btf_dump__dump_type;
>   		btf_dump__free;
>   		btf_dump__new;
> 

Re: [PATCH v4 bpf-next 5/9] libbpf: add tracepoint attach API

[Yonghong Song]

On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> Allow attaching BPF programs to kernel tracepoint BPF hooks specified by
> category and name.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> ---
>   tools/lib/bpf/libbpf.c   | 79 ++++++++++++++++++++++++++++++++++++++++
>   tools/lib/bpf/libbpf.h   |  4 ++
>   tools/lib/bpf/libbpf.map |  1 +
>   3 files changed, 84 insertions(+)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 2f79e9563db9..8ad4f915df38 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4184,6 +4184,85 @@ struct bpf_link *bpf_program__attach_uprobe(struct bpf_program *prog,
>   	return link;
>   }
>   
> +static int determine_tracepoint_id(const char *tp_category,
> +				   const char *tp_name)
> +{
> +	char file[PATH_MAX];
> +	int ret;
> +
> +	ret = snprintf(file, sizeof(file),
> +		       "/sys/kernel/debug/tracing/events/%s/%s/id",
> +		       tp_category, tp_name);
> +	if (ret < 0)
> +		return -errno;
> +	if (ret >= sizeof(file)) {
> +		pr_debug("tracepoint %s/%s path is too long\n",
> +			 tp_category, tp_name);
> +		return -E2BIG;
> +	}
> +	return parse_value_from_file(file, "%d\n");
> +}
> +
> +static int perf_event_open_tracepoint(const char *tp_category,
> +				      const char *tp_name)
> +{
> +	struct perf_event_attr attr = {};
> +	char errmsg[STRERR_BUFSIZE];
> +	int tp_id, pfd, err;
> +
> +	tp_id = determine_tracepoint_id(tp_category, tp_name);
> +	if (tp_id < 0) {
> +		pr_warning("failed to determine tracepoint '%s/%s' perf ID: %s\n",

nit: "perf ID" is not accurate. Maybe "event ID" or "perf event ID"?

> +			   tp_category, tp_name,
> +			   libbpf_strerror_r(tp_id, errmsg, sizeof(errmsg)));
> +		return tp_id;
> +	}
> +
> +	attr.type = PERF_TYPE_TRACEPOINT;
> +	attr.size = sizeof(attr);
> +	attr.config = tp_id;
> +
> +	pfd = syscall(__NR_perf_event_open, &attr, -1 /* pid */, 0 /* cpu */,
> +		      -1 /* group_fd */, PERF_FLAG_FD_CLOEXEC);
> +	if (pfd < 0) {
> +		err = -errno;
> +		pr_warning("tracepoint '%s/%s' perf_event_open() failed: %s\n",
> +			   tp_category, tp_name,
> +			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> +		return err;
> +	}
> +	return pfd;
> +}
> +
> +struct bpf_link *bpf_program__attach_tracepoint(struct bpf_program *prog,
> +						const char *tp_category,
> +						const char *tp_name)
> +{
> +	char errmsg[STRERR_BUFSIZE];
> +	struct bpf_link *link;
> +	int pfd, err;
> +
> +	pfd = perf_event_open_tracepoint(tp_category, tp_name);
> +	if (pfd < 0) {
> +		pr_warning("program '%s': failed to create tracepoint '%s/%s' perf event: %s\n",
> +			   bpf_program__title(prog, false),
> +			   tp_category, tp_name,
> +			   libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> +		return ERR_PTR(pfd);
> +	}
> +	link = bpf_program__attach_perf_event(prog, pfd);
> +	if (IS_ERR(link)) {
> +		close(pfd);
> +		err = PTR_ERR(link);
> +		pr_warning("program '%s': failed to attach to tracepoint '%s/%s': %s\n",
> +			   bpf_program__title(prog, false),
> +			   tp_category, tp_name,
> +			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> +		return link;
> +	}
> +	return link;
> +}
> +
>   enum bpf_perf_event_ret
>   bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
>   			   void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index bd767cc11967..60611f4b4e1d 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -178,6 +178,10 @@ LIBBPF_API struct bpf_link *
>   bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
>   			   pid_t pid, const char *binary_path,
>   			   size_t func_offset);
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_tracepoint(struct bpf_program *prog,
> +			       const char *tp_category,
> +			       const char *tp_name);
>   
>   struct bpf_insn;
>   
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 57a40fb60718..3c618b75ef65 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -171,6 +171,7 @@ LIBBPF_0.0.4 {
>   		bpf_object__load_xattr;
>   		bpf_program__attach_kprobe;
>   		bpf_program__attach_perf_event;
> +		bpf_program__attach_tracepoint;
>   		bpf_program__attach_uprobe;
>   		btf_dump__dump_type;
>   		btf_dump__free;
> 

Re: [PATCH v4 bpf-next 6/9] libbpf: add raw tracepoint attach API

[Yonghong Song]

On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> Add a wrapper utilizing bpf_link "infrastructure" to allow attaching BPF
> programs to raw tracepoints.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> ---
>   tools/lib/bpf/libbpf.c   | 37 +++++++++++++++++++++++++++++++++++++
>   tools/lib/bpf/libbpf.h   |  3 +++
>   tools/lib/bpf/libbpf.map |  1 +
>   3 files changed, 41 insertions(+)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 8ad4f915df38..f8c7a7ecb35e 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4263,6 +4263,43 @@ struct bpf_link *bpf_program__attach_tracepoint(struct bpf_program *prog,
>   	return link;
>   }
>   
> +static int bpf_link__destroy_fd(struct bpf_link *link)
> +{
> +	struct bpf_link_fd *l = (void *)link;
> +
> +	return close(l->fd);
> +}
> +
> +struct bpf_link *bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
> +						    const char *tp_name)
> +{
> +	char errmsg[STRERR_BUFSIZE];
> +	struct bpf_link_fd *link;
> +	int prog_fd, pfd;
> +
> +	prog_fd = bpf_program__fd(prog);
> +	if (prog_fd < 0) {
> +		pr_warning("program '%s': can't attach before loaded\n",
> +			   bpf_program__title(prog, false));
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	link = malloc(sizeof(*link));
> +	link->link.destroy = &bpf_link__destroy_fd;

You can move the "link = malloc(...)" etc. after 
bpf_raw_tracepoint_open(). That way, you do not need to free(link)
in the error case.

> +
> +	pfd = bpf_raw_tracepoint_open(tp_name, prog_fd);
> +	if (pfd < 0) {
> +		pfd = -errno;
> +		free(link);
> +		pr_warning("program '%s': failed to attach to raw tracepoint '%s': %s\n",
> +			   bpf_program__title(prog, false), tp_name,
> +			   libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> +		return ERR_PTR(pfd);
> +	}
> +	link->fd = pfd;
> +	return (struct bpf_link *)link;
> +}
> +
>   enum bpf_perf_event_ret
>   bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
>   			   void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 60611f4b4e1d..f55933784f95 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -182,6 +182,9 @@ LIBBPF_API struct bpf_link *
>   bpf_program__attach_tracepoint(struct bpf_program *prog,
>   			       const char *tp_category,
>   			       const char *tp_name);
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
> +				   const char *tp_name);
>   
>   struct bpf_insn;
>   
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 3c618b75ef65..e6b7d4edbc93 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -171,6 +171,7 @@ LIBBPF_0.0.4 {
>   		bpf_object__load_xattr;
>   		bpf_program__attach_kprobe;
>   		bpf_program__attach_perf_event;
> +		bpf_program__attach_raw_tracepoint;
>   		bpf_program__attach_tracepoint;
>   		bpf_program__attach_uprobe;
>   		btf_dump__dump_type;
> 

Re: [PATCH v4 bpf-next 7/9] selftests/bpf: switch test to new attach_perf_event API

[Yonghong Song]

On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> Use new bpf_program__attach_perf_event() in test previously relying on
> direct ioctl manipulations.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> Reviewed-by: Stanislav Fomichev <sdf@google.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> ---
>   .../bpf/prog_tests/stacktrace_build_id_nmi.c  | 31 +++++++++----------
>   1 file changed, 15 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
> index 1c1a2f75f3d8..9557b7dfb782 100644
> --- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
> +++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
> @@ -17,6 +17,7 @@ static __u64 read_perf_max_sample_freq(void)
>   void test_stacktrace_build_id_nmi(void)
>   {
>   	int control_map_fd, stackid_hmap_fd, stackmap_fd, stack_amap_fd;
> +	const char *prog_name = "tracepoint/random/urandom_read";
>   	const char *file = "./test_stacktrace_build_id.o";
>   	int err, pmu_fd, prog_fd;
>   	struct perf_event_attr attr = {
> @@ -25,7 +26,9 @@ void test_stacktrace_build_id_nmi(void)
>   		.config = PERF_COUNT_HW_CPU_CYCLES,
>   	};
>   	__u32 key, previous_key, val, duration = 0;
> +	struct bpf_program *prog;
>   	struct bpf_object *obj;
> +	struct bpf_link *link;
>   	char buf[256];
>   	int i, j;
>   	struct bpf_stack_build_id id_offs[PERF_MAX_STACK_DEPTH];
> @@ -39,6 +42,10 @@ void test_stacktrace_build_id_nmi(void)
>   	if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno))
>   		return;
>   
> +	prog = bpf_object__find_program_by_title(obj, prog_name);
> +	if (CHECK(!prog, "find_prog", "prog '%s' not found\n", prog_name))
> +		goto close_prog;
> +
>   	pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */,
>   			 0 /* cpu 0 */, -1 /* group id */,
>   			 0 /* flags */);
> @@ -47,15 +54,12 @@ void test_stacktrace_build_id_nmi(void)
>   		  pmu_fd, errno))
>   		goto close_prog;
>   
> -	err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
> -	if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n",
> -		  err, errno))
> -		goto close_pmu;
> -
> -	err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
> -	if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n",
> -		  err, errno))
> -		goto disable_pmu;
> +	link = bpf_program__attach_perf_event(prog, pmu_fd);
> +	if (CHECK(IS_ERR(link), "attach_perf_event",
> +		  "err %ld\n", PTR_ERR(link))) {
> +		close(pmu_fd);
> +		goto close_prog;
> +	}
>   
>   	/* find map fds */
>   	control_map_fd = bpf_find_map(__func__, obj, "control_map");
> @@ -134,8 +138,7 @@ void test_stacktrace_build_id_nmi(void)
>   	 * try it one more time.
>   	 */
>   	if (build_id_matches < 1 && retry--) {
> -		ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
> -		close(pmu_fd);
> +		bpf_link__destroy(link);
>   		bpf_object__close(obj);
>   		printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
>   		       __func__);
> @@ -154,11 +157,7 @@ void test_stacktrace_build_id_nmi(void)
>   	 */
>   
>   disable_pmu:
> -	ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
> -
> -close_pmu:
> -	close(pmu_fd);
> -
> +	bpf_link__destroy(link);

There is a problem in bpf_link__destroy(link).
The "link = bpf_program__attach_perf_event(prog, pmu_fd)"
may be an error pointer (IS_ERR(link) is true), in which
case, link should be reset to NULL and then call 
bpf_link__destroy(link). Otherwise, the program may
segfault or function incorrectly.

>   close_prog:
>   	bpf_object__close(obj);
>   }
> 

Re: [PATCH v4 bpf-next 8/9] selftests/bpf: add kprobe/uprobe selftests

[Yonghong Song]

On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> Add tests verifying kprobe/kretprobe/uprobe/uretprobe APIs work as
> expected.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> Reviewed-by: Stanislav Fomichev <sdf@google.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> ---
>   .../selftests/bpf/prog_tests/attach_probe.c   | 155 ++++++++++++++++++
>   .../selftests/bpf/progs/test_attach_probe.c   |  55 +++++++
>   2 files changed, 210 insertions(+)
>   create mode 100644 tools/testing/selftests/bpf/prog_tests/attach_probe.c
>   create mode 100644 tools/testing/selftests/bpf/progs/test_attach_probe.c
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> new file mode 100644
> index 000000000000..f22929063c58
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> @@ -0,0 +1,155 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <test_progs.h>
> +
> +ssize_t get_base_addr() {
> +	size_t start;
> +	char buf[256];
> +	FILE *f;
> +
> +	f = fopen("/proc/self/maps", "r");
> +	if (!f)
> +		return -errno;
> +
> +	while (fscanf(f, "%zx-%*x %s %*s\n", &start, buf) == 2) {
> +		if (strcmp(buf, "r-xp") == 0) {
> +			fclose(f);
> +			return start;
> +		}
> +	}
> +
> +	fclose(f);
> +	return -EINVAL;
> +}
> +
> +void test_attach_probe(void)
> +{
> +	const char *kprobe_name = "kprobe/sys_nanosleep";
> +	const char *kretprobe_name = "kretprobe/sys_nanosleep";
> +	const char *uprobe_name = "uprobe/trigger_func";
> +	const char *uretprobe_name = "uretprobe/trigger_func";
> +	const int kprobe_idx = 0, kretprobe_idx = 1;
> +	const int uprobe_idx = 2, uretprobe_idx = 3;
> +	const char *file = "./test_attach_probe.o";
> +	struct bpf_program *kprobe_prog, *kretprobe_prog;
> +	struct bpf_program *uprobe_prog, *uretprobe_prog;
> +	struct bpf_object *obj;
> +	int err, prog_fd, duration = 0, res;
> +	struct bpf_link *kprobe_link = NULL;
> +	struct bpf_link *kretprobe_link = NULL;
> +	struct bpf_link *uprobe_link = NULL;
> +	struct bpf_link *uretprobe_link = NULL;
> +	int results_map_fd;
> +	size_t uprobe_offset;
> +	ssize_t base_addr;
> +
> +	base_addr = get_base_addr();
> +	if (CHECK(base_addr < 0, "get_base_addr",
> +		  "failed to find base addr: %zd", base_addr))
> +		return;
> +	uprobe_offset = (size_t)&get_base_addr - base_addr;
> +
> +	/* load programs */
> +	err = bpf_prog_load(file, BPF_PROG_TYPE_KPROBE, &obj, &prog_fd);
> +	if (CHECK(err, "obj_load", "err %d errno %d\n", err, errno))
> +		return;
> +
> +	kprobe_prog = bpf_object__find_program_by_title(obj, kprobe_name);
> +	if (CHECK(!kprobe_prog, "find_probe",
> +		  "prog '%s' not found\n", kprobe_name))
> +		goto cleanup;
> +	kretprobe_prog = bpf_object__find_program_by_title(obj, kretprobe_name);
> +	if (CHECK(!kretprobe_prog, "find_probe",
> +		  "prog '%s' not found\n", kretprobe_name))
> +		goto cleanup;
> +	uprobe_prog = bpf_object__find_program_by_title(obj, uprobe_name);
> +	if (CHECK(!uprobe_prog, "find_probe",
> +		  "prog '%s' not found\n", uprobe_name))
> +		goto cleanup;
> +	uretprobe_prog = bpf_object__find_program_by_title(obj, uretprobe_name);
> +	if (CHECK(!uretprobe_prog, "find_probe",
> +		  "prog '%s' not found\n", uretprobe_name))
> +		goto cleanup;
> +
> +	/* load maps */
> +	results_map_fd = bpf_find_map(__func__, obj, "results_map");
> +	if (CHECK(results_map_fd < 0, "find_results_map",
> +		  "err %d\n", results_map_fd))
> +		goto cleanup;
> +
> +	kprobe_link = bpf_program__attach_kprobe(kprobe_prog,
> +						 false /* retprobe */,
> +						 "sys_nanosleep");
> +	if (CHECK(IS_ERR(kprobe_link), "attach_kprobe",
> +		  "err %ld\n", PTR_ERR(kprobe_link)))
> +		goto cleanup;
> +
> +	kretprobe_link = bpf_program__attach_kprobe(kretprobe_prog,
> +						    true /* retprobe */,
> +						    "sys_nanosleep");
> +	if (CHECK(IS_ERR(kretprobe_link), "attach_kretprobe",
> +		  "err %ld\n", PTR_ERR(kretprobe_link)))
> +		goto cleanup;
> +
> +	uprobe_link = bpf_program__attach_uprobe(uprobe_prog,
> +						 false /* retprobe */,
> +						 0 /* self pid */,
> +						 "/proc/self/exe",
> +						 uprobe_offset);
> +	if (CHECK(IS_ERR(uprobe_link), "attach_uprobe",
> +		  "err %ld\n", PTR_ERR(uprobe_link)))
> +		goto cleanup;
> +
> +	uretprobe_link = bpf_program__attach_uprobe(uretprobe_prog,
> +						    true /* retprobe */,
> +						    -1 /* any pid */,
> +						    "/proc/self/exe",
> +						    uprobe_offset);
> +	if (CHECK(IS_ERR(uretprobe_link), "attach_uretprobe",
> +		  "err %ld\n", PTR_ERR(uretprobe_link)))
> +		goto cleanup;
> +
> +	/* trigger & validate kprobe && kretprobe */
> +	usleep(1);
> +
> +	err = bpf_map_lookup_elem(results_map_fd, &kprobe_idx, &res);
> +	if (CHECK(err, "get_kprobe_res",
> +		  "failed to get kprobe res: %d\n", err))
> +		goto cleanup;
> +	if (CHECK(res != kprobe_idx + 1, "check_kprobe_res",
> +		  "wrong kprobe res: %d\n", res))
> +		goto cleanup;
> +
> +	err = bpf_map_lookup_elem(results_map_fd, &kretprobe_idx, &res);
> +	if (CHECK(err, "get_kretprobe_res",
> +		  "failed to get kretprobe res: %d\n", err))
> +		goto cleanup;
> +	if (CHECK(res != kretprobe_idx + 1, "check_kretprobe_res",
> +		  "wrong kretprobe res: %d\n", res))
> +		goto cleanup;
> +
> +	/* trigger & validate uprobe & uretprobe */
> +	get_base_addr();
> +
> +	err = bpf_map_lookup_elem(results_map_fd, &uprobe_idx, &res);
> +	if (CHECK(err, "get_uprobe_res",
> +		  "failed to get uprobe res: %d\n", err))
> +		goto cleanup;
> +	if (CHECK(res != uprobe_idx + 1, "check_uprobe_res",
> +		  "wrong uprobe res: %d\n", res))
> +		goto cleanup;
> +
> +	err = bpf_map_lookup_elem(results_map_fd, &uretprobe_idx, &res);
> +	if (CHECK(err, "get_uretprobe_res",
> +		  "failed to get uretprobe res: %d\n", err))
> +		goto cleanup;
> +	if (CHECK(res != uretprobe_idx + 1, "check_uretprobe_res",
> +		  "wrong uretprobe res: %d\n", res))
> +		goto cleanup;
> +
> +cleanup:
> +	bpf_link__destroy(kprobe_link);
> +	bpf_link__destroy(kretprobe_link);
> +	bpf_link__destroy(uprobe_link);
> +	bpf_link__destroy(uretprobe_link);

if any error happens, kprobe_link etc. will be a non-NULL pointer
indicating an error. the above bpf_link__destroy() won't work
properly. The same for patch #9.

> +	bpf_object__close(obj);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_attach_probe.c b/tools/testing/selftests/bpf/progs/test_attach_probe.c
> new file mode 100644
> index 000000000000..7a7c5cd728c8
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_attach_probe.c
> @@ -0,0 +1,55 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2017 Facebook
> +
> +#include <linux/ptrace.h>
> +#include <linux/bpf.h>
> +#include "bpf_helpers.h"
> +
> +struct {
> +	int type;
> +	int max_entries;
> +	int *key;
> +	int *value;
> +} results_map SEC(".maps") = {
> +	.type = BPF_MAP_TYPE_ARRAY,
> +	.max_entries = 4,
> +};
> +
> +SEC("kprobe/sys_nanosleep")
> +int handle_sys_nanosleep_entry(struct pt_regs *ctx)
> +{
> +	const int key = 0, value = 1;
> +
> +	bpf_map_update_elem(&results_map, &key, &value, 0);
> +	return 0;
> +}
> +
> +SEC("kretprobe/sys_nanosleep")
> +int handle_sys_getpid_return(struct pt_regs *ctx)
> +{
> +	const int key = 1, value = 2;
> +
> +	bpf_map_update_elem(&results_map, &key, &value, 0);
> +	return 0;
> +}
> +
> +SEC("uprobe/trigger_func")
> +int handle_uprobe_entry(struct pt_regs *ctx)
> +{
> +	const int key = 2, value = 3;
> +
> +	bpf_map_update_elem(&results_map, &key, &value, 0);
> +	return 0;
> +}
> +
> +SEC("uretprobe/trigger_func")
> +int handle_uprobe_return(struct pt_regs *ctx)
> +{
> +	const int key = 3, value = 4;
> +
> +	bpf_map_update_elem(&results_map, &key, &value, 0);
> +	return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> +__u32 _version SEC("version") = 1;
> 

Re: [PATCH v4 bpf-next 8/9] selftests/bpf: add kprobe/uprobe selftests

[Yonghong Song]

On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> Add tests verifying kprobe/kretprobe/uprobe/uretprobe APIs work as
> expected.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> Reviewed-by: Stanislav Fomichev <sdf@google.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> ---
>   .../selftests/bpf/prog_tests/attach_probe.c   | 155 ++++++++++++++++++
>   .../selftests/bpf/progs/test_attach_probe.c   |  55 +++++++
>   2 files changed, 210 insertions(+)
>   create mode 100644 tools/testing/selftests/bpf/prog_tests/attach_probe.c
>   create mode 100644 tools/testing/selftests/bpf/progs/test_attach_probe.c
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> new file mode 100644
> index 000000000000..f22929063c58
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> @@ -0,0 +1,155 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <test_progs.h>
> +
> +ssize_t get_base_addr() {
> +	size_t start;
> +	char buf[256];
> +	FILE *f;
> +
> +	f = fopen("/proc/self/maps", "r");
> +	if (!f)
> +		return -errno;
> +
> +	while (fscanf(f, "%zx-%*x %s %*s\n", &start, buf) == 2) {
> +		if (strcmp(buf, "r-xp") == 0) {
> +			fclose(f);
> +			return start;
> +		}
> +	}
> +
> +	fclose(f);
> +	return -EINVAL;
> +}
> +
> +void test_attach_probe(void)
> +{
> +	const char *kprobe_name = "kprobe/sys_nanosleep";
> +	const char *kretprobe_name = "kretprobe/sys_nanosleep";
> +	const char *uprobe_name = "uprobe/trigger_func";
> +	const char *uretprobe_name = "uretprobe/trigger_func";
> +	const int kprobe_idx = 0, kretprobe_idx = 1;
> +	const int uprobe_idx = 2, uretprobe_idx = 3;
> +	const char *file = "./test_attach_probe.o";
> +	struct bpf_program *kprobe_prog, *kretprobe_prog;
> +	struct bpf_program *uprobe_prog, *uretprobe_prog;
> +	struct bpf_object *obj;
> +	int err, prog_fd, duration = 0, res;
> +	struct bpf_link *kprobe_link = NULL;
> +	struct bpf_link *kretprobe_link = NULL;
> +	struct bpf_link *uprobe_link = NULL;
> +	struct bpf_link *uretprobe_link = NULL;
> +	int results_map_fd;
> +	size_t uprobe_offset;
> +	ssize_t base_addr;
> +
> +	base_addr = get_base_addr();
> +	if (CHECK(base_addr < 0, "get_base_addr",
> +		  "failed to find base addr: %zd", base_addr))
> +		return;
> +	uprobe_offset = (size_t)&get_base_addr - base_addr;
> +
> +	/* load programs */
> +	err = bpf_prog_load(file, BPF_PROG_TYPE_KPROBE, &obj, &prog_fd);
> +	if (CHECK(err, "obj_load", "err %d errno %d\n", err, errno))
> +		return;
> +
> +	kprobe_prog = bpf_object__find_program_by_title(obj, kprobe_name);
> +	if (CHECK(!kprobe_prog, "find_probe",
> +		  "prog '%s' not found\n", kprobe_name))
> +		goto cleanup;
> +	kretprobe_prog = bpf_object__find_program_by_title(obj, kretprobe_name);
> +	if (CHECK(!kretprobe_prog, "find_probe",
> +		  "prog '%s' not found\n", kretprobe_name))
> +		goto cleanup;
> +	uprobe_prog = bpf_object__find_program_by_title(obj, uprobe_name);
> +	if (CHECK(!uprobe_prog, "find_probe",
> +		  "prog '%s' not found\n", uprobe_name))
> +		goto cleanup;
> +	uretprobe_prog = bpf_object__find_program_by_title(obj, uretprobe_name);
> +	if (CHECK(!uretprobe_prog, "find_probe",
> +		  "prog '%s' not found\n", uretprobe_name))
> +		goto cleanup;
> +
> +	/* load maps */
> +	results_map_fd = bpf_find_map(__func__, obj, "results_map");
> +	if (CHECK(results_map_fd < 0, "find_results_map",
> +		  "err %d\n", results_map_fd))
> +		goto cleanup;
> +
> +	kprobe_link = bpf_program__attach_kprobe(kprobe_prog,
> +						 false /* retprobe */,
> +						 "sys_nanosleep");

Another thing, in current kernel, `sys_nanosleep`does not exist
on x64. It is `__x64_sys_nanosleep`. See samples/bpf/bpf_load.c
function load_and_attach(). You can use macros to differentiate
different architectures.

> +	if (CHECK(IS_ERR(kprobe_link), "attach_kprobe",
> +		  "err %ld\n", PTR_ERR(kprobe_link)))
> +		goto cleanup;
> +
> +	kretprobe_link = bpf_program__attach_kprobe(kretprobe_prog,
> +						    true /* retprobe */,
> +						    "sys_nanosleep");
> +	if (CHECK(IS_ERR(kretprobe_link), "attach_kretprobe",
> +		  "err %ld\n", PTR_ERR(kretprobe_link)))
> +		goto cleanup;
> +
> +	uprobe_link = bpf_program__attach_uprobe(uprobe_prog,
> +						 false /* retprobe */,
> +						 0 /* self pid */,
> +						 "/proc/self/exe",
> +						 uprobe_offset);
> +	if (CHECK(IS_ERR(uprobe_link), "attach_uprobe",
> +		  "err %ld\n", PTR_ERR(uprobe_link)))
> +		goto cleanup;
> +
> +	uretprobe_link = bpf_program__attach_uprobe(uretprobe_prog,
> +						    true /* retprobe */,
> +						    -1 /* any pid */,
> +						    "/proc/self/exe",
> +						    uprobe_offset);
> +	if (CHECK(IS_ERR(uretprobe_link), "attach_uretprobe",
> +		  "err %ld\n", PTR_ERR(uretprobe_link)))
> +		goto cleanup;
> +
> +	/* trigger & validate kprobe && kretprobe */
> +	usleep(1);
> +
> +	err = bpf_map_lookup_elem(results_map_fd, &kprobe_idx, &res);
> +	if (CHECK(err, "get_kprobe_res",
> +		  "failed to get kprobe res: %d\n", err))
> +		goto cleanup;
> +	if (CHECK(res != kprobe_idx + 1, "check_kprobe_res",
> +		  "wrong kprobe res: %d\n", res))
> +		goto cleanup;
> +
> +	err = bpf_map_lookup_elem(results_map_fd, &kretprobe_idx, &res);
> +	if (CHECK(err, "get_kretprobe_res",
> +		  "failed to get kretprobe res: %d\n", err))
> +		goto cleanup;
> +	if (CHECK(res != kretprobe_idx + 1, "check_kretprobe_res",
> +		  "wrong kretprobe res: %d\n", res))
> +		goto cleanup;
> +
> +	/* trigger & validate uprobe & uretprobe */
> +	get_base_addr();
> +
> +	err = bpf_map_lookup_elem(results_map_fd, &uprobe_idx, &res);
> +	if (CHECK(err, "get_uprobe_res",
> +		  "failed to get uprobe res: %d\n", err))
> +		goto cleanup;
> +	if (CHECK(res != uprobe_idx + 1, "check_uprobe_res",
> +		  "wrong uprobe res: %d\n", res))
> +		goto cleanup;
> +
> +	err = bpf_map_lookup_elem(results_map_fd, &uretprobe_idx, &res);
> +	if (CHECK(err, "get_uretprobe_res",
> +		  "failed to get uretprobe res: %d\n", err))
> +		goto cleanup;
> +	if (CHECK(res != uretprobe_idx + 1, "check_uretprobe_res",
> +		  "wrong uretprobe res: %d\n", res))
> +		goto cleanup;
> +
> +cleanup:
> +	bpf_link__destroy(kprobe_link);
> +	bpf_link__destroy(kretprobe_link);
> +	bpf_link__destroy(uprobe_link);
> +	bpf_link__destroy(uretprobe_link);
> +	bpf_object__close(obj);
> +}
[...]

Re: [PATCH v4 bpf-next 3/9] libbpf: add ability to attach/detach BPF program to perf event

[Andrii Nakryiko]

On Mon, Jul 1, 2019 at 10:03 AM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> > bpf_program__attach_perf_event allows to attach BPF program to existing
> > perf event hook, providing most generic and most low-level way to attach BPF
> > programs. It returns struct bpf_link, which should be passed to
> > bpf_link__destroy to detach and free resources, associated with a link.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> >   tools/lib/bpf/libbpf.c   | 61 ++++++++++++++++++++++++++++++++++++++++
> >   tools/lib/bpf/libbpf.h   |  3 ++
> >   tools/lib/bpf/libbpf.map |  1 +
> >   3 files changed, 65 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 455795e6f8af..98c155ec3bfa 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -32,6 +32,7 @@
> >   #include <linux/limits.h>
> >   #include <linux/perf_event.h>
> >   #include <linux/ring_buffer.h>
> > +#include <sys/ioctl.h>
> >   #include <sys/stat.h>
> >   #include <sys/types.h>
> >   #include <sys/vfs.h>
> > @@ -3958,6 +3959,66 @@ int bpf_link__destroy(struct bpf_link *link)
> >       return err;
> >   }
> >
> > +struct bpf_link_fd {
> > +     struct bpf_link link; /* has to be at the top of struct */
> > +     int fd; /* hook FD */
> > +};
> > +
> > +static int bpf_link__destroy_perf_event(struct bpf_link *link)
> > +{
> > +     struct bpf_link_fd *l = (void *)link;
> > +     int err;
> > +
> > +     if (l->fd < 0)
> > +             return 0;
> > +
> > +     err = ioctl(l->fd, PERF_EVENT_IOC_DISABLE, 0);
> > +     if (err)
> > +             err = -errno;
> > +
> > +     close(l->fd);
> > +     return err;
> > +}
> > +
> > +struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
> > +                                             int pfd)
> > +{
> > +     char errmsg[STRERR_BUFSIZE];
> > +     struct bpf_link_fd *link;
> > +     int prog_fd, err;
> > +
> > +     prog_fd = bpf_program__fd(prog);
> > +     if (prog_fd < 0) {
> > +             pr_warning("program '%s': can't attach before loaded\n",
> > +                        bpf_program__title(prog, false));
> > +             return ERR_PTR(-EINVAL);
> > +     }
>
> should we check validity of pfd here?
> If pfd < 0, we just return ERR_PTR(-EINVAL)?

I can add that. I didn't do it, because in general, you can provide fd
>= 0 which is still not a valid FD for PERF_EVENT_IOC_SET_BPF and
PERF_EVENT_IOC_ENABLE, so in general we can't detect this reliably.

> This way, in bpf_link__destroy_perf_event(), we do not need to check
> l->fd < 0 since it will be always nonnegative.

That check is not needed anyway, because even if pfd < 0, ioctl should
fail and return error. I'll remove that check.

>
> > +
> > +     link = malloc(sizeof(*link));
> > +     if (!link)
> > +             return ERR_PTR(-ENOMEM);
> > +     link->link.destroy = &bpf_link__destroy_perf_event;
> > +     link->fd = pfd;
> > +
> > +     if (ioctl(pfd, PERF_EVENT_IOC_SET_BPF, prog_fd) < 0) {
> > +             err = -errno;
> > +             free(link);
> > +             pr_warning("program '%s': failed to attach to pfd %d: %s\n",
> > +                        bpf_program__title(prog, false), pfd,
> > +                        libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> > +             return ERR_PTR(err);
> > +     }
> > +     if (ioctl(pfd, PERF_EVENT_IOC_ENABLE, 0) < 0) {
> > +             err = -errno;
> > +             free(link);
> > +             pr_warning("program '%s': failed to enable pfd %d: %s\n",
> > +                        bpf_program__title(prog, false), pfd,
> > +                        libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> > +             return ERR_PTR(err);
> > +     }
> > +     return (struct bpf_link *)link;
> > +}
> > +
> >   enum bpf_perf_event_ret
> >   bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
> >                          void **copy_mem, size_t *copy_size,
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index 5082a5ebb0c2..1bf66c4a9330 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -169,6 +169,9 @@ struct bpf_link;
> >
> >   LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
> >
> > +LIBBPF_API struct bpf_link *
> > +bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
> > +
> >   struct bpf_insn;
> >
> >   /*
> > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > index 3cde850fc8da..756f5aa802e9 100644
> > --- a/tools/lib/bpf/libbpf.map
> > +++ b/tools/lib/bpf/libbpf.map
> > @@ -169,6 +169,7 @@ LIBBPF_0.0.4 {
> >       global:
> >               bpf_link__destroy;
> >               bpf_object__load_xattr;
> > +             bpf_program__attach_perf_event;
> >               btf_dump__dump_type;
> >               btf_dump__free;
> >               btf_dump__new;
> >

Re: [PATCH v4 bpf-next 4/9] libbpf: add kprobe/uprobe attach API

[Andrii Nakryiko]

On Mon, Jul 1, 2019 at 10:09 AM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> > Add ability to attach to kernel and user probes and retprobes.
> > Implementation depends on perf event support for kprobes/uprobes.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> >   tools/lib/bpf/libbpf.c   | 165 +++++++++++++++++++++++++++++++++++++++
> >   tools/lib/bpf/libbpf.h   |   7 ++
> >   tools/lib/bpf/libbpf.map |   2 +
> >   3 files changed, 174 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 98c155ec3bfa..2f79e9563db9 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -4019,6 +4019,171 @@ struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
> >       return (struct bpf_link *)link;
> >   }
> >
> > +static int parse_value_from_file(const char *file, const char *fmt)
>
> Here, the value from the file must be positive int values to avoid
> confusion between valid value vs. error code.
> Could you add a comment to state this fact for the current
> uprobe/kprobe/tracepoint support?

You are right. I'll name it parse_uint_from_file to make it more
obvious (though even that is not strictly true, we expect value in the
range of [0, 2^32-1]. I'll leave a comment.

>
> > +{
> > +     char buf[STRERR_BUFSIZE];
> > +     int err, ret;
> > +     FILE *f;
> > +
> > +     f = fopen(file, "r");
> > +     if (!f) {
> > +             err = -errno;
> > +             pr_debug("failed to open '%s': %s\n", file,
> > +                      libbpf_strerror_r(err, buf, sizeof(buf)));
> > +             fclose(f);
>
> fclose(f) is not needed. fopen has failed.

*facepalm*, of course, thanks!

>
> > +             return err;
> > +     }
> > +     err = fscanf(f, fmt, &ret);
> > +     if (err != 1) {
> > +             err = err == EOF ? -EIO : -errno;
> > +             pr_debug("failed to parse '%s': %s\n", file,
> > +                     libbpf_strerror_r(err, buf, sizeof(buf)));
> > +             fclose(f);
> > +             return err;
> > +     }
> > +     fclose(f);
> > +     return ret;
> > +}
> > +
> > +static int determine_kprobe_perf_type(void)
> > +{
> > +     const char *file = "/sys/bus/event_source/devices/kprobe/type";
> > +
> > +     return parse_value_from_file(file, "%d\n");
> > +}
> > +
> > +static int determine_uprobe_perf_type(void)
> > +{
> > +     const char *file = "/sys/bus/event_source/devices/uprobe/type";
> > +
> > +     return parse_value_from_file(file, "%d\n");
> > +}
> > +
> > +static int determine_kprobe_retprobe_bit(void)
> > +{
> > +     const char *file = "/sys/bus/event_source/devices/kprobe/format/retprobe";
> > +
> > +     return parse_value_from_file(file, "config:%d\n");
> > +}
> > +
> > +static int determine_uprobe_retprobe_bit(void)
> > +{
> > +     const char *file = "/sys/bus/event_source/devices/uprobe/format/retprobe";
> > +
> > +     return parse_value_from_file(file, "config:%d\n");
> > +}
> > +
> > +static int perf_event_open_probe(bool uprobe, bool retprobe, const char *name,
> > +                              uint64_t offset, int pid)
> > +{
> > +     struct perf_event_attr attr = {};
> > +     char errmsg[STRERR_BUFSIZE];
> > +     int type, pfd, err;
> > +
> > +     type = uprobe ? determine_uprobe_perf_type()
> > +                   : determine_kprobe_perf_type();
> > +     if (type < 0) {
> > +             pr_warning("failed to determine %s perf type: %s\n",
> > +                        uprobe ? "uprobe" : "kprobe",
> > +                        libbpf_strerror_r(type, errmsg, sizeof(errmsg)));
> > +             return type;
> > +     }
> > +     if (retprobe) {
> > +             int bit = uprobe ? determine_uprobe_retprobe_bit()
> > +                              : determine_kprobe_retprobe_bit();
> > +
> > +             if (bit < 0) {
> > +                     pr_warning("failed to determine %s retprobe bit: %s\n",
> > +                                uprobe ? "uprobe" : "kprobe",
> > +                                libbpf_strerror_r(bit, errmsg,
> > +                                                  sizeof(errmsg)));
> > +                     return bit;
> > +             }
> > +             attr.config |= 1 << bit;
> > +     }
> > +     attr.size = sizeof(attr);
> > +     attr.type = type;
> > +     attr.config1 = (uint64_t)(void *)name; /* kprobe_func or uprobe_path */
> > +     attr.config2 = offset;                 /* kprobe_addr or probe_offset */
> > +
> > +     /* pid filter is meaningful only for uprobes */
> > +     pfd = syscall(__NR_perf_event_open, &attr,
> > +                   pid < 0 ? -1 : pid /* pid */,
> > +                   pid == -1 ? 0 : -1 /* cpu */,
> > +                   -1 /* group_fd */, PERF_FLAG_FD_CLOEXEC);
> > +     if (pfd < 0) {
> > +             err = -errno;
> > +             pr_warning("%s perf_event_open() failed: %s\n",
> > +                        uprobe ? "uprobe" : "kprobe",
> > +                        libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> > +             return err;
> > +     }
> > +     return pfd;
> > +}
> > +
> > +struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog,
> > +                                         bool retprobe,
> > +                                         const char *func_name)
> > +{
> > +     char errmsg[STRERR_BUFSIZE];
> > +     struct bpf_link *link;
> > +     int pfd, err;
> > +
> > +     pfd = perf_event_open_probe(false /* uprobe */, retprobe, func_name,
> > +                                 0 /* offset */, -1 /* pid */);
> > +     if (pfd < 0) {
> > +             pr_warning("program '%s': failed to create %s '%s' perf event: %s\n",
> > +                        bpf_program__title(prog, false),
> > +                        retprobe ? "kretprobe" : "kprobe", func_name,
> > +                        libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> > +             return ERR_PTR(pfd);
> > +     }
> > +     link = bpf_program__attach_perf_event(prog, pfd);
> > +     if (IS_ERR(link)) {
> > +             close(pfd);
> > +             err = PTR_ERR(link);
> > +             pr_warning("program '%s': failed to attach to %s '%s': %s\n",
> > +                        bpf_program__title(prog, false),
> > +                        retprobe ? "kretprobe" : "kprobe", func_name,
> > +                        libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> > +             return link;
> > +     }
> > +     return link;
> > +}
> > +
> > +struct bpf_link *bpf_program__attach_uprobe(struct bpf_program *prog,
> > +                                         bool retprobe, pid_t pid,
> > +                                         const char *binary_path,
> > +                                         size_t func_offset)
> > +{
> > +     char errmsg[STRERR_BUFSIZE];
> > +     struct bpf_link *link;
> > +     int pfd, err;
> > +
> > +     pfd = perf_event_open_probe(true /* uprobe */, retprobe,
> > +                                 binary_path, func_offset, pid);
> > +     if (pfd < 0) {
> > +             pr_warning("program '%s': failed to create %s '%s:0x%zx' perf event: %s\n",
> > +                        bpf_program__title(prog, false),
> > +                        retprobe ? "uretprobe" : "uprobe",
> > +                        binary_path, func_offset,
> > +                        libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> > +             return ERR_PTR(pfd);
> > +     }
> > +     link = bpf_program__attach_perf_event(prog, pfd);
> > +     if (IS_ERR(link)) {
> > +             close(pfd);
> > +             err = PTR_ERR(link);
> > +             pr_warning("program '%s': failed to attach to %s '%s:0x%zx': %s\n",
> > +                        bpf_program__title(prog, false),
> > +                        retprobe ? "uretprobe" : "uprobe",
> > +                        binary_path, func_offset,
> > +                        libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> > +             return link;
> > +     }
> > +     return link;
> > +}
> > +
> >   enum bpf_perf_event_ret
> >   bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
> >                          void **copy_mem, size_t *copy_size,
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index 1bf66c4a9330..bd767cc11967 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -171,6 +171,13 @@ LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
> >
> >   LIBBPF_API struct bpf_link *
> >   bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
> > +LIBBPF_API struct bpf_link *
> > +bpf_program__attach_kprobe(struct bpf_program *prog, bool retprobe,
> > +                        const char *func_name);
> > +LIBBPF_API struct bpf_link *
> > +bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
> > +                        pid_t pid, const char *binary_path,
> > +                        size_t func_offset);
> >
> >   struct bpf_insn;
> >
> > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > index 756f5aa802e9..57a40fb60718 100644
> > --- a/tools/lib/bpf/libbpf.map
> > +++ b/tools/lib/bpf/libbpf.map
> > @@ -169,7 +169,9 @@ LIBBPF_0.0.4 {
> >       global:
> >               bpf_link__destroy;
> >               bpf_object__load_xattr;
> > +             bpf_program__attach_kprobe;
> >               bpf_program__attach_perf_event;
> > +             bpf_program__attach_uprobe;
> >               btf_dump__dump_type;
> >               btf_dump__free;
> >               btf_dump__new;
> >

Re: [PATCH v4 bpf-next 6/9] libbpf: add raw tracepoint attach API

[Andrii Nakryiko]

On Mon, Jul 1, 2019 at 10:13 AM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> > Add a wrapper utilizing bpf_link "infrastructure" to allow attaching BPF
> > programs to raw tracepoints.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > Acked-by: Song Liu <songliubraving@fb.com>
> > ---
> >   tools/lib/bpf/libbpf.c   | 37 +++++++++++++++++++++++++++++++++++++
> >   tools/lib/bpf/libbpf.h   |  3 +++
> >   tools/lib/bpf/libbpf.map |  1 +
> >   3 files changed, 41 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 8ad4f915df38..f8c7a7ecb35e 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -4263,6 +4263,43 @@ struct bpf_link *bpf_program__attach_tracepoint(struct bpf_program *prog,
> >       return link;
> >   }
> >
> > +static int bpf_link__destroy_fd(struct bpf_link *link)
> > +{
> > +     struct bpf_link_fd *l = (void *)link;
> > +
> > +     return close(l->fd);
> > +}
> > +
> > +struct bpf_link *bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
> > +                                                 const char *tp_name)
> > +{
> > +     char errmsg[STRERR_BUFSIZE];
> > +     struct bpf_link_fd *link;
> > +     int prog_fd, pfd;
> > +
> > +     prog_fd = bpf_program__fd(prog);
> > +     if (prog_fd < 0) {
> > +             pr_warning("program '%s': can't attach before loaded\n",
> > +                        bpf_program__title(prog, false));
> > +             return ERR_PTR(-EINVAL);
> > +     }
> > +
> > +     link = malloc(sizeof(*link));
> > +     link->link.destroy = &bpf_link__destroy_fd;
>
> You can move the "link = malloc(...)" etc. after
> bpf_raw_tracepoint_open(). That way, you do not need to free(link)
> in the error case.

It's either `free(link)` here, or `close(pfd)` if malloc fails after
we attached program. Either way extra clean up is needed. I went with
the first one.

>
> > +
> > +     pfd = bpf_raw_tracepoint_open(tp_name, prog_fd);
> > +     if (pfd < 0) {
> > +             pfd = -errno;
> > +             free(link);
> > +             pr_warning("program '%s': failed to attach to raw tracepoint '%s': %s\n",
> > +                        bpf_program__title(prog, false), tp_name,
> > +                        libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> > +             return ERR_PTR(pfd);
> > +     }
> > +     link->fd = pfd;
> > +     return (struct bpf_link *)link;
> > +}
> > +
> >   enum bpf_perf_event_ret
> >   bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
> >                          void **copy_mem, size_t *copy_size,
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index 60611f4b4e1d..f55933784f95 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -182,6 +182,9 @@ LIBBPF_API struct bpf_link *
> >   bpf_program__attach_tracepoint(struct bpf_program *prog,
> >                              const char *tp_category,
> >                              const char *tp_name);
> > +LIBBPF_API struct bpf_link *
> > +bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
> > +                                const char *tp_name);
> >
> >   struct bpf_insn;
> >
> > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > index 3c618b75ef65..e6b7d4edbc93 100644
> > --- a/tools/lib/bpf/libbpf.map
> > +++ b/tools/lib/bpf/libbpf.map
> > @@ -171,6 +171,7 @@ LIBBPF_0.0.4 {
> >               bpf_object__load_xattr;
> >               bpf_program__attach_kprobe;
> >               bpf_program__attach_perf_event;
> > +             bpf_program__attach_raw_tracepoint;
> >               bpf_program__attach_tracepoint;
> >               bpf_program__attach_uprobe;
> >               btf_dump__dump_type;
> >

Re: [PATCH v4 bpf-next 7/9] selftests/bpf: switch test to new attach_perf_event API

[Andrii Nakryiko]

On Mon, Jul 1, 2019 at 10:16 AM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> > Use new bpf_program__attach_perf_event() in test previously relying on
> > direct ioctl manipulations.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > Reviewed-by: Stanislav Fomichev <sdf@google.com>
> > Acked-by: Song Liu <songliubraving@fb.com>
> > ---
> >   .../bpf/prog_tests/stacktrace_build_id_nmi.c  | 31 +++++++++----------
> >   1 file changed, 15 insertions(+), 16 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
> > index 1c1a2f75f3d8..9557b7dfb782 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
> > @@ -17,6 +17,7 @@ static __u64 read_perf_max_sample_freq(void)
> >   void test_stacktrace_build_id_nmi(void)
> >   {
> >       int control_map_fd, stackid_hmap_fd, stackmap_fd, stack_amap_fd;
> > +     const char *prog_name = "tracepoint/random/urandom_read";
> >       const char *file = "./test_stacktrace_build_id.o";
> >       int err, pmu_fd, prog_fd;
> >       struct perf_event_attr attr = {
> > @@ -25,7 +26,9 @@ void test_stacktrace_build_id_nmi(void)
> >               .config = PERF_COUNT_HW_CPU_CYCLES,
> >       };
> >       __u32 key, previous_key, val, duration = 0;
> > +     struct bpf_program *prog;
> >       struct bpf_object *obj;
> > +     struct bpf_link *link;
> >       char buf[256];
> >       int i, j;
> >       struct bpf_stack_build_id id_offs[PERF_MAX_STACK_DEPTH];
> > @@ -39,6 +42,10 @@ void test_stacktrace_build_id_nmi(void)
> >       if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno))
> >               return;
> >
> > +     prog = bpf_object__find_program_by_title(obj, prog_name);
> > +     if (CHECK(!prog, "find_prog", "prog '%s' not found\n", prog_name))
> > +             goto close_prog;
> > +
> >       pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */,
> >                        0 /* cpu 0 */, -1 /* group id */,
> >                        0 /* flags */);
> > @@ -47,15 +54,12 @@ void test_stacktrace_build_id_nmi(void)
> >                 pmu_fd, errno))
> >               goto close_prog;
> >
> > -     err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
> > -     if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n",
> > -               err, errno))
> > -             goto close_pmu;
> > -
> > -     err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
> > -     if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n",
> > -               err, errno))
> > -             goto disable_pmu;
> > +     link = bpf_program__attach_perf_event(prog, pmu_fd);
> > +     if (CHECK(IS_ERR(link), "attach_perf_event",
> > +               "err %ld\n", PTR_ERR(link))) {
> > +             close(pmu_fd);
> > +             goto close_prog;
> > +     }
> >
> >       /* find map fds */
> >       control_map_fd = bpf_find_map(__func__, obj, "control_map");
> > @@ -134,8 +138,7 @@ void test_stacktrace_build_id_nmi(void)
> >        * try it one more time.
> >        */
> >       if (build_id_matches < 1 && retry--) {
> > -             ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
> > -             close(pmu_fd);
> > +             bpf_link__destroy(link);
> >               bpf_object__close(obj);
> >               printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
> >                      __func__);
> > @@ -154,11 +157,7 @@ void test_stacktrace_build_id_nmi(void)
> >        */
> >
> >   disable_pmu:
> > -     ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
> > -
> > -close_pmu:
> > -     close(pmu_fd);
> > -
> > +     bpf_link__destroy(link);
>
> There is a problem in bpf_link__destroy(link).
> The "link = bpf_program__attach_perf_event(prog, pmu_fd)"
> may be an error pointer (IS_ERR(link) is true), in which
> case, link should be reset to NULL and then call
> bpf_link__destroy(link). Otherwise, the program may
> segfault or function incorrectly.

Not really, if bpf_program__attach_perf_event fails and IS_ERR(link)
is true, we'll close pmu_fd explicitly and `goto close_prog` bypassing
bpf_link__destroy. `goto disable_pmu` is done only after we
successfully established attached link.

So unless I still miss something, I think this will work reliably.

>
> >   close_prog:
> >       bpf_object__close(obj);
> >   }
> >

Re: [PATCH v4 bpf-next 3/9] libbpf: add ability to attach/detach BPF program to perf event

[Yonghong Song]

On 7/1/19 2:57 PM, Andrii Nakryiko wrote:
> On Mon, Jul 1, 2019 at 10:03 AM Yonghong Song <yhs@fb.com> wrote:
>>
>>
>>
>> On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
>>> bpf_program__attach_perf_event allows to attach BPF program to existing
>>> perf event hook, providing most generic and most low-level way to attach BPF
>>> programs. It returns struct bpf_link, which should be passed to
>>> bpf_link__destroy to detach and free resources, associated with a link.
>>>
>>> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>>> ---
>>>    tools/lib/bpf/libbpf.c   | 61 ++++++++++++++++++++++++++++++++++++++++
>>>    tools/lib/bpf/libbpf.h   |  3 ++
>>>    tools/lib/bpf/libbpf.map |  1 +
>>>    3 files changed, 65 insertions(+)
>>>
>>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>>> index 455795e6f8af..98c155ec3bfa 100644
>>> --- a/tools/lib/bpf/libbpf.c
>>> +++ b/tools/lib/bpf/libbpf.c
>>> @@ -32,6 +32,7 @@
>>>    #include <linux/limits.h>
>>>    #include <linux/perf_event.h>
>>>    #include <linux/ring_buffer.h>
>>> +#include <sys/ioctl.h>
>>>    #include <sys/stat.h>
>>>    #include <sys/types.h>
>>>    #include <sys/vfs.h>
>>> @@ -3958,6 +3959,66 @@ int bpf_link__destroy(struct bpf_link *link)
>>>        return err;
>>>    }
>>>
>>> +struct bpf_link_fd {
>>> +     struct bpf_link link; /* has to be at the top of struct */
>>> +     int fd; /* hook FD */
>>> +};
>>> +
>>> +static int bpf_link__destroy_perf_event(struct bpf_link *link)
>>> +{
>>> +     struct bpf_link_fd *l = (void *)link;
>>> +     int err;
>>> +
>>> +     if (l->fd < 0)
>>> +             return 0;
>>> +
>>> +     err = ioctl(l->fd, PERF_EVENT_IOC_DISABLE, 0);
>>> +     if (err)
>>> +             err = -errno;
>>> +
>>> +     close(l->fd);
>>> +     return err;
>>> +}
>>> +
>>> +struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
>>> +                                             int pfd)
>>> +{
>>> +     char errmsg[STRERR_BUFSIZE];
>>> +     struct bpf_link_fd *link;
>>> +     int prog_fd, err;
>>> +
>>> +     prog_fd = bpf_program__fd(prog);
>>> +     if (prog_fd < 0) {
>>> +             pr_warning("program '%s': can't attach before loaded\n",
>>> +                        bpf_program__title(prog, false));
>>> +             return ERR_PTR(-EINVAL);
>>> +     }
>>
>> should we check validity of pfd here?
>> If pfd < 0, we just return ERR_PTR(-EINVAL)?
> 
> I can add that. I didn't do it, because in general, you can provide fd
>> = 0 which is still not a valid FD for PERF_EVENT_IOC_SET_BPF and
> PERF_EVENT_IOC_ENABLE, so in general we can't detect this reliably.

I just want a check for validity of input parameter which will failure 
later with dedicated error message. But the same negative pfd will
be printed in ioctl error message. I am okay with this.

> 
>> This way, in bpf_link__destroy_perf_event(), we do not need to check
>> l->fd < 0 since it will be always nonnegative.
> 
> That check is not needed anyway, because even if pfd < 0, ioctl should
> fail and return error. I'll remove that check.
> 
>>
>>> +
>>> +     link = malloc(sizeof(*link));
>>> +     if (!link)
>>> +             return ERR_PTR(-ENOMEM);
>>> +     link->link.destroy = &bpf_link__destroy_perf_event;
>>> +     link->fd = pfd;
>>> +
>>> +     if (ioctl(pfd, PERF_EVENT_IOC_SET_BPF, prog_fd) < 0) {
>>> +             err = -errno;
>>> +             free(link);
>>> +             pr_warning("program '%s': failed to attach to pfd %d: %s\n",
>>> +                        bpf_program__title(prog, false), pfd,
>>> +                        libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
>>> +             return ERR_PTR(err);
>>> +     }
>>> +     if (ioctl(pfd, PERF_EVENT_IOC_ENABLE, 0) < 0) {
>>> +             err = -errno;
>>> +             free(link);
>>> +             pr_warning("program '%s': failed to enable pfd %d: %s\n",
>>> +                        bpf_program__title(prog, false), pfd,
>>> +                        libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
>>> +             return ERR_PTR(err);
>>> +     }
>>> +     return (struct bpf_link *)link;
>>> +}
>>> +
>>>    enum bpf_perf_event_ret
>>>    bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
>>>                           void **copy_mem, size_t *copy_size,
>>> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
>>> index 5082a5ebb0c2..1bf66c4a9330 100644
>>> --- a/tools/lib/bpf/libbpf.h
>>> +++ b/tools/lib/bpf/libbpf.h
>>> @@ -169,6 +169,9 @@ struct bpf_link;
>>>
>>>    LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
>>>
>>> +LIBBPF_API struct bpf_link *
>>> +bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
>>> +
>>>    struct bpf_insn;
>>>
>>>    /*
>>> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
>>> index 3cde850fc8da..756f5aa802e9 100644
>>> --- a/tools/lib/bpf/libbpf.map
>>> +++ b/tools/lib/bpf/libbpf.map
>>> @@ -169,6 +169,7 @@ LIBBPF_0.0.4 {
>>>        global:
>>>                bpf_link__destroy;
>>>                bpf_object__load_xattr;
>>> +             bpf_program__attach_perf_event;
>>>                btf_dump__dump_type;
>>>                btf_dump__free;
>>>                btf_dump__new;
>>>

Re: [PATCH v4 bpf-next 8/9] selftests/bpf: add kprobe/uprobe selftests

[Andrii Nakryiko]

On Mon, Jul 1, 2019 at 10:18 AM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> > Add tests verifying kprobe/kretprobe/uprobe/uretprobe APIs work as
> > expected.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > Reviewed-by: Stanislav Fomichev <sdf@google.com>
> > Acked-by: Song Liu <songliubraving@fb.com>
> > ---
> >   .../selftests/bpf/prog_tests/attach_probe.c   | 155 ++++++++++++++++++
> >   .../selftests/bpf/progs/test_attach_probe.c   |  55 +++++++
> >   2 files changed, 210 insertions(+)
> >   create mode 100644 tools/testing/selftests/bpf/prog_tests/attach_probe.c
> >   create mode 100644 tools/testing/selftests/bpf/progs/test_attach_probe.c
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> > new file mode 100644
> > index 000000000000..f22929063c58
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> > @@ -0,0 +1,155 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include <test_progs.h>
> > +
> > +ssize_t get_base_addr() {
> > +     size_t start;
> > +     char buf[256];
> > +     FILE *f;
> > +
> > +     f = fopen("/proc/self/maps", "r");
> > +     if (!f)
> > +             return -errno;
> > +
> > +     while (fscanf(f, "%zx-%*x %s %*s\n", &start, buf) == 2) {
> > +             if (strcmp(buf, "r-xp") == 0) {
> > +                     fclose(f);
> > +                     return start;
> > +             }
> > +     }
> > +
> > +     fclose(f);
> > +     return -EINVAL;
> > +}
> > +
> > +void test_attach_probe(void)
> > +{
> > +     const char *kprobe_name = "kprobe/sys_nanosleep";
> > +     const char *kretprobe_name = "kretprobe/sys_nanosleep";
> > +     const char *uprobe_name = "uprobe/trigger_func";
> > +     const char *uretprobe_name = "uretprobe/trigger_func";
> > +     const int kprobe_idx = 0, kretprobe_idx = 1;
> > +     const int uprobe_idx = 2, uretprobe_idx = 3;
> > +     const char *file = "./test_attach_probe.o";
> > +     struct bpf_program *kprobe_prog, *kretprobe_prog;
> > +     struct bpf_program *uprobe_prog, *uretprobe_prog;
> > +     struct bpf_object *obj;
> > +     int err, prog_fd, duration = 0, res;
> > +     struct bpf_link *kprobe_link = NULL;
> > +     struct bpf_link *kretprobe_link = NULL;
> > +     struct bpf_link *uprobe_link = NULL;
> > +     struct bpf_link *uretprobe_link = NULL;
> > +     int results_map_fd;
> > +     size_t uprobe_offset;
> > +     ssize_t base_addr;
> > +
> > +     base_addr = get_base_addr();
> > +     if (CHECK(base_addr < 0, "get_base_addr",
> > +               "failed to find base addr: %zd", base_addr))
> > +             return;
> > +     uprobe_offset = (size_t)&get_base_addr - base_addr;
> > +
> > +     /* load programs */
> > +     err = bpf_prog_load(file, BPF_PROG_TYPE_KPROBE, &obj, &prog_fd);
> > +     if (CHECK(err, "obj_load", "err %d errno %d\n", err, errno))
> > +             return;
> > +
> > +     kprobe_prog = bpf_object__find_program_by_title(obj, kprobe_name);
> > +     if (CHECK(!kprobe_prog, "find_probe",
> > +               "prog '%s' not found\n", kprobe_name))
> > +             goto cleanup;
> > +     kretprobe_prog = bpf_object__find_program_by_title(obj, kretprobe_name);
> > +     if (CHECK(!kretprobe_prog, "find_probe",
> > +               "prog '%s' not found\n", kretprobe_name))
> > +             goto cleanup;
> > +     uprobe_prog = bpf_object__find_program_by_title(obj, uprobe_name);
> > +     if (CHECK(!uprobe_prog, "find_probe",
> > +               "prog '%s' not found\n", uprobe_name))
> > +             goto cleanup;
> > +     uretprobe_prog = bpf_object__find_program_by_title(obj, uretprobe_name);
> > +     if (CHECK(!uretprobe_prog, "find_probe",
> > +               "prog '%s' not found\n", uretprobe_name))
> > +             goto cleanup;
> > +
> > +     /* load maps */
> > +     results_map_fd = bpf_find_map(__func__, obj, "results_map");
> > +     if (CHECK(results_map_fd < 0, "find_results_map",
> > +               "err %d\n", results_map_fd))
> > +             goto cleanup;
> > +
> > +     kprobe_link = bpf_program__attach_kprobe(kprobe_prog,
> > +                                              false /* retprobe */,
> > +                                              "sys_nanosleep");
> > +     if (CHECK(IS_ERR(kprobe_link), "attach_kprobe",
> > +               "err %ld\n", PTR_ERR(kprobe_link)))
> > +             goto cleanup;
> > +
> > +     kretprobe_link = bpf_program__attach_kprobe(kretprobe_prog,
> > +                                                 true /* retprobe */,
> > +                                                 "sys_nanosleep");
> > +     if (CHECK(IS_ERR(kretprobe_link), "attach_kretprobe",
> > +               "err %ld\n", PTR_ERR(kretprobe_link)))
> > +             goto cleanup;
> > +
> > +     uprobe_link = bpf_program__attach_uprobe(uprobe_prog,
> > +                                              false /* retprobe */,
> > +                                              0 /* self pid */,
> > +                                              "/proc/self/exe",
> > +                                              uprobe_offset);
> > +     if (CHECK(IS_ERR(uprobe_link), "attach_uprobe",
> > +               "err %ld\n", PTR_ERR(uprobe_link)))
> > +             goto cleanup;
> > +
> > +     uretprobe_link = bpf_program__attach_uprobe(uretprobe_prog,
> > +                                                 true /* retprobe */,
> > +                                                 -1 /* any pid */,
> > +                                                 "/proc/self/exe",
> > +                                                 uprobe_offset);
> > +     if (CHECK(IS_ERR(uretprobe_link), "attach_uretprobe",
> > +               "err %ld\n", PTR_ERR(uretprobe_link)))
> > +             goto cleanup;
> > +
> > +     /* trigger & validate kprobe && kretprobe */
> > +     usleep(1);
> > +
> > +     err = bpf_map_lookup_elem(results_map_fd, &kprobe_idx, &res);
> > +     if (CHECK(err, "get_kprobe_res",
> > +               "failed to get kprobe res: %d\n", err))
> > +             goto cleanup;
> > +     if (CHECK(res != kprobe_idx + 1, "check_kprobe_res",
> > +               "wrong kprobe res: %d\n", res))
> > +             goto cleanup;
> > +
> > +     err = bpf_map_lookup_elem(results_map_fd, &kretprobe_idx, &res);
> > +     if (CHECK(err, "get_kretprobe_res",
> > +               "failed to get kretprobe res: %d\n", err))
> > +             goto cleanup;
> > +     if (CHECK(res != kretprobe_idx + 1, "check_kretprobe_res",
> > +               "wrong kretprobe res: %d\n", res))
> > +             goto cleanup;
> > +
> > +     /* trigger & validate uprobe & uretprobe */
> > +     get_base_addr();
> > +
> > +     err = bpf_map_lookup_elem(results_map_fd, &uprobe_idx, &res);
> > +     if (CHECK(err, "get_uprobe_res",
> > +               "failed to get uprobe res: %d\n", err))
> > +             goto cleanup;
> > +     if (CHECK(res != uprobe_idx + 1, "check_uprobe_res",
> > +               "wrong uprobe res: %d\n", res))
> > +             goto cleanup;
> > +
> > +     err = bpf_map_lookup_elem(results_map_fd, &uretprobe_idx, &res);
> > +     if (CHECK(err, "get_uretprobe_res",
> > +               "failed to get uretprobe res: %d\n", err))
> > +             goto cleanup;
> > +     if (CHECK(res != uretprobe_idx + 1, "check_uretprobe_res",
> > +               "wrong uretprobe res: %d\n", res))
> > +             goto cleanup;
> > +
> > +cleanup:
> > +     bpf_link__destroy(kprobe_link);
> > +     bpf_link__destroy(kretprobe_link);
> > +     bpf_link__destroy(uprobe_link);
> > +     bpf_link__destroy(uretprobe_link);
>
> if any error happens, kprobe_link etc. will be a non-NULL pointer
> indicating an error. the above bpf_link__destroy() won't work
> properly. The same for patch #9.

Yes, you are absolutely right. Fixing.

I just also double-checked patch #9, it doesn't have this problem:
- stacktrace_build_id.c and stacktrace_map.c never call destroy before
bpf_link is successfully established.
- stacktrace_map_raw_tp.c uses if (!IS_ERR_OR_NULL(link)) check before
bpf_link__destroy.

So I think it's just this one.

>
> > +     bpf_object__close(obj);
> > +}
> > diff --git a/tools/testing/selftests/bpf/progs/test_attach_probe.c b/tools/testing/selftests/bpf/progs/test_attach_probe.c
> > new file mode 100644
> > index 000000000000..7a7c5cd728c8
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/test_attach_probe.c
> > @@ -0,0 +1,55 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (c) 2017 Facebook
> > +
> > +#include <linux/ptrace.h>
> > +#include <linux/bpf.h>
> > +#include "bpf_helpers.h"
> > +
> > +struct {
> > +     int type;
> > +     int max_entries;
> > +     int *key;
> > +     int *value;
> > +} results_map SEC(".maps") = {
> > +     .type = BPF_MAP_TYPE_ARRAY,
> > +     .max_entries = 4,
> > +};
> > +
> > +SEC("kprobe/sys_nanosleep")
> > +int handle_sys_nanosleep_entry(struct pt_regs *ctx)
> > +{
> > +     const int key = 0, value = 1;
> > +
> > +     bpf_map_update_elem(&results_map, &key, &value, 0);
> > +     return 0;
> > +}
> > +
> > +SEC("kretprobe/sys_nanosleep")
> > +int handle_sys_getpid_return(struct pt_regs *ctx)
> > +{
> > +     const int key = 1, value = 2;
> > +
> > +     bpf_map_update_elem(&results_map, &key, &value, 0);
> > +     return 0;
> > +}
> > +
> > +SEC("uprobe/trigger_func")
> > +int handle_uprobe_entry(struct pt_regs *ctx)
> > +{
> > +     const int key = 2, value = 3;
> > +
> > +     bpf_map_update_elem(&results_map, &key, &value, 0);
> > +     return 0;
> > +}
> > +
> > +SEC("uretprobe/trigger_func")
> > +int handle_uprobe_return(struct pt_regs *ctx)
> > +{
> > +     const int key = 3, value = 4;
> > +
> > +     bpf_map_update_elem(&results_map, &key, &value, 0);
> > +     return 0;
> > +}
> > +
> > +char _license[] SEC("license") = "GPL";
> > +__u32 _version SEC("version") = 1;
> >

Re: [PATCH v4 bpf-next 6/9] libbpf: add raw tracepoint attach API

[Yonghong Song]

On 7/1/19 3:26 PM, Andrii Nakryiko wrote:
> On Mon, Jul 1, 2019 at 10:13 AM Yonghong Song <yhs@fb.com> wrote:
>>
>>
>>
>> On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
>>> Add a wrapper utilizing bpf_link "infrastructure" to allow attaching BPF
>>> programs to raw tracepoints.
>>>
>>> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>>> Acked-by: Song Liu <songliubraving@fb.com>
>>> ---
>>>    tools/lib/bpf/libbpf.c   | 37 +++++++++++++++++++++++++++++++++++++
>>>    tools/lib/bpf/libbpf.h   |  3 +++
>>>    tools/lib/bpf/libbpf.map |  1 +
>>>    3 files changed, 41 insertions(+)
>>>
>>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>>> index 8ad4f915df38..f8c7a7ecb35e 100644
>>> --- a/tools/lib/bpf/libbpf.c
>>> +++ b/tools/lib/bpf/libbpf.c
>>> @@ -4263,6 +4263,43 @@ struct bpf_link *bpf_program__attach_tracepoint(struct bpf_program *prog,
>>>        return link;
>>>    }
>>>
>>> +static int bpf_link__destroy_fd(struct bpf_link *link)
>>> +{
>>> +     struct bpf_link_fd *l = (void *)link;
>>> +
>>> +     return close(l->fd);
>>> +}
>>> +
>>> +struct bpf_link *bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
>>> +                                                 const char *tp_name)
>>> +{
>>> +     char errmsg[STRERR_BUFSIZE];
>>> +     struct bpf_link_fd *link;
>>> +     int prog_fd, pfd;
>>> +
>>> +     prog_fd = bpf_program__fd(prog);
>>> +     if (prog_fd < 0) {
>>> +             pr_warning("program '%s': can't attach before loaded\n",
>>> +                        bpf_program__title(prog, false));
>>> +             return ERR_PTR(-EINVAL);
>>> +     }
>>> +
>>> +     link = malloc(sizeof(*link));
>>> +     link->link.destroy = &bpf_link__destroy_fd;
>>
>> You can move the "link = malloc(...)" etc. after
>> bpf_raw_tracepoint_open(). That way, you do not need to free(link)
>> in the error case.
> 
> It's either `free(link)` here, or `close(pfd)` if malloc fails after
> we attached program. Either way extra clean up is needed. I went with
> the first one.

Okay with me.
BTW, do you want to check whether malloc() may failure and link may be NULL?

>>
>>> +
>>> +     pfd = bpf_raw_tracepoint_open(tp_name, prog_fd);
>>> +     if (pfd < 0) {
>>> +             pfd = -errno;
>>> +             free(link);
>>> +             pr_warning("program '%s': failed to attach to raw tracepoint '%s': %s\n",
>>> +                        bpf_program__title(prog, false), tp_name,
>>> +                        libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
>>> +             return ERR_PTR(pfd);
>>> +     }
>>> +     link->fd = pfd;
>>> +     return (struct bpf_link *)link;
>>> +}
>>> +
>>>    enum bpf_perf_event_ret
>>>    bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
>>>                           void **copy_mem, size_t *copy_size,
>>> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
>>> index 60611f4b4e1d..f55933784f95 100644
>>> --- a/tools/lib/bpf/libbpf.h
>>> +++ b/tools/lib/bpf/libbpf.h
>>> @@ -182,6 +182,9 @@ LIBBPF_API struct bpf_link *
>>>    bpf_program__attach_tracepoint(struct bpf_program *prog,
>>>                               const char *tp_category,
>>>                               const char *tp_name);
>>> +LIBBPF_API struct bpf_link *
>>> +bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
>>> +                                const char *tp_name);
>>>
>>>    struct bpf_insn;
>>>
>>> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
>>> index 3c618b75ef65..e6b7d4edbc93 100644
>>> --- a/tools/lib/bpf/libbpf.map
>>> +++ b/tools/lib/bpf/libbpf.map
>>> @@ -171,6 +171,7 @@ LIBBPF_0.0.4 {
>>>                bpf_object__load_xattr;
>>>                bpf_program__attach_kprobe;
>>>                bpf_program__attach_perf_event;
>>> +             bpf_program__attach_raw_tracepoint;
>>>                bpf_program__attach_tracepoint;
>>>                bpf_program__attach_uprobe;
>>>                btf_dump__dump_type;
>>>

Re: [PATCH v4 bpf-next 7/9] selftests/bpf: switch test to new attach_perf_event API

[Yonghong Song]

On 7/1/19 3:32 PM, Andrii Nakryiko wrote:
> On Mon, Jul 1, 2019 at 10:16 AM Yonghong Song <yhs@fb.com> wrote:
>>
>>
>>
>> On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
>>> Use new bpf_program__attach_perf_event() in test previously relying on
>>> direct ioctl manipulations.
>>>
>>> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>>> Reviewed-by: Stanislav Fomichev <sdf@google.com>
>>> Acked-by: Song Liu <songliubraving@fb.com>
>>> ---
>>>    .../bpf/prog_tests/stacktrace_build_id_nmi.c  | 31 +++++++++----------
>>>    1 file changed, 15 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
>>> index 1c1a2f75f3d8..9557b7dfb782 100644
>>> --- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
>>> +++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
>>> @@ -17,6 +17,7 @@ static __u64 read_perf_max_sample_freq(void)
>>>    void test_stacktrace_build_id_nmi(void)
>>>    {
>>>        int control_map_fd, stackid_hmap_fd, stackmap_fd, stack_amap_fd;
>>> +     const char *prog_name = "tracepoint/random/urandom_read";
>>>        const char *file = "./test_stacktrace_build_id.o";
>>>        int err, pmu_fd, prog_fd;
>>>        struct perf_event_attr attr = {
>>> @@ -25,7 +26,9 @@ void test_stacktrace_build_id_nmi(void)
>>>                .config = PERF_COUNT_HW_CPU_CYCLES,
>>>        };
>>>        __u32 key, previous_key, val, duration = 0;
>>> +     struct bpf_program *prog;
>>>        struct bpf_object *obj;
>>> +     struct bpf_link *link;
>>>        char buf[256];
>>>        int i, j;
>>>        struct bpf_stack_build_id id_offs[PERF_MAX_STACK_DEPTH];
>>> @@ -39,6 +42,10 @@ void test_stacktrace_build_id_nmi(void)
>>>        if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno))
>>>                return;
>>>
>>> +     prog = bpf_object__find_program_by_title(obj, prog_name);
>>> +     if (CHECK(!prog, "find_prog", "prog '%s' not found\n", prog_name))
>>> +             goto close_prog;
>>> +
>>>        pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */,
>>>                         0 /* cpu 0 */, -1 /* group id */,
>>>                         0 /* flags */);
>>> @@ -47,15 +54,12 @@ void test_stacktrace_build_id_nmi(void)
>>>                  pmu_fd, errno))
>>>                goto close_prog;
>>>
>>> -     err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
>>> -     if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n",
>>> -               err, errno))
>>> -             goto close_pmu;
>>> -
>>> -     err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
>>> -     if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n",
>>> -               err, errno))
>>> -             goto disable_pmu;
>>> +     link = bpf_program__attach_perf_event(prog, pmu_fd);
>>> +     if (CHECK(IS_ERR(link), "attach_perf_event",
>>> +               "err %ld\n", PTR_ERR(link))) {
>>> +             close(pmu_fd);
>>> +             goto close_prog;
>>> +     }
>>>
>>>        /* find map fds */
>>>        control_map_fd = bpf_find_map(__func__, obj, "control_map");
>>> @@ -134,8 +138,7 @@ void test_stacktrace_build_id_nmi(void)
>>>         * try it one more time.
>>>         */
>>>        if (build_id_matches < 1 && retry--) {
>>> -             ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
>>> -             close(pmu_fd);
>>> +             bpf_link__destroy(link);
>>>                bpf_object__close(obj);
>>>                printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
>>>                       __func__);
>>> @@ -154,11 +157,7 @@ void test_stacktrace_build_id_nmi(void)
>>>         */
>>>
>>>    disable_pmu:
>>> -     ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
>>> -
>>> -close_pmu:
>>> -     close(pmu_fd);
>>> -
>>> +     bpf_link__destroy(link);
>>
>> There is a problem in bpf_link__destroy(link).
>> The "link = bpf_program__attach_perf_event(prog, pmu_fd)"
>> may be an error pointer (IS_ERR(link) is true), in which
>> case, link should be reset to NULL and then call
>> bpf_link__destroy(link). Otherwise, the program may
>> segfault or function incorrectly.
> 
> Not really, if bpf_program__attach_perf_event fails and IS_ERR(link)
> is true, we'll close pmu_fd explicitly and `goto close_prog` bypassing
> bpf_link__destroy. `goto disable_pmu` is done only after we
> successfully established attached link.
> 
> So unless I still miss something, I think this will work reliably.

Double checked again. You are correct. We do not have issues here.

> 
>>
>>>    close_prog:
>>>        bpf_object__close(obj);
>>>    }
>>>

Re: [PATCH v4 bpf-next 6/9] libbpf: add raw tracepoint attach API

[Andrii Nakryiko]

On Mon, Jul 1, 2019 at 4:04 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 7/1/19 3:26 PM, Andrii Nakryiko wrote:
> > On Mon, Jul 1, 2019 at 10:13 AM Yonghong Song <yhs@fb.com> wrote:
> >>
> >>
> >>
> >> On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> >>> Add a wrapper utilizing bpf_link "infrastructure" to allow attaching BPF
> >>> programs to raw tracepoints.
> >>>
> >>> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> >>> Acked-by: Song Liu <songliubraving@fb.com>
> >>> ---
> >>>    tools/lib/bpf/libbpf.c   | 37 +++++++++++++++++++++++++++++++++++++
> >>>    tools/lib/bpf/libbpf.h   |  3 +++
> >>>    tools/lib/bpf/libbpf.map |  1 +
> >>>    3 files changed, 41 insertions(+)
> >>>
> >>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> >>> index 8ad4f915df38..f8c7a7ecb35e 100644
> >>> --- a/tools/lib/bpf/libbpf.c
> >>> +++ b/tools/lib/bpf/libbpf.c
> >>> @@ -4263,6 +4263,43 @@ struct bpf_link *bpf_program__attach_tracepoint(struct bpf_program *prog,
> >>>        return link;
> >>>    }
> >>>
> >>> +static int bpf_link__destroy_fd(struct bpf_link *link)
> >>> +{
> >>> +     struct bpf_link_fd *l = (void *)link;
> >>> +
> >>> +     return close(l->fd);
> >>> +}
> >>> +
> >>> +struct bpf_link *bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
> >>> +                                                 const char *tp_name)
> >>> +{
> >>> +     char errmsg[STRERR_BUFSIZE];
> >>> +     struct bpf_link_fd *link;
> >>> +     int prog_fd, pfd;
> >>> +
> >>> +     prog_fd = bpf_program__fd(prog);
> >>> +     if (prog_fd < 0) {
> >>> +             pr_warning("program '%s': can't attach before loaded\n",
> >>> +                        bpf_program__title(prog, false));
> >>> +             return ERR_PTR(-EINVAL);
> >>> +     }
> >>> +
> >>> +     link = malloc(sizeof(*link));
> >>> +     link->link.destroy = &bpf_link__destroy_fd;
> >>
> >> You can move the "link = malloc(...)" etc. after
> >> bpf_raw_tracepoint_open(). That way, you do not need to free(link)
> >> in the error case.
> >
> > It's either `free(link)` here, or `close(pfd)` if malloc fails after
> > we attached program. Either way extra clean up is needed. I went with
> > the first one.
>
> Okay with me.
> BTW, do you want to check whether malloc() may failure and link may be NULL?

Oh, you have a fresh eye! ;) fixed.

>
> >>
> >>> +
> >>> +     pfd = bpf_raw_tracepoint_open(tp_name, prog_fd);
> >>> +     if (pfd < 0) {
> >>> +             pfd = -errno;
> >>> +             free(link);
> >>> +             pr_warning("program '%s': failed to attach to raw tracepoint '%s': %s\n",
> >>> +                        bpf_program__title(prog, false), tp_name,
> >>> +                        libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> >>> +             return ERR_PTR(pfd);
> >>> +     }
> >>> +     link->fd = pfd;
> >>> +     return (struct bpf_link *)link;
> >>> +}
> >>> +
> >>>    enum bpf_perf_event_ret
> >>>    bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
> >>>                           void **copy_mem, size_t *copy_size,
> >>> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> >>> index 60611f4b4e1d..f55933784f95 100644
> >>> --- a/tools/lib/bpf/libbpf.h
> >>> +++ b/tools/lib/bpf/libbpf.h
> >>> @@ -182,6 +182,9 @@ LIBBPF_API struct bpf_link *
> >>>    bpf_program__attach_tracepoint(struct bpf_program *prog,
> >>>                               const char *tp_category,
> >>>                               const char *tp_name);
> >>> +LIBBPF_API struct bpf_link *
> >>> +bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
> >>> +                                const char *tp_name);
> >>>
> >>>    struct bpf_insn;
> >>>
> >>> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> >>> index 3c618b75ef65..e6b7d4edbc93 100644
> >>> --- a/tools/lib/bpf/libbpf.map
> >>> +++ b/tools/lib/bpf/libbpf.map
> >>> @@ -171,6 +171,7 @@ LIBBPF_0.0.4 {
> >>>                bpf_object__load_xattr;
> >>>                bpf_program__attach_kprobe;
> >>>                bpf_program__attach_perf_event;
> >>> +             bpf_program__attach_raw_tracepoint;
> >>>                bpf_program__attach_tracepoint;
> >>>                bpf_program__attach_uprobe;
> >>>                btf_dump__dump_type;
> >>>