On Fri, May 13, 2022 at 8:13 PM Yonghong Song <yhs@xxxxxx> wrote:
>
> Add enum64 deduplication support. BTF_KIND_ENUM64 handling
> is very similar to BTF_KIND_ENUM.
>
> Signed-off-by: Yonghong Song <yhs@xxxxxx>
> ---
> tools/lib/bpf/btf.c | 55 +++++++++++++++++++++++++++++++++------------
> tools/lib/bpf/btf.h | 5 +++++
> 2 files changed, 46 insertions(+), 14 deletions(-)
>
[...]
> +static bool btf_equal_enum64_val(struct btf_type *t1, struct btf_type *t2)
> +{
> + const struct btf_enum64 *m1, *m2;
> + __u16 vlen = btf_vlen(t1);
> + int i;
> +
> + m1 = btf_enum64(t1);
> + m2 = btf_enum64(t2);
> + for (i = 0; i < vlen; i++) {
> + if (m1->name_off != m2->name_off || m1->val_lo32 != m2->val_lo32 ||
> + m1->val_hi32 != m2->val_hi32)
> + return false;
> + m1++;
> + m2++;
> + }
> + return true;
> +}
> +
> +/* Check structural equality of two ENUMs. */
> +static bool btf_equal_enum_or_enum64(struct btf_type *t1, struct btf_type *t2)
I find this helper quite confusing. It implies it can compare any enum
or enum64 with each other, but it really allows only enum vs enum and
enum64 vs enum64 (as it should!). Let's keep
btf_equal_enum()/btf_compat_enum() completely intact and add
btf_equal_enum64()/btf_compat_enum64() separately (few lines of
copy-pasted code is totally fine to keep them separate, IMO). See
below.
> +{
> + if (!btf_equal_common(t1, t2))
> + return false;
> +
> + if (btf_is_enum(t1))
> + return btf_equal_enum32_val(t1, t2);
> + return btf_equal_enum64_val(t1, t2);
> +}
> +
> static inline bool btf_is_enum_fwd(struct btf_type *t)
> {
> - return btf_is_enum(t) && btf_vlen(t) == 0;
> + return btf_type_is_any_enum(t) && btf_vlen(t) == 0;
> }
>
> -static bool btf_compat_enum(struct btf_type *t1, struct btf_type *t2)
> +static bool btf_compat_enum_or_enum64(struct btf_type *t1, struct btf_type *t2)
> {
> if (!btf_is_enum_fwd(t1) && !btf_is_enum_fwd(t2))
> - return btf_equal_enum(t1, t2);
> + return btf_equal_enum_or_enum64(t1, t2);
> /* ignore vlen when comparing */
> return t1->name_off == t2->name_off &&
> (t1->info & ~0xffff) == (t2->info & ~0xffff) &&
> @@ -3829,6 +3853,7 @@ static int btf_dedup_prep(struct btf_dedup *d)
> h = btf_hash_int_decl_tag(t);
> break;
> case BTF_KIND_ENUM:
> + case BTF_KIND_ENUM64:
> h = btf_hash_enum(t);
> break;
> case BTF_KIND_STRUCT:
> @@ -3898,15 +3923,16 @@ static int btf_dedup_prim_type(struct btf_dedup *d, __u32 type_id)
> break;
>
> case BTF_KIND_ENUM:
> + case BTF_KIND_ENUM64:
> h = btf_hash_enum(t);
> for_each_dedup_cand(d, hash_entry, h) {
> cand_id = (__u32)(long)hash_entry->value;
> cand = btf_type_by_id(d->btf, cand_id);
> - if (btf_equal_enum(t, cand)) {
> + if (btf_equal_enum_or_enum64(t, cand)) {
I'd rather have this enum vs enum64 distinction right here:
if ((btf_is_enum(t) && btf_equal_enum(t, cand)) ||
(btf_is_enum64(t) && btf_equal_enum64(t, cand))) { ... }
> new_id = cand_id;
> break;
> }
> - if (btf_compat_enum(t, cand)) {
> + if (btf_compat_enum_or_enum64(t, cand)) {
and same here with (btf_is_enum && btf_compat_enum) || (btf_is_num64
&& btf_compat_enum64) ?
Basically, I'd like to avoid worrying if we are accidentally mixing
enum and enum64 or not. WDYT?
> if (btf_is_enum_fwd(t)) {
> /* resolve fwd to full enum */
> new_id = cand_id;
> @@ -4211,7 +4237,8 @@ static int btf_dedup_is_equiv(struct btf_dedup *d, __u32 cand_id,
> return btf_equal_int_tag(cand_type, canon_type);
>
> case BTF_KIND_ENUM:
> - return btf_compat_enum(cand_type, canon_type);
> + case BTF_KIND_ENUM64:
> + return btf_compat_enum_or_enum64(cand_type, canon_type);
>
and here we just know whether we need btf_compat_enum vs btf_compat_enum64.
> case BTF_KIND_FWD:
> case BTF_KIND_FLOAT:
> diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
> index a41463bf9060..b22c648c69ff 100644
> --- a/tools/lib/bpf/btf.h
> +++ b/tools/lib/bpf/btf.h
> @@ -531,6 +531,11 @@ static inline bool btf_is_type_tag(const struct btf_type *t)
> return btf_kind(t) == BTF_KIND_TYPE_TAG;
> }
>
> +static inline bool btf_type_is_any_enum(const struct btf_type *t)
> +{
> + return btf_is_enum(t) || btf_is_enum64(t);
> +}
> +
> static inline __u8 btf_int_encoding(const struct btf_type *t)
> {
> return BTF_INT_ENCODING(*(__u32 *)(t + 1));
> --
> 2.30.2
>
