[PATCH bpf-next 0/4] libbpf: add raw BTF type dumping

[PATCH bpf-next 0/4] libbpf: add raw BTF type dumping

[Andrii Nakryiko]

Add btf_dump__dump_type_raw() API that emits human-readable low-level BTF type
information, same as bpftool output. bpftool is not switched to this API
because bpftool still needs to perform all the same BTF type processing logic
to do JSON output, so benefits are pretty much zero.

Raw BTF type output is extremely useful when debugging issues with BTF. It's
also handy to be able to do that in selftests. Raw BTF type output doesn't
hide any information like BTF-to-C conversion might (e.g., not emitting
BTF_KIND_FUNC, BTF_KIND_VAR and BTF_KIND_DATASEC), so is the most robust way
to look at BTF data without going all the way to deciphering binary BTF info.

Also, now that BTF can be extended with write APIs, teach btf_dump to work
with such modifiable BTFs, including the BTF-to-C convertion APIs. A self-test
to validate such incremental BTF-to-C conversion is added in patch #4.

Andrii Nakryiko (4):
  libbpf: make btf_dump work with modifiable BTF
  libbpf: add raw dumping of BTF types
  selftests/bpf: add checking of raw type dump in BTF writer APIs
    selftests
  selftests/bpf: test "incremental" btf_dump in C format

 tools/lib/bpf/btf.c                           |  17 ++
 tools/lib/bpf/btf.h                           |   1 +
 tools/lib/bpf/btf_dump.c                      | 243 ++++++++++++++++--
 tools/lib/bpf/libbpf.map                      |   1 +
 tools/lib/bpf/libbpf_internal.h               |   1 +
 .../selftests/bpf/prog_tests/btf_dump.c       | 105 ++++++++
 .../selftests/bpf/prog_tests/btf_write.c      |  67 ++++-
 7 files changed, 410 insertions(+), 25 deletions(-)

-- 
2.24.1

[PATCH bpf-next 1/4] libbpf: make btf_dump work with modifiable BTF

[Andrii Nakryiko]

Ensure that btf_dump can accommodate new BTF types being appended to BTF
instance after struct btf_dump was created. This came up during attemp to
use btf_dump for raw type dumping in selftests, but given changes are not
excessive, it's good to not have any gotchas in API usage, so I decided to
support such use case in general.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/btf.c             | 17 ++++++++
 tools/lib/bpf/btf_dump.c        | 69 ++++++++++++++++++++++-----------
 tools/lib/bpf/libbpf_internal.h |  1 +
 3 files changed, 65 insertions(+), 22 deletions(-)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index e1dbd766c698..df4fd9132079 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -146,6 +146,23 @@ void *btf_add_mem(void **data, size_t *cap_cnt, size_t elem_sz,
 	return new_data + cur_cnt * elem_sz;
 }
 
+/* Ensure given dynamically allocated memory region has enough allocated space
+ * to accommodate *need_cnt* elements of size *elem_sz* bytes each
+ */
+int btf_ensure_mem(void **data, size_t *cap_cnt, size_t elem_sz, size_t need_cnt)
+{
+	void *p;
+
+	if (need_cnt <= *cap_cnt)
+		return 0;
+
+	p = btf_add_mem(data, cap_cnt, elem_sz, *cap_cnt, SIZE_MAX, need_cnt - *cap_cnt);
+	if (!p)
+		return -ENOMEM;
+
+	return 0;
+}
+
 static int btf_add_type_idx_entry(struct btf *btf, __u32 type_off)
 {
 	__u32 *p;
diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
index 91310e528a3a..2f9d685bd522 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -60,11 +60,14 @@ struct btf_dump {
 	struct btf_dump_opts opts;
 	int ptr_sz;
 	bool strip_mods;
+	int last_id;
 
 	/* per-type auxiliary state */
 	struct btf_dump_type_aux_state *type_states;
+	size_t type_states_cap;
 	/* per-type optional cached unique name, must be freed, if present */
 	const char **cached_names;
+	size_t cached_names_cap;
 
 	/* topo-sorted list of dependent type definitions */
 	__u32 *emit_queue;
@@ -113,6 +116,7 @@ static void btf_dump_printf(const struct btf_dump *d, const char *fmt, ...)
 }
 
 static int btf_dump_mark_referenced(struct btf_dump *d);
+static int btf_dump_resize(struct btf_dump *d);
 
 struct btf_dump *btf_dump__new(const struct btf *btf,
 			       const struct btf_ext *btf_ext,
@@ -144,25 +148,8 @@ struct btf_dump *btf_dump__new(const struct btf *btf,
 		d->ident_names = NULL;
 		goto err;
 	}
-	d->type_states = calloc(1 + btf__get_nr_types(d->btf),
-				sizeof(d->type_states[0]));
-	if (!d->type_states) {
-		err = -ENOMEM;
-		goto err;
-	}
-	d->cached_names = calloc(1 + btf__get_nr_types(d->btf),
-				 sizeof(d->cached_names[0]));
-	if (!d->cached_names) {
-		err = -ENOMEM;
-		goto err;
-	}
 
-	/* VOID is special */
-	d->type_states[0].order_state = ORDERED;
-	d->type_states[0].emit_state = EMITTED;
-
-	/* eagerly determine referenced types for anon enums */
-	err = btf_dump_mark_referenced(d);
+	err = btf_dump_resize(d);
 	if (err)
 		goto err;
 
@@ -172,9 +159,38 @@ struct btf_dump *btf_dump__new(const struct btf *btf,
 	return ERR_PTR(err);
 }
 
+static int btf_dump_resize(struct btf_dump *d)
+{
+	int err, last_id = btf__get_nr_types(d->btf);
+
+	if (last_id <= d->last_id)
+		return 0;
+
+	if (btf_ensure_mem((void **)&d->type_states, &d->type_states_cap,
+			   sizeof(*d->type_states), last_id + 1))
+		return -ENOMEM;
+	if (btf_ensure_mem((void **)&d->cached_names, &d->cached_names_cap,
+			   sizeof(*d->cached_names), last_id + 1))
+		return -ENOMEM;
+
+	if (d->last_id == 0) {
+		/* VOID is special */
+		d->type_states[0].order_state = ORDERED;
+		d->type_states[0].emit_state = EMITTED;
+	}
+
+	/* eagerly determine referenced types for anon enums */
+	err = btf_dump_mark_referenced(d);
+	if (err)
+		return err;
+
+	d->last_id = last_id;
+	return 0;
+}
+
 void btf_dump__free(struct btf_dump *d)
 {
-	int i, cnt;
+	int i;
 
 	if (IS_ERR_OR_NULL(d))
 		return;
@@ -182,7 +198,7 @@ void btf_dump__free(struct btf_dump *d)
 	free(d->type_states);
 	if (d->cached_names) {
 		/* any set cached name is owned by us and should be freed */
-		for (i = 0, cnt = btf__get_nr_types(d->btf); i <= cnt; i++) {
+		for (i = 0; i <= d->last_id; i++) {
 			if (d->cached_names[i])
 				free((void *)d->cached_names[i]);
 		}
@@ -222,6 +238,10 @@ int btf_dump__dump_type(struct btf_dump *d, __u32 id)
 	if (id > btf__get_nr_types(d->btf))
 		return -EINVAL;
 
+	err = btf_dump_resize(d);
+	if (err)
+		return err;
+
 	d->emit_queue_cnt = 0;
 	err = btf_dump_order_type(d, id, false);
 	if (err < 0)
@@ -251,7 +271,7 @@ static int btf_dump_mark_referenced(struct btf_dump *d)
 	const struct btf_type *t;
 	__u16 vlen;
 
-	for (i = 1; i <= n; i++) {
+	for (i = d->last_id + 1; i <= n; i++) {
 		t = btf__type_by_id(d->btf, i);
 		vlen = btf_vlen(t);
 
@@ -306,6 +326,7 @@ static int btf_dump_mark_referenced(struct btf_dump *d)
 	}
 	return 0;
 }
+
 static int btf_dump_add_emit_queue_id(struct btf_dump *d, __u32 id)
 {
 	__u32 *new_queue;
@@ -1049,11 +1070,15 @@ int btf_dump__emit_type_decl(struct btf_dump *d, __u32 id,
 			     const struct btf_dump_emit_type_decl_opts *opts)
 {
 	const char *fname;
-	int lvl;
+	int lvl, err;
 
 	if (!OPTS_VALID(opts, btf_dump_emit_type_decl_opts))
 		return -EINVAL;
 
+	err = btf_dump_resize(d);
+	if (err)
+		return -EINVAL;
+
 	fname = OPTS_GET(opts, field_name, "");
 	lvl = OPTS_GET(opts, indent_level, 0);
 	d->strip_mods = OPTS_GET(opts, strip_mods, false);
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index eed5b624a784..d99bc847bf84 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -107,6 +107,7 @@ static inline void *libbpf_reallocarray(void *ptr, size_t nmemb, size_t size)
 
 void *btf_add_mem(void **data, size_t *cap_cnt, size_t elem_sz,
 		  size_t cur_cnt, size_t max_cnt, size_t add_cnt);
+int btf_ensure_mem(void **data, size_t *cap_cnt, size_t elem_sz, size_t need_cnt);
 
 static inline bool libbpf_validate_opts(const char *opts,
 					size_t opts_sz, size_t user_sz,
-- 
2.24.1

[PATCH bpf-next 2/4] libbpf: add raw dumping of BTF types

[Andrii Nakryiko]

Extend btf_dump APIs with ability to dump raw textual representation of BTF
types, with the same format as used by `bpftool btf dump file` command. Such
functionality is really useful for debugging issues with BTF, testing, BTF
introspection, etc. It is going to be used in BPF selftests for validating
BTF types.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/btf.h      |   1 +
 tools/lib/bpf/btf_dump.c | 174 +++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.map |   1 +
 3 files changed, 176 insertions(+)

diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index 57247240a20a..327ac6f39587 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h
@@ -146,6 +146,7 @@ LIBBPF_API struct btf_dump *btf_dump__new(const struct btf *btf,
 LIBBPF_API void btf_dump__free(struct btf_dump *d);
 
 LIBBPF_API int btf_dump__dump_type(struct btf_dump *d, __u32 id);
+LIBBPF_API int btf_dump__dump_type_raw(const struct btf_dump *d, __u32 id);
 
 struct btf_dump_emit_type_decl_opts {
 	/* size of this struct, for forward/backward compatiblity */
diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
index 2f9d685bd522..a01720b225cb 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -1438,3 +1438,177 @@ static const char *btf_dump_ident_name(struct btf_dump *d, __u32 id)
 {
 	return btf_dump_resolve_name(d, id, d->ident_names);
 }
+
+static const char * const btf_kind_str_mapping[] = {
+	[BTF_KIND_UNKN]		= "UNKNOWN",
+	[BTF_KIND_INT]		= "INT",
+	[BTF_KIND_PTR]		= "PTR",
+	[BTF_KIND_ARRAY]	= "ARRAY",
+	[BTF_KIND_STRUCT]	= "STRUCT",
+	[BTF_KIND_UNION]	= "UNION",
+	[BTF_KIND_ENUM]		= "ENUM",
+	[BTF_KIND_FWD]		= "FWD",
+	[BTF_KIND_TYPEDEF]	= "TYPEDEF",
+	[BTF_KIND_VOLATILE]	= "VOLATILE",
+	[BTF_KIND_CONST]	= "CONST",
+	[BTF_KIND_RESTRICT]	= "RESTRICT",
+	[BTF_KIND_FUNC]		= "FUNC",
+	[BTF_KIND_FUNC_PROTO]	= "FUNC_PROTO",
+	[BTF_KIND_VAR]		= "VAR",
+	[BTF_KIND_DATASEC]	= "DATASEC",
+};
+
+static const char *btf_kind_str(__u16 kind)
+{
+	if (kind > BTF_KIND_DATASEC)
+		return "UNKNOWN";
+	return btf_kind_str_mapping[kind];
+}
+
+static const char *btf_int_enc_str(__u8 encoding)
+{
+	switch (encoding) {
+	case 0:
+		return "(none)";
+	case BTF_INT_SIGNED:
+		return "SIGNED";
+	case BTF_INT_CHAR:
+		return "CHAR";
+	case BTF_INT_BOOL:
+		return "BOOL";
+	default:
+		return "UNKN";
+	}
+}
+
+static const char *btf_var_linkage_str(__u32 linkage)
+{
+	switch (linkage) {
+	case BTF_VAR_STATIC:
+		return "static";
+	case BTF_VAR_GLOBAL_ALLOCATED:
+		return "global-alloc";
+	default:
+		return "(unknown)";
+	}
+}
+
+static const char *btf_func_linkage_str(const struct btf_type *t)
+{
+	switch (btf_vlen(t)) {
+	case BTF_FUNC_STATIC:
+		return "static";
+	case BTF_FUNC_GLOBAL:
+		return "global";
+	case BTF_FUNC_EXTERN:
+		return "extern";
+	default:
+		return "(unknown)";
+	}
+}
+
+static const char *btf_str(const struct btf *btf, __u32 off)
+{
+	if (!off)
+		return "(anon)";
+	return btf__str_by_offset(btf, off) ?: "(invalid)";
+}
+
+int btf_dump__dump_type_raw(const struct btf_dump *d, __u32 id)
+{
+	const struct btf_type *t;
+	int kind, i;
+	__u32 vlen;
+
+	t = btf__type_by_id(d->btf, id);
+	if (!t)
+		return -EINVAL;
+
+	vlen = btf_vlen(t);
+	kind = btf_kind(t);
+
+	btf_dump_printf(d, "[%u] %s '%s'", id, btf_kind_str(kind), btf_str(d->btf, t->name_off));
+
+	switch (kind) {
+	case BTF_KIND_INT:
+		btf_dump_printf(d, " size=%u bits_offset=%u nr_bits=%u encoding=%s",
+				t->size, btf_int_offset(t), btf_int_bits(t),
+				btf_int_enc_str(btf_int_encoding(t)));
+		break;
+	case BTF_KIND_PTR:
+	case BTF_KIND_CONST:
+	case BTF_KIND_VOLATILE:
+	case BTF_KIND_RESTRICT:
+	case BTF_KIND_TYPEDEF:
+		btf_dump_printf(d, " type_id=%u", t->type);
+		break;
+	case BTF_KIND_ARRAY: {
+		const struct btf_array *arr = btf_array(t);
+
+		btf_dump_printf(d, " type_id=%u index_type_id=%u nr_elems=%u",
+				arr->type, arr->index_type, arr->nelems);
+		break;
+	}
+	case BTF_KIND_STRUCT:
+	case BTF_KIND_UNION: {
+		const struct btf_member *m = btf_members(t);
+
+		btf_dump_printf(d, " size=%u vlen=%u", t->size, vlen);
+		for (i = 0; i < vlen; i++, m++) {
+			__u32 bit_off, bit_sz;
+
+			bit_off = btf_member_bit_offset(t, i);
+			bit_sz = btf_member_bitfield_size(t, i);
+			btf_dump_printf(d, "\n\t'%s' type_id=%u bits_offset=%u",
+					btf_str(d->btf, m->name_off), m->type, bit_off);
+			if (bit_sz)
+				btf_dump_printf(d, " bitfield_size=%u", bit_sz);
+		}
+		break;
+	}
+	case BTF_KIND_ENUM: {
+		const struct btf_enum *v = btf_enum(t);
+
+		btf_dump_printf(d, " size=%u vlen=%u", t->size, vlen);
+		for (i = 0; i < vlen; i++, v++) {
+			btf_dump_printf(d, "\n\t'%s' val=%u",
+					btf_str(d->btf, v->name_off), v->val);
+		}
+		break;
+	}
+	case BTF_KIND_FWD:
+		btf_dump_printf(d, " fwd_kind=%s", btf_kflag(t) ? "union" : "struct");
+		break;
+	case BTF_KIND_FUNC:
+		btf_dump_printf(d, " type_id=%u linkage=%s", t->type, btf_func_linkage_str(t));
+		break;
+	case BTF_KIND_FUNC_PROTO: {
+		const struct btf_param *p = btf_params(t);
+
+		btf_dump_printf(d, " ret_type_id=%u vlen=%u", t->type, vlen);
+		for (i = 0; i < vlen; i++, p++) {
+			btf_dump_printf(d, "\n\t'%s' type_id=%u",
+					btf_str(d->btf, p->name_off), p->type);
+		}
+		break;
+	}
+	case BTF_KIND_VAR:
+		btf_dump_printf(d, " type_id=%u, linkage=%s",
+				t->type, btf_var_linkage_str(btf_var(t)->linkage));
+		break;
+	case BTF_KIND_DATASEC: {
+		const struct btf_var_secinfo *v = btf_var_secinfos(t);
+
+		btf_dump_printf(d, " size=%u vlen=%u", t->size, vlen);
+		for (i = 0; i < vlen; i++, v++) {
+			btf_dump_printf(d, "\n\ttype_id=%u offset=%u size=%u",
+					v->type, v->offset, v->size);
+		}
+		break;
+	}
+	default:
+		break;
+	}
+
+	return 0;
+}
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 4ebfadf45b47..8d357c24cf3d 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -331,6 +331,7 @@ LIBBPF_0.2.0 {
 		btf__new_empty;
 		btf__set_endianness;
 		btf__str_by_offset;
+		btf_dump__dump_type_raw;
 		perf_buffer__buffer_cnt;
 		perf_buffer__buffer_fd;
 		perf_buffer__epoll_fd;
-- 
2.24.1

[PATCH bpf-next 3/4] selftests/bpf: add checking of raw type dump in BTF writer APIs selftests

[Andrii Nakryiko]

Cross-validate raw BTF dump and writable BTF in a single selftest. Raw type
dump checks also serve as a good self-documentation.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 .../selftests/bpf/prog_tests/btf_write.c      | 67 ++++++++++++++++++-
 1 file changed, 64 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/btf_write.c b/tools/testing/selftests/bpf/prog_tests/btf_write.c
index 314e1e7c36df..bbf842fc8f48 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf_write.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf_write.c
@@ -1,22 +1,49 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (c) 2020 Facebook */
+#define _GNU_SOURCE
 #include <test_progs.h>
 #include <bpf/btf.h>
 
 static int duration = 0;
 
+static char *dump_buf;
+static size_t dump_buf_sz;
+static FILE *dump_buf_file;
+
+static void dump_fn(void *ctx, const char *fmt, va_list args)
+{
+	vfprintf(dump_buf_file, fmt, args);
+}
+
+static void check_type_dump(struct btf_dump *d, int type_id, const char *exp)
+{
+	fseek(dump_buf_file, 0, SEEK_SET);
+	btf_dump__dump_type_raw(d, type_id);
+	fflush(dump_buf_file);
+	dump_buf[dump_buf_sz] = 0; /* some libc implementations don't do this */
+	ASSERT_STREQ(dump_buf, exp, "type_raw_dump");
+}
+
 void test_btf_write() {
 	const struct btf_var_secinfo *vi;
 	const struct btf_type *t;
 	const struct btf_member *m;
 	const struct btf_enum *v;
 	const struct btf_param *p;
-	struct btf *btf;
+	struct btf *btf = NULL;
+	struct btf_dump *d = NULL;
 	int id, err, str_off;
 
+	dump_buf_file = open_memstream(&dump_buf, &dump_buf_sz);
+	if (CHECK(!dump_buf_file, "dump_memstream", "failed: %d\n", errno))
+		return;
+
 	btf = btf__new_empty();
 	if (CHECK(IS_ERR(btf), "new_empty", "failed: %ld\n", PTR_ERR(btf)))
-		return;
+		goto err_out;
+	d = btf_dump__new(btf, NULL, NULL, dump_fn);
+	if (!ASSERT_OK(libbpf_get_error(d), "btf_dump__new"))
+		goto err_out;
 
 	str_off = btf__find_str(btf, "int");
 	ASSERT_EQ(str_off, -ENOENT, "int_str_missing_off");
@@ -39,6 +66,7 @@ void test_btf_write() {
 	ASSERT_EQ(t->size, 4, "int_sz");
 	ASSERT_EQ(btf_int_encoding(t), BTF_INT_SIGNED, "int_enc");
 	ASSERT_EQ(btf_int_bits(t), 32, "int_bits");
+	check_type_dump(d, 1, "[1] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED");
 
 	/* invalid int size */
 	id = btf__add_int(btf, "bad sz int", 7, 0);
@@ -59,24 +87,28 @@ void test_btf_write() {
 	t = btf__type_by_id(btf, 2);
 	ASSERT_EQ(btf_kind(t), BTF_KIND_PTR, "ptr_kind");
 	ASSERT_EQ(t->type, 1, "ptr_type");
+	check_type_dump(d, 2, "[2] PTR '(anon)' type_id=1");
 
 	id = btf__add_const(btf, 5); /* points forward to restrict */
 	ASSERT_EQ(id, 3, "const_id");
 	t = btf__type_by_id(btf, 3);
 	ASSERT_EQ(btf_kind(t), BTF_KIND_CONST, "const_kind");
 	ASSERT_EQ(t->type, 5, "const_type");
+	check_type_dump(d, 3, "[3] CONST '(anon)' type_id=5");
 
 	id = btf__add_volatile(btf, 3);
 	ASSERT_EQ(id, 4, "volatile_id");
 	t = btf__type_by_id(btf, 4);
 	ASSERT_EQ(btf_kind(t), BTF_KIND_VOLATILE, "volatile_kind");
 	ASSERT_EQ(t->type, 3, "volatile_type");
+	check_type_dump(d, 4, "[4] VOLATILE '(anon)' type_id=3");
 
 	id = btf__add_restrict(btf, 4);
 	ASSERT_EQ(id, 5, "restrict_id");
 	t = btf__type_by_id(btf, 5);
 	ASSERT_EQ(btf_kind(t), BTF_KIND_RESTRICT, "restrict_kind");
 	ASSERT_EQ(t->type, 4, "restrict_type");
+	check_type_dump(d, 5, "[5] RESTRICT '(anon)' type_id=4");
 
 	/* ARRAY */
 	id = btf__add_array(btf, 1, 2, 10); /* int *[10] */
@@ -86,6 +118,7 @@ void test_btf_write() {
 	ASSERT_EQ(btf_array(t)->index_type, 1, "array_index_type");
 	ASSERT_EQ(btf_array(t)->type, 2, "array_elem_type");
 	ASSERT_EQ(btf_array(t)->nelems, 10, "array_nelems");
+	check_type_dump(d, 6, "[6] ARRAY '(anon)' type_id=2 index_type_id=1 nr_elems=10");
 
 	/* STRUCT */
 	err = btf__add_field(btf, "field", 1, 0, 0);
@@ -113,6 +146,10 @@ void test_btf_write() {
 	ASSERT_EQ(m->type, 1, "f2_type");
 	ASSERT_EQ(btf_member_bit_offset(t, 1), 32, "f2_bit_off");
 	ASSERT_EQ(btf_member_bitfield_size(t, 1), 16, "f2_bit_sz");
+	check_type_dump(d, 7,
+			"[7] STRUCT 's1' size=8 vlen=2\n"
+			"\t'f1' type_id=1 bits_offset=0\n"
+			"\t'f2' type_id=1 bits_offset=32 bitfield_size=16");
 
 	/* UNION */
 	id = btf__add_union(btf, "u1", 8);
@@ -136,6 +173,9 @@ void test_btf_write() {
 	ASSERT_EQ(m->type, 1, "f1_type");
 	ASSERT_EQ(btf_member_bit_offset(t, 0), 0, "f1_bit_off");
 	ASSERT_EQ(btf_member_bitfield_size(t, 0), 16, "f1_bit_sz");
+	check_type_dump(d, 8,
+			"[8] UNION 'u1' size=8 vlen=1\n"
+			"\t'f1' type_id=1 bits_offset=0 bitfield_size=16");
 
 	/* ENUM */
 	id = btf__add_enum(btf, "e1", 4);
@@ -156,6 +196,10 @@ void test_btf_write() {
 	v = btf_enum(t) + 1;
 	ASSERT_STREQ(btf__str_by_offset(btf, v->name_off), "v2", "v2_name");
 	ASSERT_EQ(v->val, 2, "v2_val");
+	check_type_dump(d, 9,
+			"[9] ENUM 'e1' size=4 vlen=2\n"
+			"\t'v1' val=1\n"
+			"\t'v2' val=2");
 
 	/* FWDs */
 	id = btf__add_fwd(btf, "struct_fwd", BTF_FWD_STRUCT);
@@ -164,6 +208,7 @@ void test_btf_write() {
 	ASSERT_STREQ(btf__str_by_offset(btf, t->name_off), "struct_fwd", "fwd_name");
 	ASSERT_EQ(btf_kind(t), BTF_KIND_FWD, "fwd_kind");
 	ASSERT_EQ(btf_kflag(t), 0, "fwd_kflag");
+	check_type_dump(d, 10, "[10] FWD 'struct_fwd' fwd_kind=struct");
 
 	id = btf__add_fwd(btf, "union_fwd", BTF_FWD_UNION);
 	ASSERT_EQ(id, 11, "union_fwd_id");
@@ -171,6 +216,7 @@ void test_btf_write() {
 	ASSERT_STREQ(btf__str_by_offset(btf, t->name_off), "union_fwd", "fwd_name");
 	ASSERT_EQ(btf_kind(t), BTF_KIND_FWD, "fwd_kind");
 	ASSERT_EQ(btf_kflag(t), 1, "fwd_kflag");
+	check_type_dump(d, 11, "[11] FWD 'union_fwd' fwd_kind=union");
 
 	id = btf__add_fwd(btf, "enum_fwd", BTF_FWD_ENUM);
 	ASSERT_EQ(id, 12, "enum_fwd_id");
@@ -179,6 +225,7 @@ void test_btf_write() {
 	ASSERT_EQ(btf_kind(t), BTF_KIND_ENUM, "enum_fwd_kind");
 	ASSERT_EQ(btf_vlen(t), 0, "enum_fwd_kind");
 	ASSERT_EQ(t->size, 4, "enum_fwd_sz");
+	check_type_dump(d, 12, "[12] ENUM 'enum_fwd' size=4 vlen=0");
 
 	/* TYPEDEF */
 	id = btf__add_typedef(btf, "typedef1", 1);
@@ -187,6 +234,7 @@ void test_btf_write() {
 	ASSERT_STREQ(btf__str_by_offset(btf, t->name_off), "typedef1", "typedef_name");
 	ASSERT_EQ(btf_kind(t), BTF_KIND_TYPEDEF, "typedef_kind");
 	ASSERT_EQ(t->type, 1, "typedef_type");
+	check_type_dump(d, 13, "[13] TYPEDEF 'typedef1' type_id=1");
 
 	/* FUNC & FUNC_PROTO */
 	id = btf__add_func(btf, "func1", BTF_FUNC_GLOBAL, 15);
@@ -196,6 +244,7 @@ void test_btf_write() {
 	ASSERT_EQ(t->type, 15, "func_type");
 	ASSERT_EQ(btf_kind(t), BTF_KIND_FUNC, "func_kind");
 	ASSERT_EQ(btf_vlen(t), BTF_FUNC_GLOBAL, "func_vlen");
+	check_type_dump(d, 14, "[14] FUNC 'func1' type_id=15 linkage=global");
 
 	id = btf__add_func_proto(btf, 1);
 	ASSERT_EQ(id, 15, "func_proto_id");
@@ -214,6 +263,10 @@ void test_btf_write() {
 	p = btf_params(t) + 1;
 	ASSERT_STREQ(btf__str_by_offset(btf, p->name_off), "p2", "p2_name");
 	ASSERT_EQ(p->type, 2, "p2_type");
+	check_type_dump(d, 15,
+			"[15] FUNC_PROTO '(anon)' ret_type_id=1 vlen=2\n"
+			"\t'p1' type_id=1\n"
+			"\t'p2' type_id=2");
 
 	/* VAR */
 	id = btf__add_var(btf, "var1", BTF_VAR_GLOBAL_ALLOCATED, 1);
@@ -223,6 +276,7 @@ void test_btf_write() {
 	ASSERT_EQ(btf_kind(t), BTF_KIND_VAR, "var_kind");
 	ASSERT_EQ(t->type, 1, "var_type");
 	ASSERT_EQ(btf_var(t)->linkage, BTF_VAR_GLOBAL_ALLOCATED, "var_type");
+	check_type_dump(d, 16, "[16] VAR 'var1' type_id=1, linkage=global-alloc");
 
 	/* DATASECT */
 	id = btf__add_datasec(btf, "datasec1", 12);
@@ -239,6 +293,13 @@ void test_btf_write() {
 	ASSERT_EQ(vi->type, 1, "v1_type");
 	ASSERT_EQ(vi->offset, 4, "v1_off");
 	ASSERT_EQ(vi->size, 8, "v1_sz");
-
+	check_type_dump(d, 17,
+			"[17] DATASEC 'datasec1' size=12 vlen=1\n"
+			"\ttype_id=1 offset=4 size=8");
+
+err_out:
+	fclose(dump_buf_file);
+	free(dump_buf);
+	btf_dump__free(d);
 	btf__free(btf);
 }
-- 
2.24.1

[PATCH bpf-next 4/4] selftests/bpf: test "incremental" btf_dump in C format

[Andrii Nakryiko]

Add test validating that btf_dump works fine with BTFs that are modified and
incrementally generated.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 .../selftests/bpf/prog_tests/btf_dump.c       | 105 ++++++++++++++++++
 1 file changed, 105 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/btf_dump.c b/tools/testing/selftests/bpf/prog_tests/btf_dump.c
index 39fb81d9daeb..c60091ee8a21 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf_dump.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf_dump.c
@@ -129,6 +129,109 @@ static int test_btf_dump_case(int n, struct btf_dump_test_case *t)
 	return err;
 }
 
+static char *dump_buf;
+static size_t dump_buf_sz;
+static FILE *dump_buf_file;
+
+void test_btf_dump_incremental(void)
+{
+	struct btf *btf = NULL;
+	struct btf_dump *d = NULL;
+	struct btf_dump_opts opts;
+	int id, err, i;
+
+	dump_buf_file = open_memstream(&dump_buf, &dump_buf_sz);
+	if (!ASSERT_OK_PTR(dump_buf_file, "dump_memstream"))
+		return;
+	btf = btf__new_empty();
+	if (!ASSERT_OK_PTR(btf, "new_empty"))
+		goto err_out;
+	opts.ctx = dump_buf_file;
+	d = btf_dump__new(btf, NULL, &opts, btf_dump_printf);
+	if (!ASSERT_OK(libbpf_get_error(d), "btf_dump__new"))
+		goto err_out;
+
+	/* First, generate BTF corresponding to the following C code:
+	 *
+	 * enum { VAL = 1 };
+	 *
+	 * struct s { int x; };
+	 *
+	 */
+	id = btf__add_enum(btf, NULL, 4);
+	ASSERT_EQ(id, 1, "enum_id");
+	err = btf__add_enum_value(btf, "VAL", 1);
+	ASSERT_OK(err, "enum_val_ok");
+
+	id = btf__add_int(btf, "int", 4, BTF_INT_SIGNED);
+	ASSERT_EQ(id, 2, "int_id");
+
+	id = btf__add_struct(btf, "s", 4);
+	ASSERT_EQ(id, 3, "struct_id");
+	err = btf__add_field(btf, "x", 2, 0, 0);
+	ASSERT_OK(err, "field_ok");
+
+	for (i = 1; i <= btf__get_nr_types(btf); i++) {
+		err = btf_dump__dump_type(d, i);
+		ASSERT_OK(err, "dump_type_ok");
+	}
+
+	fflush(dump_buf_file);
+	dump_buf[dump_buf_sz] = 0; /* some libc implementations don't do this */
+	ASSERT_STREQ(dump_buf,
+"enum {\n"
+"	VAL = 1,\n"
+"};\n"
+"\n"
+"struct s {\n"
+"	int x;\n"
+"};\n\n", "c_dump1");
+
+	/* Now, after dumping original BTF, append another struct that embeds
+	 * anonymous enum. It also has a name conflict with the first struct:
+	 *
+	 * struct s___2 {
+	 *     enum { VAL___2 = 1 } x;
+	 *     struct s s;
+	 * };
+	 *
+	 * This will test that btf_dump'er maintains internal state properly.
+	 * Note that VAL___2 enum value. It's because we've already emitted
+	 * that enum as a global anonymous enum, so btf_dump will ensure that
+	 * enum values don't conflict;
+	 *
+	 */
+	fseek(dump_buf_file, 0, SEEK_SET);
+
+	id = btf__add_struct(btf, "s", 4);
+	ASSERT_EQ(id, 4, "struct_id");
+	err = btf__add_field(btf, "x", 1, 0, 0);
+	ASSERT_OK(err, "field_ok");
+	err = btf__add_field(btf, "s", 3, 32, 0);
+	ASSERT_OK(err, "field_ok");
+
+	for (i = 1; i <= btf__get_nr_types(btf); i++) {
+		err = btf_dump__dump_type(d, i);
+		ASSERT_OK(err, "dump_type_ok");
+	}
+
+	fflush(dump_buf_file);
+	dump_buf[dump_buf_sz] = 0; /* some libc implementations don't do this */
+	ASSERT_STREQ(dump_buf,
+"struct s___2 {\n"
+"	enum {\n"
+"		VAL___2 = 1,\n"
+"	} x;\n"
+"	struct s s;\n"
+"};\n\n" , "c_dump1");
+
+err_out:
+	fclose(dump_buf_file);
+	free(dump_buf);
+	btf_dump__free(d);
+	btf__free(btf);
+}
+
 void test_btf_dump() {
 	int i;
 
@@ -140,4 +243,6 @@ void test_btf_dump() {
 
 		test_btf_dump_case(i, &btf_dump_test_cases[i]);
 	}
+	if (test__start_subtest("btf_dump: incremental"))
+		test_btf_dump_incremental();
 }
-- 
2.24.1

Re: [PATCH bpf-next 0/4] libbpf: add raw BTF type dumping

[Alexei Starovoitov]

On Tue, Sep 29, 2020 at 04:28:39PM -0700, Andrii Nakryiko wrote:
> Add btf_dump__dump_type_raw() API that emits human-readable low-level BTF type
> information, same as bpftool output. bpftool is not switched to this API
> because bpftool still needs to perform all the same BTF type processing logic
> to do JSON output, so benefits are pretty much zero.

If the only existing user cannot actually use such api it speaks heavily
against adding such api to libbpf. Comparing strings in tests is nice, but
could be done with C output just as well.

Re: [PATCH bpf-next 0/4] libbpf: add raw BTF type dumping

[Andrii Nakryiko]

On Tue, Sep 29, 2020 at 5:03 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue, Sep 29, 2020 at 04:28:39PM -0700, Andrii Nakryiko wrote:
> > Add btf_dump__dump_type_raw() API that emits human-readable low-level BTF type
> > information, same as bpftool output. bpftool is not switched to this API
> > because bpftool still needs to perform all the same BTF type processing logic
> > to do JSON output, so benefits are pretty much zero.
>
> If the only existing user cannot actually use such api it speaks heavily
> against adding such api to libbpf. Comparing strings in tests is nice, but
> could be done with C output just as well.

It certainly can, it just won't save much code, because bpftool would
still need to have a big switch over BTF type kinds to do JSON output.
I can do such conversion, if you prefer. I'm also thinking about
switching pahole to use this during BTF dedup verbose mode, if Arnaldo
will be fine with changing output format a bit.

Re: [PATCH bpf-next 0/4] libbpf: add raw BTF type dumping

[Alexei Starovoitov]

On Tue, Sep 29, 2020 at 05:44:48PM -0700, Andrii Nakryiko wrote:
> On Tue, Sep 29, 2020 at 5:03 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Tue, Sep 29, 2020 at 04:28:39PM -0700, Andrii Nakryiko wrote:
> > > Add btf_dump__dump_type_raw() API that emits human-readable low-level BTF type
> > > information, same as bpftool output. bpftool is not switched to this API
> > > because bpftool still needs to perform all the same BTF type processing logic
> > > to do JSON output, so benefits are pretty much zero.
> >
> > If the only existing user cannot actually use such api it speaks heavily
> > against adding such api to libbpf. Comparing strings in tests is nice, but
> > could be done with C output just as well.
> 
> It certainly can, it just won't save much code, because bpftool would
> still need to have a big switch over BTF type kinds to do JSON output.

So you're saying that most of the dump_btf_type() in bpftool/btf.c will stay as-is.
Only 'if (json_output)' will become unconditional? Hmm.
I know you don't want json in libbpf, but I think it's the point of
making a call on such things. Either libbpf gets to dump both
json and text dump_btf_type()-like output or it stays with C only.
Doing C and this text and not doing json is inconsistent.
Either libbpf can print btf in many different ways or it stays with C.
2nd format is not special in any way.
I don't think that text and json formats bring much value comparing to C,
so I would be fine with C only. But if we allow 2nd format we should
do json at the same time too to save bpftool the hassle.
And in the future we should allow 4th and 5th formats.

Re: [PATCH bpf-next 0/4] libbpf: add raw BTF type dumping

[Andrii Nakryiko]

On Tue, Sep 29, 2020 at 8:18 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue, Sep 29, 2020 at 05:44:48PM -0700, Andrii Nakryiko wrote:
> > On Tue, Sep 29, 2020 at 5:03 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Tue, Sep 29, 2020 at 04:28:39PM -0700, Andrii Nakryiko wrote:
> > > > Add btf_dump__dump_type_raw() API that emits human-readable low-level BTF type
> > > > information, same as bpftool output. bpftool is not switched to this API
> > > > because bpftool still needs to perform all the same BTF type processing logic
> > > > to do JSON output, so benefits are pretty much zero.
> > >
> > > If the only existing user cannot actually use such api it speaks heavily
> > > against adding such api to libbpf. Comparing strings in tests is nice, but
> > > could be done with C output just as well.
> >
> > It certainly can, it just won't save much code, because bpftool would
> > still need to have a big switch over BTF type kinds to do JSON output.
>
> So you're saying that most of the dump_btf_type() in bpftool/btf.c will stay as-is.
> Only 'if (json_output)' will become unconditional? Hmm.

Yes.

> I know you don't want json in libbpf, but I think it's the point of
> making a call on such things. Either libbpf gets to dump both
> json and text dump_btf_type()-like output or it stays with C only.

Right, I don't think JSON belongs in libbpf. But I fail to see why
this is the point where we need to make such a decision.

> Doing C and this text and not doing json is inconsistent.

Inconsistent with what? I've never found bpftool's raw BTF dump in
JSON format useful. At all. Saying raw BTF dump is useful and
consistent (?) only if it's both human-readable text and JSON makes no
sense to me. Libbpf doesn't have to re-implement entire bpftool
functionality.

> Either libbpf can print btf in many different ways or it stays with C.
> 2nd format is not special in any way.

I don't understand your point. With my patch it now can dump it as
valid C language definition or as a textual low-level BTF
representation.

If you are saying it should emit it in Go format, Rust format, or
other language-specific way, then sure, maybe, but it sure won't
re-use C-specific logic of btf_dump__dump_type() as is, because it is
C language specific. For Go there would be different logic, just as
for any other language. And someone will have to implement it (and
there would need to be a compelling use case for that, of course). And
it will be a different API, or at least a generic API with some enum
specifying "format" (which is the same thing, really, but inferior for
customizability reasons).

But JSON is different from that. It's just a more machine-friendly
output of textual low-level BTF dump. It could have been BSON or YAML,
but I hope you don't suggest to emit in those formats as well.

> I don't think that text and json formats bring much value comparing to C,
> so I would be fine with C only.

Noted. I disagree and find it very useful all the time, it's pretty
much the only way I look at BTF. C output is not complete: it doesn't
show functions, data sections and variables. It's not a replacement
for raw BTF dump. I don't even consider it as a different "format".
It's an entirely different and complementary (not alternative) view
(interpretation) of BTF.

> But if we allow 2nd format we should
> do json at the same time too to save bpftool the hassle.

There is no hassle for bpftool, code is written and working. Libbpf's
goal is not to minimize bpftool code either. So I hear you, but I
don't think about this the same way.

> And in the future we should allow 4th and 5th formats.

Ok, but there is no contradiction with what I'm doing here.



Regardless, feel free to drop patches #2 and #3, but patch #1 fixes
real issue, so would be nice to land it anyways. Patch #4 adds test
for changes in patch #1. Let me know if you want me to respin with
just those 2 patches.

Re: [PATCH bpf-next 0/4] libbpf: add raw BTF type dumping

[Alexei Starovoitov]

On Wed, Sep 30, 2020 at 11:22:50AM -0700, Andrii Nakryiko wrote:
> 
> If you are saying it should emit it in Go format, Rust format, or
> other language-specific way, then sure, 

Yes. that's what I'm saying. cloudflare and cilium are favoring golang.
Hopefully they can adopt skeleton when it's generated in golang.
It would probably mean some support from libbpf and vmlinux.go
Which means BTF dumping in golang.

> maybe, but it sure won't
> re-use C-specific logic of btf_dump__dump_type() as is, because it is
> C language specific. For Go there would be different logic, just as
> for any other language.

sure. that's fine.

> And someone will have to implement it (and
> there would need to be a compelling use case for that, of course). And
> it will be a different API, or at least a generic API with some enum
> specifying "format" (which is the same thing, really, but inferior for
> customizability reasons).

yes. New or reusing api doesn't matter much.
The question is what dumpers libbpf provides.

> But JSON is different from that. It's just a more machine-friendly
> output of textual low-level BTF dump. It could have been BSON or YAML,
> but I hope you don't suggest to emit in those formats as well.

why not. If libbpf does more than one there is no reason to restrict.

> 
> > I don't think that text and json formats bring much value comparing to C,
> > so I would be fine with C only.
> 
> Noted. I disagree and find it very useful all the time, it's pretty
> much the only way I look at BTF. C output is not complete: it doesn't
> show functions, data sections and variables. It's not a replacement
> for raw BTF dump. 

Ok, but it's easy to add dumping of these extra data into vmlinux.h
They can come inside /* */ or as 'extern'.
So C output can be complete and suitable for selftest's strcmp.

> Regardless, feel free to drop patches #2 and #3, but patch #1 fixes
> real issue, so would be nice to land it anyways. Patch #4 adds test
> for changes in patch #1. Let me know if you want me to respin with
> just those 2 patches.

Applied 1 and 4. I was waiting to patchwork bot to notice this partial
application, but looks like it's not that smart... yet.

Re: [PATCH bpf-next 0/4] libbpf: add raw BTF type dumping

[Andrii Nakryiko]

On Wed, Sep 30, 2020 at 2:29 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Sep 30, 2020 at 11:22:50AM -0700, Andrii Nakryiko wrote:
> >
> > If you are saying it should emit it in Go format, Rust format, or
> > other language-specific way, then sure,
>
> Yes. that's what I'm saying. cloudflare and cilium are favoring golang.
> Hopefully they can adopt skeleton when it's generated in golang.
> It would probably mean some support from libbpf and vmlinux.go
> Which means BTF dumping in golang.

Yes, if they were to adopt the skeleton approach, we'd need some sort
of BTF-to-Go struct dumping. But as for vmlinux.h, keep in mind that
that thing is supposed to be only included from the BPF side, which so
far is always pure C (apart from RedBPF approach of compiling Rust
code into BPF code). I don't think we want to have BPF-side code
written in Go?

>
> > maybe, but it sure won't
> > re-use C-specific logic of btf_dump__dump_type() as is, because it is
> > C language specific. For Go there would be different logic, just as
> > for any other language.
>
> sure. that's fine.
>
> > And someone will have to implement it (and
> > there would need to be a compelling use case for that, of course). And
> > it will be a different API, or at least a generic API with some enum
> > specifying "format" (which is the same thing, really, but inferior for
> > customizability reasons).
>
> yes. New or reusing api doesn't matter much.
> The question is what dumpers libbpf provides.
>
> > But JSON is different from that. It's just a more machine-friendly
> > output of textual low-level BTF dump. It could have been BSON or YAML,
> > but I hope you don't suggest to emit in those formats as well.
>
> why not. If libbpf does more than one there is no reason to restrict.

just extra code and maintenance burden without clear benefits, that's
the only reason

>
> >
> > > I don't think that text and json formats bring much value comparing to C,
> > > so I would be fine with C only.
> >
> > Noted. I disagree and find it very useful all the time, it's pretty
> > much the only way I look at BTF. C output is not complete: it doesn't
> > show functions, data sections and variables. It's not a replacement
> > for raw BTF dump.
>
> Ok, but it's easy to add dumping of these extra data into vmlinux.h
> They can come inside /* */ or as 'extern'.
> So C output can be complete and suitable for selftest's strcmp.

yeah, comments might work to "augment" vmlinux.h. There is still the
question of output type ordering, it's not always a single unique
ordering, which makes it harder to use for testing arbitrary BTFs. I
was very careful with existing BTF dump tests to ensure the order of
types is unique, but as a general case that's not true.

E.g., these two are equivalent:

struct a;

struct b { struct a *a; };

struct a { struct b *b; };

And:

struct b;

struct a { struct b *b; };

struct b { struct a *a; };

>
> > Regardless, feel free to drop patches #2 and #3, but patch #1 fixes
> > real issue, so would be nice to land it anyways. Patch #4 adds test
> > for changes in patch #1. Let me know if you want me to respin with
> > just those 2 patches.
>
> Applied 1 and 4. I was waiting to patchwork bot to notice this partial

thanks!

> application, but looks like it's not that smart... yet.

software, maybe some day :)