Re: [PATCH bpf-next v2 10/18] libbpf: Add enum64 relocation support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 5/17/22 4:32 PM, Andrii Nakryiko wrote:
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?

okay.


                         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()?

old comment is not accurate any more, will remove.


-               *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

will do.


+               }
                 return len;
         }

--
2.30.2




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux