[PATCH v2 bpf-next 0/9] bpf: support BTF kind layout info, CRCs

[PATCH v2 bpf-next 0/9] bpf: support BTF kind layout info, CRCs

[Alan Maguire]

By separating parsing BTF from using all the information
it provides, we allow BTF to encode new features even if
they cannot be used.  This is helpful in particular for
cases where newer tools for BTF generation run on an
older kernel; BTF kinds may be present that the kernel
cannot yet use, but at least it can parse the BTF
provided.  Meanwhile userspace tools with newer libbpf
may be able to use the newer information.

The intent is to support encoding of kind layouts
optionally so that tools like pahole can add this
information.  So for each kind we record

- kind-related flags
- length of singular element following struct btf_type
- length of each of the btf_vlen() elements following

In addition we make space in the BTF header for
CRC32s computed over the BTF along with a CRC for
the base BTF; this allows split BTF to identify
a mismatch explicitly.

The ideas here were discussed at [1], with further
discussion at [2].

Future work can take more advantage of these features
such as

- using base CRC to identify base/module BTF mismatch
  explicitly
- using absence of a base BTF CRC as evidence that
  BTF is standalone

...and new BTF kind addition should present less
trouble, provided the kinds are optional.  BTF
parsing _will_ still fail if a non-optional
kind is encountered, as the assumption is that
such kinds are needed.  To take a few examples,
the tag kinds are optional, however enum64
is required, so BTF containing an enum64
(that did not know about enum64) would be
rejected.  This makes sense as if for example
a struct contained an enum64 we would not
be able to fully represent that struct unless
we knew about enum64s.

Patch 1 is the UAPI changes, patches 2-3 provide
libbpf support for handling and using kind layouts.
Patch 4 adds kernel support for handling and using
kind layouts.  Patch 5 adds libbpf support to add
kind layouts and CRCs based on options passed
to btf__new_empty_opts().   Patch 6 updates
pahole flags for kernel/module BTF generation
to generate kind layout/CRCs if that support
is available.  Patch 7 adds bpftool support
to dump BTF metadata (header + kind layout).
Patch 8 documents this.  Patch 9 is a set
of selftests covering encoding/decoding of
BTF kind layout information, and patch 10
is a dwarves patch to add kind layout encoding
and CRC support via btf__new_empty_opts().

Changes since RFC

- Terminology change from meta -> kind_layout
  (Alexei and Andrii)
- Simplify representation, removing meta header
  and just having kind layout section (Alexei)
- Fixed bpftool to have JSON support, support
  prefix match, documented changes (Quentin)
- Separated metadata opts into add_kind_layout
  and add_crc
- Added additional positive/negative tests
  to cover basic unknown kind, one with an
  info_sz object following it and one with
  N elem_sz elements following it.
- Updated pahole-flags to use help output
  rather than version to see if features
  are present

[1] https://lore.kernel.org/bpf/CAEf4BzYjWHRdNNw4B=eOXOs_ONrDwrgX4bn=Nuc1g8JPFC34MA@mail.gmail.com/
[2] https://lore.kernel.org/bpf/20230531201936.1992188-1-alan.maguire@oracle.com/

Alan Maguire (9):
  btf: add kind layout encoding, crcs to UAPI
  libbpf: support handling of kind layout section in BTF
  libbpf: use kind layout to compute an unknown kind size
  btf: support kernel parsing of BTF with kind layout
  libbpf: add kind layout encoding, crc support
  btf: generate BTF kind layout for vmlinux/module BTF
  bpftool: add BTF dump "format meta" to dump header/metadata
  bpftool: update doc to describe bpftool btf dump .. format meta
  selftests/bpf: test kind encoding/decoding

 include/uapi/linux/btf.h                      |  24 ++
 kernel/bpf/btf.c                              | 102 +++++--
 scripts/pahole-flags.sh                       |   7 +
 .../bpf/bpftool/Documentation/bpftool-btf.rst |  16 +-
 tools/bpf/bpftool/bash-completion/bpftool     |   2 +-
 tools/bpf/bpftool/btf.c                       |  93 +++++-
 tools/include/uapi/linux/btf.h                |  24 ++
 tools/lib/bpf/btf.c                           | 272 +++++++++++++++---
 tools/lib/bpf/btf.h                           |  11 +
 tools/lib/bpf/libbpf.map                      |   1 +
 .../selftests/bpf/prog_tests/btf_kind.c       | 187 ++++++++++++
 11 files changed, 670 insertions(+), 69 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/btf_kind.c

-- 
2.39.3

[PATCH v2 bpf-next 1/9] btf: add kind layout encoding, crcs to UAPI

[Alan Maguire]

BTF kind layouts provide information to parse BTF kinds.
By separating parsing BTF from using all the information
it provides, we allow BTF to encode new features even if
they cannot be used.  This is helpful in particular for
cases where newer tools for BTF generation run on an
older kernel; BTF kinds may be present that the kernel
cannot yet use, but at least it can parse the BTF
provided.  Meanwhile userspace tools with newer libbpf
may be able to use the newer information.

The intent is to support encoding of kind layouts
optionally so that tools like pahole can add this
information.  So for each kind we record

- kind-related flags
- length of singular element following struct btf_type
- length of each of the btf_vlen() elements following

In addition we make space in the BTF header for
CRC32s computed over the BTF along with a CRC for
the base BTF; this allows split BTF to identify
a mismatch explicitly.

The ideas here were discussed at [1], [2]; hence

Suggested-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>

[1] https://lore.kernel.org/bpf/CAEf4BzYjWHRdNNw4B=eOXOs_ONrDwrgX4bn=Nuc1g8JPFC34MA@mail.gmail.com/
[2] https://lore.kernel.org/bpf/20230531201936.1992188-1-alan.maguire@oracle.com/
---
 include/uapi/linux/btf.h       | 24 ++++++++++++++++++++++++
 tools/include/uapi/linux/btf.h | 24 ++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
index ec1798b6d3ff..cea9125ed953 100644
--- a/include/uapi/linux/btf.h
+++ b/include/uapi/linux/btf.h
@@ -8,6 +8,22 @@
 #define BTF_MAGIC	0xeB9F
 #define BTF_VERSION	1
 
+/* is this information required? If so it cannot be sanitized safely. */
+#define BTF_KIND_LAYOUT_OPTIONAL		(1 << 0)
+
+/* kind layout section consists of a struct btf_kind_layout for each known
+ * kind at BTF encoding time.
+ */
+struct btf_kind_layout {
+	__u16 flags;		/* see BTF_KIND_LAYOUT_* values above */
+	__u8 info_sz;		/* size of singular element after btf_type */
+	__u8 elem_sz;		/* size of each of btf_vlen(t) elements */
+};
+
+/* for CRCs for BTF, base BTF to be considered usable, flags must be set. */
+#define BTF_FLAG_CRC_SET		(1 << 0)
+#define BTF_FLAG_BASE_CRC_SET		(1 << 1)
+
 struct btf_header {
 	__u16	magic;
 	__u8	version;
@@ -19,8 +35,16 @@ struct btf_header {
 	__u32	type_len;	/* length of type section	*/
 	__u32	str_off;	/* offset of string section	*/
 	__u32	str_len;	/* length of string section	*/
+	__u32	kind_layout_off;/* offset of kind layout section */
+	__u32	kind_layout_len;/* length of kind layout section */
+
+	__u32	crc;		/* crc of BTF; used if flags set BTF_FLAG_CRC_VALID */
+	__u32	base_crc;	/* crc of base BTF; used if flags set BTF_FLAG_BASE_CRC_VALID */
 };
 
+/* required minimum BTF header length */
+#define BTF_HEADER_MIN_LEN	(sizeof(struct btf_header) - 16)
+
 /* Max # of type identifier */
 #define BTF_MAX_TYPE	0x000fffff
 /* Max offset into the string section */
diff --git a/tools/include/uapi/linux/btf.h b/tools/include/uapi/linux/btf.h
index ec1798b6d3ff..cea9125ed953 100644
--- a/tools/include/uapi/linux/btf.h
+++ b/tools/include/uapi/linux/btf.h
@@ -8,6 +8,22 @@
 #define BTF_MAGIC	0xeB9F
 #define BTF_VERSION	1
 
+/* is this information required? If so it cannot be sanitized safely. */
+#define BTF_KIND_LAYOUT_OPTIONAL		(1 << 0)
+
+/* kind layout section consists of a struct btf_kind_layout for each known
+ * kind at BTF encoding time.
+ */
+struct btf_kind_layout {
+	__u16 flags;		/* see BTF_KIND_LAYOUT_* values above */
+	__u8 info_sz;		/* size of singular element after btf_type */
+	__u8 elem_sz;		/* size of each of btf_vlen(t) elements */
+};
+
+/* for CRCs for BTF, base BTF to be considered usable, flags must be set. */
+#define BTF_FLAG_CRC_SET		(1 << 0)
+#define BTF_FLAG_BASE_CRC_SET		(1 << 1)
+
 struct btf_header {
 	__u16	magic;
 	__u8	version;
@@ -19,8 +35,16 @@ struct btf_header {
 	__u32	type_len;	/* length of type section	*/
 	__u32	str_off;	/* offset of string section	*/
 	__u32	str_len;	/* length of string section	*/
+	__u32	kind_layout_off;/* offset of kind layout section */
+	__u32	kind_layout_len;/* length of kind layout section */
+
+	__u32	crc;		/* crc of BTF; used if flags set BTF_FLAG_CRC_VALID */
+	__u32	base_crc;	/* crc of base BTF; used if flags set BTF_FLAG_BASE_CRC_VALID */
 };
 
+/* required minimum BTF header length */
+#define BTF_HEADER_MIN_LEN	(sizeof(struct btf_header) - 16)
+
 /* Max # of type identifier */
 #define BTF_MAX_TYPE	0x000fffff
 /* Max offset into the string section */
-- 
2.39.3

[PATCH v2 bpf-next 2/9] libbpf: support handling of kind layout section in BTF

[Alan Maguire]

support reading in kind layout fixing endian issues on reading;
also support writing kind layout section to raw BTF object.
There is not yet an API to populate the kind layout with meaningful
information.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 tools/lib/bpf/btf.c | 132 +++++++++++++++++++++++++++++++++-----------
 1 file changed, 99 insertions(+), 33 deletions(-)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 8484b563b53d..f9f919fdc728 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -39,40 +39,44 @@ 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 kind layout point inside that memory region to their
+	 * respective parts of BTF representation:
 	 *
-	 * +--------------------------------+
-	 * |  Header  |  Types  |  Strings  |
-	 * +--------------------------------+
-	 * ^          ^         ^
-	 * |          |         |
-	 * hdr        |         |
-	 * types_data-+         |
-	 * strs_data------------+
+	 * +--------------------------------+-------------+
+	 * |  Header  |  Types  |  Strings  | Kind Layout |
+	 * +--------------------------------+-------------+
+	 * ^          ^         ^           ^
+	 * |          |         |           |
+	 * hdr        |         |           |
+	 * types_data-+         |           |
+	 * strs_data------------+           |
+	 * kind_layout----------------------+
+	 *
+	 * kind_layout 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) kind layout:
 	 *
-	 * +----------+  +---------+  +-----------+
-	 * |  Header  |  |  Types  |  |  Strings  |
-	 * +----------+  +---------+  +-----------+
-	 * ^             ^            ^
-	 * |             |            |
-	 * hdr           |            |
-	 * types_data----+            |
-	 * strset__data(strs_set)-----+
+	 * +----------+  +---------+  +-----------+  +-------------+
+	 * |  Header  |  |  Types  |  |  Strings  |  | Kind layout |
+	 * +----------+  +---------+  +-----------+  +-------------+
+	 * ^             ^            ^              ^
+	 * |             |            |              |
+	 * hdr           |            |              |
+	 * types_data----+            |              |
+	 * strset__data(strs_set)-----+              |
+	 * kind_layout-------------------------------+
 	 *
-	 *               +----------+---------+-----------+
-	 *               |  Header  |  Types  |  Strings  |
-	 * raw_data----->+----------+---------+-----------+
+	 *               +----------+---------+-----------+-------------+
+	 *               |  Header  |  Types  |  Strings  | Kind Layout |
+	 * raw_data----->+----------+---------+-----------+-------------+
 	 */
 	struct btf_header *hdr;
 
@@ -116,6 +120,8 @@ struct btf {
 	/* whether strings are already deduplicated */
 	bool strs_deduped;
 
+	struct btf_kind_layout *kind_layout;
+
 	/* BTF object FD, if loaded into kernel */
 	int fd;
 
@@ -215,6 +221,13 @@ 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->kind_layout_off = bswap_32(h->kind_layout_off);
+		h->kind_layout_len = bswap_32(h->kind_layout_len);
+		h->crc = bswap_32(h->crc);
+		h->base_crc = bswap_32(h->base_crc);
+	}
+
 }
 
 static int btf_parse_hdr(struct btf *btf)
@@ -222,14 +235,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 < BTF_HEADER_MIN_LEN) {
 		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 != BTF_HEADER_MIN_LEN) {
 			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 +301,32 @@ static int btf_parse_str_sec(struct btf *btf)
 	return 0;
 }
 
+static void btf_bswap_kind_layout_sec(struct btf_kind_layout *k, int len)
+{
+	struct btf_kind_layout *end = (void *)k + len;
+
+	while (k < end) {
+		k->flags = bswap_16(k->flags);
+		k++;
+	}
+}
+
+static int btf_parse_kind_layout_sec(struct btf *btf)
+{
+	const struct btf_header *hdr = btf->hdr;
+
+	if (hdr->hdr_len < sizeof(struct btf_header) ||
+	    !hdr->kind_layout_off || !hdr->kind_layout_len)
+		return 0;
+	if (hdr->kind_layout_len < sizeof(struct btf_kind_layout)) {
+		pr_debug("Invalid BTF kind layout section\n");
+		return -EINVAL;
+	}
+	btf->kind_layout = btf->raw_data + btf->hdr->hdr_len + btf->hdr->kind_layout_off;
+
+	return 0;
+}
+
 static int btf_type_size(const struct btf_type *t)
 {
 	const int base_size = sizeof(struct btf_type);
@@ -901,6 +943,7 @@ static struct btf *btf_new(const void *data, __u32 size, struct btf *base_btf)
 	btf->types_data = btf->raw_data + btf->hdr->hdr_len + btf->hdr->type_off;
 
 	err = btf_parse_str_sec(btf);
+	err = err ?: btf_parse_kind_layout_sec(btf);
 	err = err ?: btf_parse_type_sec(btf);
 	if (err)
 		goto done;
@@ -1267,6 +1310,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->kind_layout) {
+		data_sz = roundup(data_sz, 4);
+		data_sz += hdr->kind_layout_len;
+		hdr->kind_layout_off = roundup(hdr->type_len + hdr->str_len, 4);
+	}
 	data = calloc(1, data_sz);
 	if (!data)
 		return NULL;
@@ -1293,8 +1341,15 @@ 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 4 byte alignment to match offset above */
+	p = data + hdr->hdr_len + roundup(hdr->type_len + hdr->str_len, 4);
 
+	if (btf->kind_layout) {
+		memcpy(p, btf->kind_layout, hdr->kind_layout_len);
+		if (swap_endian)
+			btf_bswap_kind_layout_sec(p, hdr->kind_layout_len);
+		p += hdr->kind_layout_len;
+	}
 	*size = data_sz;
 	return data;
 err_out:
@@ -1425,13 +1480,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 kind layout). Also invalidate
+ * cached raw_data, if any.
  */
 static int btf_ensure_modifiable(struct btf *btf)
 {
-	void *hdr, *types;
+	void *hdr, *types, *kind_layout = NULL;
 	struct strset *set = NULL;
 	int err = -ENOMEM;
 
@@ -1446,9 +1501,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->kind_layout_off && btf->hdr->kind_layout_len) {
+		kind_layout = calloc(1, btf->hdr->kind_layout_len);
+		if (!kind_layout)
+			goto err_out;
+	}
 
 	memcpy(hdr, btf->hdr, btf->hdr->hdr_len);
 	memcpy(types, btf->types_data, btf->hdr->type_len);
+	if (kind_layout)
+		memcpy(kind_layout, btf->kind_layout, btf->hdr->kind_layout_len);
 
 	/* build lookup index for all strings */
 	set = strset__new(BTF_MAX_STR_OFFSET, btf->strs_data, btf->hdr->str_len);
@@ -1463,6 +1526,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->kind_layout = kind_layout;
+
 	/* if BTF was created from scratch, all strings are guaranteed to be
 	 * unique and deduplicated
 	 */
@@ -1480,6 +1545,7 @@ static int btf_ensure_modifiable(struct btf *btf)
 	strset__free(set);
 	free(hdr);
 	free(types);
+	free(kind_layout);
 	return err;
 }
 
-- 
2.39.3

[PATCH v2 bpf-next 3/9] libbpf: use kind layout to compute an unknown kind size

[Alan Maguire]

This allows BTF parsing to proceed even if we do not know the
kind.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 tools/lib/bpf/btf.c | 41 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index f9f919fdc728..457997c2a43c 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -327,7 +327,35 @@ static int btf_parse_kind_layout_sec(struct btf *btf)
 	return 0;
 }
 
-static int btf_type_size(const struct btf_type *t)
+/* for unknown kinds, consult kind layout. */
+static int btf_type_size_unknown(const struct btf *btf, const struct btf_type *t)
+{
+	int size = sizeof(struct btf_type);
+	struct btf_kind_layout *k = NULL;
+	__u16 vlen = btf_vlen(t);
+	__u8 kind = btf_kind(t);
+
+	if (btf->kind_layout)
+		k = &btf->kind_layout[kind];
+
+	if (!k || (void *)k > ((void *)btf->kind_layout + btf->hdr->kind_layout_len)) {
+		pr_debug("Unsupported BTF_KIND: %u\n", btf_kind(t));
+		return -EINVAL;
+	}
+
+	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;
+	}
+
+	size += k->info_sz;
+	size += vlen * k->elem_sz;
+
+	return size;
+}
+
+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);
 	}
 }
 
@@ -463,7 +490,7 @@ static int btf_parse_type_sec(struct btf *btf)
 		if (btf->swapped_endian)
 			btf_bswap_type_base(next_type);
 
-		type_size = btf_type_size(next_type);
+		type_size = btf_type_size(btf, next_type);
 		if (type_size < 0)
 			return type_size;
 		if (next_type + type_size > end_type) {
@@ -1672,7 +1699,7 @@ int btf__add_type(struct btf *btf, const struct btf *src_btf, const struct btf_t
 	struct btf_type *t;
 	int sz, err;
 
-	sz = btf_type_size(src_type);
+	sz = btf_type_size(src_btf, src_type);
 	if (sz < 0)
 		return libbpf_err(sz);
 
@@ -1753,7 +1780,7 @@ int btf__add_btf(struct btf *btf, const struct btf *src_btf)
 	memcpy(t, src_btf->types_data, data_sz);
 
 	for (i = 0; i < cnt; i++) {
-		sz = btf_type_size(t);
+		sz = btf_type_size(src_btf, t);
 		if (sz < 0) {
 			/* unlikely, has to be corrupted src_btf */
 			err = sz;
@@ -4749,7 +4776,7 @@ static int btf_dedup_compact_types(struct btf_dedup *d)
 			continue;
 
 		t = btf__type_by_id(d->btf, id);
-		len = btf_type_size(t);
+		len = btf_type_size(d->btf, t);
 		if (len < 0)
 			return len;
 
-- 
2.39.3

[PATCH v2 bpf-next 4/9] btf: support kernel parsing of BTF with kind layout

[Alan Maguire]

Use kind layout to parse BTF with unknown kinds that have a
kind layout representation.

Validate kind layout if present, and use it to parse BTF with
unrecognized kinds. Reject BTF that contains a type
of a kind that is not optional.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 kernel/bpf/btf.c | 102 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 82 insertions(+), 20 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index bd2cac057928..ffe3926ea051 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -257,6 +257,7 @@ struct btf {
 	struct btf_kfunc_set_tab *kfunc_set_tab;
 	struct btf_id_dtor_kfunc_tab *dtor_kfunc_tab;
 	struct btf_struct_metas *struct_meta_tab;
+	struct btf_kind_layout *kind_layout;
 
 	/* split BTF support */
 	struct btf *base_btf;
@@ -4965,22 +4966,41 @@ static s32 btf_check_meta(struct btf_verifier_env *env,
 		return -EINVAL;
 	}
 
-	if (BTF_INFO_KIND(t->info) > BTF_KIND_MAX ||
-	    BTF_INFO_KIND(t->info) == BTF_KIND_UNKN) {
-		btf_verifier_log(env, "[%u] Invalid kind:%u",
-				 env->log_type_id, BTF_INFO_KIND(t->info));
-		return -EINVAL;
-	}
-
 	if (!btf_name_offset_valid(env->btf, t->name_off)) {
 		btf_verifier_log(env, "[%u] Invalid name_offset:%u",
 				 env->log_type_id, t->name_off);
 		return -EINVAL;
 	}
 
-	var_meta_size = btf_type_ops(t)->check_meta(env, t, meta_left);
-	if (var_meta_size < 0)
-		return var_meta_size;
+	if (BTF_INFO_KIND(t->info) == BTF_KIND_UNKN) {
+		btf_verifier_log(env, "[%u] Invalid kind:%u",
+				 env->log_type_id, BTF_INFO_KIND(t->info));
+		return -EINVAL;
+	}
+
+	if (BTF_INFO_KIND(t->info) > BTF_KIND_MAX && env->btf->kind_layout &&
+	    (BTF_INFO_KIND(t->info) * sizeof(struct btf_kind_layout)) <
+	     env->btf->hdr.kind_layout_len) {
+		struct btf_kind_layout *k = &env->btf->kind_layout[BTF_INFO_KIND(t->info)];
+
+		if (!(k->flags & BTF_KIND_LAYOUT_OPTIONAL)) {
+			btf_verifier_log(env, "[%u] unknown but required kind %u",
+					 env->log_type_id,
+					 BTF_INFO_KIND(t->info));
+			return -EINVAL;
+		}
+		var_meta_size = sizeof(struct btf_type);
+		var_meta_size += k->info_sz + (btf_type_vlen(t) * k->elem_sz);
+	} else {
+		if (BTF_INFO_KIND(t->info) > BTF_KIND_MAX) {
+			btf_verifier_log(env, "[%u] Invalid kind:%u",
+					 env->log_type_id, BTF_INFO_KIND(t->info));
+			return -EINVAL;
+		}
+		var_meta_size = btf_type_ops(t)->check_meta(env, t, meta_left);
+		if (var_meta_size < 0)
+			return var_meta_size;
+	}
 
 	meta_left -= var_meta_size;
 
@@ -5155,7 +5175,8 @@ static int btf_parse_str_sec(struct btf_verifier_env *env)
 	start = btf->nohdr_data + hdr->str_off;
 	end = start + hdr->str_len;
 
-	if (end != btf->data + btf->data_size) {
+	if (hdr->hdr_len < sizeof(struct btf_header) &&
+	    end != btf->data + btf->data_size) {
 		btf_verifier_log(env, "String section is not at the end");
 		return -EINVAL;
 	}
@@ -5176,9 +5197,41 @@ static int btf_parse_str_sec(struct btf_verifier_env *env)
 	return 0;
 }
 
+static int btf_parse_kind_layout_sec(struct btf_verifier_env *env)
+{
+	const struct btf_header *hdr = &env->btf->hdr;
+	struct btf *btf = env->btf;
+	void *start, *end;
+
+	if (hdr->hdr_len < sizeof(struct btf_header) ||
+	    hdr->kind_layout_len == 0)
+		return 0;
+
+	/* Kind layout section must align to 4 bytes */
+	if (hdr->kind_layout_off & (sizeof(u32) - 1)) {
+		btf_verifier_log(env, "Unaligned kind_layout_off");
+		return -EINVAL;
+	}
+	start = btf->nohdr_data + hdr->kind_layout_off;
+	end = start + hdr->kind_layout_len;
+
+	if (hdr->kind_layout_len < sizeof(struct btf_kind_layout)) {
+		btf_verifier_log(env, "Kind layout section is too small");
+		return -EINVAL;
+	}
+	if (end != btf->data + btf->data_size) {
+		btf_verifier_log(env, "Kind layout section is not at the end");
+		return -EINVAL;
+	}
+	btf->kind_layout = start;
+
+	return 0;
+}
+
 static const size_t btf_sec_info_offset[] = {
 	offsetof(struct btf_header, type_off),
 	offsetof(struct btf_header, str_off),
+	offsetof(struct btf_header, kind_layout_off),
 };
 
 static int btf_sec_info_cmp(const void *a, const void *b)
@@ -5193,32 +5246,37 @@ static int btf_check_sec_info(struct btf_verifier_env *env,
 			      u32 btf_data_size)
 {
 	struct btf_sec_info secs[ARRAY_SIZE(btf_sec_info_offset)];
-	u32 total, expected_total, i;
+	u32 nr_secs = ARRAY_SIZE(btf_sec_info_offset);
+	u32 total, expected_total, gap, i;
 	const struct btf_header *hdr;
 	const struct btf *btf;
 
 	btf = env->btf;
 	hdr = &btf->hdr;
 
+	if (hdr->hdr_len < sizeof(struct btf_header))
+		nr_secs--;
+
 	/* Populate the secs from hdr */
-	for (i = 0; i < ARRAY_SIZE(btf_sec_info_offset); i++)
+	for (i = 0; i < nr_secs; i++)
 		secs[i] = *(struct btf_sec_info *)((void *)hdr +
 						   btf_sec_info_offset[i]);
 
-	sort(secs, ARRAY_SIZE(btf_sec_info_offset),
+	sort(secs, nr_secs,
 	     sizeof(struct btf_sec_info), btf_sec_info_cmp, NULL);
 
 	/* Check for gaps and overlap among sections */
 	total = 0;
 	expected_total = btf_data_size - hdr->hdr_len;
-	for (i = 0; i < ARRAY_SIZE(btf_sec_info_offset); i++) {
+	for (i = 0; i < nr_secs; i++) {
 		if (expected_total < secs[i].off) {
 			btf_verifier_log(env, "Invalid section offset");
 			return -EINVAL;
 		}
-		if (total < secs[i].off) {
-			/* gap */
-			btf_verifier_log(env, "Unsupported section found");
+		gap = secs[i].off - total;
+		if (gap >= 4) {
+			/* gap larger than alignment gap */
+			btf_verifier_log(env, "Unsupported section gap found");
 			return -EINVAL;
 		}
 		if (total > secs[i].off) {
@@ -5230,7 +5288,7 @@ static int btf_check_sec_info(struct btf_verifier_env *env,
 					 "Total section length too long");
 			return -EINVAL;
 		}
-		total += secs[i].len;
+		total += secs[i].len + gap;
 	}
 
 	/* There is data other than hdr and known sections */
@@ -5293,7 +5351,7 @@ static int btf_parse_hdr(struct btf_verifier_env *env)
 		return -ENOTSUPP;
 	}
 
-	if (hdr->flags) {
+	if (hdr->flags & ~(BTF_FLAG_CRC_SET | BTF_FLAG_BASE_CRC_SET)) {
 		btf_verifier_log(env, "Unsupported flags");
 		return -ENOTSUPP;
 	}
@@ -5530,6 +5588,10 @@ static struct btf *btf_parse(const union bpf_attr *attr, bpfptr_t uattr, u32 uat
 	if (err)
 		goto errout;
 
+	err = btf_parse_kind_layout_sec(env);
+	if (err)
+		goto errout;
+
 	err = btf_parse_type_sec(env);
 	if (err)
 		goto errout;
-- 
2.39.3

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

[Alan Maguire]

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@oracle.com>
---
 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_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);
+
+		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));
 }
 
 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;
+
+	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));
+	}
+
 	*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 */
+	bool add_crc;		/* add CRC information */
+	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

[PATCH v2 bpf-next 6/9] btf: generate BTF kind layout for vmlinux/module BTF

[Alan Maguire]

Generate BTF kind layout information, crcs for kernel and module BTF
if support is available in pahole.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 scripts/pahole-flags.sh | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/scripts/pahole-flags.sh b/scripts/pahole-flags.sh
index 728d55190d97..cb304e0a4434 100755
--- a/scripts/pahole-flags.sh
+++ b/scripts/pahole-flags.sh
@@ -25,6 +25,13 @@ if [ "${pahole_ver}" -ge "124" ]; then
 fi
 if [ "${pahole_ver}" -ge "125" ]; then
 	extra_paholeopt="${extra_paholeopt} --skip_encoding_btf_inconsistent_proto --btf_gen_optimized"
+	pahole_help="$(${PAHOLE} --help)"
+	if [[ "$pahole_help" =~ "btf_gen_kind_layout" ]]; then
+		extra_paholeopt="${extra_paholeopt} --btf_gen_kind_layout"
+	fi
+	if [[ "$pahole_help" =~ "btf_gen_crc" ]]; then
+		extra_paholeopt="${extra_paholeopt} --btf_gen_crc"
+	fi
 fi
 
 echo ${extra_paholeopt}
-- 
2.39.3

[PATCH v2 bpf-next 7/9] bpftool: add BTF dump "format meta" to dump header/metadata

[Alan Maguire]

Provide a way to dump BTF metadata info via bpftool; this
consists of BTF size, header fields and kind layout info
(if available); for example

$ bpftool btf dump file vmlinux format meta
size 4966513   
magic 0xeb9f      
version 1         
flags 0x0         
hdr_len 24        
type_len 2929900   
type_off 0         
str_len 2036589   
str_off 2929900   

...or for vmlinux with kind layout, crc:

$ bpftool btf dump file vmlinux format meta
size 5034496   
magic 0xeb9f      
version 1         
flags 0x1         
hdr_len 40        
type_len 2973628   
type_off 0         
str_len 2060745   
str_off 2973628   
kind_layout_len 80        
kind_layout_off 5034376   
crc 0xb6a5171f  
base_crc 0x0         
kind 0    flags 0x0    info_sz 0    elem_sz 0   
kind 1    flags 0x0    info_sz 4    elem_sz 0   
kind 2    flags 0x0    info_sz 0    elem_sz 0   
kind 3    flags 0x0    info_sz 12   elem_sz 0   
kind 4    flags 0x0    info_sz 0    elem_sz 12
...

JSON output is also supported:

$ bpftool -j btf dump file vmlinux format meta
{"size":4904369,{"header":"magic":60319,"version":1,"flags":0,"hdr_len":24,"type_len":2893508,"type_off":0,"str_len":2010837,"str_off":2893508}}

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 tools/bpf/bpftool/bash-completion/bpftool |  2 +-
 tools/bpf/bpftool/btf.c                   | 93 ++++++++++++++++++++++-
 2 files changed, 92 insertions(+), 3 deletions(-)

diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 085bf18f3659..4c186d4efb35 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -937,7 +937,7 @@ _bpftool()
                             return 0
                             ;;
                         format)
-                            COMPREPLY=( $( compgen -W "c raw" -- "$cur" ) )
+                            COMPREPLY=( $( compgen -W "c raw meta" -- "$cur" ) )
                             ;;
                         *)
                             # emit extra options
diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
index 91fcb75babe3..56f40adcc161 100644
--- a/tools/bpf/bpftool/btf.c
+++ b/tools/bpf/bpftool/btf.c
@@ -504,6 +504,90 @@ static int dump_btf_c(const struct btf *btf,
 	return err;
 }
 
+static int dump_btf_meta(const struct btf *btf)
+{
+	const struct btf_header *hdr;
+	const struct btf_kind_layout *k;
+	const void *data;
+	__u32 data_sz;
+	__u8 i, nr_kinds;
+
+	data = btf__raw_data(btf, &data_sz);
+	if (!data)
+		return -ENOMEM;
+	hdr = data;
+	if (json_output) {
+		jsonw_start_object(json_wtr);   /* btf metadata object */
+		jsonw_uint_field(json_wtr, "size", data_sz);
+		jsonw_start_object(json_wtr);
+		jsonw_name(json_wtr, "header");
+		jsonw_uint_field(json_wtr, "magic", hdr->magic);
+		jsonw_uint_field(json_wtr, "version", hdr->version);
+		jsonw_uint_field(json_wtr, "flags", hdr->flags);
+		jsonw_uint_field(json_wtr, "hdr_len", hdr->hdr_len);
+		jsonw_uint_field(json_wtr, "type_len", hdr->type_len);
+		jsonw_uint_field(json_wtr, "type_off", hdr->type_off);
+		jsonw_uint_field(json_wtr, "str_len", hdr->str_len);
+		jsonw_uint_field(json_wtr, "str_off", hdr->str_off);
+	} else {
+		printf("size %-10d\n", data_sz);
+		printf("magic 0x%-10x\nversion %-10d\nflags 0x%-10x\nhdr_len %-10d\n",
+		       hdr->magic, hdr->version, hdr->flags, hdr->hdr_len);
+		printf("type_len %-10d\ntype_off %-10d\n", hdr->type_len, hdr->type_off);
+		printf("str_len %-10d\nstr_off %-10d\n", hdr->str_len, hdr->str_off);
+	}
+
+	if (hdr->hdr_len < sizeof(struct btf_header) ||
+	    hdr->kind_layout_len == 0 || hdr->kind_layout_len == 0) {
+		if (json_output) {
+			jsonw_end_object(json_wtr); /* header object */
+			jsonw_end_object(json_wtr); /* metadata object */
+		}
+		return 0;
+	}
+
+	if (json_output) {
+		jsonw_uint_field(json_wtr, "kind_layout_len", hdr->kind_layout_len);
+		jsonw_uint_field(json_wtr, "kind_layout_offset", hdr->kind_layout_off);
+		jsonw_uint_field(json_wtr, "crc", hdr->crc);
+		jsonw_uint_field(json_wtr, "base_crc", hdr->base_crc);
+		jsonw_end_object(json_wtr); /* end header object */
+
+		jsonw_start_object(json_wtr);
+		jsonw_name(json_wtr, "kind_layouts");
+		jsonw_start_array(json_wtr);
+	} else {
+		printf("kind_layout_len %-10d\nkind_layout_off %-10d\n",
+		       hdr->kind_layout_len, hdr->kind_layout_off);
+		printf("crc 0x%-10x\nbase_crc 0x%-10x\n",
+		       hdr->crc, hdr->base_crc);
+	}
+
+	k = (void *)hdr + hdr->hdr_len + hdr->kind_layout_off;
+	nr_kinds = hdr->kind_layout_len / sizeof(*k);
+	for (i = 0; i < nr_kinds; i++) {
+		if (json_output) {
+			jsonw_start_object(json_wtr);
+			jsonw_name(json_wtr, "kind_layout");
+			jsonw_uint_field(json_wtr, "kind", i);
+			jsonw_uint_field(json_wtr, "flags", k[i].flags);
+			jsonw_uint_field(json_wtr, "info_sz", k[i].info_sz);
+			jsonw_uint_field(json_wtr, "elem_sz", k[i].elem_sz);
+			jsonw_end_object(json_wtr);
+		} else {
+			printf("kind %-4d flags 0x%-4x info_sz %-4d elem_sz %-4d\n",
+			       i, k[i].flags, k[i].info_sz, k[i].elem_sz);
+		}
+	}
+	if (json_output) {
+		jsonw_end_array(json_wtr);
+		jsonw_end_object(json_wtr); /* end kind layout */
+		jsonw_end_object(json_wtr); /* end metadata object */
+	}
+
+	return 0;
+}
+
 static const char sysfs_vmlinux[] = "/sys/kernel/btf/vmlinux";
 
 static struct btf *get_vmlinux_btf_from_sysfs(void)
@@ -553,6 +637,7 @@ static int do_dump(int argc, char **argv)
 	__u32 root_type_ids[2];
 	int root_type_cnt = 0;
 	bool dump_c = false;
+	bool dump_meta = false;
 	__u32 btf_id = -1;
 	const char *src;
 	int fd = -1;
@@ -654,10 +739,12 @@ static int do_dump(int argc, char **argv)
 			}
 			if (strcmp(*argv, "c") == 0) {
 				dump_c = true;
+			} else if (is_prefix(*argv, "meta")) {
+				dump_meta = true;
 			} else if (strcmp(*argv, "raw") == 0) {
 				dump_c = false;
 			} else {
-				p_err("unrecognized format specifier: '%s', possible values: raw, c",
+				p_err("unrecognized format specifier: '%s', possible values: raw, c, meta",
 				      *argv);
 				err = -EINVAL;
 				goto done;
@@ -692,6 +779,8 @@ static int do_dump(int argc, char **argv)
 			goto done;
 		}
 		err = dump_btf_c(btf, root_type_ids, root_type_cnt);
+	} else if (dump_meta) {
+		err = dump_btf_meta(btf);
 	} else {
 		err = dump_btf_raw(btf, root_type_ids, root_type_cnt);
 	}
@@ -1063,7 +1152,7 @@ static int do_help(int argc, char **argv)
 		"       %1$s %2$s help\n"
 		"\n"
 		"       BTF_SRC := { id BTF_ID | prog PROG | map MAP [{key | value | kv | all}] | file FILE }\n"
-		"       FORMAT  := { raw | c }\n"
+		"       FORMAT  := { raw | c | meta }\n"
 		"       " HELP_SPEC_MAP "\n"
 		"       " HELP_SPEC_PROGRAM "\n"
 		"       " HELP_SPEC_OPTIONS " |\n"
-- 
2.39.3

[PATCH v2 bpf-next 8/9] bpftool: update doc to describe bpftool btf dump .. format meta

[Alan Maguire]

...and provide an example of output.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 tools/bpf/bpftool/Documentation/bpftool-btf.rst | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-btf.rst b/tools/bpf/bpftool/Documentation/bpftool-btf.rst
index 342716f74ec4..6dd779dddbde 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-btf.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-btf.rst
@@ -28,7 +28,7 @@ BTF COMMANDS
 |	**bpftool** **btf help**
 |
 |	*BTF_SRC* := { **id** *BTF_ID* | **prog** *PROG* | **map** *MAP* [{**key** | **value** | **kv** | **all**}] | **file** *FILE* }
-|	*FORMAT* := { **raw** | **c** }
+|	*FORMAT* := { **raw** | **c** | **meta** }
 |	*MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
 |	*PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* }
 
@@ -67,8 +67,8 @@ DESCRIPTION
 		  typically produced by clang or pahole.
 
 		  **format** option can be used to override default (raw)
-		  output format. Raw (**raw**) or C-syntax (**c**) output
-		  formats are supported.
+		  output format. Raw (**raw**), C-syntax (**c**) and
+                  BTF metadata (**meta**) formats are supported.
 
 	**bpftool btf help**
 		  Print short help message.
@@ -266,3 +266,13 @@ All the standard ways to specify map or program are supported:
   [104859] FUNC 'smbalert_work' type_id=9695 linkage=static
   [104860] FUNC 'smbus_alert' type_id=71367 linkage=static
   [104861] FUNC 'smbus_do_alert' type_id=84827 linkage=static
+
+
+ **# bpftool btf dump file vmlinux format meta**
+
+ ::
+ size 4904369
+ magic 0xeb9f       version 1          flags 0x0          hdr_len 24
+ type      len 2893508    off 0
+ str       len 2010837    off 2893508
+
-- 
2.39.3

[PATCH v2 bpf-next 9/9] selftests/bpf: test kind encoding/decoding

[Alan Maguire]

verify btf__new_empty_opts() adds kind layouts for all kinds supported,
and after adding kind-related types for an unknown kind, ensure that
parsing uses this info when that kind is encountered rather than
giving up.  Also verify that presence of a required kind will fail
parsing.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 .../selftests/bpf/prog_tests/btf_kind.c       | 187 ++++++++++++++++++
 1 file changed, 187 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/btf_kind.c

diff --git a/tools/testing/selftests/bpf/prog_tests/btf_kind.c b/tools/testing/selftests/bpf/prog_tests/btf_kind.c
new file mode 100644
index 000000000000..ff37126b6bc0
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/btf_kind.c
@@ -0,0 +1,187 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023, Oracle and/or its affiliates. */
+
+#include <test_progs.h>
+#include <bpf/btf.h>
+#include <bpf/libbpf.h>
+
+/* verify kind encoding exists for each kind */
+void test_btf_kind_encoding(struct btf *btf)
+{
+	const struct btf_header *hdr;
+	const void *raw_btf;
+	__u32 raw_size;
+
+	raw_btf = btf__raw_data(btf, &raw_size);
+	if (!ASSERT_OK_PTR(raw_btf, "btf__raw_data"))
+		return;
+
+	hdr = raw_btf;
+
+	ASSERT_GT(hdr->kind_layout_off, hdr->str_off, "kind layout off");
+	ASSERT_EQ(hdr->kind_layout_len, sizeof(struct btf_kind_layout) * NR_BTF_KINDS,
+		  "kind_layout_len");
+}
+
+static void write_raw_btf(const char *btf_path, void *raw_btf, size_t raw_size)
+{
+	int fd = open(btf_path, O_WRONLY | O_CREAT);
+
+	write(fd, raw_btf, raw_size);
+	close(fd);
+}
+
+/* fabricate an unrecognized kind at BTF_KIND_MAX + 1, and after adding
+ * the appropriate struct/typedefs to the BTF such that it recognizes
+ * this kind, ensure that parsing of BTF containing the unrecognized kind
+ * can succeed.
+ */
+void test_btf_kind_decoding(struct btf *btf)
+{
+	__s32 int_id, unrec_id, id, id2;
+	struct btf_type *t;
+	char btf_path[64];
+	const void *raw_btf;
+	void *new_raw_btf;
+	struct btf *new_btf;
+	struct btf_header *hdr;
+	struct btf_kind_layout *k;
+	__u32 raw_size;
+
+	int_id = btf__add_int(btf, "test_char", 1, BTF_INT_CHAR);
+	if (!ASSERT_GT(int_id, 0, "add_int_id"))
+		return;
+
+	/* now create our type with unrecognized kind by adding a typedef kind
+	 * we will overwrite it with our unrecognized kind value.
+	 */
+	unrec_id = btf__add_typedef(btf, "unrec_kind", int_id);
+	if (!ASSERT_GT(unrec_id, 0, "add_unrec_id"))
+		return;
+
+	/* add an id after it that we will look up to verify we can parse
+	 * beyond unrecognized kinds.
+	 */
+	id = btf__add_typedef(btf, "test_lookup", int_id);
+	if (!ASSERT_GT(id, 0, "add_test_lookup_id"))
+		return;
+	id2 = btf__add_typedef(btf, "test_lookup2", int_id);
+	if (!ASSERT_GT(id2, 0, "add_test_lookup_id2"))
+		return;
+
+	raw_btf = (void *)btf__raw_data(btf, &raw_size);
+	if (!ASSERT_OK_PTR(raw_btf, "btf__raw_data"))
+		return;
+
+	new_raw_btf = calloc(1, raw_size + sizeof(*k));
+	memcpy(new_raw_btf, raw_btf, raw_size);
+
+	/* add new layout description */
+	hdr = new_raw_btf;
+	hdr->kind_layout_len += sizeof(*k);
+	k = new_raw_btf + hdr->hdr_len + hdr->kind_layout_off;
+	k[NR_BTF_KINDS].flags = BTF_KIND_LAYOUT_OPTIONAL;
+	k[NR_BTF_KINDS].info_sz = 0;
+	k[NR_BTF_KINDS].elem_sz = 0;
+
+	/* now modify our typedef added above to be an unrecognized kind. */
+	t = (void *)hdr + hdr->hdr_len + hdr->type_off + sizeof(struct btf_type) +
+		sizeof(__u32);
+	t->info = (NR_BTF_KINDS << 24);
+
+	/* now write our BTF to a raw file, ready for parsing. */
+	snprintf(btf_path, sizeof(btf_path), "/tmp/btf_kind.%d", getpid());
+
+	write_raw_btf(btf_path, new_raw_btf, raw_size + sizeof(*k));
+
+	/* verify parsing succeeds, and that we can read type info past
+	 * the unrecognized kind.
+	 */
+	new_btf = btf__parse_raw(btf_path);
+	if (ASSERT_OK_PTR(new_btf, "btf__parse_raw")) {
+		ASSERT_EQ(btf__find_by_name_kind(new_btf, "test_lookup",
+						 BTF_KIND_TYPEDEF), id,
+			  "verify_id_lookup");
+		ASSERT_EQ(btf__find_by_name_kind(new_btf, "test_lookup2",
+						 BTF_KIND_TYPEDEF), id2,
+			  "verify_id2_lookup");
+
+		/* verify the kernel can handle unrecognized kinds. */
+		ASSERT_EQ(btf__load_into_kernel(new_btf), 0, "btf_load_into_kernel");
+	}
+	btf__free(new_btf);
+
+	/* next, change info_sz to equal sizeof(struct btf_type); this means the
+	 * "test_lookup" kind will be reinterpreted as a singular info element
+	 * following the unrecognized kind.
+	 */
+	k[NR_BTF_KINDS].info_sz = sizeof(struct btf_type);
+	write_raw_btf(btf_path, new_raw_btf, raw_size + sizeof(*k));
+
+	new_btf = btf__parse_raw(btf_path);
+	if (ASSERT_OK_PTR(new_btf, "btf__parse_raw")) {
+		ASSERT_EQ(btf__find_by_name_kind(new_btf, "test_lookup",
+						 BTF_KIND_TYPEDEF), -ENOENT,
+			  "verify_id_not_found");
+		/* id of "test_lookup2" will be id2 -1 as we have removed one type */
+		ASSERT_EQ(btf__find_by_name_kind(new_btf, "test_lookup2",
+						 BTF_KIND_TYPEDEF), id2 - 1,
+			  "verify_id_lookup2");
+
+		/* verify the kernel can handle unrecognized kinds. */
+		ASSERT_EQ(btf__load_into_kernel(new_btf), 0, "btf_load_into_kernel");
+	}
+	btf__free(new_btf);
+
+	/* next, change elem_sz to equal sizeof(struct btf_type)/2 and set
+	 * vlen associated with unrecognized type to 2; this allows us to verify
+	 * vlen-specified BTF can still be parsed.
+	 */
+	k[NR_BTF_KINDS].info_sz = 0;
+	k[NR_BTF_KINDS].elem_sz = sizeof(struct btf_type)/2;
+	t->info |= 2;
+	write_raw_btf(btf_path, new_raw_btf, raw_size + sizeof(*k));
+
+	new_btf = btf__parse_raw(btf_path);
+	if (ASSERT_OK_PTR(new_btf, "btf__parse_raw")) {
+		ASSERT_EQ(btf__find_by_name_kind(new_btf, "test_lookup",
+						 BTF_KIND_TYPEDEF), -ENOENT,
+			  "verify_id_not_found");
+		/* id of "test_lookup2" will be id2 -1 as we have removed one type */
+		ASSERT_EQ(btf__find_by_name_kind(new_btf, "test_lookup2",
+						 BTF_KIND_TYPEDEF), id2 - 1,
+			  "verify_id_lookup2");
+
+		/* verify the kernel can handle unrecognized kinds. */
+		ASSERT_EQ(btf__load_into_kernel(new_btf), 0, "btf_load_into_kernel");
+	}
+	btf__free(new_btf);
+
+	/* next, change kind to required (no optional flag) and ensure parsing fails. */
+	k[NR_BTF_KINDS].flags = 0;
+	write_raw_btf(btf_path, new_raw_btf, raw_size + sizeof(*k));
+
+	new_btf = btf__parse_raw(btf_path);
+	ASSERT_ERR_PTR(new_btf, "btf__parse_raw_required");
+
+	free(new_raw_btf);
+	unlink(btf_path);
+}
+
+void test_btf_kind(void)
+{
+	LIBBPF_OPTS(btf_new_opts, opts);
+
+	opts.add_kind_layout = true;
+
+	struct btf *btf = btf__new_empty_opts(&opts);
+
+	if (!ASSERT_OK_PTR(btf, "btf_new"))
+		return;
+
+	if (test__start_subtest("btf_kind_encoding"))
+		test_btf_kind_encoding(btf);
+	if (test__start_subtest("btf_kind_decoding"))
+		test_btf_kind_decoding(btf);
+	btf__free(btf);
+}
-- 
2.39.3

[PATCH dwarves] dwarves: encode BTF kind layout, crcs

[Alan Maguire]

Encode kind layout at time of BTF encoding via --btf_gen_kind_layout
and set CRC if --btf_gen_crc is set.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 btf_encoder.c | 10 ++++++++--
 btf_encoder.h |  2 +-
 dwarves.h     |  2 ++
 lib/bpf       |  2 +-
 pahole.c      | 19 ++++++++++++++++++-
 5 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index 65f6e71..19cf003 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -1625,17 +1625,23 @@ out:
 	return err;
 }
 
-struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filename, struct btf *base_btf, bool skip_encoding_vars, bool force, bool gen_floats, bool verbose)
+struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filename, struct btf *base_btf, bool skip_encoding_vars, bool force, bool gen_floats, struct conf_load *conf_load, bool verbose)
 {
 	struct btf_encoder *encoder = zalloc(sizeof(*encoder));
 
 	if (encoder) {
+		LIBBPF_OPTS(btf_new_opts, opts);
+
+		opts.base_btf = base_btf;
+		opts.add_kind_layout = conf_load->btf_gen_kind_layout;
+		opts.add_crc = conf_load->btf_gen_crc;
+
 		encoder->raw_output = detached_filename != NULL;
 		encoder->filename = strdup(encoder->raw_output ? detached_filename : cu->filename);
 		if (encoder->filename == NULL)
 			goto out_delete;
 
-		encoder->btf = btf__new_empty_split(base_btf);
+		encoder->btf = btf__new_empty_opts(&opts);
 		if (encoder->btf == NULL)
 			goto out_delete;
 
diff --git a/btf_encoder.h b/btf_encoder.h
index 34516bb..fbbf564 100644
--- a/btf_encoder.h
+++ b/btf_encoder.h
@@ -16,7 +16,7 @@ struct btf;
 struct cu;
 struct list_head;
 
-struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filename, struct btf *base_btf, bool skip_encoding_vars, bool force, bool gen_floats, bool verbose);
+struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filename, struct btf *base_btf, bool skip_encoding_vars, bool force, bool gen_floats, struct conf_load *conf_load, bool verbose);
 void btf_encoder__delete(struct btf_encoder *encoder);
 
 int btf_encoder__encode(struct btf_encoder *encoder);
diff --git a/dwarves.h b/dwarves.h
index eb1a6df..0360807 100644
--- a/dwarves.h
+++ b/dwarves.h
@@ -68,6 +68,8 @@ struct conf_load {
 	bool			skip_encoding_btf_enum64;
 	bool			btf_gen_optimized;
 	bool			skip_encoding_btf_inconsistent_proto;
+	bool			btf_gen_kind_layout;
+	bool			btf_gen_crc;
 	uint8_t			hashtable_bits;
 	uint8_t			max_hashtable_bits;
 	uint16_t		kabi_prefix_len;
diff --git a/lib/bpf b/lib/bpf
index 6597330..3f591a6 160000
--- a/lib/bpf
+++ b/lib/bpf
@@ -1 +1 @@
-Subproject commit 6597330c45d185381900037f0130712cd326ae59
+Subproject commit 3f591a66103d49b311956618d440a84cf4d30715
diff --git a/pahole.c b/pahole.c
index 6fc4ed6..e7268a3 100644
--- a/pahole.c
+++ b/pahole.c
@@ -1232,6 +1232,8 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version;
 #define ARGP_skip_emitting_atomic_typedefs 338
 #define ARGP_btf_gen_optimized  339
 #define ARGP_skip_encoding_btf_inconsistent_proto 340
+#define ARGP_btf_gen_kind_layout   341
+#define ARGP_btf_gen_crc           342
 
 static const struct argp_option pahole__options[] = {
 	{
@@ -1654,6 +1656,16 @@ static const struct argp_option pahole__options[] = {
 		.key = ARGP_skip_encoding_btf_inconsistent_proto,
 		.doc = "Skip functions that have multiple inconsistent function prototypes sharing the same name, or that use unexpected registers for parameter values."
 	},
+	{
+		.name = "btf_gen_kind_layout",
+		.key  = ARGP_btf_gen_kind_layout,
+		.doc  = "Generate BTF kind layout information about kinds available."
+	},
+	{
+		.name = "btf_gen_crc",
+		.key  = ARGP_btf_gen_crc,
+		.doc  = "Generate CRC in BTF header."
+	},
 	{
 		.name = NULL,
 	}
@@ -1829,6 +1841,10 @@ static error_t pahole__options_parser(int key, char *arg,
 		conf_load.btf_gen_optimized = true;		break;
 	case ARGP_skip_encoding_btf_inconsistent_proto:
 		conf_load.skip_encoding_btf_inconsistent_proto = true; break;
+	case ARGP_btf_gen_kind_layout:
+		conf_load.btf_gen_kind_layout = true;		break;
+	case ARGP_btf_gen_crc:
+		conf_load.btf_gen_crc = true;			break;
 	default:
 		return ARGP_ERR_UNKNOWN;
 	}
@@ -3064,7 +3080,7 @@ static enum load_steal_kind pahole_stealer(struct cu *cu,
 			 * create it.
 			 */
 			btf_encoder = btf_encoder__new(cu, detached_btf_filename, conf_load->base_btf, skip_encoding_btf_vars,
-						       btf_encode_force, btf_gen_floats, global_verbose);
+						       btf_encode_force, btf_gen_floats, conf_load, global_verbose);
 			if (btf_encoder && thr_data) {
 				struct thread_data *thread = thr_data;
 
@@ -3096,6 +3112,7 @@ static enum load_steal_kind pahole_stealer(struct cu *cu,
 							 skip_encoding_btf_vars,
 							 btf_encode_force,
 							 btf_gen_floats,
+							 conf_load,
 							 global_verbose);
 				thread->btf = btf_encoder__btf(thread->encoder);
 			}
-- 
2.31.1

Re: [PATCH v2 bpf-next 6/9] btf: generate BTF kind layout for vmlinux/module BTF

[kernel test robot]

Hi Alan,

kernel test robot noticed the following build errors:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Alan-Maguire/btf-add-kind-layout-encoding-crcs-to-UAPI/20230617-012110
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link:    https://lore.kernel.org/r/20230616171728.530116-7-alan.maguire%40oracle.com
patch subject: [PATCH v2 bpf-next 6/9] btf: generate BTF kind layout for vmlinux/module BTF
config: loongarch-allyesconfig (https://download.01.org/0day-ci/archive/20230617/202306170238.L0eHQOJd-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230617/202306170238.L0eHQOJd-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306170238.L0eHQOJd-lkp@intel.com/

All errors (new ones prefixed by >>):

>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
--
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   loongarch64-linux-gcc: error: unrecognized command-line option '-mexplicit-relocs'
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
>> scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   loongarch64-linux-gcc: error: unrecognized command-line option '-mexplicit-relocs'
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found
   scripts/pahole-flags.sh: 29: [[: not found
   scripts/pahole-flags.sh: 32: [[: not found

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 bpf-next 0/9] bpf: support BTF kind layout info, CRCs

[Alexei Starovoitov]

On Fri, Jun 16, 2023 at 06:17:18PM +0100, Alan Maguire wrote:
> By separating parsing BTF from using all the information
> it provides, we allow BTF to encode new features even if
> they cannot be used.  This is helpful in particular for
> cases where newer tools for BTF generation run on an
> older kernel; BTF kinds may be present that the kernel
> cannot yet use, but at least it can parse the BTF
> provided.  Meanwhile userspace tools with newer libbpf
> may be able to use the newer information.

Overall looks great, but
why such narrow formatting? It's much less than 80.

> 
> The intent is to support encoding of kind layouts
> optionally so that tools like pahole can add this
> information.  So for each kind we record
> 
> - kind-related flags
> - length of singular element following struct btf_type
> - length of each of the btf_vlen() elements following
> 
> In addition we make space in the BTF header for
> CRC32s computed over the BTF along with a CRC for
> the base BTF; this allows split BTF to identify
> a mismatch explicitly.
> 
> The ideas here were discussed at [1], with further
> discussion at [2].
> 
> Future work can take more advantage of these features
> such as
> 
> - using base CRC to identify base/module BTF mismatch
>   explicitly
> - using absence of a base BTF CRC as evidence that
>   BTF is standalone

That's fine to have as a follow up, but with BTF_FLAG_CRC_SET
the kernel should check the crc.
Calling crc32c on modern cpus should be plenty fast.
It won't slow down btf verification.

Re: [PATCH v2 bpf-next 1/9] btf: add kind layout encoding, crcs to UAPI

[Alexei Starovoitov]

On Fri, Jun 16, 2023 at 06:17:19PM +0100, Alan Maguire wrote:
> BTF kind layouts provide information to parse BTF kinds.
> By separating parsing BTF from using all the information
> it provides, we allow BTF to encode new features even if
> they cannot be used.  This is helpful in particular for
> cases where newer tools for BTF generation run on an
> older kernel; BTF kinds may be present that the kernel
> cannot yet use, but at least it can parse the BTF
> provided.  Meanwhile userspace tools with newer libbpf
> may be able to use the newer information.
> 
> The intent is to support encoding of kind layouts
> optionally so that tools like pahole can add this
> information.  So for each kind we record
> 
> - kind-related flags
> - length of singular element following struct btf_type
> - length of each of the btf_vlen() elements following
> 
> In addition we make space in the BTF header for
> CRC32s computed over the BTF along with a CRC for
> the base BTF; this allows split BTF to identify
> a mismatch explicitly.
> 
> The ideas here were discussed at [1], [2]; hence
> 
> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> 
> [1] https://lore.kernel.org/bpf/CAEf4BzYjWHRdNNw4B=eOXOs_ONrDwrgX4bn=Nuc1g8JPFC34MA@mail.gmail.com/
> [2] https://lore.kernel.org/bpf/20230531201936.1992188-1-alan.maguire@oracle.com/
> ---
>  include/uapi/linux/btf.h       | 24 ++++++++++++++++++++++++
>  tools/include/uapi/linux/btf.h | 24 ++++++++++++++++++++++++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
> index ec1798b6d3ff..cea9125ed953 100644
> --- a/include/uapi/linux/btf.h
> +++ b/include/uapi/linux/btf.h
> @@ -8,6 +8,22 @@
>  #define BTF_MAGIC	0xeB9F
>  #define BTF_VERSION	1
>  
> +/* is this information required? If so it cannot be sanitized safely. */
> +#define BTF_KIND_LAYOUT_OPTIONAL		(1 << 0)
> +
> +/* kind layout section consists of a struct btf_kind_layout for each known
> + * kind at BTF encoding time.
> + */
> +struct btf_kind_layout {
> +	__u16 flags;		/* see BTF_KIND_LAYOUT_* values above */
> +	__u8 info_sz;		/* size of singular element after btf_type */
> +	__u8 elem_sz;		/* size of each of btf_vlen(t) elements */
> +};
> +
> +/* for CRCs for BTF, base BTF to be considered usable, flags must be set. */
> +#define BTF_FLAG_CRC_SET		(1 << 0)
> +#define BTF_FLAG_BASE_CRC_SET		(1 << 1)
> +
>  struct btf_header {
>  	__u16	magic;
>  	__u8	version;
> @@ -19,8 +35,16 @@ struct btf_header {
>  	__u32	type_len;	/* length of type section	*/
>  	__u32	str_off;	/* offset of string section	*/
>  	__u32	str_len;	/* length of string section	*/
> +	__u32	kind_layout_off;/* offset of kind layout section */
> +	__u32	kind_layout_len;/* length of kind layout section */
> +
> +	__u32	crc;		/* crc of BTF; used if flags set BTF_FLAG_CRC_VALID */
> +	__u32	base_crc;	/* crc of base BTF; used if flags set BTF_FLAG_BASE_CRC_VALID */

typo ? should be BTF_FLAG_CRC_SET ?

Re: [PATCH v2 bpf-next 6/9] btf: generate BTF kind layout for vmlinux/module BTF

[Jiri Olsa]

On Fri, Jun 16, 2023 at 06:17:24PM +0100, Alan Maguire wrote:
> Generate BTF kind layout information, crcs for kernel and module BTF
> if support is available in pahole.
> 
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  scripts/pahole-flags.sh | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/scripts/pahole-flags.sh b/scripts/pahole-flags.sh
> index 728d55190d97..cb304e0a4434 100755
> --- a/scripts/pahole-flags.sh
> +++ b/scripts/pahole-flags.sh
> @@ -25,6 +25,13 @@ if [ "${pahole_ver}" -ge "124" ]; then
>  fi
>  if [ "${pahole_ver}" -ge "125" ]; then
>  	extra_paholeopt="${extra_paholeopt} --skip_encoding_btf_inconsistent_proto --btf_gen_optimized"
> +	pahole_help="$(${PAHOLE} --help)"

nice ;-)

> +	if [[ "$pahole_help" =~ "btf_gen_kind_layout" ]]; then
> +		extra_paholeopt="${extra_paholeopt} --btf_gen_kind_layout"
> +	fi
> +	if [[ "$pahole_help" =~ "btf_gen_crc" ]]; then
> +		extra_paholeopt="${extra_paholeopt} --btf_gen_crc"
> +	fi

do we need to have an option to enable crc? could it be by default?

it's sort of related to the layout changes and I wonder we will want
'not to have it' if there's support for it in BTF

jirka

>  fi
>  
>  echo ${extra_paholeopt}
> -- 
> 2.39.3
> 

Re: [PATCH v2 bpf-next 4/9] btf: support kernel parsing of BTF with kind layout

[Jiri Olsa]

On Fri, Jun 16, 2023 at 06:17:22PM +0100, Alan Maguire wrote:

SNIP

>  static int btf_sec_info_cmp(const void *a, const void *b)
> @@ -5193,32 +5246,37 @@ static int btf_check_sec_info(struct btf_verifier_env *env,
>  			      u32 btf_data_size)
>  {
>  	struct btf_sec_info secs[ARRAY_SIZE(btf_sec_info_offset)];
> -	u32 total, expected_total, i;
> +	u32 nr_secs = ARRAY_SIZE(btf_sec_info_offset);
> +	u32 total, expected_total, gap, i;
>  	const struct btf_header *hdr;
>  	const struct btf *btf;
>  
>  	btf = env->btf;
>  	hdr = &btf->hdr;
>  
> +	if (hdr->hdr_len < sizeof(struct btf_header))
> +		nr_secs--;
> +
>  	/* Populate the secs from hdr */
> -	for (i = 0; i < ARRAY_SIZE(btf_sec_info_offset); i++)
> +	for (i = 0; i < nr_secs; i++)
>  		secs[i] = *(struct btf_sec_info *)((void *)hdr +
>  						   btf_sec_info_offset[i]);
>  
> -	sort(secs, ARRAY_SIZE(btf_sec_info_offset),
> +	sort(secs, nr_secs,
>  	     sizeof(struct btf_sec_info), btf_sec_info_cmp, NULL);
>  
>  	/* Check for gaps and overlap among sections */
>  	total = 0;
>  	expected_total = btf_data_size - hdr->hdr_len;
> -	for (i = 0; i < ARRAY_SIZE(btf_sec_info_offset); i++) {
> +	for (i = 0; i < nr_secs; i++) {
>  		if (expected_total < secs[i].off) {
>  			btf_verifier_log(env, "Invalid section offset");
>  			return -EINVAL;
>  		}
> -		if (total < secs[i].off) {
> -			/* gap */
> -			btf_verifier_log(env, "Unsupported section found");
> +		gap = secs[i].off - total;
> +		if (gap >= 4) {
> +			/* gap larger than alignment gap */
> +			btf_verifier_log(env, "Unsupported section gap found");
>  			return -EINVAL;

this sems to break several btf header tests with:

	do_test_raw:PASS:check 0 nsec
	do_test_raw:FAIL:check expected err_str:Unsupported section found

	magic: 0xeb9f
	version: 1
	flags: 0x0
	hdr_len: 40
	type_off: 4
	type_len: 16
	str_off: 16
	str_len: 5
	btf_total_size: 61
	Unsupported section gap found
	#23/48   btf/btf_header test. Gap between hdr and type:FAIL


jirka

>  		}
>  		if (total > secs[i].off) {
> @@ -5230,7 +5288,7 @@ static int btf_check_sec_info(struct btf_verifier_env *env,
>  					 "Total section length too long");
>  			return -EINVAL;
>  		}
> -		total += secs[i].len;
> +		total += secs[i].len + gap;
>  	}
>  
>  	/* There is data other than hdr and known sections */
> @@ -5293,7 +5351,7 @@ static int btf_parse_hdr(struct btf_verifier_env *env)
>  		return -ENOTSUPP;
>  	}
>  
> -	if (hdr->flags) {
> +	if (hdr->flags & ~(BTF_FLAG_CRC_SET | BTF_FLAG_BASE_CRC_SET)) {
>  		btf_verifier_log(env, "Unsupported flags");
>  		return -ENOTSUPP;
>  	}
> @@ -5530,6 +5588,10 @@ static struct btf *btf_parse(const union bpf_attr *attr, bpfptr_t uattr, u32 uat
>  	if (err)
>  		goto errout;
>  
> +	err = btf_parse_kind_layout_sec(env);
> +	if (err)
> +		goto errout;
> +
>  	err = btf_parse_type_sec(env);
>  	if (err)
>  		goto errout;
> -- 
> 2.39.3
> 

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

[Yonghong Song]

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@oracle.com>
> ---
>   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.

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?

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?

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

Re: [PATCH v2 bpf-next 4/9] btf: support kernel parsing of BTF with kind layout

[Alan Maguire]

On 18/06/2023 14:08, Jiri Olsa wrote:
> On Fri, Jun 16, 2023 at 06:17:22PM +0100, Alan Maguire wrote:
> 
> SNIP
> 
>>  static int btf_sec_info_cmp(const void *a, const void *b)
>> @@ -5193,32 +5246,37 @@ static int btf_check_sec_info(struct btf_verifier_env *env,
>>  			      u32 btf_data_size)
>>  {
>>  	struct btf_sec_info secs[ARRAY_SIZE(btf_sec_info_offset)];
>> -	u32 total, expected_total, i;
>> +	u32 nr_secs = ARRAY_SIZE(btf_sec_info_offset);
>> +	u32 total, expected_total, gap, i;
>>  	const struct btf_header *hdr;
>>  	const struct btf *btf;
>>  
>>  	btf = env->btf;
>>  	hdr = &btf->hdr;
>>  
>> +	if (hdr->hdr_len < sizeof(struct btf_header))
>> +		nr_secs--;
>> +
>>  	/* Populate the secs from hdr */
>> -	for (i = 0; i < ARRAY_SIZE(btf_sec_info_offset); i++)
>> +	for (i = 0; i < nr_secs; i++)
>>  		secs[i] = *(struct btf_sec_info *)((void *)hdr +
>>  						   btf_sec_info_offset[i]);
>>  
>> -	sort(secs, ARRAY_SIZE(btf_sec_info_offset),
>> +	sort(secs, nr_secs,
>>  	     sizeof(struct btf_sec_info), btf_sec_info_cmp, NULL);
>>  
>>  	/* Check for gaps and overlap among sections */
>>  	total = 0;
>>  	expected_total = btf_data_size - hdr->hdr_len;
>> -	for (i = 0; i < ARRAY_SIZE(btf_sec_info_offset); i++) {
>> +	for (i = 0; i < nr_secs; i++) {
>>  		if (expected_total < secs[i].off) {
>>  			btf_verifier_log(env, "Invalid section offset");
>>  			return -EINVAL;
>>  		}
>> -		if (total < secs[i].off) {
>> -			/* gap */
>> -			btf_verifier_log(env, "Unsupported section found");
>> +		gap = secs[i].off - total;
>> +		if (gap >= 4) {
>> +			/* gap larger than alignment gap */
>> +			btf_verifier_log(env, "Unsupported section gap found");
>>  			return -EINVAL;
> 
> this sems to break several btf header tests with:
> 
> 	do_test_raw:PASS:check 0 nsec
> 	do_test_raw:FAIL:check expected err_str:Unsupported section found
> 
> 	magic: 0xeb9f
> 	version: 1
> 	flags: 0x0
> 	hdr_len: 40
> 	type_off: 4
> 	type_len: 16
> 	str_off: 16
> 	str_len: 5
> 	btf_total_size: 61
> 	Unsupported section gap found
> 	#23/48   btf/btf_header test. Gap between hdr and type:FAIL
> 
>

thanks for spotting this Jiri! I've reworked the logic and the
messages for v3 (in progress), and these pass now.

Alan

> jirka
> 
>>  		}
>>  		if (total > secs[i].off) {
>> @@ -5230,7 +5288,7 @@ static int btf_check_sec_info(struct btf_verifier_env *env,
>>  					 "Total section length too long");
>>  			return -EINVAL;
>>  		}
>> -		total += secs[i].len;
>> +		total += secs[i].len + gap;
>>  	}
>>  
>>  	/* There is data other than hdr and known sections */
>> @@ -5293,7 +5351,7 @@ static int btf_parse_hdr(struct btf_verifier_env *env)
>>  		return -ENOTSUPP;
>>  	}
>>  
>> -	if (hdr->flags) {
>> +	if (hdr->flags & ~(BTF_FLAG_CRC_SET | BTF_FLAG_BASE_CRC_SET)) {
>>  		btf_verifier_log(env, "Unsupported flags");
>>  		return -ENOTSUPP;
>>  	}
>> @@ -5530,6 +5588,10 @@ static struct btf *btf_parse(const union bpf_attr *attr, bpfptr_t uattr, u32 uat
>>  	if (err)
>>  		goto errout;
>>  
>> +	err = btf_parse_kind_layout_sec(env);
>> +	if (err)
>> +		goto errout;
>> +
>>  	err = btf_parse_type_sec(env);
>>  	if (err)
>>  		goto errout;
>> -- 
>> 2.39.3
>>

Re: [PATCH v2 bpf-next 0/9] bpf: support BTF kind layout info, CRCs

[Alan Maguire]

On 17/06/2023 01:39, Alexei Starovoitov wrote:
> On Fri, Jun 16, 2023 at 06:17:18PM +0100, Alan Maguire wrote:
>> By separating parsing BTF from using all the information
>> it provides, we allow BTF to encode new features even if
>> they cannot be used.  This is helpful in particular for
>> cases where newer tools for BTF generation run on an
>> older kernel; BTF kinds may be present that the kernel
>> cannot yet use, but at least it can parse the BTF
>> provided.  Meanwhile userspace tools with newer libbpf
>> may be able to use the newer information.
> 
> Overall looks great, but
> why such narrow formatting? It's much less than 80.
> 
>>
>> The intent is to support encoding of kind layouts
>> optionally so that tools like pahole can add this
>> information.  So for each kind we record
>>
>> - kind-related flags
>> - length of singular element following struct btf_type
>> - length of each of the btf_vlen() elements following
>>
>> In addition we make space in the BTF header for
>> CRC32s computed over the BTF along with a CRC for
>> the base BTF; this allows split BTF to identify
>> a mismatch explicitly.
>>
>> The ideas here were discussed at [1], with further
>> discussion at [2].
>>
>> Future work can take more advantage of these features
>> such as
>>
>> - using base CRC to identify base/module BTF mismatch
>>   explicitly
>> - using absence of a base BTF CRC as evidence that
>>   BTF is standalone
> 
> That's fine to have as a follow up, but with BTF_FLAG_CRC_SET
> the kernel should check the crc.
> Calling crc32c on modern cpus should be plenty fast.
> It won't slow down btf verification.

Sure; I'll roll this into v3 and fix formatting and
the typo in btf.h. Thanks!

Re: [PATCH v2 bpf-next 6/9] btf: generate BTF kind layout for vmlinux/module BTF

[Alan Maguire]

On 18/06/2023 14:07, Jiri Olsa wrote:
> On Fri, Jun 16, 2023 at 06:17:24PM +0100, Alan Maguire wrote:
>> Generate BTF kind layout information, crcs for kernel and module BTF
>> if support is available in pahole.
>>
>> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>> ---
>>  scripts/pahole-flags.sh | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/scripts/pahole-flags.sh b/scripts/pahole-flags.sh
>> index 728d55190d97..cb304e0a4434 100755
>> --- a/scripts/pahole-flags.sh
>> +++ b/scripts/pahole-flags.sh
>> @@ -25,6 +25,13 @@ if [ "${pahole_ver}" -ge "124" ]; then
>>  fi
>>  if [ "${pahole_ver}" -ge "125" ]; then
>>  	extra_paholeopt="${extra_paholeopt} --skip_encoding_btf_inconsistent_proto --btf_gen_optimized"
>> +	pahole_help="$(${PAHOLE} --help)"
> 
> nice ;-)
> 
>> +	if [[ "$pahole_help" =~ "btf_gen_kind_layout" ]]; then
>> +		extra_paholeopt="${extra_paholeopt} --btf_gen_kind_layout"
>> +	fi
>> +	if [[ "$pahole_help" =~ "btf_gen_crc" ]]; then
>> +		extra_paholeopt="${extra_paholeopt} --btf_gen_crc"
>> +	fi
> 
> do we need to have an option to enable crc? could it be by default?
> 
> it's sort of related to the layout changes and I wonder we will want
> 'not to have it' if there's support for it in BTF
> 

I'm reluctant to enable by default yet because without CRC and kind
layout the new header format will work on older kernels too.  With
the kind layout len/offset 0 and CRCs 0, the header is just slightly
larger than the original.

I originally combined the two in the metadata section header, but
I think as separate concepts it makes sense to have separate
flags.

Thanks!

Alan

> jirka
> 
>>  fi
>>  
>>  echo ${extra_paholeopt}
>> -- 
>> 2.39.3
>>

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

[Alan Maguire]

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@oracle.com>
>> ---
>>   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.

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

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

Thanks!

Alan

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

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

[Yonghong Song]

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

Re: [PATCH v2 bpf-next 1/9] btf: add kind layout encoding, crcs to UAPI

[Andrii Nakryiko]

On Fri, Jun 16, 2023 at 10:17 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> BTF kind layouts provide information to parse BTF kinds.
> By separating parsing BTF from using all the information
> it provides, we allow BTF to encode new features even if
> they cannot be used.  This is helpful in particular for
> cases where newer tools for BTF generation run on an
> older kernel; BTF kinds may be present that the kernel
> cannot yet use, but at least it can parse the BTF
> provided.  Meanwhile userspace tools with newer libbpf
> may be able to use the newer information.
>
> The intent is to support encoding of kind layouts
> optionally so that tools like pahole can add this
> information.  So for each kind we record
>
> - kind-related flags
> - length of singular element following struct btf_type
> - length of each of the btf_vlen() elements following
>
> In addition we make space in the BTF header for
> CRC32s computed over the BTF along with a CRC for
> the base BTF; this allows split BTF to identify
> a mismatch explicitly.
>
> The ideas here were discussed at [1], [2]; hence
>
> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>
> [1] https://lore.kernel.org/bpf/CAEf4BzYjWHRdNNw4B=eOXOs_ONrDwrgX4bn=Nuc1g8JPFC34MA@mail.gmail.com/
> [2] https://lore.kernel.org/bpf/20230531201936.1992188-1-alan.maguire@oracle.com/
> ---
>  include/uapi/linux/btf.h       | 24 ++++++++++++++++++++++++
>  tools/include/uapi/linux/btf.h | 24 ++++++++++++++++++++++++
>  2 files changed, 48 insertions(+)
>
> diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
> index ec1798b6d3ff..cea9125ed953 100644
> --- a/include/uapi/linux/btf.h
> +++ b/include/uapi/linux/btf.h
> @@ -8,6 +8,22 @@
>  #define BTF_MAGIC      0xeB9F
>  #define BTF_VERSION    1
>
> +/* is this information required? If so it cannot be sanitized safely. */
> +#define BTF_KIND_LAYOUT_OPTIONAL               (1 << 0)

hm.. I thought we agreed to not have OPTIONAL flag last time, no? From
kernel's perspective nothing is optional. From libbpf perspective
everything should be optional, unless we get type_id reference to
something that we don't recognize. So why the flag and extra code to
handle it?

We can always add it later, if necessary.

> +
> +/* kind layout section consists of a struct btf_kind_layout for each known
> + * kind at BTF encoding time.
> + */
> +struct btf_kind_layout {
> +       __u16 flags;            /* see BTF_KIND_LAYOUT_* values above */
> +       __u8 info_sz;           /* size of singular element after btf_type */
> +       __u8 elem_sz;           /* size of each of btf_vlen(t) elements */
> +};
> +
> +/* for CRCs for BTF, base BTF to be considered usable, flags must be set. */
> +#define BTF_FLAG_CRC_SET               (1 << 0)
> +#define BTF_FLAG_BASE_CRC_SET          (1 << 1)
> +
>  struct btf_header {
>         __u16   magic;
>         __u8    version;
> @@ -19,8 +35,16 @@ struct btf_header {
>         __u32   type_len;       /* length of type section       */
>         __u32   str_off;        /* offset of string section     */
>         __u32   str_len;        /* length of string section     */
> +       __u32   kind_layout_off;/* offset of kind layout section */
> +       __u32   kind_layout_len;/* length of kind layout section */
> +
> +       __u32   crc;            /* crc of BTF; used if flags set BTF_FLAG_CRC_VALID */
> +       __u32   base_crc;       /* crc of base BTF; used if flags set BTF_FLAG_BASE_CRC_VALID */
>  };
>
> +/* required minimum BTF header length */
> +#define BTF_HEADER_MIN_LEN     (sizeof(struct btf_header) - 16)

offsetof(struct btf_header, kind_layout_off) ?

but actually why this needs to be a part of UAPI?

> +
>  /* Max # of type identifier */
>  #define BTF_MAX_TYPE   0x000fffff
>  /* Max offset into the string section */
> diff --git a/tools/include/uapi/linux/btf.h b/tools/include/uapi/linux/btf.h
> index ec1798b6d3ff..cea9125ed953 100644
> --- a/tools/include/uapi/linux/btf.h
> +++ b/tools/include/uapi/linux/btf.h
> @@ -8,6 +8,22 @@
>  #define BTF_MAGIC      0xeB9F
>  #define BTF_VERSION    1
>
> +/* is this information required? If so it cannot be sanitized safely. */
> +#define BTF_KIND_LAYOUT_OPTIONAL               (1 << 0)
> +
> +/* kind layout section consists of a struct btf_kind_layout for each known
> + * kind at BTF encoding time.
> + */
> +struct btf_kind_layout {
> +       __u16 flags;            /* see BTF_KIND_LAYOUT_* values above */
> +       __u8 info_sz;           /* size of singular element after btf_type */
> +       __u8 elem_sz;           /* size of each of btf_vlen(t) elements */
> +};
> +
> +/* for CRCs for BTF, base BTF to be considered usable, flags must be set. */
> +#define BTF_FLAG_CRC_SET               (1 << 0)
> +#define BTF_FLAG_BASE_CRC_SET          (1 << 1)
> +
>  struct btf_header {
>         __u16   magic;
>         __u8    version;
> @@ -19,8 +35,16 @@ struct btf_header {
>         __u32   type_len;       /* length of type section       */
>         __u32   str_off;        /* offset of string section     */
>         __u32   str_len;        /* length of string section     */
> +       __u32   kind_layout_off;/* offset of kind layout section */
> +       __u32   kind_layout_len;/* length of kind layout section */
> +
> +       __u32   crc;            /* crc of BTF; used if flags set BTF_FLAG_CRC_VALID */

why are we making crc optional? shouldn't we just say that crc is
always filled out?

> +       __u32   base_crc;       /* crc of base BTF; used if flags set BTF_FLAG_BASE_CRC_VALID */

here it would be nice if we could just rely on zero meaning not set,
but I suspect not everyone will be happy about this, as technically
crc 0 is a valid crc :(


>  };
>
> +/* required minimum BTF header length */
> +#define BTF_HEADER_MIN_LEN     (sizeof(struct btf_header) - 16)
> +
>  /* Max # of type identifier */
>  #define BTF_MAX_TYPE   0x000fffff
>  /* Max offset into the string section */
> --
> 2.39.3
>

Re: [PATCH v2 bpf-next 2/9] libbpf: support handling of kind layout section in BTF

[Andrii Nakryiko]

On Fri, Jun 16, 2023 at 10:17 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> support reading in kind layout fixing endian issues on reading;
> also support writing kind layout section to raw BTF object.
> There is not yet an API to populate the kind layout with meaningful
> information.
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  tools/lib/bpf/btf.c | 132 +++++++++++++++++++++++++++++++++-----------
>  1 file changed, 99 insertions(+), 33 deletions(-)
>
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index 8484b563b53d..f9f919fdc728 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -39,40 +39,44 @@ struct btf {
>

[...]

>         struct btf_header *hdr;
>
> @@ -116,6 +120,8 @@ struct btf {
>         /* whether strings are already deduplicated */
>         bool strs_deduped;
>
> +       struct btf_kind_layout *kind_layout;
> +
>         /* BTF object FD, if loaded into kernel */
>         int fd;
>
> @@ -215,6 +221,13 @@ 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->kind_layout_off = bswap_32(h->kind_layout_off);
> +               h->kind_layout_len = bswap_32(h->kind_layout_len);
> +               h->crc = bswap_32(h->crc);
> +               h->base_crc = bswap_32(h->base_crc);
> +       }
> +

nit: unnecessary empty line

>  }
>
>  static int btf_parse_hdr(struct btf *btf)
> @@ -222,14 +235,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 < BTF_HEADER_MIN_LEN) {
>                 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 != BTF_HEADER_MIN_LEN) {
>                         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 +301,32 @@ static int btf_parse_str_sec(struct btf *btf)
>         return 0;
>  }
>
> +static void btf_bswap_kind_layout_sec(struct btf_kind_layout *k, int len)
> +{
> +       struct btf_kind_layout *end = (void *)k + len;
> +
> +       while (k < end) {
> +               k->flags = bswap_16(k->flags);
> +               k++;
> +       }
> +}
> +
> +static int btf_parse_kind_layout_sec(struct btf *btf)
> +{
> +       const struct btf_header *hdr = btf->hdr;
> +
> +       if (hdr->hdr_len < sizeof(struct btf_header) ||
> +           !hdr->kind_layout_off || !hdr->kind_layout_len)
> +               return 0;

instead of having to remember to check `hdr->hdr_len < sizeof(struct
btf_header)` before accessing kind_layout_off and kind_layout_len,
let's just allocate a copy of full btf_header on initialization, copy
min(sizeof(struct btf_header), hdr->len) into it, and then point
btf->hdr to this copy everywhere?

> +       if (hdr->kind_layout_len < sizeof(struct btf_kind_layout)) {

shouldn't it be the check that hdr->kind_layout_len is a multiple of
sizeof(struct btf_kind_layout) ?

> +               pr_debug("Invalid BTF kind layout section\n");
> +               return -EINVAL;
> +       }
> +       btf->kind_layout = btf->raw_data + btf->hdr->hdr_len + btf->hdr->kind_layout_off;
> +
> +       return 0;
> +}
> +
>  static int btf_type_size(const struct btf_type *t)
>  {
>         const int base_size = sizeof(struct btf_type);
> @@ -901,6 +943,7 @@ static struct btf *btf_new(const void *data, __u32 size, struct btf *base_btf)
>         btf->types_data = btf->raw_data + btf->hdr->hdr_len + btf->hdr->type_off;
>
>         err = btf_parse_str_sec(btf);
> +       err = err ?: btf_parse_kind_layout_sec(btf);
>         err = err ?: btf_parse_type_sec(btf);
>         if (err)
>                 goto done;
> @@ -1267,6 +1310,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->kind_layout) {
> +               data_sz = roundup(data_sz, 4);
> +               data_sz += hdr->kind_layout_len;
> +               hdr->kind_layout_off = roundup(hdr->type_len + hdr->str_len, 4);

can we avoid modifying hdr here? we expect const struct btf *, so it's
a bit iffy that we are adjusting header here

maybe we can just make sure that kind_layout_off is always maintained correctly?

> +       }
>         data = calloc(1, data_sz);
>         if (!data)
>                 return NULL;
> @@ -1293,8 +1341,15 @@ 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 4 byte alignment to match offset above */
> +       p = data + hdr->hdr_len + roundup(hdr->type_len + hdr->str_len, 4);

instead of reimplementing roundup logic, why not just use
hdr->kind_layout_off here?

>
> +       if (btf->kind_layout) {
> +               memcpy(p, btf->kind_layout, hdr->kind_layout_len);
> +               if (swap_endian)
> +                       btf_bswap_kind_layout_sec(p, hdr->kind_layout_len);
> +               p += hdr->kind_layout_len;
> +       }
>         *size = data_sz;
>         return data;
>  err_out:
> @@ -1425,13 +1480,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

nit: gmail suggests that "a" is not necessary before "three" here

> + * regions for header, types, strings and optional kind layout). Also invalidate
> + * cached raw_data, if any.
>   */
>  static int btf_ensure_modifiable(struct btf *btf)
>  {
> -       void *hdr, *types;
> +       void *hdr, *types, *kind_layout = NULL;
>         struct strset *set = NULL;
>         int err = -ENOMEM;
>
> @@ -1446,9 +1501,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->kind_layout_off && btf->hdr->kind_layout_len) {
> +               kind_layout = calloc(1, btf->hdr->kind_layout_len);
> +               if (!kind_layout)
> +                       goto err_out;
> +       }
>
>         memcpy(hdr, btf->hdr, btf->hdr->hdr_len);
>         memcpy(types, btf->types_data, btf->hdr->type_len);
> +       if (kind_layout)
> +               memcpy(kind_layout, btf->kind_layout, btf->hdr->kind_layout_len);
>

let's just emit kind_layout always, why making it optional on writing
out new BTF?

why not make it always present internally in libbpf, and either read
it from BTF, if it's present, or created it from scratch based on
libbpf's version and knowledge of all the kinds? This will be simpler,
the only place where we'd need to handle it optionally is during
initialization


>         /* build lookup index for all strings */
>         set = strset__new(BTF_MAX_STR_OFFSET, btf->strs_data, btf->hdr->str_len);
> @@ -1463,6 +1526,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->kind_layout = kind_layout;
> +
>         /* if BTF was created from scratch, all strings are guaranteed to be
>          * unique and deduplicated
>          */
> @@ -1480,6 +1545,7 @@ static int btf_ensure_modifiable(struct btf *btf)
>         strset__free(set);
>         free(hdr);
>         free(types);
> +       free(kind_layout);
>         return err;
>  }
>
> --
> 2.39.3
>

Re: [PATCH v2 bpf-next 3/9] libbpf: use kind layout to compute an unknown kind size

[Andrii Nakryiko]

On Fri, Jun 16, 2023 at 10:18 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> This allows BTF parsing to proceed even if we do not know the
> kind.
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  tools/lib/bpf/btf.c | 41 ++++++++++++++++++++++++++++++++++-------
>  1 file changed, 34 insertions(+), 7 deletions(-)
>
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index f9f919fdc728..457997c2a43c 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -327,7 +327,35 @@ static int btf_parse_kind_layout_sec(struct btf *btf)
>         return 0;
>  }
>
> -static int btf_type_size(const struct btf_type *t)
> +/* for unknown kinds, consult kind layout. */
> +static int btf_type_size_unknown(const struct btf *btf, const struct btf_type *t)
> +{
> +       int size = sizeof(struct btf_type);
> +       struct btf_kind_layout *k = NULL;
> +       __u16 vlen = btf_vlen(t);
> +       __u8 kind = btf_kind(t);
> +
> +       if (btf->kind_layout)
> +               k = &btf->kind_layout[kind];

see my suggestion on the previous patch. Let's make this layout
information mandatory internally and always use it consistently.

simpler code, more testing for this new code path


> +
> +       if (!k || (void *)k > ((void *)btf->kind_layout + btf->hdr->kind_layout_len)) {
> +               pr_debug("Unsupported BTF_KIND: %u\n", btf_kind(t));
> +               return -EINVAL;
> +       }
> +
> +       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;
> +       }
> +
> +       size += k->info_sz;
> +       size += vlen * k->elem_sz;
> +
> +       return size;
> +}
> +
> +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);
>         }
>  }
>
> @@ -463,7 +490,7 @@ static int btf_parse_type_sec(struct btf *btf)
>                 if (btf->swapped_endian)
>                         btf_bswap_type_base(next_type);
>
> -               type_size = btf_type_size(next_type);
> +               type_size = btf_type_size(btf, next_type);
>                 if (type_size < 0)
>                         return type_size;
>                 if (next_type + type_size > end_type) {
> @@ -1672,7 +1699,7 @@ int btf__add_type(struct btf *btf, const struct btf *src_btf, const struct btf_t
>         struct btf_type *t;
>         int sz, err;
>
> -       sz = btf_type_size(src_type);
> +       sz = btf_type_size(src_btf, src_type);
>         if (sz < 0)
>                 return libbpf_err(sz);
>
> @@ -1753,7 +1780,7 @@ int btf__add_btf(struct btf *btf, const struct btf *src_btf)
>         memcpy(t, src_btf->types_data, data_sz);
>
>         for (i = 0; i < cnt; i++) {
> -               sz = btf_type_size(t);
> +               sz = btf_type_size(src_btf, t);
>                 if (sz < 0) {
>                         /* unlikely, has to be corrupted src_btf */
>                         err = sz;
> @@ -4749,7 +4776,7 @@ static int btf_dedup_compact_types(struct btf_dedup *d)
>                         continue;
>
>                 t = btf__type_by_id(d->btf, id);
> -               len = btf_type_size(t);
> +               len = btf_type_size(d->btf, t);
>                 if (len < 0)
>                         return len;
>
> --
> 2.39.3
>

Re: [PATCH v2 bpf-next 4/9] btf: support kernel parsing of BTF with kind layout

[Andrii Nakryiko]

On Fri, Jun 16, 2023 at 10:18 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> Use kind layout to parse BTF with unknown kinds that have a
> kind layout representation.
>
> Validate kind layout if present, and use it to parse BTF with
> unrecognized kinds. Reject BTF that contains a type
> of a kind that is not optional.
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  kernel/bpf/btf.c | 102 +++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 82 insertions(+), 20 deletions(-)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index bd2cac057928..ffe3926ea051 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -257,6 +257,7 @@ struct btf {
>         struct btf_kfunc_set_tab *kfunc_set_tab;
>         struct btf_id_dtor_kfunc_tab *dtor_kfunc_tab;
>         struct btf_struct_metas *struct_meta_tab;
> +       struct btf_kind_layout *kind_layout;
>
>         /* split BTF support */
>         struct btf *base_btf;
> @@ -4965,22 +4966,41 @@ static s32 btf_check_meta(struct btf_verifier_env *env,
>                 return -EINVAL;
>         }
>
> -       if (BTF_INFO_KIND(t->info) > BTF_KIND_MAX ||
> -           BTF_INFO_KIND(t->info) == BTF_KIND_UNKN) {
> -               btf_verifier_log(env, "[%u] Invalid kind:%u",
> -                                env->log_type_id, BTF_INFO_KIND(t->info));
> -               return -EINVAL;
> -       }
> -
>         if (!btf_name_offset_valid(env->btf, t->name_off)) {
>                 btf_verifier_log(env, "[%u] Invalid name_offset:%u",
>                                  env->log_type_id, t->name_off);
>                 return -EINVAL;
>         }
>
> -       var_meta_size = btf_type_ops(t)->check_meta(env, t, meta_left);
> -       if (var_meta_size < 0)
> -               return var_meta_size;
> +       if (BTF_INFO_KIND(t->info) == BTF_KIND_UNKN) {
> +               btf_verifier_log(env, "[%u] Invalid kind:%u",
> +                                env->log_type_id, BTF_INFO_KIND(t->info));
> +               return -EINVAL;
> +       }
> +
> +       if (BTF_INFO_KIND(t->info) > BTF_KIND_MAX && env->btf->kind_layout &&
> +           (BTF_INFO_KIND(t->info) * sizeof(struct btf_kind_layout)) <
> +            env->btf->hdr.kind_layout_len) {
> +               struct btf_kind_layout *k = &env->btf->kind_layout[BTF_INFO_KIND(t->info)];
> +
> +               if (!(k->flags & BTF_KIND_LAYOUT_OPTIONAL)) {

same question as on previous patch, should kernel trust and handle
OPTIONAL flag?

I'd say let's drop it for now, doesn't seem worth the trouble

> +                       btf_verifier_log(env, "[%u] unknown but required kind %u",
> +                                        env->log_type_id,
> +                                        BTF_INFO_KIND(t->info));
> +                       return -EINVAL;
> +               }
> +               var_meta_size = sizeof(struct btf_type);
> +               var_meta_size += k->info_sz + (btf_type_vlen(t) * k->elem_sz);
> +       } else {
> +               if (BTF_INFO_KIND(t->info) > BTF_KIND_MAX) {
> +                       btf_verifier_log(env, "[%u] Invalid kind:%u",
> +                                        env->log_type_id, BTF_INFO_KIND(t->info));
> +                       return -EINVAL;
> +               }
> +               var_meta_size = btf_type_ops(t)->check_meta(env, t, meta_left);
> +               if (var_meta_size < 0)
> +                       return var_meta_size;
> +       }
>
>         meta_left -= var_meta_size;
>
> @@ -5155,7 +5175,8 @@ static int btf_parse_str_sec(struct btf_verifier_env *env)
>         start = btf->nohdr_data + hdr->str_off;
>         end = start + hdr->str_len;
>
> -       if (end != btf->data + btf->data_size) {
> +       if (hdr->hdr_len < sizeof(struct btf_header) &&
> +           end != btf->data + btf->data_size) {
>                 btf_verifier_log(env, "String section is not at the end");
>                 return -EINVAL;
>         }
> @@ -5176,9 +5197,41 @@ static int btf_parse_str_sec(struct btf_verifier_env *env)
>         return 0;
>  }
>
> +static int btf_parse_kind_layout_sec(struct btf_verifier_env *env)
> +{
> +       const struct btf_header *hdr = &env->btf->hdr;
> +       struct btf *btf = env->btf;
> +       void *start, *end;
> +
> +       if (hdr->hdr_len < sizeof(struct btf_header) ||
> +           hdr->kind_layout_len == 0)

let's make sure that kind_layout_off is zero in this case as well

> +               return 0;
> +
> +       /* Kind layout section must align to 4 bytes */
> +       if (hdr->kind_layout_off & (sizeof(u32) - 1)) {
> +               btf_verifier_log(env, "Unaligned kind_layout_off");
> +               return -EINVAL;
> +       }
> +       start = btf->nohdr_data + hdr->kind_layout_off;
> +       end = start + hdr->kind_layout_len;
> +
> +       if (hdr->kind_layout_len < sizeof(struct btf_kind_layout)) {

same as on libbpf side, more generally kind_layout_len should be a
multiple of sizeof(struct btf_kind_layout)

> +               btf_verifier_log(env, "Kind layout section is too small");
> +               return -EINVAL;
> +       }
> +       if (end != btf->data + btf->data_size) {
> +               btf_verifier_log(env, "Kind layout section is not at the end");
> +               return -EINVAL;
> +       }
> +       btf->kind_layout = start;
> +
> +       return 0;
> +}
> +

[...]

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

[Andrii Nakryiko]

On Fri, Jun 16, 2023 at 10:18 AM Alan Maguire <alan.maguire@oracle.com> 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@oracle.com>
> ---
>  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
>

Re: [PATCH v2 bpf-next 7/9] bpftool: add BTF dump "format meta" to dump header/metadata

[Andrii Nakryiko]

On Fri, Jun 16, 2023 at 10:18 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> Provide a way to dump BTF metadata info via bpftool; this
> consists of BTF size, header fields and kind layout info
> (if available); for example
>
> $ bpftool btf dump file vmlinux format meta
> size 4966513
> magic 0xeb9f
> version 1
> flags 0x0
> hdr_len 24
> type_len 2929900
> type_off 0
> str_len 2036589
> str_off 2929900
>
> ...or for vmlinux with kind layout, crc:
>
> $ bpftool btf dump file vmlinux format meta
> size 5034496
> magic 0xeb9f
> version 1
> flags 0x1
> hdr_len 40
> type_len 2973628
> type_off 0
> str_len 2060745
> str_off 2973628
> kind_layout_len 80
> kind_layout_off 5034376
> crc 0xb6a5171f
> base_crc 0x0
> kind 0    flags 0x0    info_sz 0    elem_sz 0
> kind 1    flags 0x0    info_sz 4    elem_sz 0
> kind 2    flags 0x0    info_sz 0    elem_sz 0
> kind 3    flags 0x0    info_sz 12   elem_sz 0
> kind 4    flags 0x0    info_sz 0    elem_sz 12

well, bpftool will know symbolic names for most of these, so why not
emit them? I think it's fine to emit both, something like


kind INT (1)  flags ....
...

and for unknown:

kind <unknown> (123) flags ....


WDYT?

> ...
>
> JSON output is also supported:
>
> $ bpftool -j btf dump file vmlinux format meta
> {"size":4904369,{"header":"magic":60319,"version":1,"flags":0,"hdr_len":24,"type_len":2893508,"type_off":0,"str_len":2010837,"str_off":2893508}}
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  tools/bpf/bpftool/bash-completion/bpftool |  2 +-
>  tools/bpf/bpftool/btf.c                   | 93 ++++++++++++++++++++++-
>  2 files changed, 92 insertions(+), 3 deletions(-)
>

[...]

Re: [PATCH v2 bpf-next 9/9] selftests/bpf: test kind encoding/decoding

[Andrii Nakryiko]

On Fri, Jun 16, 2023 at 10:18 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> verify btf__new_empty_opts() adds kind layouts for all kinds supported,
> and after adding kind-related types for an unknown kind, ensure that
> parsing uses this info when that kind is encountered rather than
> giving up.  Also verify that presence of a required kind will fail
> parsing.
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  .../selftests/bpf/prog_tests/btf_kind.c       | 187 ++++++++++++++++++
>  1 file changed, 187 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/btf_kind.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/btf_kind.c b/tools/testing/selftests/bpf/prog_tests/btf_kind.c
> new file mode 100644
> index 000000000000..ff37126b6bc0
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/btf_kind.c
> @@ -0,0 +1,187 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2023, Oracle and/or its affiliates. */
> +
> +#include <test_progs.h>
> +#include <bpf/btf.h>
> +#include <bpf/libbpf.h>
> +
> +/* verify kind encoding exists for each kind */
> +void test_btf_kind_encoding(struct btf *btf)

static

> +{
> +       const struct btf_header *hdr;
> +       const void *raw_btf;
> +       __u32 raw_size;
> +
> +       raw_btf = btf__raw_data(btf, &raw_size);
> +       if (!ASSERT_OK_PTR(raw_btf, "btf__raw_data"))
> +               return;
> +
> +       hdr = raw_btf;
> +
> +       ASSERT_GT(hdr->kind_layout_off, hdr->str_off, "kind layout off");

check that it's multiple of 4 maybe?

> +       ASSERT_EQ(hdr->kind_layout_len, sizeof(struct btf_kind_layout) * NR_BTF_KINDS,
> +                 "kind_layout_len");
> +}
> +
> +static void write_raw_btf(const char *btf_path, void *raw_btf, size_t raw_size)
> +{
> +       int fd = open(btf_path, O_WRONLY | O_CREAT);
> +
> +       write(fd, raw_btf, raw_size);
> +       close(fd);
> +}

why bother with writing/reading to/from file, if you can just parse it
from memory with btf__new() ?

> +
> +/* fabricate an unrecognized kind at BTF_KIND_MAX + 1, and after adding
> + * the appropriate struct/typedefs to the BTF such that it recognizes
> + * this kind, ensure that parsing of BTF containing the unrecognized kind
> + * can succeed.
> + */
> +void test_btf_kind_decoding(struct btf *btf)
> +{
> +       __s32 int_id, unrec_id, id, id2;
> +       struct btf_type *t;
> +       char btf_path[64];
> +       const void *raw_btf;
> +       void *new_raw_btf;
> +       struct btf *new_btf;
> +       struct btf_header *hdr;
> +       struct btf_kind_layout *k;
> +       __u32 raw_size;
> +

[...]

> +
> +void test_btf_kind(void)
> +{
> +       LIBBPF_OPTS(btf_new_opts, opts);
> +
> +       opts.add_kind_layout = true;
> +
> +       struct btf *btf = btf__new_empty_opts(&opts);

are you trying to save 3 lines of code here but instead coupling
encoding/decoding subtests? Why? I had to go and check that there is
no expectation that test_btf_kind_encoding() has to be run first
before test_btf_kind_decoding(btf). Doesn't seem like there is, but
why doing this empty btf instantiation outside of each subtest? Keep
it simple, create empty btf inside the subtest as necessary.


> +
> +       if (!ASSERT_OK_PTR(btf, "btf_new"))
> +               return;
> +
> +       if (test__start_subtest("btf_kind_encoding"))
> +               test_btf_kind_encoding(btf);
> +       if (test__start_subtest("btf_kind_decoding"))
> +               test_btf_kind_decoding(btf);
> +       btf__free(btf);
> +}
> --
> 2.39.3
>

Re: [PATCH v2 bpf-next 7/9] bpftool: add BTF dump "format meta" to dump header/metadata

[Quentin Monnet]

Thanks Alan, and apologies for the late review!

2023-06-16 18:17 UTC+0100 ~ Alan Maguire <alan.maguire@oracle.com>
> Provide a way to dump BTF metadata info via bpftool; this
> consists of BTF size, header fields and kind layout info
> (if available); for example
> 
> $ bpftool btf dump file vmlinux format meta
> size 4966513   
> magic 0xeb9f      
> version 1         
> flags 0x0         
> hdr_len 24        
> type_len 2929900   
> type_off 0         
> str_len 2036589   
> str_off 2929900   
> 
> ...or for vmlinux with kind layout, crc:
> 
> $ bpftool btf dump file vmlinux format meta
> size 5034496   
> magic 0xeb9f      
> version 1         
> flags 0x1         
> hdr_len 40        
> type_len 2973628   
> type_off 0         
> str_len 2060745   
> str_off 2973628   
> kind_layout_len 80        
> kind_layout_off 5034376   
> crc 0xb6a5171f  
> base_crc 0x0         
> kind 0    flags 0x0    info_sz 0    elem_sz 0   
> kind 1    flags 0x0    info_sz 4    elem_sz 0   
> kind 2    flags 0x0    info_sz 0    elem_sz 0   
> kind 3    flags 0x0    info_sz 12   elem_sz 0   
> kind 4    flags 0x0    info_sz 0    elem_sz 12
> ...
> 
> JSON output is also supported:
> 
> $ bpftool -j btf dump file vmlinux format meta
> {"size":4904369,{"header":"magic":60319,"version":1,"flags":0,"hdr_len":24,"type_len":2893508,"type_off":0,"str_len":2010837,"str_off":2893508}}

This is not valid JSON. I suspect that instead of:

	{"size":4904369,{"header":"magic":60319, ...

you meant the following?:

	{"size":4904369,"header":{"magic":60319,

Could you please also provide a JSON example with the kind_layouts?

> 
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  tools/bpf/bpftool/bash-completion/bpftool |  2 +-
>  tools/bpf/bpftool/btf.c                   | 93 ++++++++++++++++++++++-
>  2 files changed, 92 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
> index 085bf18f3659..4c186d4efb35 100644
> --- a/tools/bpf/bpftool/bash-completion/bpftool
> +++ b/tools/bpf/bpftool/bash-completion/bpftool
> @@ -937,7 +937,7 @@ _bpftool()
>                              return 0
>                              ;;
>                          format)
> -                            COMPREPLY=( $( compgen -W "c raw" -- "$cur" ) )
> +                            COMPREPLY=( $( compgen -W "c raw meta" -- "$cur" ) )
>                              ;;
>                          *)
>                              # emit extra options
> diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
> index 91fcb75babe3..56f40adcc161 100644
> --- a/tools/bpf/bpftool/btf.c
> +++ b/tools/bpf/bpftool/btf.c
> @@ -504,6 +504,90 @@ static int dump_btf_c(const struct btf *btf,
>  	return err;
>  }
>  
> +static int dump_btf_meta(const struct btf *btf)
> +{
> +	const struct btf_header *hdr;
> +	const struct btf_kind_layout *k;
> +	const void *data;
> +	__u32 data_sz;
> +	__u8 i, nr_kinds;
> +
> +	data = btf__raw_data(btf, &data_sz);
> +	if (!data)
> +		return -ENOMEM;
> +	hdr = data;
> +	if (json_output) {
> +		jsonw_start_object(json_wtr);   /* btf metadata object */
> +		jsonw_uint_field(json_wtr, "size", data_sz);
> +		jsonw_start_object(json_wtr);

... /* header object */

> +		jsonw_name(json_wtr, "header");

I think we need to swap the above two lines to fix the JSON.

> +		jsonw_uint_field(json_wtr, "magic", hdr->magic);
> +		jsonw_uint_field(json_wtr, "version", hdr->version);
> +		jsonw_uint_field(json_wtr, "flags", hdr->flags);
> +		jsonw_uint_field(json_wtr, "hdr_len", hdr->hdr_len);
> +		jsonw_uint_field(json_wtr, "type_len", hdr->type_len);
> +		jsonw_uint_field(json_wtr, "type_off", hdr->type_off);
> +		jsonw_uint_field(json_wtr, "str_len", hdr->str_len);
> +		jsonw_uint_field(json_wtr, "str_off", hdr->str_off);
> +	} else {
> +		printf("size %-10d\n", data_sz);
> +		printf("magic 0x%-10x\nversion %-10d\nflags 0x%-10x\nhdr_len %-10d\n",
> +		       hdr->magic, hdr->version, hdr->flags, hdr->hdr_len);
> +		printf("type_len %-10d\ntype_off %-10d\n", hdr->type_len, hdr->type_off);
> +		printf("str_len %-10d\nstr_off %-10d\n", hdr->str_len, hdr->str_off);
> +	}
> +
> +	if (hdr->hdr_len < sizeof(struct btf_header) ||
> +	    hdr->kind_layout_len == 0 || hdr->kind_layout_len == 0) {
> +		if (json_output) {
> +			jsonw_end_object(json_wtr); /* header object */
> +			jsonw_end_object(json_wtr); /* metadata object */
> +		}
> +		return 0;
> +	}
> +
> +	if (json_output) {
> +		jsonw_uint_field(json_wtr, "kind_layout_len", hdr->kind_layout_len);
> +		jsonw_uint_field(json_wtr, "kind_layout_offset", hdr->kind_layout_off);
> +		jsonw_uint_field(json_wtr, "crc", hdr->crc);
> +		jsonw_uint_field(json_wtr, "base_crc", hdr->base_crc);
> +		jsonw_end_object(json_wtr); /* end header object */
> +
> +		jsonw_start_object(json_wtr);
> +		jsonw_name(json_wtr, "kind_layouts");

It seems to me we have the same JSON error here as for the header.

> +		jsonw_start_array(json_wtr);
> +	} else {
> +		printf("kind_layout_len %-10d\nkind_layout_off %-10d\n",
> +		       hdr->kind_layout_len, hdr->kind_layout_off);
> +		printf("crc 0x%-10x\nbase_crc 0x%-10x\n",
> +		       hdr->crc, hdr->base_crc);
> +	}
> +
> +	k = (void *)hdr + hdr->hdr_len + hdr->kind_layout_off;
> +	nr_kinds = hdr->kind_layout_len / sizeof(*k);
> +	for (i = 0; i < nr_kinds; i++) {
> +		if (json_output) {
> +			jsonw_start_object(json_wtr);
> +			jsonw_name(json_wtr, "kind_layout");

And here?

> +			jsonw_uint_field(json_wtr, "kind", i);
> +			jsonw_uint_field(json_wtr, "flags", k[i].flags);
> +			jsonw_uint_field(json_wtr, "info_sz", k[i].info_sz);
> +			jsonw_uint_field(json_wtr, "elem_sz", k[i].elem_sz);
> +			jsonw_end_object(json_wtr);
> +		} else {
> +			printf("kind %-4d flags 0x%-4x info_sz %-4d elem_sz %-4d\n",
> +			       i, k[i].flags, k[i].info_sz, k[i].elem_sz);
> +		}
> +	}
> +	if (json_output) {
> +		jsonw_end_array(json_wtr);
> +		jsonw_end_object(json_wtr); /* end kind layout */
> +		jsonw_end_object(json_wtr); /* end metadata object */
> +	}
> +
> +	return 0;
> +}
> +

There's a number of locations where we split between two functions for
JSON and plain output, for better clarity. So we could have
dump_btf_meta_plain() and dump_btf_meta_json(). I think it would be
easier to read, but don't feel strongly so also OK if you prefer to keep
the current form to avoid duplicating the logics.

Thanks for adding all those items I asked on v1!

Re: [PATCH v2 bpf-next 8/9] bpftool: update doc to describe bpftool btf dump .. format meta

[Quentin Monnet]

2023-06-16 18:17 UTC+0100 ~ Alan Maguire <alan.maguire@oracle.com>
> ...and provide an example of output.
> 
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  tools/bpf/bpftool/Documentation/bpftool-btf.rst | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-btf.rst b/tools/bpf/bpftool/Documentation/bpftool-btf.rst
> index 342716f74ec4..6dd779dddbde 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-btf.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-btf.rst
> @@ -28,7 +28,7 @@ BTF COMMANDS
>  |	**bpftool** **btf help**
>  |
>  |	*BTF_SRC* := { **id** *BTF_ID* | **prog** *PROG* | **map** *MAP* [{**key** | **value** | **kv** | **all**}] | **file** *FILE* }
> -|	*FORMAT* := { **raw** | **c** }
> +|	*FORMAT* := { **raw** | **c** | **meta** }
>  |	*MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
>  |	*PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* }
>  
> @@ -67,8 +67,8 @@ DESCRIPTION
>  		  typically produced by clang or pahole.
>  
>  		  **format** option can be used to override default (raw)
> -		  output format. Raw (**raw**) or C-syntax (**c**) output
> -		  formats are supported.
> +		  output format. Raw (**raw**), C-syntax (**c**) and
> +                  BTF metadata (**meta**) formats are supported.

Please fix the indent, we're using a mix of tabs then spaces here (I
should try to fix that one day).

Any chance we get a one-sentence description of the metadata format,
please? :)

>  
>  	**bpftool btf help**
>  		  Print short help message.
> @@ -266,3 +266,13 @@ All the standard ways to specify map or program are supported:
>    [104859] FUNC 'smbalert_work' type_id=9695 linkage=static
>    [104860] FUNC 'smbus_alert' type_id=71367 linkage=static
>    [104861] FUNC 'smbus_do_alert' type_id=84827 linkage=static
> +
> +
> + **# bpftool btf dump file vmlinux format meta**
> +
> + ::

Missing blank line here

> + size 4904369
> + magic 0xeb9f       version 1          flags 0x0          hdr_len 24
> + type      len 2893508    off 0
> + str       len 2010837    off 2893508
> +

And please fix the indent here as well, the double-column should have no
indent but the example itself should have some (other examples on the
page use two spaces here).

Thanks a lot for the doc, and adding the example!

Quentin

Re: [PATCH v2 bpf-next 1/9] btf: add kind layout encoding, crcs to UAPI

[Alan Maguire]

On 22/06/2023 23:02, Andrii Nakryiko wrote:
> On Fri, Jun 16, 2023 at 10:17 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>>
>> BTF kind layouts provide information to parse BTF kinds.
>> By separating parsing BTF from using all the information
>> it provides, we allow BTF to encode new features even if
>> they cannot be used.  This is helpful in particular for
>> cases where newer tools for BTF generation run on an
>> older kernel; BTF kinds may be present that the kernel
>> cannot yet use, but at least it can parse the BTF
>> provided.  Meanwhile userspace tools with newer libbpf
>> may be able to use the newer information.
>>
>> The intent is to support encoding of kind layouts
>> optionally so that tools like pahole can add this
>> information.  So for each kind we record
>>
>> - kind-related flags
>> - length of singular element following struct btf_type
>> - length of each of the btf_vlen() elements following
>>
>> In addition we make space in the BTF header for
>> CRC32s computed over the BTF along with a CRC for
>> the base BTF; this allows split BTF to identify
>> a mismatch explicitly.
>>
>> The ideas here were discussed at [1], [2]; hence
>>
>> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
>> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>>
>> [1] https://lore.kernel.org/bpf/CAEf4BzYjWHRdNNw4B=eOXOs_ONrDwrgX4bn=Nuc1g8JPFC34MA@mail.gmail.com/
>> [2] https://lore.kernel.org/bpf/20230531201936.1992188-1-alan.maguire@oracle.com/
>> ---
>>  include/uapi/linux/btf.h       | 24 ++++++++++++++++++++++++
>>  tools/include/uapi/linux/btf.h | 24 ++++++++++++++++++++++++
>>  2 files changed, 48 insertions(+)
>>
>> diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
>> index ec1798b6d3ff..cea9125ed953 100644
>> --- a/include/uapi/linux/btf.h
>> +++ b/include/uapi/linux/btf.h
>> @@ -8,6 +8,22 @@
>>  #define BTF_MAGIC      0xeB9F
>>  #define BTF_VERSION    1
>>
>> +/* is this information required? If so it cannot be sanitized safely. */
>> +#define BTF_KIND_LAYOUT_OPTIONAL               (1 << 0)
> 
> hm.. I thought we agreed to not have OPTIONAL flag last time, no? From
> kernel's perspective nothing is optional. From libbpf perspective
> everything should be optional, unless we get type_id reference to
> something that we don't recognize. So why the flag and extra code to
> handle it?
> 
> We can always add it later, if necessary.
>

I totally agree we need to reject any BTF that contains references
to unknown objects if these references are made via known ones;
so for example an enum64 in a struct (in the case we didn't know
about an enum64). However, it's possible a BTF kind could point
_at_ other BTF kinds but not be pointed _to_ by any known kinds;
in such a case wouldn't optional make sense even for the kernel
to say "ignore any kinds that we don't know about that aren't
participating in any known BTF relationships"? Default assumption
without the optional flag would be to reject such BTF.

>> +
>> +/* kind layout section consists of a struct btf_kind_layout for each known
>> + * kind at BTF encoding time.
>> + */
>> +struct btf_kind_layout {
>> +       __u16 flags;            /* see BTF_KIND_LAYOUT_* values above */
>> +       __u8 info_sz;           /* size of singular element after btf_type */
>> +       __u8 elem_sz;           /* size of each of btf_vlen(t) elements */
>> +};
>> +
>> +/* for CRCs for BTF, base BTF to be considered usable, flags must be set. */
>> +#define BTF_FLAG_CRC_SET               (1 << 0)
>> +#define BTF_FLAG_BASE_CRC_SET          (1 << 1)
>> +
>>  struct btf_header {
>>         __u16   magic;
>>         __u8    version;
>> @@ -19,8 +35,16 @@ struct btf_header {
>>         __u32   type_len;       /* length of type section       */
>>         __u32   str_off;        /* offset of string section     */
>>         __u32   str_len;        /* length of string section     */
>> +       __u32   kind_layout_off;/* offset of kind layout section */
>> +       __u32   kind_layout_len;/* length of kind layout section */
>> +
>> +       __u32   crc;            /* crc of BTF; used if flags set BTF_FLAG_CRC_VALID */
>> +       __u32   base_crc;       /* crc of base BTF; used if flags set BTF_FLAG_BASE_CRC_VALID */
>>  };
>>
>> +/* required minimum BTF header length */
>> +#define BTF_HEADER_MIN_LEN     (sizeof(struct btf_header) - 16)
> 
> offsetof(struct btf_header, kind_layout_off) ?
> 
> but actually why this needs to be a part of UAPI?
> 

no not really. I was trying to come up with a more elegant
way of differentiating between the old and new header formats
on the basis of size and eventually just gave up and added
a #define. It can absolutely be removed.

>> +
>>  /* Max # of type identifier */
>>  #define BTF_MAX_TYPE   0x000fffff
>>  /* Max offset into the string section */
>> diff --git a/tools/include/uapi/linux/btf.h b/tools/include/uapi/linux/btf.h
>> index ec1798b6d3ff..cea9125ed953 100644
>> --- a/tools/include/uapi/linux/btf.h
>> +++ b/tools/include/uapi/linux/btf.h
>> @@ -8,6 +8,22 @@
>>  #define BTF_MAGIC      0xeB9F
>>  #define BTF_VERSION    1
>>
>> +/* is this information required? If so it cannot be sanitized safely. */
>> +#define BTF_KIND_LAYOUT_OPTIONAL               (1 << 0)
>> +
>> +/* kind layout section consists of a struct btf_kind_layout for each known
>> + * kind at BTF encoding time.
>> + */
>> +struct btf_kind_layout {
>> +       __u16 flags;            /* see BTF_KIND_LAYOUT_* values above */
>> +       __u8 info_sz;           /* size of singular element after btf_type */
>> +       __u8 elem_sz;           /* size of each of btf_vlen(t) elements */
>> +};
>> +
>> +/* for CRCs for BTF, base BTF to be considered usable, flags must be set. */
>> +#define BTF_FLAG_CRC_SET               (1 << 0)
>> +#define BTF_FLAG_BASE_CRC_SET          (1 << 1)
>> +
>>  struct btf_header {
>>         __u16   magic;
>>         __u8    version;
>> @@ -19,8 +35,16 @@ struct btf_header {
>>         __u32   type_len;       /* length of type section       */
>>         __u32   str_off;        /* offset of string section     */
>>         __u32   str_len;        /* length of string section     */
>> +       __u32   kind_layout_off;/* offset of kind layout section */
>> +       __u32   kind_layout_len;/* length of kind layout section */
>> +
>> +       __u32   crc;            /* crc of BTF; used if flags set BTF_FLAG_CRC_VALID */
> 
> why are we making crc optional? shouldn't we just say that crc is
> always filled out?
> 

The approach I took was to have libbpf/pahole be flexible about
specification of crcs and kind layout; neither, one of these or both
are supported. When neither are specified we'll still generate
a larger header, but it will be zeros for the new fields so parseable
by older libbpf/kernel. I think we probably need to make it optional
for a while to support new pahole on older kernels.


>> +       __u32   base_crc;       /* crc of base BTF; used if flags set BTF_FLAG_BASE_CRC_VALID */
> 
> here it would be nice if we could just rely on zero meaning not set,
> but I suspect not everyone will be happy about this, as technically
> crc 0 is a valid crc :(
>

Right, I think we're stuck with the flags unfortunately.
Thanks for the review (and apologies for the belated response!)

Alan

> 
>>  };
>>
>> +/* required minimum BTF header length */
>> +#define BTF_HEADER_MIN_LEN     (sizeof(struct btf_header) - 16)
>> +
>>  /* Max # of type identifier */
>>  #define BTF_MAX_TYPE   0x000fffff
>>  /* Max offset into the string section */
>> --
>> 2.39.3
>>

Re: [PATCH v2 bpf-next 1/9] btf: add kind layout encoding, crcs to UAPI

[Andrii Nakryiko]

On Mon, Jul 3, 2023 at 6:42 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On 22/06/2023 23:02, Andrii Nakryiko wrote:
> > On Fri, Jun 16, 2023 at 10:17 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> >>
> >> BTF kind layouts provide information to parse BTF kinds.
> >> By separating parsing BTF from using all the information
> >> it provides, we allow BTF to encode new features even if
> >> they cannot be used.  This is helpful in particular for
> >> cases where newer tools for BTF generation run on an
> >> older kernel; BTF kinds may be present that the kernel
> >> cannot yet use, but at least it can parse the BTF
> >> provided.  Meanwhile userspace tools with newer libbpf
> >> may be able to use the newer information.
> >>
> >> The intent is to support encoding of kind layouts
> >> optionally so that tools like pahole can add this
> >> information.  So for each kind we record
> >>
> >> - kind-related flags
> >> - length of singular element following struct btf_type
> >> - length of each of the btf_vlen() elements following
> >>
> >> In addition we make space in the BTF header for
> >> CRC32s computed over the BTF along with a CRC for
> >> the base BTF; this allows split BTF to identify
> >> a mismatch explicitly.
> >>
> >> The ideas here were discussed at [1], [2]; hence
> >>
> >> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
> >> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> >>
> >> [1] https://lore.kernel.org/bpf/CAEf4BzYjWHRdNNw4B=eOXOs_ONrDwrgX4bn=Nuc1g8JPFC34MA@mail.gmail.com/
> >> [2] https://lore.kernel.org/bpf/20230531201936.1992188-1-alan.maguire@oracle.com/
> >> ---
> >>  include/uapi/linux/btf.h       | 24 ++++++++++++++++++++++++
> >>  tools/include/uapi/linux/btf.h | 24 ++++++++++++++++++++++++
> >>  2 files changed, 48 insertions(+)
> >>
> >> diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
> >> index ec1798b6d3ff..cea9125ed953 100644
> >> --- a/include/uapi/linux/btf.h
> >> +++ b/include/uapi/linux/btf.h
> >> @@ -8,6 +8,22 @@
> >>  #define BTF_MAGIC      0xeB9F
> >>  #define BTF_VERSION    1
> >>
> >> +/* is this information required? If so it cannot be sanitized safely. */
> >> +#define BTF_KIND_LAYOUT_OPTIONAL               (1 << 0)
> >
> > hm.. I thought we agreed to not have OPTIONAL flag last time, no? From
> > kernel's perspective nothing is optional. From libbpf perspective
> > everything should be optional, unless we get type_id reference to
> > something that we don't recognize. So why the flag and extra code to
> > handle it?
> >
> > We can always add it later, if necessary.
> >
>
> I totally agree we need to reject any BTF that contains references
> to unknown objects if these references are made via known ones;
> so for example an enum64 in a struct (in the case we didn't know
> about an enum64). However, it's possible a BTF kind could point
> _at_ other BTF kinds but not be pointed _to_ by any known kinds;
> in such a case wouldn't optional make sense even for the kernel
> to say "ignore any kinds that we don't know about that aren't
> participating in any known BTF relationships"? Default assumption
> without the optional flag would be to reject such BTF.

I think it's simpler (and would follow what we've been doing with
kernel-side strict validation of everything) to reject everything
unknown. "Being pointed to" isn't always contained within BTF itself.
E.g., for line and func info, type_id comes during BPF_PROG_LOAD. So
at the point of BTF validation you don't know that some FUNCs will be
pointed to (as an example). So I'd avoid making unnecessarily
dangerous assumptions that some pieces of information can be ignored.

And in general, kernel doesn't trust user-space data without
validation, so we'd have to double-check all this OPTIONAL flags
somehow anyways.

>
> >> +
> >> +/* kind layout section consists of a struct btf_kind_layout for each known
> >> + * kind at BTF encoding time.
> >> + */
> >> +struct btf_kind_layout {
> >> +       __u16 flags;            /* see BTF_KIND_LAYOUT_* values above */
> >> +       __u8 info_sz;           /* size of singular element after btf_type */
> >> +       __u8 elem_sz;           /* size of each of btf_vlen(t) elements */
> >> +};
> >> +
> >> +/* for CRCs for BTF, base BTF to be considered usable, flags must be set. */
> >> +#define BTF_FLAG_CRC_SET               (1 << 0)
> >> +#define BTF_FLAG_BASE_CRC_SET          (1 << 1)
> >> +
> >>  struct btf_header {
> >>         __u16   magic;
> >>         __u8    version;
> >> @@ -19,8 +35,16 @@ struct btf_header {
> >>         __u32   type_len;       /* length of type section       */
> >>         __u32   str_off;        /* offset of string section     */
> >>         __u32   str_len;        /* length of string section     */
> >> +       __u32   kind_layout_off;/* offset of kind layout section */
> >> +       __u32   kind_layout_len;/* length of kind layout section */
> >> +
> >> +       __u32   crc;            /* crc of BTF; used if flags set BTF_FLAG_CRC_VALID */
> >> +       __u32   base_crc;       /* crc of base BTF; used if flags set BTF_FLAG_BASE_CRC_VALID */
> >>  };
> >>
> >> +/* required minimum BTF header length */
> >> +#define BTF_HEADER_MIN_LEN     (sizeof(struct btf_header) - 16)
> >
> > offsetof(struct btf_header, kind_layout_off) ?
> >
> > but actually why this needs to be a part of UAPI?
> >
>
> no not really. I was trying to come up with a more elegant
> way of differentiating between the old and new header formats
> on the basis of size and eventually just gave up and added
> a #define. It can absolutely be removed.

right, so that's why just checking if field is present based on
btf_header.len and field offset is a good approach? Let's drop
unnecessary constants from UAPI header

>
> >> +
> >>  /* Max # of type identifier */
> >>  #define BTF_MAX_TYPE   0x000fffff
> >>  /* Max offset into the string section */
> >> diff --git a/tools/include/uapi/linux/btf.h b/tools/include/uapi/linux/btf.h
> >> index ec1798b6d3ff..cea9125ed953 100644
> >> --- a/tools/include/uapi/linux/btf.h
> >> +++ b/tools/include/uapi/linux/btf.h
> >> @@ -8,6 +8,22 @@
> >>  #define BTF_MAGIC      0xeB9F
> >>  #define BTF_VERSION    1
> >>
> >> +/* is this information required? If so it cannot be sanitized safely. */
> >> +#define BTF_KIND_LAYOUT_OPTIONAL               (1 << 0)
> >> +
> >> +/* kind layout section consists of a struct btf_kind_layout for each known
> >> + * kind at BTF encoding time.
> >> + */
> >> +struct btf_kind_layout {
> >> +       __u16 flags;            /* see BTF_KIND_LAYOUT_* values above */
> >> +       __u8 info_sz;           /* size of singular element after btf_type */
> >> +       __u8 elem_sz;           /* size of each of btf_vlen(t) elements */
> >> +};
> >> +
> >> +/* for CRCs for BTF, base BTF to be considered usable, flags must be set. */
> >> +#define BTF_FLAG_CRC_SET               (1 << 0)
> >> +#define BTF_FLAG_BASE_CRC_SET          (1 << 1)
> >> +
> >>  struct btf_header {
> >>         __u16   magic;
> >>         __u8    version;
> >> @@ -19,8 +35,16 @@ struct btf_header {
> >>         __u32   type_len;       /* length of type section       */
> >>         __u32   str_off;        /* offset of string section     */
> >>         __u32   str_len;        /* length of string section     */
> >> +       __u32   kind_layout_off;/* offset of kind layout section */
> >> +       __u32   kind_layout_len;/* length of kind layout section */
> >> +
> >> +       __u32   crc;            /* crc of BTF; used if flags set BTF_FLAG_CRC_VALID */
> >
> > why are we making crc optional? shouldn't we just say that crc is
> > always filled out?
> >
>
> The approach I took was to have libbpf/pahole be flexible about
> specification of crcs and kind layout; neither, one of these or both
> are supported. When neither are specified we'll still generate
> a larger header, but it will be zeros for the new fields so parseable
> by older libbpf/kernel. I think we probably need to make it optional
> for a while to support new pahole on older kernels.

I'm not sure how this "optional for a while" will turn to
"non-optional", tbh, and who and when will decide that. I think the
"new pahole on old kernel" problem is solvable easily by making all
this new stuff opt-in. New kernel Makefiles will request pahole to
emit them, if pahole is new enough. Old kernels won't know about this
feature and even new pahole won't emit it.

I don't feel too strongly about it, just generally feeling like
minimizing all the different supportable variations.

>
>
> >> +       __u32   base_crc;       /* crc of base BTF; used if flags set BTF_FLAG_BASE_CRC_VALID */
> >
> > here it would be nice if we could just rely on zero meaning not set,
> > but I suspect not everyone will be happy about this, as technically
> > crc 0 is a valid crc :(
> >
>
> Right, I think we're stuck with the flags unfortunately.

yep. one extra reason why I like the idea of this being string offset,
but whatever, small thing


> Thanks for the review (and apologies for the belated response!)
>
> Alan
>
> >
> >>  };
> >>
> >> +/* required minimum BTF header length */
> >> +#define BTF_HEADER_MIN_LEN     (sizeof(struct btf_header) - 16)
> >> +
> >>  /* Max # of type identifier */
> >>  #define BTF_MAX_TYPE   0x000fffff
> >>  /* Max offset into the string section */
> >> --
> >> 2.39.3
> >>

Re: [PATCH v2 bpf-next 1/9] btf: add kind layout encoding, crcs to UAPI

[Alan Maguire]

On 06/07/2023 00:48, Andrii Nakryiko wrote:
> On Mon, Jul 3, 2023 at 6:42 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>>
>> On 22/06/2023 23:02, Andrii Nakryiko wrote:
>>> On Fri, Jun 16, 2023 at 10:17 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>>>>
>>>> BTF kind layouts provide information to parse BTF kinds.
>>>> By separating parsing BTF from using all the information
>>>> it provides, we allow BTF to encode new features even if
>>>> they cannot be used.  This is helpful in particular for
>>>> cases where newer tools for BTF generation run on an
>>>> older kernel; BTF kinds may be present that the kernel
>>>> cannot yet use, but at least it can parse the BTF
>>>> provided.  Meanwhile userspace tools with newer libbpf
>>>> may be able to use the newer information.
>>>>
>>>> The intent is to support encoding of kind layouts
>>>> optionally so that tools like pahole can add this
>>>> information.  So for each kind we record
>>>>
>>>> - kind-related flags
>>>> - length of singular element following struct btf_type
>>>> - length of each of the btf_vlen() elements following
>>>>
>>>> In addition we make space in the BTF header for
>>>> CRC32s computed over the BTF along with a CRC for
>>>> the base BTF; this allows split BTF to identify
>>>> a mismatch explicitly.
>>>>
>>>> The ideas here were discussed at [1], [2]; hence
>>>>
>>>> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
>>>> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>>>>
>>>> [1] https://lore.kernel.org/bpf/CAEf4BzYjWHRdNNw4B=eOXOs_ONrDwrgX4bn=Nuc1g8JPFC34MA@mail.gmail.com/
>>>> [2] https://lore.kernel.org/bpf/20230531201936.1992188-1-alan.maguire@oracle.com/
>>>> ---
>>>>  include/uapi/linux/btf.h       | 24 ++++++++++++++++++++++++
>>>>  tools/include/uapi/linux/btf.h | 24 ++++++++++++++++++++++++
>>>>  2 files changed, 48 insertions(+)
>>>>
>>>> diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
>>>> index ec1798b6d3ff..cea9125ed953 100644
>>>> --- a/include/uapi/linux/btf.h
>>>> +++ b/include/uapi/linux/btf.h
>>>> @@ -8,6 +8,22 @@
>>>>  #define BTF_MAGIC      0xeB9F
>>>>  #define BTF_VERSION    1
>>>>
>>>> +/* is this information required? If so it cannot be sanitized safely. */
>>>> +#define BTF_KIND_LAYOUT_OPTIONAL               (1 << 0)
>>>
>>> hm.. I thought we agreed to not have OPTIONAL flag last time, no? From
>>> kernel's perspective nothing is optional. From libbpf perspective
>>> everything should be optional, unless we get type_id reference to
>>> something that we don't recognize. So why the flag and extra code to
>>> handle it?
>>>
>>> We can always add it later, if necessary.
>>>
>>
>> I totally agree we need to reject any BTF that contains references
>> to unknown objects if these references are made via known ones;
>> so for example an enum64 in a struct (in the case we didn't know
>> about an enum64). However, it's possible a BTF kind could point
>> _at_ other BTF kinds but not be pointed _to_ by any known kinds;
>> in such a case wouldn't optional make sense even for the kernel
>> to say "ignore any kinds that we don't know about that aren't
>> participating in any known BTF relationships"? Default assumption
>> without the optional flag would be to reject such BTF.
> 
> I think it's simpler (and would follow what we've been doing with
> kernel-side strict validation of everything) to reject everything
> unknown. "Being pointed to" isn't always contained within BTF itself.
> E.g., for line and func info, type_id comes during BPF_PROG_LOAD. So
> at the point of BTF validation you don't know that some FUNCs will be
> pointed to (as an example). So I'd avoid making unnecessarily
> dangerous assumptions that some pieces of information can be ignored.
> 
> And in general, kernel doesn't trust user-space data without
> validation, so we'd have to double-check all this OPTIONAL flagsole
> somehow anyways.
> 

makes sense. I think I'd been hoping the BTF kind layout would help
address the "newer pahole runs on older kernel" problem, but it
doesn't really. More on that below...

>>
>>>> +
>>>> +/* kind layout section consists of a struct btf_kind_layout for each known
>>>> + * kind at BTF encoding time.
>>>> + */
>>>> +struct btf_kind_layout {
>>>> +       __u16 flags;            /* see BTF_KIND_LAYOUT_* values above */
>>>> +       __u8 info_sz;           /* size of singular element after btf_type */
>>>> +       __u8 elem_sz;           /* size of each of btf_vlen(t) elements */
>>>> +};
>>>> +
>>>> +/* for CRCs for BTF, base BTF to be considered usable, flags must be set. */
>>>> +#define BTF_FLAG_CRC_SET               (1 << 0)
>>>> +#define BTF_FLAG_BASE_CRC_SET          (1 << 1)
>>>> +
>>>>  struct btf_header {
>>>>         __u16   magic;
>>>>         __u8    version;
>>>> @@ -19,8 +35,16 @@ struct btf_header {
>>>>         __u32   type_len;       /* length of type section       */
>>>>         __u32   str_off;        /* offset of string section     */
>>>>         __u32   str_len;        /* length of string section     */
>>>> +       __u32   kind_layout_off;/* offset of kind layout section */
>>>> +       __u32   kind_layout_len;/* length of kind layout section */
>>>> +
>>>> +       __u32   crc;            /* crc of BTF; used if flags set BTF_FLAG_CRC_VALID */
>>>> +       __u32   base_crc;       /* crc of base BTF; used if flags set BTF_FLAG_BASE_CRC_VALID */
>>>>  };
>>>>
>>>> +/* required minimum BTF header length */
>>>> +#define BTF_HEADER_MIN_LEN     (sizeof(struct btf_header) - 16)
>>>
>>> offsetof(struct btf_header, kind_layout_off) ?
>>>
>>> but actually why this needs to be a part of UAPI?
>>>
>>
>> no not really. I was trying to come up with a more elegant
>> way of differentiating between the old and new header formats
>> on the basis of size and eventually just gave up and added
>> a #define. It can absolutely be removed.
> 
> right, so that's why just checking if field is present based on
> btf_header.len and field offset is a good approach? Let's drop
> unnecessary constants from UAPI header
> 
>>
>>>> +
>>>>  /* Max # of type identifier */
>>>>  #define BTF_MAX_TYPE   0x000fffff
>>>>  /* Max offset into the string section */
>>>> diff --git a/tools/include/uapi/linux/btf.h b/tools/include/uapi/linux/btf.h
>>>> index ec1798b6d3ff..cea9125ed953 100644
>>>> --- a/tools/include/uapi/linux/btf.h
>>>> +++ b/tools/include/uapi/linux/btf.h
>>>> @@ -8,6 +8,22 @@
>>>>  #define BTF_MAGIC      0xeB9F
>>>>  #define BTF_VERSION    1
>>>>
>>>> +/* is this information required? If so it cannot be sanitized safely. */
>>>> +#define BTF_KIND_LAYOUT_OPTIONAL               (1 << 0)
>>>> +
>>>> +/* kind layout section consists of a struct btf_kind_layout for each known
>>>> + * kind at BTF encoding time.
>>>> + */
>>>> +struct btf_kind_layout {
>>>> +       __u16 flags;            /* see BTF_KIND_LAYOUT_* values above */
>>>> +       __u8 info_sz;           /* size of singular element after btf_type */
>>>> +       __u8 elem_sz;           /* size of each of btf_vlen(t) elements */
>>>> +};
>>>> +
>>>> +/* for CRCs for BTF, base BTF to be considered usable, flags must be set. */
>>>> +#define BTF_FLAG_CRC_SET               (1 << 0)
>>>> +#define BTF_FLAG_BASE_CRC_SET          (1 << 1)
>>>> +
>>>>  struct btf_header {
>>>>         __u16   magic;
>>>>         __u8    version;
>>>> @@ -19,8 +35,16 @@ struct btf_header {
>>>>         __u32   type_len;       /* length of type section       */
>>>>         __u32   str_off;        /* offset of string section     */
>>>>         __u32   str_len;        /* length of string section     */
>>>> +       __u32   kind_layout_off;/* offset of kind layout section */
>>>> +       __u32   kind_layout_len;/* length of kind layout section */
>>>> +
>>>> +       __u32   crc;            /* crc of BTF; used if flags set BTF_FLAG_CRC_VALID */
>>>
>>> why are we making crc optional? shouldn't we just say that crc is
>>> always filled out?
>>>
>>
>> The approach I took was to have libbpf/pahole be flexible about
>> specification of crcs and kind layout; neither, one of these or both
>> are supported. When neither are specified we'll still generate
>> a larger header, but it will be zeros for the new fields so parseable
>> by older libbpf/kernel. I think we probably need to make it optional
>> for a while to support new pahole on older kernels.
> 
> I'm not sure how this "optional for a while" will turn to
> "non-optional", tbh, and who and when will decide that. I think the
> "new pahole on old kernel" problem is solvable easily by making all
> this new stuff opt-in. New kernel Makefiles will request pahole to
> emit them, if pahole is new enough. Old kernels won't know about this
> feature and even new pahole won't emit it.
>

Another approach would be if we could auto-detect the kinds supported
by an older kernel, and not emit anything > BTF_KIND_MAX for that
kernel. I've put together a proof-of-concept for that, see [1].

> I don't feel too strongly about it, just generally feeling like
> minimizing all the different supportable variations.
> 

Yeah, hopefully some of these options can go away eventually.
The auto-detection scheme in [1], or something like it, might
make things easier in future. Thanks!

Alan

[1]
https://lore.kernel.org/bpf/20230720201443.224040-1-alan.maguire@oracle.com/
>>
>>
>>>> +       __u32   base_crc;       /* crc of base BTF; used if flags set BTF_FLAG_BASE_CRC_VALID */
>>>
>>> here it would be nice if we could just rely on zero meaning not set,
>>> but I suspect not everyone will be happy about this, as technically
>>> crc 0 is a valid crc :(
>>>
>>
>> Right, I think we're stuck with the flags unfortunately.
> 
> yep. one extra reason why I like the idea of this being string offset,
> but whatever, small thing
> 
> 
>> Thanks for the review (and apologies for the belated response!)
>>
>> Alan
>>
>>>
>>>>  };
>>>>
>>>> +/* required minimum BTF header length */
>>>> +#define BTF_HEADER_MIN_LEN     (sizeof(struct btf_header) - 16)
>>>> +
>>>>  /* Max # of type identifier */
>>>>  #define BTF_MAX_TYPE   0x000fffff
>>>>  /* Max offset into the string section */
>>>> --
>>>> 2.39.3
>>>>
>