On Fri, May 13, 2022 at 8:13 PM Yonghong Song <yhs@xxxxxx> wrote:
>
> The enum64 relocation support is added. The bpf local type
> could be either enum or enum64 and the remote type could be
> either enum or enum64 too. The all combinations of local enum/enum64
> and remote enum/enum64 are supported.
>
> Signed-off-by: Yonghong Song <yhs@xxxxxx>
> ---
> tools/lib/bpf/btf.h | 7 ++++++
> tools/lib/bpf/libbpf.c | 7 +++---
> tools/lib/bpf/relo_core.c | 49 ++++++++++++++++++++++++++-------------
> 3 files changed, 44 insertions(+), 19 deletions(-)
>
[...]
> memset(targ_spec, 0, sizeof(*targ_spec));
> targ_spec->btf = targ_btf;
> @@ -494,18 +498,22 @@ static int bpf_core_spec_match(struct bpf_core_spec *local_spec,
>
> if (core_relo_is_enumval_based(local_spec->relo_kind)) {
> size_t local_essent_len, targ_essent_len;
> - const struct btf_enum *e;
> const char *targ_name;
>
> /* has to resolve to an enum */
> targ_type = skip_mods_and_typedefs(targ_spec->btf, targ_id, &targ_id);
> - if (!btf_is_enum(targ_type))
> + if (!btf_type_is_any_enum(targ_type))
just noticed this discrepancy, can you please rename
s/btf_type_is_any_enum/btf_is_any_enum/ so it's consistent with
btf_is_enum and btf_is_enum64?
> return 0;
>
> local_essent_len = bpf_core_essential_name_len(local_acc->name);
>
> - for (i = 0, e = btf_enum(targ_type); i < btf_vlen(targ_type); i++, e++) {
> - targ_name = btf__name_by_offset(targ_spec->btf, e->name_off);
> + for (i = 0; i < btf_vlen(targ_type); i++) {
> + if (btf_is_enum(targ_type))
> + name_off = btf_enum(targ_type)[i].name_off;
> + else
> + name_off = btf_enum64(targ_type)[i].name_off;
> +
> + targ_name = btf__name_by_offset(targ_spec->btf, name_off);
> targ_essent_len = bpf_core_essential_name_len(targ_name);
> if (targ_essent_len != local_essent_len)
> continue;
> @@ -681,7 +689,7 @@ static int bpf_core_calc_field_relo(const char *prog_name,
> break;
> case BPF_CORE_FIELD_SIGNED:
> /* enums will be assumed unsigned */
we don't have to assume anymore, right? let's use kflag for btf_is_any_enum()?
> - *val = btf_is_enum(mt) ||
> + *val = btf_type_is_any_enum(mt) ||
> (btf_int_encoding(mt) & BTF_INT_SIGNED);
> if (validate)
> *validate = true; /* signedness is never ambiguous */
[...]
> @@ -1089,10 +1097,19 @@ int bpf_core_format_spec(char *buf, size_t buf_sz, const struct bpf_core_spec *s
>
> if (core_relo_is_enumval_based(spec->relo_kind)) {
> t = skip_mods_and_typedefs(spec->btf, type_id, NULL);
> - e = btf_enum(t) + spec->raw_spec[0];
> - s = btf__name_by_offset(spec->btf, e->name_off);
> + if (btf_is_enum(t)) {
> + const struct btf_enum *e;
>
> - append_buf("::%s = %u", s, e->val);
> + e = btf_enum(t) + spec->raw_spec[0];
> + s = btf__name_by_offset(spec->btf, e->name_off);
> + append_buf("::%s = %u", s, e->val);
> + } else {
> + const struct btf_enum64 *e;
> +
> + e = btf_enum64(t) + spec->raw_spec[0];
> + s = btf__name_by_offset(spec->btf, e->name_off);
> + append_buf("::%s = %llu", s, btf_enum64_value(e));
nit: we do have a sign bit now, so maybe let's print %lld or %llu
(same for %d and %u above)? btw, please cast (unsigned long long) here
> + }
> return len;
> }
>
> --
> 2.30.2
>
