On Wed, Jun 22, 2022 at 09:15:01PM -0700, Andrii Nakryiko wrote:
> On Wed, Jun 22, 2022 at 10:35 AM Daniel Müller <deso@xxxxxxxxxx> wrote:
> >
> > This change merges the two existing implementations of the
> > bpf_core_types_are_compat() function into relo_core.c, inheriting the
> > recursion tracking from the kernel and the usage of
> > btf_kind_core_compat() from libbpf. The kernel is left untouched and
> > will be adjusted subsequently.
> >
> > Signed-off-by: Daniel Müller <deso@xxxxxxxxxx>
> > ---
>
> I don't feel very strongly about this, but given we are consolidating
> kernel and libbpf code, I think it makes sense to do it in one patch.
Sure.
> > tools/lib/bpf/libbpf.c | 72 +-----------------------------------
> > tools/lib/bpf/relo_core.c | 78 +++++++++++++++++++++++++++++++++++++++
> > tools/lib/bpf/relo_core.h | 2 +
> > 3 files changed, 81 insertions(+), 71 deletions(-)
> >
>
> [...]
>
> > - default:
> > - pr_warn("unexpected kind %s relocated, local [%d], target [%d]\n",
> > - btf_kind_str(local_type), local_id, targ_id);
> > - return 0;
> > - }
> > + return bpf_core_types_are_compat_recur(local_btf, local_id, targ_btf, targ_id, INT_MAX);
>
> INT_MAX seems like an overkill, let's just hard-code 32 just like we
> have for a local recursion limit here?
Okay.
> > }
> >
>
> [...]
>
> > /*
> > * Turn bpf_core_relo into a low- and high-level spec representation,
> > * validating correctness along the way, as well as calculating resulting
> > diff --git a/tools/lib/bpf/relo_core.h b/tools/lib/bpf/relo_core.h
> > index 7df0da0..b8998f 100644
> > --- a/tools/lib/bpf/relo_core.h
> > +++ b/tools/lib/bpf/relo_core.h
> > @@ -68,6 +68,8 @@ struct bpf_core_relo_res {
> > __u32 new_type_id;
> > };
> >
> > +int bpf_core_types_are_compat_recur(const struct btf *local_btf, __u32 local_id,
> > + const struct btf *targ_btf, __u32 targ_id, int level);
>
> Just leave it called __bpf_core_types_are_compat like in kernel, it is
> clearly an "internal" version of bpf_core_types_are_compat(), so it's
> more proper naming convention.
Sounds good.
[...]
Thanks,
Daniel
