* Re: [PATCH v3 bpf-next 4/9] libbpf: add kprobe/uprobe attach API @ 2019-07-08 15:11 Matt Hart 2019-07-08 17:58 ` Andrii Nakryiko 0 siblings, 1 reply; 9+ messages in thread From: Matt Hart @ 2019-07-08 15:11 UTC (permalink / raw) To: andriin; +Cc: andrii.nakryiko, ast, bpf, daniel, kernel-team, netdev, sdf 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? ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 bpf-next 4/9] libbpf: add kprobe/uprobe attach API 2019-07-08 15:11 [PATCH v3 bpf-next 4/9] libbpf: add kprobe/uprobe attach API Matt Hart @ 2019-07-08 17:58 ` Andrii Nakryiko 2019-07-08 19:27 ` Matt Hart 0 siblings, 1 reply; 9+ messages in thread From: Andrii Nakryiko @ 2019-07-08 17:58 UTC (permalink / raw) To: Matt Hart Cc: Andrii Nakryiko, Alexei Starovoitov, bpf, Daniel Borkmann, Kernel Team, Networking, Stanislav Fomichev 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! ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 bpf-next 4/9] libbpf: add kprobe/uprobe attach API 2019-07-08 17:58 ` Andrii Nakryiko @ 2019-07-08 19:27 ` Matt Hart 2019-07-08 19:55 ` Andrii Nakryiko 0 siblings, 1 reply; 9+ messages in thread From: Matt Hart @ 2019-07-08 19:27 UTC (permalink / raw) To: Andrii Nakryiko Cc: Andrii Nakryiko, Alexei Starovoitov, bpf, Daniel Borkmann, Kernel Team, Networking, Stanislav Fomichev 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! ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 bpf-next 4/9] libbpf: add kprobe/uprobe attach API 2019-07-08 19:27 ` Matt Hart @ 2019-07-08 19:55 ` Andrii Nakryiko 0 siblings, 0 replies; 9+ messages in thread From: Andrii Nakryiko @ 2019-07-08 19:55 UTC (permalink / raw) To: Matt Hart Cc: Andrii Nakryiko, Alexei Starovoitov, bpf, Daniel Borkmann, Kernel Team, Networking, Stanislav Fomichev 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! ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 bpf-next 0/9] libbpf: add bpf_link and tracing attach APIs @ 2019-06-28 5:52 Andrii Nakryiko 2019-06-28 5:52 ` [PATCH v3 bpf-next 4/9] libbpf: add kprobe/uprobe attach API Andrii Nakryiko 0 siblings, 1 reply; 9+ messages in thread From: Andrii Nakryiko @ 2019-06-28 5:52 UTC (permalink / raw) To: andrii.nakryiko, ast, daniel, netdev, bpf, sdf, kernel-team Cc: 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 bpf-next 4/9] libbpf: add kprobe/uprobe attach API 2019-06-28 5:52 [PATCH v3 bpf-next 0/9] libbpf: add bpf_link and tracing attach APIs Andrii Nakryiko @ 2019-06-28 5:52 ` Andrii Nakryiko 2019-06-28 19:46 ` Song Liu 0 siblings, 1 reply; 9+ messages in thread From: Andrii Nakryiko @ 2019-06-28 5:52 UTC (permalink / raw) To: andrii.nakryiko, ast, daniel, netdev, bpf, sdf, kernel-team Cc: 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 bpf-next 4/9] libbpf: add kprobe/uprobe attach API 2019-06-28 5:52 ` [PATCH v3 bpf-next 4/9] libbpf: add kprobe/uprobe attach API Andrii Nakryiko @ 2019-06-28 19:46 ` Song Liu 2019-06-28 19:59 ` Andrii Nakryiko 0 siblings, 1 reply; 9+ messages in thread From: Song Liu @ 2019-06-28 19:46 UTC (permalink / raw) To: Andrii Nakryiko Cc: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Networking, bpf, Stanislav Fomichev, Kernel Team 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 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 bpf-next 4/9] libbpf: add kprobe/uprobe attach API 2019-06-28 19:46 ` Song Liu @ 2019-06-28 19:59 ` Andrii Nakryiko 2019-06-28 20:09 ` Song Liu 0 siblings, 1 reply; 9+ messages in thread From: Andrii Nakryiko @ 2019-06-28 19:59 UTC (permalink / raw) To: Song Liu Cc: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Networking, bpf, Stanislav Fomichev, Kernel Team 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 > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 bpf-next 4/9] libbpf: add kprobe/uprobe attach API 2019-06-28 19:59 ` Andrii Nakryiko @ 2019-06-28 20:09 ` Song Liu 2019-06-28 20:34 ` Andrii Nakryiko 0 siblings, 1 reply; 9+ messages in thread From: Song Liu @ 2019-06-28 20:09 UTC (permalink / raw) To: Andrii Nakryiko Cc: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Networking, bpf, Stanislav Fomichev, Kernel Team 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 bpf-next 4/9] libbpf: add kprobe/uprobe attach API 2019-06-28 20:09 ` Song Liu @ 2019-06-28 20:34 ` Andrii Nakryiko 0 siblings, 0 replies; 9+ messages in thread From: Andrii Nakryiko @ 2019-06-28 20:34 UTC (permalink / raw) To: Song Liu Cc: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Networking, bpf, Stanislav Fomichev, Kernel Team 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-07-08 19:55 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-08 15:11 [PATCH v3 bpf-next 4/9] libbpf: add kprobe/uprobe attach API Matt Hart 2019-07-08 17:58 ` Andrii Nakryiko 2019-07-08 19:27 ` Matt Hart 2019-07-08 19:55 ` Andrii Nakryiko -- strict thread matches above, loose matches on Subject: below -- 2019-06-28 5:52 [PATCH v3 bpf-next 0/9] libbpf: add bpf_link and tracing attach APIs Andrii Nakryiko 2019-06-28 5:52 ` [PATCH v3 bpf-next 4/9] libbpf: add kprobe/uprobe attach API Andrii Nakryiko 2019-06-28 19:46 ` Song Liu 2019-06-28 19:59 ` Andrii Nakryiko 2019-06-28 20:09 ` Song Liu 2019-06-28 20:34 ` Andrii Nakryiko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).