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

[PATCH v3 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.

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                        | 402 ++++++++++++++++++
 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, 687 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 v3 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>
---
 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 v3 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>
---
 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 v3 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   | 58 ++++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h   |  3 +++
 tools/lib/bpf/libbpf.map |  1 +
 3 files changed, 62 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 455795e6f8af..606705f878ba 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,63 @@ 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);
+	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 bpf_fd, err;
+
+	bpf_fd = bpf_program__fd(prog);
+	if (bpf_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, bpf_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 v3 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   | 213 +++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h   |   7 ++
 tools/lib/bpf/libbpf.map |   2 +
 3 files changed, 222 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 606705f878ba..65d2fef41003 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4016,6 +4016,219 @@ struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
 	return (struct bpf_link *)link;
 }
 
+static int parse_uint(const char *buf)
+{
+	int ret;
+
+	errno = 0;
+	ret = (int)strtol(buf, NULL, 10);
+	if (errno) {
+		ret = -errno;
+		pr_debug("failed to parse '%s' as unsigned int\n", buf);
+		return ret;
+	}
+	if (ret < 0) {
+		pr_debug("failed to parse '%s' as unsigned int\n", buf);
+		return -EINVAL;
+	}
+	return ret;
+}
+
+static int parse_uint_from_file(const char* file)
+{
+	char buf[STRERR_BUFSIZE];
+	int fd, ret;
+
+	fd = open(file, O_RDONLY);
+	if (fd < 0) {
+		ret = -errno;
+		pr_debug("failed to open '%s': %s\n", file,
+			 libbpf_strerror_r(ret, buf, sizeof(buf)));
+		return ret;
+	}
+	ret = read(fd, buf, sizeof(buf));
+	ret = ret < 0 ? -errno : ret;
+	close(fd);
+	if (ret < 0) {
+		pr_debug("failed to read '%s': %s\n", file,
+			libbpf_strerror_r(ret, buf, sizeof(buf)));
+		return ret;
+	}
+	if (ret == 0 || ret >= sizeof(buf)) {
+		buf[sizeof(buf) - 1] = 0;
+		pr_debug("unexpected input from '%s': '%s'\n", file, buf);
+		return -EINVAL;
+	}
+	return parse_uint(buf);
+}
+
+static int determine_kprobe_perf_type(void)
+{
+	const char *file = "/sys/bus/event_source/devices/kprobe/type";
+	return parse_uint_from_file(file);
+}
+
+static int determine_uprobe_perf_type(void)
+{
+	const char *file = "/sys/bus/event_source/devices/uprobe/type";
+	return parse_uint_from_file(file);
+}
+
+static int parse_config_from_file(const char *file)
+{
+	char buf[STRERR_BUFSIZE];
+	int fd, ret;
+
+	fd = open(file, O_RDONLY);
+	if (fd < 0) {
+		ret = -errno;
+		pr_debug("failed to open '%s': %s\n", file,
+			 libbpf_strerror_r(ret, buf, sizeof(buf)));
+		return ret;
+	}
+	ret = read(fd, buf, sizeof(buf));
+	ret = ret < 0 ? -errno : ret;
+	close(fd);
+	if (ret < 0) {
+		pr_debug("failed to read '%s': %s\n", file,
+			libbpf_strerror_r(ret, buf, sizeof(buf)));
+		return ret;
+	}
+	if (ret == 0 || ret >= sizeof(buf)) {
+		buf[sizeof(buf) - 1] = 0;
+		pr_debug("unexpected input from '%s': '%s'\n", file, buf);
+		return -EINVAL;
+	}
+	if (strncmp(buf, "config:", 7)) {
+		pr_debug("expected 'config:' prefix, found '%s'\n", buf);
+		return -EINVAL;
+	}
+	return parse_uint(buf + 7);
+}
+
+static int determine_kprobe_retprobe_bit(void)
+{
+	const char *file = "/sys/bus/event_source/devices/kprobe/format/retprobe";
+	return parse_config_from_file(file);
+}
+
+static int determine_uprobe_retprobe_bit(void)
+{
+	const char *file = "/sys/bus/event_source/devices/uprobe/format/retprobe";
+	return parse_config_from_file(file);
+}
+
+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 v3 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>
---
 tools/lib/bpf/libbpf.c   | 78 ++++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h   |  4 +++
 tools/lib/bpf/libbpf.map |  1 +
 3 files changed, 83 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 65d2fef41003..76d1599a7d56 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4229,6 +4229,84 @@ 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_uint_from_file(file);
+}
+
+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 v3 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>
---
 tools/lib/bpf/libbpf.c   | 36 ++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h   |  3 +++
 tools/lib/bpf/libbpf.map |  1 +
 3 files changed, 40 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 76d1599a7d56..a2a98faa9642 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4307,6 +4307,42 @@ 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 bpf_fd, pfd;
+
+	bpf_fd = bpf_program__fd(prog);
+	if (bpf_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, bpf_fd);
+	if (pfd < 0) {
+		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 v3 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>
---
 .../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 v3 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>
---
 .../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 v3 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>
---
 .../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 v3 bpf-next 2/9] libbpf: introduce concept of bpf_link

[Stanislav Fomichev]

On 06/27, Andrii Nakryiko wrote:
> 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>
> ---
>  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 {
Maybe call it bpf_attachment? You call the bpf_program__attach_to_blah
and you get an attachment?

> +	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
> 

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

[Stanislav Fomichev]

On 06/27, 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   | 58 ++++++++++++++++++++++++++++++++++++++++
>  tools/lib/bpf/libbpf.h   |  3 +++
>  tools/lib/bpf/libbpf.map |  1 +
>  3 files changed, 62 insertions(+)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 455795e6f8af..606705f878ba 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,63 @@ 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 */
> +};
Any cons to storing everything in bpf_link, instead of creating a
"subclass"? Less things to worry about.

> +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);
> +	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 bpf_fd, err;
> +
> +	bpf_fd = bpf_program__fd(prog);
> +	if (bpf_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, bpf_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 v3 bpf-next 2/9] libbpf: introduce concept of bpf_link

[Andrii Nakryiko]

On Fri, Jun 28, 2019 at 9:02 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> On 06/27, Andrii Nakryiko wrote:
> > 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>
> > ---
> >  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 {
> Maybe call it bpf_attachment? You call the bpf_program__attach_to_blah
> and you get an attachment?

I wanted to keep it as short as possible, bpf_attachment is way too
long (it's also why as an alternative I've proposed bpf_assoc, not
bpf_association, but bpf_attach isn't great shortening).

>
> > +     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
> >

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

[Andrii Nakryiko]

On Fri, Jun 28, 2019 at 9:04 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> On 06/27, 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   | 58 ++++++++++++++++++++++++++++++++++++++++
> >  tools/lib/bpf/libbpf.h   |  3 +++
> >  tools/lib/bpf/libbpf.map |  1 +
> >  3 files changed, 62 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 455795e6f8af..606705f878ba 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,63 @@ 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 */
> > +};
> Any cons to storing everything in bpf_link, instead of creating a
> "subclass"? Less things to worry about.

Yes, it's not always enough to just have single FD to detach BPF
program. Check bpf_prog_detach and bpf_prog_detach2 in
tools/lib/bpf/bpf.c. For some types of attachment you have to provide
target_fd+attach_type, for some target_fd+attach_type+attach_bpf_fd.
So those two will use their own bpf_link extensions.

I haven't implemented those attachment APIs yet, but we should.

What should go into bpf_link itself is any information that's common
to any kind of attachment (e.g, "kind of attachment" itself). It's
conceivable that we might allow "casting" bpf_link into specific
variation and having extra "methods" on those. I haven't done that, as
I didn't have a need yet.

>
> > +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);
> > +     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 bpf_fd, err;
> > +
> > +     bpf_fd = bpf_program__fd(prog);
> > +     if (bpf_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, bpf_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 v3 bpf-next 2/9] libbpf: introduce concept of bpf_link

[Stanislav Fomichev]

On 06/28, Andrii Nakryiko wrote:
> On Fri, Jun 28, 2019 at 9:02 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
> >
> > On 06/27, Andrii Nakryiko wrote:
> > > 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>
> > > ---
> > >  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 {
> > Maybe call it bpf_attachment? You call the bpf_program__attach_to_blah
> > and you get an attachment?
> 
> I wanted to keep it as short as possible, bpf_attachment is way too
> long (it's also why as an alternative I've proposed bpf_assoc, not
> bpf_association, but bpf_attach isn't great shortening).
Why do you want to keep it short? We have far longer names than
bpf_attachment in libbpf. That shouldn't be a big concern.

> > > +     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
> > >

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

[Alexei Starovoitov]

On 6/28/19 10:45 AM, Stanislav Fomichev wrote:
>>>>
>>>> +struct bpf_link {
>>> Maybe call it bpf_attachment? You call the bpf_program__attach_to_blah
>>> and you get an attachment?
>>
>> I wanted to keep it as short as possible, bpf_attachment is way too
>> long (it's also why as an alternative I've proposed bpf_assoc, not
>> bpf_association, but bpf_attach isn't great shortening).
> Why do you want to keep it short? We have far longer names than
> bpf_attachment in libbpf. That shouldn't be a big concern.

Naming is hard. I also prefer short.
imo the word 'link' describes the concept better than 'attachment'.

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

[Stanislav Fomichev]

On 06/28, Andrii Nakryiko wrote:
> On Fri, Jun 28, 2019 at 9:04 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
> >
> > On 06/27, 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   | 58 ++++++++++++++++++++++++++++++++++++++++
> > >  tools/lib/bpf/libbpf.h   |  3 +++
> > >  tools/lib/bpf/libbpf.map |  1 +
> > >  3 files changed, 62 insertions(+)
> > >
> > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > index 455795e6f8af..606705f878ba 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,63 @@ 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 */
> > > +};
> > Any cons to storing everything in bpf_link, instead of creating a
> > "subclass"? Less things to worry about.
> 
> Yes, it's not always enough to just have single FD to detach BPF
> program. Check bpf_prog_detach and bpf_prog_detach2 in
> tools/lib/bpf/bpf.c. For some types of attachment you have to provide
> target_fd+attach_type, for some target_fd+attach_type+attach_bpf_fd.
> So those two will use their own bpf_link extensions.
> 
> I haven't implemented those attachment APIs yet, but we should.
> 
> What should go into bpf_link itself is any information that's common
> to any kind of attachment (e.g, "kind of attachment" itself). It's
> conceivable that we might allow "casting" bpf_link into specific
> variation and having extra "methods" on those. I haven't done that, as
> I didn't have a need yet.
You're optimizing for a memory footprint, I guess. I was trying to
point out that maybe it's easier just to put everything in the bpf_link
and don't do any castings. Some events would use attach_type, some
won't. But, OTOH, maybe having a specific bpf_link variation per
attachment is more readable, idk :-)

> > > +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);
> > > +     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 bpf_fd, err;
> > > +
> > > +     bpf_fd = bpf_program__fd(prog);
> > > +     if (bpf_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, bpf_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 v3 bpf-next 2/9] libbpf: introduce concept of bpf_link

[Stanislav Fomichev]

On 06/28, Alexei Starovoitov wrote:
> On 6/28/19 10:45 AM, Stanislav Fomichev wrote:
> >>>>
> >>>> +struct bpf_link {
> >>> Maybe call it bpf_attachment? You call the bpf_program__attach_to_blah
> >>> and you get an attachment?
> >>
> >> I wanted to keep it as short as possible, bpf_attachment is way too
> >> long (it's also why as an alternative I've proposed bpf_assoc, not
> >> bpf_association, but bpf_attach isn't great shortening).
> > Why do you want to keep it short? We have far longer names than
> > bpf_attachment in libbpf. That shouldn't be a big concern.
> 
> Naming is hard. I also prefer short.
There are only two hard things in Computer Science :-)

> imo the word 'link' describes the concept better than 'attachment'.

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

[Andrii Nakryiko]

On Fri, Jun 28, 2019 at 10:54 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> On 06/28, Andrii Nakryiko wrote:
> > On Fri, Jun 28, 2019 at 9:04 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
> > >
> > > On 06/27, 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   | 58 ++++++++++++++++++++++++++++++++++++++++
> > > >  tools/lib/bpf/libbpf.h   |  3 +++
> > > >  tools/lib/bpf/libbpf.map |  1 +
> > > >  3 files changed, 62 insertions(+)
> > > >
> > > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > > index 455795e6f8af..606705f878ba 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,63 @@ 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 */
> > > > +};
> > > Any cons to storing everything in bpf_link, instead of creating a
> > > "subclass"? Less things to worry about.
> >
> > Yes, it's not always enough to just have single FD to detach BPF
> > program. Check bpf_prog_detach and bpf_prog_detach2 in
> > tools/lib/bpf/bpf.c. For some types of attachment you have to provide
> > target_fd+attach_type, for some target_fd+attach_type+attach_bpf_fd.
> > So those two will use their own bpf_link extensions.
> >
> > I haven't implemented those attachment APIs yet, but we should.
> >
> > What should go into bpf_link itself is any information that's common
> > to any kind of attachment (e.g, "kind of attachment" itself). It's
> > conceivable that we might allow "casting" bpf_link into specific
> > variation and having extra "methods" on those. I haven't done that, as
> > I didn't have a need yet.
> You're optimizing for a memory footprint, I guess. I was trying to
> point out that maybe it's easier just to put everything in the bpf_link
> and don't do any castings. Some events would use attach_type, some
> won't. But, OTOH, maybe having a specific bpf_link variation per
> attachment is more readable, idk :-)

Ah, I see, you want to have superset of all possible things that
constitute bpf_link. I generally don't like that, because it becomes
harder to understand what's really used and what's there just in case
:)

Good thing is that all this is libbpf-internal, so we can change any
of that easily without any breakage of users.

>
> > > > +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);
> > > > +     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 bpf_fd, err;
> > > > +
> > > > +     bpf_fd = bpf_program__fd(prog);
> > > > +     if (bpf_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, bpf_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 v3 bpf-next 3/9] libbpf: add ability to attach/detach BPF program to perf event

[Stanislav Fomichev]

On 06/27, 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   | 58 ++++++++++++++++++++++++++++++++++++++++
>  tools/lib/bpf/libbpf.h   |  3 +++
>  tools/lib/bpf/libbpf.map |  1 +
>  3 files changed, 62 insertions(+)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 455795e6f8af..606705f878ba 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,63 @@ 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);
> +	close(l->fd);
> +	return err;
Why not return -errno from ioctl here (as you do elsewhere)?

> +}
> +
> +struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
> +						int pfd)
> +{
> +	char errmsg[STRERR_BUFSIZE];
> +	struct bpf_link_fd *link;
> +	int bpf_fd, err;
> +
> +	bpf_fd = bpf_program__fd(prog);
> +	if (bpf_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, bpf_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 v3 bpf-next 3/9] libbpf: add ability to attach/detach BPF program to perf event

[Andrii Nakryiko]

On Fri, Jun 28, 2019 at 11:00 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> On 06/27, 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   | 58 ++++++++++++++++++++++++++++++++++++++++
> >  tools/lib/bpf/libbpf.h   |  3 +++
> >  tools/lib/bpf/libbpf.map |  1 +
> >  3 files changed, 62 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 455795e6f8af..606705f878ba 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,63 @@ 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);
> > +     close(l->fd);
> > +     return err;
> Why not return -errno from ioctl here (as you do elsewhere)?

Good catch, will fix, thanks!

As an aside, this whole returning error on close/destroy is a bit
moot, as there is little one can do if any of teardown steps fail
(except crash, which is not great response :) ). So the strategy would
be to still free all memory and try to close all FDs, before returning
error (again, which one, first, last? eh..).

>
> > +}
> > +
> > +struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
> > +                                             int pfd)
> > +{
> > +     char errmsg[STRERR_BUFSIZE];
> > +     struct bpf_link_fd *link;
> > +     int bpf_fd, err;
> > +
> > +     bpf_fd = bpf_program__fd(prog);
> > +     if (bpf_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, bpf_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 v3 bpf-next 3/9] libbpf: add ability to attach/detach BPF program to perf event

[Stanislav Fomichev]

On 06/27, 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   | 58 ++++++++++++++++++++++++++++++++++++++++
>  tools/lib/bpf/libbpf.h   |  3 +++
>  tools/lib/bpf/libbpf.map |  1 +
>  3 files changed, 62 insertions(+)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 455795e6f8af..606705f878ba 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,63 @@ 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);
> +	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 bpf_fd, err;
nit: maybe prog_fd instead of bpf_fd? Should be more consistent with
other places in libbf:

$ grep -Iriw bpf_fd | wc -l
2
$ grep -Iriw prog_fd | wc -l
39

> +
> +	bpf_fd = bpf_program__fd(prog);
> +	if (bpf_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, bpf_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 v3 bpf-next 3/9] libbpf: add ability to attach/detach BPF program to perf event

[Stanislav Fomichev]

On 06/28, Andrii Nakryiko wrote:
> On Fri, Jun 28, 2019 at 11:00 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
> >
> > On 06/27, 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   | 58 ++++++++++++++++++++++++++++++++++++++++
> > >  tools/lib/bpf/libbpf.h   |  3 +++
> > >  tools/lib/bpf/libbpf.map |  1 +
> > >  3 files changed, 62 insertions(+)
> > >
> > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > index 455795e6f8af..606705f878ba 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,63 @@ 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);
> > > +     close(l->fd);
> > > +     return err;
> > Why not return -errno from ioctl here (as you do elsewhere)?
> 
> Good catch, will fix, thanks!
> 
> As an aside, this whole returning error on close/destroy is a bit
> moot, as there is little one can do if any of teardown steps fail
> (except crash, which is not great response :) ). So the strategy would
> be to still free all memory and try to close all FDs, before returning
> error (again, which one, first, last? eh..).
Agreed, it's like nobody cares about close() return code. So don't
bother with this fix unless you'd do another respin for a different
reason.

> > > +}
> > > +
> > > +struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
> > > +                                             int pfd)
> > > +{
> > > +     char errmsg[STRERR_BUFSIZE];
> > > +     struct bpf_link_fd *link;
> > > +     int bpf_fd, err;
> > > +
> > > +     bpf_fd = bpf_program__fd(prog);
> > > +     if (bpf_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, bpf_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 v3 bpf-next 3/9] libbpf: add ability to attach/detach BPF program to perf event

[Andrii Nakryiko]

On Fri, Jun 28, 2019 at 11:14 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> On 06/28, Andrii Nakryiko wrote:
> > On Fri, Jun 28, 2019 at 11:00 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
> > >
> > > On 06/27, 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   | 58 ++++++++++++++++++++++++++++++++++++++++
> > > >  tools/lib/bpf/libbpf.h   |  3 +++
> > > >  tools/lib/bpf/libbpf.map |  1 +
> > > >  3 files changed, 62 insertions(+)
> > > >
> > > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > > index 455795e6f8af..606705f878ba 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,63 @@ 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);
> > > > +     close(l->fd);
> > > > +     return err;
> > > Why not return -errno from ioctl here (as you do elsewhere)?
> >
> > Good catch, will fix, thanks!
> >
> > As an aside, this whole returning error on close/destroy is a bit
> > moot, as there is little one can do if any of teardown steps fail
> > (except crash, which is not great response :) ). So the strategy would
> > be to still free all memory and try to close all FDs, before returning
> > error (again, which one, first, last? eh..).
> Agreed, it's like nobody cares about close() return code. So don't
> bother with this fix unless you'd do another respin for a different
> reason.

I still want something more meaningful than -1 :) Will renamed bpf_fd as well.

>
> > > > +}
> > > > +
> > > > +struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
> > > > +                                             int pfd)
> > > > +{
> > > > +     char errmsg[STRERR_BUFSIZE];
> > > > +     struct bpf_link_fd *link;
> > > > +     int bpf_fd, err;
> > > > +
> > > > +     bpf_fd = bpf_program__fd(prog);
> > > > +     if (bpf_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, bpf_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 v3 bpf-next 1/9] libbpf: make libbpf_strerror_r agnostic to sign of error

[Song Liu]

On Thu, Jun 27, 2019 at 10:53 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> 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
>

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

[Song Liu]

On Thu, Jun 27, 2019 at 10:53 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> 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>

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

[Song Liu]

On Thu, Jun 27, 2019 at 10:53 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>
> ---
>  tools/lib/bpf/libbpf.c   | 213 +++++++++++++++++++++++++++++++++++++++
>  tools/lib/bpf/libbpf.h   |   7 ++
>  tools/lib/bpf/libbpf.map |   2 +
>  3 files changed, 222 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 606705f878ba..65d2fef41003 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4016,6 +4016,219 @@ struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
>         return (struct bpf_link *)link;
>  }
>
> +static int parse_uint(const char *buf)
> +{
> +       int ret;
> +
> +       errno = 0;
> +       ret = (int)strtol(buf, NULL, 10);
> +       if (errno) {
> +               ret = -errno;
> +               pr_debug("failed to parse '%s' as unsigned int\n", buf);
> +               return ret;
> +       }
> +       if (ret < 0) {
> +               pr_debug("failed to parse '%s' as unsigned int\n", buf);
> +               return -EINVAL;
> +       }
> +       return ret;
> +}
> +
> +static int parse_uint_from_file(const char* file)
> +{
> +       char buf[STRERR_BUFSIZE];
> +       int fd, ret;
> +
> +       fd = open(file, O_RDONLY);
> +       if (fd < 0) {
> +               ret = -errno;
> +               pr_debug("failed to open '%s': %s\n", file,
> +                        libbpf_strerror_r(ret, buf, sizeof(buf)));
> +               return ret;
> +       }
> +       ret = read(fd, buf, sizeof(buf));
> +       ret = ret < 0 ? -errno : ret;
> +       close(fd);
> +       if (ret < 0) {
> +               pr_debug("failed to read '%s': %s\n", file,
> +                       libbpf_strerror_r(ret, buf, sizeof(buf)));
> +               return ret;
> +       }
> +       if (ret == 0 || ret >= sizeof(buf)) {
> +               buf[sizeof(buf) - 1] = 0;
> +               pr_debug("unexpected input from '%s': '%s'\n", file, buf);
> +               return -EINVAL;
> +       }
> +       return parse_uint(buf);
> +}
> +
> +static int determine_kprobe_perf_type(void)
> +{
> +       const char *file = "/sys/bus/event_source/devices/kprobe/type";
> +       return parse_uint_from_file(file);
> +}
> +
> +static int determine_uprobe_perf_type(void)
> +{
> +       const char *file = "/sys/bus/event_source/devices/uprobe/type";
> +       return parse_uint_from_file(file);
> +}
> +
> +static int parse_config_from_file(const char *file)
> +{
> +       char buf[STRERR_BUFSIZE];
> +       int fd, ret;
> +
> +       fd = open(file, O_RDONLY);
> +       if (fd < 0) {
> +               ret = -errno;
> +               pr_debug("failed to open '%s': %s\n", file,
> +                        libbpf_strerror_r(ret, buf, sizeof(buf)));
> +               return ret;
> +       }
> +       ret = read(fd, buf, sizeof(buf));
> +       ret = ret < 0 ? -errno : ret;
> +       close(fd);
> +       if (ret < 0) {
> +               pr_debug("failed to read '%s': %s\n", file,
> +                       libbpf_strerror_r(ret, buf, sizeof(buf)));
> +               return ret;
> +       }
> +       if (ret == 0 || ret >= sizeof(buf)) {
> +               buf[sizeof(buf) - 1] = 0;
> +               pr_debug("unexpected input from '%s': '%s'\n", file, buf);
> +               return -EINVAL;
> +       }
> +       if (strncmp(buf, "config:", 7)) {
> +               pr_debug("expected 'config:' prefix, found '%s'\n", buf);
> +               return -EINVAL;
> +       }
> +       return parse_uint(buf + 7);
> +}
> +
> +static int determine_kprobe_retprobe_bit(void)
> +{
> +       const char *file = "/sys/bus/event_source/devices/kprobe/format/retprobe";
> +       return parse_config_from_file(file);
> +}
> +
> +static int determine_uprobe_retprobe_bit(void)
> +{
> +       const char *file = "/sys/bus/event_source/devices/uprobe/format/retprobe";
> +       return parse_config_from_file(file);
> +}

Can we do the above with fscanf? Would that be easier?

> +
> +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)));

We have another warning in bpf_program__attach_[k|u]probe(). I guess
we can remove this one here.

> +               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
>

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

[Song Liu]

On Thu, Jun 27, 2019 at 10:53 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> Allow attaching BPF programs to kernel tracepoint BPF hooks specified by
> category and name.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>  tools/lib/bpf/libbpf.c   | 78 ++++++++++++++++++++++++++++++++++++++++
>  tools/lib/bpf/libbpf.h   |  4 +++
>  tools/lib/bpf/libbpf.map |  1 +
>  3 files changed, 83 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 65d2fef41003..76d1599a7d56 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4229,6 +4229,84 @@ 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_uint_from_file(file);
> +}
> +
> +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;
> +       }

Similar to the 4/9, I guess we can remove some duplicated pr_warning().

Thanks,
Song

> +       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
>

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

[Song Liu]

On Thu, Jun 27, 2019 at 10:53 PM Andrii Nakryiko <andriin@fb.com> 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>

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

[Andrii Nakryiko]

On Fri, Jun 28, 2019 at 12:46 PM Song Liu <liu.song.a23@gmail.com> wrote:
>
> On Thu, Jun 27, 2019 at 10:53 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>
> > ---
> >  tools/lib/bpf/libbpf.c   | 213 +++++++++++++++++++++++++++++++++++++++
> >  tools/lib/bpf/libbpf.h   |   7 ++
> >  tools/lib/bpf/libbpf.map |   2 +
> >  3 files changed, 222 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 606705f878ba..65d2fef41003 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -4016,6 +4016,219 @@ struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
> >         return (struct bpf_link *)link;
> >  }
> >
> > +static int parse_uint(const char *buf)
> > +{
> > +       int ret;
> > +
> > +       errno = 0;
> > +       ret = (int)strtol(buf, NULL, 10);
> > +       if (errno) {
> > +               ret = -errno;
> > +               pr_debug("failed to parse '%s' as unsigned int\n", buf);
> > +               return ret;
> > +       }
> > +       if (ret < 0) {
> > +               pr_debug("failed to parse '%s' as unsigned int\n", buf);
> > +               return -EINVAL;
> > +       }
> > +       return ret;
> > +}
> > +
> > +static int parse_uint_from_file(const char* file)
> > +{
> > +       char buf[STRERR_BUFSIZE];
> > +       int fd, ret;
> > +
> > +       fd = open(file, O_RDONLY);
> > +       if (fd < 0) {
> > +               ret = -errno;
> > +               pr_debug("failed to open '%s': %s\n", file,
> > +                        libbpf_strerror_r(ret, buf, sizeof(buf)));
> > +               return ret;
> > +       }
> > +       ret = read(fd, buf, sizeof(buf));
> > +       ret = ret < 0 ? -errno : ret;
> > +       close(fd);
> > +       if (ret < 0) {
> > +               pr_debug("failed to read '%s': %s\n", file,
> > +                       libbpf_strerror_r(ret, buf, sizeof(buf)));
> > +               return ret;
> > +       }
> > +       if (ret == 0 || ret >= sizeof(buf)) {
> > +               buf[sizeof(buf) - 1] = 0;
> > +               pr_debug("unexpected input from '%s': '%s'\n", file, buf);
> > +               return -EINVAL;
> > +       }
> > +       return parse_uint(buf);
> > +}
> > +
> > +static int determine_kprobe_perf_type(void)
> > +{
> > +       const char *file = "/sys/bus/event_source/devices/kprobe/type";
> > +       return parse_uint_from_file(file);
> > +}
> > +
> > +static int determine_uprobe_perf_type(void)
> > +{
> > +       const char *file = "/sys/bus/event_source/devices/uprobe/type";
> > +       return parse_uint_from_file(file);
> > +}
> > +
> > +static int parse_config_from_file(const char *file)
> > +{
> > +       char buf[STRERR_BUFSIZE];
> > +       int fd, ret;
> > +
> > +       fd = open(file, O_RDONLY);
> > +       if (fd < 0) {
> > +               ret = -errno;
> > +               pr_debug("failed to open '%s': %s\n", file,
> > +                        libbpf_strerror_r(ret, buf, sizeof(buf)));
> > +               return ret;
> > +       }
> > +       ret = read(fd, buf, sizeof(buf));
> > +       ret = ret < 0 ? -errno : ret;
> > +       close(fd);
> > +       if (ret < 0) {
> > +               pr_debug("failed to read '%s': %s\n", file,
> > +                       libbpf_strerror_r(ret, buf, sizeof(buf)));
> > +               return ret;
> > +       }
> > +       if (ret == 0 || ret >= sizeof(buf)) {
> > +               buf[sizeof(buf) - 1] = 0;
> > +               pr_debug("unexpected input from '%s': '%s'\n", file, buf);
> > +               return -EINVAL;
> > +       }
> > +       if (strncmp(buf, "config:", 7)) {
> > +               pr_debug("expected 'config:' prefix, found '%s'\n", buf);
> > +               return -EINVAL;
> > +       }
> > +       return parse_uint(buf + 7);
> > +}
> > +
> > +static int determine_kprobe_retprobe_bit(void)
> > +{
> > +       const char *file = "/sys/bus/event_source/devices/kprobe/format/retprobe";
> > +       return parse_config_from_file(file);
> > +}
> > +
> > +static int determine_uprobe_retprobe_bit(void)
> > +{
> > +       const char *file = "/sys/bus/event_source/devices/uprobe/format/retprobe";
> > +       return parse_config_from_file(file);
> > +}
>
> Can we do the above with fscanf? Would that be easier?

It would be less code, but also less strict semantics. E.g., fscanf
would happily leave out any garbage after number (e.g., 123blablabla,
would still parse). Also, from brief googling, fscanf doesn't handle
overflows well.

So I guess I'd vote for this more verbose, but also more strict
checking, unless you insist on fscanf.

>
> > +
> > +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)));
>
> We have another warning in bpf_program__attach_[k|u]probe(). I guess
> we can remove this one here.

This points specifically to perf_event_open() failing versus other
possible failures. Messages in attach_{k,u}probe won't have that, they
will repeat more generic "failed to attach" message. Believe me, if
something goes wrong in libbpf, I'd rather have too much logging than
too little :)

>
> > +               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
> >

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

[Song Liu]

On Fri, Jun 28, 2019 at 12:59 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Fri, Jun 28, 2019 at 12:46 PM Song Liu <liu.song.a23@gmail.com> wrote:
> >
> > On Thu, Jun 27, 2019 at 10:53 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>
> > > ---
> > >  tools/lib/bpf/libbpf.c   | 213 +++++++++++++++++++++++++++++++++++++++
> > >  tools/lib/bpf/libbpf.h   |   7 ++
> > >  tools/lib/bpf/libbpf.map |   2 +
> > >  3 files changed, 222 insertions(+)
> > >
> > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > index 606705f878ba..65d2fef41003 100644
> > > --- a/tools/lib/bpf/libbpf.c
> > > +++ b/tools/lib/bpf/libbpf.c
> > > @@ -4016,6 +4016,219 @@ struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
> > >         return (struct bpf_link *)link;
> > >  }
> > >
> > > +static int parse_uint(const char *buf)
> > > +{
> > > +       int ret;
> > > +
> > > +       errno = 0;
> > > +       ret = (int)strtol(buf, NULL, 10);
> > > +       if (errno) {
> > > +               ret = -errno;
> > > +               pr_debug("failed to parse '%s' as unsigned int\n", buf);
> > > +               return ret;
> > > +       }
> > > +       if (ret < 0) {
> > > +               pr_debug("failed to parse '%s' as unsigned int\n", buf);
> > > +               return -EINVAL;
> > > +       }
> > > +       return ret;
> > > +}
> > > +
> > > +static int parse_uint_from_file(const char* file)
> > > +{
> > > +       char buf[STRERR_BUFSIZE];
> > > +       int fd, ret;
> > > +
> > > +       fd = open(file, O_RDONLY);
> > > +       if (fd < 0) {
> > > +               ret = -errno;
> > > +               pr_debug("failed to open '%s': %s\n", file,
> > > +                        libbpf_strerror_r(ret, buf, sizeof(buf)));
> > > +               return ret;
> > > +       }
> > > +       ret = read(fd, buf, sizeof(buf));
> > > +       ret = ret < 0 ? -errno : ret;
> > > +       close(fd);
> > > +       if (ret < 0) {
> > > +               pr_debug("failed to read '%s': %s\n", file,
> > > +                       libbpf_strerror_r(ret, buf, sizeof(buf)));
> > > +               return ret;
> > > +       }
> > > +       if (ret == 0 || ret >= sizeof(buf)) {
> > > +               buf[sizeof(buf) - 1] = 0;
> > > +               pr_debug("unexpected input from '%s': '%s'\n", file, buf);
> > > +               return -EINVAL;
> > > +       }
> > > +       return parse_uint(buf);
> > > +}
> > > +
> > > +static int determine_kprobe_perf_type(void)
> > > +{
> > > +       const char *file = "/sys/bus/event_source/devices/kprobe/type";
> > > +       return parse_uint_from_file(file);
> > > +}
> > > +
> > > +static int determine_uprobe_perf_type(void)
> > > +{
> > > +       const char *file = "/sys/bus/event_source/devices/uprobe/type";
> > > +       return parse_uint_from_file(file);
> > > +}
> > > +
> > > +static int parse_config_from_file(const char *file)
> > > +{
> > > +       char buf[STRERR_BUFSIZE];
> > > +       int fd, ret;
> > > +
> > > +       fd = open(file, O_RDONLY);
> > > +       if (fd < 0) {
> > > +               ret = -errno;
> > > +               pr_debug("failed to open '%s': %s\n", file,
> > > +                        libbpf_strerror_r(ret, buf, sizeof(buf)));
> > > +               return ret;
> > > +       }
> > > +       ret = read(fd, buf, sizeof(buf));
> > > +       ret = ret < 0 ? -errno : ret;
> > > +       close(fd);
> > > +       if (ret < 0) {
> > > +               pr_debug("failed to read '%s': %s\n", file,
> > > +                       libbpf_strerror_r(ret, buf, sizeof(buf)));
> > > +               return ret;
> > > +       }
> > > +       if (ret == 0 || ret >= sizeof(buf)) {
> > > +               buf[sizeof(buf) - 1] = 0;
> > > +               pr_debug("unexpected input from '%s': '%s'\n", file, buf);
> > > +               return -EINVAL;
> > > +       }
> > > +       if (strncmp(buf, "config:", 7)) {
> > > +               pr_debug("expected 'config:' prefix, found '%s'\n", buf);
> > > +               return -EINVAL;
> > > +       }
> > > +       return parse_uint(buf + 7);
> > > +}
> > > +
> > > +static int determine_kprobe_retprobe_bit(void)
> > > +{
> > > +       const char *file = "/sys/bus/event_source/devices/kprobe/format/retprobe";
> > > +       return parse_config_from_file(file);
> > > +}
> > > +
> > > +static int determine_uprobe_retprobe_bit(void)
> > > +{
> > > +       const char *file = "/sys/bus/event_source/devices/uprobe/format/retprobe";
> > > +       return parse_config_from_file(file);
> > > +}
> >
> > Can we do the above with fscanf? Would that be easier?
>
> It would be less code, but also less strict semantics. E.g., fscanf
> would happily leave out any garbage after number (e.g., 123blablabla,
> would still parse). Also, from brief googling, fscanf doesn't handle
> overflows well.
>
> So I guess I'd vote for this more verbose, but also more strict
> checking, unless you insist on fscanf.

I don't think we need to worry about kernel giving garbage in sysfs.
Most common error gonna be the file doesn't exist. Error messages
like "Failed to parse <filename>" would be sufficient.

Let's keep it simpler.

>
> >
> > > +
> > > +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)));
> >
> > We have another warning in bpf_program__attach_[k|u]probe(). I guess
> > we can remove this one here.
>
> This points specifically to perf_event_open() failing versus other
> possible failures. Messages in attach_{k,u}probe won't have that, they
> will repeat more generic "failed to attach" message. Believe me, if
> something goes wrong in libbpf, I'd rather have too much logging than
> too little :)
>

Fair enough. Let's be verbose here. :)

Song

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

[Song Liu]

On Thu, Jun 27, 2019 at 10:53 PM Andrii Nakryiko <andriin@fb.com> 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);
>  close_prog:
>         bpf_object__close(obj);
>  }
> --
> 2.17.1
>

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

[Song Liu]

On Thu, Jun 27, 2019 at 10:54 PM Andrii Nakryiko <andriin@fb.com> 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);
> +       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
>

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

[Song Liu]

On Thu, Jun 27, 2019 at 10:53 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> 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(-)

More deletions. Nice!

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

[Song Liu]

On Fri, Jun 28, 2019 at 12:50 PM Song Liu <liu.song.a23@gmail.com> wrote:
>
> On Thu, Jun 27, 2019 at 10:53 PM Andrii Nakryiko <andriin@fb.com> 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   | 78 ++++++++++++++++++++++++++++++++++++++++
> >  tools/lib/bpf/libbpf.h   |  4 +++
> >  tools/lib/bpf/libbpf.map |  1 +
> >  3 files changed, 83 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 65d2fef41003..76d1599a7d56 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -4229,6 +4229,84 @@ 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_uint_from_file(file);
> > +}
> > +
> > +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;
> > +       }
>
> Similar to the 4/9, I guess we can remove some duplicated pr_warning().
>
> Thanks,
> Song
>
> > +       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
> >

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

[Andrii Nakryiko]

On Fri, Jun 28, 2019 at 1:09 PM Song Liu <liu.song.a23@gmail.com> wrote:
>
> On Fri, Jun 28, 2019 at 12:59 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Fri, Jun 28, 2019 at 12:46 PM Song Liu <liu.song.a23@gmail.com> wrote:
> > >
> > > On Thu, Jun 27, 2019 at 10:53 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>
> > > > ---
> > > >  tools/lib/bpf/libbpf.c   | 213 +++++++++++++++++++++++++++++++++++++++
> > > >  tools/lib/bpf/libbpf.h   |   7 ++
> > > >  tools/lib/bpf/libbpf.map |   2 +
> > > >  3 files changed, 222 insertions(+)
> > > >
> > > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > > index 606705f878ba..65d2fef41003 100644
> > > > --- a/tools/lib/bpf/libbpf.c
> > > > +++ b/tools/lib/bpf/libbpf.c
> > > > @@ -4016,6 +4016,219 @@ struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
> > > >         return (struct bpf_link *)link;
> > > >  }
> > > >
> > > > +static int parse_uint(const char *buf)
> > > > +{
> > > > +       int ret;
> > > > +
> > > > +       errno = 0;
> > > > +       ret = (int)strtol(buf, NULL, 10);
> > > > +       if (errno) {
> > > > +               ret = -errno;
> > > > +               pr_debug("failed to parse '%s' as unsigned int\n", buf);
> > > > +               return ret;
> > > > +       }
> > > > +       if (ret < 0) {
> > > > +               pr_debug("failed to parse '%s' as unsigned int\n", buf);
> > > > +               return -EINVAL;
> > > > +       }
> > > > +       return ret;
> > > > +}
> > > > +
> > > > +static int parse_uint_from_file(const char* file)
> > > > +{
> > > > +       char buf[STRERR_BUFSIZE];
> > > > +       int fd, ret;
> > > > +
> > > > +       fd = open(file, O_RDONLY);
> > > > +       if (fd < 0) {
> > > > +               ret = -errno;
> > > > +               pr_debug("failed to open '%s': %s\n", file,
> > > > +                        libbpf_strerror_r(ret, buf, sizeof(buf)));
> > > > +               return ret;
> > > > +       }
> > > > +       ret = read(fd, buf, sizeof(buf));
> > > > +       ret = ret < 0 ? -errno : ret;
> > > > +       close(fd);
> > > > +       if (ret < 0) {
> > > > +               pr_debug("failed to read '%s': %s\n", file,
> > > > +                       libbpf_strerror_r(ret, buf, sizeof(buf)));
> > > > +               return ret;
> > > > +       }
> > > > +       if (ret == 0 || ret >= sizeof(buf)) {
> > > > +               buf[sizeof(buf) - 1] = 0;
> > > > +               pr_debug("unexpected input from '%s': '%s'\n", file, buf);
> > > > +               return -EINVAL;
> > > > +       }
> > > > +       return parse_uint(buf);
> > > > +}
> > > > +
> > > > +static int determine_kprobe_perf_type(void)
> > > > +{
> > > > +       const char *file = "/sys/bus/event_source/devices/kprobe/type";
> > > > +       return parse_uint_from_file(file);
> > > > +}
> > > > +
> > > > +static int determine_uprobe_perf_type(void)
> > > > +{
> > > > +       const char *file = "/sys/bus/event_source/devices/uprobe/type";
> > > > +       return parse_uint_from_file(file);
> > > > +}
> > > > +
> > > > +static int parse_config_from_file(const char *file)
> > > > +{
> > > > +       char buf[STRERR_BUFSIZE];
> > > > +       int fd, ret;
> > > > +
> > > > +       fd = open(file, O_RDONLY);
> > > > +       if (fd < 0) {
> > > > +               ret = -errno;
> > > > +               pr_debug("failed to open '%s': %s\n", file,
> > > > +                        libbpf_strerror_r(ret, buf, sizeof(buf)));
> > > > +               return ret;
> > > > +       }
> > > > +       ret = read(fd, buf, sizeof(buf));
> > > > +       ret = ret < 0 ? -errno : ret;
> > > > +       close(fd);
> > > > +       if (ret < 0) {
> > > > +               pr_debug("failed to read '%s': %s\n", file,
> > > > +                       libbpf_strerror_r(ret, buf, sizeof(buf)));
> > > > +               return ret;
> > > > +       }
> > > > +       if (ret == 0 || ret >= sizeof(buf)) {
> > > > +               buf[sizeof(buf) - 1] = 0;
> > > > +               pr_debug("unexpected input from '%s': '%s'\n", file, buf);
> > > > +               return -EINVAL;
> > > > +       }
> > > > +       if (strncmp(buf, "config:", 7)) {
> > > > +               pr_debug("expected 'config:' prefix, found '%s'\n", buf);
> > > > +               return -EINVAL;
> > > > +       }
> > > > +       return parse_uint(buf + 7);
> > > > +}
> > > > +
> > > > +static int determine_kprobe_retprobe_bit(void)
> > > > +{
> > > > +       const char *file = "/sys/bus/event_source/devices/kprobe/format/retprobe";
> > > > +       return parse_config_from_file(file);
> > > > +}
> > > > +
> > > > +static int determine_uprobe_retprobe_bit(void)
> > > > +{
> > > > +       const char *file = "/sys/bus/event_source/devices/uprobe/format/retprobe";
> > > > +       return parse_config_from_file(file);
> > > > +}
> > >
> > > Can we do the above with fscanf? Would that be easier?
> >
> > It would be less code, but also less strict semantics. E.g., fscanf
> > would happily leave out any garbage after number (e.g., 123blablabla,
> > would still parse). Also, from brief googling, fscanf doesn't handle
> > overflows well.
> >
> > So I guess I'd vote for this more verbose, but also more strict
> > checking, unless you insist on fscanf.
>
> I don't think we need to worry about kernel giving garbage in sysfs.
> Most common error gonna be the file doesn't exist. Error messages
> like "Failed to parse <filename>" would be sufficient.
>
> Let's keep it simpler.

Ok, will switch to fscanf.

>
> >
> > >
> > > > +
> > > > +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)));
> > >
> > > We have another warning in bpf_program__attach_[k|u]probe(). I guess
> > > we can remove this one here.
> >
> > This points specifically to perf_event_open() failing versus other
> > possible failures. Messages in attach_{k,u}probe won't have that, they
> > will repeat more generic "failed to attach" message. Believe me, if
> > something goes wrong in libbpf, I'd rather have too much logging than
> > too little :)
> >
>
> Fair enough. Let's be verbose here. :)
>
> Song

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

[Andrii Nakryiko]

On Mon, Jul 8, 2019 at 12:27 PM Matt Hart <matthew.hart@linaro.org> wrote:
>
> On Mon, 8 Jul 2019 at 18:58, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >
> > On Mon, Jul 8, 2019 at 8:11 AM Matt Hart <matthew.hart@linaro.org> wrote:
> > >
> > > Hi all,
> > >
> > > I bisected a perf build error on ARMv7 to this patch:
> > > libbpf.c: In function ‘perf_event_open_probe’:
> > > libbpf.c:4112:17: error: cast from pointer to integer of different
> > > size [-Werror=pointer-to-int-cast]
> > >   attr.config1 = (uint64_t)(void *)name; /* kprobe_func or uprobe_path */
> > >                  ^
> > >
> > > Is this a known issue?
> >
> > No, thanks for reporting!
> >
> > It should be
> >
> > attr.config1 = (uint64_t)(uintptr_t)(void *)name;
> >
> > to avoid warning on 32-bit architectures.
>
> Tested with manual change and can confirm perf now builds without errors.

Thanks for testing!

I'll add Tested-by: Matt Hart <matthew.hart@linaro.org> when posting a fix.

>
> >
> > I'll post a fix later today, but if you could verify this fixes
> > warning for you, I'd really appreciate that! Thanks!

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

[Matt Hart]

On Mon, 8 Jul 2019 at 18:58, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Mon, Jul 8, 2019 at 8:11 AM Matt Hart <matthew.hart@linaro.org> wrote:
> >
> > Hi all,
> >
> > I bisected a perf build error on ARMv7 to this patch:
> > libbpf.c: In function ‘perf_event_open_probe’:
> > libbpf.c:4112:17: error: cast from pointer to integer of different
> > size [-Werror=pointer-to-int-cast]
> >   attr.config1 = (uint64_t)(void *)name; /* kprobe_func or uprobe_path */
> >                  ^
> >
> > Is this a known issue?
>
> No, thanks for reporting!
>
> It should be
>
> attr.config1 = (uint64_t)(uintptr_t)(void *)name;
>
> to avoid warning on 32-bit architectures.

Tested with manual change and can confirm perf now builds without errors.

>
> I'll post a fix later today, but if you could verify this fixes
> warning for you, I'd really appreciate that! Thanks!

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

[Andrii Nakryiko]

On Mon, Jul 8, 2019 at 8:11 AM Matt Hart <matthew.hart@linaro.org> wrote:
>
> Hi all,
>
> I bisected a perf build error on ARMv7 to this patch:
> libbpf.c: In function ‘perf_event_open_probe’:
> libbpf.c:4112:17: error: cast from pointer to integer of different
> size [-Werror=pointer-to-int-cast]
>   attr.config1 = (uint64_t)(void *)name; /* kprobe_func or uprobe_path */
>                  ^
>
> Is this a known issue?

No, thanks for reporting!

It should be

attr.config1 = (uint64_t)(uintptr_t)(void *)name;

to avoid warning on 32-bit architectures.

I'll post a fix later today, but if you could verify this fixes
warning for you, I'd really appreciate that! Thanks!

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

[Matt Hart]

Hi all,

I bisected a perf build error on ARMv7 to this patch:
libbpf.c: In function ‘perf_event_open_probe’:
libbpf.c:4112:17: error: cast from pointer to integer of different
size [-Werror=pointer-to-int-cast]
  attr.config1 = (uint64_t)(void *)name; /* kprobe_func or uprobe_path */
                 ^

Is this a known issue?