diff mbox series

[bpf-next,1/3] libbpf: allow "incomplete" basic tracing SEC() definitions

Message ID 20220408203433.2988727-2-andrii@kernel.org (mailing list archive)
State New, archived
Delegated to: BPF
Headers show
Series Add target-less tracing SEC() definitions | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR pending PR summary
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 6 maintainers not CCed: songliubraving@fb.com netdev@vger.kernel.org kafai@fb.com yhs@fb.com john.fastabend@gmail.com kpsingh@kernel.org
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: line length of 103 exceeds 80 columns WARNING: line length of 84 exceeds 80 columns WARNING: line length of 91 exceeds 80 columns WARNING: line length of 93 exceeds 80 columns WARNING: line length of 94 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-1 success Logs for Kernel LATEST on ubuntu-latest + selftests
bpf/vmtest-bpf-next-VM_Test-2 pending Logs for Kernel LATEST on z15 + selftests

Commit Message

Andrii Nakryiko April 8, 2022, 8:34 p.m. UTC
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(-)

Comments

Song Liu April 8, 2022, 8:46 p.m. UTC | #1
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.

> +
> +               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
>
Andrii Nakryiko April 8, 2022, 10:21 p.m. UTC | #2
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
> >
Song Liu April 8, 2022, 10:36 p.m. UTC | #3
> On Apr 8, 2022, at 3:21 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> 
> 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.

Sounds good to me. 

Acked-by: Song Liu <songliubraving@fb.com>
diff mbox series

Patch

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;
+
+		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);