| Message ID | 20230630083344.984305-16-jolsa@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf: Add multi uprobe link | expand |
On Fri, Jun 30, 2023 at 1:36 AM Jiri Olsa <jolsa@kernel.org> wrote: > > Adding uprobe-multi link detection. It will be used later in > bpf_program__attach_usdt function to check and use uprobe_multi > link over standard uprobe links. > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > tools/lib/bpf/libbpf.c | 35 +++++++++++++++++++++++++++++++++ > tools/lib/bpf/libbpf_internal.h | 2 ++ > 2 files changed, 37 insertions(+) > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 06092b9752f1..4f61f9dc1748 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -4817,6 +4817,38 @@ static int probe_perf_link(void) > return link_fd < 0 && err == -EBADF; > } > > +static int probe_uprobe_multi_link(void) > +{ > + LIBBPF_OPTS(bpf_prog_load_opts, load_opts, > + .expected_attach_type = BPF_TRACE_UPROBE_MULTI, > + ); > + LIBBPF_OPTS(bpf_link_create_opts, link_opts); > + struct bpf_insn insns[] = { > + BPF_MOV64_IMM(BPF_REG_0, 0), > + BPF_EXIT_INSN(), > + }; > + unsigned long offset = 0; > + int prog_fd, link_fd; > + > + prog_fd = bpf_prog_load(BPF_PROG_TYPE_KPROBE, NULL, "GPL", > + insns, ARRAY_SIZE(insns), &load_opts); > + if (prog_fd < 0) > + return -errno; > + > + /* create single uprobe on offset 0 in current process */ > + link_opts.uprobe_multi.path = "/proc/self/exe"; > + link_opts.uprobe_multi.offsets = &offset; > + link_opts.uprobe_multi.cnt = 1; > + > + link_fd = bpf_link_create(prog_fd, -1, BPF_TRACE_UPROBE_MULTI, &link_opts); > + so I'd like us to avoid successfully attaching anything. This might have unintended consequences (e.g., unintentionally breaking backing huge pages into normal pages, just because we happen to successfully attach briefly). So let's work on feature detection that fails to create a link, but does it in a way that we know that the feature itself is supported by the kernel Some ideas we could do: 1. Pass invalid file (e.g., just root, "/" as path), but modify kernel-side logic to return not -EINVAL, but -EBADF (and I think it would be good to do this anyway). Then expect -EBADF as a signal that the feature is supported. 2. Also, we can return -EPROTO instead of -EINVAL on invalid combination of paramers or something like that I'd start with -EBADF change. In general, we should write kernel-side code in such a way that allows simple and efficient feature-detection. We shouldn't repeat the nightmare of memcg-based mem accounting :( > + if (link_fd >= 0) > + close(link_fd); > + close(prog_fd); > + > + return link_fd >= 0; > +} > + > static int probe_kern_bpf_cookie(void) > { > struct bpf_insn insns[] = { > @@ -4913,6 +4945,9 @@ static struct kern_feature_desc { > [FEAT_SYSCALL_WRAPPER] = { > "Kernel using syscall wrapper", probe_kern_syscall_wrapper, > }, > + [FEAT_UPROBE_MULTI_LINK] = { > + "BPF uprobe multi link support", probe_uprobe_multi_link, nit: BPF multi-uprobe link support > + }, > }; > > bool kernel_supports(const struct bpf_object *obj, enum kern_feature_id feat_id) > diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h > index 7d75b92e531a..9c04b3fe1207 100644 > --- a/tools/lib/bpf/libbpf_internal.h > +++ b/tools/lib/bpf/libbpf_internal.h > @@ -354,6 +354,8 @@ enum kern_feature_id { > FEAT_BTF_ENUM64, > /* Kernel uses syscall wrapper (CONFIG_ARCH_HAS_SYSCALL_WRAPPER) */ > FEAT_SYSCALL_WRAPPER, > + /* BPF uprobe_multi link support */ same, multi-uprobe link support > + FEAT_UPROBE_MULTI_LINK, > __FEAT_CNT, > }; > > -- > 2.41.0 >
On Thu, Jul 06, 2023 at 09:20:42PM -0700, Andrii Nakryiko wrote: > On Fri, Jun 30, 2023 at 1:36 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > Adding uprobe-multi link detection. It will be used later in > > bpf_program__attach_usdt function to check and use uprobe_multi > > link over standard uprobe links. > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > --- > > tools/lib/bpf/libbpf.c | 35 +++++++++++++++++++++++++++++++++ > > tools/lib/bpf/libbpf_internal.h | 2 ++ > > 2 files changed, 37 insertions(+) > > > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > > index 06092b9752f1..4f61f9dc1748 100644 > > --- a/tools/lib/bpf/libbpf.c > > +++ b/tools/lib/bpf/libbpf.c > > @@ -4817,6 +4817,38 @@ static int probe_perf_link(void) > > return link_fd < 0 && err == -EBADF; > > } > > > > +static int probe_uprobe_multi_link(void) > > +{ > > + LIBBPF_OPTS(bpf_prog_load_opts, load_opts, > > + .expected_attach_type = BPF_TRACE_UPROBE_MULTI, > > + ); > > + LIBBPF_OPTS(bpf_link_create_opts, link_opts); > > + struct bpf_insn insns[] = { > > + BPF_MOV64_IMM(BPF_REG_0, 0), > > + BPF_EXIT_INSN(), > > + }; > > + unsigned long offset = 0; > > + int prog_fd, link_fd; > > + > > + prog_fd = bpf_prog_load(BPF_PROG_TYPE_KPROBE, NULL, "GPL", > > + insns, ARRAY_SIZE(insns), &load_opts); > > + if (prog_fd < 0) > > + return -errno; > > + > > + /* create single uprobe on offset 0 in current process */ > > + link_opts.uprobe_multi.path = "/proc/self/exe"; > > + link_opts.uprobe_multi.offsets = &offset; > > + link_opts.uprobe_multi.cnt = 1; > > + > > + link_fd = bpf_link_create(prog_fd, -1, BPF_TRACE_UPROBE_MULTI, &link_opts); > > + > > so I'd like us to avoid successfully attaching anything. This might > have unintended consequences (e.g., unintentionally breaking backing > huge pages into normal pages, just because we happen to successfully > attach briefly). So let's work on feature detection that fails to > create a link, but does it in a way that we know that the feature > itself is supported by the kernel > > Some ideas we could do: > > 1. Pass invalid file (e.g., just root, "/" as path), but modify > kernel-side logic to return not -EINVAL, but -EBADF (and I think it > would be good to do this anyway). Then expect -EBADF as a signal that > the feature is supported. ah ok, so -EBADF from inside uprobe_multi link setup code would mean it's supported, anything else means it's not.. should work also for old kernels, I don't think we have -EBADF in related paths, will check > > 2. Also, we can return -EPROTO instead of -EINVAL on invalid > combination of paramers or something like that > > I'd start with -EBADF change. > > In general, we should write kernel-side code in such a way that allows > simple and efficient feature-detection. We shouldn't repeat the > nightmare of memcg-based mem accounting :( > > > + if (link_fd >= 0) > > + close(link_fd); > > + close(prog_fd); > > + > > + return link_fd >= 0; > > +} > > + > > static int probe_kern_bpf_cookie(void) > > { > > struct bpf_insn insns[] = { > > @@ -4913,6 +4945,9 @@ static struct kern_feature_desc { > > [FEAT_SYSCALL_WRAPPER] = { > > "Kernel using syscall wrapper", probe_kern_syscall_wrapper, > > }, > > + [FEAT_UPROBE_MULTI_LINK] = { > > + "BPF uprobe multi link support", probe_uprobe_multi_link, > > nit: BPF multi-uprobe link support > > > + }, > > }; > > > > bool kernel_supports(const struct bpf_object *obj, enum kern_feature_id feat_id) > > diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h > > index 7d75b92e531a..9c04b3fe1207 100644 > > --- a/tools/lib/bpf/libbpf_internal.h > > +++ b/tools/lib/bpf/libbpf_internal.h > > @@ -354,6 +354,8 @@ enum kern_feature_id { > > FEAT_BTF_ENUM64, > > /* Kernel uses syscall wrapper (CONFIG_ARCH_HAS_SYSCALL_WRAPPER) */ > > FEAT_SYSCALL_WRAPPER, > > + /* BPF uprobe_multi link support */ > > same, multi-uprobe link support ok, thanks jirka > > > > > + FEAT_UPROBE_MULTI_LINK, > > __FEAT_CNT, > > }; > > > > -- > > 2.41.0 > >
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 06092b9752f1..4f61f9dc1748 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -4817,6 +4817,38 @@ static int probe_perf_link(void) return link_fd < 0 && err == -EBADF; } +static int probe_uprobe_multi_link(void) +{ + LIBBPF_OPTS(bpf_prog_load_opts, load_opts, + .expected_attach_type = BPF_TRACE_UPROBE_MULTI, + ); + LIBBPF_OPTS(bpf_link_create_opts, link_opts); + struct bpf_insn insns[] = { + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_EXIT_INSN(), + }; + unsigned long offset = 0; + int prog_fd, link_fd; + + prog_fd = bpf_prog_load(BPF_PROG_TYPE_KPROBE, NULL, "GPL", + insns, ARRAY_SIZE(insns), &load_opts); + if (prog_fd < 0) + return -errno; + + /* create single uprobe on offset 0 in current process */ + link_opts.uprobe_multi.path = "/proc/self/exe"; + link_opts.uprobe_multi.offsets = &offset; + link_opts.uprobe_multi.cnt = 1; + + link_fd = bpf_link_create(prog_fd, -1, BPF_TRACE_UPROBE_MULTI, &link_opts); + + if (link_fd >= 0) + close(link_fd); + close(prog_fd); + + return link_fd >= 0; +} + static int probe_kern_bpf_cookie(void) { struct bpf_insn insns[] = { @@ -4913,6 +4945,9 @@ static struct kern_feature_desc { [FEAT_SYSCALL_WRAPPER] = { "Kernel using syscall wrapper", probe_kern_syscall_wrapper, }, + [FEAT_UPROBE_MULTI_LINK] = { + "BPF uprobe multi link support", probe_uprobe_multi_link, + }, }; bool kernel_supports(const struct bpf_object *obj, enum kern_feature_id feat_id) diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h index 7d75b92e531a..9c04b3fe1207 100644 --- a/tools/lib/bpf/libbpf_internal.h +++ b/tools/lib/bpf/libbpf_internal.h @@ -354,6 +354,8 @@ enum kern_feature_id { FEAT_BTF_ENUM64, /* Kernel uses syscall wrapper (CONFIG_ARCH_HAS_SYSCALL_WRAPPER) */ FEAT_SYSCALL_WRAPPER, + /* BPF uprobe_multi link support */ + FEAT_UPROBE_MULTI_LINK, __FEAT_CNT, };
Adding uprobe-multi link detection. It will be used later in bpf_program__attach_usdt function to check and use uprobe_multi link over standard uprobe links. Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- tools/lib/bpf/libbpf.c | 35 +++++++++++++++++++++++++++++++++ tools/lib/bpf/libbpf_internal.h | 2 ++ 2 files changed, 37 insertions(+)