linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 bpf-next 0/3] libbpf: BTF dumper support for typed data
@ 2021-07-15 15:15 Alan Maguire
  2021-07-15 15:15 ` [PATCH v6 bpf-next 1/3] " Alan Maguire
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Alan Maguire @ 2021-07-15 15:15 UTC (permalink / raw)
  To: ast, daniel, andrii
  Cc: kafai, songliubraving, yhs, john.fastabend, kpsingh, morbo,
	shuah, bpf, netdev, linux-kselftest, linux-kernel, Alan Maguire

Add a libbpf dumper function that supports dumping a representation
of data passed in using the BTF id associated with the data in a
manner similar to the bpf_snprintf_btf helper.

Default output format is identical to that dumped by bpf_snprintf_btf()
(bar using tabs instead of spaces for indentation, but the indent string
can be customized also); for example, a "struct sk_buff" representation
would look like this:

(struct sk_buff){
        (union){
                (struct){
                        .next = (struct sk_buff *)0xffffffffffffffff,
                        .prev = (struct sk_buff *)0xffffffffffffffff,
                        (union){
                                .dev = (struct net_device *)0xffffffffffffffff,
                                .dev_scratch = (long unsigned int)18446744073709551615,
                        },
        },
...

Patch 1 implements the dump functionality in a manner similar
to that in kernel/bpf/btf.c, but with a view to fitting into
libbpf more naturally.  For example, rather than using flags,
boolean dump options are used to control output.  In addition,
rather than combining checks for display (such as is this
field zero?) and actual display - as is done for the kernel
code - the code is organized to separate zero and overflow
checks from type display.

Patch 2 adds ASSERT_STRNEQ() for use in the following BTF dumper
tests.

Patch 3 consists of selftests that utilize a dump printf function
to snprintf the dump output to a string for comparison with
expected output.  Tests deliberately mirror those in
snprintf_btf helper test to keep output consistent, but
also cover overflow handling, var/section display.

Changes since v5 [1]
 - readjust dump options to avoid unnecessary padding (Andrii, patch 1).
 - tidied up bitfield data checking/retrieval using Andrii's suggestions.
   Removed code where we adjust data pointer prior to calling bitfield
   functions as this adjustment is not needed, provided we use the type
   size as the number of bytes to iterate over when retrieving the
   full value we apply bit shifting operations to retrieve the bitfield
   value.  With these chances, the *_int_bits() functions were no longer needed
   (Andrii, patch 1).
 - coalesced the "is zero" checking for ints, floats and pointers
   into btf_dump_base_type_check_zero(), using a memcmp() of the
   size of the data.  This can be derived from t->size for ints
   and floats, and pointer size is retrieved from dump's ptr_sz
   field (Andrii, patch 1).
 - Added alignment-aware handling for int, enum, float retrieval.
   Packed data structures can force ints, enums and floats to be
   aligned on different boundaries; for example, the 

struct p {
        char f1;
        int f2;
} __attribute__((packed));

   ...will have the int f2 field offset at byte 1, rather than at
   byte 4 for an unpacked structure.  The problem is directly
   dereferencing that as an int is problematic on some platforms.
   For ints and enums, we can reuse bitfield retrieval to get the 
   value for display, while for floats we use a local union of the
   floating-point types and memcpy into it, ensuring we can then 
   dereference pointers into that union which will have safe alignment
   (Andrii, patch 1).
 - added comments to explain why we increment depth prior to displaying
   opening parens, and decrement it prior to displaying closing parens
   for structs, unions and arrays.  The reason is that we don't want
   to have a trailing newline when displaying a type.  The logic that
   handles this says "don't show a newline when the depth we're at is 0".
   For this to work for opening parens then we need to bump depth before
   showing opening parens + newline, and when we close out structure
   we need to show closing parens after reducing depth so that we don't
   append a newline to a top-level structure. So as a result we have

struct foo {\n
 struct bar {\n
 }\n
}

 - silently truncate provided indent string with strncat() if > 31 bytes
   (Andrii, patch 1).
 - fixed ASSERT_STRNEQ() macro to show only n bytes of string
   (Andrii, patch 2).
 - fixed strncat() of type data string to avoid stack corruption
   (Andrii, patch 3).
 - removed early returns from dump type tests (Andrii, patch 3).
 - have tests explicitly specify prefix (enum, struct, union)
   (Andrii, patch 3).
 - switch from CHECK() to ASSERT_* where possible (Andrii, patch 3).

Changes since v4 [2]
- Andrii kindly provided code to unify emitting a prepended cast
  (for example "(int)") with existing code, and this had the nice
  benefit of adding array indices in type specifications (Andrii,
  patches 1, 3)
- Fixed indent_str option to make it a const char *, stored in a
  fixed-length buffer internally (Andrii, patch 1)
- Reworked bit shift logic to minimize endian-specific interactions,
  and use same macros as found elsewhere in libbpf to determine endianness
  (Andrii, patch 1)
- Fixed type emitting to ensure that a trailing '\n' is not displayed;
  newlines are added during struct/array display, but for a single type
  the last character is no longer a newline (Andrii, patches 1, 3)
- Added support for ASSERT_STRNEQ() macro (Andrii, patch 2)
- Split tests into subtests for int, char, enum etc rather than one
  "dump type data" subtest (Andrii, patch 3)
- Made better use of ASSERT* macros (Andrii, patch 3)
- Got rid of some other TEST_* macros that were unneeded (Andrii, patch 3)
- Switched to using "struct fs_context" to verify enum bitfield values
  (Andrii, patch 3)

Changes since v3 [3]
- Retained separation of emitting of type name cast prefixing
  type values from existing functionality such as btf_dump_emit_type_chain()
  since initial code-shared version had so many exceptions it became
  hard to read.  For example, we don't emit a type name if the type
  to be displayed is an array member, we also always emit "forward"
  definitions for structs/unions that aren't really forward definitions
  (we just want a "struct foo" output for "(struct foo){.bar = ...".
  We also always ignore modifiers const/volatile/restrict as they
  clutter output when emitting large types.
- Added configurable 4-char indent string option; defaults to tab
  (Andrii)
- Added support for BTF_KIND_FLOAT and associated tests (Andrii)
- Added support for BTF_KIND_FUNC_PROTO function pointers to
  improve output of "ops" structures; for example:

(struct file_operations){
        .owner = (struct module *)0xffffffffffffffff,
        .llseek = (loff_t(*)(struct file *, loff_t, int))0xffffffffffffffff,
        ...
  Added associated test also (Andrii)
- Added handling for enum bitfields and associated test (Andrii)
- Allocation of "struct btf_dump_data" done on-demand (Andrii)
- Removed ".field = " output from function emitting type name and
  into caller (Andrii)
- Removed BTF_INT_OFFSET() support (Andrii)
- Use libbpf_err() to set errno for error cases (Andrii)
- btf_dump_dump_type_data() returns size written, which is used
  when returning successfully from btf_dump__dump_type_data()
  (Andrii)

Changes since v2 [4]
- Renamed function to btf_dump__dump_type_data, reorganized
  arguments such that opts are last (Andrii)
- Modified code to separate questions about display such
  as have we overflowed?/is this field zero? from actual
  display of typed data, such that we ask those questions
  separately from the code that actually displays typed data
  (Andrii)
- Reworked code to handle overflow - where we do not provide
  enough data for the type we wish to display - by returning
  -E2BIG and attempting to present as much data as possible.
  Such a mode of operation allows for tracers which retrieve
  partial data (such as first 1024 bytes of a
  "struct task_struct" say), and want to display that partial
  data, while also knowing that it is not the full type.
 Such tracers can then denote this (perhaps via "..." or
  similar).
- Explored reusing existing type emit functions, such as
  passing in a type id stack with a single type id to
  btf_dump_emit_type_chain() to support the display of
  typed data where a "cast" is prepended to the data to
  denote its type; "(int)1", "(struct foo){", etc.
  However the task of emitting a
  ".field_name = (typecast)" did not match well with model
  of walking the stack to display innermost types first
  and made the resultant code harder to read.  Added a
  dedicated btf_dump_emit_type_name() function instead which
  is only ~70 lines (Andrii)
- Various cleanups around bitfield macros, unneeded member
  iteration macros, avoiding compiler complaints when
  displaying int da ta by casting to long long, etc (Andrii)
- Use DECLARE_LIBBPF_OPTS() in defining opts for tests (Andrii)
- Added more type tests, overflow tests, var tests and
  section tests.

Changes since RFC [5]
- The initial approach explored was to share the kernel code
  with libbpf using #defines to paper over the different needs;
  however it makes more sense to try and fit in with libbpf
  code style for maintenance.  A comment in the code points at
  the implementation in kernel/bpf/btf.c and notes that any
  issues found in it should be fixed there or vice versa;
  mirroring the tests should help with this also
  (Andrii)

[1] https://lore.kernel.org/bpf/1624092968-5598-1-git-send-email-alan.maguire@oracle.com/
[2] https://lore.kernel.org/bpf/CAEf4BzYtbnphCkhz0epMKE4zWfvSOiMpu+-SXp9hadsrRApuZw@mail.gmail.com/T/
[3] https://lore.kernel.org/bpf/1622131170-8260-1-git-send-email-alan.maguire@oracle.com/
[4] https://lore.kernel.org/bpf/1610921764-7526-1-git-send-email-alan.maguire@oracle.com/
[5] https://lore.kernel.org/bpf/1610386373-24162-1-git-send-email-alan.maguire@oracle.com/

Alan Maguire (3):
  libbpf: BTF dumper support for typed data
  selftests/bpf: add ASSERT_STRNEQ() variant for test_progs
  selftests/bpf: add dump type data tests to btf dump tests

 tools/lib/bpf/btf.h                               |  19 +
 tools/lib/bpf/btf_dump.c                          | 819 +++++++++++++++++++++-
 tools/lib/bpf/libbpf.map                          |   1 +
 tools/testing/selftests/bpf/prog_tests/btf_dump.c | 600 ++++++++++++++++
 tools/testing/selftests/bpf/test_progs.h          |  12 +
 5 files changed, 1446 insertions(+), 5 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v6 bpf-next 1/3] libbpf: BTF dumper support for typed data
  2021-07-15 15:15 [PATCH v6 bpf-next 0/3] libbpf: BTF dumper support for typed data Alan Maguire
@ 2021-07-15 15:15 ` Alan Maguire
  2021-07-16  6:24   ` Andrii Nakryiko
                     ` (2 more replies)
  2021-07-15 15:15 ` [PATCH v6 bpf-next 2/3] selftests/bpf: add ASSERT_STRNEQ() variant for test_progs Alan Maguire
  2021-07-15 15:15 ` [PATCH v6 bpf-next 3/3] selftests/bpf: add dump type data tests to btf dump tests Alan Maguire
  2 siblings, 3 replies; 10+ messages in thread
From: Alan Maguire @ 2021-07-15 15:15 UTC (permalink / raw)
  To: ast, daniel, andrii
  Cc: kafai, songliubraving, yhs, john.fastabend, kpsingh, morbo,
	shuah, bpf, netdev, linux-kselftest, linux-kernel, Alan Maguire

Add a BTF dumper for typed data, so that the user can dump a typed
version of the data provided.

The API is

int btf_dump__dump_type_data(struct btf_dump *d, __u32 id,
                             void *data, size_t data_sz,
                             const struct btf_dump_type_data_opts *opts);

...where the id is the BTF id of the data pointed to by the "void *"
argument; for example the BTF id of "struct sk_buff" for a
"struct skb *" data pointer.  Options supported are

 - a starting indent level (indent_lvl)
 - a user-specified indent string which will be printed once per
   indent level; if NULL, tab is chosen but any string <= 32 chars
   can be provided.
 - a set of boolean options to control dump display, similar to those
   used for BPF helper bpf_snprintf_btf().  Options are
        - compact : omit newlines and other indentation
        - skip_names: omit member names
        - emit_zeroes: show zero-value members

Default output format is identical to that dumped by bpf_snprintf_btf(),
for example a "struct sk_buff" representation would look like this:

struct sk_buff){
	(union){
		(struct){
			.next = (struct sk_buff *)0xffffffffffffffff,
			.prev = (struct sk_buff *)0xffffffffffffffff,
		(union){
			.dev = (struct net_device *)0xffffffffffffffff,
			.dev_scratch = (long unsigned int)18446744073709551615,
		},
	},
...

If the data structure is larger than the *data_sz*
number of bytes that are available in *data*, as much
of the data as possible will be dumped and -E2BIG will
be returned.  This is useful as tracers will sometimes
not be able to capture all of the data associated with
a type; for example a "struct task_struct" is ~16k.
Being able to specify that only a subset is available is
important for such cases.  On success, the amount of data
dumped is returned.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 tools/lib/bpf/btf.h      |  19 ++
 tools/lib/bpf/btf_dump.c | 819 ++++++++++++++++++++++++++++++++++++++++++++++-
 tools/lib/bpf/libbpf.map |   1 +
 3 files changed, 834 insertions(+), 5 deletions(-)

diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index b54f1c3..374e9f1 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h
@@ -184,6 +184,25 @@ struct btf_dump_emit_type_decl_opts {
 btf_dump__emit_type_decl(struct btf_dump *d, __u32 id,
 			 const struct btf_dump_emit_type_decl_opts *opts);
 
+
+struct btf_dump_type_data_opts {
+	/* size of this struct, for forward/backward compatibility */
+	size_t sz;
+	const char *indent_str;
+	int indent_level;
+	/* below match "show" flags for bpf_show_snprintf() */
+	bool compact;		/* no newlines/indentation */
+	bool skip_names;	/* skip member/type names */
+	bool emit_zeroes;	/* show 0-valued fields */
+	size_t :0;
+};
+#define btf_dump_type_data_opts__last_field emit_zeroes
+
+LIBBPF_API int
+btf_dump__dump_type_data(struct btf_dump *d, __u32 id,
+			 const void *data, size_t data_sz,
+			 const struct btf_dump_type_data_opts *opts);
+
 /*
  * A set of helpers for easier BTF types handling
  */
diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
index 5dc6b517..929cf93 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -10,6 +10,8 @@
 #include <stddef.h>
 #include <stdlib.h>
 #include <string.h>
+#include <ctype.h>
+#include <endian.h>
 #include <errno.h>
 #include <linux/err.h>
 #include <linux/btf.h>
@@ -53,6 +55,26 @@ struct btf_dump_type_aux_state {
 	__u8 referenced: 1;
 };
 
+/* indent string length; one indent string is added for each indent level */
+#define BTF_DATA_INDENT_STR_LEN			32
+
+/*
+ * Common internal data for BTF type data dump operations.
+ */
+struct btf_dump_data {
+	const void *data_end;		/* end of valid data to show */
+	bool compact;
+	bool skip_names;
+	bool emit_zeroes;
+	__u8 indent_lvl;	/* base indent level */
+	char indent_str[BTF_DATA_INDENT_STR_LEN];
+	/* below are used during iteration */
+	int depth;
+	bool is_array_member;
+	bool is_array_terminated;
+	bool is_array_char;
+};
+
 struct btf_dump {
 	const struct btf *btf;
 	const struct btf_ext *btf_ext;
@@ -60,6 +82,7 @@ struct btf_dump {
 	struct btf_dump_opts opts;
 	int ptr_sz;
 	bool strip_mods;
+	bool skip_anon_defs;
 	int last_id;
 
 	/* per-type auxiliary state */
@@ -89,6 +112,10 @@ struct btf_dump {
 	 * name occurrences
 	 */
 	struct hashmap *ident_names;
+	/*
+	 * data for typed display; allocated if needed.
+	 */
+	struct btf_dump_data *typed_dump;
 };
 
 static size_t str_hash_fn(const void *key, void *ctx)
@@ -765,11 +792,11 @@ static void btf_dump_emit_type(struct btf_dump *d, __u32 id, __u32 cont_id)
 		break;
 	case BTF_KIND_FUNC_PROTO: {
 		const struct btf_param *p = btf_params(t);
-		__u16 vlen = btf_vlen(t);
+		__u16 n = btf_vlen(t);
 		int i;
 
 		btf_dump_emit_type(d, t->type, cont_id);
-		for (i = 0; i < vlen; i++, p++)
+		for (i = 0; i < n; i++, p++)
 			btf_dump_emit_type(d, p->type, cont_id);
 
 		break;
@@ -852,8 +879,9 @@ static void btf_dump_emit_bit_padding(const struct btf_dump *d,
 static void btf_dump_emit_struct_fwd(struct btf_dump *d, __u32 id,
 				     const struct btf_type *t)
 {
-	btf_dump_printf(d, "%s %s",
+	btf_dump_printf(d, "%s%s%s",
 			btf_is_struct(t) ? "struct" : "union",
+			t->name_off ? " " : "",
 			btf_dump_type_name(d, id));
 }
 
@@ -1259,7 +1287,7 @@ static void btf_dump_emit_type_chain(struct btf_dump *d,
 		case BTF_KIND_UNION:
 			btf_dump_emit_mods(d, decls);
 			/* inline anonymous struct/union */
-			if (t->name_off == 0)
+			if (t->name_off == 0 && !d->skip_anon_defs)
 				btf_dump_emit_struct_def(d, id, t, lvl);
 			else
 				btf_dump_emit_struct_fwd(d, id, t);
@@ -1267,7 +1295,7 @@ static void btf_dump_emit_type_chain(struct btf_dump *d,
 		case BTF_KIND_ENUM:
 			btf_dump_emit_mods(d, decls);
 			/* inline anonymous enum */
-			if (t->name_off == 0)
+			if (t->name_off == 0 && !d->skip_anon_defs)
 				btf_dump_emit_enum_def(d, id, t, lvl);
 			else
 				btf_dump_emit_enum_fwd(d, id, t);
@@ -1392,6 +1420,39 @@ static void btf_dump_emit_type_chain(struct btf_dump *d,
 	btf_dump_emit_name(d, fname, last_was_ptr);
 }
 
+/* show type name as (type_name) */
+static void btf_dump_emit_type_cast(struct btf_dump *d, __u32 id,
+				    bool top_level)
+{
+	const struct btf_type *t;
+
+	/* for array members, we don't bother emitting type name for each
+	 * member to avoid the redundancy of
+	 * .name = (char[4])[(char)'f',(char)'o',(char)'o',]
+	 */
+	if (d->typed_dump->is_array_member)
+		return;
+
+	/* avoid type name specification for variable/section; it will be done
+	 * for the associated variable value(s).
+	 */
+	t = btf__type_by_id(d->btf, id);
+	if (btf_is_var(t) || btf_is_datasec(t))
+		return;
+
+	if (top_level)
+		btf_dump_printf(d, "(");
+
+	d->skip_anon_defs = true;
+	d->strip_mods = true;
+	btf_dump_emit_type_decl(d, id, "", 0);
+	d->strip_mods = false;
+	d->skip_anon_defs = false;
+
+	if (top_level)
+		btf_dump_printf(d, ")");
+}
+
 /* return number of duplicates (occurrences) of a given name */
 static size_t btf_dump_name_dups(struct btf_dump *d, struct hashmap *name_map,
 				 const char *orig_name)
@@ -1442,3 +1503,751 @@ static const char *btf_dump_ident_name(struct btf_dump *d, __u32 id)
 {
 	return btf_dump_resolve_name(d, id, d->ident_names);
 }
+
+static int btf_dump_dump_type_data(struct btf_dump *d,
+				   const char *fname,
+				   const struct btf_type *t,
+				   __u32 id,
+				   const void *data,
+				   __u8 bits_offset,
+				   __u8 bit_sz);
+
+static const char *btf_dump_data_newline(struct btf_dump *d)
+{
+	return d->typed_dump->compact || d->typed_dump->depth == 0 ? "" : "\n";
+}
+
+static const char *btf_dump_data_delim(struct btf_dump *d)
+{
+	return d->typed_dump->depth == 0 ? "" : ",";
+}
+
+static void btf_dump_data_pfx(struct btf_dump *d)
+{
+	int i, lvl = d->typed_dump->indent_lvl + d->typed_dump->depth;
+
+	if (d->typed_dump->compact)
+		return;
+
+	for (i = 0; i < lvl; i++)
+		btf_dump_printf(d, "%s", d->typed_dump->indent_str);
+}
+
+/* A macro is used here as btf_type_value[s]() appends format specifiers
+ * to the format specifier passed in; these do the work of appending
+ * delimiters etc while the caller simply has to specify the type values
+ * in the format specifier + value(s).
+ */
+#define btf_dump_type_values(d, fmt, ...)				\
+	btf_dump_printf(d, fmt "%s%s",					\
+			##__VA_ARGS__,					\
+			btf_dump_data_delim(d),				\
+			btf_dump_data_newline(d))
+
+static int btf_dump_unsupported_data(struct btf_dump *d,
+				     const struct btf_type *t,
+				     __u32 id)
+{
+	btf_dump_printf(d, "<unsupported kind:%u>", btf_kind(t));
+	return -ENOTSUP;
+}
+
+static void btf_dump_int128(struct btf_dump *d,
+			    const struct btf_type *t,
+			    const void *data)
+{
+	__int128 num = *(__int128 *)data;
+
+	if ((num >> 64) == 0)
+		btf_dump_type_values(d, "0x%llx", (long long)num);
+	else
+		btf_dump_type_values(d, "0x%llx%016llx", (long long)num >> 32,
+				     (long long)num);
+}
+
+static unsigned __int128 btf_dump_bitfield_get_data(struct btf_dump *d,
+						    const struct btf_type *t,
+						    const void *data,
+						    __u8 bits_offset,
+						    __u8 bit_sz)
+{
+	__u16 left_shift_bits, right_shift_bits;
+	__u8 nr_copy_bits, nr_copy_bytes;
+	unsigned __int128 num = 0, ret;
+	const __u8 *bytes = data;
+	int i;
+
+	/* Bitfield value retrieval is done in two steps; first relevant bytes are
+	 * stored in num, then we left/right shift num to eliminate irrelevant bits.
+	 */
+	nr_copy_bits = bit_sz + bits_offset;
+	nr_copy_bytes = t->size;
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+	for (i = nr_copy_bytes - 1; i >= 0; i--)
+		num = num * 256 + bytes[i];
+#elif __BYTE_ORDER == __BIG_ENDIAN
+	for (i = 0; i < nr_copy_bytes; i++)
+		num = num * 256 + bytes[i];
+#else
+# error "Unrecognized __BYTE_ORDER__"
+#endif
+	left_shift_bits = 128 - nr_copy_bits;
+	right_shift_bits = 128 - bit_sz;
+
+	ret = (num << left_shift_bits) >> right_shift_bits;
+
+	return ret;
+}
+
+static int btf_dump_bitfield_check_zero(struct btf_dump *d,
+					const struct btf_type *t,
+					const void *data,
+					__u8 bits_offset,
+					__u8 bit_sz)
+{
+	__int128 check_num;
+
+	check_num = btf_dump_bitfield_get_data(d, t, data, bits_offset, bit_sz);
+	if (check_num == 0)
+		return -ENODATA;
+	return 0;
+}
+
+static int btf_dump_bitfield_data(struct btf_dump *d,
+				  const struct btf_type *t,
+				  const void *data,
+				  __u8 bits_offset,
+				  __u8 bit_sz)
+{
+	unsigned __int128 print_num;
+
+	print_num = btf_dump_bitfield_get_data(d, t, data, bits_offset, bit_sz);
+	btf_dump_int128(d, t, &print_num);
+
+	return 0;
+}
+
+/* ints, floats and ptrs */
+static int btf_dump_base_type_check_zero(struct btf_dump *d,
+					 const struct btf_type *t,
+					 __u32 id,
+					 const void *data)
+{
+	static __u8 bytecmp[16] = {};
+	int nr_bytes;
+
+	/* For pointer types, pointer size is not defined on a per-type basis.
+	 * On dump creation however, we store the pointer size.
+	 */
+	if (btf_kind(t) == BTF_KIND_PTR)
+		nr_bytes = d->ptr_sz;
+	else
+		nr_bytes = t->size;
+
+	if (nr_bytes < 1 || nr_bytes > 16) {
+		pr_warn("unexpected size %d for id [%u]\n", nr_bytes, id);
+		return -EINVAL;
+	}
+
+	if (memcmp(data, bytecmp, nr_bytes) == 0)
+		return -ENODATA;
+	return 0;
+}
+
+static int btf_dump_int_data(struct btf_dump *d,
+			     const struct btf_type *t,
+			     __u32 type_id,
+			     const void *data,
+			     __u8 bits_offset)
+{
+	__u8 encoding = btf_int_encoding(t);
+	bool sign = encoding & BTF_INT_SIGNED;
+	int sz = t->size;
+
+	if (sz == 0) {
+		pr_warn("unexpected size %d for id [%u]\n", sz, type_id);
+		return -EINVAL;
+	}
+
+	/* handle packed int data - accesses of integers not aligned on
+	 * int boundaries can cause problems on some platforms.
+	 */
+	if (((uintptr_t)data) % sz)
+		return btf_dump_bitfield_data(d, t, data, 0, 0);
+
+	switch (sz) {
+	case 16:
+		btf_dump_int128(d, t, data);
+		break;
+	case 8:
+		if (sign)
+			btf_dump_type_values(d, "%lld", *(long long *)data);
+		else
+			btf_dump_type_values(d, "%llu", *(unsigned long long *)data);
+		break;
+	case 4:
+		if (sign)
+			btf_dump_type_values(d, "%d", *(__s32 *)data);
+		else
+			btf_dump_type_values(d, "%u", *(__u32 *)data);
+		break;
+	case 2:
+		if (sign)
+			btf_dump_type_values(d, "%d", *(__s16 *)data);
+		else
+			btf_dump_type_values(d, "%u", *(__u16 *)data);
+		break;
+	case 1:
+		if (d->typed_dump->is_array_char) {
+			/* check for null terminator */
+			if (d->typed_dump->is_array_terminated)
+				break;
+			if (*(char *)data == '\0') {
+				d->typed_dump->is_array_terminated = true;
+				break;
+			}
+			if (isprint(*(char *)data)) {
+				btf_dump_type_values(d, "'%c'", *(char *)data);
+				break;
+			}
+		}
+		if (sign)
+			btf_dump_type_values(d, "%d", *(__s8 *)data);
+		else
+			btf_dump_type_values(d, "%u", *(__u8 *)data);
+		break;
+	default:
+		pr_warn("unexpected sz %d for id [%u]\n", sz, type_id);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+union float_data {
+	long double ld;
+	double d;
+	float f;
+};
+
+static int btf_dump_float_data(struct btf_dump *d,
+			       const struct btf_type *t,
+			       __u32 type_id,
+			       const void *data)
+{
+	const union float_data *flp = data;
+	union float_data fl;
+	int sz = t->size;
+
+	/* handle unaligned data; copy to local union */
+	if (((uintptr_t)data) % sz) {
+		memcpy(&fl, data, sz);
+		flp = &fl;
+	}
+
+	switch (sz) {
+	case 16:
+		btf_dump_type_values(d, "%Lf", flp->ld);
+		break;
+	case 8:
+		btf_dump_type_values(d, "%lf", flp->d);
+		break;
+	case 4:
+		btf_dump_type_values(d, "%f", flp->f);
+		break;
+	default:
+		pr_warn("unexpected size %d for id [%u]\n", sz, type_id);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int btf_dump_var_data(struct btf_dump *d,
+			     const struct btf_type *v,
+			     __u32 id,
+			     const void *data)
+{
+	enum btf_func_linkage linkage = btf_var(v)->linkage;
+	const struct btf_type *t;
+	const char *l;
+	__u32 type_id;
+
+	switch (linkage) {
+	case BTF_FUNC_STATIC:
+		l = "static ";
+		break;
+	case BTF_FUNC_EXTERN:
+		l = "extern ";
+		break;
+	case BTF_FUNC_GLOBAL:
+	default:
+		l = "";
+		break;
+	}
+
+	/* format of output here is [linkage] [type] [varname] = (type)value,
+	 * for example "static int cpu_profile_flip = (int)1"
+	 */
+	btf_dump_printf(d, "%s", l);
+	type_id = v->type;
+	t = btf__type_by_id(d->btf, type_id);
+	btf_dump_emit_type_cast(d, type_id, false);
+	btf_dump_printf(d, " %s = ", btf_name_of(d, v->name_off));
+	return btf_dump_dump_type_data(d, NULL, t, type_id, data, 0, 0);
+}
+
+static int btf_dump_array_data(struct btf_dump *d,
+			       const struct btf_type *t,
+			       __u32 id,
+			       const void *data)
+{
+	const struct btf_array *array = btf_array(t);
+	const struct btf_type *elem_type;
+	__u32 i, elem_size = 0, elem_type_id;
+	bool is_array_member;
+
+	elem_type_id = array->type;
+	elem_type = skip_mods_and_typedefs(d->btf, elem_type_id, NULL);
+	elem_size = btf__resolve_size(d->btf, elem_type_id);
+	if (elem_size <= 0) {
+		pr_warn("unexpected elem size %d for array type [%u]\n", elem_size, id);
+		return -EINVAL;
+	}
+
+	if (btf_is_int(elem_type)) {
+		/*
+		 * BTF_INT_CHAR encoding never seems to be set for
+		 * char arrays, so if size is 1 and element is
+		 * printable as a char, we'll do that.
+		 */
+		if (elem_size == 1)
+			d->typed_dump->is_array_char = true;
+	}
+
+	/* note that we increment depth before calling btf_dump_print() below;
+	 * this is intentional.  btf_dump_data_newline() will not print a
+	 * newline for depth 0 (since this leaves us with trailing newlines
+	 * at the end of typed display), so depth is incremented first.
+	 * For similar reasons, we decrement depth before showing the closing
+	 * parenthesis.
+	 */
+	d->typed_dump->depth++;
+	btf_dump_printf(d, "[%s", btf_dump_data_newline(d));
+
+	/* may be a multidimensional array, so store current "is array member"
+	 * status so we can restore it correctly later.
+	 */
+	is_array_member = d->typed_dump->is_array_member;
+	d->typed_dump->is_array_member = true;
+	for (i = 0; i < array->nelems; i++, data += elem_size) {
+		if (d->typed_dump->is_array_terminated)
+			break;
+		btf_dump_dump_type_data(d, NULL, elem_type, elem_type_id, data, 0, 0);
+	}
+	d->typed_dump->is_array_member = is_array_member;
+	d->typed_dump->depth--;
+	btf_dump_data_pfx(d);
+	btf_dump_type_values(d, "]");
+
+	return 0;
+}
+
+static int btf_dump_struct_data(struct btf_dump *d,
+				const struct btf_type *t,
+				__u32 id,
+				const void *data)
+{
+	const struct btf_member *m = btf_members(t);
+	__u16 n = btf_vlen(t);
+	int i, err;
+
+	/* note that we increment depth before calling btf_dump_print() below;
+	 * this is intentional.  btf_dump_data_newline() will not print a
+	 * newline for depth 0 (since this leaves us with trailing newlines
+	 * at the end of typed display), so depth is incremented first.
+	 * For similar reasons, we decrement depth before showing the closing
+	 * parenthesis.
+	 */
+	d->typed_dump->depth++;
+	btf_dump_printf(d, "{%s", btf_dump_data_newline(d));
+
+	for (i = 0; i < n; i++, m++) {
+		const struct btf_type *mtype;
+		const char *mname;
+		__u32 moffset;
+		__u8 bit_sz;
+
+		mtype = btf__type_by_id(d->btf, m->type);
+		mname = btf_name_of(d, m->name_off);
+		moffset = btf_member_bit_offset(t, i);
+
+		bit_sz = btf_member_bitfield_size(t, i);
+		err = btf_dump_dump_type_data(d, mname, mtype, m->type, data + moffset / 8,
+					      moffset % 8, bit_sz);
+		if (err < 0)
+			return err;
+	}
+	d->typed_dump->depth--;
+	btf_dump_data_pfx(d);
+	btf_dump_type_values(d, "}");
+	return err;
+}
+
+static int btf_dump_ptr_data(struct btf_dump *d,
+			      const struct btf_type *t,
+			      __u32 id,
+			      const void *data)
+{
+	btf_dump_type_values(d, "%p", *(void **)data);
+	return 0;
+}
+
+static int btf_dump_get_enum_value(struct btf_dump *d,
+				   const struct btf_type *t,
+				   const void *data,
+				   __u32 id,
+				   __s64 *value)
+{
+	int sz = t->size;
+
+	/* handle unaligned enum value */
+	if (((uintptr_t)data) % sz) {
+		*value = (__s64)btf_dump_bitfield_get_data(d, t, data, 0, 0);
+		return 0;
+	}
+	switch (t->size) {
+	case 8:
+		*value = *(__s64 *)data;
+		return 0;
+	case 4:
+		*value = *(__s32 *)data;
+		return 0;
+	case 2:
+		*value = *(__s16 *)data;
+		return 0;
+	case 1:
+		*value = *(__s8 *)data;
+		return 0;
+	default:
+		pr_warn("unexpected size %d for enum, id:[%u]\n", t->size, id);
+		return -EINVAL;
+	}
+}
+
+static int btf_dump_enum_data(struct btf_dump *d,
+			      const struct btf_type *t,
+			      __u32 id,
+			      const void *data)
+{
+	const struct btf_enum *e;
+	__s64 value;
+	int i, err;
+
+	err = btf_dump_get_enum_value(d, t, data, id, &value);
+	if (err)
+		return err;
+
+	for (i = 0, e = btf_enum(t); i < btf_vlen(t); i++, e++) {
+		if (value != e->val)
+			continue;
+		btf_dump_type_values(d, "%s", btf_name_of(d, e->name_off));
+		return 0;
+	}
+
+	btf_dump_type_values(d, "%d", value);
+	return 0;
+}
+
+static int btf_dump_datasec_data(struct btf_dump *d,
+				 const struct btf_type *t,
+				 __u32 id,
+				 const void *data)
+{
+	const struct btf_var_secinfo *vsi;
+	const struct btf_type *var;
+	__u32 i;
+	int err;
+
+	btf_dump_type_values(d, "SEC(\"%s\") ", btf_name_of(d, t->name_off));
+
+	for (i = 0, vsi = btf_var_secinfos(t); i < btf_vlen(t); i++, vsi++) {
+		var = btf__type_by_id(d->btf, vsi->type);
+		err = btf_dump_dump_type_data(d, NULL, var, vsi->type, data + vsi->offset, 0, 0);
+		if (err < 0)
+			return err;
+		btf_dump_printf(d, ";");
+	}
+	return 0;
+}
+
+/* return size of type, or if base type overflows, return -E2BIG. */
+static int btf_dump_type_data_check_overflow(struct btf_dump *d,
+					     const struct btf_type *t,
+					     __u32 id,
+					     const void *data,
+					     __u8 bits_offset)
+{
+	__s64 size = btf__resolve_size(d->btf, id);
+
+	if (size < 0 || size >= INT_MAX) {
+		pr_warn("unexpected size [%lld] for id [%u]\n",
+			size, id);
+		return -EINVAL;
+	}
+
+	/* Only do overflow checking for base types; we do not want to
+	 * avoid showing part of a struct, union or array, even if we
+	 * do not have enough data to show the full object.  By
+	 * restricting overflow checking to base types we can ensure
+	 * that partial display succeeds, while avoiding overflowing
+	 * and using bogus data for display.
+	 */
+	t = skip_mods_and_typedefs(d->btf, id, NULL);
+	if (!t) {
+		pr_warn("unexpected error skipping mods/typedefs for id [%u]\n",
+			id);
+		return -EINVAL;
+	}
+
+	switch (btf_kind(t)) {
+	case BTF_KIND_INT:
+	case BTF_KIND_FLOAT:
+	case BTF_KIND_PTR:
+	case BTF_KIND_ENUM:
+		if (data + bits_offset / 8 + size > d->typed_dump->data_end)
+			return -E2BIG;
+		break;
+	default:
+		break;
+	}
+	return (int)size;
+}
+
+static int btf_dump_type_data_check_zero(struct btf_dump *d,
+					 const struct btf_type *t,
+					 __u32 id,
+					 const void *data,
+					 __u8 bits_offset,
+					 __u8 bit_sz)
+{
+	__s64 value;
+	int i, err;
+
+	/* toplevel exceptions; we show zero values if
+	 * - we ask for them (emit_zeros)
+	 * - if we are at top-level so we see "struct empty { }"
+	 * - or if we are an array member and the array is non-empty and
+	 *   not a char array; we don't want to be in a situation where we
+	 *   have an integer array 0, 1, 0, 1 and only show non-zero values.
+	 *   If the array contains zeroes only, or is a char array starting
+	 *   with a '\0', the array-level check_zero() will prevent showing it;
+	 *   we are concerned with determining zero value at the array member
+	 *   level here.
+	 */
+	if (d->typed_dump->emit_zeroes || d->typed_dump->depth == 0 ||
+	    (d->typed_dump->is_array_member &&
+	     !d->typed_dump->is_array_char))
+		return 0;
+
+	t = skip_mods_and_typedefs(d->btf, id, NULL);
+
+	switch (btf_kind(t)) {
+	case BTF_KIND_INT:
+		if (bit_sz)
+			return btf_dump_bitfield_check_zero(d, t, data, bits_offset, bit_sz);
+		return btf_dump_base_type_check_zero(d, t, id, data);
+	case BTF_KIND_FLOAT:
+	case BTF_KIND_PTR:
+		return btf_dump_base_type_check_zero(d, t, id, data);
+	case BTF_KIND_ARRAY: {
+		const struct btf_array *array = btf_array(t);
+		const struct btf_type *elem_type;
+		__u32 elem_type_id, elem_size;
+		bool ischar;
+
+		elem_type_id = array->type;
+		elem_size = btf__resolve_size(d->btf, elem_type_id);
+		elem_type = skip_mods_and_typedefs(d->btf, elem_type_id, NULL);
+
+		ischar = btf_is_int(elem_type) && elem_size == 1;
+
+		/* check all elements; if _any_ element is nonzero, all
+		 * of array is displayed.  We make an exception however
+		 * for char arrays where the first element is 0; these
+		 * are considered zeroed also, even if later elements are
+		 * non-zero because the string is terminated.
+		 */
+		for (i = 0; i < array->nelems; i++) {
+			if (i == 0 && ischar && *(char *)data == 0)
+				return -ENODATA;
+			err = btf_dump_type_data_check_zero(d, elem_type,
+							    elem_type_id,
+							    data +
+							    (i * elem_size),
+							    bits_offset, 0);
+			if (err != -ENODATA)
+				return err;
+		}
+		return -ENODATA;
+	}
+	case BTF_KIND_STRUCT:
+	case BTF_KIND_UNION: {
+		const struct btf_member *m = btf_members(t);
+		__u16 n = btf_vlen(t);
+
+		/* if any struct/union member is non-zero, the struct/union
+		 * is considered non-zero and dumped.
+		 */
+		for (i = 0; i < n; i++, m++) {
+			const struct btf_type *mtype;
+			__u32 moffset;
+
+			mtype = btf__type_by_id(d->btf, m->type);
+			moffset = btf_member_bit_offset(t, i);
+
+			/* btf_int_bits() does not store member bitfield size;
+			 * bitfield size needs to be stored here so int display
+			 * of member can retrieve it.
+			 */
+			bit_sz = btf_member_bitfield_size(t, i);
+			err = btf_dump_type_data_check_zero(d, mtype, m->type, data + moffset / 8,
+							    moffset % 8, bit_sz);
+			if (err != ENODATA)
+				return err;
+		}
+		return -ENODATA;
+	}
+	case BTF_KIND_ENUM:
+		if (btf_dump_get_enum_value(d, t, data, id, &value))
+			return 0;
+		if (value == 0)
+			return -ENODATA;
+		return 0;
+	default:
+		return 0;
+	}
+}
+
+/* returns size of data dumped, or error. */
+static int btf_dump_dump_type_data(struct btf_dump *d,
+				   const char *fname,
+				   const struct btf_type *t,
+				   __u32 id,
+				   const void *data,
+				   __u8 bits_offset,
+				   __u8 bit_sz)
+{
+	int size, err;
+
+	size = btf_dump_type_data_check_overflow(d, t, id, data, bits_offset);
+	if (size < 0)
+		return size;
+	err = btf_dump_type_data_check_zero(d, t, id, data, bits_offset, bit_sz);
+	if (err) {
+		/* zeroed data is expected and not an error, so simply skip
+		 * dumping such data.  Record other errors however.
+		 */
+		if (err == -ENODATA)
+			return size;
+		return err;
+	}
+	btf_dump_data_pfx(d);
+
+	if (!d->typed_dump->skip_names) {
+		if (fname && strlen(fname) > 0)
+			btf_dump_printf(d, ".%s = ", fname);
+		btf_dump_emit_type_cast(d, id, true);
+	}
+
+	t = skip_mods_and_typedefs(d->btf, id, NULL);
+
+	switch (btf_kind(t)) {
+	case BTF_KIND_UNKN:
+	case BTF_KIND_FWD:
+	case BTF_KIND_FUNC:
+	case BTF_KIND_FUNC_PROTO:
+		err = btf_dump_unsupported_data(d, t, id);
+		break;
+	case BTF_KIND_INT:
+		if (bit_sz)
+			err = btf_dump_bitfield_data(d, t, data, bits_offset, bit_sz);
+		else
+			err = btf_dump_int_data(d, t, id, data, bits_offset);
+		break;
+	case BTF_KIND_FLOAT:
+		err = btf_dump_float_data(d, t, id, data);
+		break;
+	case BTF_KIND_PTR:
+		err = btf_dump_ptr_data(d, t, id, data);
+		break;
+	case BTF_KIND_ARRAY:
+		err = btf_dump_array_data(d, t, id, data);
+		break;
+	case BTF_KIND_STRUCT:
+	case BTF_KIND_UNION:
+		err = btf_dump_struct_data(d, t, id, data);
+		break;
+	case BTF_KIND_ENUM:
+		/* handle bitfield and int enum values */
+		if (bit_sz) {
+			unsigned __int128 print_num;
+			__s64 enum_val;
+
+			print_num = btf_dump_bitfield_get_data(d, t, data, bits_offset, bit_sz);
+			enum_val = (__s64)print_num;
+			err = btf_dump_enum_data(d, t, id, &enum_val);
+		} else
+			err = btf_dump_enum_data(d, t, id, data);
+		break;
+	case BTF_KIND_VAR:
+		err = btf_dump_var_data(d, t, id, data);
+		break;
+	case BTF_KIND_DATASEC:
+		err = btf_dump_datasec_data(d, t, id, data);
+		break;
+	default:
+		pr_warn("unexpected kind [%u] for id [%u]\n",
+			BTF_INFO_KIND(t->info), id);
+		return -EINVAL;
+	}
+	if (err < 0)
+		return err;
+	return size;
+}
+
+int btf_dump__dump_type_data(struct btf_dump *d, __u32 id,
+			     const void *data, size_t data_sz,
+			     const struct btf_dump_type_data_opts *opts)
+{
+	const struct btf_type *t;
+	int ret;
+
+	if (!OPTS_VALID(opts, btf_dump_type_data_opts))
+		return libbpf_err(-EINVAL);
+
+	t = btf__type_by_id(d->btf, id);
+	if (!t)
+		return libbpf_err(-ENOENT);
+
+	d->typed_dump = calloc(1, sizeof(struct btf_dump_data));
+	if (!d->typed_dump)
+		return libbpf_err(-ENOMEM);
+
+	d->typed_dump->data_end = data + data_sz;
+	d->typed_dump->indent_lvl = OPTS_GET(opts, indent_level, 0);
+	/* default indent string is a tab */
+	if (!opts->indent_str)
+		d->typed_dump->indent_str[0] = '\t';
+	else
+		strncat(d->typed_dump->indent_str, opts->indent_str,
+			sizeof(d->typed_dump->indent_str) - 1);
+
+	d->typed_dump->compact = OPTS_GET(opts, compact, false);
+	d->typed_dump->skip_names = OPTS_GET(opts, skip_names, false);
+	d->typed_dump->emit_zeroes = OPTS_GET(opts, emit_zeroes, false);
+
+	ret = btf_dump_dump_type_data(d, NULL, t, id, data, 0, 0);
+
+	free(d->typed_dump);
+
+	return libbpf_err(ret);
+}
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 944c99d..5bfc107 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -373,5 +373,6 @@ LIBBPF_0.5.0 {
 		bpf_map__initial_value;
 		bpf_map_lookup_and_delete_elem_flags;
 		bpf_object__gen_loader;
+		btf_dump__dump_type_data;
 		libbpf_set_strict_mode;
 } LIBBPF_0.4.0;
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v6 bpf-next 2/3] selftests/bpf: add ASSERT_STRNEQ() variant for test_progs
  2021-07-15 15:15 [PATCH v6 bpf-next 0/3] libbpf: BTF dumper support for typed data Alan Maguire
  2021-07-15 15:15 ` [PATCH v6 bpf-next 1/3] " Alan Maguire
@ 2021-07-15 15:15 ` Alan Maguire
  2021-07-15 15:15 ` [PATCH v6 bpf-next 3/3] selftests/bpf: add dump type data tests to btf dump tests Alan Maguire
  2 siblings, 0 replies; 10+ messages in thread
From: Alan Maguire @ 2021-07-15 15:15 UTC (permalink / raw)
  To: ast, daniel, andrii
  Cc: kafai, songliubraving, yhs, john.fastabend, kpsingh, morbo,
	shuah, bpf, netdev, linux-kselftest, linux-kernel, Alan Maguire

It will support strncmp()-style string comparisons.

Suggested-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 tools/testing/selftests/bpf/test_progs.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index 8ef7f33..c8c2bf8 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -221,6 +221,18 @@ struct test_env {
 	___ok;								\
 })
 
+#define ASSERT_STRNEQ(actual, expected, len, name) ({			\
+	static int duration = 0;					\
+	const char *___act = actual;					\
+	const char *___exp = expected;					\
+	int ___len = len;						\
+	bool ___ok = strncmp(___act, ___exp, ___len) == 0;		\
+	CHECK(!___ok, (name),						\
+	      "unexpected %s: actual '%.*s' != expected '%.*s'\n",	\
+	      (name), ___len, ___act, ___len, ___exp);			\
+	___ok;								\
+})
+
 #define ASSERT_OK(res, name) ({						\
 	static int duration = 0;					\
 	long long ___res = (res);					\
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v6 bpf-next 3/3] selftests/bpf: add dump type data tests to btf dump tests
  2021-07-15 15:15 [PATCH v6 bpf-next 0/3] libbpf: BTF dumper support for typed data Alan Maguire
  2021-07-15 15:15 ` [PATCH v6 bpf-next 1/3] " Alan Maguire
  2021-07-15 15:15 ` [PATCH v6 bpf-next 2/3] selftests/bpf: add ASSERT_STRNEQ() variant for test_progs Alan Maguire
@ 2021-07-15 15:15 ` Alan Maguire
  2021-07-16  6:24   ` Andrii Nakryiko
  2 siblings, 1 reply; 10+ messages in thread
From: Alan Maguire @ 2021-07-15 15:15 UTC (permalink / raw)
  To: ast, daniel, andrii
  Cc: kafai, songliubraving, yhs, john.fastabend, kpsingh, morbo,
	shuah, bpf, netdev, linux-kselftest, linux-kernel, Alan Maguire

Test various type data dumping operations by comparing expected
format with the dumped string; an snprintf-style printf function
is used to record the string dumped.  Also verify overflow handling
where the data passed does not cover the full size of a type,
such as would occur if a tracer has a portion of the 8k
"struct task_struct".

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

diff --git a/tools/testing/selftests/bpf/prog_tests/btf_dump.c b/tools/testing/selftests/bpf/prog_tests/btf_dump.c
index 1b90e68..2bfa47e 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf_dump.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf_dump.c
@@ -232,7 +232,577 @@ void test_btf_dump_incremental(void)
 	btf__free(btf);
 }
 
+#define STRSIZE				4096
+
+static void btf_dump_snprintf(void *ctx, const char *fmt, va_list args)
+{
+	char *s = ctx, new[STRSIZE];
+
+	vsnprintf(new, STRSIZE, fmt, args);
+	if (strlen(s) < STRSIZE)
+		strncat(s, new, STRSIZE - strlen(s) - 1);
+}
+
+static int btf_dump_data(struct btf *btf, struct btf_dump *d,
+			 char *name, char *prefix, __u64 flags, void *ptr,
+			 size_t ptr_sz, char *str, const char *expected_val)
+{
+	DECLARE_LIBBPF_OPTS(btf_dump_type_data_opts, opts);
+	size_t type_sz;
+	__s32 type_id;
+	int ret = 0;
+
+	if (flags & BTF_F_COMPACT)
+		opts.compact = true;
+	if (flags & BTF_F_NONAME)
+		opts.skip_names = true;
+	if (flags & BTF_F_ZERO)
+		opts.emit_zeroes = true;
+	if (prefix) {
+		ASSERT_STRNEQ(name, prefix, strlen(prefix),
+			      "verify prefix match");
+		name += strlen(prefix) + 1;
+	}
+	type_id = btf__find_by_name(btf, name);
+	if (!ASSERT_GE(type_id, 0, "find type id"))
+		return -ENOENT;
+	type_sz = btf__resolve_size(btf, type_id);
+	str[0] = '\0';
+	ret = btf_dump__dump_type_data(d, type_id, ptr, ptr_sz, &opts);
+	if (type_sz <= ptr_sz) {
+		if (!ASSERT_EQ(ret, type_sz, "failed/unexpected type_sz"))
+			return -EINVAL;
+	} else {
+		if (!ASSERT_EQ(ret, -E2BIG, "failed to return -E2BIG"))
+			return -EINVAL;
+	}
+	if (!ASSERT_STREQ(str, expected_val, "ensure expected/actual match"))
+		return -EFAULT;
+	return 0;
+}
+
+#define TEST_BTF_DUMP_DATA(_b, _d, _prefix, _str, _type, _flags,	\
+			   _expected, ...)				\
+	do {								\
+		char __ptrtype[64] = #_type;				\
+		char *_ptrtype = (char *)__ptrtype;			\
+		_type _ptrdata = __VA_ARGS__;				\
+		void *_ptr = &_ptrdata;					\
+									\
+		(void) btf_dump_data(_b, _d, _ptrtype, _prefix, _flags,	\
+				     _ptr, sizeof(_type), _str,		\
+				     _expected);			\
+	} while (0)
+
+/* Use where expected data string matches its stringified declaration */
+#define TEST_BTF_DUMP_DATA_C(_b, _d, _prefix,  _str, _type, _flags,	\
+			     ...)					\
+	TEST_BTF_DUMP_DATA(_b, _d, _prefix, _str, _type, _flags,	\
+			   "(" #_type ")" #__VA_ARGS__,	__VA_ARGS__)
+
+/* overflow test; pass typesize < expected type size, ensure E2BIG returned */
+#define TEST_BTF_DUMP_DATA_OVER(_b, _d, _prefix, _str, _type, _type_sz,	\
+				_expected, ...)				\
+	do {								\
+		char __ptrtype[64] = #_type;				\
+		char *_ptrtype = (char *)__ptrtype;			\
+		_type _ptrdata = __VA_ARGS__;				\
+		void *_ptr = &_ptrdata;					\
+									\
+		(void) btf_dump_data(_b, _d, _ptrtype, _prefix, 0,	\
+				     _ptr, _type_sz, _str, _expected);	\
+	} while (0)
+
+#define TEST_BTF_DUMP_VAR(_b, _d, _prefix, _str, _var, _type, _flags,	\
+			  _expected, ...)				\
+	do {								\
+		_type _ptrdata = __VA_ARGS__;				\
+		void *_ptr = &_ptrdata;					\
+									\
+		(void) btf_dump_data(_b, _d, _var, _prefix, _flags,	\
+				     _ptr, sizeof(_type), _str,		\
+				     _expected);			\
+	} while (0)
+
+static void test_btf_dump_int_data(struct btf *btf, struct btf_dump *d,
+				   char *str)
+{
+	/* simple int */
+	TEST_BTF_DUMP_DATA_C(btf, d, NULL, str, int, BTF_F_COMPACT, 1234);
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, int, BTF_F_COMPACT | BTF_F_NONAME,
+			   "1234", 1234);
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, int, 0, "(int)1234", 1234);
+
+	/* zero value should be printed at toplevel */
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, int, BTF_F_COMPACT, "(int)0", 0);
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, int, BTF_F_COMPACT | BTF_F_NONAME,
+			   "0", 0);
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, int, BTF_F_COMPACT | BTF_F_ZERO,
+			   "(int)0", 0);
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, int,
+			   BTF_F_COMPACT | BTF_F_NONAME | BTF_F_ZERO,
+			   "0", 0);
+	TEST_BTF_DUMP_DATA_C(btf, d, NULL, str, int, BTF_F_COMPACT, -4567);
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, int, BTF_F_COMPACT | BTF_F_NONAME,
+			   "-4567", -4567);
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, int, 0, "(int)-4567", -4567);
+
+	TEST_BTF_DUMP_DATA_OVER(btf, d, NULL, str, int, sizeof(int)-1, "", 1);
+}
+
+static void test_btf_dump_float_data(struct btf *btf, struct btf_dump *d,
+				     char *str)
+{
+	float t1 = 1.234567;
+	float t2 = -1.234567;
+	float t3 = 0.0;
+	double t4 = 5.678912;
+	double t5 = -5.678912;
+	double t6 = 0.0;
+	long double t7 = 9.876543;
+	long double t8 = -9.876543;
+	long double t9 = 0.0;
+
+	/* since the kernel does not likely have any float types in its BTF, we
+	 * will need to add some of various sizes.
+	 */
+
+	ASSERT_GT(btf__add_float(btf, "test_float", 4), 0, "add float");
+	ASSERT_OK(btf_dump_data(btf, d, "test_float", NULL, 0, &t1, 4, str,
+				"(test_float)1.234567"), "dump float");
+	ASSERT_OK(btf_dump_data(btf, d, "test_float", NULL, 0, &t2, 4, str,
+				"(test_float)-1.234567"), "dump float");
+	ASSERT_OK(btf_dump_data(btf, d, "test_float", NULL, 0, &t3, 4, str,
+				"(test_float)0.000000"), "dump float");
+
+	ASSERT_GT(btf__add_float(btf, "test_double", 8), 0, "add_double");
+	ASSERT_OK(btf_dump_data(btf, d, "test_double", NULL, 0, &t4, 8, str,
+		  "(test_double)5.678912"), "dump double");
+	ASSERT_OK(btf_dump_data(btf, d, "test_double", NULL, 0, &t5, 8, str,
+		  "(test_double)-5.678912"), "dump double");
+	ASSERT_OK(btf_dump_data(btf, d, "test_double", NULL, 0, &t6, 8, str,
+				"(test_double)0.000000"), "dump double");
+
+	ASSERT_GT(btf__add_float(btf, "test_long_double", 16), 0, "add long double");
+	ASSERT_OK(btf_dump_data(btf, d, "test_long_double", NULL, 0, &t7, 16,
+				str, "(test_long_double)9.876543"),
+				"dump long_double");
+	ASSERT_OK(btf_dump_data(btf, d, "test_long_double", NULL, 0, &t8, 16,
+				str, "(test_long_double)-9.876543"),
+				"dump long_double");
+	ASSERT_OK(btf_dump_data(btf, d, "test_long_double", NULL, 0, &t9, 16,
+				str, "(test_long_double)0.000000"),
+				"dump long_double");
+}
+
+static void test_btf_dump_char_data(struct btf *btf, struct btf_dump *d,
+				    char *str)
+{
+	/* simple char */
+	TEST_BTF_DUMP_DATA_C(btf, d, NULL, str, char, BTF_F_COMPACT, 100);
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, char, BTF_F_COMPACT | BTF_F_NONAME,
+			   "100", 100);
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, char, 0, "(char)100", 100);
+	/* zero value should be printed at toplevel */
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, char, BTF_F_COMPACT,
+			   "(char)0", 0);
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, char, BTF_F_COMPACT | BTF_F_NONAME,
+			   "0", 0);
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, char, BTF_F_COMPACT | BTF_F_ZERO,
+			   "(char)0", 0);
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, char, BTF_F_COMPACT | BTF_F_NONAME | BTF_F_ZERO,
+			   "0", 0);
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, char, 0, "(char)0", 0);
+
+	TEST_BTF_DUMP_DATA_OVER(btf, d, NULL, str, char, sizeof(char)-1, "", 100);
+}
+
+static void test_btf_dump_typedef_data(struct btf *btf, struct btf_dump *d,
+				       char *str)
+{
+	/* simple typedef */
+	TEST_BTF_DUMP_DATA_C(btf, d, NULL, str, uint64_t, BTF_F_COMPACT, 100);
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, u64, BTF_F_COMPACT | BTF_F_NONAME,
+			   "1", 1);
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, u64, 0, "(u64)1", 1);
+	/* zero value should be printed at toplevel */
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, u64, BTF_F_COMPACT, "(u64)0", 0);
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, u64, BTF_F_COMPACT | BTF_F_NONAME,
+			   "0", 0);
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, u64, BTF_F_COMPACT | BTF_F_ZERO,
+			   "(u64)0", 0);
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, u64,
+			   BTF_F_COMPACT | BTF_F_NONAME | BTF_F_ZERO,
+			   "0", 0);
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, u64, 0, "(u64)0", 0);
+
+	/* typedef struct */
+	TEST_BTF_DUMP_DATA_C(btf, d, NULL, str, atomic_t, BTF_F_COMPACT,
+			     {.counter = (int)1,});
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, atomic_t, BTF_F_COMPACT | BTF_F_NONAME,
+			   "{1,}", { .counter = 1 });
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, atomic_t, 0,
+"(atomic_t){\n"
+"	.counter = (int)1,\n"
+"}",
+			   {.counter = 1,});
+	/* typedef with 0 value should be printed at toplevel */
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, atomic_t, BTF_F_COMPACT, "(atomic_t){}",
+			   {.counter = 0,});
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, atomic_t, BTF_F_COMPACT | BTF_F_NONAME,
+			   "{}", {.counter = 0,});
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, atomic_t, 0,
+"(atomic_t){\n"
+"}",
+			   {.counter = 0,});
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, atomic_t, BTF_F_COMPACT | BTF_F_ZERO,
+			   "(atomic_t){.counter = (int)0,}",
+			   {.counter = 0,});
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, atomic_t,
+			   BTF_F_COMPACT | BTF_F_NONAME | BTF_F_ZERO,
+			   "{0,}", {.counter = 0,});
+	TEST_BTF_DUMP_DATA(btf, d, NULL, str, atomic_t, BTF_F_ZERO,
+"(atomic_t){\n"
+"	.counter = (int)0,\n"
+"}",
+			   { .counter = 0,});
+
+	/* overflow should show type but not value since it overflows */
+	TEST_BTF_DUMP_DATA_OVER(btf, d, NULL, str, atomic_t, sizeof(atomic_t)-1,
+				"(atomic_t){\n", { .counter = 1});
+}
+
+static void test_btf_dump_enum_data(struct btf *btf, struct btf_dump *d,
+				    char *str)
+{
+	/* enum where enum value does (and does not) exist */
+	TEST_BTF_DUMP_DATA_C(btf, d, "enum", str, enum bpf_cmd, BTF_F_COMPACT,
+			     BPF_MAP_CREATE);
+	TEST_BTF_DUMP_DATA(btf, d, "enum", str, enum bpf_cmd, BTF_F_COMPACT,
+			   "(enum bpf_cmd)BPF_MAP_CREATE", 0);
+	TEST_BTF_DUMP_DATA(btf, d, "enum", str, enum bpf_cmd,
+			   BTF_F_COMPACT | BTF_F_NONAME,
+			   "BPF_MAP_CREATE",
+			   BPF_MAP_CREATE);
+	TEST_BTF_DUMP_DATA(btf, d, "enum", str, enum bpf_cmd, 0,
+			   "(enum bpf_cmd)BPF_MAP_CREATE",
+			   BPF_MAP_CREATE);
+	TEST_BTF_DUMP_DATA(btf, d, "enum", str, enum bpf_cmd,
+			   BTF_F_COMPACT | BTF_F_NONAME | BTF_F_ZERO,
+			   "BPF_MAP_CREATE", 0);
+	TEST_BTF_DUMP_DATA(btf, d, "enum", str, enum bpf_cmd,
+			   BTF_F_COMPACT | BTF_F_ZERO,
+			   "(enum bpf_cmd)BPF_MAP_CREATE",
+			   BPF_MAP_CREATE);
+	TEST_BTF_DUMP_DATA(btf, d, "enum", str, enum bpf_cmd,
+			   BTF_F_COMPACT | BTF_F_NONAME | BTF_F_ZERO,
+			   "BPF_MAP_CREATE", BPF_MAP_CREATE);
+	TEST_BTF_DUMP_DATA_C(btf, d, "enum", str, enum bpf_cmd, BTF_F_COMPACT, 2000);
+	TEST_BTF_DUMP_DATA(btf, d, "enum", str, enum bpf_cmd,
+			   BTF_F_COMPACT | BTF_F_NONAME,
+			   "2000", 2000);
+	TEST_BTF_DUMP_DATA(btf, d, "enum", str, enum bpf_cmd, 0,
+			   "(enum bpf_cmd)2000", 2000);
+
+	TEST_BTF_DUMP_DATA_OVER(btf, d, "enum", str, enum bpf_cmd,
+				sizeof(enum bpf_cmd) - 1, "", BPF_MAP_CREATE);
+}
+
+static void test_btf_dump_struct_data(struct btf *btf, struct btf_dump *d,
+				      char *str)
+{
+	DECLARE_LIBBPF_OPTS(btf_dump_type_data_opts, opts);
+	char zero_data[512] = { };
+	char type_data[512];
+	void *fops = type_data;
+	void *skb = type_data;
+	size_t type_sz;
+	__s32 type_id;
+	char *cmpstr;
+	int ret;
+
+	memset(type_data, 255, sizeof(type_data));
+
+	/* simple struct */
+	TEST_BTF_DUMP_DATA_C(btf, d, "struct", str, struct btf_enum, BTF_F_COMPACT,
+			     {.name_off = (__u32)3,.val = (__s32)-1,});
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct btf_enum,
+			   BTF_F_COMPACT | BTF_F_NONAME,
+			   "{3,-1,}",
+			   { .name_off = 3, .val = -1,});
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct btf_enum, 0,
+"(struct btf_enum){\n"
+"	.name_off = (__u32)3,\n"
+"	.val = (__s32)-1,\n"
+"}",
+			   { .name_off = 3, .val = -1,});
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct btf_enum,
+			   BTF_F_COMPACT | BTF_F_NONAME,
+			   "{-1,}",
+			   { .name_off = 0, .val = -1,});
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct btf_enum,
+			   BTF_F_COMPACT | BTF_F_NONAME | BTF_F_ZERO,
+			   "{0,-1,}",
+			   { .name_off = 0, .val = -1,});
+	/* empty struct should be printed */
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct btf_enum, BTF_F_COMPACT,
+			   "(struct btf_enum){}",
+			   { .name_off = 0, .val = 0,});
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct btf_enum,
+			   BTF_F_COMPACT | BTF_F_NONAME,
+			   "{}",
+			   { .name_off = 0, .val = 0,});
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct btf_enum, 0,
+"(struct btf_enum){\n"
+"}",
+			   { .name_off = 0, .val = 0,});
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct btf_enum,
+			   BTF_F_COMPACT | BTF_F_ZERO,
+			   "(struct btf_enum){.name_off = (__u32)0,.val = (__s32)0,}",
+			   { .name_off = 0, .val = 0,});
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct btf_enum,
+			   BTF_F_ZERO,
+"(struct btf_enum){\n"
+"	.name_off = (__u32)0,\n"
+"	.val = (__s32)0,\n"
+"}",
+			   { .name_off = 0, .val = 0,});
+
+	/* struct with pointers */
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct list_head, BTF_F_COMPACT,
+			   "(struct list_head){.next = (struct list_head *)0x1,}",
+			   { .next = (struct list_head *)1 });
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct list_head, 0,
+"(struct list_head){\n"
+"	.next = (struct list_head *)0x1,\n"
+"}",
+			   { .next = (struct list_head *)1 });
+	/* NULL pointer should not be displayed */
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct list_head, BTF_F_COMPACT,
+			   "(struct list_head){}",
+			   { .next = (struct list_head *)0 });
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct list_head, 0,
+"(struct list_head){\n"
+"}",
+			   { .next = (struct list_head *)0 });
+
+	/* struct with function pointers */
+	type_id = btf__find_by_name(btf, "file_operations");
+	if (ASSERT_GT(type_id, 0, "find type id")) {
+		type_sz = btf__resolve_size(btf, type_id);
+		str[0] = '\0';
+
+		ret = btf_dump__dump_type_data(d, type_id, fops, type_sz, &opts);
+		ASSERT_EQ(ret, type_sz,
+			  "unexpected return value dumping file_operations");
+		cmpstr =
+"(struct file_operations){\n"
+"	.owner = (struct module *)0xffffffffffffffff,\n"
+"	.llseek = (loff_t (*)(struct file *, loff_t, int))0xffffffffffffffff,";
+
+		ASSERT_STRNEQ(str, cmpstr, strlen(cmpstr), "file_operations");
+	}
+
+	/* struct with char array */
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct bpf_prog_info, BTF_F_COMPACT,
+			   "(struct bpf_prog_info){.name = (char[16])['f','o','o',],}",
+			   { .name = "foo",});
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct bpf_prog_info,
+			   BTF_F_COMPACT | BTF_F_NONAME,
+			   "{['f','o','o',],}",
+			   {.name = "foo",});
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct bpf_prog_info, 0,
+"(struct bpf_prog_info){\n"
+"	.name = (char[16])[\n"
+"		'f',\n"
+"		'o',\n"
+"		'o',\n"
+"	],\n"
+"}",
+			   {.name = "foo",});
+	/* leading null char means do not display string */
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct bpf_prog_info, BTF_F_COMPACT,
+			   "(struct bpf_prog_info){}",
+			   {.name = {'\0', 'f', 'o', 'o'}});
+	/* handle non-printable characters */
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct bpf_prog_info, BTF_F_COMPACT,
+			   "(struct bpf_prog_info){.name = (char[16])[1,2,3,],}",
+			   { .name = {1, 2, 3, 0}});
+
+	/* struct with non-char array */
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct __sk_buff, BTF_F_COMPACT,
+			   "(struct __sk_buff){.cb = (__u32[5])[1,2,3,4,5,],}",
+			   { .cb = {1, 2, 3, 4, 5,},});
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct __sk_buff,
+			   BTF_F_COMPACT | BTF_F_NONAME,
+			   "{[1,2,3,4,5,],}",
+			   { .cb = { 1, 2, 3, 4, 5},});
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct __sk_buff, 0,
+"(struct __sk_buff){\n"
+"	.cb = (__u32[5])[\n"
+"		1,\n"
+"		2,\n"
+"		3,\n"
+"		4,\n"
+"		5,\n"
+"	],\n"
+"}",
+			   { .cb = { 1, 2, 3, 4, 5},});
+	/* For non-char, arrays, show non-zero values only */
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct __sk_buff, BTF_F_COMPACT,
+			   "(struct __sk_buff){.cb = (__u32[5])[0,0,1,0,0,],}",
+			   { .cb = { 0, 0, 1, 0, 0},});
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct __sk_buff, 0,
+"(struct __sk_buff){\n"
+"	.cb = (__u32[5])[\n"
+"		0,\n"
+"		0,\n"
+"		1,\n"
+"		0,\n"
+"		0,\n"
+"	],\n"
+"}",
+			   { .cb = { 0, 0, 1, 0, 0},});
+
+	/* struct with bitfields */
+	TEST_BTF_DUMP_DATA_C(btf, d, "struct", str, struct bpf_insn, BTF_F_COMPACT,
+		{.code = (__u8)1,.dst_reg = (__u8)0x2,.src_reg = (__u8)0x3,.off = (__s16)4,.imm = (__s32)5,});
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct bpf_insn,
+			   BTF_F_COMPACT | BTF_F_NONAME,
+			   "{1,0x2,0x3,4,5,}",
+			   { .code = 1, .dst_reg = 0x2, .src_reg = 0x3, .off = 4,
+			     .imm = 5,});
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct bpf_insn, 0,
+"(struct bpf_insn){\n"
+"	.code = (__u8)1,\n"
+"	.dst_reg = (__u8)0x2,\n"
+"	.src_reg = (__u8)0x3,\n"
+"	.off = (__s16)4,\n"
+"	.imm = (__s32)5,\n"
+"}",
+			   {.code = 1, .dst_reg = 2, .src_reg = 3, .off = 4, .imm = 5});
+
+	/* zeroed bitfields should not be displayed */
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct bpf_insn, BTF_F_COMPACT,
+			   "(struct bpf_insn){.dst_reg = (__u8)0x1,}",
+			   { .code = 0, .dst_reg = 1});
+
+	/* struct with enum bitfield */
+	type_id = btf__find_by_name(btf, "fs_context");
+	if (ASSERT_GT(type_id,  0, "find fs_context")) {
+		type_sz = btf__resolve_size(btf, type_id);
+		str[0] = '\0';
+
+		opts.emit_zeroes = true;
+		ret = btf_dump__dump_type_data(d, type_id, zero_data, type_sz, &opts);
+		ASSERT_EQ(ret, type_sz,
+			  "unexpected return value dumping fs_context");
+
+		ASSERT_NEQ(strstr(str, "FS_CONTEXT_FOR_MOUNT"), NULL,
+				  "bitfield value not present");
+	}
+
+	/* struct with nested anon union */
+	TEST_BTF_DUMP_DATA(btf, d, "struct", str, struct bpf_sock_ops, BTF_F_COMPACT,
+			   "(struct bpf_sock_ops){.op = (__u32)1,(union){.args = (__u32[4])[1,2,3,4,],.reply = (__u32)1,.replylong = (__u32[4])[1,2,3,4,],},}",
+			   { .op = 1, .args = { 1, 2, 3, 4}});
+
+	/* union with nested struct */
+	TEST_BTF_DUMP_DATA(btf, d, "union", str, union bpf_iter_link_info, BTF_F_COMPACT,
+			   "(union bpf_iter_link_info){.map = (struct){.map_fd = (__u32)1,},}",
+			   { .map = { .map_fd = 1 }});
+
+	/* struct skb with nested structs/unions; because type output is so
+	 * complex, we don't do a string comparison, just verify we return
+	 * the type size as the amount of data displayed.
+	 */
+	type_id = btf__find_by_name(btf, "sk_buff");
+	if (ASSERT_GT(type_id, 0, "find struct sk_buff")) {
+		type_sz = btf__resolve_size(btf, type_id);
+		str[0] = '\0';
+
+		ret = btf_dump__dump_type_data(d, type_id, skb, type_sz, &opts);
+		ASSERT_EQ(ret, type_sz,
+			  "unexpected return value dumping sk_buff");
+	}
+
+	/* overflow bpf_sock_ops struct with final element nonzero/zero.
+	 * Regardless of the value of the final field, we don't have all the
+	 * data we need to display it, so we should trigger an overflow.
+	 * In other words oveflow checking should trump "is field zero?"
+	 * checks because if we've overflowed, it shouldn't matter what the
+	 * field is - we can't trust its value so shouldn't display it.
+	 */
+	TEST_BTF_DUMP_DATA_OVER(btf, d, "struct", str, struct bpf_sock_ops,
+				sizeof(struct bpf_sock_ops) - 1,
+				"(struct bpf_sock_ops){\n\t.op = (__u32)1,\n",
+				{ .op = 1, .skb_tcp_flags = 2});
+	TEST_BTF_DUMP_DATA_OVER(btf, d, "struct", str, struct bpf_sock_ops,
+				sizeof(struct bpf_sock_ops) - 1,
+				"(struct bpf_sock_ops){\n\t.op = (__u32)1,\n",
+				{ .op = 1, .skb_tcp_flags = 0});
+}
+
+static void test_btf_dump_var_data(struct btf *btf, struct btf_dump *d,
+				   char *str)
+{
+	TEST_BTF_DUMP_VAR(btf, d, NULL, str, "cpu_number", int, BTF_F_COMPACT,
+			  "int cpu_number = (int)100", 100);
+	TEST_BTF_DUMP_VAR(btf, d, NULL, str, "cpu_profile_flip", int, BTF_F_COMPACT,
+			  "static int cpu_profile_flip = (int)2", 2);
+}
+
+static void test_btf_datasec(struct btf *btf, struct btf_dump *d, char *str,
+			     const char *name, const char *expected_val,
+			     void *data, size_t data_sz)
+{
+	DECLARE_LIBBPF_OPTS(btf_dump_type_data_opts, opts);
+	int ret = 0, cmp;
+	size_t secsize;
+	__s32 type_id;
+
+	opts.compact = true;
+
+	type_id = btf__find_by_name(btf, name);
+	if (!ASSERT_GT(type_id, 0, "find type id"))
+		return;
+
+	secsize = btf__resolve_size(btf, type_id);
+	ASSERT_EQ(secsize,  0, "verify section size");
+
+	str[0] = '\0';
+	ret = btf_dump__dump_type_data(d, type_id, data, data_sz, &opts);
+	ASSERT_EQ(ret, 0, "unexpected return value");
+
+	cmp = strcmp(str, expected_val);
+	ASSERT_EQ(cmp, 0, "ensure expected/actual match");
+}
+
+static void test_btf_dump_datasec_data(char *str)
+{
+	struct btf *btf = btf__parse("xdping_kern.o", NULL);
+	struct btf_dump_opts opts = { .ctx = str };
+	char license[4] = "GPL";
+	struct btf_dump *d;
+
+	if (!ASSERT_NEQ(btf, NULL, "xdping_kern.o BTF not found"))
+		return;
+
+	d = btf_dump__new(btf, NULL, &opts, btf_dump_snprintf);
+
+	if (!ASSERT_NEQ(d, NULL, "could not create BTF dump"))
+		return;
+
+	test_btf_datasec(btf, d, str, "license",
+			 "SEC(\"license\") char[4] _license = (char[4])['G','P','L',];",
+			 license, sizeof(license));
+}
+
 void test_btf_dump() {
+	char str[STRSIZE];
+	struct btf_dump_opts opts = { .ctx = str };
+	struct btf_dump *d;
+	struct btf *btf;
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(btf_dump_test_cases); i++) {
@@ -245,4 +815,34 @@ void test_btf_dump() {
 	}
 	if (test__start_subtest("btf_dump: incremental"))
 		test_btf_dump_incremental();
+
+	btf = libbpf_find_kernel_btf();
+	if (!ASSERT_NEQ(btf, NULL, "no kernel BTF found"))
+		return;
+
+	d = btf_dump__new(btf, NULL, &opts, btf_dump_snprintf);
+
+	if (!ASSERT_NEQ(d, NULL, "could not create BTF dump"))
+		return;
+
+	/* Verify type display for various types. */
+	if (test__start_subtest("btf_dump: int_data"))
+		test_btf_dump_int_data(btf, d, str);
+	if (test__start_subtest("btf_dump: float_data"))
+		test_btf_dump_float_data(btf, d, str);
+	if (test__start_subtest("btf_dump: char_data"))
+		test_btf_dump_char_data(btf, d, str);
+	if (test__start_subtest("btf_dump: typedef_data"))
+		test_btf_dump_typedef_data(btf, d, str);
+	if (test__start_subtest("btf_dump: enum_data"))
+		test_btf_dump_enum_data(btf, d, str);
+	if (test__start_subtest("btf_dump: struct_data"))
+		test_btf_dump_struct_data(btf, d, str);
+	if (test__start_subtest("btf_dump: var_data"))
+		test_btf_dump_var_data(btf, d, str);
+	btf_dump__free(d);
+	btf__free(btf);
+
+	if (test__start_subtest("btf_dump: datasec_data"))
+		test_btf_dump_datasec_data(str);
 }
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v6 bpf-next 1/3] libbpf: BTF dumper support for typed data
  2021-07-15 15:15 ` [PATCH v6 bpf-next 1/3] " Alan Maguire
@ 2021-07-16  6:24   ` Andrii Nakryiko
  2021-07-16 20:55     ` Andrii Nakryiko
  2021-07-16 21:58   ` Andrii Nakryiko
  2021-07-19 12:11   ` Naresh Kamboju
  2 siblings, 1 reply; 10+ messages in thread
From: Andrii Nakryiko @ 2021-07-16  6:24 UTC (permalink / raw)
  To: Alan Maguire
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
	Song Liu, Yonghong Song, john fastabend, KP Singh, Bill Wendling,
	Shuah Khan, bpf, Networking, open list:KERNEL SELFTEST FRAMEWORK,
	open list

On Thu, Jul 15, 2021 at 8:15 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> Add a BTF dumper for typed data, so that the user can dump a typed
> version of the data provided.
>
> The API is
>
> int btf_dump__dump_type_data(struct btf_dump *d, __u32 id,
>                              void *data, size_t data_sz,
>                              const struct btf_dump_type_data_opts *opts);
>
> ...where the id is the BTF id of the data pointed to by the "void *"
> argument; for example the BTF id of "struct sk_buff" for a
> "struct skb *" data pointer.  Options supported are
>
>  - a starting indent level (indent_lvl)
>  - a user-specified indent string which will be printed once per
>    indent level; if NULL, tab is chosen but any string <= 32 chars
>    can be provided.
>  - a set of boolean options to control dump display, similar to those
>    used for BPF helper bpf_snprintf_btf().  Options are
>         - compact : omit newlines and other indentation
>         - skip_names: omit member names
>         - emit_zeroes: show zero-value members
>
> Default output format is identical to that dumped by bpf_snprintf_btf(),
> for example a "struct sk_buff" representation would look like this:
>
> struct sk_buff){
>         (union){
>                 (struct){
>                         .next = (struct sk_buff *)0xffffffffffffffff,
>                         .prev = (struct sk_buff *)0xffffffffffffffff,
>                 (union){
>                         .dev = (struct net_device *)0xffffffffffffffff,
>                         .dev_scratch = (long unsigned int)18446744073709551615,
>                 },
>         },
> ...
>
> If the data structure is larger than the *data_sz*
> number of bytes that are available in *data*, as much
> of the data as possible will be dumped and -E2BIG will
> be returned.  This is useful as tracers will sometimes
> not be able to capture all of the data associated with
> a type; for example a "struct task_struct" is ~16k.
> Being able to specify that only a subset is available is
> important for such cases.  On success, the amount of data
> dumped is returned.
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---

Ok, this looks great. I think I found a few residual problems, so
please see comments below and address them. But I'm inclined to land
this patch set as is because it's in a good shape already, and it is
pretty, so it's hard and time-consuming to weed through minor (at this
point) changes between versions. So please send follow-up patch(es)
with fixes. Hopefully soon enough before the libbpf release. Thanks a
lot for working on this and persevering, this is a great API!

I'll apply a patch set to bpf-next when it will open up for new patches. Thanks.

>  tools/lib/bpf/btf.h      |  19 ++
>  tools/lib/bpf/btf_dump.c | 819 ++++++++++++++++++++++++++++++++++++++++++++++-
>  tools/lib/bpf/libbpf.map |   1 +
>  3 files changed, 834 insertions(+), 5 deletions(-)

I also wanted to call out this ^^ versus:

a) initial kernel-sharing version:

  >  18 files changed, 3236 insertions(+), 1319 deletions(-)

b) initial libbpf-only version:

  >  6 files changed, 1251 insertions(+), 3 deletions(-)

And the API actually gained in supported features and correctness.

>

[...]

> +
> +union float_data {
> +       long double ld;
> +       double d;
> +       float f;
> +};

clever

> +
> +static int btf_dump_float_data(struct btf_dump *d,
> +                              const struct btf_type *t,
> +                              __u32 type_id,
> +                              const void *data)
> +{
> +       const union float_data *flp = data;
> +       union float_data fl;
> +       int sz = t->size;
> +
> +       /* handle unaligned data; copy to local union */
> +       if (((uintptr_t)data) % sz) {
> +               memcpy(&fl, data, sz);
> +               flp = &fl;
> +       }
> +
> +       switch (sz) {
> +       case 16:
> +               btf_dump_type_values(d, "%Lf", flp->ld);
> +               break;
> +       case 8:
> +               btf_dump_type_values(d, "%lf", flp->d);
> +               break;
> +       case 4:
> +               btf_dump_type_values(d, "%f", flp->f);
> +               break;
> +       default:
> +               pr_warn("unexpected size %d for id [%u]\n", sz, type_id);
> +               return -EINVAL;
> +       }
> +       return 0;
> +}
> +

[...]

> +
> +static int btf_dump_array_data(struct btf_dump *d,
> +                              const struct btf_type *t,
> +                              __u32 id,
> +                              const void *data)
> +{
> +       const struct btf_array *array = btf_array(t);
> +       const struct btf_type *elem_type;
> +       __u32 i, elem_size = 0, elem_type_id;
> +       bool is_array_member;
> +
> +       elem_type_id = array->type;
> +       elem_type = skip_mods_and_typedefs(d->btf, elem_type_id, NULL);
> +       elem_size = btf__resolve_size(d->btf, elem_type_id);
> +       if (elem_size <= 0) {
> +               pr_warn("unexpected elem size %d for array type [%u]\n", elem_size, id);
> +               return -EINVAL;
> +       }
> +
> +       if (btf_is_int(elem_type)) {
> +               /*
> +                * BTF_INT_CHAR encoding never seems to be set for
> +                * char arrays, so if size is 1 and element is
> +                * printable as a char, we'll do that.
> +                */
> +               if (elem_size == 1)
> +                       d->typed_dump->is_array_char = true;
> +       }
> +
> +       /* note that we increment depth before calling btf_dump_print() below;
> +        * this is intentional.  btf_dump_data_newline() will not print a
> +        * newline for depth 0 (since this leaves us with trailing newlines
> +        * at the end of typed display), so depth is incremented first.
> +        * For similar reasons, we decrement depth before showing the closing
> +        * parenthesis.
> +        */
> +       d->typed_dump->depth++;
> +       btf_dump_printf(d, "[%s", btf_dump_data_newline(d));
> +
> +       /* may be a multidimensional array, so store current "is array member"
> +        * status so we can restore it correctly later.
> +        */
> +       is_array_member = d->typed_dump->is_array_member;
> +       d->typed_dump->is_array_member = true;
> +       for (i = 0; i < array->nelems; i++, data += elem_size) {
> +               if (d->typed_dump->is_array_terminated)
> +                       break;

I suspect this logic breaks for multi-dimensional char arrays. Please
check and add follow-up tests and fixes, no need to address that in
this patch set, you've suffered enough.


> +               btf_dump_dump_type_data(d, NULL, elem_type, elem_type_id, data, 0, 0);
> +       }
> +       d->typed_dump->is_array_member = is_array_member;
> +       d->typed_dump->depth--;
> +       btf_dump_data_pfx(d);
> +       btf_dump_type_values(d, "]");
> +
> +       return 0;
> +}
> +
> +static int btf_dump_struct_data(struct btf_dump *d,
> +                               const struct btf_type *t,
> +                               __u32 id,
> +                               const void *data)
> +{
> +       const struct btf_member *m = btf_members(t);
> +       __u16 n = btf_vlen(t);
> +       int i, err;
> +
> +       /* note that we increment depth before calling btf_dump_print() below;
> +        * this is intentional.  btf_dump_data_newline() will not print a
> +        * newline for depth 0 (since this leaves us with trailing newlines
> +        * at the end of typed display), so depth is incremented first.
> +        * For similar reasons, we decrement depth before showing the closing
> +        * parenthesis.
> +        */

ah, ok, I see. I sort of randomly stumbled on this from a purely
aesthetic reasons, but I'm happy we clarified this because it's
completely non-obvious

> +       d->typed_dump->depth++;
> +       btf_dump_printf(d, "{%s", btf_dump_data_newline(d));
> +
> +       for (i = 0; i < n; i++, m++) {
> +               const struct btf_type *mtype;
> +               const char *mname;
> +               __u32 moffset;
> +               __u8 bit_sz;
> +
> +               mtype = btf__type_by_id(d->btf, m->type);
> +               mname = btf_name_of(d, m->name_off);
> +               moffset = btf_member_bit_offset(t, i);
> +
> +               bit_sz = btf_member_bitfield_size(t, i);
> +               err = btf_dump_dump_type_data(d, mname, mtype, m->type, data + moffset / 8,
> +                                             moffset % 8, bit_sz);
> +               if (err < 0)
> +                       return err;
> +       }
> +       d->typed_dump->depth--;
> +       btf_dump_data_pfx(d);
> +       btf_dump_type_values(d, "}");
> +       return err;
> +}
> +
> +static int btf_dump_ptr_data(struct btf_dump *d,
> +                             const struct btf_type *t,
> +                             __u32 id,
> +                             const void *data)
> +{
> +       btf_dump_type_values(d, "%p", *(void **)data);

Wait, you fixed pointer zero checking logic and misaligned reads for
ints/floats, but none of that for actually printing pointers?...
Please send a follow-up fix.

> +       return 0;
> +}
> +
> +static int btf_dump_get_enum_value(struct btf_dump *d,
> +                                  const struct btf_type *t,
> +                                  const void *data,
> +                                  __u32 id,
> +                                  __s64 *value)
> +{
> +       int sz = t->size;
> +
> +       /* handle unaligned enum value */
> +       if (((uintptr_t)data) % sz) {

nit: probably worth a small helper with obvious name to avoid extra
comments and all those ((()))

> +               *value = (__s64)btf_dump_bitfield_get_data(d, t, data, 0, 0);
> +               return 0;
> +       }

[...]

> +               elem_type_id = array->type;
> +               elem_size = btf__resolve_size(d->btf, elem_type_id);
> +               elem_type = skip_mods_and_typedefs(d->btf, elem_type_id, NULL);
> +
> +               ischar = btf_is_int(elem_type) && elem_size == 1;
> +
> +               /* check all elements; if _any_ element is nonzero, all
> +                * of array is displayed.  We make an exception however
> +                * for char arrays where the first element is 0; these
> +                * are considered zeroed also, even if later elements are
> +                * non-zero because the string is terminated.
> +                */
> +               for (i = 0; i < array->nelems; i++) {
> +                       if (i == 0 && ischar && *(char *)data == 0)
> +                               return -ENODATA;

same here, this might be too aggressive for something like char a[2][10] ?

> +                       err = btf_dump_type_data_check_zero(d, elem_type,
> +                                                           elem_type_id,
> +                                                           data +
> +                                                           (i * elem_size),
> +                                                           bits_offset, 0);
> +                       if (err != -ENODATA)
> +                               return err;
> +               }
> +               return -ENODATA;
> +       }
> +       case BTF_KIND_STRUCT:
> +       case BTF_KIND_UNION: {
> +               const struct btf_member *m = btf_members(t);
> +               __u16 n = btf_vlen(t);
> +
> +               /* if any struct/union member is non-zero, the struct/union
> +                * is considered non-zero and dumped.
> +                */
> +               for (i = 0; i < n; i++, m++) {
> +                       const struct btf_type *mtype;
> +                       __u32 moffset;
> +
> +                       mtype = btf__type_by_id(d->btf, m->type);
> +                       moffset = btf_member_bit_offset(t, i);
> +
> +                       /* btf_int_bits() does not store member bitfield size;
> +                        * bitfield size needs to be stored here so int display
> +                        * of member can retrieve it.
> +                        */
> +                       bit_sz = btf_member_bitfield_size(t, i);
> +                       err = btf_dump_type_data_check_zero(d, mtype, m->type, data + moffset / 8,
> +                                                           moffset % 8, bit_sz);
> +                       if (err != ENODATA)
> +                               return err;
> +               }
> +               return -ENODATA;
> +       }
> +       case BTF_KIND_ENUM:
> +               if (btf_dump_get_enum_value(d, t, data, id, &value))
> +                       return 0;

why not propagating error here?

> +               if (value == 0)
> +                       return -ENODATA;
> +               return 0;
> +       default:
> +               return 0;
> +       }
> +}
> +

[...]

> +       case BTF_KIND_ARRAY:
> +               err = btf_dump_array_data(d, t, id, data);
> +               break;
> +       case BTF_KIND_STRUCT:
> +       case BTF_KIND_UNION:
> +               err = btf_dump_struct_data(d, t, id, data);
> +               break;
> +       case BTF_KIND_ENUM:
> +               /* handle bitfield and int enum values */
> +               if (bit_sz) {
> +                       unsigned __int128 print_num;
> +                       __s64 enum_val;
> +
> +                       print_num = btf_dump_bitfield_get_data(d, t, data, bits_offset, bit_sz);
> +                       enum_val = (__s64)print_num;
> +                       err = btf_dump_enum_data(d, t, id, &enum_val);

this is broken on big-endian, no? Basically almost always it will be
printing either 0, -1 or 0xffffffff?..

> +               } else
> +                       err = btf_dump_enum_data(d, t, id, data);
> +               break;
> +       case BTF_KIND_VAR:
> +               err = btf_dump_var_data(d, t, id, data);
> +               break;
> +       case BTF_KIND_DATASEC:
> +               err = btf_dump_datasec_data(d, t, id, data);
> +               break;
> +       default:
> +               pr_warn("unexpected kind [%u] for id [%u]\n",
> +                       BTF_INFO_KIND(t->info), id);
> +               return -EINVAL;
> +       }
> +       if (err < 0)
> +               return err;
> +       return size;
> +}
> +
> +int btf_dump__dump_type_data(struct btf_dump *d, __u32 id,
> +                            const void *data, size_t data_sz,
> +                            const struct btf_dump_type_data_opts *opts)
> +{
> +       const struct btf_type *t;
> +       int ret;
> +
> +       if (!OPTS_VALID(opts, btf_dump_type_data_opts))
> +               return libbpf_err(-EINVAL);
> +
> +       t = btf__type_by_id(d->btf, id);
> +       if (!t)
> +               return libbpf_err(-ENOENT);
> +
> +       d->typed_dump = calloc(1, sizeof(struct btf_dump_data));

just realized this doesn't have to be calloc()'ed, it can be on the
stack zero-initialized variable; feel free to switch in the follow up
as well

> +       if (!d->typed_dump)
> +               return libbpf_err(-ENOMEM);

then we won't need to handle this at all

> +
> +       d->typed_dump->data_end = data + data_sz;
> +       d->typed_dump->indent_lvl = OPTS_GET(opts, indent_level, 0);
> +       /* default indent string is a tab */
> +       if (!opts->indent_str)
> +               d->typed_dump->indent_str[0] = '\t';

[...]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v6 bpf-next 3/3] selftests/bpf: add dump type data tests to btf dump tests
  2021-07-15 15:15 ` [PATCH v6 bpf-next 3/3] selftests/bpf: add dump type data tests to btf dump tests Alan Maguire
@ 2021-07-16  6:24   ` Andrii Nakryiko
  0 siblings, 0 replies; 10+ messages in thread
From: Andrii Nakryiko @ 2021-07-16  6:24 UTC (permalink / raw)
  To: Alan Maguire
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
	Song Liu, Yonghong Song, john fastabend, KP Singh, Bill Wendling,
	Shuah Khan, bpf, Networking, open list:KERNEL SELFTEST FRAMEWORK,
	open list

On Thu, Jul 15, 2021 at 8:16 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> Test various type data dumping operations by comparing expected
> format with the dumped string; an snprintf-style printf function
> is used to record the string dumped.  Also verify overflow handling
> where the data passed does not cover the full size of a type,
> such as would occur if a tracer has a portion of the 8k
> "struct task_struct".
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  tools/testing/selftests/bpf/prog_tests/btf_dump.c | 600 ++++++++++++++++++++++
>  1 file changed, 600 insertions(+)
>

[...]

> @@ -245,4 +815,34 @@ void test_btf_dump() {
>         }
>         if (test__start_subtest("btf_dump: incremental"))
>                 test_btf_dump_incremental();
> +
> +       btf = libbpf_find_kernel_btf();
> +       if (!ASSERT_NEQ(btf, NULL, "no kernel BTF found"))
> +               return;
> +
> +       d = btf_dump__new(btf, NULL, &opts, btf_dump_snprintf);
> +
> +       if (!ASSERT_NEQ(d, NULL, "could not create BTF dump"))

better use ASSERT_OK_PTR() for this, I'll adjust when applying


> +               return;
> +
> +       /* Verify type display for various types. */
> +       if (test__start_subtest("btf_dump: int_data"))
> +               test_btf_dump_int_data(btf, d, str);
> +       if (test__start_subtest("btf_dump: float_data"))
> +               test_btf_dump_float_data(btf, d, str);
> +       if (test__start_subtest("btf_dump: char_data"))
> +               test_btf_dump_char_data(btf, d, str);
> +       if (test__start_subtest("btf_dump: typedef_data"))
> +               test_btf_dump_typedef_data(btf, d, str);
> +       if (test__start_subtest("btf_dump: enum_data"))
> +               test_btf_dump_enum_data(btf, d, str);
> +       if (test__start_subtest("btf_dump: struct_data"))
> +               test_btf_dump_struct_data(btf, d, str);
> +       if (test__start_subtest("btf_dump: var_data"))
> +               test_btf_dump_var_data(btf, d, str);
> +       btf_dump__free(d);
> +       btf__free(btf);
> +
> +       if (test__start_subtest("btf_dump: datasec_data"))
> +               test_btf_dump_datasec_data(str);
>  }
> --
> 1.8.3.1
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v6 bpf-next 1/3] libbpf: BTF dumper support for typed data
  2021-07-16  6:24   ` Andrii Nakryiko
@ 2021-07-16 20:55     ` Andrii Nakryiko
  0 siblings, 0 replies; 10+ messages in thread
From: Andrii Nakryiko @ 2021-07-16 20:55 UTC (permalink / raw)
  To: Alan Maguire
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
	Song Liu, Yonghong Song, john fastabend, KP Singh, Bill Wendling,
	Shuah Khan, bpf, Networking, open list:KERNEL SELFTEST FRAMEWORK,
	open list

On Thu, Jul 15, 2021 at 11:24 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, Jul 15, 2021 at 8:15 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> >
> > Add a BTF dumper for typed data, so that the user can dump a typed
> > version of the data provided.
> >
> > The API is
> >
> > int btf_dump__dump_type_data(struct btf_dump *d, __u32 id,
> >                              void *data, size_t data_sz,
> >                              const struct btf_dump_type_data_opts *opts);
> >
> > ...where the id is the BTF id of the data pointed to by the "void *"
> > argument; for example the BTF id of "struct sk_buff" for a
> > "struct skb *" data pointer.  Options supported are
> >
> >  - a starting indent level (indent_lvl)
> >  - a user-specified indent string which will be printed once per
> >    indent level; if NULL, tab is chosen but any string <= 32 chars
> >    can be provided.
> >  - a set of boolean options to control dump display, similar to those
> >    used for BPF helper bpf_snprintf_btf().  Options are
> >         - compact : omit newlines and other indentation
> >         - skip_names: omit member names
> >         - emit_zeroes: show zero-value members
> >
> > Default output format is identical to that dumped by bpf_snprintf_btf(),
> > for example a "struct sk_buff" representation would look like this:
> >
> > struct sk_buff){
> >         (union){
> >                 (struct){
> >                         .next = (struct sk_buff *)0xffffffffffffffff,
> >                         .prev = (struct sk_buff *)0xffffffffffffffff,
> >                 (union){
> >                         .dev = (struct net_device *)0xffffffffffffffff,
> >                         .dev_scratch = (long unsigned int)18446744073709551615,
> >                 },
> >         },
> > ...
> >
> > If the data structure is larger than the *data_sz*
> > number of bytes that are available in *data*, as much
> > of the data as possible will be dumped and -E2BIG will
> > be returned.  This is useful as tracers will sometimes
> > not be able to capture all of the data associated with
> > a type; for example a "struct task_struct" is ~16k.
> > Being able to specify that only a subset is available is
> > important for such cases.  On success, the amount of data
> > dumped is returned.
> >
> > Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> > ---
>
> Ok, this looks great. I think I found a few residual problems, so
> please see comments below and address them. But I'm inclined to land
> this patch set as is because it's in a good shape already, and it is
> pretty, so it's hard and time-consuming to weed through minor (at this
> point) changes between versions. So please send follow-up patch(es)
> with fixes. Hopefully soon enough before the libbpf release. Thanks a
> lot for working on this and persevering, this is a great API!
>
> I'll apply a patch set to bpf-next when it will open up for new patches. Thanks.

Applied to bpf-next.

>
> >  tools/lib/bpf/btf.h      |  19 ++
> >  tools/lib/bpf/btf_dump.c | 819 ++++++++++++++++++++++++++++++++++++++++++++++-
> >  tools/lib/bpf/libbpf.map |   1 +
> >  3 files changed, 834 insertions(+), 5 deletions(-)
>
> I also wanted to call out this ^^ versus:
>
> a) initial kernel-sharing version:
>
>   >  18 files changed, 3236 insertions(+), 1319 deletions(-)
>
> b) initial libbpf-only version:
>
>   >  6 files changed, 1251 insertions(+), 3 deletions(-)
>
> And the API actually gained in supported features and correctness.
>
> >
>
> [...]
>
> > +
> > +union float_data {
> > +       long double ld;
> > +       double d;
> > +       float f;
> > +};
>
> clever
>
> > +
> > +static int btf_dump_float_data(struct btf_dump *d,
> > +                              const struct btf_type *t,
> > +                              __u32 type_id,
> > +                              const void *data)
> > +{
> > +       const union float_data *flp = data;
> > +       union float_data fl;
> > +       int sz = t->size;
> > +
> > +       /* handle unaligned data; copy to local union */
> > +       if (((uintptr_t)data) % sz) {
> > +               memcpy(&fl, data, sz);
> > +               flp = &fl;
> > +       }
> > +
> > +       switch (sz) {
> > +       case 16:
> > +               btf_dump_type_values(d, "%Lf", flp->ld);
> > +               break;
> > +       case 8:
> > +               btf_dump_type_values(d, "%lf", flp->d);
> > +               break;
> > +       case 4:
> > +               btf_dump_type_values(d, "%f", flp->f);
> > +               break;
> > +       default:
> > +               pr_warn("unexpected size %d for id [%u]\n", sz, type_id);
> > +               return -EINVAL;
> > +       }
> > +       return 0;
> > +}
> > +
>
> [...]
>
> > +
> > +static int btf_dump_array_data(struct btf_dump *d,
> > +                              const struct btf_type *t,
> > +                              __u32 id,
> > +                              const void *data)
> > +{
> > +       const struct btf_array *array = btf_array(t);
> > +       const struct btf_type *elem_type;
> > +       __u32 i, elem_size = 0, elem_type_id;
> > +       bool is_array_member;
> > +
> > +       elem_type_id = array->type;
> > +       elem_type = skip_mods_and_typedefs(d->btf, elem_type_id, NULL);
> > +       elem_size = btf__resolve_size(d->btf, elem_type_id);
> > +       if (elem_size <= 0) {
> > +               pr_warn("unexpected elem size %d for array type [%u]\n", elem_size, id);
> > +               return -EINVAL;
> > +       }
> > +
> > +       if (btf_is_int(elem_type)) {
> > +               /*
> > +                * BTF_INT_CHAR encoding never seems to be set for
> > +                * char arrays, so if size is 1 and element is
> > +                * printable as a char, we'll do that.
> > +                */
> > +               if (elem_size == 1)
> > +                       d->typed_dump->is_array_char = true;
> > +       }
> > +
> > +       /* note that we increment depth before calling btf_dump_print() below;
> > +        * this is intentional.  btf_dump_data_newline() will not print a
> > +        * newline for depth 0 (since this leaves us with trailing newlines
> > +        * at the end of typed display), so depth is incremented first.
> > +        * For similar reasons, we decrement depth before showing the closing
> > +        * parenthesis.
> > +        */
> > +       d->typed_dump->depth++;
> > +       btf_dump_printf(d, "[%s", btf_dump_data_newline(d));
> > +
> > +       /* may be a multidimensional array, so store current "is array member"
> > +        * status so we can restore it correctly later.
> > +        */
> > +       is_array_member = d->typed_dump->is_array_member;
> > +       d->typed_dump->is_array_member = true;
> > +       for (i = 0; i < array->nelems; i++, data += elem_size) {
> > +               if (d->typed_dump->is_array_terminated)
> > +                       break;
>
> I suspect this logic breaks for multi-dimensional char arrays. Please
> check and add follow-up tests and fixes, no need to address that in
> this patch set, you've suffered enough.
>
>
> > +               btf_dump_dump_type_data(d, NULL, elem_type, elem_type_id, data, 0, 0);
> > +       }
> > +       d->typed_dump->is_array_member = is_array_member;
> > +       d->typed_dump->depth--;
> > +       btf_dump_data_pfx(d);
> > +       btf_dump_type_values(d, "]");
> > +
> > +       return 0;
> > +}
> > +
> > +static int btf_dump_struct_data(struct btf_dump *d,
> > +                               const struct btf_type *t,
> > +                               __u32 id,
> > +                               const void *data)
> > +{
> > +       const struct btf_member *m = btf_members(t);
> > +       __u16 n = btf_vlen(t);
> > +       int i, err;
> > +
> > +       /* note that we increment depth before calling btf_dump_print() below;
> > +        * this is intentional.  btf_dump_data_newline() will not print a
> > +        * newline for depth 0 (since this leaves us with trailing newlines
> > +        * at the end of typed display), so depth is incremented first.
> > +        * For similar reasons, we decrement depth before showing the closing
> > +        * parenthesis.
> > +        */
>
> ah, ok, I see. I sort of randomly stumbled on this from a purely
> aesthetic reasons, but I'm happy we clarified this because it's
> completely non-obvious
>
> > +       d->typed_dump->depth++;
> > +       btf_dump_printf(d, "{%s", btf_dump_data_newline(d));
> > +
> > +       for (i = 0; i < n; i++, m++) {
> > +               const struct btf_type *mtype;
> > +               const char *mname;
> > +               __u32 moffset;
> > +               __u8 bit_sz;
> > +
> > +               mtype = btf__type_by_id(d->btf, m->type);
> > +               mname = btf_name_of(d, m->name_off);
> > +               moffset = btf_member_bit_offset(t, i);
> > +
> > +               bit_sz = btf_member_bitfield_size(t, i);
> > +               err = btf_dump_dump_type_data(d, mname, mtype, m->type, data + moffset / 8,
> > +                                             moffset % 8, bit_sz);
> > +               if (err < 0)
> > +                       return err;
> > +       }
> > +       d->typed_dump->depth--;
> > +       btf_dump_data_pfx(d);
> > +       btf_dump_type_values(d, "}");
> > +       return err;
> > +}
> > +
> > +static int btf_dump_ptr_data(struct btf_dump *d,
> > +                             const struct btf_type *t,
> > +                             __u32 id,
> > +                             const void *data)
> > +{
> > +       btf_dump_type_values(d, "%p", *(void **)data);
>
> Wait, you fixed pointer zero checking logic and misaligned reads for
> ints/floats, but none of that for actually printing pointers?...
> Please send a follow-up fix.
>
> > +       return 0;
> > +}
> > +
> > +static int btf_dump_get_enum_value(struct btf_dump *d,
> > +                                  const struct btf_type *t,
> > +                                  const void *data,
> > +                                  __u32 id,
> > +                                  __s64 *value)
> > +{
> > +       int sz = t->size;
> > +
> > +       /* handle unaligned enum value */
> > +       if (((uintptr_t)data) % sz) {
>
> nit: probably worth a small helper with obvious name to avoid extra
> comments and all those ((()))
>
> > +               *value = (__s64)btf_dump_bitfield_get_data(d, t, data, 0, 0);
> > +               return 0;
> > +       }
>
> [...]
>
> > +               elem_type_id = array->type;
> > +               elem_size = btf__resolve_size(d->btf, elem_type_id);
> > +               elem_type = skip_mods_and_typedefs(d->btf, elem_type_id, NULL);
> > +
> > +               ischar = btf_is_int(elem_type) && elem_size == 1;
> > +
> > +               /* check all elements; if _any_ element is nonzero, all
> > +                * of array is displayed.  We make an exception however
> > +                * for char arrays where the first element is 0; these
> > +                * are considered zeroed also, even if later elements are
> > +                * non-zero because the string is terminated.
> > +                */
> > +               for (i = 0; i < array->nelems; i++) {
> > +                       if (i == 0 && ischar && *(char *)data == 0)
> > +                               return -ENODATA;
>
> same here, this might be too aggressive for something like char a[2][10] ?
>
> > +                       err = btf_dump_type_data_check_zero(d, elem_type,
> > +                                                           elem_type_id,
> > +                                                           data +
> > +                                                           (i * elem_size),
> > +                                                           bits_offset, 0);
> > +                       if (err != -ENODATA)
> > +                               return err;
> > +               }
> > +               return -ENODATA;
> > +       }
> > +       case BTF_KIND_STRUCT:
> > +       case BTF_KIND_UNION: {
> > +               const struct btf_member *m = btf_members(t);
> > +               __u16 n = btf_vlen(t);
> > +
> > +               /* if any struct/union member is non-zero, the struct/union
> > +                * is considered non-zero and dumped.
> > +                */
> > +               for (i = 0; i < n; i++, m++) {
> > +                       const struct btf_type *mtype;
> > +                       __u32 moffset;
> > +
> > +                       mtype = btf__type_by_id(d->btf, m->type);
> > +                       moffset = btf_member_bit_offset(t, i);
> > +
> > +                       /* btf_int_bits() does not store member bitfield size;
> > +                        * bitfield size needs to be stored here so int display
> > +                        * of member can retrieve it.
> > +                        */
> > +                       bit_sz = btf_member_bitfield_size(t, i);
> > +                       err = btf_dump_type_data_check_zero(d, mtype, m->type, data + moffset / 8,
> > +                                                           moffset % 8, bit_sz);
> > +                       if (err != ENODATA)
> > +                               return err;
> > +               }
> > +               return -ENODATA;
> > +       }
> > +       case BTF_KIND_ENUM:
> > +               if (btf_dump_get_enum_value(d, t, data, id, &value))
> > +                       return 0;
>
> why not propagating error here?
>
> > +               if (value == 0)
> > +                       return -ENODATA;
> > +               return 0;
> > +       default:
> > +               return 0;
> > +       }
> > +}
> > +
>
> [...]
>
> > +       case BTF_KIND_ARRAY:
> > +               err = btf_dump_array_data(d, t, id, data);
> > +               break;
> > +       case BTF_KIND_STRUCT:
> > +       case BTF_KIND_UNION:
> > +               err = btf_dump_struct_data(d, t, id, data);
> > +               break;
> > +       case BTF_KIND_ENUM:
> > +               /* handle bitfield and int enum values */
> > +               if (bit_sz) {
> > +                       unsigned __int128 print_num;
> > +                       __s64 enum_val;
> > +
> > +                       print_num = btf_dump_bitfield_get_data(d, t, data, bits_offset, bit_sz);
> > +                       enum_val = (__s64)print_num;
> > +                       err = btf_dump_enum_data(d, t, id, &enum_val);
>
> this is broken on big-endian, no? Basically almost always it will be
> printing either 0, -1 or 0xffffffff?..
>
> > +               } else
> > +                       err = btf_dump_enum_data(d, t, id, data);
> > +               break;
> > +       case BTF_KIND_VAR:
> > +               err = btf_dump_var_data(d, t, id, data);
> > +               break;
> > +       case BTF_KIND_DATASEC:
> > +               err = btf_dump_datasec_data(d, t, id, data);
> > +               break;
> > +       default:
> > +               pr_warn("unexpected kind [%u] for id [%u]\n",
> > +                       BTF_INFO_KIND(t->info), id);
> > +               return -EINVAL;
> > +       }
> > +       if (err < 0)
> > +               return err;
> > +       return size;
> > +}
> > +
> > +int btf_dump__dump_type_data(struct btf_dump *d, __u32 id,
> > +                            const void *data, size_t data_sz,
> > +                            const struct btf_dump_type_data_opts *opts)
> > +{
> > +       const struct btf_type *t;
> > +       int ret;
> > +
> > +       if (!OPTS_VALID(opts, btf_dump_type_data_opts))
> > +               return libbpf_err(-EINVAL);
> > +
> > +       t = btf__type_by_id(d->btf, id);
> > +       if (!t)
> > +               return libbpf_err(-ENOENT);
> > +
> > +       d->typed_dump = calloc(1, sizeof(struct btf_dump_data));
>
> just realized this doesn't have to be calloc()'ed, it can be on the
> stack zero-initialized variable; feel free to switch in the follow up
> as well
>
> > +       if (!d->typed_dump)
> > +               return libbpf_err(-ENOMEM);
>
> then we won't need to handle this at all
>
> > +
> > +       d->typed_dump->data_end = data + data_sz;
> > +       d->typed_dump->indent_lvl = OPTS_GET(opts, indent_level, 0);
> > +       /* default indent string is a tab */
> > +       if (!opts->indent_str)
> > +               d->typed_dump->indent_str[0] = '\t';
>
> [...]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v6 bpf-next 1/3] libbpf: BTF dumper support for typed data
  2021-07-15 15:15 ` [PATCH v6 bpf-next 1/3] " Alan Maguire
  2021-07-16  6:24   ` Andrii Nakryiko
@ 2021-07-16 21:58   ` Andrii Nakryiko
  2021-07-19 12:11   ` Naresh Kamboju
  2 siblings, 0 replies; 10+ messages in thread
From: Andrii Nakryiko @ 2021-07-16 21:58 UTC (permalink / raw)
  To: Alan Maguire
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
	Song Liu, Yonghong Song, john fastabend, KP Singh, Bill Wendling,
	Shuah Khan, bpf, Networking, open list:KERNEL SELFTEST FRAMEWORK,
	open list

On Thu, Jul 15, 2021 at 8:15 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> Add a BTF dumper for typed data, so that the user can dump a typed
> version of the data provided.
>
> The API is
>
> int btf_dump__dump_type_data(struct btf_dump *d, __u32 id,
>                              void *data, size_t data_sz,
>                              const struct btf_dump_type_data_opts *opts);
>
> ...where the id is the BTF id of the data pointed to by the "void *"
> argument; for example the BTF id of "struct sk_buff" for a
> "struct skb *" data pointer.  Options supported are
>
>  - a starting indent level (indent_lvl)
>  - a user-specified indent string which will be printed once per
>    indent level; if NULL, tab is chosen but any string <= 32 chars
>    can be provided.
>  - a set of boolean options to control dump display, similar to those
>    used for BPF helper bpf_snprintf_btf().  Options are
>         - compact : omit newlines and other indentation
>         - skip_names: omit member names
>         - emit_zeroes: show zero-value members
>
> Default output format is identical to that dumped by bpf_snprintf_btf(),
> for example a "struct sk_buff" representation would look like this:
>
> struct sk_buff){
>         (union){
>                 (struct){
>                         .next = (struct sk_buff *)0xffffffffffffffff,
>                         .prev = (struct sk_buff *)0xffffffffffffffff,
>                 (union){
>                         .dev = (struct net_device *)0xffffffffffffffff,
>                         .dev_scratch = (long unsigned int)18446744073709551615,
>                 },
>         },
> ...
>
> If the data structure is larger than the *data_sz*
> number of bytes that are available in *data*, as much
> of the data as possible will be dumped and -E2BIG will
> be returned.  This is useful as tracers will sometimes
> not be able to capture all of the data associated with
> a type; for example a "struct task_struct" is ~16k.
> Being able to specify that only a subset is available is
> important for such cases.  On success, the amount of data
> dumped is returned.
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  tools/lib/bpf/btf.h      |  19 ++
>  tools/lib/bpf/btf_dump.c | 819 ++++++++++++++++++++++++++++++++++++++++++++++-
>  tools/lib/bpf/libbpf.map |   1 +
>  3 files changed, 834 insertions(+), 5 deletions(-)
>

[...]

> +/* return size of type, or if base type overflows, return -E2BIG. */
> +static int btf_dump_type_data_check_overflow(struct btf_dump *d,
> +                                            const struct btf_type *t,
> +                                            __u32 id,
> +                                            const void *data,
> +                                            __u8 bits_offset)
> +{
> +       __s64 size = btf__resolve_size(d->btf, id);
> +
> +       if (size < 0 || size >= INT_MAX) {
> +               pr_warn("unexpected size [%lld] for id [%u]\n",
> +                       size, id);

ppc64le arch doesn't like the %lld:

 In file included from btf_dump.c:22:
btf_dump.c: In function 'btf_dump_type_data_check_overflow':
libbpf_internal.h:111:22: error: format '%lld' expects argument of
type 'long long int', but argument 3 has type '__s64' {aka 'long int'}
[-Werror=format=]
  111 |  libbpf_print(level, "libbpf: " fmt, ##__VA_ARGS__); \
      |                      ^~~~~~~~~~
libbpf_internal.h:114:27: note: in expansion of macro '__pr'
  114 | #define pr_warn(fmt, ...) __pr(LIBBPF_WARN, fmt, ##__VA_ARGS__)
      |                           ^~~~
btf_dump.c:1992:3: note: in expansion of macro 'pr_warn'
 1992 |   pr_warn("unexpected size [%lld] for id [%u]\n",
      |   ^~~~~~~
btf_dump.c:1992:32: note: format string is defined here
 1992 |   pr_warn("unexpected size [%lld] for id [%u]\n",
      |                             ~~~^
      |                                |
      |                                long long int
      |                             %ld


Cast to size_t and use %zu.

> +               return -EINVAL;
> +       }
> +

[...]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v6 bpf-next 1/3] libbpf: BTF dumper support for typed data
  2021-07-15 15:15 ` [PATCH v6 bpf-next 1/3] " Alan Maguire
  2021-07-16  6:24   ` Andrii Nakryiko
  2021-07-16 21:58   ` Andrii Nakryiko
@ 2021-07-19 12:11   ` Naresh Kamboju
  2021-07-19 14:15     ` Alan Maguire
  2 siblings, 1 reply; 10+ messages in thread
From: Naresh Kamboju @ 2021-07-19 12:11 UTC (permalink / raw)
  To: Alan Maguire, Linux-Next Mailing List
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, john.fastabend,
	kpsingh, morbo, Shuah Khan, bpf, Netdev,
	open list:KERNEL SELFTEST FRAMEWORK, open list, Stephen Rothwell,
	lkft-triage

On Thu, 15 Jul 2021 at 20:46, Alan Maguire <alan.maguire@oracle.com> wrote:
>
> Add a BTF dumper for typed data, so that the user can dump a typed
> version of the data provided.
>
> The API is
>
> int btf_dump__dump_type_data(struct btf_dump *d, __u32 id,
>                              void *data, size_t data_sz,
>                              const struct btf_dump_type_data_opts *opts);
>
> ...where the id is the BTF id of the data pointed to by the "void *"
> argument; for example the BTF id of "struct sk_buff" for a
> "struct skb *" data pointer.  Options supported are
>
>  - a starting indent level (indent_lvl)
>  - a user-specified indent string which will be printed once per
>    indent level; if NULL, tab is chosen but any string <= 32 chars
>    can be provided.
>  - a set of boolean options to control dump display, similar to those
>    used for BPF helper bpf_snprintf_btf().  Options are
>         - compact : omit newlines and other indentation
>         - skip_names: omit member names
>         - emit_zeroes: show zero-value members
>
> Default output format is identical to that dumped by bpf_snprintf_btf(),
> for example a "struct sk_buff" representation would look like this:
>
> struct sk_buff){
>         (union){
>                 (struct){
>                         .next = (struct sk_buff *)0xffffffffffffffff,
>                         .prev = (struct sk_buff *)0xffffffffffffffff,
>                 (union){
>                         .dev = (struct net_device *)0xffffffffffffffff,
>                         .dev_scratch = (long unsigned int)18446744073709551615,
>                 },
>         },
> ...
>
> If the data structure is larger than the *data_sz*
> number of bytes that are available in *data*, as much
> of the data as possible will be dumped and -E2BIG will
> be returned.  This is useful as tracers will sometimes
> not be able to capture all of the data associated with
> a type; for example a "struct task_struct" is ~16k.
> Being able to specify that only a subset is available is
> important for such cases.  On success, the amount of data
> dumped is returned.
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  tools/lib/bpf/btf.h      |  19 ++
>  tools/lib/bpf/btf_dump.c | 819 ++++++++++++++++++++++++++++++++++++++++++++++-
>  tools/lib/bpf/libbpf.map |   1 +
>  3 files changed, 834 insertions(+), 5 deletions(-)

<trim>

> diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
> index 5dc6b517..929cf93 100644
> --- a/tools/lib/bpf/btf_dump.c
> +++ b/tools/lib/bpf/btf_dump.c


Following perf build errors noticed on i386 and arm 32-bit architectures on
linux next 20210719 tag with gcc-11.

metadata:
--------------
   git_repo: https://gitlab.com/Linaro/lkft/mirrors/next/linux-next
   git_short_log: 08076eab6fef ( Add linux-next specific files for 20210719 )
   toolchain: gcc-11
   target_arch: arm and i386


> +static void btf_dump_int128(struct btf_dump *d,
> +                           const struct btf_type *t,
> +                           const void *data)
> +{
> +       __int128 num = *(__int128 *)data;


btf_dump.c: In function 'btf_dump_int128':
btf_dump.c:1559:9: error: expected expression before '__int128'
 1559 |         __int128 num = *(__int128 *)data;
      |         ^~~~~~~~
btf_dump.c:1561:14: error: 'num' undeclared (first use in this function)
 1561 |         if ((num >> 64) == 0)
      |              ^~~
btf_dump.c:1561:14: note: each undeclared identifier is reported only
once for each function it appears in
btf_dump.c: At top level:
btf_dump.c:1568:17: error: '__int128' is not supported on this target
 1568 | static unsigned __int128 btf_dump_bitfield_get_data(struct btf_dump *d,
      |                 ^~~~~~~~
btf_dump.c: In function 'btf_dump_bitfield_get_data':
btf_dump.c:1576:18: error: '__int128' is not supported on this target
 1576 |         unsigned __int128 num = 0, ret;
      |                  ^~~~~~~~
btf_dump.c: In function 'btf_dump_bitfield_check_zero':
btf_dump.c:1608:9: error: expected expression before '__int128'
 1608 |         __int128 check_num;
      |         ^~~~~~~~
btf_dump.c:1610:9: error: 'check_num' undeclared (first use in this function)
 1610 |         check_num = btf_dump_bitfield_get_data(d, t, data,
bits_offset, bit_sz);
      |         ^~~~~~~~~
btf_dump.c: In function 'btf_dump_bitfield_data':
btf_dump.c:1622:18: error: '__int128' is not supported on this target
 1622 |         unsigned __int128 print_num;
      |                  ^~~~~~~~
btf_dump.c: In function 'btf_dump_dump_type_data':
btf_dump.c:2212:34: error: '__int128' is not supported on this target
 2212 |                         unsigned __int128 print_num;
      |                                  ^~~~~~~~


Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>

reference build link,
build: https://builds.tuxbuild.com/1vWeCpIox9EoV35c80bwOvU9nbb/
config: https://builds.tuxbuild.com/1vWeCpIox9EoV35c80bwOvU9nbb/config


steps to reproduce:
---------------------
# TuxMake is a command line tool and Python library that provides
# portable and repeatable Linux kernel builds across a variety of
# architectures, toolchains, kernel configurations, and make targets.
#
# TuxMake supports the concept of runtimes.
# See https://docs.tuxmake.org/runtimes/, for that to work it requires
# that you install podman or docker on your system.
#
# To install tuxmake on your system globally:
# sudo pip3 install -U tuxmake
#
# See https://docs.tuxmake.org/ for complete documentation.


tuxmake --runtime podman --target-arch arm --toolchain gcc-11
--kconfig defconfig --kconfig-add
https://builds.tuxbuild.com/1vWeCpIox9EoV35c80bwOvU9nbb/config


--
Linaro LKFT
https://lkft.linaro.org

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v6 bpf-next 1/3] libbpf: BTF dumper support for typed data
  2021-07-19 12:11   ` Naresh Kamboju
@ 2021-07-19 14:15     ` Alan Maguire
  0 siblings, 0 replies; 10+ messages in thread
From: Alan Maguire @ 2021-07-19 14:15 UTC (permalink / raw)
  To: Naresh Kamboju, andrii
  Cc: Alan Maguire, Linux-Next Mailing List, Alexei Starovoitov,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	john.fastabend, kpsingh, morbo, Shuah Khan, bpf, Netdev,
	open list:KERNEL SELFTEST FRAMEWORK, open list, Stephen Rothwell,
	lkft-triage


On Mon, 19 Jul 2021, Naresh Kamboju wrote:

> On Thu, 15 Jul 2021 at 20:46, Alan Maguire <alan.maguire@oracle.com> wrote:
> >
> > Add a BTF dumper for typed data, so that the user can dump a typed
> > version of the data provided.
> 
> <trim>
> 
> > diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
> > index 5dc6b517..929cf93 100644
> > --- a/tools/lib/bpf/btf_dump.c
> > +++ b/tools/lib/bpf/btf_dump.c
> 
> 
> Following perf build errors noticed on i386 and arm 32-bit architectures on
> linux next 20210719 tag with gcc-11.
> 
> metadata:
> --------------
>    git_repo: https://gitlab.com/Linaro/lkft/mirrors/next/linux-next
>    git_short_log: 08076eab6fef ( Add linux-next specific files for 20210719 )
>    toolchain: gcc-11
>    target_arch: arm and i386
> 
> 
> > +static void btf_dump_int128(struct btf_dump *d,
> > +                           const struct btf_type *t,
> > +                           const void *data)
> > +{
> > +       __int128 num = *(__int128 *)data;
> 
> 
> btf_dump.c: In function 'btf_dump_int128':
> btf_dump.c:1559:9: error: expected expression before '__int128'
>  1559 |         __int128 num = *(__int128 *)data;
>       |         ^~~~~~~~
> btf_dump.c:1561:14: error: 'num' undeclared (first use in this function)
>  1561 |         if ((num >> 64) == 0)
>       |              ^~~
> btf_dump.c:1561:14: note: each undeclared identifier is reported only
> once for each function it appears in
> btf_dump.c: At top level:
> btf_dump.c:1568:17: error: '__int128' is not supported on this target
>  1568 | static unsigned __int128 btf_dump_bitfield_get_data(struct btf_dump *d,
>       |                 ^~~~~~~~
> btf_dump.c: In function 'btf_dump_bitfield_get_data':
> btf_dump.c:1576:18: error: '__int128' is not supported on this target
>  1576 |         unsigned __int128 num = 0, ret;
>       |                  ^~~~~~~~
> btf_dump.c: In function 'btf_dump_bitfield_check_zero':
> btf_dump.c:1608:9: error: expected expression before '__int128'
>  1608 |         __int128 check_num;
>       |         ^~~~~~~~
> btf_dump.c:1610:9: error: 'check_num' undeclared (first use in this function)
>  1610 |         check_num = btf_dump_bitfield_get_data(d, t, data,
> bits_offset, bit_sz);
>       |         ^~~~~~~~~
> btf_dump.c: In function 'btf_dump_bitfield_data':
> btf_dump.c:1622:18: error: '__int128' is not supported on this target
>  1622 |         unsigned __int128 print_num;
>       |                  ^~~~~~~~
> btf_dump.c: In function 'btf_dump_dump_type_data':
> btf_dump.c:2212:34: error: '__int128' is not supported on this target
>  2212 |                         unsigned __int128 print_num;
>       |                                  ^~~~~~~~
> 
>

Thanks for the report Naresh! Andrii, I'm thinking the best
approach might be to remove use of int128 and have the bitfield
computations operate on a __u64 representation instead.  With
that change, we would only lose the ability to handle int128
bitfields; what do you think? I hope to have something ready
shortly covering that, the non-propogation of return values
and the endianness issues with enum handling - in fact the
latter goes away if the bitfield computations are done for
64-bit values.

Thanks!

Alan

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-07-19 14:16 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-15 15:15 [PATCH v6 bpf-next 0/3] libbpf: BTF dumper support for typed data Alan Maguire
2021-07-15 15:15 ` [PATCH v6 bpf-next 1/3] " Alan Maguire
2021-07-16  6:24   ` Andrii Nakryiko
2021-07-16 20:55     ` Andrii Nakryiko
2021-07-16 21:58   ` Andrii Nakryiko
2021-07-19 12:11   ` Naresh Kamboju
2021-07-19 14:15     ` Alan Maguire
2021-07-15 15:15 ` [PATCH v6 bpf-next 2/3] selftests/bpf: add ASSERT_STRNEQ() variant for test_progs Alan Maguire
2021-07-15 15:15 ` [PATCH v6 bpf-next 3/3] selftests/bpf: add dump type data tests to btf dump tests Alan Maguire
2021-07-16  6:24   ` Andrii Nakryiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).