Message-ID: <YMDJT4fnLCOsfFuS@krava>
Date:   Wed, 9 Jun 2021 15:59:43 +0200
From:   Jiri Olsa <jolsa@...hat.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     Jiri Olsa <jolsa@...nel.org>, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andriin@...com>,
        "Steven Rostedt (VMware)" <rostedt@...dmis.org>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...omium.org>, Daniel Xu <dxu@...uu.xyz>,
        Viktor Malik <vmalik@...hat.com>
Subject: Re: [PATCH 14/19] libbpf: Add btf__find_by_pattern_kind function

On Tue, Jun 08, 2021 at 10:29:19PM -0700, Andrii Nakryiko wrote:
> On Sat, Jun 5, 2021 at 4:14 AM Jiri Olsa <jolsa@...nel.org> wrote:
> >
> > Adding btf__find_by_pattern_kind function that returns
> > array of BTF ids for given function name pattern.
> >
> > Using libc's regex.h support for that.
> >
> > Signed-off-by: Jiri Olsa <jolsa@...nel.org>
> > ---
> >  tools/lib/bpf/btf.c | 68 +++++++++++++++++++++++++++++++++++++++++++++
> >  tools/lib/bpf/btf.h |  3 ++
> >  2 files changed, 71 insertions(+)
> >
> > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > index b46760b93bb4..421dd6c1e44a 100644
> > --- a/tools/lib/bpf/btf.c
> > +++ b/tools/lib/bpf/btf.c
> > @@ -1,6 +1,7 @@
> >  // SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
> >  /* Copyright (c) 2018 Facebook */
> >
> > +#define _GNU_SOURCE
> >  #include <byteswap.h>
> >  #include <endian.h>
> >  #include <stdio.h>
> > @@ -16,6 +17,7 @@
> >  #include <linux/err.h>
> >  #include <linux/btf.h>
> >  #include <gelf.h>
> > +#include <regex.h>
> >  #include "btf.h"
> >  #include "bpf.h"
> >  #include "libbpf.h"
> > @@ -711,6 +713,72 @@ __s32 btf__find_by_name_kind(const struct btf *btf, const char *type_name,
> >         return libbpf_err(-ENOENT);
> >  }
> >
> > +static bool is_wildcard(char c)
> > +{
> > +       static const char *wildchars = "*?[|";
> > +
> > +       return strchr(wildchars, c);
> > +}
> > +
> > +int btf__find_by_pattern_kind(const struct btf *btf,
> > +                             const char *type_pattern, __u32 kind,
> > +                             __s32 **__ids)
> > +{
> > +       __u32 i, nr_types = btf__get_nr_types(btf);
> > +       __s32 *ids = NULL;
> > +       int cnt = 0, alloc = 0, ret;
> > +       regex_t regex;
> > +       char *pattern;
> > +
> > +       if (kind == BTF_KIND_UNKN || !strcmp(type_pattern, "void"))
> > +               return 0;
> > +
> > +       /* When the pattern does not start with wildcard, treat it as
> > +        * if we'd want to match it from the beginning of the string.
> > +        */
> 
> This assumption is absolutely atrocious. If we say it's regexp, then
> it has to always be regexp, not something based on some random
> heuristic based on the first character.
> 
> Taking a step back, though. Do we really need to provide this API? Why
> applications can't implement it on their own, given regexp
> functionality is provided by libc. Which I didn't know, actually, so
> that's pretty nice, assuming that it's also available in more minimal
> implementations like musl.
> 

so the only purpose for this function is to support wildcards in
tests like:

  SEC("fentry.multi/bpf_fentry_test*")

so the generic skeleton attach function can work.. but that can be
removed and the test programs can be attached manually through some
other attach function that will have list of functions as argument

jirka

> > +       asprintf(&pattern, "%s%s",
> > +                is_wildcard(type_pattern[0]) ? "^" : "",
> > +                type_pattern);
> > +
> > +       ret = regcomp(&regex, pattern, REG_EXTENDED);
> > +       if (ret) {
> > +               pr_warn("failed to compile regex\n");
> > +               free(pattern);
> > +               return -EINVAL;
> > +       }
> > +
> > +       free(pattern);
> > +
> > +       for (i = 1; i <= nr_types; i++) {
> > +               const struct btf_type *t = btf__type_by_id(btf, i);
> > +               const char *name;
> > +               __s32 *p;
> > +
> > +               if (btf_kind(t) != kind)
> > +                       continue;
> > +               name = btf__name_by_offset(btf, t->name_off);
> > +               if (name && regexec(&regex, name, 0, NULL, 0))
> > +                       continue;
> > +               if (cnt == alloc) {
> > +                       alloc = max(100, alloc * 3 / 2);
> > +                       p = realloc(ids, alloc * sizeof(__u32));
> 
> this memory allocation and re-allocation on behalf of users is another
> argument against this API
> 
> > +                       if (!p) {
> > +                               free(ids);
> > +                               regfree(&regex);
> > +                               return -ENOMEM;
> > +                       }
> > +                       ids = p;
> > +               }
> > +
> > +               ids[cnt] = i;
> > +               cnt++;
> > +       }
> > +
> > +       regfree(&regex);
> > +       *__ids = ids;
> > +       return cnt ?: -ENOENT;
> > +}
> > +
> >  static bool btf_is_modifiable(const struct btf *btf)
> >  {
> >         return (void *)btf->hdr != btf->raw_data;
> > diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
> > index b54f1c3ebd57..036857aded94 100644
> > --- a/tools/lib/bpf/btf.h
> > +++ b/tools/lib/bpf/btf.h
> > @@ -371,6 +371,9 @@ btf_var_secinfos(const struct btf_type *t)
> >         return (struct btf_var_secinfo *)(t + 1);
> >  }
> >
> > +int btf__find_by_pattern_kind(const struct btf *btf,
> > +                             const char *type_pattern, __u32 kind,
> > +                             __s32 **__ids);
> >  #ifdef __cplusplus
> >  } /* extern "C" */
> >  #endif
> > --
> > 2.31.1
> >
> 

Powered by blists - more mailing lists