bpf: btf: print bpftool map data with btf

This consumes functionality exported in the previous patch. It does the main job of printing with BTF data. This is used in the following patch to provide a more readable output of a map's dump. It relies on json_writer to do json printing. Below is sample output where map keys are ints and values are of type struct A:

typedef int int_type;

enum E {
        E0,
        E1,
};

struct B {
        int x;
        int y;
};

struct A {
        int m;
        unsigned long long n;
        char o;
        int p[8];
        int q[4][8];
        enum E r;
        void *s;
        struct B t;
        const int u;
        int_type v;
        unsigned int w1: 3;
        unsigned int w2: 3;
};

$ sudo bpftool map dump id 14
[{
        "key": 0,
        "value": {
            "m": 1,
            "n": 2,
            "o": "c",
            "p": [15,16,17,18,15,16,17,18
            ],
            "q": [[25,26,27,28,25,26,27,28
                ],[35,36,37,38,35,36,37,38
                ],[45,46,47,48,45,46,47,48
                ],[55,56,57,58,55,56,57,58
                ]
            ],
            "r": 1,
            "s": 0x7ffd80531cf8,
            "t": {
                "x": 5,
                "y": 10
            },
            "u": 100,
            "v": 20,
            "w1": 0x7,
            "w2": 0x3
        }
    }
]

This patch uses json's {} and [] to imply struct/union and array. More explicit information can be added later. For example, a command line option can be introduced to print whether a key or value is struct or union, name of a struct etc. This will however come at the expense of duplicating info when, for example, printing an array of structs. enums are printed as ints without their names.

Signed-off-by Okash Khawaja <[email protected]>

---
 tools/bpf/bpftool/btf_dumper.c |  263 +++++++++++++++++++++++++++++++++++++++++
 tools/bpf/bpftool/btf_dumper.h |   23 +++
 2 files changed, 286 insertions(+)

tools/bpf/bpftool/btf_dumper.c new file +8 -0

@@ -0,0 +1,263 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2018 Facebook */
+
+#include <linux/btf.h>
+#include <linux/err.h>
+#include <stdio.h> /* for (FILE *) used by json_writer */
+#include <linux/bitops.h>
+#include <string.h>

tools/bpf/bpftool/btf_dumper.c new file +10 -0

+#define BITS_ROUNDDOWN_BYTES(bits) ((bits) >> 3)
+#define BITS_ROUNDUP_BYTES(bits) \
+	(BITS_ROUNDDOWN_BYTES(bits) + !!BITS_PER_BYTE_MASKED(bits))
+
+static int btf_dumper_do_type(const struct btf_dumper *d, uint32_t type_id,
+			      uint8_t bit_offset, const void *data);
+
+static void btf_dumper_ptr(const void *data, json_writer_t *jw,
+			   bool is_plain_text)
+{

tools/bpf/bpftool/btf_dumper.c new file +9 -0

+}
+
+static int btf_dumper_array(const struct btf_dumper *d, uint32_t type_id,
+			    const void *data)
+{
+	const struct btf_type *t = btf__type_by_id(d->btf, type_id);
+	struct btf_array *arr = (struct btf_array *)(t + 1);
+	int64_t elem_size;
+	int ret = 0;

tools/bpf/bpftool/btf_dumper.c new file +6 -0

+		if (ret)
+			break;
+	}
+
+	jsonw_end_array(d->jw);
+	return ret;

tools/bpf/bpftool/btf_dumper.c new file +41 -0

+	upper_bits = BITS_PER_BYTE_MASKED(bits_to_copy);
+	if (upper_bits) {
+		uint8_t mask = (1 << upper_bits) - 1;
+
+		print_num.u8_nums[bytes_to_copy - 1] &= mask;
+	}
+
+	print_num.u64_num >>= bit_offset;
+
+	if (is_plain_text)
+		jsonw_printf(jw, "0x%llx", print_num.u64_num);
+	else
+		jsonw_printf(jw, "%llu", print_num.u64_num);
+}
+
+static int btf_dumper_int(const struct btf_type *t, uint8_t bit_offset,
+			  const void *data, json_writer_t *jw,
+			  bool is_plain_text)
+{
+	uint32_t *int_type = (uint32_t *)(t + 1);
+	uint32_t bits = BTF_INT_BITS(*int_type);
+	int ret = 0;
+
+	/* if this is bit field */
+	if (bit_offset || BTF_INT_OFFSET(*int_type) ||
+	    BITS_PER_BYTE_MASKED(bits)) {
+		btf_dumper_int_bits(*int_type, bit_offset, data, jw,
+				    is_plain_text);
+		return ret;
+	}
+
+	switch (BTF_INT_ENCODING(*int_type)) {
+	case 0:
+		if (BTF_INT_BITS(*int_type) == 64)
+			jsonw_printf(jw, "%lu", *((uint64_t *)data));
+		else if (BTF_INT_BITS(*int_type) == 32)
+			jsonw_printf(jw, "%u", *((uint32_t *)data));
+		else if (BTF_INT_BITS(*int_type) == 16)
+			jsonw_printf(jw, "%hu", *((uint16_t *)data));
+		else if (BTF_INT_BITS(*int_type) == 8)
+			jsonw_printf(jw, "%hhu", *((uint8_t *)data));

tools/bpf/bpftool/btf_dumper.c new file +7 -0

+	struct btf_member *m;
+	int ret = 0;
+
+	int i, vlen;
+
+	if (!t)
+		return -EINVAL;

tools/bpf/bpftool/btf_dumper.c new file +26 -0

+	}
+
+	jsonw_end_object(d->jw);
+
+	return 0;
+}
+
+static int btf_dumper_do_type(const struct btf_dumper *d, uint32_t type_id,
+			      uint8_t bit_offset, const void *data)
+{
+	const struct btf_type *t = btf__type_by_id(d->btf, type_id);
+	int ret = 0;
+
+	switch (BTF_INFO_KIND(t->info)) {
+	case BTF_KIND_INT:
+		ret = btf_dumper_int(t, bit_offset, data, d->jw,
+				     d->is_plain_text);
+		break;
+	case BTF_KIND_STRUCT:
+	case BTF_KIND_UNION:
+		ret = btf_dumper_struct(d, type_id, data);
+		break;
+	case BTF_KIND_ARRAY:
+		ret = btf_dumper_array(d, type_id, data);
+		break;
+	case BTF_KIND_ENUM:

tools/bpf/bpftool/btf_dumper.c new file +5 -0

+		ret = -EINVAL;
+		break;
+	case BTF_KIND_TYPEDEF:
+	case BTF_KIND_VOLATILE:
+	case BTF_KIND_CONST:

tools/bpf/bpftool/btf_dumper.c new file +2 -0

+	return btf_dumper_do_type(d, type_id, 0, data);
+}

tools/bpf/bpftool/btf_dumper.h new file +13 -0

@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2018 Facebook */
+
+#ifndef BTF_DUMPER_H
+#define BTF_DUMPER_H
+
+struct btf_dumper {
+	const struct btf *btf;
+	json_writer_t *jw;
+	bool is_plain_text;
+};
+
+/* btf_dumper_type - print data along with type information

On Mon, 2 Jul 2018 11:39:15 -0700, Okash Khawaja wrote:

tools/bpf/bpftool/btf_dumper.c line 16

Quoted 3 lines

tools/bpf/bpftool/btf_dumper.c new file +2 -0

+#define BITS_PER_BYTE_MASK (BITS_PER_BYTE - 1)
+#define BITS_PER_BYTE_MASKED(bits) ((bits) & BITS_PER_BYTE_MASK)

Perhaps it's just me but BIT_OFFSET or BIT_COUNT as a name of this macro would make it more obvious to parse in the code below.

Quoted 11 lines

tools/bpf/bpftool/btf_dumper.c new file +10 -0

+#define BITS_ROUNDDOWN_BYTES(bits) ((bits) >> 3)
+#define BITS_ROUNDUP_BYTES(bits) \
+	(BITS_ROUNDDOWN_BYTES(bits) + !!BITS_PER_BYTE_MASKED(bits))
+
+static int btf_dumper_do_type(const struct btf_dumper *d, uint32_t type_id,
+			      uint8_t bit_offset, const void *data);
+
+static void btf_dumper_ptr(const void *data, json_writer_t *jw,
+			   bool is_plain_text)
+{

tools/bpf/bpftool/btf_dumper.c line 36

Quoted 10 lines

tools/bpf/bpftool/btf_dumper.c new file +10 -0

+	if (is_plain_text)
+		jsonw_printf(jw, "%p", *((uintptr_t *)data));
+	else
+		jsonw_printf(jw, "%u", *((uintptr_t *)data));
+}
+
+static int btf_dumper_modifier(const struct btf_dumper *d, uint32_t type_id,
+			       const void *data)
+{
+	int32_t actual_type_id = btf__resolve_type(d->btf, type_id);

Please prefer kernel types like __u32 wherever possible.

tools/bpf/bpftool/btf_dumper.c line 44

Quoted 8 lines

tools/bpf/bpftool/btf_dumper.c new file +8 -0

+	int ret;
+
+	if (actual_type_id < 0)
+		return actual_type_id;
+
+	ret = btf_dumper_do_type(d, actual_type_id, 0, data);
+
+	return ret;

ret is unnecessary.

tools/bpf/bpftool/btf_dumper.c line 49

Quoted 5 lines

tools/bpf/bpftool/btf_dumper.c new file +5 -0

+}
+
+static void btf_dumper_enum(const void *data, json_writer_t *jw)
+{
+	jsonw_printf(jw, "%d", *((int32_t *)data));

Unnecessary parenthesis. There is a lot of those, please remove them all.

Quoted 9 lines

tools/bpf/bpftool/btf_dumper.c new file +9 -0

+}
+
+static int btf_dumper_array(const struct btf_dumper *d, uint32_t type_id,
+			    const void *data)
+{
+	const struct btf_type *t = btf__type_by_id(d->btf, type_id);
+	struct btf_array *arr = (struct btf_array *)(t + 1);
+	int64_t elem_size;
+	int ret = 0;

tools/bpf/bpftool/btf_dumper.c line 68

Quoted 10 lines

tools/bpf/bpftool/btf_dumper.c new file +10 -0

+	uint32_t i;
+
+	elem_size = btf__resolve_size(d->btf, arr->type);
+	if (elem_size < 0)
+		return elem_size;
+
+	jsonw_start_array(d->jw);
+	for (i = 0; i < arr->nelems; i++) {
+		ret = btf_dumper_do_type(d, arr->type, 0,
+					 data + (i * elem_size));

Unnecessary parenthesis.

Quoted 6 lines

tools/bpf/bpftool/btf_dumper.c new file +6 -0

+		if (ret)
+			break;
+	}
+
+	jsonw_end_array(d->jw);
+	return ret;

tools/bpf/bpftool/btf_dumper.c line 84

Quoted 10 lines

tools/bpf/bpftool/btf_dumper.c new file +10 -0

+}
+
+static void btf_dumper_int_bits(uint32_t int_type, uint8_t bit_offset,
+				const void *data, json_writer_t *jw,
+				bool is_plain_text)
+{
+	uint32_t bits = BTF_INT_BITS(int_type);
+	uint16_t total_bits_offset;
+	uint16_t bytes_to_copy;
+	uint16_t bits_to_copy;

Please use normal int types for things which don't have to be explicitly sized. Using explicitly sized variables is bad style, and ALU operations other than on word or byte quantities are usually slower on modern CPUs.

tools/bpf/bpftool/btf_dumper.c line 88

Quoted 4 lines

tools/bpf/bpftool/btf_dumper.c new file +4 -0

+	uint8_t upper_bits;
+	union {
+		uint64_t u64_num;
+		uint8_t u8_nums[8];

Are the int types in BTF constrained to 64bit at most?

tools/bpf/bpftool/btf_dumper.c line 98

Quoted 10 lines

tools/bpf/bpftool/btf_dumper.c new file +10 -0

+	} print_num;
+
+	total_bits_offset = bit_offset + BTF_INT_OFFSET(int_type);
+	data += BITS_ROUNDDOWN_BYTES(total_bits_offset);
+	bit_offset = BITS_PER_BYTE_MASKED(total_bits_offset);
+	bits_to_copy = bits + bit_offset;
+	bytes_to_copy = BITS_ROUNDUP_BYTES(bits_to_copy);
+
+	print_num.u64_num = 0;
+	memcpy(&print_num.u64_num, data, bytes_to_copy);

This scheme is unlikely to work on big endian machines...

Quoted 42 lines

tools/bpf/bpftool/btf_dumper.c new file +41 -0

+	upper_bits = BITS_PER_BYTE_MASKED(bits_to_copy);
+	if (upper_bits) {
+		uint8_t mask = (1 << upper_bits) - 1;
+
+		print_num.u8_nums[bytes_to_copy - 1] &= mask;
+	}
+
+	print_num.u64_num >>= bit_offset;
+
+	if (is_plain_text)
+		jsonw_printf(jw, "0x%llx", print_num.u64_num);
+	else
+		jsonw_printf(jw, "%llu", print_num.u64_num);
+}
+
+static int btf_dumper_int(const struct btf_type *t, uint8_t bit_offset,
+			  const void *data, json_writer_t *jw,
+			  bool is_plain_text)
+{
+	uint32_t *int_type = (uint32_t *)(t + 1);
+	uint32_t bits = BTF_INT_BITS(*int_type);
+	int ret = 0;
+
+	/* if this is bit field */
+	if (bit_offset || BTF_INT_OFFSET(*int_type) ||
+	    BITS_PER_BYTE_MASKED(bits)) {
+		btf_dumper_int_bits(*int_type, bit_offset, data, jw,
+				    is_plain_text);
+		return ret;
+	}
+
+	switch (BTF_INT_ENCODING(*int_type)) {
+	case 0:
+		if (BTF_INT_BITS(*int_type) == 64)
+			jsonw_printf(jw, "%lu", *((uint64_t *)data));
+		else if (BTF_INT_BITS(*int_type) == 32)
+			jsonw_printf(jw, "%u", *((uint32_t *)data));
+		else if (BTF_INT_BITS(*int_type) == 16)
+			jsonw_printf(jw, "%hu", *((uint16_t *)data));
+		else if (BTF_INT_BITS(*int_type) == 8)
+			jsonw_printf(jw, "%hhu", *((uint8_t *)data));

tools/bpf/bpftool/btf_dumper.c line 150

Quoted 10 lines

tools/bpf/bpftool/btf_dumper.c new file +10 -0

+		else
+			btf_dumper_int_bits(*int_type, bit_offset, data, jw,
+					    is_plain_text);
+		break;
+	case BTF_INT_SIGNED:
+		if (BTF_INT_BITS(*int_type) == 64)
+			jsonw_printf(jw, "%ld", *((int64_t *)data));
+		else if (BTF_INT_BITS(*int_type) == 32)
+			jsonw_printf(jw, "%d", *((int32_t *)data));
+		else if (BTF_INT_BITS(*int_type) ==  16)

Please drop the double space. Both for 16 where it makes no sense and for 8 where it's marginally useful but not really.

tools/bpf/bpftool/btf_dumper.c line 160

Quoted 10 lines

tools/bpf/bpftool/btf_dumper.c new file +10 -0

+			jsonw_printf(jw, "%hd", *((int16_t *)data));
+		else if (BTF_INT_BITS(*int_type) ==  8)
+			jsonw_printf(jw, "%hhd", *((int8_t *)data));
+		else
+			btf_dumper_int_bits(*int_type, bit_offset, data, jw,
+					    is_plain_text);
+		break;
+	case BTF_INT_CHAR:
+		if (*((char *)data) == '\0')
+			jsonw_null(jw);

Mm.. I don't think 0 char is equivalent to null.

tools/bpf/bpftool/btf_dumper.c line 162

Quoted 3 lines

tools/bpf/bpftool/btf_dumper.c new file +2 -0

+		else if (isprint(*((char *)data)))
+			jsonw_printf(jw, "\"%c\"", *((char *)data));

This looks very suspicious. So if I see a "6" for a char field it's either a 6 ('\u0006') or a 54 ('6')...

tools/bpf/bpftool/btf_dumper.c line 167

Quoted 6 lines

tools/bpf/bpftool/btf_dumper.c new file +5 -0

+		else
+			if (is_plain_text)
+				jsonw_printf(jw, "%hhx", *((char *)data));
+			else
+				jsonw_printf(jw, "%hhd", *((char *)data));

... I think you need to always print a string, and express it as \u00%02hhx for non-printable.

tools/bpf/bpftool/btf_dumper.c line 174

Quoted 8 lines

tools/bpf/bpftool/btf_dumper.c new file +7 -0

+		break;
+	case BTF_INT_BOOL:
+		jsonw_bool(jw, *((int *)data));
+		break;
+	default:
+		/* shouldn't happen */
+		ret = -EINVAL;

You only set ret to something else than 0 here just to break and immediately return. Please remove the ret variable.

tools/bpf/bpftool/btf_dumper.c line 184

Quoted 10 lines

tools/bpf/bpftool/btf_dumper.c new file +10 -0

+		break;
+	}
+
+	return ret;
+}
+
+static int btf_dumper_struct(const struct btf_dumper *d, uint32_t type_id,
+			     const void *data)
+{
+	const struct btf_type *t = btf__type_by_id(d->btf, type_id);

Please don't call functions which need error checking as initialized the if below looks very awkward..

Quoted 7 lines

tools/bpf/bpftool/btf_dumper.c new file +7 -0

+	struct btf_member *m;
+	int ret = 0;
+
+	int i, vlen;
+
+	if (!t)
+		return -EINVAL;

tools/bpf/bpftool/btf_dumper.c line 200

Quoted 10 lines

tools/bpf/bpftool/btf_dumper.c new file +10 -0

+
+	vlen = BTF_INFO_VLEN(t->info);
+	jsonw_start_object(d->jw);
+	m = (struct btf_member *)(t + 1);
+
+	for (i = 0; i < vlen; i++) {
+		jsonw_name(d->jw, btf__name_by_offset(d->btf, m[i].name_off));
+		ret = btf_dumper_do_type(d, m[i].type,
+					 BITS_PER_BYTE_MASKED(m[i].offset), data
+					 + BITS_ROUNDDOWN_BYTES(m[i].offset));

Please use a temp variable to avoid this awkward multi-line sum.

Review

Quoted 2 lines

(quoted diff) +2 -0

+		if (ret)
+			return ret;

You can't return without jsonw_end_object().

Quoted 26 lines

tools/bpf/bpftool/btf_dumper.c new file +26 -0

+	}
+
+	jsonw_end_object(d->jw);
+
+	return 0;
+}
+
+static int btf_dumper_do_type(const struct btf_dumper *d, uint32_t type_id,
+			      uint8_t bit_offset, const void *data)
+{
+	const struct btf_type *t = btf__type_by_id(d->btf, type_id);
+	int ret = 0;
+
+	switch (BTF_INFO_KIND(t->info)) {
+	case BTF_KIND_INT:
+		ret = btf_dumper_int(t, bit_offset, data, d->jw,
+				     d->is_plain_text);
+		break;
+	case BTF_KIND_STRUCT:
+	case BTF_KIND_UNION:
+		ret = btf_dumper_struct(d, type_id, data);
+		break;
+	case BTF_KIND_ARRAY:
+		ret = btf_dumper_array(d, type_id, data);
+		break;
+	case BTF_KIND_ENUM:

tools/bpf/bpftool/btf_dumper.c line 238

Quoted 10 lines

tools/bpf/bpftool/btf_dumper.c new file +10 -0

+		btf_dumper_enum(data, d->jw);
+		break;
+	case BTF_KIND_PTR:
+		btf_dumper_ptr(data, d->jw, d->is_plain_text);
+		break;
+	case BTF_KIND_UNKN:
+		jsonw_printf(d->jw, "(unknown)");
+		break;
+	case BTF_KIND_FWD:
+		/* map key or value can't be forward */

Right, but you have to print _something_, otherwise we would have a name without a value, which would break JSON, no?

Quoted 5 lines

tools/bpf/bpftool/btf_dumper.c new file +5 -0

+		ret = -EINVAL;
+		break;
+	case BTF_KIND_TYPEDEF:
+	case BTF_KIND_VOLATILE:
+	case BTF_KIND_CONST:

tools/bpf/bpftool/btf_dumper.c line 253

Quoted 10 lines

tools/bpf/bpftool/btf_dumper.c new file +10 -0

+	case BTF_KIND_RESTRICT:
+		ret = btf_dumper_modifier(d, type_id, data);
+		break;
+	default:
+		jsonw_printf(d->jw, "(unsupported-kind");
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;

Why return ret; at all, there is no code after the switch just return directly from cases and save 9 LOC.

tools/bpf/bpftool/btf_dumper.c line 260

Quoted 7 lines

tools/bpf/bpftool/btf_dumper.c new file +7 -0

+}
+
+int32_t btf_dumper_type(const struct btf_dumper *d, uint32_t type_id,
+			const void *data)
+{
+	if (!d)
+		return -EINVAL;

No need for defensive programming.

Quoted 23 lines

tools/bpf/bpftool/btf_dumper.c new file +2 -0

+	return btf_dumper_do_type(d, type_id, 0, data);
+}

tools/bpf/bpftool/btf_dumper.h new file +18 -0

@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2018 Facebook */
+
+#ifndef BTF_DUMPER_H
+#define BTF_DUMPER_H
+
+struct btf_dumper {
+	const struct btf *btf;
+	json_writer_t *jw;
+	bool is_plain_text;
+};
+
+/* btf_dumper_type - print data along with type information
+ * @d: an instance containing context for dumping types
+ * @type_id: index in btf->types array. this points to the type to be dumped
+ * @data: pointer the actual data, i.e. the values to be printed
+ *
+ * Returns zero on success and negative error code otherwise

tools/bpf/bpftool/btf_dumper.h line 23

Quoted 5 lines

tools/bpf/bpftool/btf_dumper.h new file +5 -0

+ */
+int32_t btf_dumper_type(const struct btf_dumper *d, uint32_t type_id,
+			const void *data);
+
+#endif

Please don't add header files for a single struct and single function. Just put this in main.h.

Quoted 6 lines · Jakub Kicinski

tools/bpf/bpftool/btf_dumper.c line 16

Quoted 2 lines · Okash Khawaja

tools/bpf/bpftool/btf_dumper.c new file +2 -0

+#define BITS_PER_BYTE_MASK (BITS_PER_BYTE - 1)
+#define BITS_PER_BYTE_MASKED(bits) ((bits) & BITS_PER_BYTE_MASK)  

Perhaps it's just me but BIT_OFFSET or BIT_COUNT as a name of this macro would make it more obvious to parse in the code below.

I don't mind either. However these macro names are also used inside kernel for same purpose. For sake of consistency, I'd recommend we keep them :)

Quoted 4 lines

Quoted 4 lines

tools/bpf/bpftool/btf_dumper.c new file +4 -0

+	} print_num;
+
+	total_bits_offset = bit_offset + BTF_INT_OFFSET(int_type);
+	data += BITS_ROUNDDOWN_BYTES(total_bits_offset);

Quoted 8 lines

tools/bpf/bpftool/btf_dumper.c line 98

Quoted 6 lines

tools/bpf/bpftool/btf_dumper.c new file +6 -0

+	bit_offset = BITS_PER_BYTE_MASKED(total_bits_offset);
+	bits_to_copy = bits + bit_offset;
+	bytes_to_copy = BITS_ROUNDUP_BYTES(bits_to_copy);
+
+	print_num.u64_num = 0;
+	memcpy(&print_num.u64_num, data, bytes_to_copy);  

This scheme is unlikely to work on big endian machines...

Can you give an example how?

Quoted 55 lines

Quoted 41 lines

tools/bpf/bpftool/btf_dumper.c new file +41 -0

+	upper_bits = BITS_PER_BYTE_MASKED(bits_to_copy);
+	if (upper_bits) {
+		uint8_t mask = (1 << upper_bits) - 1;
+
+		print_num.u8_nums[bytes_to_copy - 1] &= mask;
+	}
+
+	print_num.u64_num >>= bit_offset;
+
+	if (is_plain_text)
+		jsonw_printf(jw, "0x%llx", print_num.u64_num);
+	else
+		jsonw_printf(jw, "%llu", print_num.u64_num);
+}
+
+static int btf_dumper_int(const struct btf_type *t, uint8_t bit_offset,
+			  const void *data, json_writer_t *jw,
+			  bool is_plain_text)
+{
+	uint32_t *int_type = (uint32_t *)(t + 1);
+	uint32_t bits = BTF_INT_BITS(*int_type);
+	int ret = 0;
+
+	/* if this is bit field */
+	if (bit_offset || BTF_INT_OFFSET(*int_type) ||
+	    BITS_PER_BYTE_MASKED(bits)) {
+		btf_dumper_int_bits(*int_type, bit_offset, data, jw,
+				    is_plain_text);
+		return ret;
+	}
+
+	switch (BTF_INT_ENCODING(*int_type)) {
+	case 0:
+		if (BTF_INT_BITS(*int_type) == 64)
+			jsonw_printf(jw, "%lu", *((uint64_t *)data));
+		else if (BTF_INT_BITS(*int_type) == 32)
+			jsonw_printf(jw, "%u", *((uint32_t *)data));
+		else if (BTF_INT_BITS(*int_type) == 16)
+			jsonw_printf(jw, "%hu", *((uint16_t *)data));
+		else if (BTF_INT_BITS(*int_type) == 8)
+			jsonw_printf(jw, "%hhu", *((uint8_t *)data));

tools/bpf/bpftool/btf_dumper.c line 150

Quoted 10 lines

tools/bpf/bpftool/btf_dumper.c new file +10 -0

+		else
+			btf_dumper_int_bits(*int_type, bit_offset, data, jw,
+					    is_plain_text);
+		break;
+	case BTF_INT_SIGNED:
+		if (BTF_INT_BITS(*int_type) == 64)
+			jsonw_printf(jw, "%ld", *((int64_t *)data));
+		else if (BTF_INT_BITS(*int_type) == 32)
+			jsonw_printf(jw, "%d", *((int32_t *)data));
+		else if (BTF_INT_BITS(*int_type) ==  16)  

Please drop the double space. Both for 16 where it makes no sense and for 8 where it's marginally useful but not really.

Quoted 12 lines

tools/bpf/bpftool/btf_dumper.c line 160

Quoted 10 lines

tools/bpf/bpftool/btf_dumper.c new file +10 -0

+			jsonw_printf(jw, "%hd", *((int16_t *)data));
+		else if (BTF_INT_BITS(*int_type) ==  8)
+			jsonw_printf(jw, "%hhd", *((int8_t *)data));
+		else
+			btf_dumper_int_bits(*int_type, bit_offset, data, jw,
+					    is_plain_text);
+		break;
+	case BTF_INT_CHAR:
+		if (*((char *)data) == '\0')
+			jsonw_null(jw);  

Mm.. I don't think 0 char is equivalent to null.

Yes, thanks. Will fix.

Quoted 1 line

Quoted 5 lines

tools/bpf/bpftool/btf_dumper.c line 162

Quoted 2 lines

tools/bpf/bpftool/btf_dumper.c new file +2 -0

+		else if (isprint(*((char *)data)))
+			jsonw_printf(jw, "\"%c\"", *((char *)data));  

This looks very suspicious. So if I see a "6" for a char field it's either a 6 ('\u0006') or a 54 ('6')...

It will always be 54. May be I missed your point. Could you explain why it would be other than 54?

Quoted 3 lines

tools/bpf/bpftool/btf_dumper.c line 165

Quoted 3 lines

tools/bpf/bpftool/btf_dumper.c new file +3 -0

+		else
+			if (is_plain_text)
+				jsonw_printf(jw, "%hhx", *((char *)data));

Quoted 5 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+			else
+				jsonw_printf(jw, "%hhd", *((char *)data));  

... I think you need to always print a string, and express it as \u00%02hhx for non-printable.

Okay that makes sense

Quoted 6 lines

Quoted 5 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+			else
+				jsonw_printf(jw, "%hhd", *((char *)data));    

... I think you need to always print a string, and express it as \u00%02hhx for non-printable.

Okay that makes sense

Yeah, IDK, char can be used as a byte as well as a string. In eBPF it may actually be more likely to just be used as a raw byte buffer...

Either way I think it may be nice to keep it consistent, at least for the JSON output could we do either always ints or always characters?

Quoted 9 lines · Jakub Kicinski

Quoted 6 lines

Quoted 5 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+			else
+				jsonw_printf(jw, "%hhd", *((char *)data));    

... I think you need to always print a string, and express it as \u00%02hhx for non-printable.

Okay that makes sense

Yeah, IDK, char can be used as a byte as well as a string. In eBPF it may actually be more likely to just be used as a raw byte buffer...

Actually, what is the definition/purpose of BTF_INT_CHAR? There seems to be no BTF_INT_SHORT and BTF_INT_SIGNED can simply be of size 8... Is normal int only used for bitfields of size 8 and BTF_INT_CHAR for char variables?

The kernel seems to be rejecting combinations of those flags, is unsigned char going to not be marked as char then?

Quoted 3 lines

Either way I think it may be nice to keep it consistent, at least for the JSON output could we do either always ints or always characters?

Quoted 16 lines · Jakub Kicinski

Quoted 9 lines · Jakub Kicinski

Quoted 6 lines

Quoted 5 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+			else
+				jsonw_printf(jw, "%hhd", *((char *)data));    

... I think you need to always print a string, and express it as \u00%02hhx for non-printable.

Okay that makes sense

Yeah, IDK, char can be used as a byte as well as a string. In eBPF it may actually be more likely to just be used as a raw byte buffer...

Actually, what is the definition/purpose of BTF_INT_CHAR? There seems to be no BTF_INT_SHORT and BTF_INT_SIGNED can simply be of size 8... Is normal int only used for bitfields of size 8 and BTF_INT_CHAR for char variables?

Quoted 2 lines

The kernel seems to be rejecting combinations of those flags, is unsigned char going to not be marked as char then?

BTF_INT_ENOCODING (CHAR/SIGNED/BOOL) is for formatting (e.g. pretty print). It is mainly how CTF is using it also. Hence, BTF_INT_ENCODINGs is not a 1:1 mapping to C integer types.
The size of an interger is described by BTF_INT_BITS instead.

Quoted 4 lines

Quoted 3 lines

Either way I think it may be nice to keep it consistent, at least for the JSON output could we do either always ints or always characters?

Quoted 25 lines · Martin KaFai Lau

Quoted 11 lines · Jakub Kicinski

Quoted 9 lines · Jakub Kicinski

Quoted 6 lines

Quoted 5 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+			else
+				jsonw_printf(jw, "%hhd", *((char *)data));      

... I think you need to always print a string, and express it as \u00%02hhx for non-printable.

Okay that makes sense

Yeah, IDK, char can be used as a byte as well as a string. In eBPF it may actually be more likely to just be used as a raw byte buffer...

Quoted 7 lines

Actually, what is the definition/purpose of BTF_INT_CHAR? There seems to be no BTF_INT_SHORT and BTF_INT_SIGNED can simply be of size 8... Is normal int only used for bitfields of size 8 and BTF_INT_CHAR for char variables?

The kernel seems to be rejecting combinations of those flags, is unsigned char going to not be marked as char then?

BTF_INT_ENOCODING (CHAR/SIGNED/BOOL) is for formatting (e.g. pretty print). It is mainly how CTF is using it also. Hence, BTF_INT_ENCODINGs is not a 1:1 mapping to C integer types.
The size of an interger is described by BTF_INT_BITS instead.

Quoted 1 line

Quoted 3 lines

Quoted 3 lines

Quoted 2 lines

Either way I think it may be nice to keep it consistent, at least for the JSON output could we do either always ints or always characters?

for !isprint() case, will "\x%02hhx" make more sense?

Quoted 10 lines · Okash Khawaja

Quoted 6 lines · Jakub Kicinski

tools/bpf/bpftool/btf_dumper.c line 16

Quoted 2 lines · Okash Khawaja

tools/bpf/bpftool/btf_dumper.c new file +2 -0

+#define BITS_PER_BYTE_MASK (BITS_PER_BYTE - 1)
+#define BITS_PER_BYTE_MASKED(bits) ((bits) & BITS_PER_BYTE_MASK)  

Perhaps it's just me but BIT_OFFSET or BIT_COUNT as a name of this macro would make it more obvious to parse in the code below.

I don't mind either. However these macro names are also used inside kernel for same purpose. For sake of consistency, I'd recommend we keep them :)

Ugh, okay :)

Quoted 12 lines

Quoted 4 lines

Quoted 4 lines

tools/bpf/bpftool/btf_dumper.c new file +4 -0

+	} print_num;
+
+	total_bits_offset = bit_offset + BTF_INT_OFFSET(int_type);
+	data += BITS_ROUNDDOWN_BYTES(total_bits_offset);

Quoted 8 lines

tools/bpf/bpftool/btf_dumper.c line 98

Quoted 6 lines

tools/bpf/bpftool/btf_dumper.c new file +6 -0

+	bit_offset = BITS_PER_BYTE_MASKED(total_bits_offset);
+	bits_to_copy = bits + bit_offset;
+	bytes_to_copy = BITS_ROUNDUP_BYTES(bits_to_copy);
+
+	print_num.u64_num = 0;
+	memcpy(&print_num.u64_num, data, bytes_to_copy);  

This scheme is unlikely to work on big endian machines...

Quoted 1 line

Can you give an example how?

On BE:

Input:         [0x01, 0x82]
Bit length:    15
Bytes to copy:  2
bit_offset:     0
upper_bits:     7

print_num.u64_num = 0;
# [0, 0, 0, 0, 0, 0, 0, 0]

memcpy(&print_num.u64_num, data, bytes_to_copy);
# [0x01, 0x82, 0, 0, 0, 0, 0, 0]

mask = (1 << upper_bits) - 1;

# mask = 0x7f

print_num.u8_nums[bytes_to_copy - 1] &= mask;
# [0x01, 0x02, 0, 0, 0, 0, 0, 0]

printf("0x%llx", print_num.u64_num);
# 0x0102000000000000 AKA 72620543991349248
# expected:
# 0x0102 AKA 258

Am I missing something?

Quoted 71 lines

Quoted 56 lines

Quoted 42 lines

tools/bpf/bpftool/btf_dumper.c new file +41 -0

+	upper_bits = BITS_PER_BYTE_MASKED(bits_to_copy);
+	if (upper_bits) {
+		uint8_t mask = (1 << upper_bits) - 1;
+
+		print_num.u8_nums[bytes_to_copy - 1] &= mask;
+	}
+
+	print_num.u64_num >>= bit_offset;
+
+	if (is_plain_text)
+		jsonw_printf(jw, "0x%llx", print_num.u64_num);
+	else
+		jsonw_printf(jw, "%llu", print_num.u64_num);
+}
+
+static int btf_dumper_int(const struct btf_type *t, uint8_t bit_offset,
+			  const void *data, json_writer_t *jw,
+			  bool is_plain_text)
+{
+	uint32_t *int_type = (uint32_t *)(t + 1);
+	uint32_t bits = BTF_INT_BITS(*int_type);
+	int ret = 0;
+
+	/* if this is bit field */
+	if (bit_offset || BTF_INT_OFFSET(*int_type) ||
+	    BITS_PER_BYTE_MASKED(bits)) {
+		btf_dumper_int_bits(*int_type, bit_offset, data, jw,
+				    is_plain_text);
+		return ret;
+	}
+
+	switch (BTF_INT_ENCODING(*int_type)) {
+	case 0:
+		if (BTF_INT_BITS(*int_type) == 64)
+			jsonw_printf(jw, "%lu", *((uint64_t *)data));
+		else if (BTF_INT_BITS(*int_type) == 32)
+			jsonw_printf(jw, "%u", *((uint32_t *)data));
+		else if (BTF_INT_BITS(*int_type) == 16)
+			jsonw_printf(jw, "%hu", *((uint16_t *)data));
+		else if (BTF_INT_BITS(*int_type) == 8)
+			jsonw_printf(jw, "%hhu", *((uint8_t *)data));

tools/bpf/bpftool/btf_dumper.c line 150

Quoted 10 lines

tools/bpf/bpftool/btf_dumper.c new file +10 -0

+		else
+			btf_dumper_int_bits(*int_type, bit_offset, data, jw,
+					    is_plain_text);
+		break;
+	case BTF_INT_SIGNED:
+		if (BTF_INT_BITS(*int_type) == 64)
+			jsonw_printf(jw, "%ld", *((int64_t *)data));
+		else if (BTF_INT_BITS(*int_type) == 32)
+			jsonw_printf(jw, "%d", *((int32_t *)data));
+		else if (BTF_INT_BITS(*int_type) ==  16)  

Please drop the double space. Both for 16 where it makes no sense and for 8 where it's marginally useful but not really.

Quoted 12 lines

tools/bpf/bpftool/btf_dumper.c line 160

Quoted 10 lines

tools/bpf/bpftool/btf_dumper.c new file +10 -0

+			jsonw_printf(jw, "%hd", *((int16_t *)data));
+		else if (BTF_INT_BITS(*int_type) ==  8)
+			jsonw_printf(jw, "%hhd", *((int8_t *)data));
+		else
+			btf_dumper_int_bits(*int_type, bit_offset, data, jw,
+					    is_plain_text);
+		break;
+	case BTF_INT_CHAR:
+		if (*((char *)data) == '\0')
+			jsonw_null(jw);  

Mm.. I don't think 0 char is equivalent to null.

Yes, thanks. Will fix.

Quoted 1 line

Quoted 7 lines

Quoted 5 lines

tools/bpf/bpftool/btf_dumper.c line 162

Quoted 2 lines

tools/bpf/bpftool/btf_dumper.c new file +2 -0

+		else if (isprint(*((char *)data)))
+			jsonw_printf(jw, "\"%c\"", *((char *)data));  

This looks very suspicious. So if I see a "6" for a char field it's either a 6 ('\u0006') or a 54 ('6')...

It will always be 54. May be I missed your point. Could you explain why it would be other than 54?

Ah, I think I missed that %c is in quotes...

Quoted 3 lines

Quoted 3 lines

tools/bpf/bpftool/btf_dumper.c line 165

Quoted 3 lines

tools/bpf/bpftool/btf_dumper.c new file +3 -0

+		else
+			if (is_plain_text)
+				jsonw_printf(jw, "%hhx", *((char *)data));

This seems to be missing a "0x" prefix?

Quoted 6 lines

Quoted 6 lines

Review

Quoted 3 lines

(quoted diff) +2 -0

+			else
+				jsonw_printf(jw, "%hhd", *((char *)data));  

... I think you need to always print a string, and express it as \u00%02hhx for non-printable.

Quoted 1 line

Okay that makes sense

Yeah, IDK, char can be used as a byte as well as a string. In eBPF it may actually be more likely to just be used as a raw byte buffer... Either way I think it may be nice to keep it consistent, at least for the JSON output could we do either always ints or always characters?

This patch augments the output of bpftool's map dump and map lookup commands to print data along side btf info, if the correspondin btf info is available. The outputs for each of map dump and map lookup commands are augmented in two ways:

1. when neither of -j and -p are supplied, btf-ful map data is printed whose aim is human readability. This means no commitments for json- or backward- compatibility.

2. when either -j or -p are supplied, a new json object named "formatted" is added for each key-value pair. This object contains the same data as the key-value pair, but with btf info. "formatted" object promises json- and backward- compatibility. Below is a sample output.

$ bpftool map dump -p id 8
[{
        "key": ["0x0f","0x00","0x00","0x00"
        ],
        "value": ["0x03", "0x00", "0x00", "0x00", ...
        ],
        "formatted": {
                "key": 15,
                "value": {
                        "int_field":  3,
                        ...
                }
        }
}
]

This patch calls btf_dumper introduced in previous patch to accomplish the above. Indeed, btf-ful info is only displayed if btf data for the given map is available. Otherwise existing output is displayed as-is.

Signed-off-by Okash Khawaja <[email protected]>

---
 tools/bpf/bpftool/map.c |  174 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 166 insertions(+), 8 deletions(-)

tools/bpf/bpftool/map.c +0 -0

@@ -41,9 +41,13 @@
 #include <unistd.h>

tools/bpf/bpftool/map.c +4 -0

 static const char * const map_type_name[] = {

@@ -148,8 +152,99 @@ int map_parse_fd_and_info(int *argc, cha
 	return fd;
 }
 
+static int do_dump_btf(const struct btf_dumper *d,
+		       struct bpf_map_info *map_info, void *key,
+		       void *value)
+{

tools/bpf/bpftool/map.c +26 -0

+	struct bpf_btf_info btf_info = { 0 };
+	__u32 len = sizeof(btf_info);
+	void *ptr = NULL, *temp_ptr;
+	struct btf *btf = NULL;
+	uint32_t last_size;
+	int err;
+
+	if (btf_fd < 0)
+		return NULL;
+
+	/* we won't know btf_size until we call bpf_obj_get_info_by_fd(). so
+	 * let's start with a sane default - 4KiB here - and resize it only if
+	 * bpf_obj_get_info_by_fd() needs a bigger buffer. the do-while loop
+	 * below should run a maximum of two iterations and that will be when
+	 * we have to resize to a bigger buffer.
+	 */
+	btf_info.btf_size = 4096;
+	do {
+		last_size = btf_info.btf_size;
+		temp_ptr = realloc(ptr, last_size);
+		if (!temp_ptr) {
+			p_err("unable to allocate memory for debug info");
+			goto exit_free;
+		}
+
+		ptr = temp_ptr;

tools/bpf/bpftool/map.c +22 -1

+		btf = NULL;
+	}
+
+exit_free:
+	close(btf_fd);
+	free(ptr);
+
+	return btf;
+}
+
+static json_writer_t *get_btf_writer(void)
+{
+	json_writer_t *jw = jsonw_new(stdout);
+
+	if (!jw)
+		return NULL;
+	jsonw_pretty(jw, true);
+
+	return jw;
+}
+
 static void print_entry_json(struct bpf_map_info *info, unsigned char *key,
-			     unsigned char *value)
+			     unsigned char *value, struct btf *btf)
 {
 	jsonw_start_object(json_wtr);
 

tools/bpf/bpftool/map.c +8 -1

+			jsonw_name(json_wtr, "formatted");
+			do_dump_btf(&d, info, key, value);
+		}
 	} else {
 		unsigned int i, n;
 

@@ -508,10 +612,12 @@ static int do_show(int argc, char **argv
 
 static int do_dump(int argc, char **argv)
 {
+	struct bpf_map_info info = {};
 	void *key, *value, *prev_key;
 	unsigned int num_elems = 0;
-	struct bpf_map_info info = {};
 	__u32 len = sizeof(info);
+	json_writer_t *btf_wtr;
+	struct btf *btf = NULL;
 	int err;
 	int fd;
 

@@ -537,8 +643,22 @@ static int do_dump(int argc, char **argv
 	}
 
 	prev_key = NULL;
+
+	btf = get_btf(&info);
 	if (json_output)

tools/bpf/bpftool/map.c +35 -6

+			}
+		}
+
 	while (true) {
 		err = bpf_map_get_next_key(fd, prev_key, key);
 		if (err) {

@@ -549,9 +669,18 @@ static int do_dump(int argc, char **argv
 
 		if (!bpf_map_lookup_elem(fd, key, value)) {
 			if (json_output)
-				print_entry_json(&info, key, value);
+				print_entry_json(&info, key, value, btf);
 			else
-				print_entry_plain(&info, key, value);
+				if (btf) {
+					struct btf_dumper d = {
+						.btf = btf,
+						.jw = btf_wtr,
+						.is_plain_text = true,
+					};
+					do_dump_btf(&d, &info, key, value);
+				} else {
+					print_entry_plain(&info, key, value);
+				}
 		} else {
 			if (json_output) {
 				jsonw_name(json_wtr, "key");

@@ -574,14 +703,19 @@ static int do_dump(int argc, char **argv
 
 	if (json_output)
 		jsonw_end_array(json_wtr);
-	else
+	else if (btf) {
+		jsonw_end_array(btf_wtr);
+		jsonw_destroy(&btf_wtr);
+	} else {
 		printf("Found %u element%s\n", num_elems,
 		       num_elems != 1 ? "s" : "");
+	}
 
 exit_free:
 	free(key);
 	free(value);
 	close(fd);
+	btf__free(btf);
 
 	return err;
 }

@@ -637,6 +771,8 @@ static int do_lookup(int argc, char **ar
 {
 	struct bpf_map_info info = {};
 	__u32 len = sizeof(info);
+	json_writer_t *btf_wtr;
+	struct btf *btf = NULL;
 	void *key, *value;
 	int err;
 	int fd;

@@ -662,10 +798,31 @@ static int do_lookup(int argc, char **ar
 
 	err = bpf_map_lookup_elem(fd, key, value);
 	if (!err) {
-		if (json_output)
-			print_entry_json(&info, key, value);
-		else
+		btf = get_btf(&info);
+		if (json_output) {
+			print_entry_json(&info, key, value, btf);
+		} else if (btf) {
+			/* if here json_wtr wouldn't have been initialised,
+			 * so let's create separate writer for btf
+			 */
+			btf_wtr = get_btf_writer();
+			if (!btf_wtr) {
+				p_info("failed to create json writer for btf. falling back to plain output");
+				btf__free(btf);
+				btf = NULL;
+				print_entry_plain(&info, key, value);

tools/bpf/bpftool/map.c +2 -0

 			print_entry_plain(&info, key, value);
+		}
 	} else if (errno == ENOENT) {
 		if (json_output) {
 			jsonw_null(json_wtr);

@@ -682,6 +839,7 @@ exit_free:
 	free(key);
 	free(value);
 	close(fd);
+	btf__free(btf);
 
 	return err;
 }