* [PATCH v4 bpf-next 01/17] btf: add kind layout encoding, crcs to UAPI
2023-11-12 12:48 [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF Alan Maguire
@ 2023-11-12 12:48 ` Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 02/17] libbpf: support kind layout section handling in BTF Alan Maguire
` (16 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Alan Maguire @ 2023-11-12 12:48 UTC (permalink / raw)
To: ast, daniel, andrii, jolsa
Cc: quentin, eddyz87, martin.lau, song, yonghong.song, john.fastabend,
kpsingh, sdf, haoluo, masahiroy, bpf, 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 | 18 ++++++++++++++++++
tools/include/uapi/linux/btf.h | 18 ++++++++++++++++++
2 files changed, 36 insertions(+)
diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
index ec1798b6d3ff..5cc8cc793cd2 100644
--- a/include/uapi/linux/btf.h
+++ b/include/uapi/linux/btf.h
@@ -8,6 +8,19 @@
#define BTF_MAGIC 0xeB9F
#define BTF_VERSION 1
+/* kind layout section consists of a struct btf_kind_layout for each known
+ * kind at BTF encoding time.
+ */
+struct btf_kind_layout {
+ __u16 flags; /* currently unused */
+ __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,6 +32,11 @@ 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_SET */
+ __u32 base_crc; /* crc of base BTF; used if flags set BTF_FLAG_BASE_CRC_SET */
};
/* Max # of type identifier */
diff --git a/tools/include/uapi/linux/btf.h b/tools/include/uapi/linux/btf.h
index ec1798b6d3ff..5cc8cc793cd2 100644
--- a/tools/include/uapi/linux/btf.h
+++ b/tools/include/uapi/linux/btf.h
@@ -8,6 +8,19 @@
#define BTF_MAGIC 0xeB9F
#define BTF_VERSION 1
+/* kind layout section consists of a struct btf_kind_layout for each known
+ * kind at BTF encoding time.
+ */
+struct btf_kind_layout {
+ __u16 flags; /* currently unused */
+ __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,6 +32,11 @@ 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_SET */
+ __u32 base_crc; /* crc of base BTF; used if flags set BTF_FLAG_BASE_CRC_SET */
};
/* Max # of type identifier */
--
2.31.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 bpf-next 02/17] libbpf: support kind layout section handling in BTF
2023-11-12 12:48 [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 01/17] btf: add kind layout encoding, crcs to UAPI Alan Maguire
@ 2023-11-12 12:48 ` Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 03/17] libbpf: use kind layout to compute an unknown kind size Alan Maguire
` (15 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Alan Maguire @ 2023-11-12 12:48 UTC (permalink / raw)
To: ast, daniel, andrii, jolsa
Cc: quentin, eddyz87, martin.lau, song, yonghong.song, john.fastabend,
kpsingh, sdf, haoluo, masahiroy, bpf, 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.
As part of this, we need to consider multiple valid BTF header
sizes; the original or the kind layout/CRC-extended headers.
So to support this, the "struct btf" representation is modified
to always allocate a "struct btf_header" and copy the valid
portion from the raw data to it; this means we can always safely
check fields like btf->hdr->crc or btf->hdr->kind_layout_len.
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
tools/lib/bpf/btf.c | 218 +++++++++++++++++++++++++++++++-------------
1 file changed, 157 insertions(+), 61 deletions(-)
diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index ee95fd379d4d..1d043fe49d4c 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -39,42 +39,53 @@ 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
+ * in a contiguous memory block. The type_data, and, strs_data
* point inside that memory region to their respective parts of BTF
* representation:
*
- * +--------------------------------+
- * | Header | Types | Strings |
- * +--------------------------------+
- * ^ ^ ^
- * | | |
- * hdr | |
- * types_data-+ |
- * strs_data------------+
+ * +--------------------------------+---------------------+
+ * | Header | Types | Strings |Optional kind layout |
+ * +--------------------------------+---------------------+
+ * ^ ^ ^ ^
+ * | | | |
+ * raw_data | | |
+ * types_data-+ | |
+ * strs_data------------+ |
+ * kind_layout----------------------+
+ *
+ * A separate struct btf_header is allocated for btf->hdr,
+ * and header information is copied into it. This allows us
+ * to handle header data for various header formats; the original,
+ * the extended header with CRCs/kind layout, etc.
*
* 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 four independent memory
+ * regions.
+ *
* 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 optionally kind_layout. kind_layout optionally points to a
+ * kind_layout array - this allows us to encode information about
+ * the kinds known at encoding time. If kind_layout is NULL no
+ * kind information is encoded.
*
- * +----------+ +---------+ +-----------+
- * | 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 | Optional kind layout|
+ * raw_data----->+----------+---------+-----------+---------------------+
*/
- struct btf_header *hdr;
+ struct btf_header *hdr; /* separately-allocated header data */
void *types_data;
size_t types_data_cap; /* used size stored in hdr->type_len */
@@ -116,6 +127,14 @@ struct btf {
/* whether strings are already deduplicated */
bool strs_deduped;
+ /* Points either at raw kind layout data in parsed BTF (if present), or
+ * at an allocated kind layout array when BTF is modifiable.
+ */
+ void *kind_layout;
+
+ /* is BTF modifiable? i.e. is it split into separate sections as described above? */
+ bool modifiable;
+
/* BTF object FD, if loaded into kernel */
int fd;
@@ -207,7 +226,7 @@ static int btf_add_type_idx_entry(struct btf *btf, __u32 type_off)
return 0;
}
-static void btf_bswap_hdr(struct btf_header *h)
+static void btf_bswap_hdr(struct btf_header *h, __u32 hdr_len)
{
h->magic = bswap_16(h->magic);
h->hdr_len = bswap_32(h->hdr_len);
@@ -215,50 +234,70 @@ 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);
+ /* May be operating on raw data with hdr_len that does not include below fields */
+ if (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)
{
- struct btf_header *hdr = btf->hdr;
+ struct btf_header *hdr = btf->raw_data;
+ __u32 hdr_len = hdr->hdr_len;
__u32 meta_left;
- if (btf->raw_size < sizeof(struct btf_header)) {
+ if (btf->raw_size < offsetofend(struct btf_header, str_len)) {
pr_debug("BTF header not found\n");
return -EINVAL;
}
if (hdr->magic == bswap_16(BTF_MAGIC)) {
btf->swapped_endian = true;
- if (bswap_32(hdr->hdr_len) != sizeof(struct btf_header)) {
+ hdr_len = bswap_32(hdr->hdr_len);
+ if (hdr_len < offsetofend(struct btf_header, str_len)) {
pr_warn("Can't load BTF with non-native endianness due to unsupported header length %u\n",
- bswap_32(hdr->hdr_len));
+ hdr_len);
return -ENOTSUP;
}
- btf_bswap_hdr(hdr);
} else if (hdr->magic != BTF_MAGIC) {
pr_debug("Invalid BTF magic: %x\n", hdr->magic);
return -EINVAL;
}
- if (btf->raw_size < hdr->hdr_len) {
+ if (btf->raw_size < hdr_len) {
pr_debug("BTF header len %u larger than data size %u\n",
- hdr->hdr_len, btf->raw_size);
+ hdr_len, btf->raw_size);
return -EINVAL;
}
- meta_left = btf->raw_size - hdr->hdr_len;
- if (meta_left < (long long)hdr->str_off + hdr->str_len) {
+ /* At this point, we have basic header information, so allocate btf->hdr */
+ btf->hdr = calloc(1, sizeof(struct btf_header));
+ if (!btf->hdr) {
+ pr_debug("BTF header allocation failed\n");
+ return -ENOMEM;
+ }
+ if (btf->swapped_endian)
+ btf_bswap_hdr(hdr, hdr_len);
+ memcpy(btf->hdr, hdr, hdr_len < sizeof(struct btf_header) ? hdr_len :
+ sizeof(struct btf_header));
+
+ meta_left = btf->raw_size - hdr_len;
+ if (meta_left < (long long)btf->hdr->str_off + btf->hdr->str_len) {
pr_debug("Invalid BTF total size: %u\n", btf->raw_size);
return -EINVAL;
}
- if ((long long)hdr->type_off + hdr->type_len > hdr->str_off) {
+ if ((long long)btf->hdr->type_off + btf->hdr->type_len > btf->hdr->str_off) {
pr_debug("Invalid BTF data sections layout: type data at %u + %u, strings data at %u + %u\n",
- hdr->type_off, hdr->type_len, hdr->str_off, hdr->str_len);
+ btf->hdr->type_off, btf->hdr->type_len, btf->hdr->str_off,
+ btf->hdr->str_len);
return -EINVAL;
}
- if (hdr->type_off % 4) {
+ if (btf->hdr->type_off % 4) {
pr_debug("BTF type section is not aligned to 4 bytes\n");
return -EINVAL;
}
@@ -285,6 +324,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->kind_layout_off || !hdr->kind_layout_len)
+ return 0;
+
+ if (hdr->kind_layout_len % sizeof(struct btf_kind_layout) != 0) {
+ 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);
@@ -944,7 +1009,8 @@ __s32 btf__find_by_name_kind(const struct btf *btf, const char *type_name,
static bool btf_is_modifiable(const struct btf *btf)
{
- return (void *)btf->hdr != btf->raw_data;
+ /* BTF is modifiable if split into multiple sections */
+ return btf->modifiable;
}
void btf__free(struct btf *btf)
@@ -961,12 +1027,16 @@ void btf__free(struct btf *btf)
* sections, so we need to free all of them individually. It
* might still have a cached contiguous raw data present,
* which will be unconditionally freed below.
+ *
+ * Optional kind layout information may be present too.
*/
- free(btf->hdr);
free(btf->types_data);
strset__free(btf->strs_set);
+ free(btf->kind_layout);
}
free(btf->raw_data);
+ if (btf->hdr != btf->raw_data)
+ free(btf->hdr);
free(btf->raw_data_swapped);
free(btf->type_offs);
free(btf);
@@ -974,6 +1044,7 @@ void btf__free(struct btf *btf)
static struct btf *btf_new_empty(struct btf *base_btf)
{
+ struct btf_header *hdr;
struct btf *btf;
btf = calloc(1, sizeof(*btf));
@@ -1001,14 +1072,20 @@ static struct btf *btf_new_empty(struct btf *base_btf)
return ERR_PTR(-ENOMEM);
}
- btf->hdr = btf->raw_data;
- btf->hdr->hdr_len = sizeof(struct btf_header);
- btf->hdr->magic = BTF_MAGIC;
- btf->hdr->version = BTF_VERSION;
+ hdr = btf->raw_data;
+ hdr->hdr_len = sizeof(struct btf_header);
+ hdr->magic = BTF_MAGIC;
+ hdr->version = BTF_VERSION;
- btf->types_data = btf->raw_data + btf->hdr->hdr_len;
- btf->strs_data = btf->raw_data + btf->hdr->hdr_len;
- btf->hdr->str_len = base_btf ? 0 : 1; /* empty string at offset 0 */
+ btf->types_data = btf->raw_data + hdr->hdr_len;
+ btf->strs_data = btf->raw_data + hdr->hdr_len;
+ hdr->str_len = base_btf ? 0 : 1; /* empty string at offset 0 */
+ btf->hdr = calloc(1, sizeof(struct btf_header));
+ if (!btf->hdr) {
+ free(btf);
+ return ERR_PTR(-ENOMEM);
+ }
+ memcpy(btf->hdr, hdr, sizeof(*hdr));
return btf;
}
@@ -1051,7 +1128,6 @@ static struct btf *btf_new(const void *data, __u32 size, struct btf *base_btf)
memcpy(btf->raw_data, data, size);
btf->raw_size = size;
- btf->hdr = btf->raw_data;
err = btf_parse_hdr(btf);
if (err)
goto done;
@@ -1060,6 +1136,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);
err = err ?: btf_sanity_check(btf);
if (err)
@@ -1427,6 +1504,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;
@@ -1434,7 +1516,7 @@ static void *btf_get_raw_data(const struct btf *btf, __u32 *size, bool swap_endi
memcpy(p, hdr, hdr->hdr_len);
if (swap_endian)
- btf_bswap_hdr(p);
+ btf_bswap_hdr(p, hdr->hdr_len);
p += hdr->hdr_len;
memcpy(p, btf->types_data, hdr->type_len);
@@ -1453,7 +1535,13 @@ 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;
+
+ if (btf->kind_layout) {
+ p = data + hdr->hdr_len + hdr->kind_layout_off;
+ memcpy(p, btf->kind_layout, hdr->kind_layout_len);
+ if (swap_endian)
+ btf_bswap_kind_layout_sec(p, hdr->kind_layout_len);
+ }
*size = data_sz;
return data;
@@ -1585,13 +1673,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 memory regions
+ * for types and strings, with kind layout section if needed (btf->hdr
+ * is already a separate region). Also invalidate cached raw_data, if any.
*/
static int btf_ensure_modifiable(struct btf *btf)
{
- void *hdr, *types;
+ void *types, *kind_layout = NULL;
struct strset *set = NULL;
int err = -ENOMEM;
@@ -1601,15 +1689,20 @@ static int btf_ensure_modifiable(struct btf *btf)
return 0;
}
- /* split raw data into three memory regions */
- hdr = malloc(btf->hdr->hdr_len);
+ /* split raw data into memory regions; btf->hdr is done already. */
types = malloc(btf->hdr->type_len);
- if (!hdr || !types)
+ if (!types)
goto err_out;
-
- memcpy(hdr, btf->hdr, btf->hdr->hdr_len);
memcpy(types, btf->types_data, btf->hdr->type_len);
+ if (btf->hdr->kind_layout_len && btf->hdr->kind_layout_off) {
+ kind_layout = malloc(btf->hdr->kind_layout_len);
+ if (!kind_layout)
+ goto err_out;
+ memcpy(kind_layout, btf->raw_data + btf->hdr->hdr_len + btf->hdr->kind_layout_off,
+ 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);
if (IS_ERR(set)) {
@@ -1618,11 +1711,12 @@ static int btf_ensure_modifiable(struct btf *btf)
}
/* only when everything was successful, update internal state */
- btf->hdr = hdr;
btf->types_data = types;
btf->types_data_cap = btf->hdr->type_len;
btf->strs_data = NULL;
btf->strs_set = set;
+ if (kind_layout)
+ btf->kind_layout = kind_layout;
/* if BTF was created from scratch, all strings are guaranteed to be
* unique and deduplicated
*/
@@ -1634,12 +1728,14 @@ static int btf_ensure_modifiable(struct btf *btf)
/* invalidate raw_data representation */
btf_invalidate_raw_data(btf);
+ btf->modifiable = true;
+
return 0;
err_out:
strset__free(set);
- free(hdr);
free(types);
+ free(kind_layout);
return err;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 bpf-next 03/17] libbpf: use kind layout to compute an unknown kind size
2023-11-12 12:48 [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 01/17] btf: add kind layout encoding, crcs to UAPI Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 02/17] libbpf: support kind layout section handling in BTF Alan Maguire
@ 2023-11-12 12:48 ` Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 04/17] libbpf: add kind layout encoding, crc support Alan Maguire
` (14 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Alan Maguire @ 2023-11-12 12:48 UTC (permalink / raw)
To: ast, daniel, andrii, jolsa
Cc: quentin, eddyz87, martin.lau, song, yonghong.song, john.fastabend,
kpsingh, sdf, haoluo, masahiroy, bpf, 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 | 35 ++++++++++++++++++++++++++++-------
1 file changed, 28 insertions(+), 7 deletions(-)
diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 1d043fe49d4c..973da2b21df2 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -350,7 +350,29 @@ 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 = &((struct btf_kind_layout *)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;
+ }
+
+ 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);
@@ -386,8 +408,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);
}
}
@@ -486,7 +507,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) {
@@ -1862,7 +1883,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);
@@ -1943,7 +1964,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;
@@ -4939,7 +4960,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.31.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 bpf-next 04/17] libbpf: add kind layout encoding, crc support
2023-11-12 12:48 [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF Alan Maguire
` (2 preceding siblings ...)
2023-11-12 12:48 ` [PATCH v4 bpf-next 03/17] libbpf: use kind layout to compute an unknown kind size Alan Maguire
@ 2023-11-12 12:48 ` Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 05/17] libbpf: BTF validation can use kind layout for unknown kinds Alan Maguire
` (13 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Alan Maguire @ 2023-11-12 12:48 UTC (permalink / raw)
To: ast, daniel, andrii, jolsa
Cc: quentin, eddyz87, martin.lau, song, yonghong.song, john.fastabend,
kpsingh, sdf, haoluo, masahiroy, bpf, 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.
Kind layout information is maintained in btf.c in the
kind_layouts[] array; when BTF is created with the
add_kind_layout option it represents the current view
of supported BTF kinds.
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
tools/lib/bpf/btf.c | 80 ++++++++++++++++++++++++++++++++++++++--
tools/lib/bpf/btf.h | 11 ++++++
tools/lib/bpf/libbpf.map | 1 +
3 files changed, 89 insertions(+), 3 deletions(-)
diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 973da2b21df2..fa6104860ebc 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"
@@ -28,6 +29,35 @@
static struct btf_type btf_void;
+/* Describe how kinds are laid out; some have a singular element following the "struct btf_type",
+ * some have BTF_INFO_VLEN(t->info) elements. Specify sizes for both. Flags are currently unused.
+ * Kind layout can be optionally added to the BTF representation in a dedicated section to
+ * facilitate parsing. New kinds must be added here.
+ */
+struct btf_kind_layout kind_layouts[NR_BTF_KINDS] = {
+/* flags singular element size vlen element(s) size */
+{ 0, 0, 0 }, /* _UNKN */
+{ 0, sizeof(__u32), 0 }, /* _INT */
+{ 0, 0, 0 }, /* _PTR */
+{ 0, sizeof(struct btf_array), 0 }, /* _ARRAY */
+{ 0, 0, sizeof(struct btf_member) }, /* _STRUCT */
+{ 0, 0, sizeof(struct btf_member) }, /* _UNION */
+{ 0, 0, sizeof(struct btf_enum) }, /* _ENUM */
+{ 0, 0, 0 }, /* _FWD */
+{ 0, 0, 0 }, /* _TYPEDEF */
+{ 0, 0, 0 }, /* _VOLATILE */
+{ 0, 0, 0 }, /* _CONST */
+{ 0, 0, 0 }, /* _RESTRICT */
+{ 0, 0, 0 }, /* _FUNC */
+{ 0, 0, sizeof(struct btf_param) }, /* _FUNC_PROTO */
+{ 0, sizeof(struct btf_var), 0 }, /* _VAR */
+{ 0, 0, sizeof(struct btf_var_secinfo) }, /* _DATASEC */
+{ 0, 0, 0 }, /* _FLOAT */
+{ 0, sizeof(struct btf_decl_tag), 0 }, /* _DECL_TAG */
+{ 0, 0, 0 }, /* _TYPE_TAG */
+{ 0, 0, sizeof(struct btf_enum64) }, /* _ENUM64 */
+};
+
struct btf {
/* raw BTF data in native endianness */
void *raw_data;
@@ -1063,8 +1093,9 @@ void btf__free(struct btf *btf)
free(btf);
}
-static struct btf *btf_new_empty(struct btf *base_btf)
+static struct btf *btf_new_empty(struct btf_new_opts *opts)
{
+ struct btf *base_btf = OPTS_GET(opts, base_btf, NULL);
struct btf_header *hdr;
struct btf *btf;
@@ -1106,6 +1137,29 @@ static struct btf *btf_new_empty(struct btf *base_btf)
free(btf);
return ERR_PTR(-ENOMEM);
}
+
+ if (opts->add_kind_layout) {
+ hdr->kind_layout_len = sizeof(kind_layouts);
+ btf->kind_layout = malloc(hdr->kind_layout_len);
+ if (!btf->kind_layout) {
+ free(btf->hdr);
+ free(btf);
+ return ERR_PTR(-ENOMEM);
+ }
+ memcpy(btf->kind_layout, kind_layouts, sizeof(kind_layouts));
+ }
+ 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) {
+ hdr->base_crc = base_hdr->crc;
+ hdr->flags |= BTF_FLAG_BASE_CRC_SET;
+ }
+ }
+ hdr->flags |= BTF_FLAG_CRC_SET;
+ }
memcpy(btf->hdr, hdr, sizeof(*hdr));
return btf;
@@ -1113,12 +1167,26 @@ static struct btf *btf_new_empty(struct btf *base_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)
@@ -1564,6 +1632,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);
}
+ if (hdr->flags & BTF_FLAG_CRC_SET) {
+ struct btf_header *h = data;
+
+ h->crc = crc32(0L, (const Bytef *)data, data_sz);
+ }
+
*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 b52dc28dc8af..b8c0716133d1 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -401,6 +401,7 @@ LIBBPF_1.3.0 {
bpf_program__attach_netkit;
bpf_program__attach_tcx;
bpf_program__attach_uprobe_multi;
+ btf__new_empty_opts;
ring__avail_data_size;
ring__consume;
ring__consumer_pos;
--
2.31.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 bpf-next 05/17] libbpf: BTF validation can use kind layout for unknown kinds
2023-11-12 12:48 [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF Alan Maguire
` (3 preceding siblings ...)
2023-11-12 12:48 ` [PATCH v4 bpf-next 04/17] libbpf: add kind layout encoding, crc support Alan Maguire
@ 2023-11-12 12:48 ` Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 06/17] btf: support kernel parsing of BTF with kind layout Alan Maguire
` (12 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Alan Maguire @ 2023-11-12 12:48 UTC (permalink / raw)
To: ast, daniel, andrii, jolsa
Cc: quentin, eddyz87, martin.lau, song, yonghong.song, john.fastabend,
kpsingh, sdf, haoluo, masahiroy, bpf, Alan Maguire
BTF parsing can use kind layout to navigate unknown kinds, so
btf_validate_type() should take kind layout information into
account to avoid failure when an unrecognized kind is met.
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
tools/lib/bpf/btf.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index fa6104860ebc..b0f8d735debb 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -698,8 +698,12 @@ static int btf_validate_type(const struct btf *btf, const struct btf_type *t, __
break;
}
default:
- pr_warn("btf: type [%u]: unrecognized kind %u\n", id, kind);
- return -EINVAL;
+ /* Kind may be represented in kind layout information. */
+ if (btf_type_size_unknown(btf, t) < 0) {
+ pr_warn("btf: type [%u]: unrecognized kind %u\n", id, kind);
+ return -EINVAL;
+ }
+ break;
}
return 0;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 bpf-next 06/17] btf: support kernel parsing of BTF with kind layout
2023-11-12 12:48 [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF Alan Maguire
` (4 preceding siblings ...)
2023-11-12 12:48 ` [PATCH v4 bpf-next 05/17] libbpf: BTF validation can use kind layout for unknown kinds Alan Maguire
@ 2023-11-12 12:48 ` Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 07/17] bpf: add BTF CRC verification where present Alan Maguire
` (11 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Alan Maguire @ 2023-11-12 12:48 UTC (permalink / raw)
To: ast, daniel, andrii, jolsa
Cc: quentin, eddyz87, martin.lau, song, yonghong.song, john.fastabend,
kpsingh, sdf, haoluo, masahiroy, bpf, Alan Maguire
Validate kind layout if present, but because the kernel must be
strict in what it accepts, reject BTF with unsupported kinds,
even if they are in the kind layout information.
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
kernel/bpf/btf.c | 98 +++++++++++++++++++++++++++++++++++++-----------
1 file changed, 77 insertions(+), 21 deletions(-)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 63cf4128fc05..554b5b795d6f 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -258,6 +258,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;
@@ -4969,23 +4970,36 @@ 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) {
+ 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;
+ }
+
+ 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_name_offset_valid(env->btf, t->name_off)) {
- btf_verifier_log(env, "[%u] Invalid name_offset:%u",
- env->log_type_id, t->name_off);
+ 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) {
+ btf_verifier_log(env, "[%u] unknown but required kind %u",
+ env->log_type_id,
+ BTF_INFO_KIND(t->info));
return -EINVAL;
+ } 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;
}
- 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;
return saved_meta_left - meta_left;
@@ -5159,7 +5173,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;
}
@@ -5180,9 +5195,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)
@@ -5197,44 +5244,49 @@ 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");
- return -EINVAL;
- }
if (total > secs[i].off) {
btf_verifier_log(env, "Section overlap found");
return -EINVAL;
}
+ gap = secs[i].off - total;
+ if (gap >= 4) {
+ /* gap larger than alignment gap */
+ btf_verifier_log(env, "Unsupported section found");
+ return -EINVAL;
+ }
if (expected_total - total < secs[i].len) {
btf_verifier_log(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 */
@@ -5297,7 +5349,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;
}
@@ -5534,6 +5586,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.31.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 bpf-next 07/17] bpf: add BTF CRC verification where present
2023-11-12 12:48 [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF Alan Maguire
` (5 preceding siblings ...)
2023-11-12 12:48 ` [PATCH v4 bpf-next 06/17] btf: support kernel parsing of BTF with kind layout Alan Maguire
@ 2023-11-12 12:48 ` Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 08/17] bpf: verify base BTF CRC to ensure it matches module BTF Alan Maguire
` (10 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Alan Maguire @ 2023-11-12 12:48 UTC (permalink / raw)
To: ast, daniel, andrii, jolsa
Cc: quentin, eddyz87, martin.lau, song, yonghong.song, john.fastabend,
kpsingh, sdf, haoluo, masahiroy, bpf, Alan Maguire,
Alexei Starovoitov
If a CRC is set in provided BTF, verify it.
Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
kernel/bpf/btf.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 554b5b795d6f..96c553e40b43 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -25,6 +25,7 @@
#include <linux/bsearch.h>
#include <linux/kobject.h>
#include <linux/sysfs.h>
+#include <linux/crc32.h>
#include <net/netfilter/nf_bpf_link.h>
@@ -5354,6 +5355,20 @@ static int btf_parse_hdr(struct btf_verifier_env *env)
return -ENOTSUPP;
}
+ if (hdr->flags & BTF_FLAG_CRC_SET) {
+ __u32 check, crc = hdr->crc;
+ struct btf_header *h = btf->data;
+
+ h->crc = 0;
+ check = crc32(0xffffffff, btf->data, btf_data_size);
+ check ^= ~0;
+ h->crc = crc;
+ if (check != crc) {
+ btf_verifier_log(env, "Invalid CRC; expected 0x%x ; actual 0x%x",
+ crc, check);
+ return -EINVAL;
+ }
+ }
if (!btf->base_btf && btf_data_size == hdr->hdr_len) {
btf_verifier_log(env, "No data");
return -EINVAL;
--
2.31.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 bpf-next 08/17] bpf: verify base BTF CRC to ensure it matches module BTF
2023-11-12 12:48 [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF Alan Maguire
` (6 preceding siblings ...)
2023-11-12 12:48 ` [PATCH v4 bpf-next 07/17] bpf: add BTF CRC verification where present Alan Maguire
@ 2023-11-12 12:48 ` Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 09/17] kbuild, bpf: switch to --btf_features, add crc,kind_layout features Alan Maguire
` (9 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Alan Maguire @ 2023-11-12 12:48 UTC (permalink / raw)
To: ast, daniel, andrii, jolsa
Cc: quentin, eddyz87, martin.lau, song, yonghong.song, john.fastabend,
kpsingh, sdf, haoluo, masahiroy, bpf, Alan Maguire
Having a base CRC in module BTF allows us to reject base BTF
that does not match that CRC; this allows us to recognize
incompatible BTF up-front, not having to rely on invalidation
due to internal mismatches in module/kernel BTF ids.
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
kernel/bpf/btf.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 96c553e40b43..a51dc3ef6a56 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -5369,6 +5369,24 @@ static int btf_parse_hdr(struct btf_verifier_env *env)
return -EINVAL;
}
}
+ if (hdr->flags & BTF_FLAG_BASE_CRC_SET) {
+ struct btf_header *base_hdr = &btf->base_btf->hdr;
+
+ if (!btf->base_btf) {
+ btf_verifier_log(env, "Specified base BTF CRC but no base BTF");
+ return -EINVAL;
+ }
+
+ if (!(base_hdr->flags & BTF_FLAG_CRC_SET)) {
+ btf_verifier_log(env, "Specified base BTF CRC but base BTF has no CRC");
+ return -EINVAL;
+ }
+ if (hdr->base_crc != base_hdr->crc) {
+ btf_verifier_log(env, "Specified base CRC 0x%x; differs from actual base CRC 0x%x\n",
+ hdr->base_crc, base_hdr->crc);
+ return -EINVAL;
+ }
+ }
if (!btf->base_btf && btf_data_size == hdr->hdr_len) {
btf_verifier_log(env, "No data");
return -EINVAL;
--
2.31.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 bpf-next 09/17] kbuild, bpf: switch to --btf_features, add crc,kind_layout features
2023-11-12 12:48 [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF Alan Maguire
` (7 preceding siblings ...)
2023-11-12 12:48 ` [PATCH v4 bpf-next 08/17] bpf: verify base BTF CRC to ensure it matches module BTF Alan Maguire
@ 2023-11-12 12:48 ` Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 10/17] bpftool: add BTF dump "format meta" to dump header/metadata Alan Maguire
` (8 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Alan Maguire @ 2023-11-12 12:48 UTC (permalink / raw)
To: ast, daniel, andrii, jolsa
Cc: quentin, eddyz87, martin.lau, song, yonghong.song, john.fastabend,
kpsingh, sdf, haoluo, masahiroy, bpf, Alan Maguire
For pahole v1.26 and later, --btf_features is used to specify BTF
features for encoding. Since it tolerates unknown features, no
further version checking will be needed when adding new features.
Add crc, kind_layout features.
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
scripts/Makefile.btf | 2 ++
1 file changed, 2 insertions(+)
diff --git a/scripts/Makefile.btf b/scripts/Makefile.btf
index 82377e470aed..f8ce33d7f9bb 100644
--- a/scripts/Makefile.btf
+++ b/scripts/Makefile.btf
@@ -16,4 +16,6 @@ pahole-flags-$(CONFIG_PAHOLE_HAS_LANG_EXCLUDE) += --lang_exclude=rust
pahole-flags-$(call test-ge, $(pahole-ver), 125) += --skip_encoding_btf_inconsistent_proto --btf_gen_optimized
+pahole-flags-$(call test-ge, $(pahole-ver), 126) = -j --lang_exclude=rust --btf_features=encode_force,var,float,decl_tag,type_tag,enum64,optimized_func,consistent_func,crc,kind_layout
+
export PAHOLE_FLAGS := $(pahole-flags-y)
--
2.31.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 bpf-next 10/17] bpftool: add BTF dump "format meta" to dump header/metadata
2023-11-12 12:48 [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF Alan Maguire
` (8 preceding siblings ...)
2023-11-12 12:48 ` [PATCH v4 bpf-next 09/17] kbuild, bpf: switch to --btf_features, add crc,kind_layout features Alan Maguire
@ 2023-11-12 12:48 ` Alan Maguire
2023-11-14 5:10 ` Quentin Monnet
2023-11-12 12:48 ` [PATCH v4 bpf-next 11/17] bpftool: update doc to describe bpftool btf dump .. format meta Alan Maguire
` (7 subsequent siblings)
17 siblings, 1 reply; 29+ messages in thread
From: Alan Maguire @ 2023-11-12 12:48 UTC (permalink / raw)
To: ast, daniel, andrii, jolsa
Cc: quentin, eddyz87, martin.lau, song, yonghong.song, john.fastabend,
kpsingh, sdf, haoluo, masahiroy, bpf, 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 5161076
magic 0xeb9f
version 1
flags 0x1
hdr_len 40
type_len 3036368
type_off 0
str_len 2124588
str_off 3036368
kind_layout_len 80
kind_layout_off 5160956
crc 0x64af901b
base_crc 0x0
kind 0 UNKNOWN flags 0x0 info_sz 0 elem_sz 0
kind 1 INT flags 0x0 info_sz 0 elem_sz 0
kind 2 PTR flags 0x0 info_sz 0 elem_sz 0
kind 3 ARRAY flags 0x0 info_sz 0 elem_sz 0
kind 4 STRUCT flags 0x35 info_sz 0 elem_sz 0
...
JSON output is also supported:
$ bpftool -j btf dump file vmlinux format meta
{"size":5161076,"header":{"magic":60319,"version":1,"flags":1,"hdr_len":40,"type_len":3036368,"type_off":0,"str_len":2124588,"str_off":3036368,"kind_layout_len":80,"kind_layout_offset":5160956,"crc":1689227291,"base_crc":0},"kind_layouts":[{"kind":0,"name":"UNKNOWN","flags":0,"info_sz":0,"elem_sz":0},{"kind":1,"name":"INT","flags":0,"info_sz":0,"elem_sz":0},{"kind":2,"name":"PTR","flags":0,"info_sz":0,"elem_sz":0},{"kind":3,"name":"ARRAY","flags":0,"info_sz":0,"elem_sz":0},{"kind":4,"name":"STRUCT","flags":53,"info_sz":0,"elem_sz":0},{"kind":5,"name":"UNION","flags":0,"info_sz":0,"elem_sz":0},{"kind":6,"name":"ENUM","flags":60319,"info_sz":1,"elem_sz":1},{"kind":7,"name":"FWD","flags":40,"info_sz":0,"elem_sz":0},{"kind":8,"name":"TYPEDEF","flags":0,"info_sz":0,"elem_sz":0},{"kind":9,"name":"VOLATILE","flags":0,"info_sz":0,"elem_sz":0},{"kind":10,"name":"CONST","flags":0,"info_sz":0,"elem_sz":0},{"kind":11,"name":"RESTRICT","flags":1,"info_sz":0,"elem_sz":0},{"kind":12,"name":"FUNC","flags":0,"info_sz":0,"elem_sz":0},{"kind":13,"name":"FUNC_PROTO","flags":80,"info_sz":0,"elem_sz":0},{"kind":14,"name":"VAR","flags":0,"info_sz":0,"elem_sz":0},{"kind":15,"name":"DATASEC","flags":0,"info_sz":0,"elem_sz":0},{"kind":16,"name":"FLOAT","flags":53,"info_sz":0,"elem_sz":0},{"kind":17,"name":"DECL_TAG","flags":0,"info_sz":0,"elem_sz":0},{"kind":18,"name":"TYPE_TAG","flags":11441,"info_sz":3,"elem_sz":0},{"kind":19,"name":"ENUM64","flags":0,"info_sz":0,"elem_sz":0}]}
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
tools/bpf/bpftool/bash-completion/bpftool | 2 +-
tools/bpf/bpftool/btf.c | 91 ++++++++++++++++++++++-
2 files changed, 90 insertions(+), 3 deletions(-)
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 6e4f7ce6bc01..157c3afd8247 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..208f3a587534 100644
--- a/tools/bpf/bpftool/btf.c
+++ b/tools/bpf/bpftool/btf.c
@@ -504,6 +504,88 @@ 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;
+ __u8 i, nr_kinds = 0;
+ const void *data;
+ __u32 data_sz;
+
+ 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_name(json_wtr, "header");
+ jsonw_start_object(json_wtr); /* btf header object */
+ 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)) {
+ if (json_output) {
+ jsonw_end_object(json_wtr); /* header object */
+ jsonw_end_object(json_wtr); /* metadata object */
+ }
+ return 0;
+ }
+ if (hdr->kind_layout_len > 0 && hdr->kind_layout_off > 0) {
+ k = (void *)hdr + hdr->hdr_len + hdr->kind_layout_off;
+ nr_kinds = hdr->kind_layout_len / sizeof(*k);
+ }
+ 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 */
+
+ if (nr_kinds > 0) {
+ jsonw_name(json_wtr, "kind_layouts");
+ jsonw_start_array(json_wtr);
+ for (i = 0; i < nr_kinds; i++) {
+ jsonw_start_object(json_wtr);
+ jsonw_uint_field(json_wtr, "kind", i);
+ if (i < NR_BTF_KINDS)
+ jsonw_string_field(json_wtr, "name", btf_kind_str[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);
+ }
+ jsonw_end_array(json_wtr);
+ }
+ jsonw_end_object(json_wtr); /* end metadata */
+ } 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);
+ for (i = 0; i < nr_kinds; i++) {
+ printf("kind %-4d %-10s flags 0x%-4x info_sz %-4d elem_sz %-4d\n",
+ i, i < NR_BTF_KINDS ? btf_kind_str[i] : "?",
+ k[i].flags, k[i].info_sz, k[i].elem_sz);
+ }
+ }
+
+ return 0;
+}
+
static const char sysfs_vmlinux[] = "/sys/kernel/btf/vmlinux";
static struct btf *get_vmlinux_btf_from_sysfs(void)
@@ -553,6 +635,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 +737,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 +777,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 +1150,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.31.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v4 bpf-next 10/17] bpftool: add BTF dump "format meta" to dump header/metadata
2023-11-12 12:48 ` [PATCH v4 bpf-next 10/17] bpftool: add BTF dump "format meta" to dump header/metadata Alan Maguire
@ 2023-11-14 5:10 ` Quentin Monnet
2023-11-15 8:45 ` Alan Maguire
0 siblings, 1 reply; 29+ messages in thread
From: Quentin Monnet @ 2023-11-14 5:10 UTC (permalink / raw)
To: Alan Maguire, ast, daniel, andrii, jolsa
Cc: eddyz87, martin.lau, song, yonghong.song, john.fastabend, kpsingh,
sdf, haoluo, masahiroy, bpf
2023-11-12 12:49 UTC+0000 ~ 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 5161076
> magic 0xeb9f
> version 1
> flags 0x1
> hdr_len 40
> type_len 3036368
> type_off 0
> str_len 2124588
> str_off 3036368
> kind_layout_len 80
> kind_layout_off 5160956
> crc 0x64af901b
> base_crc 0x0
> kind 0 UNKNOWN flags 0x0 info_sz 0 elem_sz 0
> kind 1 INT flags 0x0 info_sz 0 elem_sz 0
> kind 2 PTR flags 0x0 info_sz 0 elem_sz 0
> kind 3 ARRAY flags 0x0 info_sz 0 elem_sz 0
> kind 4 STRUCT flags 0x35 info_sz 0 elem_sz 0
> ...
>
> JSON output is also supported:
>
> $ bpftool -j btf dump file vmlinux format meta
> {"size":5161076,"header":{"magic":60319,"version":1,"flags":1,"hdr_len":40,"type_len":3036368,"type_off":0,"str_len":2124588,"str_off":3036368,"kind_layout_len":80,"kind_layout_offset":5160956,"crc":1689227291,"base_crc":0},"kind_layouts":[{"kind":0,"name":"UNKNOWN","flags":0,"info_sz":0,"elem_sz":0},{"kind":1,"name":"INT","flags":0,"info_sz":0,"elem_sz":0},{"kind":2,"name":"PTR","flags":0,"info_sz":0,"elem_sz":0},{"kind":3,"name":"ARRAY","flags":0,"info_sz":0,"elem_sz":0},{"kind":4,"name":"STRUCT","flags":53,"info_sz":0,"elem_sz":0},{"kind":5,"name":"UNION","flags":0,"info_sz":0,"elem_sz":0},{"kind":6,"name":"ENUM","flags":60319,"info_sz":1,"elem_sz":1},{"kind":7,"name":"FWD","flags":40,"info_sz":0,"elem_sz":0},{"kind":8,"name":"TYPEDEF","flags":0,"info_sz":0,"elem_sz":0},{"kind":9,"name":"VOLATILE","flags":0,"info_sz":0,"elem_sz":0},{"kind":10,"name":"CONST","flags":0,"info_sz":0,"elem_sz":0},{"kind":11,"name":"RESTRICT","flags":1,"info_sz":0,"elem_sz":0},{"kind":12,"name":"FUNC","flags":0,"info_sz":0,"elem_sz":0},{"kind":13,"name":"FUNC_PROTO","flags":80,"info_sz":0,"elem_sz":0},{"kind":14,"name":"VAR","flags":0,"info_sz":0,"elem_sz":0},{"kind":15,"name":"DATASEC","flags":0,"info_sz":0,"elem_sz":0},{"kind":16,"name":"FLOAT","flags":53,"info_sz":0,"elem_sz":0},{"kind":17,"name":"DECL_TAG","flags":0,"info_sz":0,"elem_sz":0},{"kind":18,"name":"TYPE_TAG","flags":11441,"info_sz":3,"elem_sz":0},{"kind":19,"name":"ENUM64","flags":0,"info_sz":0,"elem_sz":0}]}
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
> tools/bpf/bpftool/bash-completion/bpftool | 2 +-
> tools/bpf/bpftool/btf.c | 91 ++++++++++++++++++++++-
> 2 files changed, 90 insertions(+), 3 deletions(-)
>
> diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
> index 6e4f7ce6bc01..157c3afd8247 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..208f3a587534 100644
> --- a/tools/bpf/bpftool/btf.c
> +++ b/tools/bpf/bpftool/btf.c
> @@ -504,6 +504,88 @@ 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;
> + __u8 i, nr_kinds = 0;
> + const void *data;
> + __u32 data_sz;
> +
> + data = btf__raw_data(btf, &data_sz);
> + if (!data)
> + return -ENOMEM;
> + hdr = data;
> + if (json_output) {
> + jsonw_start_object(json_wtr); /* btf metadata object */
Nit: Please make sure to be consistent when aligning these comments:
there are several occurrences with spaces (here three spaces), several
ones with tabs. For these, I'd prefer tabs to align the start and end
comments for a given object/array, although I don't really using a
single space as well as long as we keep it consistent.
> + jsonw_uint_field(json_wtr, "size", data_sz);
> + jsonw_name(json_wtr, "header");
> + jsonw_start_object(json_wtr); /* btf header object */
> + 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)) {
> + if (json_output) {
> + jsonw_end_object(json_wtr); /* header object */
> + jsonw_end_object(json_wtr); /* metadata object */
Similarly, can you please keep consistent comment strings? "metadata
object" here vs. "end metadata" below.
> + }
> + return 0;
> + }
> + if (hdr->kind_layout_len > 0 && hdr->kind_layout_off > 0) {
> + k = (void *)hdr + hdr->hdr_len + hdr->kind_layout_off;
> + nr_kinds = hdr->kind_layout_len / sizeof(*k);
> + }
> + 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 */
> +
> + if (nr_kinds > 0) {
> + jsonw_name(json_wtr, "kind_layouts");
> + jsonw_start_array(json_wtr);
> + for (i = 0; i < nr_kinds; i++) {
> + jsonw_start_object(json_wtr);
> + jsonw_uint_field(json_wtr, "kind", i);
> + if (i < NR_BTF_KINDS)
> + jsonw_string_field(json_wtr, "name", btf_kind_str[i]);
I prefer to avoid conditional fields in JSON, especially in an array
it's easier to process the JSON if all items have the same structure.
Would it make sense to keep the "name" field, but use jsonw_null() (or
"UNKNOWN") for the value when there's no name to print?
Thanks,
Quentin
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v4 bpf-next 10/17] bpftool: add BTF dump "format meta" to dump header/metadata
2023-11-14 5:10 ` Quentin Monnet
@ 2023-11-15 8:45 ` Alan Maguire
2023-11-15 14:51 ` Quentin Monnet
0 siblings, 1 reply; 29+ messages in thread
From: Alan Maguire @ 2023-11-15 8:45 UTC (permalink / raw)
To: Quentin Monnet, ast, daniel, andrii, jolsa
Cc: eddyz87, martin.lau, song, yonghong.song, john.fastabend, kpsingh,
sdf, haoluo, masahiroy, bpf
On 14/11/2023 05:10, Quentin Monnet wrote:
> 2023-11-12 12:49 UTC+0000 ~ 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 5161076
>> magic 0xeb9f
>> version 1
>> flags 0x1
>> hdr_len 40
>> type_len 3036368
>> type_off 0
>> str_len 2124588
>> str_off 3036368
>> kind_layout_len 80
>> kind_layout_off 5160956
>> crc 0x64af901b
>> base_crc 0x0
>> kind 0 UNKNOWN flags 0x0 info_sz 0 elem_sz 0
>> kind 1 INT flags 0x0 info_sz 0 elem_sz 0
>> kind 2 PTR flags 0x0 info_sz 0 elem_sz 0
>> kind 3 ARRAY flags 0x0 info_sz 0 elem_sz 0
>> kind 4 STRUCT flags 0x35 info_sz 0 elem_sz 0
>> ...
>>
>> JSON output is also supported:
>>
>> $ bpftool -j btf dump file vmlinux format meta
>> {"size":5161076,"header":{"magic":60319,"version":1,"flags":1,"hdr_len":40,"type_len":3036368,"type_off":0,"str_len":2124588,"str_off":3036368,"kind_layout_len":80,"kind_layout_offset":5160956,"crc":1689227291,"base_crc":0},"kind_layouts":[{"kind":0,"name":"UNKNOWN","flags":0,"info_sz":0,"elem_sz":0},{"kind":1,"name":"INT","flags":0,"info_sz":0,"elem_sz":0},{"kind":2,"name":"PTR","flags":0,"info_sz":0,"elem_sz":0},{"kind":3,"name":"ARRAY","flags":0,"info_sz":0,"elem_sz":0},{"kind":4,"name":"STRUCT","flags":53,"info_sz":0,"elem_sz":0},{"kind":5,"name":"UNION","flags":0,"info_sz":0,"elem_sz":0},{"kind":6,"name":"ENUM","flags":60319,"info_sz":1,"elem_sz":1},{"kind":7,"name":"FWD","flags":40,"info_sz":0,"elem_sz":0},{"kind":8,"name":"TYPEDEF","flags":0,"info_sz":0,"elem_sz":0},{"kind":9,"name":"VOLATILE","flags":0,"info_sz":0,"elem_sz":0},{"kind":10,"name":"CONST","flags":0,"info_sz":0,"elem_sz":0},{"kind":11,"name":"RESTRICT","flags":1,"info_sz":0,"elem_sz":0},{"kind":12,"name":"FUNC","flags":0,"info_sz":0,"elem_sz":0},{"kind":13,"name":"FUNC_PROTO","flags":80,"info_sz":0,"elem_sz":0},{"kind":14,"name":"VAR","flags":0,"info_sz":0,"elem_sz":0},{"kind":15,"name":"DATASEC","flags":0,"info_sz":0,"elem_sz":0},{"kind":16,"name":"FLOAT","flags":53,"info_sz":0,"elem_sz":0},{"kind":17,"name":"DECL_TAG","flags":0,"info_sz":0,"elem_sz":0},{"kind":18,"name":"TYPE_TAG","flags":11441,"info_sz":3,"elem_sz":0},{"kind":19,"name":"ENUM64","flags":0,"info_sz":0,"elem_sz":0}]}
>>
>> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>> ---
>> tools/bpf/bpftool/bash-completion/bpftool | 2 +-
>> tools/bpf/bpftool/btf.c | 91 ++++++++++++++++++++++-
>> 2 files changed, 90 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
>> index 6e4f7ce6bc01..157c3afd8247 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..208f3a587534 100644
>> --- a/tools/bpf/bpftool/btf.c
>> +++ b/tools/bpf/bpftool/btf.c
>> @@ -504,6 +504,88 @@ 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;
>> + __u8 i, nr_kinds = 0;
>> + const void *data;
>> + __u32 data_sz;
>> +
>> + data = btf__raw_data(btf, &data_sz);
>> + if (!data)
>> + return -ENOMEM;
>> + hdr = data;
>> + if (json_output) {
>> + jsonw_start_object(json_wtr); /* btf metadata object */
>
> Nit: Please make sure to be consistent when aligning these comments:
> there are several occurrences with spaces (here three spaces), several
> ones with tabs. For these, I'd prefer tabs to align the start and end
> comments for a given object/array, although I don't really using a
> single space as well as long as we keep it consistent.
>
>> + jsonw_uint_field(json_wtr, "size", data_sz);
>> + jsonw_name(json_wtr, "header");
>> + jsonw_start_object(json_wtr); /* btf header object */
>> + 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)) {
>> + if (json_output) {
>> + jsonw_end_object(json_wtr); /* header object */
>> + jsonw_end_object(json_wtr); /* metadata object */
>
> Similarly, can you please keep consistent comment strings? "metadata
> object" here vs. "end metadata" below.
>
Sure, I'll fix indent consistency/naming and the docs issue in the
next revision. Thanks!
>> + }
>> + return 0;
>> + }
>> + if (hdr->kind_layout_len > 0 && hdr->kind_layout_off > 0) {
>> + k = (void *)hdr + hdr->hdr_len + hdr->kind_layout_off;
>> + nr_kinds = hdr->kind_layout_len / sizeof(*k);
>> + }
>> + 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 */
>> +
>> + if (nr_kinds > 0) {
>> + jsonw_name(json_wtr, "kind_layouts");
>> + jsonw_start_array(json_wtr);
>> + for (i = 0; i < nr_kinds; i++) {
>> + jsonw_start_object(json_wtr);
>> + jsonw_uint_field(json_wtr, "kind", i);
>> + if (i < NR_BTF_KINDS)
>> + jsonw_string_field(json_wtr, "name", btf_kind_str[i]);
>
> I prefer to avoid conditional fields in JSON, especially in an array
> it's easier to process the JSON if all items have the same structure.
> Would it make sense to keep the "name" field, but use jsonw_null() (or
> "UNKNOWN") for the value when there's no name to print?
>
The only thing about UNKNOWN is that there is a BTF_KIND_UNKN that
is displayed as UNKNOWN; what about "?" to be consistent with the
non-json output (or if there's another option of course, we could
use that for both)? Thanks!
Alan
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v4 bpf-next 10/17] bpftool: add BTF dump "format meta" to dump header/metadata
2023-11-15 8:45 ` Alan Maguire
@ 2023-11-15 14:51 ` Quentin Monnet
2023-11-16 9:16 ` Alan Maguire
0 siblings, 1 reply; 29+ messages in thread
From: Quentin Monnet @ 2023-11-15 14:51 UTC (permalink / raw)
To: Alan Maguire, ast, daniel, andrii, jolsa
Cc: eddyz87, martin.lau, song, yonghong.song, john.fastabend, kpsingh,
sdf, haoluo, masahiroy, bpf
On 15 November 2023 03:45:41 GMT-05:00, Alan Maguire <alan.maguire@oracle.com> wrote:
>On 14/11/2023 05:10, Quentin Monnet wrote:
>> 2023-11-12 12:49 UTC+0000 ~ 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 5161076
>>> magic 0xeb9f
>>> version 1
>>> flags 0x1
>>> hdr_len 40
>>> type_len 3036368
>>> type_off 0
>>> str_len 2124588
>>> str_off 3036368
>>> kind_layout_len 80
>>> kind_layout_off 5160956
>>> crc 0x64af901b
>>> base_crc 0x0
>>> kind 0 UNKNOWN flags 0x0 info_sz 0 elem_sz 0
>>> kind 1 INT flags 0x0 info_sz 0 elem_sz 0
>>> kind 2 PTR flags 0x0 info_sz 0 elem_sz 0
>>> kind 3 ARRAY flags 0x0 info_sz 0 elem_sz 0
>>> kind 4 STRUCT flags 0x35 info_sz 0 elem_sz 0
>>> ...
>>>
>>> JSON output is also supported:
>>>
>>> $ bpftool -j btf dump file vmlinux format meta
>>> {"size":5161076,"header":{"magic":60319,"version":1,"flags":1,"hdr_len":40,"type_len":3036368,"type_off":0,"str_len":2124588,"str_off":3036368,"kind_layout_len":80,"kind_layout_offset":5160956,"crc":1689227291,"base_crc":0},"kind_layouts":[{"kind":0,"name":"UNKNOWN","flags":0,"info_sz":0,"elem_sz":0},{"kind":1,"name":"INT","flags":0,"info_sz":0,"elem_sz":0},{"kind":2,"name":"PTR","flags":0,"info_sz":0,"elem_sz":0},{"kind":3,"name":"ARRAY","flags":0,"info_sz":0,"elem_sz":0},{"kind":4,"name":"STRUCT","flags":53,"info_sz":0,"elem_sz":0},{"kind":5,"name":"UNION","flags":0,"info_sz":0,"elem_sz":0},{"kind":6,"name":"ENUM","flags":60319,"info_sz":1,"elem_sz":1},{"kind":7,"name":"FWD","flags":40,"info_sz":0,"elem_sz":0},{"kind":8,"name":"TYPEDEF","flags":0,"info_sz":0,"elem_sz":0},{"kind":9,"name":"VOLATILE","flags":0,"info_sz":0,"elem_sz":0},{"kind":10,"name":"CONST","flags":0,"info_sz":0,"elem_sz":0},{"kind":11,"name":"RESTRICT","flags":1,"info_sz":0,"elem_sz":0},{"kind":12,"name":"FUNC","flags":0,"info_sz":0,"elem_sz":0},{"kind":13,"name":"FUNC_PROTO","flags":80,"info_sz":0,"elem_sz":0},{"kind":14,"name":"VAR","flags":0,"info_sz":0,"elem_sz":0},{"kind":15,"name":"DATASEC","flags":0,"info_sz":0,"elem_sz":0},{"kind":16,"name":"FLOAT","flags":53,"info_sz":0,"elem_sz":0},{"kind":17,"name":"DECL_TAG","flags":0,"info_sz":0,"elem_sz":0},{"kind":18,"name":"TYPE_TAG","flags":11441,"info_sz":3,"elem_sz":0},{"kind":19,"name":"ENUM64","flags":0,"info_sz":0,"elem_sz":0}]}
>>>
>>> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>>> ---
>>> tools/bpf/bpftool/bash-completion/bpftool | 2 +-
>>> tools/bpf/bpftool/btf.c | 91 ++++++++++++++++++++++-
>>> 2 files changed, 90 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
>>> index 6e4f7ce6bc01..157c3afd8247 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..208f3a587534 100644
>>> --- a/tools/bpf/bpftool/btf.c
>>> +++ b/tools/bpf/bpftool/btf.c
>>> @@ -504,6 +504,88 @@ 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;
>>> + __u8 i, nr_kinds = 0;
>>> + const void *data;
>>> + __u32 data_sz;
>>> +
>>> + data = btf__raw_data(btf, &data_sz);
>>> + if (!data)
>>> + return -ENOMEM;
>>> + hdr = data;
>>> + if (json_output) {
>>> + jsonw_start_object(json_wtr); /* btf metadata object */
>>
>> Nit: Please make sure to be consistent when aligning these comments:
>> there are several occurrences with spaces (here three spaces), several
>> ones with tabs. For these, I'd prefer tabs to align the start and end
>> comments for a given object/array, although I don't really using a
>> single space as well as long as we keep it consistent.
>>
>>> + jsonw_uint_field(json_wtr, "size", data_sz);
>>> + jsonw_name(json_wtr, "header");
>>> + jsonw_start_object(json_wtr); /* btf header object */
>>> + 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)) {
>>> + if (json_output) {
>>> + jsonw_end_object(json_wtr); /* header object */
>>> + jsonw_end_object(json_wtr); /* metadata object */
>>
>> Similarly, can you please keep consistent comment strings? "metadata
>> object" here vs. "end metadata" below.
>>
>
>Sure, I'll fix indent consistency/naming and the docs issue in the
>next revision. Thanks!
>
>>> + }
>>> + return 0;
>>> + }
>>> + if (hdr->kind_layout_len > 0 && hdr->kind_layout_off > 0) {
>>> + k = (void *)hdr + hdr->hdr_len + hdr->kind_layout_off;
>>> + nr_kinds = hdr->kind_layout_len / sizeof(*k);
>>> + }
>>> + 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 */
>>> +
>>> + if (nr_kinds > 0) {
>>> + jsonw_name(json_wtr, "kind_layouts");
>>> + jsonw_start_array(json_wtr);
>>> + for (i = 0; i < nr_kinds; i++) {
>>> + jsonw_start_object(json_wtr);
>>> + jsonw_uint_field(json_wtr, "kind", i);
>>> + if (i < NR_BTF_KINDS)
>>> + jsonw_string_field(json_wtr, "name", btf_kind_str[i]);
>>
>> I prefer to avoid conditional fields in JSON, especially in an array
>> it's easier to process the JSON if all items have the same structure.
>> Would it make sense to keep the "name" field, but use jsonw_null() (or
>> "UNKNOWN") for the value when there's no name to print?
>>
>
>The only thing about UNKNOWN is that there is a BTF_KIND_UNKN that
>is displayed as UNKNOWN; what about "?" to be consistent with the
>non-json output (or if there's another option of course, we could
>use that for both)? Thanks!
Right, sorry, I realised just after sending my message.
In that case we could just use a 'null' value in JSON with jsonw_null()? The object '{"name": null}' is valid. The question mark is another possibility but requires comparing strings when parsing the JSON.
Thanks,
Quentin
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v4 bpf-next 10/17] bpftool: add BTF dump "format meta" to dump header/metadata
2023-11-15 14:51 ` Quentin Monnet
@ 2023-11-16 9:16 ` Alan Maguire
0 siblings, 0 replies; 29+ messages in thread
From: Alan Maguire @ 2023-11-16 9:16 UTC (permalink / raw)
To: Quentin Monnet, ast, daniel, andrii, jolsa
Cc: eddyz87, martin.lau, song, yonghong.song, john.fastabend, kpsingh,
sdf, haoluo, masahiroy, bpf
On 15/11/2023 14:51, Quentin Monnet wrote:
>
>
> On 15 November 2023 03:45:41 GMT-05:00, Alan Maguire <alan.maguire@oracle.com> wrote:
>> On 14/11/2023 05:10, Quentin Monnet wrote:
>>> 2023-11-12 12:49 UTC+0000 ~ 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 5161076
>>>> magic 0xeb9f
>>>> version 1
>>>> flags 0x1
>>>> hdr_len 40
>>>> type_len 3036368
>>>> type_off 0
>>>> str_len 2124588
>>>> str_off 3036368
>>>> kind_layout_len 80
>>>> kind_layout_off 5160956
>>>> crc 0x64af901b
>>>> base_crc 0x0
>>>> kind 0 UNKNOWN flags 0x0 info_sz 0 elem_sz 0
>>>> kind 1 INT flags 0x0 info_sz 0 elem_sz 0
>>>> kind 2 PTR flags 0x0 info_sz 0 elem_sz 0
>>>> kind 3 ARRAY flags 0x0 info_sz 0 elem_sz 0
>>>> kind 4 STRUCT flags 0x35 info_sz 0 elem_sz 0
>>>> ...
>>>>
>>>> JSON output is also supported:
>>>>
>>>> $ bpftool -j btf dump file vmlinux format meta
>>>> {"size":5161076,"header":{"magic":60319,"version":1,"flags":1,"hdr_len":40,"type_len":3036368,"type_off":0,"str_len":2124588,"str_off":3036368,"kind_layout_len":80,"kind_layout_offset":5160956,"crc":1689227291,"base_crc":0},"kind_layouts":[{"kind":0,"name":"UNKNOWN","flags":0,"info_sz":0,"elem_sz":0},{"kind":1,"name":"INT","flags":0,"info_sz":0,"elem_sz":0},{"kind":2,"name":"PTR","flags":0,"info_sz":0,"elem_sz":0},{"kind":3,"name":"ARRAY","flags":0,"info_sz":0,"elem_sz":0},{"kind":4,"name":"STRUCT","flags":53,"info_sz":0,"elem_sz":0},{"kind":5,"name":"UNION","flags":0,"info_sz":0,"elem_sz":0},{"kind":6,"name":"ENUM","flags":60319,"info_sz":1,"elem_sz":1},{"kind":7,"name":"FWD","flags":40,"info_sz":0,"elem_sz":0},{"kind":8,"name":"TYPEDEF","flags":0,"info_sz":0,"elem_sz":0},{"kind":9,"name":"VOLATILE","flags":0,"info_sz":0,"elem_sz":0},{"kind":10,"name":"CONST","flags":0,"info_sz":0,"elem_sz":0},{"kind":11,"name":"RESTRICT","flags":1,"info_sz":0,"elem_sz":0},{"kind":12,"name":"FUNC","flags":0,"info_sz":0,"elem_sz":0},{"kind":13,"name":"FUNC_PROTO","flags":80,"info_sz":0,"elem_sz":0},{"kind":14,"name":"VAR","flags":0,"info_sz":0,"elem_sz":0},{"kind":15,"name":"DATASEC","flags":0,"info_sz":0,"elem_sz":0},{"kind":16,"name":"FLOAT","flags":53,"info_sz":0,"elem_sz":0},{"kind":17,"name":"DECL_TAG","flags":0,"info_sz":0,"elem_sz":0},{"kind":18,"name":"TYPE_TAG","flags":11441,"info_sz":3,"elem_sz":0},{"kind":19,"name":"ENUM64","flags":0,"info_sz":0,"elem_sz":0}]}
>>>>
>>>> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>>>> ---
>>>> tools/bpf/bpftool/bash-completion/bpftool | 2 +-
>>>> tools/bpf/bpftool/btf.c | 91 ++++++++++++++++++++++-
>>>> 2 files changed, 90 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
>>>> index 6e4f7ce6bc01..157c3afd8247 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..208f3a587534 100644
>>>> --- a/tools/bpf/bpftool/btf.c
>>>> +++ b/tools/bpf/bpftool/btf.c
>>>> @@ -504,6 +504,88 @@ 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;
>>>> + __u8 i, nr_kinds = 0;
>>>> + const void *data;
>>>> + __u32 data_sz;
>>>> +
>>>> + data = btf__raw_data(btf, &data_sz);
>>>> + if (!data)
>>>> + return -ENOMEM;
>>>> + hdr = data;
>>>> + if (json_output) {
>>>> + jsonw_start_object(json_wtr); /* btf metadata object */
>>>
>>> Nit: Please make sure to be consistent when aligning these comments:
>>> there are several occurrences with spaces (here three spaces), several
>>> ones with tabs. For these, I'd prefer tabs to align the start and end
>>> comments for a given object/array, although I don't really using a
>>> single space as well as long as we keep it consistent.
>>>
>>>> + jsonw_uint_field(json_wtr, "size", data_sz);
>>>> + jsonw_name(json_wtr, "header");
>>>> + jsonw_start_object(json_wtr); /* btf header object */
>>>> + 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)) {
>>>> + if (json_output) {
>>>> + jsonw_end_object(json_wtr); /* header object */
>>>> + jsonw_end_object(json_wtr); /* metadata object */
>>>
>>> Similarly, can you please keep consistent comment strings? "metadata
>>> object" here vs. "end metadata" below.
>>>
>>
>> Sure, I'll fix indent consistency/naming and the docs issue in the
>> next revision. Thanks!
>>
>>>> + }
>>>> + return 0;
>>>> + }
>>>> + if (hdr->kind_layout_len > 0 && hdr->kind_layout_off > 0) {
>>>> + k = (void *)hdr + hdr->hdr_len + hdr->kind_layout_off;
>>>> + nr_kinds = hdr->kind_layout_len / sizeof(*k);
>>>> + }
>>>> + 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 */
>>>> +
>>>> + if (nr_kinds > 0) {
>>>> + jsonw_name(json_wtr, "kind_layouts");
>>>> + jsonw_start_array(json_wtr);
>>>> + for (i = 0; i < nr_kinds; i++) {
>>>> + jsonw_start_object(json_wtr);
>>>> + jsonw_uint_field(json_wtr, "kind", i);
>>>> + if (i < NR_BTF_KINDS)
>>>> + jsonw_string_field(json_wtr, "name", btf_kind_str[i]);
>>>
>>> I prefer to avoid conditional fields in JSON, especially in an array
>>> it's easier to process the JSON if all items have the same structure.
>>> Would it make sense to keep the "name" field, but use jsonw_null() (or
>>> "UNKNOWN") for the value when there's no name to print?
>>>
>>
>> The only thing about UNKNOWN is that there is a BTF_KIND_UNKN that
>> is displayed as UNKNOWN; what about "?" to be consistent with the
>> non-json output (or if there's another option of course, we could
>> use that for both)? Thanks!
>
> Right, sorry, I realised just after sending my message.
> In that case we could just use a 'null' value in JSON with jsonw_null()? The object '{"name": null}' is valid. The question mark is another possibility but requires comparing strings when parsing the JSON.
>
Great idea! I'll use jsonw_null() for the next version.
I think given the fact we're not sure about the way forward on CRCs and
standalone BTF I'll probably just separate out the kind layout stuff +
bpftool dump meta parts since I _think_ at this point those pieces are
relatively uncontroversial.
Thanks again for the reviews Quentin!
Alan
> Thanks,
> Quentin
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v4 bpf-next 11/17] bpftool: update doc to describe bpftool btf dump .. format meta
2023-11-12 12:48 [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF Alan Maguire
` (9 preceding siblings ...)
2023-11-12 12:48 ` [PATCH v4 bpf-next 10/17] bpftool: add BTF dump "format meta" to dump header/metadata Alan Maguire
@ 2023-11-12 12:48 ` Alan Maguire
2023-11-14 5:12 ` Quentin Monnet
2023-11-12 12:48 ` [PATCH v4 bpf-next 12/17] selftests/bpf: test kind encoding/decoding Alan Maguire
` (6 subsequent siblings)
17 siblings, 1 reply; 29+ messages in thread
From: Alan Maguire @ 2023-11-12 12:48 UTC (permalink / raw)
To: ast, daniel, andrii, jolsa
Cc: quentin, eddyz87, martin.lau, song, yonghong.song, john.fastabend,
kpsingh, sdf, haoluo, masahiroy, bpf, Alan Maguire
...and provide an example of output.
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
.../bpf/bpftool/Documentation/bpftool-btf.rst | 30 +++++++++++++++++--
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-btf.rst b/tools/bpf/bpftool/Documentation/bpftool-btf.rst
index 342716f74ec4..ea8bb0a2041c 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,27 @@ 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 5161076
+ magic 0xeb9f
+ version 1
+ flags 0x1
+ hdr_len 40
+ type_len 3036368
+ type_off 0
+ str_len 2124588
+ str_off 3036368
+ kind_layout_len 80
+ kind_layout_off 5160956
+ crc 0x64af901b
+ base_crc 0x0
+ kind 0 UNKNOWN flags 0x0 info_sz 0 elem_sz 0
+ kind 1 INT flags 0x0 info_sz 0 elem_sz 0
+ kind 2 PTR flags 0x0 info_sz 0 elem_sz 0
+ kind 3 ARRAY flags 0x0 info_sz 0 elem_sz 0
+ kind 4 STRUCT flags 0x35 info_sz 0 elem_sz 0
--
2.31.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v4 bpf-next 11/17] bpftool: update doc to describe bpftool btf dump .. format meta
2023-11-12 12:48 ` [PATCH v4 bpf-next 11/17] bpftool: update doc to describe bpftool btf dump .. format meta Alan Maguire
@ 2023-11-14 5:12 ` Quentin Monnet
0 siblings, 0 replies; 29+ messages in thread
From: Quentin Monnet @ 2023-11-14 5:12 UTC (permalink / raw)
To: Alan Maguire, ast, daniel, andrii, jolsa
Cc: eddyz87, martin.lau, song, yonghong.song, john.fastabend, kpsingh,
sdf, haoluo, masahiroy, bpf
2023-11-12 12:49 UTC+0000 ~ Alan Maguire <alan.maguire@oracle.com>
> ...and provide an example of output.
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
> .../bpf/bpftool/Documentation/bpftool-btf.rst | 30 +++++++++++++++++--
> 1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-btf.rst b/tools/bpf/bpftool/Documentation/bpftool-btf.rst
> index 342716f74ec4..ea8bb0a2041c 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 for this line (it should be two tabs then two
spaces - yes I know it's a pain, we're working on improving this [0]).
[0] https://github.com/libbpf/bpftool/pull/111
>
> **bpftool btf help**
> Print short help message.
> @@ -266,3 +266,27 @@ 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 5161076
> + magic 0xeb9f
> + version 1
> + flags 0x1
> + hdr_len 40
> + type_len 3036368
> + type_off 0
> + str_len 2124588
> + str_off 3036368
> + kind_layout_len 80
> + kind_layout_off 5160956
> + crc 0x64af901b
> + base_crc 0x0
> + kind 0 UNKNOWN flags 0x0 info_sz 0 elem_sz 0
> + kind 1 INT flags 0x0 info_sz 0 elem_sz 0
> + kind 2 PTR flags 0x0 info_sz 0 elem_sz 0
> + kind 3 ARRAY flags 0x0 info_sz 0 elem_sz 0
> + kind 4 STRUCT flags 0x35 info_sz 0 elem_sz 0
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v4 bpf-next 12/17] selftests/bpf: test kind encoding/decoding
2023-11-12 12:48 [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF Alan Maguire
` (10 preceding siblings ...)
2023-11-12 12:48 ` [PATCH v4 bpf-next 11/17] bpftool: update doc to describe bpftool btf dump .. format meta Alan Maguire
@ 2023-11-12 12:48 ` Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 13/17] bpf: support standalone BTF in modules Alan Maguire
` (5 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Alan Maguire @ 2023-11-12 12:48 UTC (permalink / raw)
To: ast, daniel, andrii, jolsa
Cc: quentin, eddyz87, martin.lau, song, yonghong.song, john.fastabend,
kpsingh, sdf, haoluo, masahiroy, bpf, 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.
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
.../selftests/bpf/prog_tests/btf_kind.c | 176 ++++++++++++++++++
1 file changed, 176 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..41a052787ba6
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/btf_kind.c
@@ -0,0 +1,176 @@
+// 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 */
+static void test_btf_kind_encoding(void)
+{
+ LIBBPF_OPTS(btf_new_opts, opts);
+ const struct btf_header *hdr;
+ const void *raw_btf;
+ struct btf *btf;
+ __u32 raw_size;
+
+ opts.add_kind_layout = true;
+ btf = btf__new_empty_opts(&opts);
+ if (!ASSERT_OK_PTR(btf, "btf_new"))
+ return;
+
+ raw_btf = btf__raw_data(btf, &raw_size);
+ if (!ASSERT_OK_PTR(raw_btf, "btf__raw_data"))
+ return;
+
+ hdr = raw_btf;
+
+ ASSERT_EQ(hdr->kind_layout_off % 4, 0, "kind_layout aligned");
+ ASSERT_EQ(hdr->kind_layout_len, sizeof(struct btf_kind_layout) * NR_BTF_KINDS,
+ "kind_layout_len");
+ btf__free(btf);
+}
+
+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(void)
+{
+ __s32 int_id, unrec_id, id, id2;
+ LIBBPF_OPTS(btf_new_opts, opts);
+ struct btf *btf, *new_btf;
+ struct btf_kind_layout *k;
+ struct btf_header *hdr;
+ const void *raw_btf;
+ struct btf_type *t;
+ char btf_path[64];
+ void *new_raw_btf;
+ __u32 raw_size;
+
+ opts.add_kind_layout = true;
+ btf = btf__new_empty_opts(&opts);
+ if (!ASSERT_OK_PTR(btf, "btf_new"))
+ return;
+
+ 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].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");
+ }
+ 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");
+
+ }
+ 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");
+
+ }
+ btf__free(new_btf);
+ free(new_raw_btf);
+ unlink(btf_path);
+ btf__free(btf);
+}
+
+void test_btf_kind(void)
+{
+ if (test__start_subtest("btf_kind_encoding"))
+ test_btf_kind_encoding();
+ if (test__start_subtest("btf_kind_decoding"))
+ test_btf_kind_decoding();
+}
--
2.31.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 bpf-next 13/17] bpf: support standalone BTF in modules
2023-11-12 12:48 [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF Alan Maguire
` (11 preceding siblings ...)
2023-11-12 12:48 ` [PATCH v4 bpf-next 12/17] selftests/bpf: test kind encoding/decoding Alan Maguire
@ 2023-11-12 12:48 ` Alan Maguire
2023-11-12 15:35 ` kernel test robot
2023-11-12 20:00 ` kernel test robot
2023-11-12 12:48 ` [PATCH v4 bpf-next 14/17] kbuild, bpf: allow opt-out from using split BTF for modules Alan Maguire
` (4 subsequent siblings)
17 siblings, 2 replies; 29+ messages in thread
From: Alan Maguire @ 2023-11-12 12:48 UTC (permalink / raw)
To: ast, daniel, andrii, jolsa
Cc: quentin, eddyz87, martin.lau, song, yonghong.song, john.fastabend,
kpsingh, sdf, haoluo, masahiroy, bpf, Alan Maguire
Not all kernel modules can be built in-tree when the core
kernel is built. This presents a problem for split BTF, because
split module BTF refers to type ids in the base kernel BTF, and
if that base kernel BTF changes (even in minor ways) those
references become invalid. Such modules then cannot take
advantage of BTF (or at least they only can until the kernel
changes enough to invalidate their vmlinux type id references).
This problem has been discussed before, and the initial approach
was to allow BTF mismatch but fail to load BTF. See [1]
for more discussion.
Generating standalone BTF for modules helps solve this problem
because the BTF generated is self-referential only. However,
tooling is geared towards split BTF - for example bpftool assumes
a module's BTF is defined relative to vmlinux BTF. To handle
this, dynamic remapping of standalone BTF is done on module
load to make it appear like split BTF - type ids and string
offsets are remapped such that they appear as they would in
split BTF. It just so happens that the BTF is self-referential.
With this approach, existing tooling works with standalone
module BTF from /sys/kernel/btf in the same way as before;
no knowledge of split versus standalone BTF is required.
To verify that the module BTF is standalone we verify that
either
1. a CRC is present for BTF, but no base CRC is specified;
this indicates that BTF was not generated relative to
base BTF; or
2. the string offsets all lie within the range
<0, string_section_size>; and
3. the BTF ids all lie within range
<0, number_of_types_in_module_BTF>
Case 1 is a fastpath available when pahole generates CRCs.
The tests carried out in 2 and 3 are fallbacks for when newer
pahole is not present.
If these conditions are true, BTF and string ids are remapped
such that they appear to be split BTF.
Note that the kfunc and dtor ids need to be remapped also
on registration, since they will be out-of-date with respect
to the remapped module BTF.
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
[1] https://lore.kernel.org/bpf/YfK18x%2FXrYL4Vw8o@syu-laptop/
---
kernel/bpf/btf.c | 304 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 302 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index a51dc3ef6a56..412184ade0f1 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -267,6 +267,7 @@ struct btf {
u32 start_str_off; /* first string offset (0 for base BTF) */
char name[MODULE_NAME_LEN];
bool kernel_btf;
+ bool standalone_btf;
};
enum verifier_phase {
@@ -5882,6 +5883,253 @@ struct btf *btf_parse_vmlinux(void)
#ifdef CONFIG_DEBUG_INFO_BTF_MODULES
+/* verify module BTF is self-contained:
+ * - if CRC is set and base CRC is not: or
+ * - no string offset should exceed standalone str len; and
+ * - the max id referenced should be <= the number of btf types in module BTF
+ *
+ * Taken together with the fact that the number of module types is much less
+ * than the number of types in vmliux BTF, these imply self-reference and hence
+ * standalone BTF.
+ */
+static bool btf_module_is_standalone(struct btf_verifier_env *env)
+{
+ u32 id_max = 0, num_ids = 0;
+ struct btf *btf = env->btf;
+ struct btf_header *hdr;
+ void *cur, *end;
+ u32 end_str_off;
+
+ hdr = &btf->hdr;
+ cur = btf->nohdr_data + hdr->type_off;
+ end = cur + hdr->type_len;
+ end_str_off = hdr->str_len;
+
+ /* Quick CRC test; if base CRC is absent while CRC is present,
+ * we know BTF was generated without reference to base BTF.
+ */
+ if (hdr->hdr_len >= sizeof(struct btf_header)) {
+ if ((hdr->flags & BTF_FLAG_CRC_SET) &&
+ !(hdr->flags & BTF_FLAG_BASE_CRC_SET))
+ return true;
+ }
+
+ while (cur < end) {
+ struct btf_type *t = cur;
+ u32 meta_size = sizeof(*t);
+ struct btf_member *member;
+ struct btf_param *param;
+ struct btf_array *array;
+ struct btf_enum64 *e64;
+ struct btf_enum *e;
+ int i, vlen;
+
+ if (t->name_off >= end_str_off)
+ return false;
+
+ switch (btf_kind(t)) {
+ case BTF_KIND_FLOAT:
+ break;
+ case BTF_KIND_PTR:
+ case BTF_KIND_FWD:
+ case BTF_KIND_TYPEDEF:
+ case BTF_KIND_VOLATILE:
+ case BTF_KIND_CONST:
+ case BTF_KIND_RESTRICT:
+ case BTF_KIND_FUNC:
+ case BTF_KIND_TYPE_TAG:
+ if (t->type > id_max)
+ id_max = t->type;
+ break;
+ case BTF_KIND_VAR:
+ if (t->type > id_max)
+ id_max = t->type;
+ meta_size += sizeof(struct btf_var);
+ break;
+ case BTF_KIND_INT:
+ meta_size += sizeof(u32);
+ break;
+ case BTF_KIND_DATASEC:
+ meta_size += btf_vlen(t) * sizeof(struct btf_var_secinfo);
+ break;
+ case BTF_KIND_ARRAY:
+ array = (struct btf_array *)(t + 1);
+ if (array->type > id_max)
+ id_max = array->type;
+ if (array->index_type > id_max)
+ id_max = array->index_type;
+ meta_size += sizeof(*array);
+ break;
+ case BTF_KIND_STRUCT:
+ case BTF_KIND_UNION:
+ member = (struct btf_member *)(t + 1);
+ vlen = btf_type_vlen(t);
+ for (i = 0; i < vlen; i++, member++) {
+ if (member->name_off >= end_str_off)
+ return false;
+ if (member->type > id_max)
+ id_max = member->type;
+ }
+ meta_size += vlen * sizeof(*member);
+ break;
+ case BTF_KIND_ENUM:
+ e = (struct btf_enum *)(t + 1);
+ vlen = btf_type_vlen(t);
+ for (i = 0; i < vlen; i++, e++) {
+ if (e->name_off > end_str_off)
+ return false;
+ }
+ meta_size += vlen * sizeof(*e);
+ break;
+ case BTF_KIND_ENUM64:
+ e64 = (struct btf_enum64 *)(t + 1);
+ vlen = btf_type_vlen(t);
+ for (i = 0; i < vlen; i++, e64++) {
+ if (e64->name_off > end_str_off)
+ return false;
+ }
+ meta_size += vlen * sizeof(*e64);
+ break;
+ case BTF_KIND_FUNC_PROTO:
+ param = (struct btf_param *)(t + 1);
+ vlen = btf_type_vlen(t);
+ for (i = 0; i < vlen; i++, param++) {
+ if (param->name_off > end_str_off)
+ return false;
+ if (param->type > id_max)
+ id_max = param->type;
+ }
+ meta_size += vlen * sizeof(*param);
+ break;
+ case BTF_KIND_DECL_TAG:
+ if (t->type > id_max)
+ id_max = t->type;
+ meta_size += sizeof(struct btf_decl_tag);
+ break;
+ }
+ cur += meta_size;
+ num_ids++;
+ }
+ /* if all standalone string checks look good and we found no references
+ * to ids higher than the number present here, we can be sure it is
+ * standalone BTF.
+ */
+ return id_max <= num_ids;
+}
+
+static u32 btf_name_off_renumber(struct btf *btf, u32 name_off)
+{
+ /* no need to renumber empty string */
+ if (name_off == 0)
+ return name_off;
+ return name_off + btf->start_str_off;
+}
+
+static u32 btf_id_renumber(struct btf *btf, u32 id)
+{
+ /* no need to renumber void type id */
+ if (id == 0)
+ return id;
+
+ return id + btf->start_id - 1;
+}
+
+/* Renumber standalone BTF to appear as split BTF; name offsets must
+ * be relative to btf->start_str_offset and ids relative to btf->start_id.
+ * When user sees BTF it will appear as normal module split BTF, the only
+ * difference being it is fully self-referential and does not refer back
+ * to vmlinux BTF (aside from 0 "void" references).
+ */
+static void btf_type_renumber(struct btf_verifier_env *env, struct btf_type *t)
+{
+ struct btf_var_secinfo *secinfo;
+ struct btf *btf = env->btf;
+ struct btf_member *member;
+ struct btf_param *param;
+ struct btf_array *array;
+ struct btf_enum64 *e64;
+ struct btf_enum *e;
+ int i, vlen;
+
+ t->name_off = btf_name_off_renumber(btf, t->name_off);
+
+ switch (BTF_INFO_KIND(t->info)) {
+ case BTF_KIND_INT:
+ case BTF_KIND_FLOAT:
+ /* nothing to renumber here, no type references */
+ break;
+ case BTF_KIND_PTR:
+ case BTF_KIND_FWD:
+ case BTF_KIND_TYPEDEF:
+ case BTF_KIND_VOLATILE:
+ case BTF_KIND_CONST:
+ case BTF_KIND_RESTRICT:
+ case BTF_KIND_FUNC:
+ case BTF_KIND_VAR:
+ case BTF_KIND_TYPE_TAG:
+ case BTF_KIND_DECL_TAG:
+ /* renumber the referenced type */
+ t->type = btf_id_renumber(btf, t->type);
+ break;
+ case BTF_KIND_ARRAY:
+ array = btf_array(t);
+ array->type = btf_id_renumber(btf, array->type);
+ array->index_type = btf_id_renumber(btf, array->index_type);
+ break;
+ case BTF_KIND_STRUCT:
+ case BTF_KIND_UNION:
+ member = (struct btf_member *)(t + 1);
+ vlen = btf_type_vlen(t);
+ for (i = 0; i < vlen; i++, member++) {
+ member->type = btf_id_renumber(btf, member->type);
+ member->name_off = btf_name_off_renumber(btf, member->name_off);
+ }
+ break;
+ case BTF_KIND_FUNC_PROTO:
+ param = (struct btf_param *)(t + 1);
+ vlen = btf_type_vlen(t);
+ for (i = 0; i < vlen; i++, param++) {
+ param->type = btf_id_renumber(btf, param->type);
+ param->name_off = btf_name_off_renumber(btf, param->name_off);
+ }
+ break;
+ case BTF_KIND_DATASEC:
+ vlen = btf_type_vlen(t);
+ secinfo = (struct btf_var_secinfo *)(t + 1);
+ for (i = 0; i < vlen; i++, secinfo++)
+ secinfo->type = btf_id_renumber(btf, secinfo->type);
+ break;
+ case BTF_KIND_ENUM:
+ e = (struct btf_enum *)(t + 1);
+ vlen = btf_type_vlen(t);
+ for (i = 0; i < vlen; i++, e++)
+ e->name_off = btf_name_off_renumber(btf, e->name_off);
+ break;
+ case BTF_KIND_ENUM64:
+ e64 = (struct btf_enum64 *)(t + 1);
+ vlen = btf_type_vlen(t);
+ for (i = 0; i < vlen; i++, e64++)
+ e64->name_off = btf_name_off_renumber(btf, e64->name_off);
+ break;
+ }
+}
+
+static void btf_renumber(struct btf_verifier_env *env, struct btf *base_btf)
+{
+ struct btf *btf = env->btf;
+ int i;
+
+ btf->start_id = base_btf->nr_types;
+ btf->start_str_off = base_btf->hdr.str_len;
+ btf->base_btf = base_btf;
+
+ for (i = 1; i < btf->nr_types; i++)
+ btf_type_renumber(env, btf->types[i]);
+ /* skip past unneeded void type (we use base id 0 instead) */
+ btf->types++;
+ btf->nr_types--;
+}
+
static struct btf *btf_parse_module(const char *module_name, const void *data, unsigned int data_size)
{
struct btf_verifier_env *env = NULL;
@@ -5933,10 +6181,29 @@ static struct btf *btf_parse_module(const char *module_name, const void *data, u
if (err)
goto errout;
+ if (btf_module_is_standalone(env)) {
+ /* BTF may be standalone; in that case BTF ids and strings
+ * will all be self-referential.
+ *
+ * Later on, once we have checked all metas, we will
+ * retain start id from base BTF so it will look like
+ * split BTF (but is self-contained); renumbering is done
+ * also to give the split BTF-like appearance and not
+ * confuse pahole which assumes split BTF for modules.
+ */
+ btf->base_btf = NULL;
+ btf->start_id = 0;
+ btf->nr_types = 0;
+ btf->start_str_off = 0;
+ btf->standalone_btf = true;
+ }
err = btf_check_all_metas(env);
if (err)
goto errout;
+ if (btf->standalone_btf)
+ btf_renumber(env, base_btf);
+
err = btf_check_type_tags(env, btf, btf_nr_types(base_btf));
if (err)
goto errout;
@@ -7876,6 +8143,16 @@ static int btf_populate_kfunc_set(struct btf *btf, enum btf_kfunc_hook hook,
/* Concatenate the two sets */
memcpy(set->pairs + set->cnt, add_set->pairs, add_set->cnt * sizeof(set->pairs[0]));
+ if (btf->standalone_btf) {
+ u32 i;
+
+ /* renumber BTF ids since BTF is standalone and has been mapped to look
+ * like split BTF, while BTF kfunc ids are still old unmapped values.
+ */
+ for (i = set->cnt; i < set->cnt + add_set->cnt; i++)
+ set->pairs[i].id = btf_id_renumber(btf, set->pairs[i].id);
+ }
+
set->cnt += add_set->cnt;
sort(set->pairs, set->cnt, sizeof(set->pairs[0]), btf_id_cmp_func, NULL);
@@ -7936,7 +8213,6 @@ static int bpf_prog_type_to_kfunc_hook(enum bpf_prog_type prog_type)
case BPF_PROG_TYPE_SYSCALL:
return BTF_KFUNC_HOOK_SYSCALL;
case BPF_PROG_TYPE_CGROUP_SKB:
- case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
return BTF_KFUNC_HOOK_CGROUP_SKB;
case BPF_PROG_TYPE_SCHED_ACT:
return BTF_KFUNC_HOOK_SCHED_ACT;
@@ -8005,7 +8281,14 @@ static int __register_btf_kfunc_id_set(enum btf_kfunc_hook hook,
return PTR_ERR(btf);
for (i = 0; i < kset->set->cnt; i++) {
- ret = btf_check_kfunc_protos(btf, kset->set->pairs[i].id,
+ u32 kfunc_id = kset->set->pairs[i].id;
+
+ /* standalone BTF renumbers BTF ids to make them appear as split BTF;
+ * resolve_btfids has the old BTF ids so we need to renumber here.
+ */
+ if (btf->standalone_btf)
+ kfunc_id = btf_id_renumber(btf, kfunc_id);
+ ret = btf_check_kfunc_protos(btf, kfunc_id,
kset->set->pairs[i].flags);
if (ret)
goto err_out;
@@ -8063,6 +8346,12 @@ static int btf_check_dtor_kfuncs(struct btf *btf, const struct btf_id_dtor_kfunc
for (i = 0; i < cnt; i++) {
dtor_btf_id = dtors[i].kfunc_btf_id;
+ /* standalone BTF renumbers BTF ids to make them appear as split BTF;
+ * resolve_btfids has the old BTF ids so we need to renumber here.
+ */
+ if (btf->standalone_btf)
+ dtor_btf_id = btf_id_renumber(btf, dtor_btf_id);
+
dtor_func = btf_type_by_id(btf, dtor_btf_id);
if (!dtor_func || !btf_type_is_func(dtor_func))
return -EINVAL;
@@ -8156,6 +8445,17 @@ int register_btf_id_dtor_kfuncs(const struct btf_id_dtor_kfunc *dtors, u32 add_c
btf->dtor_kfunc_tab = tab;
memcpy(tab->dtors + tab->cnt, dtors, add_cnt * sizeof(tab->dtors[0]));
+ if (btf->standalone_btf) {
+ u32 i;
+
+ /* renumber BTF ids since BTF is standalone and has been mapped to look
+ * like split BTF, while BTF dtor ids are still old unmapped values.
+ */
+ for (i = tab->cnt; i < tab->cnt + add_cnt; i++) {
+ tab->dtors[i].btf_id = btf_id_renumber(btf, tab->dtors[i].btf_id);
+ tab->dtors[i].kfunc_btf_id = btf_id_renumber(btf, tab->dtors[i].kfunc_btf_id);
+ }
+ }
tab->cnt += add_cnt;
sort(tab->dtors, tab->cnt, sizeof(tab->dtors[0]), btf_id_cmp_func, NULL);
--
2.31.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v4 bpf-next 13/17] bpf: support standalone BTF in modules
2023-11-12 12:48 ` [PATCH v4 bpf-next 13/17] bpf: support standalone BTF in modules Alan Maguire
@ 2023-11-12 15:35 ` kernel test robot
2023-11-12 20:00 ` kernel test robot
1 sibling, 0 replies; 29+ messages in thread
From: kernel test robot @ 2023-11-12 15:35 UTC (permalink / raw)
To: Alan Maguire, ast, daniel, andrii, jolsa
Cc: oe-kbuild-all, quentin, eddyz87, martin.lau, song, yonghong.song,
john.fastabend, kpsingh, sdf, haoluo, masahiroy, bpf,
Alan Maguire
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/20231112-205314
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link: https://lore.kernel.org/r/20231112124834.388735-14-alan.maguire%40oracle.com
patch subject: [PATCH v4 bpf-next 13/17] bpf: support standalone BTF in modules
config: nios2-allmodconfig (https://download.01.org/0day-ci/archive/20231112/202311122307.bjlq3XJ5-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231112/202311122307.bjlq3XJ5-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/202311122307.bjlq3XJ5-lkp@intel.com/
All errors (new ones prefixed by >>):
kernel/bpf/btf.c: In function 'btf_seq_show':
kernel/bpf/btf.c:7447:29: warning: function 'btf_seq_show' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format]
7447 | seq_vprintf((struct seq_file *)show->target, fmt, args);
| ^~~~~~~~
kernel/bpf/btf.c: In function 'btf_snprintf_show':
kernel/bpf/btf.c:7484:9: warning: function 'btf_snprintf_show' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format]
7484 | len = vsnprintf(show->target, ssnprintf->len_left, fmt, args);
| ^~~
kernel/bpf/btf.c: In function 'btf_populate_kfunc_set':
>> kernel/bpf/btf.c:8153:44: error: implicit declaration of function 'btf_id_renumber'; did you mean 'btf_is_enum64'? [-Werror=implicit-function-declaration]
8153 | set->pairs[i].id = btf_id_renumber(btf, set->pairs[i].id);
| ^~~~~~~~~~~~~~~
| btf_is_enum64
cc1: some warnings being treated as errors
vim +8153 kernel/bpf/btf.c
8046
8047 static int btf_populate_kfunc_set(struct btf *btf, enum btf_kfunc_hook hook,
8048 const struct btf_kfunc_id_set *kset)
8049 {
8050 struct btf_kfunc_hook_filter *hook_filter;
8051 struct btf_id_set8 *add_set = kset->set;
8052 bool vmlinux_set = !btf_is_module(btf);
8053 bool add_filter = !!kset->filter;
8054 struct btf_kfunc_set_tab *tab;
8055 struct btf_id_set8 *set;
8056 u32 set_cnt;
8057 int ret;
8058
8059 if (hook >= BTF_KFUNC_HOOK_MAX) {
8060 ret = -EINVAL;
8061 goto end;
8062 }
8063
8064 if (!add_set->cnt)
8065 return 0;
8066
8067 tab = btf->kfunc_set_tab;
8068
8069 if (tab && add_filter) {
8070 u32 i;
8071
8072 hook_filter = &tab->hook_filters[hook];
8073 for (i = 0; i < hook_filter->nr_filters; i++) {
8074 if (hook_filter->filters[i] == kset->filter) {
8075 add_filter = false;
8076 break;
8077 }
8078 }
8079
8080 if (add_filter && hook_filter->nr_filters == BTF_KFUNC_FILTER_MAX_CNT) {
8081 ret = -E2BIG;
8082 goto end;
8083 }
8084 }
8085
8086 if (!tab) {
8087 tab = kzalloc(sizeof(*tab), GFP_KERNEL | __GFP_NOWARN);
8088 if (!tab)
8089 return -ENOMEM;
8090 btf->kfunc_set_tab = tab;
8091 }
8092
8093 set = tab->sets[hook];
8094 /* Warn when register_btf_kfunc_id_set is called twice for the same hook
8095 * for module sets.
8096 */
8097 if (WARN_ON_ONCE(set && !vmlinux_set)) {
8098 ret = -EINVAL;
8099 goto end;
8100 }
8101
8102 /* We don't need to allocate, concatenate, and sort module sets, because
8103 * only one is allowed per hook. Hence, we can directly assign the
8104 * pointer and return.
8105 */
8106 if (!vmlinux_set) {
8107 tab->sets[hook] = add_set;
8108 goto do_add_filter;
8109 }
8110
8111 /* In case of vmlinux sets, there may be more than one set being
8112 * registered per hook. To create a unified set, we allocate a new set
8113 * and concatenate all individual sets being registered. While each set
8114 * is individually sorted, they may become unsorted when concatenated,
8115 * hence re-sorting the final set again is required to make binary
8116 * searching the set using btf_id_set8_contains function work.
8117 */
8118 set_cnt = set ? set->cnt : 0;
8119
8120 if (set_cnt > U32_MAX - add_set->cnt) {
8121 ret = -EOVERFLOW;
8122 goto end;
8123 }
8124
8125 if (set_cnt + add_set->cnt > BTF_KFUNC_SET_MAX_CNT) {
8126 ret = -E2BIG;
8127 goto end;
8128 }
8129
8130 /* Grow set */
8131 set = krealloc(tab->sets[hook],
8132 offsetof(struct btf_id_set8, pairs[set_cnt + add_set->cnt]),
8133 GFP_KERNEL | __GFP_NOWARN);
8134 if (!set) {
8135 ret = -ENOMEM;
8136 goto end;
8137 }
8138
8139 /* For newly allocated set, initialize set->cnt to 0 */
8140 if (!tab->sets[hook])
8141 set->cnt = 0;
8142 tab->sets[hook] = set;
8143
8144 /* Concatenate the two sets */
8145 memcpy(set->pairs + set->cnt, add_set->pairs, add_set->cnt * sizeof(set->pairs[0]));
8146 if (btf->standalone_btf) {
8147 u32 i;
8148
8149 /* renumber BTF ids since BTF is standalone and has been mapped to look
8150 * like split BTF, while BTF kfunc ids are still old unmapped values.
8151 */
8152 for (i = set->cnt; i < set->cnt + add_set->cnt; i++)
> 8153 set->pairs[i].id = btf_id_renumber(btf, set->pairs[i].id);
8154 }
8155
8156 set->cnt += add_set->cnt;
8157
8158 sort(set->pairs, set->cnt, sizeof(set->pairs[0]), btf_id_cmp_func, NULL);
8159
8160 do_add_filter:
8161 if (add_filter) {
8162 hook_filter = &tab->hook_filters[hook];
8163 hook_filter->filters[hook_filter->nr_filters++] = kset->filter;
8164 }
8165 return 0;
8166 end:
8167 btf_free_kfunc_set_tab(btf);
8168 return ret;
8169 }
8170
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v4 bpf-next 13/17] bpf: support standalone BTF in modules
2023-11-12 12:48 ` [PATCH v4 bpf-next 13/17] bpf: support standalone BTF in modules Alan Maguire
2023-11-12 15:35 ` kernel test robot
@ 2023-11-12 20:00 ` kernel test robot
1 sibling, 0 replies; 29+ messages in thread
From: kernel test robot @ 2023-11-12 20:00 UTC (permalink / raw)
To: Alan Maguire, ast, daniel, andrii, jolsa
Cc: llvm, oe-kbuild-all, quentin, eddyz87, martin.lau, song,
yonghong.song, john.fastabend, kpsingh, sdf, haoluo, masahiroy,
bpf, Alan Maguire
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/20231112-205314
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link: https://lore.kernel.org/r/20231112124834.388735-14-alan.maguire%40oracle.com
patch subject: [PATCH v4 bpf-next 13/17] bpf: support standalone BTF in modules
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20231113/202311130324.ONOHc3XN-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231113/202311130324.ONOHc3XN-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/202311130324.ONOHc3XN-lkp@intel.com/
All errors (new ones prefixed by >>):
>> kernel/bpf/btf.c:8153:23: error: call to undeclared function 'btf_id_renumber'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
set->pairs[i].id = btf_id_renumber(btf, set->pairs[i].id);
^
kernel/bpf/btf.c:8290:15: error: call to undeclared function 'btf_id_renumber'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
kfunc_id = btf_id_renumber(btf, kfunc_id);
^
kernel/bpf/btf.c:8353:18: error: call to undeclared function 'btf_id_renumber'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
dtor_btf_id = btf_id_renumber(btf, dtor_btf_id);
^
kernel/bpf/btf.c:8455:27: error: call to undeclared function 'btf_id_renumber'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
tab->dtors[i].btf_id = btf_id_renumber(btf, tab->dtors[i].btf_id);
^
4 errors generated.
vim +/btf_id_renumber +8153 kernel/bpf/btf.c
8046
8047 static int btf_populate_kfunc_set(struct btf *btf, enum btf_kfunc_hook hook,
8048 const struct btf_kfunc_id_set *kset)
8049 {
8050 struct btf_kfunc_hook_filter *hook_filter;
8051 struct btf_id_set8 *add_set = kset->set;
8052 bool vmlinux_set = !btf_is_module(btf);
8053 bool add_filter = !!kset->filter;
8054 struct btf_kfunc_set_tab *tab;
8055 struct btf_id_set8 *set;
8056 u32 set_cnt;
8057 int ret;
8058
8059 if (hook >= BTF_KFUNC_HOOK_MAX) {
8060 ret = -EINVAL;
8061 goto end;
8062 }
8063
8064 if (!add_set->cnt)
8065 return 0;
8066
8067 tab = btf->kfunc_set_tab;
8068
8069 if (tab && add_filter) {
8070 u32 i;
8071
8072 hook_filter = &tab->hook_filters[hook];
8073 for (i = 0; i < hook_filter->nr_filters; i++) {
8074 if (hook_filter->filters[i] == kset->filter) {
8075 add_filter = false;
8076 break;
8077 }
8078 }
8079
8080 if (add_filter && hook_filter->nr_filters == BTF_KFUNC_FILTER_MAX_CNT) {
8081 ret = -E2BIG;
8082 goto end;
8083 }
8084 }
8085
8086 if (!tab) {
8087 tab = kzalloc(sizeof(*tab), GFP_KERNEL | __GFP_NOWARN);
8088 if (!tab)
8089 return -ENOMEM;
8090 btf->kfunc_set_tab = tab;
8091 }
8092
8093 set = tab->sets[hook];
8094 /* Warn when register_btf_kfunc_id_set is called twice for the same hook
8095 * for module sets.
8096 */
8097 if (WARN_ON_ONCE(set && !vmlinux_set)) {
8098 ret = -EINVAL;
8099 goto end;
8100 }
8101
8102 /* We don't need to allocate, concatenate, and sort module sets, because
8103 * only one is allowed per hook. Hence, we can directly assign the
8104 * pointer and return.
8105 */
8106 if (!vmlinux_set) {
8107 tab->sets[hook] = add_set;
8108 goto do_add_filter;
8109 }
8110
8111 /* In case of vmlinux sets, there may be more than one set being
8112 * registered per hook. To create a unified set, we allocate a new set
8113 * and concatenate all individual sets being registered. While each set
8114 * is individually sorted, they may become unsorted when concatenated,
8115 * hence re-sorting the final set again is required to make binary
8116 * searching the set using btf_id_set8_contains function work.
8117 */
8118 set_cnt = set ? set->cnt : 0;
8119
8120 if (set_cnt > U32_MAX - add_set->cnt) {
8121 ret = -EOVERFLOW;
8122 goto end;
8123 }
8124
8125 if (set_cnt + add_set->cnt > BTF_KFUNC_SET_MAX_CNT) {
8126 ret = -E2BIG;
8127 goto end;
8128 }
8129
8130 /* Grow set */
8131 set = krealloc(tab->sets[hook],
8132 offsetof(struct btf_id_set8, pairs[set_cnt + add_set->cnt]),
8133 GFP_KERNEL | __GFP_NOWARN);
8134 if (!set) {
8135 ret = -ENOMEM;
8136 goto end;
8137 }
8138
8139 /* For newly allocated set, initialize set->cnt to 0 */
8140 if (!tab->sets[hook])
8141 set->cnt = 0;
8142 tab->sets[hook] = set;
8143
8144 /* Concatenate the two sets */
8145 memcpy(set->pairs + set->cnt, add_set->pairs, add_set->cnt * sizeof(set->pairs[0]));
8146 if (btf->standalone_btf) {
8147 u32 i;
8148
8149 /* renumber BTF ids since BTF is standalone and has been mapped to look
8150 * like split BTF, while BTF kfunc ids are still old unmapped values.
8151 */
8152 for (i = set->cnt; i < set->cnt + add_set->cnt; i++)
> 8153 set->pairs[i].id = btf_id_renumber(btf, set->pairs[i].id);
8154 }
8155
8156 set->cnt += add_set->cnt;
8157
8158 sort(set->pairs, set->cnt, sizeof(set->pairs[0]), btf_id_cmp_func, NULL);
8159
8160 do_add_filter:
8161 if (add_filter) {
8162 hook_filter = &tab->hook_filters[hook];
8163 hook_filter->filters[hook_filter->nr_filters++] = kset->filter;
8164 }
8165 return 0;
8166 end:
8167 btf_free_kfunc_set_tab(btf);
8168 return ret;
8169 }
8170
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v4 bpf-next 14/17] kbuild, bpf: allow opt-out from using split BTF for modules
2023-11-12 12:48 [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF Alan Maguire
` (12 preceding siblings ...)
2023-11-12 12:48 ` [PATCH v4 bpf-next 13/17] bpf: support standalone BTF in modules Alan Maguire
@ 2023-11-12 12:48 ` Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 15/17] selftests/bpf: generalize module load to support specifying a module name Alan Maguire
` (3 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Alan Maguire @ 2023-11-12 12:48 UTC (permalink / raw)
To: ast, daniel, andrii, jolsa
Cc: quentin, eddyz87, martin.lau, song, yonghong.song, john.fastabend,
kpsingh, sdf, haoluo, masahiroy, bpf, Alan Maguire
By having a BTF_BASE variable defaulting to using vmlinux
as base BTF, we allow module builders to build standalone
BTF such that it is generated independently and not
de-duplicated with core vmlinux BTF. This allows such
modules to be more resilient to changes in vmlinux BTF
if they occur, as would happen if a change resulted in
a different vmlinux BTF id mapping.
Opt-out of split BTF is done via
make BTF_BASE= M=path/2/module
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
scripts/Makefile.btf | 3 +++
scripts/Makefile.modfinal | 4 ++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/scripts/Makefile.btf b/scripts/Makefile.btf
index f8ce33d7f9bb..352271a10fb5 100644
--- a/scripts/Makefile.btf
+++ b/scripts/Makefile.btf
@@ -19,3 +19,6 @@ pahole-flags-$(call test-ge, $(pahole-ver), 125) += --skip_encoding_btf_inconsis
pahole-flags-$(call test-ge, $(pahole-ver), 126) = -j --lang_exclude=rust --btf_features=encode_force,var,float,decl_tag,type_tag,enum64,optimized_func,consistent_func,crc,kind_layout
export PAHOLE_FLAGS := $(pahole-flags-y)
+
+# Allow opt-out of split BTF by overriding BTF_BASE
+export BTF_BASE := --btf_base vmlinux
diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
index 8568d256d6fb..3400d1a72127 100644
--- a/scripts/Makefile.modfinal
+++ b/scripts/Makefile.modfinal
@@ -39,8 +39,8 @@ quiet_cmd_btf_ko = BTF [M] $@
if [ ! -f vmlinux ]; then \
printf "Skipping BTF generation for %s due to unavailability of vmlinux\n" $@ 1>&2; \
else \
- LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \
- $(RESOLVE_BTFIDS) -b vmlinux $@; \
+ LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) $(BTF_BASE) $@; \
+ $(RESOLVE_BTFIDS) $(BTF_BASE) $@; \
fi;
# Same as newer-prereqs, but allows to exclude specified extra dependencies
--
2.31.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 bpf-next 15/17] selftests/bpf: generalize module load to support specifying a module name
2023-11-12 12:48 [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF Alan Maguire
` (13 preceding siblings ...)
2023-11-12 12:48 ` [PATCH v4 bpf-next 14/17] kbuild, bpf: allow opt-out from using split BTF for modules Alan Maguire
@ 2023-11-12 12:48 ` Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 16/17] selftests/bpf: build separate bpf_testmod module with standalone BTF Alan Maguire
` (2 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Alan Maguire @ 2023-11-12 12:48 UTC (permalink / raw)
To: ast, daniel, andrii, jolsa
Cc: quentin, eddyz87, martin.lau, song, yonghong.song, john.fastabend,
kpsingh, sdf, haoluo, masahiroy, bpf, Alan Maguire
This will be used in testing standalone module BTF.
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
.../selftests/bpf/prog_tests/bpf_mod_race.c | 8 +++----
.../selftests/bpf/prog_tests/module_attach.c | 6 ++---
tools/testing/selftests/bpf/test_progs.c | 6 ++---
tools/testing/selftests/bpf/test_verifier.c | 6 ++---
tools/testing/selftests/bpf/testing_helpers.c | 24 ++++++++++---------
tools/testing/selftests/bpf/testing_helpers.h | 4 ++--
6 files changed, 28 insertions(+), 26 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_mod_race.c b/tools/testing/selftests/bpf/prog_tests/bpf_mod_race.c
index fe2c502e5089..c4aeb40390a3 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_mod_race.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_mod_race.c
@@ -48,7 +48,7 @@ static _Atomic enum bpf_test_state state = _TS_INVALID;
static void *load_module_thread(void *p)
{
- if (!ASSERT_NEQ(load_bpf_testmod(false), 0, "load_module_thread must fail"))
+ if (!ASSERT_NEQ(load_bpf_testmod("bpf_testmod", false), 0, "load_module_thread must fail"))
atomic_store(&state, TS_MODULE_LOAD);
else
atomic_store(&state, TS_MODULE_LOAD_FAIL);
@@ -100,7 +100,7 @@ static void test_bpf_mod_race_config(const struct test_config *config)
if (!ASSERT_NEQ(fault_addr, MAP_FAILED, "mmap for uffd registration"))
return;
- if (!ASSERT_OK(unload_bpf_testmod(false), "unload bpf_testmod"))
+ if (!ASSERT_OK(unload_bpf_testmod("bpf_testmod", false), "unload bpf_testmod"))
goto end_mmap;
skel = bpf_mod_race__open();
@@ -178,8 +178,8 @@ static void test_bpf_mod_race_config(const struct test_config *config)
bpf_mod_race__destroy(skel);
ASSERT_OK(kern_sync_rcu(), "kern_sync_rcu");
end_module:
- unload_bpf_testmod(false);
- ASSERT_OK(load_bpf_testmod(false), "restore bpf_testmod");
+ unload_bpf_testmod("bpf_testmod", false);
+ ASSERT_OK(load_bpf_testmod("bpf_testmod", false), "restore bpf_testmod");
end_mmap:
munmap(fault_addr, 4096);
atomic_store(&state, _TS_INVALID);
diff --git a/tools/testing/selftests/bpf/prog_tests/module_attach.c b/tools/testing/selftests/bpf/prog_tests/module_attach.c
index f53d658ed080..9f1f00c63d30 100644
--- a/tools/testing/selftests/bpf/prog_tests/module_attach.c
+++ b/tools/testing/selftests/bpf/prog_tests/module_attach.c
@@ -89,21 +89,21 @@ void test_module_attach(void)
if (!ASSERT_OK_PTR(link, "attach_fentry"))
goto cleanup;
- ASSERT_ERR(unload_bpf_testmod(false), "unload_bpf_testmod");
+ ASSERT_ERR(unload_bpf_testmod("bpf_testmod", false), "unload_bpf_testmod");
bpf_link__destroy(link);
link = bpf_program__attach(skel->progs.handle_fexit);
if (!ASSERT_OK_PTR(link, "attach_fexit"))
goto cleanup;
- ASSERT_ERR(unload_bpf_testmod(false), "unload_bpf_testmod");
+ ASSERT_ERR(unload_bpf_testmod("bpf_testmod", false), "unload_bpf_testmod");
bpf_link__destroy(link);
link = bpf_program__attach(skel->progs.kprobe_multi);
if (!ASSERT_OK_PTR(link, "attach_kprobe_multi"))
goto cleanup;
- ASSERT_ERR(unload_bpf_testmod(false), "unload_bpf_testmod");
+ ASSERT_ERR(unload_bpf_testmod("bpf_testmod", false), "unload_bpf_testmod");
bpf_link__destroy(link);
cleanup:
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 1b9387890148..a3a89743e7aa 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -1659,9 +1659,9 @@ int main(int argc, char **argv)
env.has_testmod = true;
if (!env.list_test_names) {
/* ensure previous instance of the module is unloaded */
- unload_bpf_testmod(verbose());
+ unload_bpf_testmod("bpf_testmod", verbose());
- if (load_bpf_testmod(verbose())) {
+ if (load_bpf_testmod("bpf_testmod", verbose())) {
fprintf(env.stderr, "WARNING! Selftests relying on bpf_testmod.ko will be skipped.\n");
env.has_testmod = false;
}
@@ -1761,7 +1761,7 @@ int main(int argc, char **argv)
close(env.saved_netns_fd);
out:
if (!env.list_test_names && env.has_testmod)
- unload_bpf_testmod(verbose());
+ unload_bpf_testmod("bpf_testmod", verbose());
free_test_selector(&env.test_selector);
free_test_selector(&env.subtest_selector);
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 98107e0452d3..b712424d6a10 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -1804,9 +1804,9 @@ static int do_test(bool unpriv, unsigned int from, unsigned int to)
int i, passes = 0, errors = 0;
/* ensure previous instance of the module is unloaded */
- unload_bpf_testmod(verbose);
+ unload_bpf_testmod("bpf_testmod", verbose);
- if (load_bpf_testmod(verbose))
+ if (load_bpf_testmod("bpf_testmod", verbose))
return EXIT_FAILURE;
for (i = from; i < to; i++) {
@@ -1836,7 +1836,7 @@ static int do_test(bool unpriv, unsigned int from, unsigned int to)
}
}
- unload_bpf_testmod(verbose);
+ unload_bpf_testmod("bpf_testmod", verbose);
kfuncs_cleanup();
printf("Summary: %d PASSED, %d SKIPPED, %d FAILED\n", passes,
diff --git a/tools/testing/selftests/bpf/testing_helpers.c b/tools/testing/selftests/bpf/testing_helpers.c
index 8d994884c7b4..d5cde3f298f1 100644
--- a/tools/testing/selftests/bpf/testing_helpers.c
+++ b/tools/testing/selftests/bpf/testing_helpers.c
@@ -338,45 +338,47 @@ static int delete_module(const char *name, int flags)
return syscall(__NR_delete_module, name, flags);
}
-int unload_bpf_testmod(bool verbose)
+int unload_bpf_testmod(const char *name, bool verbose)
{
if (kern_sync_rcu())
fprintf(stdout, "Failed to trigger kernel-side RCU sync!\n");
- if (delete_module("bpf_testmod", 0)) {
+ if (delete_module(name, 0)) {
if (errno == ENOENT) {
if (verbose)
- fprintf(stdout, "bpf_testmod.ko is already unloaded.\n");
+ fprintf(stdout, "%s.ko is already unloaded.\n", name);
return -1;
}
- fprintf(stdout, "Failed to unload bpf_testmod.ko from kernel: %d\n", -errno);
+ fprintf(stdout, "Failed to unload %s.ko from kernel: %d\n", name, -errno);
return -1;
}
if (verbose)
- fprintf(stdout, "Successfully unloaded bpf_testmod.ko.\n");
+ fprintf(stdout, "Successfully unloaded %s.ko.\n", name);
return 0;
}
-int load_bpf_testmod(bool verbose)
+int load_bpf_testmod(const char *name, bool verbose)
{
+ char koname[PATH_MAX];
int fd;
if (verbose)
- fprintf(stdout, "Loading bpf_testmod.ko...\n");
+ fprintf(stdout, "Loading %s.ko...\n", name);
- fd = open("bpf_testmod.ko", O_RDONLY);
+ snprintf(koname, sizeof(koname), "%s.ko", name);
+ fd = open(koname, O_RDONLY);
if (fd < 0) {
- fprintf(stdout, "Can't find bpf_testmod.ko kernel module: %d\n", -errno);
+ fprintf(stdout, "Can't find %s.ko kernel module: %d\n", name, -errno);
return -ENOENT;
}
if (finit_module(fd, "", 0)) {
- fprintf(stdout, "Failed to load bpf_testmod.ko into the kernel: %d\n", -errno);
+ fprintf(stdout, "Failed to load %s.ko into the kernel: %d\n", name, -errno);
close(fd);
return -EINVAL;
}
close(fd);
if (verbose)
- fprintf(stdout, "Successfully loaded bpf_testmod.ko.\n");
+ fprintf(stdout, "Successfully loaded %s.ko.\n", name);
return 0;
}
diff --git a/tools/testing/selftests/bpf/testing_helpers.h b/tools/testing/selftests/bpf/testing_helpers.h
index 5b7a55136741..831329ad5091 100644
--- a/tools/testing/selftests/bpf/testing_helpers.h
+++ b/tools/testing/selftests/bpf/testing_helpers.h
@@ -30,8 +30,8 @@ int parse_test_list_file(const char *path,
bool is_glob_pattern);
__u64 read_perf_max_sample_freq(void);
-int load_bpf_testmod(bool verbose);
-int unload_bpf_testmod(bool verbose);
+int load_bpf_testmod(const char *name, bool verbose);
+int unload_bpf_testmod(const char *name, bool verbose);
int kern_sync_rcu(void);
static inline __u64 get_time_ns(void)
--
2.31.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 bpf-next 16/17] selftests/bpf: build separate bpf_testmod module with standalone BTF
2023-11-12 12:48 [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF Alan Maguire
` (14 preceding siblings ...)
2023-11-12 12:48 ` [PATCH v4 bpf-next 15/17] selftests/bpf: generalize module load to support specifying a module name Alan Maguire
@ 2023-11-12 12:48 ` Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 17/17] selftests/bpf: update btf_module test to ensure standalone BTF works Alan Maguire
2023-11-14 20:16 ` [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF Alan Maguire
17 siblings, 0 replies; 29+ messages in thread
From: Alan Maguire @ 2023-11-12 12:48 UTC (permalink / raw)
To: ast, daniel, andrii, jolsa
Cc: quentin, eddyz87, martin.lau, song, yonghong.song, john.fastabend,
kpsingh, sdf, haoluo, masahiroy, bpf, Alan Maguire
From bpf_testmod.c, build bpf_testmod_standalone.ko which is identical to
bpf_testmod.ko aside from being built with standalone BTF, and having enough
differences in names such that both can be loaded simultaneously with
bpf_testmod. It will be used to test standalone BTF loading.
bpf_testmod_standalone* files were generated mainly via running
sed 's/bpf_testmod/bpf_testmod_standalone/g'
...on the relevant files, and manually removing bpf_fentry_shadow_test()
from bpf_testmod_standalone.c to avoid a symbol clash that would prevent
it being loaded at the same time as bpf_testmod. kfunc iters were also
renamed.
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
.../selftests/bpf/bpf_testmod/Makefile | 10 +-
.../bpf_testmod_standalone-events.h | 57 ++
.../bpf/bpf_testmod/bpf_testmod_standalone.c | 551 ++++++++++++++++++
.../bpf/bpf_testmod/bpf_testmod_standalone.h | 31 +
.../bpf_testmod_standalone_kfunc.h | 109 ++++
5 files changed, 755 insertions(+), 3 deletions(-)
create mode 100644 tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_standalone-events.h
create mode 100644 tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_standalone.c
create mode 100644 tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_standalone.h
create mode 100644 tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_standalone_kfunc.h
diff --git a/tools/testing/selftests/bpf/bpf_testmod/Makefile b/tools/testing/selftests/bpf/bpf_testmod/Makefile
index 15cb36c4483a..d7ca383b1b0d 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/Makefile
+++ b/tools/testing/selftests/bpf/bpf_testmod/Makefile
@@ -7,13 +7,17 @@ else
Q = @
endif
-MODULES = bpf_testmod.ko
+MODULES = bpf_testmod.ko bpf_testmod_standalone.ko
+
+obj-m += bpf_testmod.o
+obj-m += bpf_testmod_standalone.o
-obj-m += bpf_testmod.o
CFLAGS_bpf_testmod.o = -I$(src)
+CFLAGS_bpf_testmod_standalone.o += -I$(src)
all:
- +$(Q)make -C $(KDIR) M=$(BPF_TESTMOD_DIR) modules
+ +$(Q)make -C $(KDIR) M=$(BPF_TESTMOD_DIR) bpf_testmod.ko
+ +$(Q)make -C $(KDIR) BTF_BASE= M=$(BPF_TESTMOD_DIR) bpf_testmod_standalone.ko
clean:
+$(Q)make -C $(KDIR) M=$(BPF_TESTMOD_DIR) clean
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_standalone-events.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_standalone-events.h
new file mode 100644
index 000000000000..ee1c668b0e3b
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_standalone-events.h
@@ -0,0 +1,57 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2020 Facebook */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM bpf_testmod_standalone
+
+#if !defined(_BPF_TESTMOD_EVENTS_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _BPF_TESTMOD_EVENTS_H
+
+#include <linux/tracepoint.h>
+#include "bpf_testmod_standalone.h"
+
+TRACE_EVENT(bpf_testmod_standalone_test_read,
+ TP_PROTO(struct task_struct *task, struct bpf_testmod_standalone_test_read_ctx *ctx),
+ TP_ARGS(task, ctx),
+ TP_STRUCT__entry(
+ __field(pid_t, pid)
+ __array(char, comm, TASK_COMM_LEN)
+ __field(loff_t, off)
+ __field(size_t, len)
+ ),
+ TP_fast_assign(
+ __entry->pid = task->pid;
+ memcpy(__entry->comm, task->comm, TASK_COMM_LEN);
+ __entry->off = ctx->off;
+ __entry->len = ctx->len;
+ ),
+ TP_printk("pid=%d comm=%s off=%llu len=%zu",
+ __entry->pid, __entry->comm, __entry->off, __entry->len)
+);
+
+/* A bare tracepoint with no event associated with it */
+DECLARE_TRACE(bpf_testmod_standalone_test_write_bare,
+ TP_PROTO(struct task_struct *task, struct bpf_testmod_standalone_test_write_ctx *ctx),
+ TP_ARGS(task, ctx)
+);
+
+#undef BPF_TESTMOD_DECLARE_TRACE
+#ifdef DECLARE_TRACE_WRITABLE
+#define BPF_TESTMOD_DECLARE_TRACE(call, proto, args, size) \
+ DECLARE_TRACE_WRITABLE(call, PARAMS(proto), PARAMS(args), size)
+#else
+#define BPF_TESTMOD_DECLARE_TRACE(call, proto, args, size) \
+ DECLARE_TRACE(call, PARAMS(proto), PARAMS(args))
+#endif
+
+BPF_TESTMOD_DECLARE_TRACE(bpf_testmod_standalone_test_writable_bare,
+ TP_PROTO(struct bpf_testmod_standalone_test_writable_ctx *ctx),
+ TP_ARGS(ctx),
+ sizeof(struct bpf_testmod_standalone_test_writable_ctx)
+);
+
+#endif /* _BPF_TESTMOD_EVENTS_H */
+
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+#define TRACE_INCLUDE_FILE bpf_testmod_standalone-events
+#include <trace/define_trace.h>
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_standalone.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_standalone.c
new file mode 100644
index 000000000000..a0d7d1ade6ff
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_standalone.c
@@ -0,0 +1,551 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2020 Facebook */
+#include <linux/btf.h>
+#include <linux/btf_ids.h>
+#include <linux/error-injection.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/percpu-defs.h>
+#include <linux/sysfs.h>
+#include <linux/tracepoint.h>
+#include "bpf_testmod_standalone.h"
+#include "bpf_testmod_standalone_kfunc.h"
+
+#define CREATE_TRACE_POINTS
+#include "bpf_testmod_standalone-events.h"
+
+typedef int (*func_proto_typedef)(long);
+typedef int (*func_proto_typedef_nested1)(func_proto_typedef);
+typedef int (*func_proto_typedef_nested2)(func_proto_typedef_nested1);
+
+DEFINE_PER_CPU(int, bpf_testmod_standalone_ksym_percpu) = 123;
+long bpf_testmod_standalone_test_struct_arg_result;
+
+struct bpf_testmod_standalone_struct_arg_1 {
+ int a;
+};
+struct bpf_testmod_standalone_struct_arg_2 {
+ long a;
+ long b;
+};
+
+struct bpf_testmod_standalone_struct_arg_3 {
+ int a;
+ int b[];
+};
+
+struct bpf_testmod_standalone_struct_arg_4 {
+ u64 a;
+ int b;
+};
+
+__diag_push();
+__diag_ignore_all("-Wmissing-prototypes",
+ "Global functions as their definitions will be in bpf_testmod_standalone.ko BTF");
+
+noinline int
+bpf_testmod_standalone_test_struct_arg_1(struct bpf_testmod_standalone_struct_arg_2 a, int b, int c) {
+ bpf_testmod_standalone_test_struct_arg_result = a.a + a.b + b + c;
+ return bpf_testmod_standalone_test_struct_arg_result;
+}
+
+noinline int
+bpf_testmod_standalone_test_struct_arg_2(int a, struct bpf_testmod_standalone_struct_arg_2 b, int c) {
+ bpf_testmod_standalone_test_struct_arg_result = a + b.a + b.b + c;
+ return bpf_testmod_standalone_test_struct_arg_result;
+}
+
+noinline int
+bpf_testmod_standalone_test_struct_arg_3(int a, int b, struct bpf_testmod_standalone_struct_arg_2 c) {
+ bpf_testmod_standalone_test_struct_arg_result = a + b + c.a + c.b;
+ return bpf_testmod_standalone_test_struct_arg_result;
+}
+
+noinline int
+bpf_testmod_standalone_test_struct_arg_4(struct bpf_testmod_standalone_struct_arg_1 a, int b,
+ int c, int d, struct bpf_testmod_standalone_struct_arg_2 e) {
+ bpf_testmod_standalone_test_struct_arg_result = a.a + b + c + d + e.a + e.b;
+ return bpf_testmod_standalone_test_struct_arg_result;
+}
+
+noinline int
+bpf_testmod_standalone_test_struct_arg_5(void) {
+ bpf_testmod_standalone_test_struct_arg_result = 1;
+ return bpf_testmod_standalone_test_struct_arg_result;
+}
+
+noinline int
+bpf_testmod_standalone_test_struct_arg_6(struct bpf_testmod_standalone_struct_arg_3 *a) {
+ bpf_testmod_standalone_test_struct_arg_result = a->b[0];
+ return bpf_testmod_standalone_test_struct_arg_result;
+}
+
+noinline int
+bpf_testmod_standalone_test_struct_arg_7(u64 a, void *b, short c, int d, void *e,
+ struct bpf_testmod_standalone_struct_arg_4 f)
+{
+ bpf_testmod_standalone_test_struct_arg_result = a + (long)b + c + d +
+ (long)e + f.a + f.b;
+ return bpf_testmod_standalone_test_struct_arg_result;
+}
+
+noinline int
+bpf_testmod_standalone_test_struct_arg_8(u64 a, void *b, short c, int d, void *e,
+ struct bpf_testmod_standalone_struct_arg_4 f, int g)
+{
+ bpf_testmod_standalone_test_struct_arg_result = a + (long)b + c + d +
+ (long)e + f.a + f.b + g;
+ return bpf_testmod_standalone_test_struct_arg_result;
+}
+
+noinline int
+bpf_testmod_standalone_test_arg_ptr_to_struct(struct bpf_testmod_standalone_struct_arg_1 *a) {
+ bpf_testmod_standalone_test_struct_arg_result = a->a;
+ return bpf_testmod_standalone_test_struct_arg_result;
+}
+
+__bpf_kfunc void
+bpf_testmod_standalone_test_mod_kfunc(int i)
+{
+ *(int *)this_cpu_ptr(&bpf_testmod_standalone_ksym_percpu) = i;
+}
+
+__bpf_kfunc int bpf_iter_testmod_standalone_seq_new(struct bpf_iter_testmod_standalone_seq *it, s64 value, int cnt)
+{
+ if (cnt < 0) {
+ it->cnt = 0;
+ return -EINVAL;
+ }
+
+ it->value = value;
+ it->cnt = cnt;
+
+ return 0;
+}
+
+__bpf_kfunc s64 *bpf_iter_testmod_standalone_seq_next(struct bpf_iter_testmod_standalone_seq *it)
+{
+ if (it->cnt <= 0)
+ return NULL;
+
+ it->cnt--;
+
+ return &it->value;
+}
+
+__bpf_kfunc void bpf_iter_testmod_standalone_seq_destroy(struct bpf_iter_testmod_standalone_seq *it)
+{
+ it->cnt = 0;
+}
+
+__bpf_kfunc void bpf_kfunc_standalone_common_test(void)
+{
+}
+
+struct bpf_testmod_standalone_btf_type_tag_1 {
+ int a;
+};
+
+struct bpf_testmod_standalone_btf_type_tag_2 {
+ struct bpf_testmod_standalone_btf_type_tag_1 __user *p;
+};
+
+struct bpf_testmod_standalone_btf_type_tag_3 {
+ struct bpf_testmod_standalone_btf_type_tag_1 __percpu *p;
+};
+
+noinline int
+bpf_testmod_standalone_test_btf_type_tag_user_1(struct bpf_testmod_standalone_btf_type_tag_1 __user *arg) {
+ BTF_TYPE_EMIT(func_proto_typedef);
+ BTF_TYPE_EMIT(func_proto_typedef_nested1);
+ BTF_TYPE_EMIT(func_proto_typedef_nested2);
+ return arg->a;
+}
+
+noinline int
+bpf_testmod_standalone_test_btf_type_tag_user_2(struct bpf_testmod_standalone_btf_type_tag_2 *arg) {
+ return arg->p->a;
+}
+
+noinline int
+bpf_testmod_standalone_test_btf_type_tag_percpu_1(struct bpf_testmod_standalone_btf_type_tag_1 __percpu *arg) {
+ return arg->a;
+}
+
+noinline int
+bpf_testmod_standalone_test_btf_type_tag_percpu_2(struct bpf_testmod_standalone_btf_type_tag_3 *arg) {
+ return arg->p->a;
+}
+
+noinline int bpf_testmod_standalone_loop_test(int n)
+{
+ /* Make sum volatile, so smart compilers, such as clang, will not
+ * optimize the code by removing the loop.
+ */
+ volatile int sum = 0;
+ int i;
+
+ /* the primary goal of this test is to test LBR. Create a lot of
+ * branches in the function, so we can catch it easily.
+ */
+ for (i = 0; i < n; i++)
+ sum += i;
+ return sum;
+}
+
+__weak noinline struct file *bpf_testmod_standalone_return_ptr(int arg)
+{
+ static struct file f = {};
+
+ switch (arg) {
+ case 1: return (void *)EINVAL; /* user addr */
+ case 2: return (void *)0xcafe4a11; /* user addr */
+ case 3: return (void *)-EINVAL; /* canonical, but invalid */
+ case 4: return (void *)(1ull << 60); /* non-canonical and invalid */
+ case 5: return (void *)~(1ull << 30); /* trigger extable */
+ case 6: return &f; /* valid addr */
+ case 7: return (void *)((long)&f | 1); /* kernel tricks */
+ default: return NULL;
+ }
+}
+
+noinline int bpf_testmod_standalone_fentry_test1(int a)
+{
+ return a + 1;
+}
+
+noinline int bpf_testmod_standalone_fentry_test2(int a, u64 b)
+{
+ return a + b;
+}
+
+noinline int bpf_testmod_standalone_fentry_test3(char a, int b, u64 c)
+{
+ return a + b + c;
+}
+
+noinline int bpf_testmod_standalone_fentry_test7(u64 a, void *b, short c, int d,
+ void *e, char f, int g)
+{
+ return a + (long)b + c + d + (long)e + f + g;
+}
+
+noinline int bpf_testmod_standalone_fentry_test11(u64 a, void *b, short c, int d,
+ void *e, char f, int g,
+ unsigned int h, long i, __u64 j,
+ unsigned long k)
+{
+ return a + (long)b + c + d + (long)e + f + g + h + i + j + k;
+}
+
+int bpf_testmod_standalone_fentry_ok;
+
+noinline ssize_t
+bpf_testmod_standalone_test_read(struct file *file, struct kobject *kobj,
+ struct bin_attribute *bin_attr,
+ char *buf, loff_t off, size_t len)
+{
+ struct bpf_testmod_standalone_test_read_ctx ctx = {
+ .buf = buf,
+ .off = off,
+ .len = len,
+ };
+ struct bpf_testmod_standalone_struct_arg_1 struct_arg1 = {10}, struct_arg1_2 = {-1};
+ struct bpf_testmod_standalone_struct_arg_2 struct_arg2 = {2, 3};
+ struct bpf_testmod_standalone_struct_arg_3 *struct_arg3;
+ struct bpf_testmod_standalone_struct_arg_4 struct_arg4 = {21, 22};
+ int i = 1;
+
+ while (bpf_testmod_standalone_return_ptr(i))
+ i++;
+
+ (void)bpf_testmod_standalone_test_struct_arg_1(struct_arg2, 1, 4);
+ (void)bpf_testmod_standalone_test_struct_arg_2(1, struct_arg2, 4);
+ (void)bpf_testmod_standalone_test_struct_arg_3(1, 4, struct_arg2);
+ (void)bpf_testmod_standalone_test_struct_arg_4(struct_arg1, 1, 2, 3, struct_arg2);
+ (void)bpf_testmod_standalone_test_struct_arg_5();
+ (void)bpf_testmod_standalone_test_struct_arg_7(16, (void *)17, 18, 19,
+ (void *)20, struct_arg4);
+ (void)bpf_testmod_standalone_test_struct_arg_8(16, (void *)17, 18, 19,
+ (void *)20, struct_arg4, 23);
+
+ (void)bpf_testmod_standalone_test_arg_ptr_to_struct(&struct_arg1_2);
+
+ struct_arg3 = kmalloc((sizeof(struct bpf_testmod_standalone_struct_arg_3) +
+ sizeof(int)), GFP_KERNEL);
+ if (struct_arg3 != NULL) {
+ struct_arg3->b[0] = 1;
+ (void)bpf_testmod_standalone_test_struct_arg_6(struct_arg3);
+ kfree(struct_arg3);
+ }
+
+ /* This is always true. Use the check to make sure the compiler
+ * doesn't remove bpf_testmod_standalone_loop_test.
+ */
+ if (bpf_testmod_standalone_loop_test(101) > 100)
+ trace_bpf_testmod_standalone_test_read(current, &ctx);
+
+ /* Magic number to enable writable tp */
+ if (len == 64) {
+ struct bpf_testmod_standalone_test_writable_ctx writable = {
+ .val = 1024,
+ };
+ trace_bpf_testmod_standalone_test_writable_bare(&writable);
+ if (writable.early_ret)
+ return snprintf(buf, len, "%d\n", writable.val);
+ }
+
+ if (bpf_testmod_standalone_fentry_test1(1) != 2 ||
+ bpf_testmod_standalone_fentry_test2(2, 3) != 5 ||
+ bpf_testmod_standalone_fentry_test3(4, 5, 6) != 15 ||
+ bpf_testmod_standalone_fentry_test7(16, (void *)17, 18, 19, (void *)20,
+ 21, 22) != 133 ||
+ bpf_testmod_standalone_fentry_test11(16, (void *)17, 18, 19, (void *)20,
+ 21, 22, 23, 24, 25, 26) != 231)
+ goto out;
+
+ bpf_testmod_standalone_fentry_ok = 1;
+out:
+ return -EIO; /* always fail */
+}
+EXPORT_SYMBOL(bpf_testmod_standalone_test_read);
+ALLOW_ERROR_INJECTION(bpf_testmod_standalone_test_read, ERRNO);
+
+noinline ssize_t
+bpf_testmod_standalone_test_write(struct file *file, struct kobject *kobj,
+ struct bin_attribute *bin_attr,
+ char *buf, loff_t off, size_t len)
+{
+ struct bpf_testmod_standalone_test_write_ctx ctx = {
+ .buf = buf,
+ .off = off,
+ .len = len,
+ };
+
+ trace_bpf_testmod_standalone_test_write_bare(current, &ctx);
+
+ return -EIO; /* always fail */
+}
+EXPORT_SYMBOL(bpf_testmod_standalone_test_write);
+ALLOW_ERROR_INJECTION(bpf_testmod_standalone_test_write, ERRNO);
+
+__diag_pop();
+
+static struct bin_attribute bin_attr_bpf_testmod_standalone_file __ro_after_init = {
+ .attr = { .name = "bpf_testmod_standalone", .mode = 0666, },
+ .read = bpf_testmod_standalone_test_read,
+ .write = bpf_testmod_standalone_test_write,
+};
+
+BTF_SET8_START(bpf_testmod_standalone_common_kfunc_ids)
+BTF_ID_FLAGS(func, bpf_iter_testmod_standalone_seq_new, KF_ITER_NEW)
+BTF_ID_FLAGS(func, bpf_iter_testmod_standalone_seq_next, KF_ITER_NEXT | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_iter_testmod_standalone_seq_destroy, KF_ITER_DESTROY)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_common_test)
+BTF_SET8_END(bpf_testmod_standalone_common_kfunc_ids)
+
+static const struct btf_kfunc_id_set bpf_testmod_standalone_common_kfunc_set = {
+ .owner = THIS_MODULE,
+ .set = &bpf_testmod_standalone_common_kfunc_ids,
+};
+
+__bpf_kfunc u64 bpf_kfunc_standalone_call_test1(struct sock *sk, u32 a, u64 b, u32 c, u64 d)
+{
+ return a + b + c + d;
+}
+
+__bpf_kfunc int bpf_kfunc_standalone_call_test2(struct sock *sk, u32 a, u32 b)
+{
+ return a + b;
+}
+
+__bpf_kfunc struct sock *bpf_kfunc_standalone_call_test3(struct sock *sk)
+{
+ return sk;
+}
+
+__bpf_kfunc long noinline bpf_kfunc_standalone_call_test4(signed char a, short b, int c, long d)
+{
+ /* Provoke the compiler to assume that the caller has sign-extended a,
+ * b and c on platforms where this is required (e.g. s390x).
+ */
+ return (long)a + (long)b + (long)c + d;
+}
+
+static struct prog_test_ref_kfunc prog_test_struct = {
+ .a = 42,
+ .b = 108,
+ .next = &prog_test_struct,
+ .cnt = REFCOUNT_INIT(1),
+};
+
+__bpf_kfunc struct prog_test_ref_kfunc *
+bpf_kfunc_standalone_call_test_acquire(unsigned long *scalar_ptr)
+{
+ refcount_inc(&prog_test_struct.cnt);
+ return &prog_test_struct;
+}
+
+__bpf_kfunc void bpf_kfunc_standalone_call_test_offset(struct prog_test_ref_kfunc *p)
+{
+ WARN_ON_ONCE(1);
+}
+
+__bpf_kfunc struct prog_test_member *
+bpf_kfunc_standalone_call_memb_acquire(void)
+{
+ WARN_ON_ONCE(1);
+ return NULL;
+}
+
+__bpf_kfunc void bpf_kfunc_standalone_call_memb1_release(struct prog_test_member1 *p)
+{
+ WARN_ON_ONCE(1);
+}
+
+static int *__bpf_kfunc_standalone_call_test_get_mem(struct prog_test_ref_kfunc *p, const int size)
+{
+ if (size > 2 * sizeof(int))
+ return NULL;
+
+ return (int *)p;
+}
+
+__bpf_kfunc int *bpf_kfunc_standalone_call_test_get_rdwr_mem(struct prog_test_ref_kfunc *p,
+ const int rdwr_buf_size)
+{
+ return __bpf_kfunc_standalone_call_test_get_mem(p, rdwr_buf_size);
+}
+
+__bpf_kfunc int *bpf_kfunc_standalone_call_test_get_rdonly_mem(struct prog_test_ref_kfunc *p,
+ const int rdonly_buf_size)
+{
+ return __bpf_kfunc_standalone_call_test_get_mem(p, rdonly_buf_size);
+}
+
+/* the next 2 ones can't be really used for testing expect to ensure
+ * that the verifier rejects the call.
+ * Acquire functions must return struct pointers, so these ones are
+ * failing.
+ */
+__bpf_kfunc int *bpf_kfunc_standalone_call_test_acq_rdonly_mem(struct prog_test_ref_kfunc *p,
+ const int rdonly_buf_size)
+{
+ return __bpf_kfunc_standalone_call_test_get_mem(p, rdonly_buf_size);
+}
+
+__bpf_kfunc void bpf_kfunc_standalone_call_int_mem_release(int *p)
+{
+}
+
+__bpf_kfunc void bpf_kfunc_standalone_call_test_pass_ctx(struct __sk_buff *skb)
+{
+}
+
+__bpf_kfunc void bpf_kfunc_standalone_call_test_pass1(struct prog_test_pass1 *p)
+{
+}
+
+__bpf_kfunc void bpf_kfunc_standalone_call_test_pass2(struct prog_test_pass2 *p)
+{
+}
+
+__bpf_kfunc void bpf_kfunc_standalone_call_test_fail1(struct prog_test_fail1 *p)
+{
+}
+
+__bpf_kfunc void bpf_kfunc_standalone_call_test_fail2(struct prog_test_fail2 *p)
+{
+}
+
+__bpf_kfunc void bpf_kfunc_standalone_call_test_fail3(struct prog_test_fail3 *p)
+{
+}
+
+__bpf_kfunc void bpf_kfunc_standalone_call_test_mem_len_pass1(void *mem, int mem__sz)
+{
+}
+
+__bpf_kfunc void bpf_kfunc_standalone_call_test_mem_len_fail1(void *mem, int len)
+{
+}
+
+__bpf_kfunc void bpf_kfunc_standalone_call_test_mem_len_fail2(u64 *mem, int len)
+{
+}
+
+__bpf_kfunc void bpf_kfunc_standalone_call_test_ref(struct prog_test_ref_kfunc *p)
+{
+ /* p != NULL, but p->cnt could be 0 */
+}
+
+__bpf_kfunc void bpf_kfunc_standalone_call_test_destructive(void)
+{
+}
+
+__bpf_kfunc static u32 bpf_kfunc_standalone_call_test_static_unused_arg(u32 arg, u32 unused)
+{
+ return arg;
+}
+
+BTF_SET8_START(bpf_testmod_standalone_check_kfunc_ids)
+BTF_ID_FLAGS(func, bpf_testmod_standalone_test_mod_kfunc)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_test1)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_test2)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_test3)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_test4)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_test_mem_len_pass1)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_test_mem_len_fail1)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_test_mem_len_fail2)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_test_acquire, KF_ACQUIRE | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_memb_acquire, KF_ACQUIRE | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_memb1_release, KF_RELEASE)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_test_get_rdwr_mem, KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_test_get_rdonly_mem, KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_test_acq_rdonly_mem, KF_ACQUIRE | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_int_mem_release, KF_RELEASE)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_test_pass_ctx)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_test_pass1)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_test_pass2)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_test_fail1)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_test_fail2)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_test_fail3)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_test_ref, KF_TRUSTED_ARGS | KF_RCU)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_test_destructive, KF_DESTRUCTIVE)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_test_static_unused_arg)
+BTF_ID_FLAGS(func, bpf_kfunc_standalone_call_test_offset)
+BTF_SET8_END(bpf_testmod_standalone_check_kfunc_ids)
+
+static const struct btf_kfunc_id_set bpf_testmod_standalone_kfunc_set = {
+ .owner = THIS_MODULE,
+ .set = &bpf_testmod_standalone_check_kfunc_ids,
+};
+
+extern int bpf_fentry_test1(int a);
+
+static int bpf_testmod_standalone_init(void)
+{
+ int ret;
+
+ ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_UNSPEC, &bpf_testmod_standalone_common_kfunc_set);
+ ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS, &bpf_testmod_standalone_kfunc_set);
+ ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &bpf_testmod_standalone_kfunc_set);
+ ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &bpf_testmod_standalone_kfunc_set);
+ if (ret < 0)
+ return ret;
+ if (bpf_fentry_test1(0) < 0)
+ return -EINVAL;
+ return sysfs_create_bin_file(kernel_kobj, &bin_attr_bpf_testmod_standalone_file);
+}
+
+static void bpf_testmod_standalone_exit(void)
+{
+ return sysfs_remove_bin_file(kernel_kobj, &bin_attr_bpf_testmod_standalone_file);
+}
+
+module_init(bpf_testmod_standalone_init);
+module_exit(bpf_testmod_standalone_exit);
+
+MODULE_AUTHOR("Andrii Nakryiko");
+MODULE_DESCRIPTION("BPF selftests module");
+MODULE_LICENSE("Dual BSD/GPL");
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_standalone.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_standalone.h
new file mode 100644
index 000000000000..a8a4d75d9a1d
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_standalone.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2020 Facebook */
+#ifndef _BPF_TESTMOD_STANDALONE_H
+#define _BPF_TESTMOD_STANDALONE_H
+
+#include <linux/types.h>
+
+struct bpf_testmod_standalone_test_read_ctx {
+ char *buf;
+ loff_t off;
+ size_t len;
+};
+
+struct bpf_testmod_standalone_test_write_ctx {
+ char *buf;
+ loff_t off;
+ size_t len;
+};
+
+struct bpf_testmod_standalone_test_writable_ctx {
+ bool early_ret;
+ int val;
+};
+
+/* BPF iter that returns *value* *n* times in a row */
+struct bpf_iter_testmod_standalone_seq {
+ s64 value;
+ int cnt;
+};
+
+#endif /* _BPF_TESTMOD_STANDALONE_H */
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_standalone_kfunc.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_standalone_kfunc.h
new file mode 100644
index 000000000000..a23cfd72d7f3
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_standalone_kfunc.h
@@ -0,0 +1,109 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _BPF_TESTMOD_KFUNC_H
+#define _BPF_TESTMOD_KFUNC_H
+
+#ifndef __KERNEL__
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#else
+#define __ksym
+struct prog_test_member1 {
+ int a;
+};
+
+struct prog_test_member {
+ struct prog_test_member1 m;
+ int c;
+};
+
+struct prog_test_ref_kfunc {
+ int a;
+ int b;
+ struct prog_test_member memb;
+ struct prog_test_ref_kfunc *next;
+ refcount_t cnt;
+};
+#endif
+
+struct prog_test_pass1 {
+ int x0;
+ struct {
+ int x1;
+ struct {
+ int x2;
+ struct {
+ int x3;
+ };
+ };
+ };
+};
+
+struct prog_test_pass2 {
+ int len;
+ short arr1[4];
+ struct {
+ char arr2[4];
+ unsigned long arr3[8];
+ } x;
+};
+
+struct prog_test_fail1 {
+ void *p;
+ int x;
+};
+
+struct prog_test_fail2 {
+ int x8;
+ struct prog_test_pass1 x;
+};
+
+struct prog_test_fail3 {
+ int len;
+ char arr1[2];
+ char arr2[];
+};
+
+struct prog_test_ref_kfunc *
+bpf_kfunc_call_test_acquire(unsigned long *scalar_ptr) __ksym;
+void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) __ksym;
+void bpf_kfunc_call_test_ref(struct prog_test_ref_kfunc *p) __ksym;
+
+void bpf_kfunc_call_test_mem_len_pass1(void *mem, int len) __ksym;
+int *bpf_kfunc_call_test_get_rdwr_mem(struct prog_test_ref_kfunc *p, const int rdwr_buf_size) __ksym;
+int *bpf_kfunc_call_test_get_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size) __ksym;
+int *bpf_kfunc_call_test_acq_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size) __ksym;
+void bpf_kfunc_call_int_mem_release(int *p) __ksym;
+
+/* The bpf_kfunc_call_test_static_unused_arg is defined as static,
+ * but bpf program compilation needs to see it as global symbol.
+ */
+#ifndef __KERNEL__
+u32 bpf_kfunc_call_test_static_unused_arg(u32 arg, u32 unused) __ksym;
+#endif
+
+void bpf_testmod_standalone_test_mod_kfunc(int i) __ksym;
+
+__u64 bpf_kfunc_call_test1(struct sock *sk, __u32 a, __u64 b,
+ __u32 c, __u64 d) __ksym;
+int bpf_kfunc_call_test2(struct sock *sk, __u32 a, __u32 b) __ksym;
+struct sock *bpf_kfunc_call_test3(struct sock *sk) __ksym;
+long bpf_kfunc_call_test4(signed char a, short b, int c, long d) __ksym;
+
+void bpf_kfunc_call_test_pass_ctx(struct __sk_buff *skb) __ksym;
+void bpf_kfunc_call_test_pass1(struct prog_test_pass1 *p) __ksym;
+void bpf_kfunc_call_test_pass2(struct prog_test_pass2 *p) __ksym;
+void bpf_kfunc_call_test_mem_len_fail2(__u64 *mem, int len) __ksym;
+
+void bpf_kfunc_call_test_destructive(void) __ksym;
+
+void bpf_kfunc_call_test_offset(struct prog_test_ref_kfunc *p);
+struct prog_test_member *bpf_kfunc_call_memb_acquire(void);
+void bpf_kfunc_call_memb1_release(struct prog_test_member1 *p);
+void bpf_kfunc_call_test_fail1(struct prog_test_fail1 *p);
+void bpf_kfunc_call_test_fail2(struct prog_test_fail2 *p);
+void bpf_kfunc_call_test_fail3(struct prog_test_fail3 *p);
+void bpf_kfunc_call_test_mem_len_fail1(void *mem, int len);
+
+void bpf_kfunc_common_test(void) __ksym;
+#endif /* _BPF_TESTMOD_KFUNC_H */
--
2.31.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 bpf-next 17/17] selftests/bpf: update btf_module test to ensure standalone BTF works
2023-11-12 12:48 [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF Alan Maguire
` (15 preceding siblings ...)
2023-11-12 12:48 ` [PATCH v4 bpf-next 16/17] selftests/bpf: build separate bpf_testmod module with standalone BTF Alan Maguire
@ 2023-11-12 12:48 ` Alan Maguire
2023-11-14 20:16 ` [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF Alan Maguire
17 siblings, 0 replies; 29+ messages in thread
From: Alan Maguire @ 2023-11-12 12:48 UTC (permalink / raw)
To: ast, daniel, andrii, jolsa
Cc: quentin, eddyz87, martin.lau, song, yonghong.song, john.fastabend,
kpsingh, sdf, haoluo, masahiroy, bpf, Alan Maguire
btf_module test verifies that loading split BTF from bpf_testmod.ko
is successful. To test standalone BTF, add a test that loads
BTF from bpf_testmod_standalone.ko and verifies we can look up BTF,
just as we could for real split BTF.
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
tools/testing/selftests/bpf/Makefile | 8 ++++----
.../selftests/bpf/prog_tests/btf_module.c | 19 +++++++++++++++++--
2 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 9c27b67bc7b1..5a4421238d5b 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -106,7 +106,7 @@ TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \
flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \
test_lirc_mode2_user xdping test_cpp runqslower bench bpf_testmod.ko \
xskxceiver xdp_redirect_multi xdp_synproxy veristat xdp_hw_metadata \
- xdp_features
+ xdp_features btf_testmod_standalone.ko
TEST_GEN_FILES += liburandom_read.so urandom_read sign-file uprobe_multi
@@ -225,9 +225,9 @@ $(OUTPUT)/sign-file: ../../../../scripts/sign-file.c
$(OUTPUT)/bpf_testmod.ko: $(VMLINUX_BTF) $(RESOLVE_BTFIDS) $(wildcard bpf_testmod/Makefile bpf_testmod/*.[ch])
$(call msg,MOD,,$@)
- $(Q)$(RM) bpf_testmod/bpf_testmod.ko # force re-compilation
+ $(Q)$(RM) bpf_testmod/bpf_testmod*.ko # force re-compilation
$(Q)$(MAKE) $(submake_extras) RESOLVE_BTFIDS=$(RESOLVE_BTFIDS) -C bpf_testmod
- $(Q)cp bpf_testmod/bpf_testmod.ko $@
+ $(Q)cp bpf_testmod/bpf_testmod*.ko $(OUTPUT)
DEFAULT_BPFTOOL := $(HOST_SCRATCH_DIR)/sbin/bpftool
ifneq ($(CROSS_COMPILE),)
@@ -728,7 +728,7 @@ EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(SCRATCH_DIR) $(HOST_SCRATCH_DIR) \
prog_tests/tests.h map_tests/tests.h verifier/tests.h \
feature bpftool \
$(addprefix $(OUTPUT)/,*.o *.skel.h *.lskel.h *.subskel.h \
- no_alu32 cpuv4 bpf_gcc bpf_testmod.ko \
+ no_alu32 cpuv4 bpf_gcc bpf_testmod*.ko \
liburandom_read.so)
.PHONY: docs docs-clean
diff --git a/tools/testing/selftests/bpf/prog_tests/btf_module.c b/tools/testing/selftests/bpf/prog_tests/btf_module.c
index 2239d1fe0332..5470342a1d08 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf_module.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf_module.c
@@ -5,11 +5,13 @@
#include <bpf/btf.h>
static const char *module_name = "bpf_testmod";
+static const char *standalone_module_name = "bpf_testmod_standalone";
static const char *symbol_name = "bpf_testmod_test_read";
+static const char *standalone_symbol_name = "bpf_testmod_standalone_test_read";
void test_btf_module()
{
- struct btf *vmlinux_btf, *module_btf;
+ struct btf *vmlinux_btf, *module_btf, *standalone_module_btf = NULL;
__s32 type_id;
if (!env.has_testmod) {
@@ -26,9 +28,22 @@ void test_btf_module()
goto cleanup;
type_id = btf__find_by_name(module_btf, symbol_name);
- ASSERT_GT(type_id, 0, "func not found");
+ if (!ASSERT_GT(type_id, 0, "func not found"))
+ goto cleanup;
+
+ if (!ASSERT_OK(load_bpf_testmod(standalone_module_name, false), "load standalone BTF module"))
+ goto cleanup;
+
+ standalone_module_btf = btf__load_module_btf(standalone_module_name, vmlinux_btf);
+ if (!ASSERT_OK_PTR(standalone_module_btf, "could not load standalone module BTF"))
+ goto cleanup;
+
+ type_id = btf__find_by_name(standalone_module_btf, standalone_symbol_name);
+ ASSERT_GT(type_id, 0, "func not found in standalone");
cleanup:
+ btf__free(standalone_module_btf);
btf__free(module_btf);
btf__free(vmlinux_btf);
+ unload_bpf_testmod(standalone_module_name, false);
}
--
2.31.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF
2023-11-12 12:48 [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF Alan Maguire
` (16 preceding siblings ...)
2023-11-12 12:48 ` [PATCH v4 bpf-next 17/17] selftests/bpf: update btf_module test to ensure standalone BTF works Alan Maguire
@ 2023-11-14 20:16 ` Alan Maguire
2023-11-21 19:44 ` Andrii Nakryiko
17 siblings, 1 reply; 29+ messages in thread
From: Alan Maguire @ 2023-11-14 20:16 UTC (permalink / raw)
To: ast, daniel, andrii, jolsa
Cc: quentin, eddyz87, martin.lau, song, yonghong.song, john.fastabend,
kpsingh, sdf, haoluo, masahiroy, bpf
On 12/11/2023 12:48, Alan Maguire wrote:
> Update struct btf_header to add a new "kind_layout" section containing
> a description of how to parse the BTF kinds known about at BTF
> encoding time. This provides the opportunity for tools that might
> not know all of these kinds - as is the case when older tools run
> on more newly-generated BTF - to still parse the BTF provided,
> even if it cannot all be used.
>
> Also add CRCs for the BTF and base BTF (if needed) from which it was
> created. CRCs provide a few useful features:
>
> - the base CRC allows us to explicitly identify when the split and
> base BTF are not matched
> - absence of a base BTF CRC can indicate that BTF is standalone;
> i.e. not defined relative to base BTF
>
> The former case can be used to explicitly reject mismatched
> module/kernel BTF rather than assuming it is matched until an
> unexpected type is encountered.
>
> The latter case is useful for modules that are not built as
> frequently as the kernel; in such cases, the module can be built
> standalone by specifying an empty BTF base:
>
> make BTF_BASE= M=path/2/module
>
> If CRCs are not present (as will be the case for pahole versions
> prior to the proposed v1.26 which will support CRC generation),
> standalone BTF can still be identified by a slower fallback
> method of examining BTF type ids to ensure that BTF is
> self-referential only.
>
> To ensure existing tooling can handle standalone BTF for kernel
> modules, we remap the type ids to start after the vmlinux
> BTF ids, to make it appear to be split BTF. This allows tools
> (and the kernel) that assume split BTF for modules to operate normally.
>
hi folks
I wanted to capture feedback received on the approach described here for
BTF module generation at my talk at LPC [1].
Stepping back, the aim is to provide a way to generate BTF for a module
such that it is somewhat resilient to minor changes in underlying BTF,
so it does not have to be rebuilt every time vmlinux is built. The
module references to vmlinux BTF ids are currently very brittle, and
even for the same kernel we get different vmlinux BTF ids if the BTF is
rebuilt. So the aim is to support a more robust method of module BTF
generation. Note that the approach described here is not needed for
modules that are built at the same time as the kernel, so it's unlikely
any in-tree modules will need this, but it will be useful for cases such
as where modules are delivered via a package and want to make use
of BTF such that it will not be invalidated.
Turning to the talk, the general consensus - I think - was that the
standalone BTF approach described in this series was problematic.
Consider kfuncs, if we have, for example, our own definition of a
structure in standalone module BTF, the BTF id of the local structure
will not match that of the core kernel, which has the potential to
confuse the verifier.
A similar problem exists for tracing; we would trace an sk_buff in
the module via the module's view of struct sk_buff, but we have no
guarantees that the module's view is still consistent with the vmlinux
representation (which actually allocated it).
Hopefully I've characterized this correctly; let me know if I missed
something here.
So we need some means to both remap BTF ids in the module BTF that refer
to the vmlinux BTF so they point at the right types, _and_ to check the
consistency of the representation of a vmlinux type between module BTF
build time and when it is loaded into the kernel.
With this in mind, I think a good way forward might be something like
the following:
For cases where we want more change-independent module BTF - which
is resilient to things like reshuffling of vmlinux BTF ids, and small
changes that don't invalidate structure use completely - we add
a "relocatable" option to the --btf_features list of features for pahole
encoding of module BTF.
This option would not be needed for modules built at the same time as
the kernel, since the BTF ids and the types they refer to are consistent.
When used however, it would tell BTF dedup in pahole to add reocation
information as well as generating usual split BTF at the time of module
BTF generation. This relocation information would consist of
descriptions of the BTF types that the module refers to in base BTF and
their dependents. By providing such descriptions, we can then reconcile
the views of types between module and kernel, or if such reconciliation
is impossible, we can refuse to use the BTF. The amount of information
needed for a module will need to be determined, but I'm hopeful in most
cases it would be a small subset of the type information
required for vmlinux as a whole.
The process of reconciling module and vmlinux BTF at module load time
would then be
1. Remap all the split BTF ids representing module-specific types
and functions to start at last_vmlinux_id + 1. Since the current
vmlinux may have a different number of types than the vmlinux
at time of encoding, this remapping is necessary.
2. For each vmlinux type in our list of relocations, check its
compatibility with the associated vmlinux type. This is
somewhat akin to the CO-RE compatibility checks. Exact rules
would need to be ironed out, but a somewhat loose approach
would be ideal such that a few minor changes in a struct
somewhere do not totally invalidate module BTF. Unlike CO-RE
though, field offset changes are _not_ good since they imply the
module has an incorrect view of the structure and might
start using fields incorrectly.
Note that this is a bit easier than BTF deduplication, because
the deduplication process that happened at module encoding time
has already done the dependency checking for us; we just need
to do a type-by-type, 1-to-1 comparison between our relocation
types and current vmlinux types.
3. If all types are consistent, BTF is loaded and we remap the
module's vmlinux BTF id references to the corresponding
vmlinux BTF ids of the current vmlinux.
I _think_ this gets us what we want; more resilient module BTF,
but with safety checks to ensure compatible representations.
There were some suggestions of using a hashing method, but I think
such a method presupposes we want exact type matches, which I suspect
would be unlikely to be useful in practice as with most stable-based
distros, small changes in types can be made due to fixes etc.
There were also a suggestion of doing a full dedup, but I think the
consensus in the room (which I agree with) is that would be hard
to do in-kernel. So the above approach is a compropmise I think;
it gets actual dedup at BTF creation time to create the list of
references and dependents, and we later check them one-by-one on module
load for compatibility.
Anyway I just wanted to try and capture the feedback received, and
lay out a possible direction. Any further thoughts or suggestions
would be much appreciated. Thanks!
Alan
[1] https://lpc.events/event/17/contributions/1576/
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF
2023-11-14 20:16 ` [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF Alan Maguire
@ 2023-11-21 19:44 ` Andrii Nakryiko
2023-11-22 17:00 ` Alan Maguire
0 siblings, 1 reply; 29+ messages in thread
From: Andrii Nakryiko @ 2023-11-21 19:44 UTC (permalink / raw)
To: Alan Maguire
Cc: ast, daniel, andrii, jolsa, quentin, eddyz87, martin.lau, song,
yonghong.song, john.fastabend, kpsingh, sdf, haoluo, masahiroy,
bpf
On Tue, Nov 14, 2023 at 12:20 PM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On 12/11/2023 12:48, Alan Maguire wrote:
> > Update struct btf_header to add a new "kind_layout" section containing
> > a description of how to parse the BTF kinds known about at BTF
> > encoding time. This provides the opportunity for tools that might
> > not know all of these kinds - as is the case when older tools run
> > on more newly-generated BTF - to still parse the BTF provided,
> > even if it cannot all be used.
> >
> > Also add CRCs for the BTF and base BTF (if needed) from which it was
> > created. CRCs provide a few useful features:
> >
> > - the base CRC allows us to explicitly identify when the split and
> > base BTF are not matched
> > - absence of a base BTF CRC can indicate that BTF is standalone;
> > i.e. not defined relative to base BTF
> >
> > The former case can be used to explicitly reject mismatched
> > module/kernel BTF rather than assuming it is matched until an
> > unexpected type is encountered.
> >
> > The latter case is useful for modules that are not built as
> > frequently as the kernel; in such cases, the module can be built
> > standalone by specifying an empty BTF base:
> >
> > make BTF_BASE= M=path/2/module
> >
> > If CRCs are not present (as will be the case for pahole versions
> > prior to the proposed v1.26 which will support CRC generation),
> > standalone BTF can still be identified by a slower fallback
> > method of examining BTF type ids to ensure that BTF is
> > self-referential only.
> >
> > To ensure existing tooling can handle standalone BTF for kernel
> > modules, we remap the type ids to start after the vmlinux
> > BTF ids, to make it appear to be split BTF. This allows tools
> > (and the kernel) that assume split BTF for modules to operate normally.
> >
>
> hi folks
>
> I wanted to capture feedback received on the approach described here for
> BTF module generation at my talk at LPC [1].
>
> Stepping back, the aim is to provide a way to generate BTF for a module
> such that it is somewhat resilient to minor changes in underlying BTF,
> so it does not have to be rebuilt every time vmlinux is built. The
> module references to vmlinux BTF ids are currently very brittle, and
> even for the same kernel we get different vmlinux BTF ids if the BTF is
> rebuilt. So the aim is to support a more robust method of module BTF
> generation. Note that the approach described here is not needed for
> modules that are built at the same time as the kernel, so it's unlikely
> any in-tree modules will need this, but it will be useful for cases such
> as where modules are delivered via a package and want to make use
> of BTF such that it will not be invalidated.
>
> Turning to the talk, the general consensus - I think - was that the
> standalone BTF approach described in this series was problematic.
> Consider kfuncs, if we have, for example, our own definition of a
> structure in standalone module BTF, the BTF id of the local structure
> will not match that of the core kernel, which has the potential to
> confuse the verifier.
>
> A similar problem exists for tracing; we would trace an sk_buff in
> the module via the module's view of struct sk_buff, but we have no
> guarantees that the module's view is still consistent with the vmlinux
> representation (which actually allocated it).
>
> Hopefully I've characterized this correctly; let me know if I missed
> something here.
Correct.
>
> So we need some means to both remap BTF ids in the module BTF that refer
> to the vmlinux BTF so they point at the right types, _and_ to check the
> consistency of the representation of a vmlinux type between module BTF
> build time and when it is loaded into the kernel.
>
> With this in mind, I think a good way forward might be something like
> the following:
>
> For cases where we want more change-independent module BTF - which
> is resilient to things like reshuffling of vmlinux BTF ids, and small
> changes that don't invalidate structure use completely - we add
> a "relocatable" option to the --btf_features list of features for pahole
> encoding of module BTF.
>
> This option would not be needed for modules built at the same time as
> the kernel, since the BTF ids and the types they refer to are consistent.
>
> When used however, it would tell BTF dedup in pahole to add reocation
> information as well as generating usual split BTF at the time of module
> BTF generation. This relocation information would consist of
> descriptions of the BTF types that the module refers to in base BTF and
> their dependents. By providing such descriptions, we can then reconcile
> the views of types between module and kernel, or if such reconciliation
> is impossible, we can refuse to use the BTF. The amount of information
> needed for a module will need to be determined, but I'm hopeful in most
> cases it would be a small subset of the type information
> required for vmlinux as a whole.
>
> The process of reconciling module and vmlinux BTF at module load time
> would then be
>
> 1. Remap all the split BTF ids representing module-specific types
> and functions to start at last_vmlinux_id + 1. Since the current
> vmlinux may have a different number of types than the vmlinux
> at time of encoding, this remapping is necessary.
Correct.
>
> 2. For each vmlinux type in our list of relocations, check its
> compatibility with the associated vmlinux type. This is
> somewhat akin to the CO-RE compatibility checks. Exact rules
Not really. CO-RE compatiblity is explicitly very permissive, while
here we want to make sure that types are actually memory
layout-compatible.
> would need to be ironed out, but a somewhat loose approach
> would be ideal such that a few minor changes in a struct
> somewhere do not totally invalidate module BTF. Unlike CO-RE
> though, field offset changes are _not_ good since they imply the
> module has an incorrect view of the structure and might
> start using fields incorrectly.
I think vmlinux type should have at least all the members that module
expects, at the same offset, with the same size. Maybe we should allow
vmlinux type to get some types at the end, not sure. How hard a
requirement it is to accommodate non-exact type matches between
vmlinux and kernel module's types?
>
> Note that this is a bit easier than BTF deduplication, because
> the deduplication process that happened at module encoding time
> has already done the dependency checking for us; we just need
> to do a type-by-type, 1-to-1 comparison between our relocation
> types and current vmlinux types.
>
> 3. If all types are consistent, BTF is loaded and we remap the
> module's vmlinux BTF id references to the corresponding
> vmlinux BTF ids of the current vmlinux.
Note that we might need to do something special for anonymous types
(modifiers, anon enums and structs/unions). Otherwise it's not clear
how to even map them between vmlinux BTF and module BTF.
>
> I _think_ this gets us what we want; more resilient module BTF,
> but with safety checks to ensure compatible representations.
> There were some suggestions of using a hashing method, but I think
> such a method presupposes we want exact type matches, which I suspect
> would be unlikely to be useful in practice as with most stable-based
> distros, small changes in types can be made due to fixes etc.
What are "small changes" and how are they automatically determined and
validated?
>
> There were also a suggestion of doing a full dedup, but I think the
> consensus in the room (which I agree with) is that would be hard
> to do in-kernel. So the above approach is a compropmise I think;
> it gets actual dedup at BTF creation time to create the list of
> references and dependents, and we later check them one-by-one on module
> load for compatibility.
>
> Anyway I just wanted to try and capture the feedback received, and
> lay out a possible direction. Any further thoughts or suggestions
> would be much appreciated. Thanks!
>
> Alan
>
> [1] https://lpc.events/event/17/contributions/1576/
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF
2023-11-21 19:44 ` Andrii Nakryiko
@ 2023-11-22 17:00 ` Alan Maguire
2023-11-22 17:42 ` Andrii Nakryiko
0 siblings, 1 reply; 29+ messages in thread
From: Alan Maguire @ 2023-11-22 17:00 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: ast, daniel, andrii, jolsa, quentin, eddyz87, martin.lau, song,
yonghong.song, john.fastabend, kpsingh, sdf, haoluo, masahiroy,
bpf
On 21/11/2023 19:44, Andrii Nakryiko wrote:
> On Tue, Nov 14, 2023 at 12:20 PM Alan Maguire <alan.maguire@oracle.com> wrote:
>>
>> hi folks
>>
>> I wanted to capture feedback received on the approach described here for
>> BTF module generation at my talk at LPC [1].
>>
>> Stepping back, the aim is to provide a way to generate BTF for a module
>> such that it is somewhat resilient to minor changes in underlying BTF,
>> so it does not have to be rebuilt every time vmlinux is built. The
>> module references to vmlinux BTF ids are currently very brittle, and
>> even for the same kernel we get different vmlinux BTF ids if the BTF is
>> rebuilt. So the aim is to support a more robust method of module BTF
>> generation. Note that the approach described here is not needed for
>> modules that are built at the same time as the kernel, so it's unlikely
>> any in-tree modules will need this, but it will be useful for cases such
>> as where modules are delivered via a package and want to make use
>> of BTF such that it will not be invalidated.
>>
>> Turning to the talk, the general consensus - I think - was that the
>> standalone BTF approach described in this series was problematic.
>> Consider kfuncs, if we have, for example, our own definition of a
>> structure in standalone module BTF, the BTF id of the local structure
>> will not match that of the core kernel, which has the potential to
>> confuse the verifier.
>>
>> A similar problem exists for tracing; we would trace an sk_buff in
>> the module via the module's view of struct sk_buff, but we have no
>> guarantees that the module's view is still consistent with the vmlinux
>> representation (which actually allocated it).
>>
>> Hopefully I've characterized this correctly; let me know if I missed
>> something here.
>
> Correct.
>
>>
>> So we need some means to both remap BTF ids in the module BTF that refer
>> to the vmlinux BTF so they point at the right types, _and_ to check the
>> consistency of the representation of a vmlinux type between module BTF
>> build time and when it is loaded into the kernel.
>>
>> With this in mind, I think a good way forward might be something like
>> the following:
>>
>> For cases where we want more change-independent module BTF - which
>> is resilient to things like reshuffling of vmlinux BTF ids, and small
>> changes that don't invalidate structure use completely - we add
>> a "relocatable" option to the --btf_features list of features for pahole
>> encoding of module BTF.
>>
>> This option would not be needed for modules built at the same time as
>> the kernel, since the BTF ids and the types they refer to are consistent.
>>
>> When used however, it would tell BTF dedup in pahole to add reocation
>> information as well as generating usual split BTF at the time of module
>> BTF generation. This relocation information would consist of
>> descriptions of the BTF types that the module refers to in base BTF and
>> their dependents. By providing such descriptions, we can then reconcile
>> the views of types between module and kernel, or if such reconciliation
>> is impossible, we can refuse to use the BTF. The amount of information
>> needed for a module will need to be determined, but I'm hopeful in most
>> cases it would be a small subset of the type information
>> required for vmlinux as a whole.
>>
>> The process of reconciling module and vmlinux BTF at module load time
>> would then be
>>
>> 1. Remap all the split BTF ids representing module-specific types
>> and functions to start at last_vmlinux_id + 1. Since the current
>> vmlinux may have a different number of types than the vmlinux
>> at time of encoding, this remapping is necessary.
>
> Correct.
>
>>
>> 2. For each vmlinux type in our list of relocations, check its
>> compatibility with the associated vmlinux type. This is
>> somewhat akin to the CO-RE compatibility checks. Exact rules
>
> Not really. CO-RE compatiblity is explicitly very permissive, while
> here we want to make sure that types are actually memory
> layout-compatible.
>
>> would need to be ironed out, but a somewhat loose approach
>> would be ideal such that a few minor changes in a struct
>> somewhere do not totally invalidate module BTF. Unlike CO-RE
>> though, field offset changes are _not_ good since they imply the
>> module has an incorrect view of the structure and might
>> start using fields incorrectly.
>
> I think vmlinux type should have at least all the members that module
> expects, at the same offset, with the same size. Maybe we should allow
> vmlinux type to get some types at the end, not sure. How hard a
> requirement it is to accommodate non-exact type matches between
> vmlinux and kernel module's types?
>
The main need is to support resilience in the face of small structure
changes such that the compiled module will still work. When backporting
fixes to a stable-based kernel - where a version of say 5.15 stable is
supported for a while and so accumulates stable fixes - often the
approach used is to use holes in structures for new fields, or if the
structure is not embedded in any module-specific structures, add fields
at the end. All existing field offsets should match. In taking that
approach, the aim is to make sure data accesses in the module are still
valid - memory layout compatibility is the goal.
>>
>> Note that this is a bit easier than BTF deduplication, because
>> the deduplication process that happened at module encoding time
>> has already done the dependency checking for us; we just need
>> to do a type-by-type, 1-to-1 comparison between our relocation
>> types and current vmlinux types.
>>
>> 3. If all types are consistent, BTF is loaded and we remap the
>> module's vmlinux BTF id references to the corresponding
>> vmlinux BTF ids of the current vmlinux.
>
> Note that we might need to do something special for anonymous types
> (modifiers, anon enums and structs/unions). Otherwise it's not clear
> how to even map them between vmlinux BTF and module BTF.
>
Good point, we'd probably need to represent some sort of parent-child
relationship to handle cases like this.
>>
>> I _think_ this gets us what we want; more resilient module BTF,
>> but with safety checks to ensure compatible representations.
>> There were some suggestions of using a hashing method, but I think
>> such a method presupposes we want exact type matches, which I suspect
>> would be unlikely to be useful in practice as with most stable-based
>> distros, small changes in types can be made due to fixes etc.
>
> What are "small changes" and how are they automatically determined and
> validated?
>
See above, field additions in data structure holes or appended to
structs for the most part. Once I have something rough working
I'll see how it performs in practice and report back. Thanks!
Alan
>>
>> There were also a suggestion of doing a full dedup, but I think the
>> consensus in the room (which I agree with) is that would be hard
>> to do in-kernel. So the above approach is a compropmise I think;
>> it gets actual dedup at BTF creation time to create the list of
>> references and dependents, and we later check them one-by-one on module
>> load for compatibility.
>>
>> Anyway I just wanted to try and capture the feedback received, and
>> lay out a possible direction. Any further thoughts or suggestions
>> would be much appreciated. Thanks!
>>
>> Alan
>>
>> [1] https://lpc.events/event/17/contributions/1576/
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF
2023-11-22 17:00 ` Alan Maguire
@ 2023-11-22 17:42 ` Andrii Nakryiko
0 siblings, 0 replies; 29+ messages in thread
From: Andrii Nakryiko @ 2023-11-22 17:42 UTC (permalink / raw)
To: Alan Maguire
Cc: ast, daniel, andrii, jolsa, quentin, eddyz87, martin.lau, song,
yonghong.song, john.fastabend, kpsingh, sdf, haoluo, masahiroy,
bpf
On Wed, Nov 22, 2023 at 9:00 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On 21/11/2023 19:44, Andrii Nakryiko wrote:
> > On Tue, Nov 14, 2023 at 12:20 PM Alan Maguire <alan.maguire@oracle.com> wrote:
> >>
> >> hi folks
> >>
> >> I wanted to capture feedback received on the approach described here for
> >> BTF module generation at my talk at LPC [1].
> >>
> >> Stepping back, the aim is to provide a way to generate BTF for a module
> >> such that it is somewhat resilient to minor changes in underlying BTF,
> >> so it does not have to be rebuilt every time vmlinux is built. The
> >> module references to vmlinux BTF ids are currently very brittle, and
> >> even for the same kernel we get different vmlinux BTF ids if the BTF is
> >> rebuilt. So the aim is to support a more robust method of module BTF
> >> generation. Note that the approach described here is not needed for
> >> modules that are built at the same time as the kernel, so it's unlikely
> >> any in-tree modules will need this, but it will be useful for cases such
> >> as where modules are delivered via a package and want to make use
> >> of BTF such that it will not be invalidated.
> >>
> >> Turning to the talk, the general consensus - I think - was that the
> >> standalone BTF approach described in this series was problematic.
> >> Consider kfuncs, if we have, for example, our own definition of a
> >> structure in standalone module BTF, the BTF id of the local structure
> >> will not match that of the core kernel, which has the potential to
> >> confuse the verifier.
> >>
> >> A similar problem exists for tracing; we would trace an sk_buff in
> >> the module via the module's view of struct sk_buff, but we have no
> >> guarantees that the module's view is still consistent with the vmlinux
> >> representation (which actually allocated it).
> >>
> >> Hopefully I've characterized this correctly; let me know if I missed
> >> something here.
> >
> > Correct.
> >
> >>
> >> So we need some means to both remap BTF ids in the module BTF that refer
> >> to the vmlinux BTF so they point at the right types, _and_ to check the
> >> consistency of the representation of a vmlinux type between module BTF
> >> build time and when it is loaded into the kernel.
> >>
> >> With this in mind, I think a good way forward might be something like
> >> the following:
> >>
> >> For cases where we want more change-independent module BTF - which
> >> is resilient to things like reshuffling of vmlinux BTF ids, and small
> >> changes that don't invalidate structure use completely - we add
> >> a "relocatable" option to the --btf_features list of features for pahole
> >> encoding of module BTF.
> >>
> >> This option would not be needed for modules built at the same time as
> >> the kernel, since the BTF ids and the types they refer to are consistent.
> >>
> >> When used however, it would tell BTF dedup in pahole to add reocation
> >> information as well as generating usual split BTF at the time of module
> >> BTF generation. This relocation information would consist of
> >> descriptions of the BTF types that the module refers to in base BTF and
> >> their dependents. By providing such descriptions, we can then reconcile
> >> the views of types between module and kernel, or if such reconciliation
> >> is impossible, we can refuse to use the BTF. The amount of information
> >> needed for a module will need to be determined, but I'm hopeful in most
> >> cases it would be a small subset of the type information
> >> required for vmlinux as a whole.
> >>
> >> The process of reconciling module and vmlinux BTF at module load time
> >> would then be
> >>
> >> 1. Remap all the split BTF ids representing module-specific types
> >> and functions to start at last_vmlinux_id + 1. Since the current
> >> vmlinux may have a different number of types than the vmlinux
> >> at time of encoding, this remapping is necessary.
> >
> > Correct.
> >
> >>
> >> 2. For each vmlinux type in our list of relocations, check its
> >> compatibility with the associated vmlinux type. This is
> >> somewhat akin to the CO-RE compatibility checks. Exact rules
> >
> > Not really. CO-RE compatiblity is explicitly very permissive, while
> > here we want to make sure that types are actually memory
> > layout-compatible.
> >
> >> would need to be ironed out, but a somewhat loose approach
> >> would be ideal such that a few minor changes in a struct
> >> somewhere do not totally invalidate module BTF. Unlike CO-RE
> >> though, field offset changes are _not_ good since they imply the
> >> module has an incorrect view of the structure and might
> >> start using fields incorrectly.
> >
> > I think vmlinux type should have at least all the members that module
> > expects, at the same offset, with the same size. Maybe we should allow
> > vmlinux type to get some types at the end, not sure. How hard a
> > requirement it is to accommodate non-exact type matches between
> > vmlinux and kernel module's types?
> >
>
> The main need is to support resilience in the face of small structure
> changes such that the compiled module will still work. When backporting
> fixes to a stable-based kernel - where a version of say 5.15 stable is
> supported for a while and so accumulates stable fixes - often the
> approach used is to use holes in structures for new fields, or if the
> structure is not embedded in any module-specific structures, add fields
> at the end. All existing field offsets should match. In taking that
> approach, the aim is to make sure data accesses in the module are still
> valid - memory layout compatibility is the goal.
So we'll need to develop some checksum/hash that would accommodate
these allowed changes.
>
> >>
> >> Note that this is a bit easier than BTF deduplication, because
> >> the deduplication process that happened at module encoding time
> >> has already done the dependency checking for us; we just need
> >> to do a type-by-type, 1-to-1 comparison between our relocation
> >> types and current vmlinux types.
> >>
> >> 3. If all types are consistent, BTF is loaded and we remap the
> >> module's vmlinux BTF id references to the corresponding
> >> vmlinux BTF ids of the current vmlinux.
> >
> > Note that we might need to do something special for anonymous types
> > (modifiers, anon enums and structs/unions). Otherwise it's not clear
> > how to even map them between vmlinux BTF and module BTF.
> >
>
> Good point, we'd probably need to represent some sort of parent-child
> relationship to handle cases like this.
Probably best to keep such anonymous types in module's BTF. It might
add a bit of duplication, but will simplify the rest a lot.
>
> >>
> >> I _think_ this gets us what we want; more resilient module BTF,
> >> but with safety checks to ensure compatible representations.
> >> There were some suggestions of using a hashing method, but I think
> >> such a method presupposes we want exact type matches, which I suspect
> >> would be unlikely to be useful in practice as with most stable-based
> >> distros, small changes in types can be made due to fixes etc.
> >
> > What are "small changes" and how are they automatically determined and
> > validated?
> >
>
> See above, field additions in data structure holes or appended to
> structs for the most part. Once I have something rough working
> I'll see how it performs in practice and report back. Thanks!
>
SGTM.
> Alan
>
>
> >>
> >> There were also a suggestion of doing a full dedup, but I think the
> >> consensus in the room (which I agree with) is that would be hard
> >> to do in-kernel. So the above approach is a compropmise I think;
> >> it gets actual dedup at BTF creation time to create the list of
> >> references and dependents, and we later check them one-by-one on module
> >> load for compatibility.
> >>
> >> Anyway I just wanted to try and capture the feedback received, and
> >> lay out a possible direction. Any further thoughts or suggestions
> >> would be much appreciated. Thanks!
> >>
> >> Alan
> >>
> >> [1] https://lpc.events/event/17/contributions/1576/
^ permalink raw reply [flat|nested] 29+ messages in thread