On Mon, Jun 5, 2023 at 4:01 AM Jiri Olsa <olsajiri@xxxxxxxxx> wrote:
>
> On Wed, May 31, 2023 at 09:19:29PM +0100, Alan Maguire wrote:
> > support reading in metadata, fixing endian issues on reading;
> > also support writing metadata section to raw BTF object.
> > There is not yet an API to populate the metadata with meaningful
> > information.
> >
> > Signed-off-by: Alan Maguire <alan.maguire@xxxxxxxxxx>
> > ---
> > tools/lib/bpf/btf.c | 141 ++++++++++++++++++++++++++++++++++----------
> > 1 file changed, 111 insertions(+), 30 deletions(-)
> >
> > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > index 8484b563b53d..036dc1505969 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"
> > @@ -39,36 +40,40 @@ struct btf {
> >
> > /*
> > * When BTF is loaded from an ELF or raw memory it is stored
> > - * in a contiguous memory block. The hdr, type_data, and, strs_data
> > - * point inside that memory region to their respective parts of BTF
> > - * representation:
> > + * in a contiguous memory block. The hdr, type_data, strs_data,
> > + * and optional meta_data point inside that memory region to their
> > + * respective parts of BTF representation:
> > *
> > - * +--------------------------------+
> > - * | Header | Types | Strings |
> > - * +--------------------------------+
> > - * ^ ^ ^
> > - * | | |
> > - * hdr | |
> > - * types_data-+ |
> > - * strs_data------------+
> > + * +--------------------------------+----------+
> > + * | Header | Types | Strings | Metadata |
> > + * +--------------------------------+----------+
> > + * ^ ^ ^ ^
> > + * | | | |
> > + * hdr | | |
> > + * types_data-+ | |
> > + * strs_data------------+ |
> > + * meta_data------------------------+
> > + *
> > + * meta_data is optional.
> > *
> > * If BTF data is later modified, e.g., due to types added or
> > * removed, BTF deduplication performed, etc, this contiguous
> > - * representation is broken up into three independently allocated
> > - * memory regions to be able to modify them independently.
> > + * representation is broken up into three or four independently
> > + * allocated memory regions to be able to modify them independently.
> > * raw_data is nulled out at that point, but can be later allocated
> > * and cached again if user calls btf__raw_data(), at which point
> > - * raw_data will contain a contiguous copy of header, types, and
> > - * strings:
> > + * raw_data will contain a contiguous copy of header, types, strings
> > + * and (again optionally) metadata:
> > *
> > - * +----------+ +---------+ +-----------+
> > - * | Header | | Types | | Strings |
> > - * +----------+ +---------+ +-----------+
> > - * ^ ^ ^
> > - * | | |
> > - * hdr | |
> > - * types_data----+ |
> > - * strset__data(strs_set)-----+
> > + * +----------+ +---------+ +-----------+ +----------+
> > + * | Header | | Types | | Strings | | Metadata |
> > + * +----------+ +---------+ +-----------+ +---------_+
> > + * ^ ^ ^ ^
> > + * | | | |
> > + * hdr | | |
> > + * types_data----+ | |
> > + * strset__data(strs_set)-----+ |
> > + * meta_data---------------------------------+
> > *
> > * +----------+---------+-----------+
> > * | Header | Types | Strings |
> > @@ -116,6 +121,8 @@ struct btf {
> > /* whether strings are already deduplicated */
> > bool strs_deduped;
> >
> > + void *meta_data;
> > +
> > /* BTF object FD, if loaded into kernel */
> > int fd;
> >
> > @@ -215,6 +222,11 @@ static void btf_bswap_hdr(struct btf_header *h)
> > h->type_len = bswap_32(h->type_len);
> > h->str_off = bswap_32(h->str_off);
> > h->str_len = bswap_32(h->str_len);
> > + if (h->hdr_len >= sizeof(struct btf_header)) {
> > + h->meta_header.meta_off = bswap_32(h->meta_header.meta_off);
> > + h->meta_header.meta_len = bswap_32(h->meta_header.meta_len);
> > + }
> > +
> > }
> >
> > static int btf_parse_hdr(struct btf *btf)
> > @@ -222,14 +234,17 @@ static int btf_parse_hdr(struct btf *btf)
> > struct btf_header *hdr = btf->hdr;
> > __u32 meta_left;
> >
> > - if (btf->raw_size < sizeof(struct btf_header)) {
> > + if (btf->raw_size < sizeof(struct btf_header) - sizeof(struct btf_meta_header)) {
> > pr_debug("BTF header not found\n");
> > return -EINVAL;
> > }
> >
> > if (hdr->magic == bswap_16(BTF_MAGIC)) {
> > + int swapped_len = bswap_32(hdr->hdr_len);
> > +
> > btf->swapped_endian = true;
> > - if (bswap_32(hdr->hdr_len) != sizeof(struct btf_header)) {
> > + if (swapped_len != sizeof(struct btf_header) &&
> > + swapped_len != sizeof(struct btf_header) - sizeof(struct btf_meta_header)) {
> > pr_warn("Can't load BTF with non-native endianness due to unsupported header length %u\n",
> > bswap_32(hdr->hdr_len));
> > return -ENOTSUP;
> > @@ -285,6 +300,42 @@ static int btf_parse_str_sec(struct btf *btf)
> > return 0;
> > }
> >
> > +static void btf_bswap_meta(struct btf_metadata *meta, int len)
> > +{
> > + struct btf_kind_meta *m = &meta->kind_meta[0];
> > + struct btf_kind_meta *end = (void *)meta + len;
> > +
> > + meta->flags = bswap_32(meta->flags);
> > + meta->crc = bswap_32(meta->crc);
> > + meta->base_crc = bswap_32(meta->base_crc);
> > + meta->description_off = bswap_32(meta->description_off);
> > +
> > + while (m < end) {
> > + m->name_off = bswap_32(m->name_off);
> > + m->flags = bswap_16(m->flags);
> > + m++;
> > + }
> > +}
> > +
> > +static int btf_parse_meta_sec(struct btf *btf)
> > +{
> > + const struct btf_header *hdr = btf->hdr;
> > +
> > + if (hdr->hdr_len < sizeof(struct btf_header) ||
> > + !hdr->meta_header.meta_off || !hdr->meta_header.meta_len)
> > + return 0;
>
> I'm trying to figure out how is the meta data optional, and it seems to be
> in here, right? but hdr->meta_header.meta_off or hdr->meta_header.meta_len
> must be set NULL or zero
>
> I'm getting crash when running btf test and it seems like correption when
> parsing BTF generated from clang, which does not have this meta support
>
> we do need clang support for this right? or bump version?
libbpf doesn't enforce that btf_header is all zeroes after last known
field (which until now was str_len). So the whole design of adding
fields to btf_header is backwards compatible from libbpf standpoint.
>From the implementation standpoint, though, we should make our lives
simpler by not using btf->raw_data for btf_header, and instead do what
we do in kernel with bpf_attr. That is, have a local zero-initialized
copy, and overwrite it with data found in btf.
>
> thanks,
> jirka
>
>
> > + if (hdr->meta_header.meta_len < sizeof(struct btf_metadata)) {
> > + pr_debug("Invalid BTF metadata section\n");
> > + return -EINVAL;
> > + }
> > + btf->meta_data = btf->raw_data + btf->hdr->hdr_len + btf->hdr->meta_header.meta_off;
> > +
> > + if (btf->swapped_endian)
> > + btf_bswap_meta(btf->meta_data, hdr->meta_header.meta_len);
> > +
> > + return 0;
> > +}
> > +
> > static int btf_type_size(const struct btf_type *t)
> > {
> > const int base_size = sizeof(struct btf_type);
> > @@ -904,6 +955,7 @@ static struct btf *btf_new(const void *data, __u32 size, struct btf *base_btf)
> > err = err ?: btf_parse_type_sec(btf);
> > if (err)
> > goto done;
> > + err = btf_parse_meta_sec(btf);
> >
> > done:
> > if (err) {
> > @@ -1267,6 +1319,11 @@ static void *btf_get_raw_data(const struct btf *btf, __u32 *size, bool swap_endi
> > }
> >
> > data_sz = hdr->hdr_len + hdr->type_len + hdr->str_len;
> > + if (btf->meta_data) {
> > + data_sz = roundup(data_sz, 8);
> > + data_sz += hdr->meta_header.meta_len;
> > + hdr->meta_header.meta_off = roundup(hdr->type_len + hdr->str_len, 8);
> > + }
> > data = calloc(1, data_sz);
> > if (!data)
> > return NULL;
> > @@ -1293,8 +1350,21 @@ static void *btf_get_raw_data(const struct btf *btf, __u32 *size, bool swap_endi
> > p += hdr->type_len;
> >
> > memcpy(p, btf_strs_data(btf), hdr->str_len);
> > - p += hdr->str_len;
> > + /* round up to 8 byte alignment to match offset above */
> > + p = data + hdr->hdr_len + roundup(hdr->type_len + hdr->str_len, 8);
> > +
> > + if (btf->meta_data) {
> > + struct btf_metadata *meta = p;
> >
> > + memcpy(p, btf->meta_data, hdr->meta_header.meta_len);
> > + if (!swap_endian) {
> > + meta->crc = crc32(0L, (const Bytef *)&data, sizeof(data));
> > + meta->flags |= BTF_META_CRC_SET;
> > + }
> > + if (swap_endian)
> > + btf_bswap_meta(p, hdr->meta_header.meta_len);
> > + p += hdr->meta_header.meta_len;
> > + }
> > *size = data_sz;
> > return data;
> > err_out:
> > @@ -1425,13 +1495,13 @@ static void btf_invalidate_raw_data(struct btf *btf)
> > }
> > }
> >
> > -/* Ensure BTF is ready to be modified (by splitting into a three memory
> > - * regions for header, types, and strings). Also invalidate cached
> > - * raw_data, if any.
> > +/* Ensure BTF is ready to be modified (by splitting into a three or four memory
> > + * regions for header, types, strings and optional metadata). Also invalidate
> > + * cached raw_data, if any.
> > */
> > static int btf_ensure_modifiable(struct btf *btf)
> > {
> > - void *hdr, *types;
> > + void *hdr, *types, *meta = NULL;
> > struct strset *set = NULL;
> > int err = -ENOMEM;
> >
> > @@ -1446,9 +1516,17 @@ static int btf_ensure_modifiable(struct btf *btf)
> > types = malloc(btf->hdr->type_len);
> > if (!hdr || !types)
> > goto err_out;
> > + if (btf->hdr->hdr_len >= sizeof(struct btf_header) &&
> > + btf->hdr->meta_header.meta_off && btf->hdr->meta_header.meta_len) {
> > + meta = calloc(1, btf->hdr->meta_header.meta_len);
> > + if (!meta)
> > + goto err_out;
> > + }
> >
> > memcpy(hdr, btf->hdr, btf->hdr->hdr_len);
> > memcpy(types, btf->types_data, btf->hdr->type_len);
> > + if (meta)
> > + memcpy(meta, btf->meta_data, btf->hdr->meta_header.meta_len);
> >
> > /* build lookup index for all strings */
> > set = strset__new(BTF_MAX_STR_OFFSET, btf->strs_data, btf->hdr->str_len);
> > @@ -1463,6 +1541,8 @@ static int btf_ensure_modifiable(struct btf *btf)
> > btf->types_data_cap = btf->hdr->type_len;
> > btf->strs_data = NULL;
> > btf->strs_set = set;
> > + btf->meta_data = meta;
> > +
> > /* if BTF was created from scratch, all strings are guaranteed to be
> > * unique and deduplicated
> > */
> > @@ -1480,6 +1560,7 @@ static int btf_ensure_modifiable(struct btf *btf)
> > strset__free(set);
> > free(hdr);
> > free(types);
> > + free(meta);
> > return err;
> > }
> >
> > --
> > 2.31.1
> >
