Message-ID: <5f6cb85452d72_4939c20820@john-XPS-13-9370.notmuch>
Date:   Thu, 24 Sep 2020 08:16:36 -0700
From:   John Fastabend <john.fastabend@...il.com>
To:     Andrii Nakryiko <andriin@...com>, bpf@...r.kernel.org,
        netdev@...r.kernel.org, ast@...com, daniel@...earbox.net
Cc:     andrii.nakryiko@...il.com, kernel-team@...com,
        Andrii Nakryiko <andriin@...com>,
        Arnaldo Carvalho de Melo <acme@...hat.com>
Subject: RE: [PATCH bpf-next 0/9] libbpf: BTF writer APIs

Andrii Nakryiko wrote:
> This patch set introduces a new set of BTF APIs to libbpf that allow to
> conveniently produce BTF types and strings. Internals of struct btf were
> changed such that it can transparently and automatically switch to writable
> mode, which allows appending BTF types and strings. This will allow for libbpf
> itself to do more intrusive modifications of program's BTF (by rewriting it,
> at least as of right now), which is necessary for the upcoming libbpf static
> linking. But they are complete and generic, so can be adopted by anyone who
> has a need to produce BTF type information.


I had this floating around on my todo list thanks a lot for doing it. I can
remove a couple more silly hacks I have floating around now!

> 
> One such example outside of libbpf is pahole, which was actually converted to
> these APIs (locally, pending landing of these changes in libbpf) completely
> and shows reduction in amount of custom pahole code necessary and brings nice
> savings in memory usage (about 370MB reduction at peak for my kernel
> configuration) and even BTF deduplication times (one second reduction,
> 23.7s -> 22.7s). Memory savings are due to avoiding pahole's own copy of
> "uncompressed" raw BTF data. Time reduction comes from faster string
> search and deduplication by relying on hashmap instead of BST used by pahole's
> own code. Consequently, these APIs are already tested on real-world
> complicated kernel BTF, but there is also pretty extensive selftest doing
> extra validations.
> 
> Selftests in patch #9 add a set of generic ASSERT_{EQ,STREQ,ERR,OK} macros
> that are useful for writing shorter and less repretitive selftests. I decided
> to keep them local to that selftest for now, but if they prove to be useful in
> more contexts we should move them to test_progs.h. And few more (e.g.,
> inequality tests) macros are probably necessary to have a more complete set.
> 
> Cc: Arnaldo Carvalho de Melo <acme@...hat.com>

For the series, I have a few nits I'll put in the patches, mostly spelling 
errors and a couple questions, otherwise this is awesome thanks.

Acked-by: John Fastabend <john.fastabend@...il.com>

> 
> Andrii Nakryiko (9):
>   libbpf: refactor internals of BTF type index
>   libbpf: remove assumption of single contiguous memory for BTF data
>   libbpf: generalize common logic for managing dynamically-sized arrays
>   libbpf: extract generic string hashing function for reuse
>   libbpf: allow modification of BTF and add btf__add_str API
>   libbpf: add btf__new_empty() to create an empty BTF object
>   libbpf: add BTF writing APIs
>   libbpf: add btf__str_by_offset() as a more generic variant of
>     name_by_offset
>   selftests/bpf: test BTF writing APIs
> 
>  tools/lib/bpf/bpf.c                           |    2 +-
>  tools/lib/bpf/bpf.h                           |    2 +-
>  tools/lib/bpf/btf.c                           | 1311 +++++++++++++++--
>  tools/lib/bpf/btf.h                           |   41 +
>  tools/lib/bpf/btf_dump.c                      |    9 +-
>  tools/lib/bpf/hashmap.h                       |   12 +
>  tools/lib/bpf/libbpf.map                      |   22 +
>  tools/lib/bpf/libbpf_internal.h               |    3 +
>  .../selftests/bpf/prog_tests/btf_write.c      |  271 ++++
>  9 files changed, 1553 insertions(+), 120 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/btf_write.c
> 
> -- 
> 2.24.1
> 

Powered by blists - more mailing lists