Re: [PATCH v2 bpf-next 5/9] libbpf: add kind layout encoding, crc support

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

 





On 6/20/23 2:09 AM, Alan Maguire wrote:
On 20/06/2023 00:24, Yonghong Song wrote:


On 6/16/23 10:17 AM, Alan Maguire wrote:
Support encoding of BTF kind layout data and crcs via
btf__new_empty_opts().

Current supported opts are base_btf, add_kind_layout and
add_crc.

Signed-off-by: Alan Maguire <alan.maguire@xxxxxxxxxx>
---
   tools/lib/bpf/btf.c      | 99 ++++++++++++++++++++++++++++++++++++++--
   tools/lib/bpf/btf.h      | 11 +++++
   tools/lib/bpf/libbpf.map |  1 +
   3 files changed, 108 insertions(+), 3 deletions(-)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 457997c2a43c..060a93809f64 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -16,6 +16,7 @@
   #include <linux/err.h>
   #include <linux/btf.h>
   #include <gelf.h>
+#include <zlib.h>
   #include "btf.h"
   #include "bpf.h"
   #include "libbpf.h"
@@ -882,8 +883,58 @@ void btf__free(struct btf *btf)
       free(btf);
   }
   -static struct btf *btf_new_empty(struct btf *base_btf)
+static void btf_add_kind_layout(struct btf *btf, __u8 kind,
+                __u16 flags, __u8 info_sz, __u8 elem_sz)
   {
+    struct btf_kind_layout *k = &btf->kind_layout[kind];
+
+    k->flags = flags;
+    k->info_sz = info_sz;
+    k->elem_sz = elem_sz;
+    btf->hdr->kind_layout_len += sizeof(*k);
+}
+
+static int btf_ensure_modifiable(struct btf *btf);
+
+static int btf_add_kind_layouts(struct btf *btf, struct btf_new_opts
*opts)
+{
+    if (btf_ensure_modifiable(btf))
+        return libbpf_err(-ENOMEM);
+
+    btf->kind_layout = calloc(NR_BTF_KINDS, sizeof(struct
btf_kind_layout));
+
+    if (!btf->kind_layout)
+        return -ENOMEM;
+
+    /* all supported kinds should describe their layout here. */
+    btf_add_kind_layout(btf, BTF_KIND_UNKN, 0, 0, 0);
+    btf_add_kind_layout(btf, BTF_KIND_INT, 0, sizeof(__u32), 0);
+    btf_add_kind_layout(btf, BTF_KIND_PTR, 0, 0, 0);
+    btf_add_kind_layout(btf, BTF_KIND_ARRAY, 0, sizeof(struct
btf_array), 0);
+    btf_add_kind_layout(btf, BTF_KIND_STRUCT, 0, 0, sizeof(struct
btf_member));
+    btf_add_kind_layout(btf, BTF_KIND_UNION, 0, 0, sizeof(struct
btf_member));
+    btf_add_kind_layout(btf, BTF_KIND_ENUM, 0, 0, sizeof(struct
btf_enum));
+    btf_add_kind_layout(btf, BTF_KIND_FWD, 0, 0, 0);
+    btf_add_kind_layout(btf, BTF_KIND_TYPEDEF, 0, 0, 0);
+    btf_add_kind_layout(btf, BTF_KIND_VOLATILE, 0, 0, 0);
+    btf_add_kind_layout(btf, BTF_KIND_CONST, 0, 0, 0);
+    btf_add_kind_layout(btf, BTF_KIND_RESTRICT, 0, 0, 0);
+    btf_add_kind_layout(btf, BTF_KIND_FUNC, 0, 0, 0);
+    btf_add_kind_layout(btf, BTF_KIND_FUNC_PROTO, 0, 0, sizeof(struct
btf_param));
+    btf_add_kind_layout(btf, BTF_KIND_VAR, 0, sizeof(struct btf_var),
0);
+    btf_add_kind_layout(btf, BTF_KIND_DATASEC, 0, 0, sizeof(struct
btf_var_secinfo));
+    btf_add_kind_layout(btf, BTF_KIND_FLOAT, 0, 0, 0);
+    btf_add_kind_layout(btf, BTF_KIND_DECL_TAG,
BTF_KIND_LAYOUT_OPTIONAL,
+                            sizeof(struct btf_decl_tag), 0);
+    btf_add_kind_layout(btf, BTF_KIND_TYPE_TAG,
BTF_KIND_LAYOUT_OPTIONAL, 0, 0);

BTF_KIND_TYPE_TAG cannot be optional. For example,
   ptr -> type_tag -> const -> int

if type_tag becomes optional, the whole type chain cannot be parsed
properly.


Ah, I missed that, thanks! You're absolutely right.

There are two separate concerns I think:

1. if an unknown kind (unknown to libbpf/kernel but present in the kind
    layout) is ever pointed at by another kind, regardless of optional
    status, the BTF must be rejected on the grounds that we don't really
    have a way to understand what the BTF means. That catches the case
    above.
2. however if an unknown kind exists in BTF and _is_ optional _and_
    is not pointed at by any known kinds, that is fine.

In other words it's logically possible for us to want to either
accept or reject BTF when we encounter unknown kinds that fall outside
of the existing type graph relations; the optional flag tells us which
to do.

I think for meta checking, the right way to handle 1 is to
reject BTF in the kind-specific meta checking for any known
kinds that can refer to other kinds; if the kind referred to
is > KIND_MAX, we reject the BTF.

I agree with your above analysis.


Also, in Patch 3, we have

+static int btf_type_size(const struct btf *btf, const struct btf_type *t)
  {
      const int base_size = sizeof(struct btf_type);
      __u16 vlen = btf_vlen(t);
@@ -363,8 +391,7 @@ static int btf_type_size(const struct btf_type *t)
      case BTF_KIND_DECL_TAG:
          return base_size + sizeof(struct btf_decl_tag);
      default:
-        pr_debug("Unsupported BTF_KIND:%u\n", btf_kind(t));
-        return -EINVAL;
+        return btf_type_size_unknown(btf, t);
      }
  }

Clearly even if we mark decl_tag as optional, it still handled properly
in the above. So decl_tag does not need BTF_KIND_LAYOUT_OPTIONAL, right?

But in btf_type_size_unknown() we have:

        if (!(k->flags & BTF_KIND_LAYOUT_OPTIONAL)) {
                 /* a required kind, and we do not know about it.. */
                 pr_debug("unknown but required kind: %u\n", kind);
                 return -EINVAL;
         }

The problem however I think is that we need to spot reference
types that refer to unknown kinds regardless of optional status
as described above.

What I mean is there is no need to tag decl_tag with
BTF_KIND_LAYOUT_OPTIONAL since for any btf with kind_layout,
decl_tag is already there. But BTF_KIND_LAYOUT_OPTIONAL is
still necessary for future kinds.


I guess what we really want to test is in the selftest:
   - Add a couple of new kinds for testing purpose, e.g.,
       BTF_KIND_OPTIONAL, BTF_KIND_NOT_OPTIONAL,
     generate two btf's which uses BTF_KIND_OPTIONAL
     and BTF_KIND_NOT_OPTIONAL respectively.
   - test these two btf's with this patch set to see whether it
     works as expected or not.

Does this make sense?


There's a test that does this currently for libbpf only (since we
need a struct btf to load into the kernel), but nothing to cover the
case where a reference type points at a kind we don't know about.
I'll update the tests to use reference types too.

Sounds good. thanks for adding additional tests.


Thanks!

Alan

+    btf_add_kind_layout(btf, BTF_KIND_ENUM64, 0, 0, sizeof(struct
btf_enum64));
+
+    return 0;
+}
+
[...]




[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