On Sat, Jun 24, 2023 at 6:18 PM Jiri Olsa <olsajiri@xxxxxxxxx> wrote:
>
> 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@xxxxxxxxxx> 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@xxxxxxxxxx>
> > > ---
> > > 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;
> }
that's a new code in new kernel that doesn't have CONFIG_UPROBES. What
about old kernels? They will return -EINVAL, no? Which we will assume
means "yep, multi-uprobe BPF link is supported", which would be wrong.
Or am I missing something?
>
> >
> > > + */
> > > + 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
