Re: [PATCHv2 bpf-next 13/24] libbpf: Add uprobe multi link detection

[Jiri Olsa <olsajiri@gmail.com>]

On Fri, Jun 23, 2023 at 01:40:20PM -0700, Andrii Nakryiko wrote:
> On Tue, Jun 20, 2023 at 1:38 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          | 29 +++++++++++++++++++++++++++++
> >  tools/lib/bpf/libbpf_internal.h |  2 ++
> >  2 files changed, 31 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index e42080258ec7..3d570898459e 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -4815,6 +4815,32 @@ static int probe_perf_link(void)
> >         return link_fd < 0 && err == -EBADF;
> >  }
> >
> > +static int probe_uprobe_multi_link(void)
> > +{
> > +       struct bpf_insn insns[] = {
> > +               BPF_MOV64_IMM(BPF_REG_0, 0),
> > +               BPF_EXIT_INSN(),
> > +       };
> > +       int prog_fd, link_fd, err;
> > +
> > +       prog_fd = bpf_prog_load(BPF_PROG_TYPE_KPROBE, NULL, "GPL",
> > +                               insns, ARRAY_SIZE(insns), NULL);
> 
> I thought we needed to specify expected_attach_type (BPF_TRACE_UPROBE_MULTI)?

hm it should.. I guess it worked because of the KPROBE/UPROBE
typo you found in the patch earlier, will check

> 
> > +       if (prog_fd < 0)
> > +               return -errno;
> > +
> > +       /* No need to specify attach function. If the link is not supported
> > +        * we will get -EOPNOTSUPP error before any other check is performed.
> 
> what will actually return this -EOPNOTSUPP? I couldn't find this in
> the code quickly, can you please point me where?

	#else /* !CONFIG_UPROBES */
	int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
	{
		return -EOPNOTSUPP;
	}

> 
> > +        */
> > +       link_fd = bpf_link_create(prog_fd, -1, BPF_TRACE_UPROBE_MULTI, NULL);
> > +       err = -errno; /* close() can clobber errno */
> > +
> > +       if (link_fd >= 0)
> > +               close(link_fd);
> > +       close(prog_fd);
> > +
> > +       return link_fd < 0 && err != -EOPNOTSUPP;
> > +}
> > +
> >  static int probe_kern_bpf_cookie(void)
> >  {
> >         struct bpf_insn insns[] = {
> > @@ -4911,6 +4937,9 @@ static struct kern_feature_desc {
> >         [FEAT_SYSCALL_WRAPPER] = {
> >                 "Kernel using syscall wrapper", probe_kern_syscall_wrapper,
> >         },
> > +       [FEAT_UPROBE_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 22b0834e7fe1..a257eb81af25 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_LINK,
> 
> UPROBE_MULTI_LINK, we might have non-multi link in the future as well

ok

thanks,
jirka