On Fri, Jun 16, 2023 at 10:18 AM Alan Maguire <alan.maguire@xxxxxxxxxx> 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. */
why not have a static table for each known kind and then just memcpy
it? Seems both more elegant and more maintainable?
static struct btf_kind_layout layout[] = {
[BTF_KIND_UNKN] = {0, 0, 0},
[BTF_KIND_UNKN] = {0, sizeof(__u32), 0},
...
[BTF_KIND_STRUCT] = {0, 0, sizeof(struct btf_member)},
};
?
> + 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_add_kind_layout(btf, BTF_KIND_ENUM64, 0, 0, sizeof(struct btf_enum64));
> +
> + return 0;
> +}
> +
> +static struct btf *btf_new_empty(struct btf_new_opts *opts)
> +{
> + struct btf *base_btf = OPTS_GET(opts, base_btf, NULL);
> struct btf *btf;
>
> btf = calloc(1, sizeof(*btf));
> @@ -920,17 +971,53 @@ static struct btf *btf_new_empty(struct btf *base_btf)
> btf->strs_data = btf->raw_data + btf->hdr->hdr_len;
> btf->hdr->str_len = base_btf ? 0 : 1; /* empty string at offset 0 */
>
> + if (opts->add_kind_layout) {
> + int err = btf_add_kind_layouts(btf, opts);
> +
as I mentioned, I'd always add it internally, but allow to control
whether it has to be emitted into raw_data
> + if (err) {
> + free(btf->raw_data);
> + free(btf);
> + return ERR_PTR(err);
> + }
> + }
> + if (opts->add_crc) {
> + if (btf->base_btf) {
> + struct btf_header *base_hdr = btf->base_btf->hdr;
> +
> + if (base_hdr->hdr_len >= sizeof(struct btf_header) &&
> + base_hdr->flags & BTF_FLAG_CRC_SET) {
> + btf->hdr->base_crc = base_hdr->crc;
> + btf->hdr->flags |= BTF_FLAG_BASE_CRC_SET;
> + }
> + }
> + btf->hdr->flags |= BTF_FLAG_CRC_SET;
> + }
> +
> return btf;
> }
>
> struct btf *btf__new_empty(void)
> {
> - return libbpf_ptr(btf_new_empty(NULL));
> + LIBBPF_OPTS(btf_new_opts, opts);
> +
> + return libbpf_ptr(btf_new_empty(&opts));
why? just pass NULL, it's fine, no need to create empty opts
> }
>
> struct btf *btf__new_empty_split(struct btf *base_btf)
> {
> - return libbpf_ptr(btf_new_empty(base_btf));
> + LIBBPF_OPTS(btf_new_opts, opts);
> +
> + opts.base_btf = base_btf;
nit: it's cleaner to initialize opts on declaration in this case
LIBBPF_OPTS(btf_new_opts, opts, .base_btf = base_btf);
> +
> + return libbpf_ptr(btf_new_empty(&opts));
> +}
> +
> +struct btf *btf__new_empty_opts(struct btf_new_opts *opts)
> +{
> + if (!OPTS_VALID(opts, btf_new_opts))
> + return libbpf_err_ptr(-EINVAL);
> +
> + return libbpf_ptr(btf_new_empty(opts));
> }
>
> static struct btf *btf_new(const void *data, __u32 size, struct btf *base_btf)
> @@ -1377,6 +1464,12 @@ static void *btf_get_raw_data(const struct btf *btf, __u32 *size, bool swap_endi
> btf_bswap_kind_layout_sec(p, hdr->kind_layout_len);
> p += hdr->kind_layout_len;
> }
> + if (hdr->flags & BTF_FLAG_CRC_SET) {
> + struct btf_header *h = data;
> +
> + h->crc = crc32(0L, (const Bytef *)&data, sizeof(data));
is crc32() part of zlib's public API?
> + }
> +
> *size = data_sz;
> return data;
> err_out:
> diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
> index 8e6880d91c84..d25bd5c19eec 100644
> --- a/tools/lib/bpf/btf.h
> +++ b/tools/lib/bpf/btf.h
> @@ -107,6 +107,17 @@ LIBBPF_API struct btf *btf__new_empty(void);
> */
> LIBBPF_API struct btf *btf__new_empty_split(struct btf *base_btf);
>
> +struct btf_new_opts {
> + size_t sz;
> + struct btf *base_btf; /* optional base BTF */
> + bool add_kind_layout; /* add BTF kind layout information */
I'd say let's make it opt-out option and by default do emit kind
layout? so rather "skip_kind_layout"
> + bool add_crc; /* add CRC information */
same for crc? "skip_crc"?
btw, does add_crc imply both crc and base_crc (if base_btf != NULL)?
just thinking out loud if we need to control that independently...
> + size_t:0;
> +};
> +#define btf_new_opts__last_field add_crc
> +
> +LIBBPF_API struct btf *btf__new_empty_opts(struct btf_new_opts *opts);
> +
> LIBBPF_API struct btf *btf__parse(const char *path, struct btf_ext **btf_ext);
> LIBBPF_API struct btf *btf__parse_split(const char *path, struct btf *base_btf);
> LIBBPF_API struct btf *btf__parse_elf(const char *path, struct btf_ext **btf_ext);
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 7521a2fb7626..edd8be4b21d0 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -395,4 +395,5 @@ LIBBPF_1.2.0 {
> LIBBPF_1.3.0 {
> global:
> bpf_obj_pin_opts;
> + btf__new_empty_opts;
> } LIBBPF_1.2.0;
> --
> 2.39.3
>
