| Message ID | 20210318062520.3838605-1-rafaeldtinoco@ubuntu.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | [RFC] libbpf: support kprobe/kretprobe events in legacy environments | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Wed, Mar 17, 2021 at 11:25 PM Rafael David Tinoco <rafaeldtinoco@ubuntu.com> wrote: > > * Request for comments version (still needs polishing). > * Based on Andrii Nakryiko's suggestion. > * Using bpf_program__set_priv in attach_kprobe() for kprobe cleanup. no-no-no, set_priv() is going to be deprecated and removed (see [0]), and is not the right mechanism here. Detachment should happen in bpf_link__destroy(). [0] https://docs.google.com/document/d/1UyjTZuPFWiPFyKk1tV5an11_iaRuec6U-ZESZ54nNTY > > Signed-off-by: Rafael David Tinoco <rafaeldtinoco@ubuntu.com> > --- > src/libbpf.c | 100 +++++++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 90 insertions(+), 10 deletions(-) > > diff --git a/src/libbpf.c b/src/libbpf.c > index 2f351d3..4dc09d3 100644 > --- a/src/libbpf.c > +++ b/src/libbpf.c > @@ -9677,8 +9677,14 @@ static int parse_uint_from_file(const char *file, const char *fmt) > > static int determine_kprobe_perf_type(void) > { > + int ret = 0; > + struct stat s; > const char *file = "/sys/bus/event_source/devices/kprobe/type"; > > + ret = stat(file, &s); > + if (ret < 0) > + return -errno; > + > return parse_uint_from_file(file, "%d\n"); > } > > @@ -9703,25 +9709,87 @@ static int determine_uprobe_retprobe_bit(void) > return parse_uint_from_file(file, "config:%d\n"); > } > > +static int determine_kprobe_perf_type_legacy(const char *func_name) > +{ > + char file[256]; nit: I suspect 256 is much longer than necessary :) > + const char *fname = "/sys/kernel/debug/tracing/events/kprobes/%s/id"; > + > + snprintf(file, sizeof(file), fname, func_name); > + > + return parse_uint_from_file(file, "%d\n"); > +} > + > +static int poke_kprobe_events(bool add, const char *name, bool kretprobe) it's probably a good idea to put a link to [0] somewhere here [0] https://www.kernel.org/doc/html/latest/trace/kprobetrace.html > +{ > + int fd, ret = 0; > + char given[256], buf[256]; nit: given -> event_name, to follow official documentation terminology ? > + const char *file = "/sys/kernel/debug/tracing/kprobe_events"; > + > + if (kretprobe && add) what if it's kretprobe removal? shouldn't you generate the same name > + snprintf(given, sizeof(given), "kprobes/%s_ret", name); > + else > + snprintf(given, sizeof(given), "kprobes/%s", name); BCC includes PID in the name of the probe and "bcc", maybe we should do something similar? > + if (add) > + snprintf(buf, sizeof(buf),"%c:%s %s\n", kretprobe ? 'r' : 'p', given, name); > + else > + snprintf(buf, sizeof(buf), "-:%s\n", given); > + > + fd = open(file, O_WRONLY|O_APPEND, 0); > + if (!fd) > + return -errno; > + ret = write(fd, buf, strlen(buf)); > + if (ret < 0) { > + ret = -errno; > + } > + close(fd); > + > + return ret; > +} > + > +static inline int add_kprobe_event_legacy(const char* func_name, bool kretprobe) > +{ > + return poke_kprobe_events(true /*add*/, func_name, kretprobe); > +} > + > +static inline int remove_kprobe_event_legacy(const char* func_name, bool kretprobe) > +{ > + return poke_kprobe_events(false /*remove*/, func_name, kretprobe); > +} > + > 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; > + bool legacy = false; > > type = uprobe ? determine_uprobe_perf_type() > : determine_kprobe_perf_type(); > if (type < 0) { I think we should do "feature probing" to decide whether we should go with legacy or modern kprobes. And just stick to that, reporting any errors. I'm not a big fan of this generic "let's try X, if it fails for *whatever* reason, let's try Y", because you 1) can ignore some serious problem 2) you'll be getting unnecessary warnings in your log > - pr_warn("failed to determine %s perf type: %s\n", > - uprobe ? "uprobe" : "kprobe", > - libbpf_strerror_r(type, errmsg, sizeof(errmsg))); > - return type; > + if (uprobe) { > + pr_warn("failed to determine %s perf type: %s\n", > + uprobe ? "uprobe" : "kprobe", > + libbpf_strerror_r(type, errmsg, sizeof(errmsg))); > + return type; > + } > + err = add_kprobe_event_legacy(name, retprobe); > + if (err < 0) { > + pr_warn("failed to add legacy kprobe events: %s\n", > + libbpf_strerror_r(err, errmsg, sizeof(errmsg))); > + return err; > + } > + type = uprobe ? type : determine_kprobe_perf_type_legacy(name); > + if (type < 0) { > + remove_kprobe_event_legacy(name, retprobe); > + pr_warn("failed to determine kprobe perf type: %s\n", > + libbpf_strerror_r(type, errmsg, sizeof(errmsg))); > + } > + legacy = true; > } > - if (retprobe) { > + if (retprobe && !legacy) { > int bit = uprobe ? determine_uprobe_retprobe_bit() > : determine_kprobe_retprobe_bit(); > - > if (bit < 0) { > pr_warn("failed to determine %s retprobe bit: %s\n", > uprobe ? "uprobe" : "kprobe", > @@ -9731,10 +9799,14 @@ static int perf_event_open_probe(bool uprobe, bool retprobe, const char *name, > attr.config |= 1 << bit; > } > attr.size = sizeof(attr); > - attr.type = type; > - attr.config1 = ptr_to_u64(name); /* kprobe_func or uprobe_path */ > - attr.config2 = offset; /* kprobe_addr or probe_offset */ > - > + if (!legacy) { > + attr.type = type; > + attr.config1 = ptr_to_u64(name); /* kprobe_func or uprobe_path */ > + attr.config2 = offset; /* kprobe_addr or probe_offset */ > + } else { > + attr.config = type; > + attr.type = PERF_TYPE_TRACEPOINT; > + } > /* pid filter is meaningful only for uprobes */ > pfd = syscall(__NR_perf_event_open, &attr, > pid < 0 ? -1 : pid /* pid */, > @@ -9750,6 +9822,11 @@ static int perf_event_open_probe(bool uprobe, bool retprobe, const char *name, > return pfd; > } > > +void bpf_program__detach_kprobe_legacy(struct bpf_program *prog, void *retprobe) > +{ > + remove_kprobe_event_legacy(prog->name, (bool) retprobe); > +} as I mentioned, this should be done by bpf_link__destroy() > + > struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog, > bool retprobe, > const char *func_name) > @@ -9766,6 +9843,9 @@ struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog, > libbpf_strerror_r(pfd, errmsg, sizeof(errmsg))); > return ERR_PTR(pfd); > } > + > + bpf_program__set_priv(prog, (void *) retprobe, bpf_program__detach_kprobe_legacy); > + > link = bpf_program__attach_perf_event(prog, pfd); > if (IS_ERR(link)) { > close(pfd); > -- > 2.27.0 >
diff --git a/src/libbpf.c b/src/libbpf.c index 2f351d3..4dc09d3 100644 --- a/src/libbpf.c +++ b/src/libbpf.c @@ -9677,8 +9677,14 @@ static int parse_uint_from_file(const char *file, const char *fmt) static int determine_kprobe_perf_type(void) { + int ret = 0; + struct stat s; const char *file = "/sys/bus/event_source/devices/kprobe/type"; + ret = stat(file, &s); + if (ret < 0) + return -errno; + return parse_uint_from_file(file, "%d\n"); } @@ -9703,25 +9709,87 @@ static int determine_uprobe_retprobe_bit(void) return parse_uint_from_file(file, "config:%d\n"); } +static int determine_kprobe_perf_type_legacy(const char *func_name) +{ + char file[256]; + const char *fname = "/sys/kernel/debug/tracing/events/kprobes/%s/id"; + + snprintf(file, sizeof(file), fname, func_name); + + return parse_uint_from_file(file, "%d\n"); +} + +static int poke_kprobe_events(bool add, const char *name, bool kretprobe) +{ + int fd, ret = 0; + char given[256], buf[256]; + const char *file = "/sys/kernel/debug/tracing/kprobe_events"; + + if (kretprobe && add) + snprintf(given, sizeof(given), "kprobes/%s_ret", name); + else + snprintf(given, sizeof(given), "kprobes/%s", name); + if (add) + snprintf(buf, sizeof(buf),"%c:%s %s\n", kretprobe ? 'r' : 'p', given, name); + else + snprintf(buf, sizeof(buf), "-:%s\n", given); + + fd = open(file, O_WRONLY|O_APPEND, 0); + if (!fd) + return -errno; + ret = write(fd, buf, strlen(buf)); + if (ret < 0) { + ret = -errno; + } + close(fd); + + return ret; +} + +static inline int add_kprobe_event_legacy(const char* func_name, bool kretprobe) +{ + return poke_kprobe_events(true /*add*/, func_name, kretprobe); +} + +static inline int remove_kprobe_event_legacy(const char* func_name, bool kretprobe) +{ + return poke_kprobe_events(false /*remove*/, func_name, kretprobe); +} + 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; + bool legacy = false; type = uprobe ? determine_uprobe_perf_type() : determine_kprobe_perf_type(); if (type < 0) { - pr_warn("failed to determine %s perf type: %s\n", - uprobe ? "uprobe" : "kprobe", - libbpf_strerror_r(type, errmsg, sizeof(errmsg))); - return type; + if (uprobe) { + pr_warn("failed to determine %s perf type: %s\n", + uprobe ? "uprobe" : "kprobe", + libbpf_strerror_r(type, errmsg, sizeof(errmsg))); + return type; + } + err = add_kprobe_event_legacy(name, retprobe); + if (err < 0) { + pr_warn("failed to add legacy kprobe events: %s\n", + libbpf_strerror_r(err, errmsg, sizeof(errmsg))); + return err; + } + type = uprobe ? type : determine_kprobe_perf_type_legacy(name); + if (type < 0) { + remove_kprobe_event_legacy(name, retprobe); + pr_warn("failed to determine kprobe perf type: %s\n", + libbpf_strerror_r(type, errmsg, sizeof(errmsg))); + } + legacy = true; } - if (retprobe) { + if (retprobe && !legacy) { int bit = uprobe ? determine_uprobe_retprobe_bit() : determine_kprobe_retprobe_bit(); - if (bit < 0) { pr_warn("failed to determine %s retprobe bit: %s\n", uprobe ? "uprobe" : "kprobe", @@ -9731,10 +9799,14 @@ static int perf_event_open_probe(bool uprobe, bool retprobe, const char *name, attr.config |= 1 << bit; } attr.size = sizeof(attr); - attr.type = type; - attr.config1 = ptr_to_u64(name); /* kprobe_func or uprobe_path */ - attr.config2 = offset; /* kprobe_addr or probe_offset */ - + if (!legacy) { + attr.type = type; + attr.config1 = ptr_to_u64(name); /* kprobe_func or uprobe_path */ + attr.config2 = offset; /* kprobe_addr or probe_offset */ + } else { + attr.config = type; + attr.type = PERF_TYPE_TRACEPOINT; + } /* pid filter is meaningful only for uprobes */ pfd = syscall(__NR_perf_event_open, &attr, pid < 0 ? -1 : pid /* pid */, @@ -9750,6 +9822,11 @@ static int perf_event_open_probe(bool uprobe, bool retprobe, const char *name, return pfd; } +void bpf_program__detach_kprobe_legacy(struct bpf_program *prog, void *retprobe) +{ + remove_kprobe_event_legacy(prog->name, (bool) retprobe); +} + struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog, bool retprobe, const char *func_name) @@ -9766,6 +9843,9 @@ struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog, libbpf_strerror_r(pfd, errmsg, sizeof(errmsg))); return ERR_PTR(pfd); } + + bpf_program__set_priv(prog, (void *) retprobe, bpf_program__detach_kprobe_legacy); + link = bpf_program__attach_perf_event(prog, pfd); if (IS_ERR(link)) { close(pfd);
* Request for comments version (still needs polishing). * Based on Andrii Nakryiko's suggestion. * Using bpf_program__set_priv in attach_kprobe() for kprobe cleanup. Signed-off-by: Rafael David Tinoco <rafaeldtinoco@ubuntu.com> --- src/libbpf.c | 100 +++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 90 insertions(+), 10 deletions(-)