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; +} +[...]
