All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <olsajiri@gmail.com>
To: Yafang Shao <laoar.shao@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	bpf@vger.kernel.org, Martin KaFai Lau <kafai@fb.com>,
	Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@chromium.org>,
	Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>
Subject: Re: [PATCH bpf-next 3/6] bpf: Add link_info support for uprobe multi link
Date: Fri, 27 Oct 2023 15:59:33 +0200	[thread overview]
Message-ID: <ZTvCRYi6OMlZYfAz@krava> (raw)
In-Reply-To: <CALOAHbAZ6=A9j3VFCLoAC_WhgQKU7injMf06=cM2sU4Hi4Sx+Q@mail.gmail.com>

On Thu, Oct 26, 2023 at 07:57:27PM +0800, Yafang Shao wrote:
> On Thu, Oct 26, 2023 at 4:24 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding support to get uprobe_link details through bpf_link_info
> > interface.
> >
> > Adding new struct uprobe_multi to struct bpf_link_info to carry
> > the uprobe_multi link details.
> >
> > The uprobe_multi.count is passed from user space to denote size
> > of array fields (offsets/ref_ctr_offsets/cookies). The actual
> > array size is stored back to uprobe_multi.count (allowing user
> > to find out the actual array size) and array fields are populated
> > up to the user passed size.
> >
> > All the non-array fields (path/count/flags/pid) are always set.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  include/uapi/linux/bpf.h       | 10 +++++
> >  kernel/trace/bpf_trace.c       | 68 ++++++++++++++++++++++++++++++++++
> >  tools/include/uapi/linux/bpf.h | 10 +++++
> >  3 files changed, 88 insertions(+)
> >
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 0f6cdf52b1da..960cf2914d63 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -6556,6 +6556,16 @@ struct bpf_link_info {
> >                         __u32 flags;
> >                         __u64 missed;
> >                 } kprobe_multi;
> > +               struct {
> > +                       __aligned_u64 path;
> > +                       __aligned_u64 offsets;
> > +                       __aligned_u64 ref_ctr_offsets;
> > +                       __aligned_u64 cookies;
> 
> The bpf cookie for the perf_event link is exposed through
> 'pid_iter.bpf.c,' while the cookies for the tracing link and
> kprobe_multi link are not exposed at all. This inconsistency can be
> confusing. I believe it would be better to include all of them in the
> link_info. The reason is that 'pid_iter' depends on the task holding
> the links, which may not exist. However, I think we handle this in a
> separate patchset. What do you think?

right, I think we should add cookies for both kprobe_multi
and tracing link, I'll add that in new version

thanks,
jirka

> 
> > +                       __u32 path_max; /* in/out: uprobe_multi path size */
> > +                       __u32 count;    /* in/out: uprobe_multi offsets/ref_ctr_offsets/cookies count */
> > +                       __u32 flags;
> > +                       __u32 pid;
> > +               } uprobe_multi;
> >                 struct {
> >                         __u32 type; /* enum bpf_perf_event_type */
> >                         __u32 :32;
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index 843b3846d3f8..9f8ad19a1a93 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -3042,6 +3042,7 @@ struct bpf_uprobe_multi_link {
> >         u32 cnt;
> >         struct bpf_uprobe *uprobes;
> >         struct task_struct *task;
> > +       u32 flags;
> >  };
> >
> >  struct bpf_uprobe_multi_run_ctx {
> > @@ -3081,9 +3082,75 @@ static void bpf_uprobe_multi_link_dealloc(struct bpf_link *link)
> >         kfree(umulti_link);
> >  }
> >
> > +static int bpf_uprobe_multi_link_fill_link_info(const struct bpf_link *link,
> > +                                               struct bpf_link_info *info)
> > +{
> > +       u64 __user *uref_ctr_offsets = u64_to_user_ptr(info->uprobe_multi.ref_ctr_offsets);
> > +       u64 __user *ucookies = u64_to_user_ptr(info->uprobe_multi.cookies);
> > +       u64 __user *uoffsets = u64_to_user_ptr(info->uprobe_multi.offsets);
> > +       u64 __user *upath = u64_to_user_ptr(info->uprobe_multi.path);
> > +       u32 upath_max = info->uprobe_multi.path_max;
> > +       struct bpf_uprobe_multi_link *umulti_link;
> > +       u32 ucount = info->uprobe_multi.count;
> > +       int err = 0, i;
> > +       char *p, *buf;
> > +       long left;
> > +
> > +       if (!upath ^ !upath_max)
> > +               return -EINVAL;
> > +
> > +       if (!uoffsets ^ !ucount)
> > +               return -EINVAL;
> > +
> > +       umulti_link = container_of(link, struct bpf_uprobe_multi_link, link);
> > +       info->uprobe_multi.count = umulti_link->cnt;
> > +       info->uprobe_multi.flags = umulti_link->flags;
> > +       info->uprobe_multi.pid = umulti_link->task ?
> > +                                task_pid_nr(umulti_link->task) : (u32) -1;
> > +
> > +       if (upath) {
> > +               if (upath_max > PATH_MAX)
> > +                       return -E2BIG;
> > +               buf = kmalloc(upath_max, GFP_KERNEL);
> > +               if (!buf)
> > +                       return -ENOMEM;
> > +               p = d_path(&umulti_link->path, buf, upath_max);
> > +               if (IS_ERR(p)) {
> > +                       kfree(buf);
> > +                       return -ENOSPC;
> > +               }
> > +               left = copy_to_user(upath, p, buf + upath_max - p);
> > +               kfree(buf);
> > +               if (left)
> > +                       return -EFAULT;
> > +       }
> > +
> > +       if (!uoffsets)
> > +               return 0;
> > +
> > +       if (ucount < umulti_link->cnt)
> > +               err = -ENOSPC;
> > +       else
> > +               ucount = umulti_link->cnt;
> > +
> > +       for (i = 0; i < ucount; i++) {
> > +               if (put_user(umulti_link->uprobes[i].offset, uoffsets + i))
> > +                       return -EFAULT;
> > +               if (uref_ctr_offsets &&
> > +                   put_user(umulti_link->uprobes[i].ref_ctr_offset, uref_ctr_offsets + i))
> > +                       return -EFAULT;
> > +               if (ucookies &&
> > +                   put_user(umulti_link->uprobes[i].cookie, ucookies + i))
> > +                       return -EFAULT;
> > +       }
> > +
> > +       return err;
> > +}
> > +
> >  static const struct bpf_link_ops bpf_uprobe_multi_link_lops = {
> >         .release = bpf_uprobe_multi_link_release,
> >         .dealloc = bpf_uprobe_multi_link_dealloc,
> > +       .fill_link_info = bpf_uprobe_multi_link_fill_link_info,
> >  };
> >
> >  static int uprobe_prog_run(struct bpf_uprobe *uprobe,
> > @@ -3272,6 +3339,7 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
> >         link->uprobes = uprobes;
> >         link->path = path;
> >         link->task = task;
> > +       link->flags = flags;
> >
> >         bpf_link_init(&link->link, BPF_LINK_TYPE_UPROBE_MULTI,
> >                       &bpf_uprobe_multi_link_lops, prog);
> > diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> > index 0f6cdf52b1da..960cf2914d63 100644
> > --- a/tools/include/uapi/linux/bpf.h
> > +++ b/tools/include/uapi/linux/bpf.h
> > @@ -6556,6 +6556,16 @@ struct bpf_link_info {
> >                         __u32 flags;
> >                         __u64 missed;
> >                 } kprobe_multi;
> > +               struct {
> > +                       __aligned_u64 path;
> > +                       __aligned_u64 offsets;
> > +                       __aligned_u64 ref_ctr_offsets;
> > +                       __aligned_u64 cookies;
> > +                       __u32 path_max; /* in/out: uprobe_multi path size */
> > +                       __u32 count;    /* in/out: uprobe_multi offsets/ref_ctr_offsets/cookies count */
> > +                       __u32 flags;
> > +                       __u32 pid;
> > +               } uprobe_multi;
> >                 struct {
> >                         __u32 type; /* enum bpf_perf_event_type */
> >                         __u32 :32;
> > --
> > 2.41.0
> >
> 
> 
> -- 
> Regards
> Yafang

  reply	other threads:[~2023-10-27 13:59 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-25 20:24 [PATCH bpf-next 0/6] bpf: Add link_info support for uprobe multi link Jiri Olsa
2023-10-25 20:24 ` [PATCH bpf-next 1/6] libbpf: Add st_type argument to elf_resolve_syms_offsets function Jiri Olsa
2023-10-26 16:29   ` Song Liu
2023-10-25 20:24 ` [PATCH bpf-next 2/6] bpf: Store ref_ctr_offsets values in bpf_uprobe array Jiri Olsa
2023-10-26 16:31   ` Song Liu
2023-10-27 13:56     ` Jiri Olsa
2023-10-27 14:23       ` Song Liu
2023-11-01 22:21         ` Andrii Nakryiko
2023-10-25 20:24 ` [PATCH bpf-next 3/6] bpf: Add link_info support for uprobe multi link Jiri Olsa
2023-10-26 11:57   ` Yafang Shao
2023-10-27 13:59     ` Jiri Olsa [this message]
2023-11-09  8:56       ` Jiri Olsa
2023-10-26 17:55   ` Song Liu
2023-10-27 14:29     ` Jiri Olsa
2023-11-01 22:21       ` Andrii Nakryiko
2023-11-02 14:58         ` Jiri Olsa
2023-11-02 16:21           ` Andrii Nakryiko
2023-10-30 10:18   ` Quentin Monnet
2023-10-30 21:17     ` Jiri Olsa
2023-11-01 22:21   ` Andrii Nakryiko
2023-11-02 14:43     ` Jiri Olsa
2023-11-02 16:19       ` Andrii Nakryiko
2023-10-25 20:24 ` [PATCH bpf-next 4/6] selftests/bpf: Use bpf_link__destroy in fill_link_info tests Jiri Olsa
2023-10-26 11:41   ` Yafang Shao
2023-10-26 18:00     ` Song Liu
2023-11-01 22:24   ` Andrii Nakryiko
2023-11-02 14:12     ` Jiri Olsa
2023-10-25 20:24 ` [PATCH bpf-next 5/6] selftests/bpf: Add link_info test for uprobe_multi link Jiri Olsa
2023-10-26 18:13   ` Song Liu
2023-11-01 22:27   ` Andrii Nakryiko
2023-10-25 20:24 ` [PATCH bpf-next 6/6] bpftool: Add support to display uprobe_multi links Jiri Olsa
2023-10-26 18:27   ` Song Liu
2023-10-30 10:17   ` Quentin Monnet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZTvCRYi6OMlZYfAz@krava \
    --to=olsajiri@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=kafai@fb.com \
    --cc=kpsingh@chromium.org \
    --cc=laoar.shao@gmail.com \
    --cc=sdf@google.com \
    --cc=songliubraving@fb.com \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.