From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Song Liu <song@kernel.org>
Cc: Andrii Nakryiko <andrii@kernel.org>, bpf <bpf@vger.kernel.org>,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Kernel Team <kernel-team@fb.com>
Subject: Re: [PATCH bpf-next 1/3] libbpf: allow "incomplete" basic tracing SEC() definitions
Date: Fri, 8 Apr 2022 15:21:53 -0700 [thread overview]
Message-ID: <CAEf4BzaZai7QZnBEcAV72D=0irABs9BjbH++rmRQkqKiKpLYvA@mail.gmail.com> (raw)
In-Reply-To: <CAPhsuW5N9qBX0kkQSLK_Sy36cPa==SVSFi+38ZNqNez05zGD6w@mail.gmail.com>
On Fri, Apr 8, 2022 at 1:46 PM Song Liu <song@kernel.org> wrote:
>
> On Fri, Apr 8, 2022 at 1:34 PM Andrii Nakryiko <andrii@kernel.org> wrote:
> >
> > In a lot of cases the target of kprobe/kretprobe, tracepoint, raw
> > tracepoint, etc BPF program might not be known at the compilation time
> > and will be discovered at runtime. This was always a supported case by
> > libbpf, with APIs like bpf_program__attach_{kprobe,tracepoint,etc}()
> > accepting full target definition, regardless of what was defined in
> > SEC() definition in BPF source code.
> >
> > Unfortunately, up till now libbpf still enforced users to specify at
> > least something for the fake target, e.g., SEC("kprobe/whatever"), which
> > is cumbersome and somewhat misleading.
> >
> > This patch allows target-less SEC() definitions for basic tracing BPF
> > program types:
> > - kprobe/kretprobe;
> > - multi-kprobe/multi-kretprobe;
> > - tracepoints;
> > - raw tracepoints.
> >
> > Such target-less SEC() definitions are meant to specify declaratively
> > proper BPF program type only. Attachment of them will have to be handled
> > programmatically using correct APIs. As such, skeleton's auto-attachment
> > of such BPF programs is skipped and generic bpf_program__attach() will
> > fail, if attempted, due to the lack of enough target information.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
> > tools/lib/bpf/libbpf.c | 69 +++++++++++++++++++++++++++++++-----------
> > 1 file changed, 51 insertions(+), 18 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 9deb1fc67f19..81911a1e1f3e 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -8668,22 +8668,22 @@ static const struct bpf_sec_def section_defs[] = {
> > SEC_DEF("socket", SOCKET_FILTER, 0, SEC_NONE | SEC_SLOPPY_PFX),
> > SEC_DEF("sk_reuseport/migrate", SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT_OR_MIGRATE, SEC_ATTACHABLE | SEC_SLOPPY_PFX),
> > SEC_DEF("sk_reuseport", SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT, SEC_ATTACHABLE | SEC_SLOPPY_PFX),
> > - SEC_DEF("kprobe/", KPROBE, 0, SEC_NONE, attach_kprobe),
> > + SEC_DEF("kprobe+", KPROBE, 0, SEC_NONE, attach_kprobe),
> > SEC_DEF("uprobe+", KPROBE, 0, SEC_NONE, attach_uprobe),
> > - SEC_DEF("kretprobe/", KPROBE, 0, SEC_NONE, attach_kprobe),
> > + SEC_DEF("kretprobe+", KPROBE, 0, SEC_NONE, attach_kprobe),
> > SEC_DEF("uretprobe+", KPROBE, 0, SEC_NONE, attach_uprobe),
> > - SEC_DEF("kprobe.multi/", KPROBE, BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
> > - SEC_DEF("kretprobe.multi/", KPROBE, BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
> > + SEC_DEF("kprobe.multi+", KPROBE, BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
> > + SEC_DEF("kretprobe.multi+", KPROBE, BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
> > SEC_DEF("usdt+", KPROBE, 0, SEC_NONE, attach_usdt),
> > SEC_DEF("tc", SCHED_CLS, 0, SEC_NONE),
> > SEC_DEF("classifier", SCHED_CLS, 0, SEC_NONE | SEC_SLOPPY_PFX | SEC_DEPRECATED),
> > SEC_DEF("action", SCHED_ACT, 0, SEC_NONE | SEC_SLOPPY_PFX),
> > - SEC_DEF("tracepoint/", TRACEPOINT, 0, SEC_NONE, attach_tp),
> > - SEC_DEF("tp/", TRACEPOINT, 0, SEC_NONE, attach_tp),
> > - SEC_DEF("raw_tracepoint/", RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp),
> > - SEC_DEF("raw_tp/", RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp),
> > - SEC_DEF("raw_tracepoint.w/", RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
> > - SEC_DEF("raw_tp.w/", RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
> > + SEC_DEF("tracepoint+", TRACEPOINT, 0, SEC_NONE, attach_tp),
> > + SEC_DEF("tp+", TRACEPOINT, 0, SEC_NONE, attach_tp),
> > + SEC_DEF("raw_tracepoint+", RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp),
> > + SEC_DEF("raw_tp+", RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp),
> > + SEC_DEF("raw_tracepoint.w+", RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
> > + SEC_DEF("raw_tp.w+", RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
> > SEC_DEF("tp_btf/", TRACING, BPF_TRACE_RAW_TP, SEC_ATTACH_BTF, attach_trace),
> > SEC_DEF("fentry/", TRACING, BPF_TRACE_FENTRY, SEC_ATTACH_BTF, attach_trace),
> > SEC_DEF("fmod_ret/", TRACING, BPF_MODIFY_RETURN, SEC_ATTACH_BTF, attach_trace),
> > @@ -10411,6 +10411,12 @@ static int attach_kprobe(const struct bpf_program *prog, long cookie, struct bpf
> > char *func;
> > int n;
> >
> > + *link = NULL;
> > +
> > + /* no auto-attach for SEC("kprobe") and SEC("kretprobe") */
> > + if (strcmp(prog->sec_name, "kprobe") == 0 || strcmp(prog->sec_name, "kretprobe") == 0)
> > + return 0;
> > +
> > opts.retprobe = str_has_pfx(prog->sec_name, "kretprobe/");
> > if (opts.retprobe)
> > func_name = prog->sec_name + sizeof("kretprobe/") - 1;
> > @@ -10441,6 +10447,13 @@ static int attach_kprobe_multi(const struct bpf_program *prog, long cookie, stru
> > char *pattern;
> > int n;
> >
> > + *link = NULL;
> > +
> > + /* no auto-attach for SEC("kprobe.multi") and SEC("kretprobe.multi") */
> > + if (strcmp(prog->sec_name, "kprobe.multi") == 0 ||
> > + strcmp(prog->sec_name, "kretprobe.multi") == 0)
> > + return 0;
> > +
> > opts.retprobe = str_has_pfx(prog->sec_name, "kretprobe.multi/");
> > if (opts.retprobe)
> > spec = prog->sec_name + sizeof("kretprobe.multi/") - 1;
> > @@ -11145,6 +11158,12 @@ static int attach_tp(const struct bpf_program *prog, long cookie, struct bpf_lin
> > if (!sec_name)
> > return -ENOMEM;
> >
> > + *link = NULL;
> > +
> > + /* no auto-attach for SEC("tp") or SEC("tracepoint") */
> > + if (strcmp(prog->sec_name, "tp") == 0 || strcmp(prog->sec_name, "tracepoint") == 0)
> > + return 0;
> > +
> > /* extract "tp/<category>/<name>" or "tracepoint/<category>/<name>" */
> > if (str_has_pfx(prog->sec_name, "tp/"))
> > tp_cat = sec_name + sizeof("tp/") - 1;
> > @@ -11196,20 +11215,34 @@ struct bpf_link *bpf_program__attach_raw_tracepoint(const struct bpf_program *pr
> > static int attach_raw_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link)
> > {
> > static const char *const prefixes[] = {
> > - "raw_tp/",
> > - "raw_tracepoint/",
> > - "raw_tp.w/",
> > - "raw_tracepoint.w/",
> > + "raw_tp",
> > + "raw_tracepoint",
> > + "raw_tp.w",
> > + "raw_tracepoint.w",
> > };
> > size_t i;
> > const char *tp_name = NULL;
> >
> > + *link = NULL;
> > +
> > for (i = 0; i < ARRAY_SIZE(prefixes); i++) {
> > - if (str_has_pfx(prog->sec_name, prefixes[i])) {
> > - tp_name = prog->sec_name + strlen(prefixes[i]);
> > - break;
> > - }
> > + size_t pfx_len;
> > +
> > + if (!str_has_pfx(prog->sec_name, prefixes[i]))
> > + continue;
> > +
> > + pfx_len = strlen(prefixes[i]);
> > + /* no auto-attach case of, e.g., SEC("raw_tp") */
> > + if (prog->sec_name[pfx_len] == '\0')
> > + return 0;
> > +
> > + if (prog->sec_name[pfx_len] != '/')
> > + continue;
>
> Maybe introduce a sec_has_pfx() function with tri-state return value:
> 1 for match with tp_name, 0, for match without tp_name, -1 for no match.
>
Hm.. tri-state might be quite confusing, but there might be some clean
ups to be done around this prefix handling for SEC_DEF()s. I'm
planning to do some more work on SEC() handling, I'll do this clean up
as a follow up, if you don't mind. Need to see how to best consolidate
this across all the places where we do this prefix matching.
> > +
> > + tp_name = prog->sec_name + pfx_len + 1;
> > + break;
> > }
> > +
> > if (!tp_name) {
> > pr_warn("prog '%s': invalid section name '%s'\n",
> > prog->name, prog->sec_name);
> > --
> > 2.30.2
> >
next prev parent reply other threads:[~2022-04-08 22:22 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-08 20:34 [PATCH bpf-next 0/3] Add target-less tracing SEC() definitions Andrii Nakryiko
2022-04-08 20:34 ` [PATCH bpf-next 1/3] libbpf: allow "incomplete" basic " Andrii Nakryiko
2022-04-08 20:46 ` Song Liu
2022-04-08 22:21 ` Andrii Nakryiko [this message]
2022-04-08 22:36 ` Song Liu
2022-04-08 20:34 ` [PATCH bpf-next 2/3] libbpf: support target-less SEC() definitions for BTF-backed programs Andrii Nakryiko
2022-04-08 21:08 ` Song Liu
2022-04-08 20:34 ` [PATCH bpf-next 3/3] selftests/bpf: use target-less SEC() definitions in various tests Andrii Nakryiko
2022-04-08 21:08 ` Song Liu
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='CAEf4BzaZai7QZnBEcAV72D=0irABs9BjbH++rmRQkqKiKpLYvA@mail.gmail.com' \
--to=andrii.nakryiko@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@fb.com \
--cc=song@kernel.org \
/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.