On Tue, Feb 7, 2023 at 3:59 AM Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > On Thu, Feb 2, 2023 at 7:18 PM <menglong8.dong@xxxxxxxxx> wrote: > > > > From: Menglong Dong <imagedong@xxxxxxxxxxx> > > > > By default, libbpf will attach the kprobe/uprobe eBPF program in the > > latest mode that supported by kernel. In this patch, we add the support > > to let users manually attach kprobe/uprobe in legacy or perf mode. > > > > There are 3 mode that supported by the kernel to attach kprobe/uprobe: > > > > LEGACY: create perf event in legacy way and don't use bpf_link > > PERF: create perf event with perf_event_open() and don't use bpf_link > > LINK: create perf event with perf_event_open() and use bpf_link > > > > Users now can manually choose the mode with > > bpf_program__attach_uprobe_opts()/bpf_program__attach_kprobe_opts(). > > > > Link: https://lore.kernel.org/bpf/20230113093427.1666466-1-imagedong@xxxxxxxxxxx/ > > Signed-off-by: Menglong Dong <imagedong@xxxxxxxxxxx> > > --- > > tools/lib/bpf/libbpf.c | 26 +++++++++++++++++++++++++- > > tools/lib/bpf/libbpf.h | 19 ++++++++++++++++--- > > 2 files changed, 41 insertions(+), 4 deletions(-) > > > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > > index eed5cec6f510..0d20bf1ee301 100644 > > --- a/tools/lib/bpf/libbpf.c > > +++ b/tools/lib/bpf/libbpf.c > > @@ -9784,7 +9784,7 @@ struct bpf_link *bpf_program__attach_perf_event_opts(const struct bpf_program *p > > link->link.dealloc = &bpf_link_perf_dealloc; > > link->perf_event_fd = pfd; > > > > - if (kernel_supports(prog->obj, FEAT_PERF_LINK)) { > > + if (kernel_supports(prog->obj, FEAT_PERF_LINK) && !opts->no_link) { > > DECLARE_LIBBPF_OPTS(bpf_link_create_opts, link_opts, > > .perf_event.bpf_cookie = OPTS_GET(opts, bpf_cookie, 0)); > > > > @@ -10148,16 +10148,28 @@ bpf_program__attach_kprobe_opts(const struct bpf_program *prog, > > struct bpf_link *link; > > size_t offset; > > bool retprobe, legacy; > > + enum probe_mode mode; > > int pfd, err; > > > > if (!OPTS_VALID(opts, bpf_kprobe_opts)) > > return libbpf_err_ptr(-EINVAL); > > > > + mode = OPTS_GET(opts, mode, PROBE_MODE_DEFAULT); > > retprobe = OPTS_GET(opts, retprobe, false); > > offset = OPTS_GET(opts, offset, 0); > > pe_opts.bpf_cookie = OPTS_GET(opts, bpf_cookie, 0); > > > > legacy = determine_kprobe_perf_type() < 0; > > + switch (mode) { > > + case PROBE_MODE_LEGACY: > > + legacy = true; > > + case PROBE_MODE_PERF: > > + pe_opts.no_link = true; > > + break; > > + default: > > + break; > > + } > > + > > if (!legacy) { > > pfd = perf_event_open_probe(false /* uprobe */, retprobe, > > func_name, offset, > > @@ -10817,10 +10829,12 @@ bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid, > > int pfd, err; > > bool retprobe, legacy; > > const char *func_name; > > + enum probe_mode mode; > > > > if (!OPTS_VALID(opts, bpf_uprobe_opts)) > > return libbpf_err_ptr(-EINVAL); > > > > + mode = OPTS_GET(opts, mode, PROBE_MODE_DEFAULT); > > retprobe = OPTS_GET(opts, retprobe, false); > > ref_ctr_off = OPTS_GET(opts, ref_ctr_offset, 0); > > pe_opts.bpf_cookie = OPTS_GET(opts, bpf_cookie, 0); > > @@ -10849,6 +10863,16 @@ bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid, > > } > > > > legacy = determine_uprobe_perf_type() < 0; > > + switch (mode) { > > + case PROBE_MODE_LEGACY: > > + legacy = true; > > + case PROBE_MODE_PERF: > > + pe_opts.no_link = true; > > + break; > > + default: > > + break; > > + } > > + > > I think this is a good place to also return errors early if, say, user > requested LINK mode, but that mode is not supported by kernel. Instead > of returning some -ENOTSUP generic error, we can error out early? > Similar for PERF mode, if only legacy is supported, and so on. Similar > for attach_uprobe, of course. > Yes, sounds great. > > if (!legacy) { > > pfd = perf_event_open_probe(true /* uprobe */, retprobe, binary_path, > > func_offset, pid, ref_ctr_off); > > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h > > index 8777ff21ea1d..7fb474e036a3 100644 > > --- a/tools/lib/bpf/libbpf.h > > +++ b/tools/lib/bpf/libbpf.h > > @@ -451,8 +451,10 @@ struct bpf_perf_event_opts { > > size_t sz; > > /* custom user-provided value fetchable through bpf_get_attach_cookie() */ > > __u64 bpf_cookie; > > + /* don't use bpf_link when attach eBPF pprogram */ > > typo: pprogram Ops, get it! > > > + bool no_link; > > I've been struggling with this "no_link" name a bit. It is quite > confusing considering that from libbpf's API side we do return `struct > bpf_link`. So I'm thinking that maybe "force_ioctl_attach" would be a > better way to describe it (and will be scary enough for people not > knowing what this is about to not set it to true?) > Yeah, "force_ioctl_attach" sounds more appropriate. > Also, we'll need size_t: 0 at the end to avoid uninitialized padding > issues (like we have in kprobe_opts and uprobe_opts) > > > }; > > -#define bpf_perf_event_opts__last_field bpf_cookie > > +#define bpf_perf_event_opts__last_field no_link > > > > LIBBPF_API struct bpf_link * > > bpf_program__attach_perf_event(const struct bpf_program *prog, int pfd); > > @@ -461,6 +463,13 @@ LIBBPF_API struct bpf_link * > > bpf_program__attach_perf_event_opts(const struct bpf_program *prog, int pfd, > > const struct bpf_perf_event_opts *opts); > > > > +enum probe_mode { > > shall we call it probe_attach_mode? > > also let's elaborate a bit more in doc comment that specifying mode > will force libbpf to use that, but if kernel doesn't support it -- > then we'll error out. > OK. > > + PROBE_MODE_DEFAULT = 0, /* latest supported by kernel */ > > + PROBE_MODE_LEGACY, > > + PROBE_MODE_PERF, > > + PROBE_MODE_LINK, > > +}; > > + > > struct bpf_kprobe_opts { > > /* size of this struct, for forward/backward compatiblity */ > > size_t sz; > > @@ -470,9 +479,11 @@ struct bpf_kprobe_opts { > > size_t offset; > > /* kprobe is return probe */ > > bool retprobe; > > + /* kprobe attach mode */ > > + enum probe_mode mode; > > nit: mode -> attach_mode? > > > size_t :0; > > }; > > -#define bpf_kprobe_opts__last_field retprobe > > +#define bpf_kprobe_opts__last_field mode > > > > LIBBPF_API struct bpf_link * > > bpf_program__attach_kprobe(const struct bpf_program *prog, bool retprobe, > > @@ -570,9 +581,11 @@ struct bpf_uprobe_opts { > > * binary_path. > > */ > > const char *func_name; > > + /* uprobe attach mode */ > > + enum probe_mode mode; > > size_t :0; > > }; > > -#define bpf_uprobe_opts__last_field func_name > > +#define bpf_uprobe_opts__last_field mode > > ditto > I'll fix these problems in the V2. Thanks! Menglong Dong > > > > /** > > * @brief **bpf_program__attach_uprobe()** attaches a BPF program > > -- > > 2.39.0 > >
