From: Jiri Olsa <olsajiri@gmail.com>
To: Andrii Nakryiko <andrii.nakryiko@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: [PATCHv3 bpf-next 16/26] libbpf: Add uprobe multi link support to bpf_program__attach_usdt
Date: Tue, 11 Jul 2023 11:04:06 +0200 [thread overview]
Message-ID: <ZK0bBlRNtaKJnUTY@krava> (raw)
In-Reply-To: <CAEf4Bzb=NjO5eqYcx8oa3nOoPXt83MA54Om=aUqn4aTJFCSHmg@mail.gmail.com>
On Thu, Jul 06, 2023 at 09:29:35PM -0700, Andrii Nakryiko wrote:
> On Fri, Jun 30, 2023 at 1:37 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding support for usdt_manager_attach_usdt to use uprobe_multi
> > link to attach to usdt probes.
> >
> > The uprobe_multi support is detected before the usdt program is
> > loaded and its expected_attach_type is set accordingly.
> >
> > If uprobe_multi support is detected the usdt_manager_attach_usdt
> > gathers uprobes info and calls bpf_program__attach_uprobe to
> > create all needed uprobes.
> >
> > If uprobe_multi support is not detected the old behaviour stays.
> >
> > Also adding usdt.s program section for sleepable usdt probes.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > tools/lib/bpf/libbpf.c | 13 +++++--
> > tools/lib/bpf/usdt.c | 78 ++++++++++++++++++++++++++++++++++--------
> > 2 files changed, 75 insertions(+), 16 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 4f61f9dc1748..e234c2e860f9 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -365,6 +365,8 @@ enum sec_def_flags {
> > SEC_SLEEPABLE = 8,
> > /* BPF program support non-linear XDP buffer */
> > SEC_XDP_FRAGS = 16,
> > + /* Setup proper attach type for usdt probes. */
> > + SEC_USDT = 32,
> > };
> >
> > struct bpf_sec_def {
> > @@ -6807,6 +6809,10 @@ static int libbpf_prepare_prog_load(struct bpf_program *prog,
> > if (prog->type == BPF_PROG_TYPE_XDP && (def & SEC_XDP_FRAGS))
> > opts->prog_flags |= BPF_F_XDP_HAS_FRAGS;
> >
> > + /* special check for usdt to use uprobe_multi link */
> > + if ((def & SEC_USDT) && kernel_supports(NULL, FEAT_UPROBE_MULTI_LINK))
>
> please pass prog->obj to kernel_supports(), it will be especially
> important later with BPF token stuff
did not realize I have it in prog->obj ;-) ok
>
> > + prog->expected_attach_type = BPF_TRACE_UPROBE_MULTI;
> > +
> > if ((def & SEC_ATTACH_BTF) && !prog->attach_btf_id) {
> > int btf_obj_fd = 0, btf_type_id = 0, err;
> > const char *attach_name;
> > @@ -6875,7 +6881,6 @@ static int bpf_object_load_prog(struct bpf_object *obj, struct bpf_program *prog
> > if (!insns || !insns_cnt)
> > return -EINVAL;
> >
> > - load_attr.expected_attach_type = prog->expected_attach_type;
> > if (kernel_supports(obj, FEAT_PROG_NAME))
> > prog_name = prog->name;
> > load_attr.attach_prog_fd = prog->attach_prog_fd;
> > @@ -6911,6 +6916,9 @@ static int bpf_object_load_prog(struct bpf_object *obj, struct bpf_program *prog
> > insns_cnt = prog->insns_cnt;
> > }
> >
> > + /* allow prog_prepare_load_fn to change expected_attach_type */
> > + load_attr.expected_attach_type = prog->expected_attach_type;
> > +
> > if (obj->gen_loader) {
> > bpf_gen__prog_load(obj->gen_loader, prog->type, prog->name,
> > license, insns, insns_cnt, &load_attr,
> > @@ -8711,7 +8719,8 @@ static const struct bpf_sec_def section_defs[] = {
> > SEC_DEF("uretprobe.multi.s+", KPROBE, BPF_TRACE_UPROBE_MULTI, SEC_SLEEPABLE, attach_uprobe_multi),
> > SEC_DEF("ksyscall+", KPROBE, 0, SEC_NONE, attach_ksyscall),
> > SEC_DEF("kretsyscall+", KPROBE, 0, SEC_NONE, attach_ksyscall),
> > - SEC_DEF("usdt+", KPROBE, 0, SEC_NONE, attach_usdt),
> > + SEC_DEF("usdt+", KPROBE, 0, SEC_USDT, attach_usdt),
> > + SEC_DEF("usdt.s+", KPROBE, 0, SEC_USDT|SEC_SLEEPABLE, attach_usdt),
>
> spaces around |
ook
>
> > SEC_DEF("tc", SCHED_CLS, 0, SEC_NONE),
> > SEC_DEF("classifier", SCHED_CLS, 0, SEC_NONE),
> > SEC_DEF("action", SCHED_ACT, 0, SEC_NONE),
> > diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c
> > index 9fa883ebc0bd..6ff66a8eaf85 100644
> > --- a/tools/lib/bpf/usdt.c
> > +++ b/tools/lib/bpf/usdt.c
> > @@ -809,6 +809,8 @@ struct bpf_link_usdt {
> > long abs_ip;
> > struct bpf_link *link;
> > } *uprobes;
> > +
> > + struct bpf_link *multi_link;
> > };
> >
> > static int bpf_link_usdt_detach(struct bpf_link *link)
> > @@ -817,6 +819,9 @@ static int bpf_link_usdt_detach(struct bpf_link *link)
> > struct usdt_manager *man = usdt_link->usdt_man;
> > int i;
> >
> > + /* When having multi_link, uprobe_cnt is 0 */
>
> misplaced comment, move down to for() loop?
right, will move
>
> > + bpf_link__destroy(usdt_link->multi_link);
> > +
> > for (i = 0; i < usdt_link->uprobe_cnt; i++) {
> > /* detach underlying uprobe link */
> > bpf_link__destroy(usdt_link->uprobes[i].link);
> > @@ -944,11 +949,13 @@ struct bpf_link *usdt_manager_attach_usdt(struct usdt_manager *man, const struct
> > const char *usdt_provider, const char *usdt_name,
> > __u64 usdt_cookie)
> > {
> > + unsigned long *offsets = NULL, *ref_ctr_offsets = NULL;
> > int i, err, spec_map_fd, ip_map_fd;
> > LIBBPF_OPTS(bpf_uprobe_opts, opts);
> > struct hashmap *specs_hash = NULL;
> > struct bpf_link_usdt *link = NULL;
> > struct usdt_target *targets = NULL;
> > + __u64 *cookies = NULL;
> > struct elf_fd elf_fd;
> > size_t target_cnt;
> >
> > @@ -995,10 +1002,21 @@ struct bpf_link *usdt_manager_attach_usdt(struct usdt_manager *man, const struct
> > link->link.detach = &bpf_link_usdt_detach;
> > link->link.dealloc = &bpf_link_usdt_dealloc;
> >
> > - link->uprobes = calloc(target_cnt, sizeof(*link->uprobes));
> > - if (!link->uprobes) {
> > - err = -ENOMEM;
> > - goto err_out;
> > + if (kernel_supports(NULL, FEAT_UPROBE_MULTI_LINK)) {
>
> see how we feature-detect has_sema_refcnt and has_bpf_cookie, let's do
> the same with UPROBE_MULTI_LINK, detect once, remember, consistently
> use it (it also matters later for BPF token)
ok
>
> > + offsets = calloc(target_cnt, sizeof(*offsets));
> > + cookies = calloc(target_cnt, sizeof(*cookies));
> > + ref_ctr_offsets = calloc(target_cnt, sizeof(*ref_ctr_offsets));
> > +
> > + if (!offsets || !ref_ctr_offsets || !cookies) {
> > + err = -ENOMEM;
> > + goto err_out;
> > + }
> > + } else {
> > + link->uprobes = calloc(target_cnt, sizeof(*link->uprobes));
> > + if (!link->uprobes) {
> > + err = -ENOMEM;
> > + goto err_out;
> > + }
> > }
> >
> > for (i = 0; i < target_cnt; i++) {
> > @@ -1039,20 +1057,48 @@ struct bpf_link *usdt_manager_attach_usdt(struct usdt_manager *man, const struct
> > goto err_out;
> > }
> >
> > - opts.ref_ctr_offset = target->sema_off;
> > - opts.bpf_cookie = man->has_bpf_cookie ? spec_id : 0;
> > - uprobe_link = bpf_program__attach_uprobe_opts(prog, pid, path,
> > - target->rel_ip, &opts);
> > - err = libbpf_get_error(uprobe_link);
> > + if (kernel_supports(NULL, FEAT_UPROBE_MULTI_LINK)) {
> > + offsets[i] = target->rel_ip;
> > + ref_ctr_offsets[i] = target->sema_off;
> > + cookies[i] = spec_id;
> > + } else {
> > + opts.ref_ctr_offset = target->sema_off;
> > + opts.bpf_cookie = man->has_bpf_cookie ? spec_id : 0;
> > + uprobe_link = bpf_program__attach_uprobe_opts(prog, pid, path,
> > + target->rel_ip, &opts);
> > + err = libbpf_get_error(uprobe_link);
> > + if (err) {
> > + pr_warn("usdt: failed to attach uprobe #%d for '%s:%s' in '%s': %d\n",
> > + i, usdt_provider, usdt_name, path, err);
> > + goto err_out;
> > + }
> > +
> > + link->uprobes[i].link = uprobe_link;
> > + link->uprobes[i].abs_ip = target->abs_ip;
> > + link->uprobe_cnt++;
> > + }
> > + }
> > +
> > + if (kernel_supports(NULL, FEAT_UPROBE_MULTI_LINK)) {
>
> same as above, we should feature-detect once per usdt_manager while we
> have associated bpf_object
ook
>
> > + LIBBPF_OPTS(bpf_uprobe_multi_opts, opts_multi,
> > + .cnt = target_cnt,
> > + .offsets = offsets,
> > + .ref_ctr_offsets = ref_ctr_offsets,
> > + .cookies = cookies,
> > + );
> > +
> > + link->multi_link = bpf_program__attach_uprobe_multi(prog, pid, path,
> > + NULL, &opts_multi);
> > + err = libbpf_get_error(link->multi_link);
>
> let's not use libbpf_get_error() in new code, there is no need, just
> `err = -errno` and `if (!link->multi_link)`
ah right, did not see its comment that says it's no longer recomended,
will change
thanks,
jirka
next prev parent reply other threads:[~2023-07-11 9:04 UTC|newest]
Thread overview: 73+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-30 8:33 [PATCHv3 bpf-next 00/26] bpf: Add multi uprobe link Jiri Olsa
2023-06-30 8:33 ` [PATCHv3 bpf-next 01/26] bpf: Add attach_type checks under bpf_prog_attach_check_attach_type Jiri Olsa
2023-07-06 22:34 ` Andrii Nakryiko
2023-06-30 8:33 ` [PATCHv3 bpf-next 02/26] bpf: Add multi uprobe link Jiri Olsa
2023-07-06 22:34 ` Andrii Nakryiko
2023-07-11 9:00 ` Jiri Olsa
2023-07-07 4:22 ` Andrii Nakryiko
2023-07-11 9:01 ` Jiri Olsa
2023-06-30 8:33 ` [PATCHv3 bpf-next 03/26] bpf: Add cookies support for uprobe_multi link Jiri Olsa
2023-07-01 3:40 ` Yafang Shao
2023-07-01 8:54 ` Jiri Olsa
2023-06-30 8:33 ` [PATCHv3 bpf-next 04/26] bpf: Add pid filter " Jiri Olsa
2023-06-30 8:33 ` [PATCHv3 bpf-next 05/26] bpf: Add bpf_get_func_ip helper support for uprobe link Jiri Olsa
2023-07-06 22:29 ` Andrii Nakryiko
2023-07-10 7:24 ` Jiri Olsa
2023-07-10 17:55 ` Andrii Nakryiko
2023-07-11 8:28 ` Jiri Olsa
2023-07-11 16:57 ` Andrii Nakryiko
2023-06-30 8:33 ` [PATCHv3 bpf-next 06/26] libbpf: Add uprobe_multi attach type and link names Jiri Olsa
2023-06-30 8:33 ` [PATCHv3 bpf-next 07/26] libbpf: Move elf_find_func_offset* functions to elf object Jiri Olsa
2023-07-06 23:02 ` Andrii Nakryiko
2023-07-11 9:05 ` Jiri Olsa
2023-07-11 17:01 ` Andrii Nakryiko
2023-07-06 23:03 ` Andrii Nakryiko
2023-07-11 9:05 ` Jiri Olsa
2023-06-30 8:33 ` [PATCHv3 bpf-next 08/26] libbpf: Add elf_open/elf_close functions Jiri Olsa
2023-07-06 23:09 ` Andrii Nakryiko
2023-07-11 9:01 ` Jiri Olsa
2023-06-30 8:33 ` [PATCHv3 bpf-next 09/26] libbpf: Add elf symbol iterator Jiri Olsa
2023-07-06 23:24 ` Andrii Nakryiko
2023-07-11 9:03 ` Jiri Olsa
2023-07-11 16:59 ` Andrii Nakryiko
2023-06-30 8:33 ` [PATCHv3 bpf-next 10/26] libbpf: Add elf_resolve_syms_offsets function Jiri Olsa
2023-07-07 3:48 ` Andrii Nakryiko
2023-07-11 9:04 ` Jiri Olsa
2023-06-30 8:33 ` [PATCHv3 bpf-next 11/26] libbpf: Add elf_resolve_pattern_offsets function Jiri Olsa
2023-07-07 3:52 ` Andrii Nakryiko
2023-07-11 9:04 ` Jiri Olsa
2023-06-30 8:33 ` [PATCHv3 bpf-next 12/26] libbpf: Add bpf_link_create support for multi uprobes Jiri Olsa
2023-06-30 8:33 ` [PATCHv3 bpf-next 13/26] libbpf: Add bpf_program__attach_uprobe_multi function Jiri Olsa
2023-07-07 4:05 ` Andrii Nakryiko
2023-07-11 9:05 ` Jiri Olsa
2023-07-11 17:02 ` Andrii Nakryiko
2023-06-30 8:33 ` [PATCHv3 bpf-next 14/26] libbpf: Add support for u[ret]probe.multi[.s] program sections Jiri Olsa
2023-07-07 4:07 ` Andrii Nakryiko
2023-06-30 8:33 ` [PATCHv3 bpf-next 15/26] libbpf: Add uprobe multi link detection Jiri Olsa
2023-07-07 4:20 ` Andrii Nakryiko
2023-07-11 9:03 ` Jiri Olsa
2023-06-30 8:33 ` [PATCHv3 bpf-next 16/26] libbpf: Add uprobe multi link support to bpf_program__attach_usdt Jiri Olsa
2023-07-07 4:29 ` Andrii Nakryiko
2023-07-11 9:04 ` Jiri Olsa [this message]
2023-06-30 8:33 ` [PATCHv3 bpf-next 17/26] selftests/bpf: Add uprobe_multi skel test Jiri Olsa
2023-06-30 8:33 ` [PATCHv3 bpf-next 18/26] selftests/bpf: Add uprobe_multi api test Jiri Olsa
2023-07-07 4:32 ` Andrii Nakryiko
2023-07-11 9:06 ` Jiri Olsa
2023-06-30 8:33 ` [PATCHv3 bpf-next 19/26] selftests/bpf: Add uprobe_multi link test Jiri Olsa
2023-07-07 4:33 ` Andrii Nakryiko
2023-07-11 9:06 ` Jiri Olsa
2023-06-30 8:33 ` [PATCHv3 bpf-next 20/26] selftests/bpf: Add uprobe_multi test program Jiri Olsa
2023-07-07 4:35 ` Andrii Nakryiko
2023-06-30 8:33 ` [PATCHv3 bpf-next 21/26] selftests/bpf: Add uprobe_multi bench test Jiri Olsa
2023-07-07 4:38 ` Andrii Nakryiko
2023-07-11 9:07 ` Jiri Olsa
2023-06-30 8:33 ` [PATCHv3 bpf-next 22/26] selftests/bpf: Add usdt_multi test program Jiri Olsa
2023-07-07 4:39 ` Andrii Nakryiko
2023-06-30 8:33 ` [PATCHv3 bpf-next 23/26] selftests/bpf: Add usdt_multi bench test Jiri Olsa
2023-07-07 4:42 ` Andrii Nakryiko
2023-07-11 9:07 ` Jiri Olsa
2023-06-30 8:33 ` [PATCHv3 bpf-next 24/26] selftests/bpf: Add uprobe_multi cookie test Jiri Olsa
2023-06-30 8:33 ` [PATCHv3 bpf-next 25/26] selftests/bpf: Add uprobe_multi pid filter tests Jiri Olsa
2023-06-30 8:33 ` [PATCHv3 bpf-next 26/26] selftests/bpf: Add extra link to uprobe_multi tests Jiri Olsa
2023-07-05 12:45 ` [PATCHv3 bpf-next 00/26] bpf: Add multi uprobe link Daniel Borkmann
2023-07-05 19:10 ` Jiri Olsa
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=ZK0bBlRNtaKJnUTY@krava \
--to=olsajiri@gmail.com \
--cc=andrii.nakryiko@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=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.