On Fri, Mar 29, 2024 at 3:04 PM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Fri, Mar 22, 2024 at 3:26 AM Alan Maguire <alan.maguire@xxxxxxxxxx> wrote:
> >
> > Share reconciliation implementation with the kernel. As part of this,
> > we also need the type/string visitation functions so add them to a
> > btf_common.c file that also gets shared with the kernel. Reconciliation
> > code between kernel and userspace is identical save for the
> > implementation of the reparenting of split BTF to the reconciled base
> > BTF; this depends on struct btf internals so is different in kernel and
> > userspace.
> >
> > One other wrinkle on the kernel side is we have to map .BTF.ids in
> > modules as they were generated with the type ids used at BTF encoding
> > time. btf_reconcile() optionally returns an array mapping from old BTF
> > ids to reconciled ids, so we use that to fix up these references where
> > needed for kfuncs.
> >
> > Signed-off-by: Alan Maguire <alan.maguire@xxxxxxxxxx>
> > ---
> > include/linux/btf.h | 29 +++++
> > kernel/bpf/Makefile | 8 ++
> > kernel/bpf/btf.c | 197 +++++++++++++++++++++++++++++-----
> > tools/lib/bpf/Build | 2 +-
> > tools/lib/bpf/btf.c | 130 ----------------------
> > tools/lib/bpf/btf_common.c | 146 +++++++++++++++++++++++++
> > tools/lib/bpf/btf_reconcile.c | 24 +++++
> > 7 files changed, 376 insertions(+), 160 deletions(-)
> > create mode 100644 tools/lib/bpf/btf_common.c
> >
>
> [...]
>
> > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > index 09a11954cad8..e034f0c26c96 100644
> > --- a/tools/lib/bpf/btf.c
> > +++ b/tools/lib/bpf/btf.c
> > @@ -5026,136 +5026,6 @@ struct btf *btf__load_module_btf(const char *module_name, struct btf *vmlinux_bt
> > return btf__parse_split(path, vmlinux_btf);
> > }
> >
> > -int btf_type_visit_type_ids(struct btf_type *t, type_id_visit_fn visit, void *ctx)
>
> This and btf_type_visit_str_offs are heavily recursive functions not
> appropriate for kernel code, so you can't just share the code with
> kernel.
Sorry, a complete brain fart on my part. Of course they are not
recursive, there is no reason for them to be. So this iterator idea I
proposed, while I think is worth doing, strictly speaking isn't
necessary for your work.
>
> And before you rush adding artificial depth limits for kernel-side,
> let's discuss implementing btf_type_visit_type_ids and
> btf_type_visit_str_offs through iterator approach, just like we did
> for elf symbols iteration.
>
> I think it would be a general improvement to the point where we can
> probably think about exposing these BTF type id/string ref iterators
> as public API, as that's a very useful functionality. I just never
> felt like callback based API is the right abstraction (and still think
> that). But iterator sounds like a good idea and is worth doing as a
> preparatory series to simplify code in libbpf and preparing everything
> for kernel as well.
>
> > -{
> > - int i, n, err;
> > -
>
> [...]
