Re: [PATCH v2 bpf-next 4/7] libbpf: wire up spec management and other arch-independent USDT logic

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Mon, Apr 4, 2022 at 12:59 PM Dave Marchevsky <davemarchevsky@fb.com> wrote:
>
> On 4/1/22 8:29 PM, Andrii Nakryiko wrote:
> > Last part of architecture-agnostic user-space USDT handling logic is to
> > set up BPF spec and, optionally, IP-to-ID maps from user-space.
> > usdt_manager performs a compact spec ID allocation to utilize
> > fixed-sized BPF maps as efficiently as possible. We also use hashmap to
> > deduplicate USDT arg spec strings and map identical strings to single
> > USDT spec, minimizing the necessary BPF map size. usdt_manager supports
> > arbitrary sequences of attachment and detachment, both of the same USDT
> > and multiple different USDTs and internally maintains a free list of
> > unused spec IDs. bpf_link_usdt's logic is extended with proper setup and
> > teardown of this spec ID free list and supporting BPF maps.
> >
> > Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
> >  tools/lib/bpf/usdt.c | 168 ++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 167 insertions(+), 1 deletion(-)
>
> [...]
>
> > diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c
> > index c9eff690e291..afbae742c081 100644
> > --- a/tools/lib/bpf/usdt.c
> > +++ b/tools/lib/bpf/usdt.c
>
> [...]
>
> > +static size_t specs_hash_fn(const void *key, void *ctx)
> > +{
> > +     const char *s = key;
> > +
> > +     return str_hash(s);
> > +}
> > +
> > +static bool specs_equal_fn(const void *key1, const void *key2, void *ctx)
> > +{
> > +     const char *s1 = key1;
> > +     const char *s2 = key2;
> > +
> > +     return strcmp(s1, s2) == 0;
> > +}
>
> IIUC, you're not worried about diabolical strings in strcmp and str_hash here
> because of sanity checking in parse_usdt_note?
>

Yeah, we perform parse_usdt_spec() before we get to hashmap, so if
there is anything too funky about spec string we'll error out much
sooner.

> Anyways,
>
> Reviewed-by: Dave Marchevsky <davemarchevsky@fb.com>