* [PATCH v3 bpf-next 1/8] bpf: provide function to get vmlinux BTF information
2020-06-23 12:07 [PATCH v3 bpf-next 0/8] bpf, printk: add BTF-based type printing Alan Maguire
@ 2020-06-23 12:07 ` Alan Maguire
2020-06-23 12:07 ` [PATCH v3 bpf-next 2/8] bpf: move to generic BTF show support, apply it to seq files/strings Alan Maguire
` (7 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Alan Maguire @ 2020-06-23 12:07 UTC (permalink / raw)
To: ast, daniel, yhs, andriin, arnaldo.melo
Cc: kafai, songliubraving, john.fastabend, kpsingh, linux, joe,
pmladek, rostedt, sergey.senozhatsky, andriy.shevchenko, corbet,
bpf, netdev, linux-kernel, linux-doc
It will be used later for BTF printk() support
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
include/linux/bpf.h | 2 ++
kernel/bpf/verifier.c | 18 ++++++++++++------
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 07052d4..a2ecebd 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1237,6 +1237,8 @@ int bpf_check(struct bpf_prog **fp, union bpf_attr *attr,
union bpf_attr __user *uattr);
void bpf_patch_call_args(struct bpf_insn *insn, u32 stack_depth);
+struct btf *bpf_get_btf_vmlinux(void);
+
/* Map specifics */
struct xdp_buff;
struct sk_buff;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index a1857c4..d448aa8 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -10878,6 +10878,17 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
}
}
+struct btf *bpf_get_btf_vmlinux(void)
+{
+ if (!btf_vmlinux && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) {
+ mutex_lock(&bpf_verifier_lock);
+ if (!btf_vmlinux)
+ btf_vmlinux = btf_parse_vmlinux();
+ mutex_unlock(&bpf_verifier_lock);
+ }
+ return btf_vmlinux;
+}
+
int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
union bpf_attr __user *uattr)
{
@@ -10911,12 +10922,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
env->ops = bpf_verifier_ops[env->prog->type];
is_priv = bpf_capable();
- if (!btf_vmlinux && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) {
- mutex_lock(&bpf_verifier_lock);
- if (!btf_vmlinux)
- btf_vmlinux = btf_parse_vmlinux();
- mutex_unlock(&bpf_verifier_lock);
- }
+ bpf_get_btf_vmlinux();
/* grab the mutex to protect few globals used by verifier */
if (!is_priv)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 bpf-next 2/8] bpf: move to generic BTF show support, apply it to seq files/strings
2020-06-23 12:07 [PATCH v3 bpf-next 0/8] bpf, printk: add BTF-based type printing Alan Maguire
2020-06-23 12:07 ` [PATCH v3 bpf-next 1/8] bpf: provide function to get vmlinux BTF information Alan Maguire
@ 2020-06-23 12:07 ` Alan Maguire
2020-06-23 12:07 ` [PATCH v3 bpf-next 3/8] checkpatch: add new BTF pointer format specifier Alan Maguire
` (6 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Alan Maguire @ 2020-06-23 12:07 UTC (permalink / raw)
To: ast, daniel, yhs, andriin, arnaldo.melo
Cc: kafai, songliubraving, john.fastabend, kpsingh, linux, joe,
pmladek, rostedt, sergey.senozhatsky, andriy.shevchenko, corbet,
bpf, netdev, linux-kernel, linux-doc
generalize the "seq_show" seq file support in btf.c to support
a generic show callback of which we support two instances; the
current seq file show, and a show with snprintf() behaviour which
instead writes the type data to a supplied string.
Both classes of show function call btf_type_show() with different
targets; the seq file or the string to be written. In the string
case we need to track additional data - length left in string to write
and length to return that we would have written (a la snprintf).
By default show will display type information, field members and
their types and values etc, and the information is indented
based upon structure depth. Zeroed fields are omitted.
Show however supports flags which modify its behaviour:
BTF_SHOW_COMPACT - suppress newline/indent.
BTF_SHOW_NONAME - suppress show of type and member names.
BTF_SHOW_PTR_RAW - do not obfuscate pointer values.
BTF_SHOW_UNSAFE - do not copy data to safe buffer before display.
BTF_SHOW_ZERO - show zeroed values (by default they are not shown).
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
include/linux/btf.h | 36 ++
kernel/bpf/btf.c | 966 ++++++++++++++++++++++++++++++++++++++++++++++------
2 files changed, 899 insertions(+), 103 deletions(-)
diff --git a/include/linux/btf.h b/include/linux/btf.h
index 5c1ea99..a8a4563 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -13,6 +13,7 @@
struct btf_member;
struct btf_type;
union bpf_attr;
+struct btf_show;
extern const struct file_operations btf_fops;
@@ -46,8 +47,43 @@ int btf_get_info_by_fd(const struct btf *btf,
const struct btf_type *btf_type_id_size(const struct btf *btf,
u32 *type_id,
u32 *ret_size);
+
+/*
+ * Options to control show behaviour.
+ * - BTF_SHOW_COMPACT: no formatting around type information
+ * - BTF_SHOW_NONAME: no struct/union member names/types
+ * - BTF_SHOW_PTR_RAW: show raw (unobfuscated) pointer values;
+ * equivalent to %px.
+ * - BTF_SHOW_ZERO: show zero-valued struct/union members; they
+ * are not displayed by default
+ * - BTF_SHOW_UNSAFE: skip use of bpf_probe_read() to safely read
+ * data before displaying it.
+ */
+#define BTF_SHOW_COMPACT (1ULL << 0)
+#define BTF_SHOW_NONAME (1ULL << 1)
+#define BTF_SHOW_PTR_RAW (1ULL << 2)
+#define BTF_SHOW_ZERO (1ULL << 3)
+#define BTF_SHOW_UNSAFE (1ULL << 4)
+
void btf_type_seq_show(const struct btf *btf, u32 type_id, void *obj,
struct seq_file *m);
+
+/*
+ * Copy len bytes of string representation of obj of BTF type_id into buf.
+ *
+ * @btf: struct btf object
+ * @type_id: type id of type obj points to
+ * @obj: pointer to typed data
+ * @buf: buffer to write to
+ * @len: maximum length to write to buf
+ * @flags: show options (see above)
+ *
+ * Return: length that would have been/was copied as per snprintf, or
+ * negative error.
+ */
+int btf_type_snprintf_show(const struct btf *btf, u32 type_id, void *obj,
+ char *buf, int len, u64 flags);
+
int btf_get_fd_by_id(u32 id);
u32 btf_id(const struct btf *btf);
bool btf_member_is_reg_int(const struct btf *btf, const struct btf_type *s,
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 58c9af1..c82cb18 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -281,6 +281,88 @@ static const char *btf_type_str(const struct btf_type *t)
return btf_kind_str[BTF_INFO_KIND(t->info)];
}
+/* Chunk size we use in safe copy of data to be shown. */
+#define BTF_SHOW_OBJ_SAFE_SIZE 256
+
+/*
+ * This is the maximum size of a base type value (equivalent to a
+ * 128-bit int); if we are at the end of our safe buffer and have
+ * less than 16 bytes space we can't be assured of being able
+ * to copy the next type safely, so in such cases we will initiate
+ * a new copy.
+ */
+#define BTF_SHOW_OBJ_BASE_TYPE_SIZE 16
+
+/*
+ * Common data to all BTF show operations. Private show functions can add
+ * their own data to a structure containing a struct btf_show and consult it
+ * in the show callback. See btf_type_show() below.
+ *
+ * One challenge with showing nested data is we want to skip 0-valued
+ * data, but in order to figure out whether a nested object is all zeros
+ * we need to walk through it. As a result, we need to make two passes
+ * when handling structs, unions and arrays; the first path simply looks
+ * for nonzero data, while the second actually does the display. The first
+ * pass is signalled by show->state.depth_check being set, and if we
+ * encounter a non-zero value we set show->state.depth_to_show to
+ * the depth at which we encountered it. When we have completed the
+ * first pass, we will know if anything needs to be displayed if
+ * depth_to_show > depth. See btf_[struct,array]_show() for the
+ * implementation of this.
+ *
+ * Another problem is we want to ensure the data for display is safe to
+ * access. To support this, the "struct obj" is used to track the data
+ * object and our safe copy of it. We copy portions of the data needed
+ * to the object "copy" buffer, but because its size is limited to
+ * BTF_SHOW_OBJ_COPY_LEN bytes, multiple copies may be required as we
+ * traverse larger objects for display.
+ *
+ * The various data type show functions all start with a call to
+ * btf_show_start_type() which returns a pointer to the safe copy
+ * of the data needed (or if BTF_SHOW_UNSAFE is specified, to the
+ * raw data itself). btf_show_obj_safe() is responsible for
+ * using probe_kernel_read() to update the safe data if necessary
+ * as we traverse the object's data. skbuff-like semantics are
+ * used:
+ *
+ * - obj.head points to the start of the toplevel object for display
+ * - obj.size is the size of the toplevel object
+ * - obj.data points to the current point in the original data at
+ * which our safe data starts. obj.data will advance as we copy
+ * portions of the data.
+ *
+ * In most cases a single copy will suffice, but larger data structures
+ * such as "struct task_struct" will require many copies. The logic in
+ * btf_show_obj_safe() handles the logic that determines if a new
+ * probe_kernel_read() is needed.
+ */
+struct btf_show {
+ u64 flags;
+ void *target; /* target of show operation (seq file, buffer) */
+ void (*showfn)(struct btf_show *show, const char *fmt, ...);
+ const struct btf *btf;
+ /* below are used during iteration */
+ struct {
+ u8 depth;
+ u8 depth_to_show;
+ u8 depth_check;
+ u8 array_member:1,
+ array_terminated:1;
+ u16 array_encoding;
+ u32 type_id;
+ int status; /* non-zero for error */
+ const struct btf_type *type;
+ const struct btf_member *member;
+ char name[KSYM_NAME_LEN]; /* space for member name/type */
+ } state;
+ struct {
+ u32 size;
+ void *head;
+ void *data;
+ u8 safe[BTF_SHOW_OBJ_SAFE_SIZE];
+ } obj;
+};
+
struct btf_kind_operations {
s32 (*check_meta)(struct btf_verifier_env *env,
const struct btf_type *t,
@@ -297,9 +379,9 @@ struct btf_kind_operations {
const struct btf_type *member_type);
void (*log_details)(struct btf_verifier_env *env,
const struct btf_type *t);
- void (*seq_show)(const struct btf *btf, const struct btf_type *t,
+ void (*show)(const struct btf *btf, const struct btf_type *t,
u32 type_id, void *data, u8 bits_offsets,
- struct seq_file *m);
+ struct btf_show *show);
};
static const struct btf_kind_operations * const kind_ops[NR_BTF_KINDS];
@@ -676,6 +758,457 @@ bool btf_member_is_reg_int(const struct btf *btf, const struct btf_type *s,
return true;
}
+/* Similar to btf_type_skip_modifiers() but does not skip typedefs. */
+static inline
+const struct btf_type *btf_type_skip_qualifiers(const struct btf *btf, u32 id)
+{
+ const struct btf_type *t = btf_type_by_id(btf, id);
+
+ while (btf_type_is_modifier(t) &&
+ BTF_INFO_KIND(t->info) != BTF_KIND_TYPEDEF) {
+ id = t->type;
+ t = btf_type_by_id(btf, t->type);
+ }
+
+ return t;
+}
+
+#define BTF_SHOW_MAX_ITER 10
+
+#define BTF_KIND_BIT(kind) (1ULL << kind)
+
+/*
+ * Populate show->state.name with type name information.
+ * Format of type name is
+ *
+ * [.member_name = ] (type_name)
+ */
+static inline const char *btf_show_name(struct btf_show *show)
+{
+ /* BTF_MAX_ITER array suffixes "[]" */
+ const char *array_suffixes = "[][][][][][][][][][]";
+ const char *array_suffix = &array_suffixes[strlen(array_suffixes)];
+ /* BTF_MAX_ITER pointer suffixes "*" */
+ const char *ptr_suffixes = "**********";
+ const char *ptr_suffix = &ptr_suffixes[strlen(ptr_suffixes)];
+ const char *name = NULL, *prefix = "", *parens = "";
+ const struct btf_member *m = show->state.member;
+ const struct btf_type *t = show->state.type;
+ const struct btf_array *array;
+ u32 id = show->state.type_id;
+ const char *member = NULL;
+ bool show_member = false;
+ u64 kinds = 0;
+ int i;
+
+ show->state.name[0] = '\0';
+
+ /*
+ * Don't show type name if we're showing an array member;
+ * in that case we show the array type so don't need to repeat
+ * ourselves for each member.
+ */
+ if (show->state.array_member)
+ return "";
+
+ /* Retrieve member name, if any. */
+ if (m) {
+ member = btf_name_by_offset(show->btf, m->name_off);
+ show_member = strlen(member) > 0;
+ id = m->type;
+ }
+
+ /*
+ * Start with type_id, as we have have resolved the struct btf_type *
+ * via btf_modifier_show() past the parent typedef to the child
+ * struct, int etc it is defined as. In such cases, the type_id
+ * still represents the starting type while the the struct btf_type *
+ * in our show->state points at the resolved type of the typedef.
+ */
+ t = btf_type_by_id(show->btf, id);
+ if (!t)
+ return "";
+
+ /*
+ * The goal here is to build up the right number of pointer and
+ * array suffixes while ensuring the type name for a typedef
+ * is represented. Along the way we accumulate a list of
+ * BTF kinds we have encountered, since these will inform later
+ * display; for example, pointer types will not require an
+ * opening "{" for struct, we will just display the pointer value.
+ *
+ * We also want to accumulate the right number of pointer or array
+ * indices in the format string while iterating until we get to
+ * the typedef/pointee/array member target type.
+ *
+ * We start by pointing at the end of pointer and array suffix
+ * strings; as we accumulate pointers and arrays we move the pointer
+ * or array string backwards so it will show the expected number of
+ * '*' or '[]' for the type. BTF_SHOW_MAX_ITER of nesting of pointers
+ * and/or arrays and typedefs are supported as a precaution.
+ *
+ * We also want to get typedef name while proceeding to resolve
+ * type it points to so that we can add parentheses if it is a
+ * "typedef struct" etc.
+ */
+ for (i = 0; i < BTF_SHOW_MAX_ITER; i++) {
+
+ switch (BTF_INFO_KIND(t->info)) {
+ case BTF_KIND_TYPEDEF:
+ if (!name)
+ name = btf_name_by_offset(show->btf,
+ t->name_off);
+ kinds |= BTF_KIND_BIT(BTF_KIND_TYPEDEF);
+ id = t->type;
+ break;
+ case BTF_KIND_ARRAY:
+ kinds |= BTF_KIND_BIT(BTF_KIND_ARRAY);
+ parens = "[";
+ if (!t)
+ return "";
+ array = btf_type_array(t);
+ if (array_suffix > array_suffixes)
+ array_suffix -= 2;
+ id = array->type;
+ break;
+ case BTF_KIND_PTR:
+ kinds |= BTF_KIND_BIT(BTF_KIND_PTR);
+ if (ptr_suffix > ptr_suffixes)
+ ptr_suffix -= 1;
+ id = t->type;
+ break;
+ default:
+ id = 0;
+ break;
+ }
+ if (!id)
+ break;
+ t = btf_type_skip_qualifiers(show->btf, id);
+ }
+ /* We may not be able to represent this type; bail to be safe */
+ if (i == BTF_SHOW_MAX_ITER)
+ return "";
+
+ if (!name)
+ name = btf_name_by_offset(show->btf, t->name_off);
+
+ switch (BTF_INFO_KIND(t->info)) {
+ case BTF_KIND_STRUCT:
+ case BTF_KIND_UNION:
+ prefix = BTF_INFO_KIND(t->info) == BTF_KIND_STRUCT ?
+ "struct" : "union";
+ /* if it's an array of struct/union, parens is already set */
+ if (!(kinds & (BTF_KIND_BIT(BTF_KIND_ARRAY))))
+ parens = "{";
+ break;
+ case BTF_KIND_ENUM:
+ prefix = "enum";
+ break;
+ default:
+ break;
+ }
+
+ /* pointer does not require parens */
+ if (kinds & BTF_KIND_BIT(BTF_KIND_PTR))
+ parens = "";
+ /* typedef does not require struct/union/enum prefix */
+ if (kinds & BTF_KIND_BIT(BTF_KIND_TYPEDEF))
+ prefix = "";
+
+ if (!name)
+ name = "";
+
+ /* Even if we don't want type name info, we want parentheses etc */
+ if (show->flags & BTF_SHOW_NONAME)
+ snprintf(show->state.name, sizeof(show->state.name), "%s",
+ parens);
+ else
+ snprintf(show->state.name, sizeof(show->state.name),
+ "%s%s%s(%s%s%s%s%s%s)%s",
+ /* first 3 strings comprise ".member = " */
+ show_member ? "." : "",
+ show_member ? member : "",
+ show_member ? " = " : "",
+ /* ...next is our prefix (struct, enum, etc) */
+ prefix,
+ strlen(prefix) > 0 && strlen(name) > 0 ? " " : "",
+ /* ...this is the type name itself */
+ name,
+ /* ...suffixed by the appropriate '*', '[]' suffixes */
+ strlen(ptr_suffix) > 0 ? " " : "", ptr_suffix,
+ array_suffix, parens);
+
+ return show->state.name;
+}
+
+#define btf_show(show, ...) \
+ do { \
+ if (!show->state.depth_check) \
+ show->showfn(show, __VA_ARGS__); \
+ } while (0)
+
+static inline const char *__btf_show_indent(struct btf_show *show)
+{
+ const char *indents = " ";
+ const char *indent = &indents[strlen(indents)];
+
+ if ((indent - show->state.depth) >= indents)
+ return indent - show->state.depth;
+ return indents;
+}
+
+#define btf_show_indent(show) \
+ ((show->flags & BTF_SHOW_COMPACT) ? "" : __btf_show_indent(show))
+
+#define btf_show_newline(show) \
+ ((show->flags & BTF_SHOW_COMPACT) ? "" : "\n")
+
+#define btf_show_delim(show) \
+ (show->state.depth == 0 ? "" : \
+ ((show->flags & BTF_SHOW_COMPACT) && show->state.type && \
+ BTF_INFO_KIND(show->state.type->info) == BTF_KIND_UNION) ? "|" : ",")
+
+#define btf_show_type_value(show, fmt, value) \
+ do { \
+ if ((value) != 0 || (show->flags & BTF_SHOW_ZERO) || \
+ show->state.depth == 0) { \
+ btf_show(show, "%s%s" fmt "%s%s", \
+ btf_show_indent(show), \
+ btf_show_name(show), \
+ value, btf_show_delim(show), \
+ btf_show_newline(show)); \
+ if (show->state.depth > show->state.depth_to_show) \
+ show->state.depth_to_show = show->state.depth; \
+ } \
+ } while (0)
+
+#define btf_show_type_values(show, fmt, ...) \
+ do { \
+ btf_show(show, "%s%s" fmt "%s%s", btf_show_indent(show), \
+ btf_show_name(show), \
+ __VA_ARGS__, btf_show_delim(show), \
+ btf_show_newline(show)); \
+ if (show->state.depth > show->state.depth_to_show) \
+ show->state.depth_to_show = show->state.depth; \
+ } while (0)
+
+/* How much is left to copy to safe buffer after @data? */
+#define btf_show_obj_size_left(show, data) \
+ (show->obj.head + show->obj.size - data)
+
+/* Is object pointed to by @data of @size already copied to our safe buffer? */
+#define btf_show_obj_is_safe(show, data, size) \
+ (data >= show->obj.data && \
+ (data + size) < (show->obj.data + BTF_SHOW_OBJ_SAFE_SIZE))
+
+/*
+ * If object pointed to by @data of @size falls within our safe buffer, return
+ * the equivalent pointer to the same safe data. Assumes probe_kernel_read()
+ * has already happened and our safe buffer is populated.
+ */
+#define __btf_show_obj_safe(show, data, size) \
+ (btf_show_obj_is_safe(show, data, size) ? \
+ show->obj.safe + (data - show->obj.data) : NULL)
+
+/*
+ * Return a safe-to-access version of data pointed to by @data.
+ * We do this by copying the relevant amount of information
+ * to the struct btf_show obj.safe buffer using probe_kernel_read().
+ *
+ * If BTF_SHOW_UNSAFE is specified, just return data as-is; no
+ * safe copy is needed.
+ *
+ * Otherwise we need to determine if we have the required amount
+ * of data (determined by the @data pointer and the size of the
+ * largest base type we can encounter (represented by
+ * BTF_SHOW_OBJ_BASE_TYPE_SIZE). Having that much data ensures
+ * that we will be able to print some of the current object,
+ * and if more is needed a copy will be triggered.
+ * Some objects such as structs will not fit into the buffer;
+ * in such cases additional copies when we iterate over their
+ * members may be needed.
+ *
+ * btf_show_obj_safe() is used to return a safe buffer for
+ * btf_show_start_type(); this ensures that as we recurse into
+ * nested types we always have safe data for the given type.
+ * This approach is somewhat wasteful; it's possible for example
+ * that when iterating over a large union we'll end up copying the
+ * same data repeatedly, but the goal is safety not performance.
+ * We use stack data as opposed to per-CPU buffers because the
+ * iteration over a type can take some time, and preemption handling
+ * would greatly complicate use of the safe buffer.
+ */
+static inline void *btf_show_obj_safe(struct btf_show *show,
+ const struct btf_type *t,
+ void *data)
+{
+ int size_left, size;
+ void *safe = NULL;
+
+ if (show->flags & BTF_SHOW_UNSAFE)
+ return data;
+
+ (void) btf_resolve_size(show->btf, t, &size, NULL, NULL);
+
+ /*
+ * Is this toplevel object? If so, set total object size and
+ * initialize pointers. Otherwise check if we still fall within
+ * our safe object data.
+ */
+ if (show->state.depth == 0) {
+ show->obj.size = size;
+ show->obj.head = data;
+ } else {
+ /*
+ * If the size of the current object is > our remaining
+ * safe buffer we _may_ need to do a new copy. However
+ * consider the case of a nested struct; it's size pushes
+ * us over the safe buffer limit, but showing any individual
+ * struct members does not. In such cases, we don't need
+ * to initiate a fresh copy yet; however we definitely need
+ * at least BTF_SHOW_OBJ_BASE_TYPE_SIZE bytes left
+ * in our buffer, regardless of the current object size.
+ * The logic here is that as we resolve types we will
+ * hit a base type at some point, and we need to be sure
+ * the next chunk of data is safely available to display
+ * that type info safely. We cannot rely on the size of
+ * the current object here because it may be much larger
+ * than our current buffer (e.g. task_struct is 8k).
+ * All we want to do here is ensure that we can print the
+ * next basic type, which we can if either
+ * - the current type size is within the safe buffer; or
+ * - at least BTF_SHOW_OBJ_BASE_TYPE_SIZE bytes are left in
+ * the safe buffer.
+ */
+ safe = __btf_show_obj_safe(show, data,
+ min(size,
+ BTF_SHOW_OBJ_BASE_TYPE_SIZE));
+ }
+
+ /*
+ * We need a new copy to our safe object, either because we haven't
+ * yet copied and are intializing safe data, or because the data
+ * we want falls outside the boundaries of the safe object.
+ */
+ if (!safe) {
+ size_left = btf_show_obj_size_left(show, data);
+ if (size_left > BTF_SHOW_OBJ_SAFE_SIZE)
+ size_left = BTF_SHOW_OBJ_SAFE_SIZE;
+ show->state.status = probe_kernel_read(show->obj.safe,
+ data,
+ size_left);
+ if (!show->state.status) {
+ show->obj.data = data;
+ safe = show->obj.safe;
+ }
+ }
+
+ return safe;
+}
+
+/*
+ * Set the type we are starting to show and return a safe data pointer
+ * to be used for showing the associated data.
+ */
+static inline void *btf_show_start_type(struct btf_show *show,
+ const struct btf_type *t,
+ u32 type_id,
+ void *data)
+{
+ show->state.type = t;
+ show->state.type_id = type_id;
+ show->state.name[0] = '\0';
+
+ return btf_show_obj_safe(show, t, data);
+}
+
+static inline void btf_show_end_type(struct btf_show *show)
+{
+ show->state.type = NULL;
+ show->state.type_id = 0;
+ show->state.name[0] = '\0';
+}
+
+static inline void *btf_show_start_aggr_type(struct btf_show *show,
+ const struct btf_type *t,
+ u32 type_id,
+ void *data)
+{
+ void *safe_data = btf_show_start_type(show, t, type_id, data);
+
+ if (!safe_data)
+ return safe_data;
+
+ btf_show(show, "%s%s%s", btf_show_indent(show),
+ btf_show_name(show),
+ btf_show_newline(show));
+ show->state.depth++;
+ return safe_data;
+}
+
+static inline void btf_show_end_aggr_type(struct btf_show *show,
+ const char *suffix)
+{
+ show->state.depth--;
+ btf_show(show, "%s%s%s%s", btf_show_indent(show), suffix,
+ btf_show_delim(show), btf_show_newline(show));
+ btf_show_end_type(show);
+}
+
+static inline void btf_show_start_member(struct btf_show *show,
+ const struct btf_member *m)
+{
+ show->state.member = m;
+}
+
+static inline void btf_show_start_array_member(struct btf_show *show)
+{
+ show->state.array_member = 1;
+ btf_show_start_member(show, NULL);
+}
+
+static inline void btf_show_end_member(struct btf_show *show)
+{
+ show->state.member = NULL;
+}
+
+static inline void btf_show_end_array_member(struct btf_show *show)
+{
+ show->state.array_member = 0;
+ btf_show_end_member(show);
+}
+
+static inline void *btf_show_start_array_type(struct btf_show *show,
+ const struct btf_type *t,
+ u32 type_id,
+ u16 array_encoding,
+ void *data)
+{
+ show->state.array_encoding = array_encoding;
+ show->state.array_terminated = 0;
+ return btf_show_start_aggr_type(show, t, type_id, data);
+}
+
+static inline void btf_show_end_array_type(struct btf_show *show)
+{
+ show->state.array_encoding = 0;
+ show->state.array_terminated = 0;
+ btf_show_end_aggr_type(show, "]");
+}
+
+static inline void *btf_show_start_struct_type(struct btf_show *show,
+ const struct btf_type *t,
+ u32 type_id,
+ void *data)
+{
+ return btf_show_start_aggr_type(show, t, type_id, data);
+}
+
+static inline void btf_show_end_struct_type(struct btf_show *show)
+{
+ btf_show_end_aggr_type(show, "}");
+}
+
__printf(2, 3) static void __btf_verifier_log(struct bpf_verifier_log *log,
const char *fmt, ...)
{
@@ -1249,11 +1782,11 @@ static int btf_df_resolve(struct btf_verifier_env *env,
return -EINVAL;
}
-static void btf_df_seq_show(const struct btf *btf, const struct btf_type *t,
- u32 type_id, void *data, u8 bits_offsets,
- struct seq_file *m)
+static void btf_df_show(const struct btf *btf, const struct btf_type *t,
+ u32 type_id, void *data, u8 bits_offsets,
+ struct btf_show *show)
{
- seq_printf(m, "<unsupported kind:%u>", BTF_INFO_KIND(t->info));
+ btf_show(show, "<unsupported kind:%u>", BTF_INFO_KIND(t->info));
}
static int btf_int_check_member(struct btf_verifier_env *env,
@@ -1426,7 +1959,7 @@ static void btf_int_log(struct btf_verifier_env *env,
btf_int_encoding_str(BTF_INT_ENCODING(int_data)));
}
-static void btf_int128_print(struct seq_file *m, void *data)
+static void btf_int128_print(struct btf_show *show, void *data)
{
/* data points to a __int128 number.
* Suppose
@@ -1445,9 +1978,10 @@ static void btf_int128_print(struct seq_file *m, void *data)
lower_num = *(u64 *)data;
#endif
if (upper_num == 0)
- seq_printf(m, "0x%llx", lower_num);
+ btf_show_type_value(show, "0x%llx", lower_num);
else
- seq_printf(m, "0x%llx%016llx", upper_num, lower_num);
+ btf_show_type_values(show, "0x%llx%016llx", upper_num,
+ lower_num);
}
static void btf_int128_shift(u64 *print_num, u16 left_shift_bits,
@@ -1491,8 +2025,8 @@ static void btf_int128_shift(u64 *print_num, u16 left_shift_bits,
#endif
}
-static void btf_bitfield_seq_show(void *data, u8 bits_offset,
- u8 nr_bits, struct seq_file *m)
+static void btf_bitfield_show(void *data, u8 bits_offset,
+ u8 nr_bits, struct btf_show *show)
{
u16 left_shift_bits, right_shift_bits;
u8 nr_copy_bytes;
@@ -1512,14 +2046,14 @@ static void btf_bitfield_seq_show(void *data, u8 bits_offset,
right_shift_bits = BITS_PER_U128 - nr_bits;
btf_int128_shift(print_num, left_shift_bits, right_shift_bits);
- btf_int128_print(m, print_num);
+ btf_int128_print(show, print_num);
}
-static void btf_int_bits_seq_show(const struct btf *btf,
- const struct btf_type *t,
- void *data, u8 bits_offset,
- struct seq_file *m)
+static void btf_int_bits_show(const struct btf *btf,
+ const struct btf_type *t,
+ void *data, u8 bits_offset,
+ struct btf_show *show)
{
u32 int_data = btf_type_int(t);
u8 nr_bits = BTF_INT_BITS(int_data);
@@ -1532,55 +2066,77 @@ static void btf_int_bits_seq_show(const struct btf *btf,
total_bits_offset = bits_offset + BTF_INT_OFFSET(int_data);
data += BITS_ROUNDDOWN_BYTES(total_bits_offset);
bits_offset = BITS_PER_BYTE_MASKED(total_bits_offset);
- btf_bitfield_seq_show(data, bits_offset, nr_bits, m);
+ btf_bitfield_show(data, bits_offset, nr_bits, show);
}
-static void btf_int_seq_show(const struct btf *btf, const struct btf_type *t,
- u32 type_id, void *data, u8 bits_offset,
- struct seq_file *m)
+static void btf_int_show(const struct btf *btf, const struct btf_type *t,
+ u32 type_id, void *data, u8 bits_offset,
+ struct btf_show *show)
{
u32 int_data = btf_type_int(t);
u8 encoding = BTF_INT_ENCODING(int_data);
bool sign = encoding & BTF_INT_SIGNED;
u8 nr_bits = BTF_INT_BITS(int_data);
+ void *safe_data;
+
+ safe_data = btf_show_start_type(show, t, type_id, data);
+ if (!safe_data)
+ return;
if (bits_offset || BTF_INT_OFFSET(int_data) ||
BITS_PER_BYTE_MASKED(nr_bits)) {
- btf_int_bits_seq_show(btf, t, data, bits_offset, m);
- return;
+ btf_int_bits_show(btf, t, safe_data, bits_offset, show);
+ goto out;
}
switch (nr_bits) {
case 128:
- btf_int128_print(m, data);
+ btf_int128_print(show, safe_data);
break;
case 64:
if (sign)
- seq_printf(m, "%lld", *(s64 *)data);
+ btf_show_type_value(show, "%lld", *(s64 *)safe_data);
else
- seq_printf(m, "%llu", *(u64 *)data);
+ btf_show_type_value(show, "%llu", *(u64 *)safe_data);
break;
case 32:
if (sign)
- seq_printf(m, "%d", *(s32 *)data);
+ btf_show_type_value(show, "%d", *(s32 *)safe_data);
else
- seq_printf(m, "%u", *(u32 *)data);
+ btf_show_type_value(show, "%u", *(u32 *)safe_data);
break;
case 16:
if (sign)
- seq_printf(m, "%d", *(s16 *)data);
+ btf_show_type_value(show, "%d", *(s16 *)safe_data);
else
- seq_printf(m, "%u", *(u16 *)data);
+ btf_show_type_value(show, "%u", *(u16 *)safe_data);
break;
case 8:
+ if (show->state.array_encoding == BTF_INT_CHAR) {
+ /* check for null terminator */
+ if (show->state.array_terminated)
+ break;
+ if (*(char *)data == '\0') {
+ show->state.array_terminated = 1;
+ break;
+ }
+ if (isprint(*(char *)data)) {
+ btf_show_type_value(show, "'%c'",
+ *(char *)safe_data);
+ break;
+ }
+ }
if (sign)
- seq_printf(m, "%d", *(s8 *)data);
+ btf_show_type_value(show, "%d", *(s8 *)safe_data);
else
- seq_printf(m, "%u", *(u8 *)data);
+ btf_show_type_value(show, "%u", *(u8 *)safe_data);
break;
default:
- btf_int_bits_seq_show(btf, t, data, bits_offset, m);
+ btf_int_bits_show(btf, t, safe_data, bits_offset, show);
+ break;
}
+out:
+ btf_show_end_type(show);
}
static const struct btf_kind_operations int_ops = {
@@ -1589,7 +2145,7 @@ static void btf_int_seq_show(const struct btf *btf, const struct btf_type *t,
.check_member = btf_int_check_member,
.check_kflag_member = btf_int_check_kflag_member,
.log_details = btf_int_log,
- .seq_show = btf_int_seq_show,
+ .show = btf_int_show,
};
static int btf_modifier_check_member(struct btf_verifier_env *env,
@@ -1853,34 +2409,44 @@ static int btf_ptr_resolve(struct btf_verifier_env *env,
return 0;
}
-static void btf_modifier_seq_show(const struct btf *btf,
- const struct btf_type *t,
- u32 type_id, void *data,
- u8 bits_offset, struct seq_file *m)
+static void btf_modifier_show(const struct btf *btf,
+ const struct btf_type *t,
+ u32 type_id, void *data,
+ u8 bits_offset, struct btf_show *show)
{
if (btf->resolved_ids)
t = btf_type_id_resolve(btf, &type_id);
else
t = btf_type_skip_modifiers(btf, type_id, NULL);
- btf_type_ops(t)->seq_show(btf, t, type_id, data, bits_offset, m);
+ btf_type_ops(t)->show(btf, t, type_id, data, bits_offset, show);
}
-static void btf_var_seq_show(const struct btf *btf, const struct btf_type *t,
- u32 type_id, void *data, u8 bits_offset,
- struct seq_file *m)
+static void btf_var_show(const struct btf *btf, const struct btf_type *t,
+ u32 type_id, void *data, u8 bits_offset,
+ struct btf_show *show)
{
t = btf_type_id_resolve(btf, &type_id);
- btf_type_ops(t)->seq_show(btf, t, type_id, data, bits_offset, m);
+ btf_type_ops(t)->show(btf, t, type_id, data, bits_offset, show);
}
-static void btf_ptr_seq_show(const struct btf *btf, const struct btf_type *t,
- u32 type_id, void *data, u8 bits_offset,
- struct seq_file *m)
+static void btf_ptr_show(const struct btf *btf, const struct btf_type *t,
+ u32 type_id, void *data, u8 bits_offset,
+ struct btf_show *show)
{
- /* It is a hashed value */
- seq_printf(m, "%p", *(void **)data);
+ void *safe_data;
+
+ safe_data = btf_show_start_type(show, t, type_id, data);
+ if (!safe_data)
+ return;
+
+ /* It is a hashed value unless BTF_SHOW_PTR_RAW is specified */
+ if (show->flags & BTF_SHOW_PTR_RAW)
+ btf_show_type_value(show, "0x%px", *(void **)safe_data);
+ else
+ btf_show_type_value(show, "0x%p", *(void **)safe_data);
+ btf_show_end_type(show);
}
static void btf_ref_type_log(struct btf_verifier_env *env,
@@ -1895,7 +2461,7 @@ static void btf_ref_type_log(struct btf_verifier_env *env,
.check_member = btf_modifier_check_member,
.check_kflag_member = btf_modifier_check_kflag_member,
.log_details = btf_ref_type_log,
- .seq_show = btf_modifier_seq_show,
+ .show = btf_modifier_show,
};
static struct btf_kind_operations ptr_ops = {
@@ -1904,7 +2470,7 @@ static void btf_ref_type_log(struct btf_verifier_env *env,
.check_member = btf_ptr_check_member,
.check_kflag_member = btf_generic_check_kflag_member,
.log_details = btf_ref_type_log,
- .seq_show = btf_ptr_seq_show,
+ .show = btf_ptr_show,
};
static s32 btf_fwd_check_meta(struct btf_verifier_env *env,
@@ -1945,7 +2511,7 @@ static void btf_fwd_type_log(struct btf_verifier_env *env,
.check_member = btf_df_check_member,
.check_kflag_member = btf_df_check_kflag_member,
.log_details = btf_fwd_type_log,
- .seq_show = btf_df_seq_show,
+ .show = btf_df_show,
};
static int btf_array_check_member(struct btf_verifier_env *env,
@@ -2104,28 +2670,90 @@ static void btf_array_log(struct btf_verifier_env *env,
array->type, array->index_type, array->nelems);
}
-static void btf_array_seq_show(const struct btf *btf, const struct btf_type *t,
- u32 type_id, void *data, u8 bits_offset,
- struct seq_file *m)
+static void __btf_array_show(const struct btf *btf, const struct btf_type *t,
+ u32 type_id, void *data, u8 bits_offset,
+ struct btf_show *show)
{
const struct btf_array *array = btf_type_array(t);
const struct btf_kind_operations *elem_ops;
const struct btf_type *elem_type;
- u32 i, elem_size, elem_type_id;
+ u32 i, elem_size = 0, elem_type_id;
+ u16 encoding = 0;
elem_type_id = array->type;
- elem_type = btf_type_id_size(btf, &elem_type_id, &elem_size);
+ elem_type = btf_type_skip_modifiers(btf, elem_type_id, NULL);
+ if (elem_type && btf_type_has_size(elem_type))
+ elem_size = elem_type->size;
+
+ if (elem_type && btf_type_is_int(elem_type)) {
+ u32 int_type = btf_type_int(elem_type);
+
+ encoding = BTF_INT_ENCODING(int_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)
+ encoding = BTF_INT_CHAR;
+ }
+
+ if (!btf_show_start_array_type(show, t, type_id, encoding, data))
+ return;
+
+ if (!elem_type)
+ goto out;
elem_ops = btf_type_ops(elem_type);
- seq_puts(m, "[");
+
for (i = 0; i < array->nelems; i++) {
- if (i)
- seq_puts(m, ",");
- elem_ops->seq_show(btf, elem_type, elem_type_id, data,
- bits_offset, m);
+ btf_show_start_array_member(show);
+
+ elem_ops->show(btf, elem_type, elem_type_id, data,
+ bits_offset, show);
data += elem_size;
+
+ btf_show_end_array_member(show);
+
+ if (show->state.array_terminated)
+ break;
}
- seq_puts(m, "]");
+out:
+ btf_show_end_array_type(show);
+}
+
+static void btf_array_show(const struct btf *btf, const struct btf_type *t,
+ u32 type_id, void *data, u8 bits_offset,
+ struct btf_show *show)
+{
+ const struct btf_member *m = show->state.member;
+
+ /*
+ * First check if any members would be shown (are non-zero).
+ * See comments above "struct btf_show" definition for more
+ * details on how this works at a high-level.
+ */
+ if (show->state.depth > 0 && !(show->flags & BTF_SHOW_ZERO)) {
+ if (!show->state.depth_check) {
+ show->state.depth_check = show->state.depth + 1;
+ show->state.depth_to_show = 0;
+ }
+ __btf_array_show(btf, t, type_id, data, bits_offset, show);
+ show->state.member = m;
+
+ if (show->state.depth_check != show->state.depth + 1)
+ return;
+ show->state.depth_check = 0;
+
+ if (show->state.depth_to_show <= show->state.depth)
+ return;
+ /*
+ * Reaching here indicates we have recursed and found
+ * non-zero array member(s).
+ */
+ }
+ __btf_array_show(btf, t, type_id, data, bits_offset, show);
}
static struct btf_kind_operations array_ops = {
@@ -2134,7 +2762,7 @@ static void btf_array_seq_show(const struct btf *btf, const struct btf_type *t,
.check_member = btf_array_check_member,
.check_kflag_member = btf_generic_check_kflag_member,
.log_details = btf_array_log,
- .seq_show = btf_array_seq_show,
+ .show = btf_array_show,
};
static int btf_struct_check_member(struct btf_verifier_env *env,
@@ -2357,15 +2985,18 @@ int btf_find_spin_lock(const struct btf *btf, const struct btf_type *t)
return off;
}
-static void btf_struct_seq_show(const struct btf *btf, const struct btf_type *t,
- u32 type_id, void *data, u8 bits_offset,
- struct seq_file *m)
+static void __btf_struct_show(const struct btf *btf, const struct btf_type *t,
+ u32 type_id, void *data, u8 bits_offset,
+ struct btf_show *show)
{
- const char *seq = BTF_INFO_KIND(t->info) == BTF_KIND_UNION ? "|" : ",";
const struct btf_member *member;
+ void *safe_data;
u32 i;
- seq_puts(m, "{");
+ safe_data = btf_show_start_struct_type(show, t, type_id, data);
+ if (!safe_data)
+ return;
+
for_each_member(i, t, member) {
const struct btf_type *member_type = btf_type_by_id(btf,
member->type);
@@ -2374,23 +3005,60 @@ static void btf_struct_seq_show(const struct btf *btf, const struct btf_type *t,
u32 bytes_offset;
u8 bits8_offset;
- if (i)
- seq_puts(m, seq);
+ btf_show_start_member(show, member);
member_offset = btf_member_bit_offset(t, member);
bitfield_size = btf_member_bitfield_size(t, member);
bytes_offset = BITS_ROUNDDOWN_BYTES(member_offset);
bits8_offset = BITS_PER_BYTE_MASKED(member_offset);
if (bitfield_size) {
- btf_bitfield_seq_show(data + bytes_offset, bits8_offset,
- bitfield_size, m);
+ btf_bitfield_show(safe_data + bytes_offset,
+ bits8_offset,
+ bitfield_size, show);
} else {
ops = btf_type_ops(member_type);
- ops->seq_show(btf, member_type, member->type,
- data + bytes_offset, bits8_offset, m);
+ ops->show(btf, member_type, member->type,
+ data + bytes_offset, bits8_offset, show);
}
+
+ btf_show_end_member(show);
}
- seq_puts(m, "}");
+
+ btf_show_end_struct_type(show);
+}
+
+static void btf_struct_show(const struct btf *btf, const struct btf_type *t,
+ u32 type_id, void *data, u8 bits_offset,
+ struct btf_show *show)
+{
+ const struct btf_member *m = show->state.member;
+
+ /*
+ * First check if any members would be shown (are non-zero).
+ * See comments above "struct btf_show" definition for more
+ * details on how this works at a high-level.
+ */
+ if (show->state.depth > 0 && !(show->flags & BTF_SHOW_ZERO)) {
+ if (!show->state.depth_check) {
+ show->state.depth_check = show->state.depth + 1;
+ show->state.depth_to_show = 0;
+ }
+ __btf_struct_show(btf, t, type_id, data, bits_offset, show);
+ /* Restore saved member data here */
+ show->state.member = m;
+ if (show->state.depth_check != show->state.depth + 1)
+ return;
+ show->state.depth_check = 0;
+
+ if (show->state.depth_to_show <= show->state.depth)
+ return;
+ /*
+ * Reaching here indicates we have recursed and found
+ * non-zero child values.
+ */
+ }
+
+ __btf_struct_show(btf, t, type_id, data, bits_offset, show);
}
static struct btf_kind_operations struct_ops = {
@@ -2399,7 +3067,7 @@ static void btf_struct_seq_show(const struct btf *btf, const struct btf_type *t,
.check_member = btf_struct_check_member,
.check_kflag_member = btf_generic_check_kflag_member,
.log_details = btf_struct_log,
- .seq_show = btf_struct_seq_show,
+ .show = btf_struct_show,
};
static int btf_enum_check_member(struct btf_verifier_env *env,
@@ -2530,24 +3198,35 @@ static void btf_enum_log(struct btf_verifier_env *env,
btf_verifier_log(env, "size=%u vlen=%u", t->size, btf_type_vlen(t));
}
-static void btf_enum_seq_show(const struct btf *btf, const struct btf_type *t,
- u32 type_id, void *data, u8 bits_offset,
- struct seq_file *m)
+static void btf_enum_show(const struct btf *btf, const struct btf_type *t,
+ u32 type_id, void *data, u8 bits_offset,
+ struct btf_show *show)
{
const struct btf_enum *enums = btf_type_enum(t);
u32 i, nr_enums = btf_type_vlen(t);
- int v = *(int *)data;
+ void *safe_data;
+ int v;
+
+ safe_data = btf_show_start_type(show, t, type_id, data);
+ if (!safe_data)
+ return;
+
+ v = *(int *)safe_data;
for (i = 0; i < nr_enums; i++) {
- if (v == enums[i].val) {
- seq_printf(m, "%s",
- __btf_name_by_offset(btf,
- enums[i].name_off));
- return;
- }
+ if (v != enums[i].val)
+ continue;
+
+ btf_show_type_value(show, "%s",
+ __btf_name_by_offset(btf,
+ enums[i].name_off));
+
+ btf_show_end_type(show);
+ return;
}
- seq_printf(m, "%d", v);
+ btf_show_type_value(show, "%d", v);
+ btf_show_end_type(show);
}
static struct btf_kind_operations enum_ops = {
@@ -2556,7 +3235,7 @@ static void btf_enum_seq_show(const struct btf *btf, const struct btf_type *t,
.check_member = btf_enum_check_member,
.check_kflag_member = btf_enum_check_kflag_member,
.log_details = btf_enum_log,
- .seq_show = btf_enum_seq_show,
+ .show = btf_enum_show,
};
static s32 btf_func_proto_check_meta(struct btf_verifier_env *env,
@@ -2643,7 +3322,7 @@ static void btf_func_proto_log(struct btf_verifier_env *env,
.check_member = btf_df_check_member,
.check_kflag_member = btf_df_check_kflag_member,
.log_details = btf_func_proto_log,
- .seq_show = btf_df_seq_show,
+ .show = btf_df_show,
};
static s32 btf_func_check_meta(struct btf_verifier_env *env,
@@ -2677,7 +3356,7 @@ static s32 btf_func_check_meta(struct btf_verifier_env *env,
.check_member = btf_df_check_member,
.check_kflag_member = btf_df_check_kflag_member,
.log_details = btf_ref_type_log,
- .seq_show = btf_df_seq_show,
+ .show = btf_df_show,
};
static s32 btf_var_check_meta(struct btf_verifier_env *env,
@@ -2741,7 +3420,7 @@ static void btf_var_log(struct btf_verifier_env *env, const struct btf_type *t)
.check_member = btf_df_check_member,
.check_kflag_member = btf_df_check_kflag_member,
.log_details = btf_var_log,
- .seq_show = btf_var_seq_show,
+ .show = btf_var_show,
};
static s32 btf_datasec_check_meta(struct btf_verifier_env *env,
@@ -2867,24 +3546,28 @@ static void btf_datasec_log(struct btf_verifier_env *env,
btf_verifier_log(env, "size=%u vlen=%u", t->size, btf_type_vlen(t));
}
-static void btf_datasec_seq_show(const struct btf *btf,
- const struct btf_type *t, u32 type_id,
- void *data, u8 bits_offset,
- struct seq_file *m)
+static void btf_datasec_show(const struct btf *btf,
+ const struct btf_type *t, u32 type_id,
+ void *data, u8 bits_offset,
+ struct btf_show *show)
{
const struct btf_var_secinfo *vsi;
const struct btf_type *var;
u32 i;
- seq_printf(m, "section (\"%s\") = {", __btf_name_by_offset(btf, t->name_off));
+ if (!btf_show_start_type(show, t, type_id, data))
+ return;
+
+ btf_show_type_value(show, "section (\"%s\") = {",
+ __btf_name_by_offset(btf, t->name_off));
for_each_vsi(i, t, vsi) {
var = btf_type_by_id(btf, vsi->type);
if (i)
- seq_puts(m, ",");
- btf_type_ops(var)->seq_show(btf, var, vsi->type,
- data + vsi->offset, bits_offset, m);
+ btf_show(show, ",");
+ btf_type_ops(var)->show(btf, var, vsi->type,
+ data + vsi->offset, bits_offset, show);
}
- seq_puts(m, "}");
+ btf_show_end_type(show);
}
static const struct btf_kind_operations datasec_ops = {
@@ -2893,7 +3576,7 @@ static void btf_datasec_seq_show(const struct btf *btf,
.check_member = btf_df_check_member,
.check_kflag_member = btf_df_check_kflag_member,
.log_details = btf_datasec_log,
- .seq_show = btf_datasec_seq_show,
+ .show = btf_datasec_show,
};
static int btf_func_proto_check(struct btf_verifier_env *env,
@@ -4554,12 +5237,89 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog,
return 0;
}
-void btf_type_seq_show(const struct btf *btf, u32 type_id, void *obj,
- struct seq_file *m)
+void btf_type_show(const struct btf *btf, u32 type_id, void *obj,
+ struct btf_show *show)
{
const struct btf_type *t = btf_type_by_id(btf, type_id);
- btf_type_ops(t)->seq_show(btf, t, type_id, obj, 0, m);
+ show->btf = btf;
+ memset(&show->state, 0, sizeof(show->state));
+ memset(&show->obj, 0, sizeof(show->obj));
+
+ btf_type_ops(t)->show(btf, t, type_id, obj, 0, show);
+}
+
+static void btf_seq_show(struct btf_show *show, const char *fmt, ...)
+{
+ va_list args;
+
+ va_start(args, fmt);
+ seq_vprintf((struct seq_file *)show->target, fmt, args);
+ va_end(args);
+}
+
+void btf_type_seq_show(const struct btf *btf, u32 type_id, void *obj,
+ struct seq_file *m)
+{
+ struct btf_show sseq;
+
+ sseq.target = m;
+ sseq.showfn = btf_seq_show;
+ sseq.flags = BTF_SHOW_NONAME | BTF_SHOW_COMPACT | BTF_SHOW_ZERO |
+ BTF_SHOW_UNSAFE;
+
+ btf_type_show(btf, type_id, obj, &sseq);
+}
+
+struct btf_show_snprintf {
+ struct btf_show show;
+ int len_left; /* space left in string */
+ int len; /* length we would have written */
+};
+
+static void btf_snprintf_show(struct btf_show *show, const char *fmt, ...)
+{
+ struct btf_show_snprintf *ssnprintf = (struct btf_show_snprintf *)show;
+ va_list args;
+ int len;
+
+ va_start(args, fmt);
+ len = vsnprintf(show->target, ssnprintf->len_left, fmt, args);
+ va_end(args);
+
+ if (len < 0) {
+ ssnprintf->len_left = 0;
+ ssnprintf->len = len;
+ } else if (len > ssnprintf->len_left) {
+ /* no space, drive on to get length we would have written */
+ ssnprintf->len_left = 0;
+ ssnprintf->len += len;
+ } else {
+ ssnprintf->len_left -= len;
+ ssnprintf->len += len;
+ show->target += len;
+ }
+}
+
+int btf_type_snprintf_show(const struct btf *btf, u32 type_id, void *obj,
+ char *buf, int len, u64 flags)
+{
+ struct btf_show_snprintf ssnprintf;
+
+ ssnprintf.show.target = buf;
+ ssnprintf.show.flags = flags;
+ ssnprintf.show.showfn = btf_snprintf_show;
+ ssnprintf.len_left = len;
+ ssnprintf.len = 0;
+
+ btf_type_show(btf, type_id, obj, (struct btf_show *)&ssnprintf);
+
+ /* If we encontered an error, return it. */
+ if (ssnprintf.show.state.status)
+ return ssnprintf.show.state.status;
+
+ /* Otherwise return length we would have written */
+ return ssnprintf.len;
}
#ifdef CONFIG_PROC_FS
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 bpf-next 3/8] checkpatch: add new BTF pointer format specifier
2020-06-23 12:07 [PATCH v3 bpf-next 0/8] bpf, printk: add BTF-based type printing Alan Maguire
2020-06-23 12:07 ` [PATCH v3 bpf-next 1/8] bpf: provide function to get vmlinux BTF information Alan Maguire
2020-06-23 12:07 ` [PATCH v3 bpf-next 2/8] bpf: move to generic BTF show support, apply it to seq files/strings Alan Maguire
@ 2020-06-23 12:07 ` Alan Maguire
2020-06-23 12:07 ` [PATCH v3 bpf-next 4/8] printk: add type-printing %pT format specifier which uses BTF Alan Maguire
` (5 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Alan Maguire @ 2020-06-23 12:07 UTC (permalink / raw)
To: ast, daniel, yhs, andriin, arnaldo.melo
Cc: kafai, songliubraving, john.fastabend, kpsingh, linux, joe,
pmladek, rostedt, sergey.senozhatsky, andriy.shevchenko, corbet,
bpf, netdev, linux-kernel, linux-doc
checkpatch complains about unknown format specifiers, so add
the BTF format specifier we will implement in a subsequent
patch to avoid errors.
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
scripts/checkpatch.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4c82060..e89631e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6148,7 +6148,7 @@ sub process {
$specifier = $1;
$extension = $2;
$qualifier = $3;
- if ($extension !~ /[SsBKRraEehMmIiUDdgVCbGNOxtf]/ ||
+ if ($extension !~ /[SsBKRraEehMmIiUDdgVCbGNOxtfT]/ ||
($extension eq "f" &&
defined $qualifier && $qualifier !~ /^w/)) {
$bad_specifier = $specifier;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 bpf-next 4/8] printk: add type-printing %pT format specifier which uses BTF
2020-06-23 12:07 [PATCH v3 bpf-next 0/8] bpf, printk: add BTF-based type printing Alan Maguire
` (2 preceding siblings ...)
2020-06-23 12:07 ` [PATCH v3 bpf-next 3/8] checkpatch: add new BTF pointer format specifier Alan Maguire
@ 2020-06-23 12:07 ` Alan Maguire
2020-06-23 12:40 ` Andy Shevchenko
` (2 more replies)
2020-06-23 12:07 ` [PATCH v3 bpf-next 5/8] printk: initialize vmlinux BTF outside of printk in late_initcall() Alan Maguire
` (4 subsequent siblings)
8 siblings, 3 replies; 18+ messages in thread
From: Alan Maguire @ 2020-06-23 12:07 UTC (permalink / raw)
To: ast, daniel, yhs, andriin, arnaldo.melo
Cc: kafai, songliubraving, john.fastabend, kpsingh, linux, joe,
pmladek, rostedt, sergey.senozhatsky, andriy.shevchenko, corbet,
bpf, netdev, linux-kernel, linux-doc
printk supports multiple pointer object type specifiers (printing
netdev features etc). Extend this support using BTF to cover
arbitrary types. "%pT" specifies the typed format, and the pointer
argument is a "struct btf_ptr *" where struct btf_ptr is as follows:
struct btf_ptr {
void *ptr;
const char *type;
u32 id;
};
Either the "type" string ("struct sk_buff") or the BTF "id" can be
used to identify the type to use in displaying the associated "ptr"
value. A convenience function to create and point at the struct
is provided:
printk(KERN_INFO "%pT", BTF_PTR_TYPE(skb, struct sk_buff));
When invoked, BTF information is used to traverse the sk_buff *
and display it. Support is present for structs, unions, enums,
typedefs and core types (though in the latter case there's not
much value in using this feature of course).
Default output is indented, but compact output can be specified
via the 'c' option. Type names/member values can be suppressed
using the 'N' option. Zero values are not displayed by default
but can be using the '0' option. Pointer values are obfuscated
unless the 'x' option is specified. As an example:
struct sk_buff *skb = alloc_skb(64, GFP_KERNEL);
pr_info("%pT", BTF_PTR_TYPE(skb, struct sk_buff));
...gives us:
(struct sk_buff){
.transport_header = (__u16)65535,
.mac_header = (__u16)65535,
.end = (sk_buff_data_t)192,
.head = (unsigned char *)0x000000006b71155a,
.data = (unsigned char *)0x000000006b71155a,
.truesize = (unsigned int)768,
.users = (refcount_t){
.refs = (atomic_t){
.counter = (int)1,
},
},
.extensions = (struct skb_ext *)0x00000000f486a130,
}
printk output is truncated at 1024 bytes. For cases where overflow
is likely, the compact/no type names display modes may be used.
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
i
---
Documentation/core-api/printk-formats.rst | 17 ++++++
include/linux/btf.h | 3 +-
include/linux/printk.h | 16 +++++
lib/vsprintf.c | 98 +++++++++++++++++++++++++++++++
4 files changed, 133 insertions(+), 1 deletion(-)
diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index 8c9aba2..8f255d0 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -563,6 +563,23 @@ For printing netdev_features_t.
Passed by reference.
+BTF-based printing of pointer data
+----------------------------------
+If '%pT' is specified, use the struct btf_ptr * along with kernel vmlinux
+BPF Type Format (BTF) to show the typed data. For example, specifying
+
+ printk(KERN_INFO "%pT", BTF_PTR_TYPE(skb, struct_sk_buff));
+
+will utilize BTF information to traverse the struct sk_buff * and display it.
+
+Supported modifers are
+ 'c' compact output (no indentation, newlines etc)
+ 'N' do not show type names
+ 'u' unsafe printing; probe_kernel_read() is not used to copy data safely
+ before use
+ 'x' show raw pointers (no obfuscation)
+ '0' show zero-valued data (it is not shown by default)
+
Thanks
======
diff --git a/include/linux/btf.h b/include/linux/btf.h
index a8a4563..e8dbf0c 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -172,10 +172,11 @@ static inline const struct btf_member *btf_type_member(const struct btf_type *t)
return (const struct btf_member *)(t + 1);
}
+struct btf *btf_parse_vmlinux(void);
+
#ifdef CONFIG_BPF_SYSCALL
const struct btf_type *btf_type_by_id(const struct btf *btf, u32 type_id);
const char *btf_name_by_offset(const struct btf *btf, u32 offset);
-struct btf *btf_parse_vmlinux(void);
struct btf *bpf_prog_get_target_btf(const struct bpf_prog *prog);
#else
static inline const struct btf_type *btf_type_by_id(const struct btf *btf,
diff --git a/include/linux/printk.h b/include/linux/printk.h
index fc8f03c..8f8f5d2 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -618,4 +618,20 @@ static inline void print_hex_dump_debug(const char *prefix_str, int prefix_type,
#define print_hex_dump_bytes(prefix_str, prefix_type, buf, len) \
print_hex_dump_debug(prefix_str, prefix_type, 16, 1, buf, len, true)
+/**
+ * struct btf_ptr is used for %pT (typed pointer) display; the
+ * additional type string/BTF id are used to render the pointer
+ * data as the appropriate type.
+ */
+struct btf_ptr {
+ void *ptr;
+ const char *type;
+ u32 id;
+};
+
+#define BTF_PTR_TYPE(ptrval, typeval) \
+ (&((struct btf_ptr){.ptr = ptrval, .type = #typeval}))
+
+#define BTF_PTR_ID(ptrval, idval) \
+ (&((struct btf_ptr){.ptr = ptrval, .id = idval}))
#endif
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 259e558..c0d209d 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -44,6 +44,7 @@
#ifdef CONFIG_BLOCK
#include <linux/blkdev.h>
#endif
+#include <linux/btf.h>
#include "../mm/internal.h" /* For the trace_print_flags arrays */
@@ -2092,6 +2093,87 @@ char *fwnode_string(char *buf, char *end, struct fwnode_handle *fwnode,
return widen_string(buf, buf - buf_start, end, spec);
}
+#define btf_modifier_flag(c) (c == 'c' ? BTF_SHOW_COMPACT : \
+ c == 'N' ? BTF_SHOW_NONAME : \
+ c == 'x' ? BTF_SHOW_PTR_RAW : \
+ c == 'u' ? BTF_SHOW_UNSAFE : \
+ c == '0' ? BTF_SHOW_ZERO : 0)
+
+static noinline_for_stack
+char *btf_string(char *buf, char *end, void *ptr, struct printf_spec spec,
+ const char *fmt)
+{
+ struct btf_ptr *bp = (struct btf_ptr *)ptr;
+ u8 btf_kind = BTF_KIND_TYPEDEF;
+ const struct btf_type *t;
+ const struct btf *btf;
+ char *buf_start = buf;
+ const char *btf_type;
+ u64 flags = 0, mod;
+ s32 btf_id;
+
+ if (check_pointer(&buf, end, ptr, spec))
+ return buf;
+
+ if (check_pointer(&buf, end, bp->ptr, spec))
+ return buf;
+
+ while (isalnum(*fmt)) {
+ mod = btf_modifier_flag(*fmt);
+ if (!mod)
+ break;
+ flags |= mod;
+ fmt++;
+ }
+
+ btf = bpf_get_btf_vmlinux();
+ if (IS_ERR_OR_NULL(btf))
+ return ptr_to_id(buf, end, bp->ptr, spec);
+
+ if (bp->type != NULL) {
+ btf_type = bp->type;
+
+ if (strncmp(bp->type, "struct ", strlen("struct ")) == 0) {
+ btf_kind = BTF_KIND_STRUCT;
+ btf_type += strlen("struct ");
+ } else if (strncmp(btf_type, "union ", strlen("union ")) == 0) {
+ btf_kind = BTF_KIND_UNION;
+ btf_type += strlen("union ");
+ } else if (strncmp(btf_type, "enum ", strlen("enum ")) == 0) {
+ btf_kind = BTF_KIND_ENUM;
+ btf_type += strlen("enum ");
+ }
+
+ if (strlen(btf_type) == 0)
+ return ptr_to_id(buf, end, bp->ptr, spec);
+
+ /*
+ * Assume type specified is a typedef as there's not much
+ * benefit in specifying int types other than wasting time
+ * on BTF lookups; we optimize for the most useful path.
+ *
+ * Fall back to BTF_KIND_INT if this fails.
+ */
+ btf_id = btf_find_by_name_kind(btf, btf_type, btf_kind);
+ if (btf_id < 0)
+ btf_id = btf_find_by_name_kind(btf, btf_type,
+ BTF_KIND_INT);
+ } else if (bp->id > 0)
+ btf_id = bp->id;
+ else
+ return ptr_to_id(buf, end, bp->ptr, spec);
+
+ if (btf_id > 0)
+ t = btf_type_by_id(btf, btf_id);
+ if (btf_id <= 0 || !t)
+ return ptr_to_id(buf, end, bp->ptr, spec);
+
+ buf += btf_type_snprintf_show(btf, btf_id, bp->ptr, buf,
+ end - buf_start, flags);
+
+ return widen_string(buf, buf - buf_start, end, spec);
+}
+
/*
* Show a '%p' thing. A kernel extension is that the '%p' is followed
* by an extra set of alphanumeric characters that are extended format
@@ -2206,6 +2288,20 @@ char *fwnode_string(char *buf, char *end, struct fwnode_handle *fwnode,
* bpf_trace_printk() where [ku] prefix specifies either kernel (k)
* or user (u) memory to probe, and:
* s a string, equivalent to "%s" on direct vsnprintf() use
+ * - 'T[cNx0]' For printing struct btf_ptr * data using BPF Type Format (BTF).
+ *
+ * Optional arguments are
+ * c compact (no indentation/newlines)
+ * N do not print type and member names
+ * x do not obfuscate pointers
+ * u do not copy data to safe buffer prior
+ * to display
+ * 0 show 0-valued data
+ *
+ * BPF_PTR_TYPE(ptr, type) can be used to place pointer and type string
+ * in the "struct btf_ptr *" expected; for example:
+ *
+ * printk(KERN_INFO "%pT", BTF_PTR_TYPE(skb, struct sk_buff));
*
* ** When making changes please also update:
* Documentation/core-api/printk-formats.rst
@@ -2297,6 +2393,8 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
default:
return error_string(buf, end, "(einval)", spec);
}
+ case 'T':
+ return btf_string(buf, end, ptr, spec, fmt + 1);
}
/* default is to _not_ leak addresses, hash before printing */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v3 bpf-next 4/8] printk: add type-printing %pT format specifier which uses BTF
2020-06-23 12:07 ` [PATCH v3 bpf-next 4/8] printk: add type-printing %pT format specifier which uses BTF Alan Maguire
@ 2020-06-23 12:40 ` Andy Shevchenko
2020-06-23 13:11 ` Rasmus Villemoes
2020-06-26 10:15 ` Petr Mladek
2 siblings, 0 replies; 18+ messages in thread
From: Andy Shevchenko @ 2020-06-23 12:40 UTC (permalink / raw)
To: Alan Maguire
Cc: ast, daniel, yhs, andriin, arnaldo.melo, kafai, songliubraving,
john.fastabend, kpsingh, linux, joe, pmladek, rostedt,
sergey.senozhatsky, corbet, bpf, netdev, linux-kernel, linux-doc
On Tue, Jun 23, 2020 at 01:07:07PM +0100, Alan Maguire wrote:
> printk supports multiple pointer object type specifiers (printing
> netdev features etc). Extend this support using BTF to cover
> arbitrary types.
Is there any plans to cover (all?) existing %p extensions?
> "%pT"
One letter namespace is quite busy area. Perhaps %pOT ?
> specifies the typed format, and the pointer
> argument is a "struct btf_ptr *" where struct btf_ptr is as follows:
>
> struct btf_ptr {
> void *ptr;
> const char *type;
> u32 id;
> };
>
> Either the "type" string ("struct sk_buff") or the BTF "id" can be
> used to identify the type to use in displaying the associated "ptr"
> value. A convenience function to create and point at the struct
> is provided:
>
> printk(KERN_INFO "%pT", BTF_PTR_TYPE(skb, struct sk_buff));
>
> When invoked, BTF information is used to traverse the sk_buff *
> and display it. Support is present for structs, unions, enums,
> typedefs and core types (though in the latter case there's not
> much value in using this feature of course).
>
> Default output is indented, but compact output can be specified
> via the 'c' option. Type names/member values can be suppressed
> using the 'N' option. Zero values are not displayed by default
> but can be using the '0' option. Pointer values are obfuscated
> unless the 'x' option is specified. As an example:
>
> struct sk_buff *skb = alloc_skb(64, GFP_KERNEL);
> pr_info("%pT", BTF_PTR_TYPE(skb, struct sk_buff));
>
> ...gives us:
>
> (struct sk_buff){
> .transport_header = (__u16)65535,
> .mac_header = (__u16)65535,
> .end = (sk_buff_data_t)192,
> .head = (unsigned char *)0x000000006b71155a,
> .data = (unsigned char *)0x000000006b71155a,
> .truesize = (unsigned int)768,
> .users = (refcount_t){
> .refs = (atomic_t){
> .counter = (int)1,
> },
> },
> .extensions = (struct skb_ext *)0x00000000f486a130,
> }
I don't see how it looks on a real console when kernel dumps something.
Care to provide? These examples better to have documented.
> printk output is truncated at 1024 bytes. For cases where overflow
> is likely, the compact/no type names display modes may be used.
How * is handled? (I mean %*pOT case)
...
> +#define BTF_PTR_TYPE(ptrval, typeval) \
> + (&((struct btf_ptr){.ptr = ptrval, .type = #typeval}))
> +
> +#define BTF_PTR_ID(ptrval, idval) \
> + (&((struct btf_ptr){.ptr = ptrval, .id = idval}))
Wouldn't be better if these will leave in its own (linker) section?
...
> +static noinline_for_stack
> +char *btf_string(char *buf, char *end, void *ptr, struct printf_spec spec,
> + const char *fmt)
> +{
> + struct btf_ptr *bp = (struct btf_ptr *)ptr;
Unneeded casting.
> + u8 btf_kind = BTF_KIND_TYPEDEF;
> + const struct btf_type *t;
> + const struct btf *btf;
> + char *buf_start = buf;
> + const char *btf_type;
> + u64 flags = 0, mod;
> + s32 btf_id;
> +
> + if (check_pointer(&buf, end, ptr, spec))
> + return buf;
> +
> + if (check_pointer(&buf, end, bp->ptr, spec))
> + return buf;
> + while (isalnum(*fmt)) {
> + mod = btf_modifier_flag(*fmt);
> + if (!mod)
> + break;
> + flags |= mod;
> + fmt++;
> + }
Can't we have explicitly all handled flags here, like other extensions do?
> + btf = bpf_get_btf_vmlinux();
> + if (IS_ERR_OR_NULL(btf))
> + return ptr_to_id(buf, end, bp->ptr, spec);
> +
> + if (bp->type != NULL) {
> + btf_type = bp->type;
> +
> + if (strncmp(bp->type, "struct ", strlen("struct ")) == 0) {
> + btf_kind = BTF_KIND_STRUCT;
> + btf_type += strlen("struct ");
> + } else if (strncmp(btf_type, "union ", strlen("union ")) == 0) {
> + btf_kind = BTF_KIND_UNION;
> + btf_type += strlen("union ");
> + } else if (strncmp(btf_type, "enum ", strlen("enum ")) == 0) {
> + btf_kind = BTF_KIND_ENUM;
> + btf_type += strlen("enum ");
> + }
Can't you provide a simple structure and do this in a loop?
Or even something like match_[partial]string() to implement?
> + if (strlen(btf_type) == 0)
Interesting way of checking btf_type == '\0'.
> + return ptr_to_id(buf, end, bp->ptr, spec);
> +
> + /*
> + * Assume type specified is a typedef as there's not much
> + * benefit in specifying int types other than wasting time
> + * on BTF lookups; we optimize for the most useful path.
> + *
> + * Fall back to BTF_KIND_INT if this fails.
> + */
> + btf_id = btf_find_by_name_kind(btf, btf_type, btf_kind);
> + if (btf_id < 0)
> + btf_id = btf_find_by_name_kind(btf, btf_type,
> + BTF_KIND_INT);
> + } else if (bp->id > 0)
> + btf_id = bp->id;
> + else
> + return ptr_to_id(buf, end, bp->ptr, spec);
> +
> + if (btf_id > 0)
> + t = btf_type_by_id(btf, btf_id);
> + if (btf_id <= 0 || !t)
> + return ptr_to_id(buf, end, bp->ptr, spec);
This can be easily incorporated in previous conditional tree.
> + buf += btf_type_snprintf_show(btf, btf_id, bp->ptr, buf,
> + end - buf_start, flags);
> +
> + return widen_string(buf, buf - buf_start, end, spec);
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v3 bpf-next 4/8] printk: add type-printing %pT format specifier which uses BTF
2020-06-23 12:07 ` [PATCH v3 bpf-next 4/8] printk: add type-printing %pT format specifier which uses BTF Alan Maguire
2020-06-23 12:40 ` Andy Shevchenko
@ 2020-06-23 13:11 ` Rasmus Villemoes
2020-06-26 10:15 ` Petr Mladek
2 siblings, 0 replies; 18+ messages in thread
From: Rasmus Villemoes @ 2020-06-23 13:11 UTC (permalink / raw)
To: Alan Maguire, ast, daniel, yhs, andriin, arnaldo.melo
Cc: kafai, songliubraving, john.fastabend, kpsingh, joe, pmladek,
rostedt, sergey.senozhatsky, andriy.shevchenko, corbet, bpf,
netdev, linux-kernel, linux-doc
On 23/06/2020 14.07, Alan Maguire wrote:
> diff --git a/include/linux/printk.h b/include/linux/printk.h
> index fc8f03c..8f8f5d2 100644
> --- a/include/linux/printk.h
> +++ b/include/linux/printk.h
> @@ -618,4 +618,20 @@ static inline void print_hex_dump_debug(const char *prefix_str, int prefix_type,
> #define print_hex_dump_bytes(prefix_str, prefix_type, buf, len) \
> print_hex_dump_debug(prefix_str, prefix_type, 16, 1, buf, len, true)
>
> +/**
> + * struct btf_ptr is used for %pT (typed pointer) display; the
> + * additional type string/BTF id are used to render the pointer
> + * data as the appropriate type.
> + */
> +struct btf_ptr {
> + void *ptr;
> + const char *type;
> + u32 id;
> +};
> +
> +#define BTF_PTR_TYPE(ptrval, typeval) \
> + (&((struct btf_ptr){.ptr = ptrval, .type = #typeval}))
> +
> +#define BTF_PTR_ID(ptrval, idval) \
> + (&((struct btf_ptr){.ptr = ptrval, .id = idval}))
Isn't there some better place to put this than printk.h? Anyway, you
probably want the ptr member to be "const void*", to avoid "... discards
const qualifier" warnings when somebody happens to have a "const struct
foobar *".
> #endif
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 259e558..c0d209d 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -44,6 +44,7 @@
> #ifdef CONFIG_BLOCK
> #include <linux/blkdev.h>
> #endif
> +#include <linux/btf.h>
>
> #include "../mm/internal.h" /* For the trace_print_flags arrays */
>
> @@ -2092,6 +2093,87 @@ char *fwnode_string(char *buf, char *end, struct fwnode_handle *fwnode,
> return widen_string(buf, buf - buf_start, end, spec);
> }
>
> +#define btf_modifier_flag(c) (c == 'c' ? BTF_SHOW_COMPACT : \
> + c == 'N' ? BTF_SHOW_NONAME : \
> + c == 'x' ? BTF_SHOW_PTR_RAW : \
> + c == 'u' ? BTF_SHOW_UNSAFE : \
> + c == '0' ? BTF_SHOW_ZERO : 0)
> +
> +static noinline_for_stack
> +char *btf_string(char *buf, char *end, void *ptr, struct printf_spec spec,
> + const char *fmt)
> +{
> + struct btf_ptr *bp = (struct btf_ptr *)ptr;
> + u8 btf_kind = BTF_KIND_TYPEDEF;
> + const struct btf_type *t;
> + const struct btf *btf;
> + char *buf_start = buf;
> + const char *btf_type;
> + u64 flags = 0, mod;
> + s32 btf_id;
> +
> + if (check_pointer(&buf, end, ptr, spec))
> + return buf;
> +
> + if (check_pointer(&buf, end, bp->ptr, spec))
> + return buf;
> +
> + while (isalnum(*fmt)) {
> + mod = btf_modifier_flag(*fmt);
> + if (!mod)
> + break;
> + flags |= mod;
> + fmt++;
> + }
> +
> + btf = bpf_get_btf_vmlinux();
AFAICT, this function is only compiled if CONFIG_BPF=y and
CONFIG_BPF_SYSCALL=y, and I don't see any static inline stub defined
anywhere. Have you built the kernel with one or both of those turned off?
Rasmus
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v3 bpf-next 4/8] printk: add type-printing %pT format specifier which uses BTF
2020-06-23 12:07 ` [PATCH v3 bpf-next 4/8] printk: add type-printing %pT format specifier which uses BTF Alan Maguire
2020-06-23 12:40 ` Andy Shevchenko
2020-06-23 13:11 ` Rasmus Villemoes
@ 2020-06-26 10:15 ` Petr Mladek
2020-06-26 11:37 ` Alan Maguire
2 siblings, 1 reply; 18+ messages in thread
From: Petr Mladek @ 2020-06-26 10:15 UTC (permalink / raw)
To: Alan Maguire, rostedt, sergey.senozhatsky, Linus Torvalds
Cc: ast, daniel, yhs, andriin, arnaldo.melo, kafai, songliubraving,
john.fastabend, kpsingh, linux, joe, andriy.shevchenko, corbet,
bpf, netdev, linux-kernel, linux-doc
On Tue 2020-06-23 13:07:07, Alan Maguire wrote:
> printk supports multiple pointer object type specifiers (printing
> netdev features etc). Extend this support using BTF to cover
> arbitrary types. "%pT" specifies the typed format, and the pointer
> argument is a "struct btf_ptr *" where struct btf_ptr is as follows:
>
> struct btf_ptr {
> void *ptr;
> const char *type;
> u32 id;
> };
>
> Either the "type" string ("struct sk_buff") or the BTF "id" can be
> used to identify the type to use in displaying the associated "ptr"
> value. A convenience function to create and point at the struct
> is provided:
>
> printk(KERN_INFO "%pT", BTF_PTR_TYPE(skb, struct sk_buff));
>
> When invoked, BTF information is used to traverse the sk_buff *
> and display it. Support is present for structs, unions, enums,
> typedefs and core types (though in the latter case there's not
> much value in using this feature of course).
>
> Default output is indented, but compact output can be specified
> via the 'c' option. Type names/member values can be suppressed
> using the 'N' option. Zero values are not displayed by default
> but can be using the '0' option. Pointer values are obfuscated
> unless the 'x' option is specified. As an example:
>
> struct sk_buff *skb = alloc_skb(64, GFP_KERNEL);
> pr_info("%pT", BTF_PTR_TYPE(skb, struct sk_buff));
>
> ...gives us:
>
> (struct sk_buff){
> .transport_header = (__u16)65535,
> .mac_header = (__u16)65535,
> .end = (sk_buff_data_t)192,
> .head = (unsigned char *)0x000000006b71155a,
> .data = (unsigned char *)0x000000006b71155a,
> .truesize = (unsigned int)768,
> .users = (refcount_t){
> .refs = (atomic_t){
> .counter = (int)1,
> },
> },
> .extensions = (struct skb_ext *)0x00000000f486a130,
> }
>
> printk output is truncated at 1024 bytes. For cases where overflow
> is likely, the compact/no type names display modes may be used.
Hmm, this scares me:
1. The long message and many lines are going to stretch printk
design in another dimensions.
2. vsprintf() is important for debugging the system. It has to be
stable. But the btf code is too complex.
I would strongly prefer to keep this outside vsprintf and printk.
Please, invert the logic and convert it into using separate printk()
call for each printed line.
More details:
Add 1: Long messages with many lines:
IMHO, all existing printk() users are far below this limit. And this is
even worse because there are many short lines. They would require
double space to add prefixes (loglevel, timestamp, caller id) when
printing to console.
You might argue that 1024bytes are enough for you. But for how long?
Now, we have huge troubles to make printk() lockless and thus more
reliable. There is no way to allocate any internal buffers
dynamically. People using kernel on small devices have problem
with large static buffers.
printk() is primary designed to print single line messages. There are
many use cases where many lines are needed and they are solved by
many separate printk() calls.
Add 2: Complex code:
vsprintf() is currently called in printk() under logbuf_lock. It
might block printk() on the entire system.
Most existing %p<modifier> handlers are implemented by relatively
simple routines inside lib/vsprinf.c. The other external routines
look simple as well.
btf looks like a huge beast to me. For example, probe_kernel_read()
prevented boot recently, see the commit 2ac5a3bf7042a1c4abb
("vsprintf: Do not break early boot with probing addresses").
Best Regards,
Petr
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v3 bpf-next 4/8] printk: add type-printing %pT format specifier which uses BTF
2020-06-26 10:15 ` Petr Mladek
@ 2020-06-26 11:37 ` Alan Maguire
2020-06-29 9:43 ` Petr Mladek
0 siblings, 1 reply; 18+ messages in thread
From: Alan Maguire @ 2020-06-26 11:37 UTC (permalink / raw)
To: Petr Mladek
Cc: Alan Maguire, rostedt, sergey.senozhatsky, Linus Torvalds, ast,
daniel, yhs, andriin, arnaldo.melo, kafai, songliubraving,
john.fastabend, kpsingh, linux, joe, andriy.shevchenko, corbet,
bpf, netdev, linux-kernel, linux-doc
On Fri, 26 Jun 2020, Petr Mladek wrote:
> On Tue 2020-06-23 13:07:07, Alan Maguire wrote:
> > printk supports multiple pointer object type specifiers (printing
> > netdev features etc). Extend this support using BTF to cover
> > arbitrary types. "%pT" specifies the typed format, and the pointer
> > argument is a "struct btf_ptr *" where struct btf_ptr is as follows:
> >
> > struct btf_ptr {
> > void *ptr;
> > const char *type;
> > u32 id;
> > };
> >
> > Either the "type" string ("struct sk_buff") or the BTF "id" can be
> > used to identify the type to use in displaying the associated "ptr"
> > value. A convenience function to create and point at the struct
> > is provided:
> >
> > printk(KERN_INFO "%pT", BTF_PTR_TYPE(skb, struct sk_buff));
> >
> > When invoked, BTF information is used to traverse the sk_buff *
> > and display it. Support is present for structs, unions, enums,
> > typedefs and core types (though in the latter case there's not
> > much value in using this feature of course).
> >
> > Default output is indented, but compact output can be specified
> > via the 'c' option. Type names/member values can be suppressed
> > using the 'N' option. Zero values are not displayed by default
> > but can be using the '0' option. Pointer values are obfuscated
> > unless the 'x' option is specified. As an example:
> >
> > struct sk_buff *skb = alloc_skb(64, GFP_KERNEL);
> > pr_info("%pT", BTF_PTR_TYPE(skb, struct sk_buff));
> >
> > ...gives us:
> >
> > (struct sk_buff){
> > .transport_header = (__u16)65535,
> > .mac_header = (__u16)65535,
> > .end = (sk_buff_data_t)192,
> > .head = (unsigned char *)0x000000006b71155a,
> > .data = (unsigned char *)0x000000006b71155a,
> > .truesize = (unsigned int)768,
> > .users = (refcount_t){
> > .refs = (atomic_t){
> > .counter = (int)1,
> > },
> > },
> > .extensions = (struct skb_ext *)0x00000000f486a130,
> > }
> >
> > printk output is truncated at 1024 bytes. For cases where overflow
> > is likely, the compact/no type names display modes may be used.
>
> Hmm, this scares me:
>
> 1. The long message and many lines are going to stretch printk
> design in another dimensions.
>
> 2. vsprintf() is important for debugging the system. It has to be
> stable. But the btf code is too complex.
>
Right on both points, and there's no way around that really. Representing
even small data structures will stretch us to or beyond the 1024 byte
limit. This can be mitigated by using compact display mode and not
printing field names, but the output becomes hard to parse then.
I think a better approach might be to start small, adding the core
btf_show functionality to BPF, allowing consumers to use it there,
perhaps via a custom helper. In the current model bpf_trace_printk()
inherits the functionality to display data from core printk, so a
different approach would be needed there. Other consumers outside of BPF
could potentially avail of the show functionality directly via the btf_show
functions in the future, but at least it would have one consumer at the
outset, and wouldn't present problems like these for printk.
> I would strongly prefer to keep this outside vsprintf and printk.
> Please, invert the logic and convert it into using separate printk()
> call for each printed line.
>
I think the above is in line with what you're suggesting?
>
> More details:
>
> Add 1: Long messages with many lines:
>
> IMHO, all existing printk() users are far below this limit. And this is
> even worse because there are many short lines. They would require
> double space to add prefixes (loglevel, timestamp, caller id) when
> printing to console.
>
> You might argue that 1024bytes are enough for you. But for how long?
>
> Now, we have huge troubles to make printk() lockless and thus more
> reliable. There is no way to allocate any internal buffers
> dynamically. People using kernel on small devices have problem
> with large static buffers.
>
> printk() is primary designed to print single line messages. There are
> many use cases where many lines are needed and they are solved by
> many separate printk() calls.
>
>
> Add 2: Complex code:
>
> vsprintf() is currently called in printk() under logbuf_lock. It
> might block printk() on the entire system.
>
> Most existing %p<modifier> handlers are implemented by relatively
> simple routines inside lib/vsprinf.c. The other external routines
> look simple as well.
>
> btf looks like a huge beast to me. For example, probe_kernel_read()
> prevented boot recently, see the commit 2ac5a3bf7042a1c4abb
> ("vsprintf: Do not break early boot with probing addresses").
>
>
Yep, no way round this either. I'll try a different approach. Thanks for
taking a look!
Alan
> Best Regards,
> Petr
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v3 bpf-next 4/8] printk: add type-printing %pT format specifier which uses BTF
2020-06-26 11:37 ` Alan Maguire
@ 2020-06-29 9:43 ` Petr Mladek
0 siblings, 0 replies; 18+ messages in thread
From: Petr Mladek @ 2020-06-29 9:43 UTC (permalink / raw)
To: Alan Maguire
Cc: rostedt, sergey.senozhatsky, Linus Torvalds, ast, daniel, yhs,
andriin, arnaldo.melo, kafai, songliubraving, john.fastabend,
kpsingh, linux, joe, andriy.shevchenko, corbet, bpf, netdev,
linux-kernel, linux-doc
On Fri 2020-06-26 12:37:19, Alan Maguire wrote:
>
> On Fri, 26 Jun 2020, Petr Mladek wrote:
>
> > On Tue 2020-06-23 13:07:07, Alan Maguire wrote:
> > >
> > > printk(KERN_INFO "%pT", BTF_PTR_TYPE(skb, struct sk_buff));
> > >
> > > struct sk_buff *skb = alloc_skb(64, GFP_KERNEL);
> > > pr_info("%pT", BTF_PTR_TYPE(skb, struct sk_buff));
> > >
> > > ...gives us:
> > >
> > > (struct sk_buff){
> > > .transport_header = (__u16)65535,
> > > .mac_header = (__u16)65535,
> > > .end = (sk_buff_data_t)192,
> > > .head = (unsigned char *)0x000000006b71155a,
> > > .data = (unsigned char *)0x000000006b71155a,
> > > .truesize = (unsigned int)768,
> > > .users = (refcount_t){
> > > .refs = (atomic_t){
> > > .counter = (int)1,
> > > },
> > > },
> > > .extensions = (struct skb_ext *)0x00000000f486a130,
> > > }
> > >
> > > printk output is truncated at 1024 bytes. For cases where overflow
> > > is likely, the compact/no type names display modes may be used.
> >
> > Hmm, this scares me:
> >
> > 1. The long message and many lines are going to stretch printk
> > design in another dimensions.
> >
> > 2. vsprintf() is important for debugging the system. It has to be
> > stable. But the btf code is too complex.
> >
>
> Right on both points, and there's no way around that really. Representing
> even small data structures will stretch us to or beyond the 1024 byte
> limit. This can be mitigated by using compact display mode and not
> printing field names, but the output becomes hard to parse then.
>
> I think a better approach might be to start small, adding the core
> btf_show functionality to BPF, allowing consumers to use it there,
> perhaps via a custom helper.
Sounds good to me.
> In the current model bpf_trace_printk() inherits the functionality
> to display data from core printk, so a different approach would
> be needed there.
BTW: Even the trace buffer has a limitation, see BUF_MAX_DATA_SIZE
in kernel/trace/ring_buffer.c. It is internally implemented as
a list of memory pages, see the comments above RB_BUFFER_OFF
definition.
It is typically 4k. I think that you might hit this limit as well.
We had to increase per-CPU buffers used by printk() in NMI context
because 4k was not enough for some backtraces.
So, using different approach would make sense even when using trace
buffer.
> Other consumers outside of BPF
> could potentially avail of the show functionality directly via the btf_show
> functions in the future, but at least it would have one consumer at the
> outset, and wouldn't present problems like these for printk.
Sounds good to me.
> > I would strongly prefer to keep this outside vsprintf and printk.
> > Please, invert the logic and convert it into using separate printk()
> > call for each printed line.
> >
>
> I think the above is in line with what you're suggesting?
Yes, as far as I understand it.
> Yep, no way round this either. I'll try a different approach. Thanks for
> taking a look!
Uff, thanks a lot for understanding. I hope that most of the code will
be reusable in some form.
Best Regards,
Petr
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 bpf-next 5/8] printk: initialize vmlinux BTF outside of printk in late_initcall()
2020-06-23 12:07 [PATCH v3 bpf-next 0/8] bpf, printk: add BTF-based type printing Alan Maguire
` (3 preceding siblings ...)
2020-06-23 12:07 ` [PATCH v3 bpf-next 4/8] printk: add type-printing %pT format specifier which uses BTF Alan Maguire
@ 2020-06-23 12:07 ` Alan Maguire
2020-06-23 12:07 ` [PATCH v3 bpf-next 6/8] printk: extend test_printf to test %pT BTF-based format specifier Alan Maguire
` (3 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Alan Maguire @ 2020-06-23 12:07 UTC (permalink / raw)
To: ast, daniel, yhs, andriin, arnaldo.melo
Cc: kafai, songliubraving, john.fastabend, kpsingh, linux, joe,
pmladek, rostedt, sergey.senozhatsky, andriy.shevchenko, corbet,
bpf, netdev, linux-kernel, linux-doc
vmlinux BTF initialization can take time so it's best to do that
outside of printk context; otherwise the first printk() using %pT
will trigger BTF initialization.
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
lib/vsprintf.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index c0d209d..8ac136a 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -3628,3 +3628,15 @@ int sscanf(const char *buf, const char *fmt, ...)
return i;
}
EXPORT_SYMBOL(sscanf);
+
+/*
+ * Initialize vmlinux BTF as it may be used by printk()s and it's better
+ * to incur the cost of initialization outside of printk context.
+ */
+static int __init init_btf_vmlinux(void)
+{
+ (void) bpf_get_btf_vmlinux();
+
+ return 0;
+}
+late_initcall(init_btf_vmlinux);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 bpf-next 6/8] printk: extend test_printf to test %pT BTF-based format specifier
2020-06-23 12:07 [PATCH v3 bpf-next 0/8] bpf, printk: add BTF-based type printing Alan Maguire
` (4 preceding siblings ...)
2020-06-23 12:07 ` [PATCH v3 bpf-next 5/8] printk: initialize vmlinux BTF outside of printk in late_initcall() Alan Maguire
@ 2020-06-23 12:07 ` Alan Maguire
2020-06-23 13:02 ` Andy Shevchenko
2020-06-23 12:07 ` [PATCH v3 bpf-next 7/8] bpf: add support for %pT format specifier for bpf_trace_printk() helper Alan Maguire
` (2 subsequent siblings)
8 siblings, 1 reply; 18+ messages in thread
From: Alan Maguire @ 2020-06-23 12:07 UTC (permalink / raw)
To: ast, daniel, yhs, andriin, arnaldo.melo
Cc: kafai, songliubraving, john.fastabend, kpsingh, linux, joe,
pmladek, rostedt, sergey.senozhatsky, andriy.shevchenko, corbet,
bpf, netdev, linux-kernel, linux-doc
Add tests to verify basic type display and to iterate through all
enums, structs, unions and typedefs ensuring expected behaviour
occurs. Since test_printf can be built as a module we need to
export a BTF kind iterator function to allow us to iterate over
all names of a particular BTF kind.
These changes add up to approximately 20,000 new tests covering
all enum, struct, union and typedefs in vmlinux BTF.
Individual tests are also added for int, char, struct, enum
and typedefs which verify output is as expected.
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
include/linux/btf.h | 3 +
kernel/bpf/btf.c | 33 ++++++
lib/test_printf.c | 316 ++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 352 insertions(+)
diff --git a/include/linux/btf.h b/include/linux/btf.h
index e8dbf0c..e3102a7 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -191,4 +191,7 @@ static inline const char *btf_name_by_offset(const struct btf *btf,
}
#endif
+/* Following function used for testing BTF-based printk-family support */
+const char *btf_vmlinux_next_type_name(u8 kind, s32 *id);
+
#endif
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index c82cb18..4e250cd 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -5459,3 +5459,36 @@ u32 btf_id(const struct btf *btf)
{
return btf->id;
}
+
+/*
+ * btf_vmlinux_next_type_name(): used in test_printf.c to
+ * iterate over types for testing.
+ * Exported as test_printf can be built as a module.
+ *
+ * @kind: BTF_KIND_* value
+ * @id: pointer to last id; value/result argument. When next
+ * type name is found, we set *id to associated id.
+ * Returns:
+ * Next type name, sets *id to associated id.
+ */
+const char *btf_vmlinux_next_type_name(u8 kind, s32 *id)
+{
+ const struct btf *btf = bpf_get_btf_vmlinux();
+ const struct btf_type *t;
+ const char *name;
+
+ if (!btf || !id)
+ return NULL;
+
+ for ((*id)++; *id <= btf->nr_types; (*id)++) {
+ t = btf->types[*id];
+ if (BTF_INFO_KIND(t->info) != kind)
+ continue;
+ name = btf_name_by_offset(btf, t->name_off);
+ if (name && strlen(name) > 0)
+ return name;
+ }
+
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(btf_vmlinux_next_type_name);
diff --git a/lib/test_printf.c b/lib/test_printf.c
index 7ac87f1..7ce7387 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -23,6 +23,9 @@
#include <linux/mm.h>
#include <linux/property.h>
+#include <linux/bpf.h>
+#include <linux/btf.h>
+#include <linux/skbuff.h>
#include "../tools/testing/selftests/kselftest_module.h"
@@ -669,6 +672,318 @@ static void __init fwnode_pointer(void)
#endif
}
+#define __TEST_BTF(fmt, type, ptr, expected) \
+ test(expected, "%pT"fmt, ptr)
+
+#define TEST_BTF_C(type, var, ...) \
+ do { \
+ type var = __VA_ARGS__; \
+ struct btf_ptr *ptr = BTF_PTR_TYPE(&var, type); \
+ pr_debug("type %s: %pTc", #type, ptr); \
+ __TEST_BTF("c", type, ptr, "(" #type ")" #__VA_ARGS__); \
+ } while (0)
+
+#define TEST_BTF(fmt, type, var, expected, ...) \
+ do { \
+ type var = __VA_ARGS__; \
+ struct btf_ptr *ptr = BTF_PTR_TYPE(&var, type); \
+ pr_debug("type %s: %pT"fmt, #type, ptr); \
+ __TEST_BTF(fmt, type, ptr, expected); \
+ } while (0)
+
+#define BTF_MAX_DATA_SIZE 65536
+
+static void __init
+btf_print_kind(u8 kind, const char *kind_name, u64 fillval)
+{
+ const char *fmt1 = "%pT", *fmt2 = "%pTN", *fmt3 = "%pT0";
+ const char *name, *fmt = fmt1;
+ int i, res1, res2, res3, res4;
+ char type_name[256];
+ char *buf, *buf2;
+ u8 *dummy_data;
+ s32 id = 0;
+
+ dummy_data = kzalloc(BTF_MAX_DATA_SIZE, GFP_KERNEL);
+
+ /* fill our dummy data with supplied fillval. */
+ for (i = 0; i < BTF_MAX_DATA_SIZE; i++)
+ dummy_data[i] = fillval;
+
+ buf = kzalloc(BTF_MAX_DATA_SIZE, GFP_KERNEL);
+ buf2 = kzalloc(BTF_MAX_DATA_SIZE, GFP_KERNEL);
+
+ for (;;) {
+ name = btf_vmlinux_next_type_name(kind, &id);
+ if (!name)
+ break;
+
+ total_tests++;
+
+ snprintf(type_name, sizeof(type_name), "%s%s",
+ kind_name, name);
+
+ res1 = snprintf(buf, BTF_MAX_DATA_SIZE, fmt1,
+ BTF_PTR_TYPE(dummy_data, type_name));
+ res2 = snprintf(buf, 0, fmt1,
+ BTF_PTR_TYPE(dummy_data, type_name));
+ res3 = snprintf(buf, BTF_MAX_DATA_SIZE, fmt2,
+ BTF_PTR_TYPE(dummy_data, type_name));
+ res4 = snprintf(buf, BTF_MAX_DATA_SIZE, fmt3,
+ BTF_PTR_TYPE(dummy_data, type_name));
+
+ (void) snprintf(buf, BTF_MAX_DATA_SIZE, "%pT",
+ BTF_PTR_TYPE(dummy_data, type_name));
+ (void) snprintf(buf2, BTF_MAX_DATA_SIZE, "%pT",
+ BTF_PTR_TYPE(dummy_data, type_name));
+
+ /*
+ * Ensure return value is > 0 and identical irrespective
+ * of whether we pass in a big enough buffer;
+ * also ensure that printing names always results in as
+ * long/longer buffer length.
+ */
+ if (res1 <= 0 || res2 <= 0 || res3 <= 0 || res4 <= 0) {
+ if (res3 <= 0)
+ fmt = fmt2;
+ if (res4 <= 0)
+ fmt = fmt3;
+
+ pr_warn("snprintf(%s%s); %d <= 0 (fmt %s)",
+ kind_name, name,
+ res1 <= 0 ? res1 : res2 <= 0 ? res2 :
+ res3 <= 0 ? res3 : res4, fmt);
+ failed_tests++;
+ } else if (res1 != res2) {
+ pr_warn("snprintf(%s%s): %d (to buf) != %d (no buf)",
+ kind_name, name, res1, res2);
+ failed_tests++;
+ } else if (res3 > res2) {
+ pr_warn("snprintf(%s%s); %d (no names) > %d (names)",
+ kind_name, name, res3, res2);
+ failed_tests++;
+ } else if (strcmp(buf, buf2) != 0) {
+ /* Safe and unsafe buffers should match. */
+ pr_warn("snprintf(%s%s); safe != unsafe",
+ kind_name, name);
+ pr_warn("safe: %s", buf);
+ pr_warn("unsafe: %s", buf2);
+ failed_tests++;
+ } else {
+ pr_debug("Printed %s%s (%d bytes)",
+ kind_name, name, res1);
+ }
+ }
+ kfree(dummy_data);
+ kfree(buf);
+ kfree(buf2);
+}
+
+/*
+ * For BTF it is the struct btf_ptr * ptr field, not the pointer itself
+ * which gets displayed, so it is that we need to hash.
+ */
+static void __init
+test_btf_hashed(const char *fmt, struct btf_ptr *p)
+{
+ char buf[PLAIN_BUF_SIZE];
+ int ret;
+
+ ret = plain_hash_to_buffer(p->ptr, buf, PLAIN_BUF_SIZE);
+ if (ret)
+ return;
+
+ test(buf, fmt, p);
+}
+
+#ifdef CONFIG_DEBUG_INFO_BTF
+
+static void __init btf_pointer_test_int(void)
+{
+ /* simple int */
+ TEST_BTF_C(int, testint, 1234);
+ TEST_BTF("cN", int, testint, "1234", 1234);
+ /* zero value should be printed at toplevel */
+ TEST_BTF("c", int, testint, "(int)0", 0);
+ TEST_BTF("cN", int, testint, "0", 0);
+ TEST_BTF("c0", int, testint, "(int)0", 0);
+ TEST_BTF("cN0", int, testint, "0", 0);
+ TEST_BTF_C(int, testint, -4567);
+ TEST_BTF("cN", int, testint, "-4567", -4567);
+}
+
+static void __init btf_pointer_test_char(void)
+{
+ /* simple char */
+ TEST_BTF_C(char, testchar, 100);
+ TEST_BTF("cN", char, testchar, "100", 100);
+ /* zero value should be printed at toplevel */
+ TEST_BTF("c", char, testchar, "(char)0", 0);
+ TEST_BTF("cN", char, testchar, "0", 0);
+ TEST_BTF("c0", char, testchar, "(char)0", 0);
+ TEST_BTF("cN0", char, testchar, "0", 0);
+}
+
+static void __init btf_pointer_test_typedef(void)
+{
+ /* simple typedef */
+ TEST_BTF_C(phys_addr_t, testtype, 100);
+ TEST_BTF("cN", phys_addr_t, testtype, "1", 1);
+ /* zero value should be printed at toplevel */
+ TEST_BTF("c", phys_addr_t, testtype, "(phys_addr_t)0", 0);
+ TEST_BTF("cN", phys_addr_t, testtype, "0", 0);
+ TEST_BTF("c0", phys_addr_t, testtype, "(phys_addr_t)0", 0);
+ TEST_BTF("cN0", phys_addr_t, testtype, "0", 0);
+
+ /* typedef struct */
+ TEST_BTF_C(atomic_t, testtype, {.counter = (int)1,});
+ TEST_BTF("cN", atomic_t, testtype, "{1,}", {.counter = 1,});
+ /* typedef with 0 value should be printed at toplevel */
+ TEST_BTF("c", atomic_t, testtype, "(atomic_t){}", {.counter = 0,});
+ TEST_BTF("cN", atomic_t, testtype, "{}", {.counter = 0,});
+ TEST_BTF("c0", atomic_t, testtype, "(atomic_t){.counter = (int)0,}",
+ {.counter = 0,});
+ TEST_BTF("cN0", atomic_t, testtype, "{0,}", {.counter = 0,});
+
+ /* typedef array */
+ TEST_BTF("c", cpumask_t, testtype,
+ "(cpumask_t){.bits = (long unsigned int[])[1,],}",
+ { .bits = {1,}});
+ TEST_BTF("cN", cpumask_t, testtype, "{[1,],}",
+ { .bits = {1,}});
+ /* typedef with 0 value should be printed at toplevel */
+ TEST_BTF("c", cpumask_t, testtype, "(cpumask_t){}", {{ 0 }});
+}
+
+static void __init btf_pointer_test_enum(void)
+{
+ /* enum where enum value does (and does not) exist */
+ TEST_BTF_C(enum bpf_cmd, testenum, BPF_MAP_CREATE);
+ TEST_BTF("c", enum bpf_cmd, testenum, "(enum bpf_cmd)BPF_MAP_CREATE",
+ 0);
+ TEST_BTF("cN", enum bpf_cmd, testenum, "BPF_MAP_CREATE",
+ BPF_MAP_CREATE);
+ TEST_BTF("cN0", enum bpf_cmd, testenum, "BPF_MAP_CREATE", 0);
+
+ TEST_BTF("c0", enum bpf_cmd, testenum, "(enum bpf_cmd)BPF_MAP_CREATE",
+ BPF_MAP_CREATE);
+ TEST_BTF("cN0", enum bpf_cmd, testenum, "BPF_MAP_CREATE",
+ BPF_MAP_CREATE);
+ TEST_BTF_C(enum bpf_cmd, testenum, 2000);
+ TEST_BTF("cN", enum bpf_cmd, testenum, "2000", 2000);
+}
+
+static void __init btf_pointer_test_struct(void)
+{
+ /* simple struct */
+ TEST_BTF_C(struct btf_enum, teststruct,
+ {.name_off = (__u32)3,.val = (__s32)-1,});
+ TEST_BTF("cN", struct btf_enum, teststruct, "{3,-1,}",
+ { .name_off = 3, .val = -1,});
+ TEST_BTF("cN", struct btf_enum, teststruct, "{-1,}",
+ { .name_off = 0, .val = -1,});
+ TEST_BTF("cN0", struct btf_enum, teststruct, "{0,-1,}",
+ { .name_off = 0, .val = -1,});
+ /* empty struct should be printed */
+ TEST_BTF("c", struct btf_enum, teststruct, "(struct btf_enum){}",
+ { .name_off = 0, .val = 0,});
+ TEST_BTF("cN", struct btf_enum, teststruct, "{}",
+ { .name_off = 0, .val = 0,});
+ TEST_BTF("c0", struct btf_enum, teststruct,
+ "(struct btf_enum){.name_off = (__u32)0,.val = (__s32)0,}",
+ { .name_off = 0, .val = 0,});
+
+ /* struct with pointers */
+ TEST_BTF("cx", struct skb_shared_info, testptr,
+ "(struct skb_shared_info){.frag_list = (struct sk_buff *)0x0000000000000001,}",
+ { .frag_list = (struct sk_buff *)1 });
+ /* NULL pointer should not be displayed */
+ TEST_BTF("cx", struct skb_shared_info, testptr,
+ "(struct skb_shared_info){}",
+ { .frag_list = (struct sk_buff *)0 });
+
+ /* struct with char array */
+ TEST_BTF("c", struct bpf_prog_info, teststruct,
+ "(struct bpf_prog_info){.name = (char[])['f','o','o',],}",
+ { .name = "foo",});
+ TEST_BTF("cN", struct bpf_prog_info, teststruct,
+ "{['f','o','o',],}",
+ {.name = "foo",});
+ /* leading null char means do not display string */
+ TEST_BTF("c", struct bpf_prog_info, teststruct,
+ "(struct bpf_prog_info){}",
+ {.name = {'\0', 'f', 'o', 'o'}});
+ /* handle non-printable characters */
+ TEST_BTF("c", struct bpf_prog_info, teststruct,
+ "(struct bpf_prog_info){.name = (char[])[1,2,3,],}",
+ { .name = {1, 2, 3, 0}});
+
+ /* struct with non-char array */
+ TEST_BTF("c", struct __sk_buff, teststruct,
+ "(struct __sk_buff){.cb = (__u32[])[1,2,3,4,5,],}",
+ { .cb = {1, 2, 3, 4, 5,},});
+ TEST_BTF("cN", struct __sk_buff, teststruct,
+ "{[1,2,3,4,5,],}",
+ { .cb = { 1, 2, 3, 4, 5},});
+ /* For non-char, arrays, show non-zero values only */
+ TEST_BTF("c", struct __sk_buff, teststruct,
+ "(struct __sk_buff){.cb = (__u32[])[1,],}",
+ { .cb = { 0, 0, 1, 0, 0},});
+
+ /* struct with struct array */
+ TEST_BTF("c", struct bpf_struct_ops, teststruct,
+ "(struct bpf_struct_ops){.func_models = (struct btf_func_model[])[(struct btf_func_model){.ret_size = (u8)1,.nr_args = (u8)2,.arg_size = (u8[])[3,4,5,],},],}",
+ { .func_models = {{ .ret_size = 1, .nr_args = 2,
+ .arg_size = { 3, 4, 5,},}}});
+
+ /* struct with bitfields */
+ TEST_BTF_C(struct bpf_insn, testbitfield,
+ {.code = (__u8)1,.dst_reg = (__u8)0x2,.src_reg = (__u8)0x3,.off = (__s16)4,.imm = (__s32)5,});
+ TEST_BTF("cN", struct bpf_insn, testbitfield, "{1,0x2,0x3,4,5,}",
+ {.code = 1, .dst_reg = 0x2, .src_reg = 0x3, .off = 4,
+ .imm = 5,});
+
+ /* struct with anon struct/unions */
+ TEST_BTF("cx", struct sk_buff, test_anon,
+ "(struct sk_buff){(union){(struct){(union){.dev = (struct net_device *)0x0000000000000001,.dev_scratch = (long unsigned int)1,},},.rbnode = (struct rb_node){.rb_left = (struct rb_node *)0x0000000000000001,},},}",
+ { .dev_scratch = 1 });
+}
+
+#endif /* CONFIG_DEBUG_INFO_BTF */
+
+static void __init
+btf_pointer(void)
+{
+ struct sk_buff *skb = alloc_skb(64, GFP_KERNEL);
+ u64 fillvals[] = { 0x0, 0xffffffffffffffff, 0x0123456789abcdef };
+ int i;
+
+#ifdef CONFIG_DEBUG_INFO_BTF
+ btf_pointer_test_int();
+ btf_pointer_test_char();
+ btf_pointer_test_typedef();
+ btf_pointer_test_enum();
+ btf_pointer_test_struct();
+#endif /* CONFIG_DEBUG_INFO_BTF */
+
+ /*
+ * Iterate every instance of each kind, printing each associated type.
+ * This constitutes around 10k tests.
+ */
+ for (i = 0; i < ARRAY_SIZE(fillvals); i++) {
+ btf_print_kind(BTF_KIND_STRUCT, "struct ", fillvals[i]);
+ btf_print_kind(BTF_KIND_UNION, "union ", fillvals[i]);
+ btf_print_kind(BTF_KIND_ENUM, "enum ", fillvals[i]);
+ btf_print_kind(BTF_KIND_TYPEDEF, "", fillvals[i]);
+ }
+
+ /* verify unknown type falls back to hashed pointer display */
+ test("(null)", "%pT", BTF_PTR_TYPE(NULL, "unknown_type"));
+ test_btf_hashed("%pT", BTF_PTR_TYPE(skb, "unknown_type"));
+
+ kfree_skb(skb);
+}
+
static void __init
test_pointer(void)
{
@@ -694,6 +1009,7 @@ static void __init fwnode_pointer(void)
flags();
errptr();
fwnode_pointer();
+ btf_pointer();
}
static void __init selftest(void)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v3 bpf-next 6/8] printk: extend test_printf to test %pT BTF-based format specifier
2020-06-23 12:07 ` [PATCH v3 bpf-next 6/8] printk: extend test_printf to test %pT BTF-based format specifier Alan Maguire
@ 2020-06-23 13:02 ` Andy Shevchenko
0 siblings, 0 replies; 18+ messages in thread
From: Andy Shevchenko @ 2020-06-23 13:02 UTC (permalink / raw)
To: Alan Maguire
Cc: ast, daniel, yhs, andriin, arnaldo.melo, kafai, songliubraving,
john.fastabend, kpsingh, linux, joe, pmladek, rostedt,
sergey.senozhatsky, corbet, bpf, netdev, linux-kernel, linux-doc
On Tue, Jun 23, 2020 at 01:07:09PM +0100, Alan Maguire wrote:
> Add tests to verify basic type display and to iterate through all
> enums, structs, unions and typedefs ensuring expected behaviour
> occurs. Since test_printf can be built as a module we need to
> export a BTF kind iterator function to allow us to iterate over
> all names of a particular BTF kind.
>
> These changes add up to approximately 20,000 new tests covering
> all enum, struct, union and typedefs in vmlinux BTF.
>
> Individual tests are also added for int, char, struct, enum
> and typedefs which verify output is as expected.
...
> #include <linux/mm.h>
>
> #include <linux/property.h>
+ blank line, you see, headers are grouped.
> +#include <linux/bpf.h>
> +#include <linux/btf.h>
> +#include <linux/skbuff.h>
> +#define __TEST_BTF(fmt, type, ptr, expected) \
> + test(expected, "%pT"fmt, ptr)
> +
> +#define TEST_BTF_C(type, var, ...) \
> + do { \
> + type var = __VA_ARGS__; \
> + struct btf_ptr *ptr = BTF_PTR_TYPE(&var, type); \
> + pr_debug("type %s: %pTc", #type, ptr); \
Hmm... Can't we modify test() (or underneath macros / functions) to do this?
> + __TEST_BTF("c", type, ptr, "(" #type ")" #__VA_ARGS__); \
> + } while (0)
> +
> +#define TEST_BTF(fmt, type, var, expected, ...) \
> + do { \
> + type var = __VA_ARGS__; \
> + struct btf_ptr *ptr = BTF_PTR_TYPE(&var, type); \
> + pr_debug("type %s: %pT"fmt, #type, ptr); \
> + __TEST_BTF(fmt, type, ptr, expected); \
> + } while (0)
...
> +static void __init
> +btf_print_kind(u8 kind, const char *kind_name, u64 fillval)
> +{
> + const char *fmt1 = "%pT", *fmt2 = "%pTN", *fmt3 = "%pT0";
This is hard to read. Provide a simple data structure or an array.
> + const char *name, *fmt = fmt1;
> + int i, res1, res2, res3, res4;
> + char type_name[256];
> + char *buf, *buf2;
> + u8 *dummy_data;
> + s32 id = 0;
> +
> + dummy_data = kzalloc(BTF_MAX_DATA_SIZE, GFP_KERNEL);
check?
> + /* fill our dummy data with supplied fillval. */
> + for (i = 0; i < BTF_MAX_DATA_SIZE; i++)
> + dummy_data[i] = fillval;
> + buf = kzalloc(BTF_MAX_DATA_SIZE, GFP_KERNEL);
> + buf2 = kzalloc(BTF_MAX_DATA_SIZE, GFP_KERNEL);
Ditto.
> + for (;;) {
> + name = btf_vmlinux_next_type_name(kind, &id);
> + if (!name)
> + break;
> +
> + total_tests++;
> +
> + snprintf(type_name, sizeof(type_name), "%s%s",
> + kind_name, name);
> +
> + res1 = snprintf(buf, BTF_MAX_DATA_SIZE, fmt1,
> + BTF_PTR_TYPE(dummy_data, type_name));
> + res2 = snprintf(buf, 0, fmt1,
> + BTF_PTR_TYPE(dummy_data, type_name));
> + res3 = snprintf(buf, BTF_MAX_DATA_SIZE, fmt2,
> + BTF_PTR_TYPE(dummy_data, type_name));
> + res4 = snprintf(buf, BTF_MAX_DATA_SIZE, fmt3,
> + BTF_PTR_TYPE(dummy_data, type_name));
> +
> + (void) snprintf(buf, BTF_MAX_DATA_SIZE, "%pT",
> + BTF_PTR_TYPE(dummy_data, type_name));
> + (void) snprintf(buf2, BTF_MAX_DATA_SIZE, "%pT",
> + BTF_PTR_TYPE(dummy_data, type_name));
> +
> + /*
> + * Ensure return value is > 0 and identical irrespective
> + * of whether we pass in a big enough buffer;
> + * also ensure that printing names always results in as
> + * long/longer buffer length.
> + */
> + if (res1 <= 0 || res2 <= 0 || res3 <= 0 || res4 <= 0) {
> + if (res3 <= 0)
> + fmt = fmt2;
> + if (res4 <= 0)
> + fmt = fmt3;
> + pr_warn("snprintf(%s%s); %d <= 0 (fmt %s)",
> + kind_name, name,
> + res1 <= 0 ? res1 : res2 <= 0 ? res2 :
> + res3 <= 0 ? res3 : res4, fmt);
> + failed_tests++;
For these kind of prints you can use a new macro, right?
> + } else if (res1 != res2) {
> + pr_warn("snprintf(%s%s): %d (to buf) != %d (no buf)",
> + kind_name, name, res1, res2);
> + failed_tests++;
Ditto.
> + } else if (res3 > res2) {
> + pr_warn("snprintf(%s%s); %d (no names) > %d (names)",
> + kind_name, name, res3, res2);
> + failed_tests++;
Ditto.
> + } else if (strcmp(buf, buf2) != 0) {
> + /* Safe and unsafe buffers should match. */
> + pr_warn("snprintf(%s%s); safe != unsafe",
> + kind_name, name);
> + pr_warn("safe: %s", buf);
> + pr_warn("unsafe: %s", buf2);
> + failed_tests++;
Perhaps also makes sense in a macro then somebody may reuse in the future.
That said, the first warning here somehow cryptic, please be more human friendly.
> + } else {
> + pr_debug("Printed %s%s (%d bytes)",
> + kind_name, name, res1);
> + }
> + }
> + kfree(dummy_data);
> + kfree(buf);
> + kfree(buf2);
> +}
...
> + TEST_BTF_C(int, testint, 1234);
> + TEST_BTF("cN", int, testint, "1234", 1234);
We use small letter macros in other cases. So can you?
...
> + /* typedef struct */
> + TEST_BTF_C(atomic_t, testtype, {.counter = (int)1,});
> + TEST_BTF("cN", atomic_t, testtype, "{1,}", {.counter = 1,});
> + /* typedef with 0 value should be printed at toplevel */
> + TEST_BTF("c", atomic_t, testtype, "(atomic_t){}", {.counter = 0,});
> + TEST_BTF("cN", atomic_t, testtype, "{}", {.counter = 0,});
> + TEST_BTF("c0", atomic_t, testtype, "(atomic_t){.counter = (int)0,}",
> + {.counter = 0,});
> + TEST_BTF("cN0", atomic_t, testtype, "{0,}", {.counter = 0,});
For one type, provide a data structure filled with test data and use loops.
Same for all similar places over the code.
...
> + u64 fillvals[] = { 0x0, 0xffffffffffffffff, 0x0123456789abcdef };
U64_MAX?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 bpf-next 7/8] bpf: add support for %pT format specifier for bpf_trace_printk() helper
2020-06-23 12:07 [PATCH v3 bpf-next 0/8] bpf, printk: add BTF-based type printing Alan Maguire
` (5 preceding siblings ...)
2020-06-23 12:07 ` [PATCH v3 bpf-next 6/8] printk: extend test_printf to test %pT BTF-based format specifier Alan Maguire
@ 2020-06-23 12:07 ` Alan Maguire
2020-06-23 13:04 ` Andy Shevchenko
2020-06-23 12:07 ` [PATCH v3 bpf-next 8/8] bpf/selftests: add tests for %pT format specifier Alan Maguire
2020-06-30 11:31 ` [PATCH v3 bpf-next 0/8] bpf, printk: add BTF-based type printing Sergey Senozhatsky
8 siblings, 1 reply; 18+ messages in thread
From: Alan Maguire @ 2020-06-23 12:07 UTC (permalink / raw)
To: ast, daniel, yhs, andriin, arnaldo.melo
Cc: kafai, songliubraving, john.fastabend, kpsingh, linux, joe,
pmladek, rostedt, sergey.senozhatsky, andriy.shevchenko, corbet,
bpf, netdev, linux-kernel, linux-doc
Allow %pT[cNx0] format specifier for BTF-based display of data associated
with pointer. The unsafe data modifier 'u' - where the source data
is traversed without copying it to a safe buffer via probe_kernel_read() -
is not supported.
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
include/uapi/linux/bpf.h | 27 ++++++++++++++++++++++-----
kernel/trace/bpf_trace.c | 24 +++++++++++++++++++++++-
tools/include/uapi/linux/bpf.h | 27 ++++++++++++++++++++++-----
3 files changed, 67 insertions(+), 11 deletions(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 1968481..ea4fbf3 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -702,7 +702,12 @@ struct bpf_stack_build_id {
* to file *\/sys/kernel/debug/tracing/trace* from DebugFS, if
* available. It can take up to three additional **u64**
* arguments (as an eBPF helpers, the total number of arguments is
- * limited to five).
+ * limited to five), and also supports %pT (BTF-based type
+ * printing), as long as BPF_READ lockdown is not active.
+ * "%pT" takes a "struct __btf_ptr *" as an argument; it
+ * consists of a pointer value and specified BTF type string or id
+ * used to select the type for display. For more details, see
+ * Documentation/core-api/printk-formats.rst.
*
* Each time the helper is called, it appends a line to the trace.
* Lines are discarded while *\/sys/kernel/debug/tracing/trace* is
@@ -738,10 +743,10 @@ struct bpf_stack_build_id {
* The conversion specifiers supported by *fmt* are similar, but
* more limited than for printk(). They are **%d**, **%i**,
* **%u**, **%x**, **%ld**, **%li**, **%lu**, **%lx**, **%lld**,
- * **%lli**, **%llu**, **%llx**, **%p**, **%s**. No modifier (size
- * of field, padding with zeroes, etc.) is available, and the
- * helper will return **-EINVAL** (but print nothing) if it
- * encounters an unknown specifier.
+ * **%lli**, **%llu**, **%llx**, **%p**, **%pT[cNx0], **%s**.
+ * Only %pT supports modifiers, and the helper will return
+ * **-EINVAL** (but print nothing) if it encouters an unknown
+ * specifier.
*
* Also, note that **bpf_trace_printk**\ () is slow, and should
* only be used for debugging purposes. For this reason, a notice
@@ -4260,4 +4265,16 @@ struct bpf_pidns_info {
__u32 pid;
__u32 tgid;
};
+
+/*
+ * struct __btf_ptr is used for %pT (typed pointer) display; the
+ * additional type string/BTF id are used to render the pointer
+ * data as the appropriate type.
+ */
+struct __btf_ptr {
+ void *ptr;
+ const char *type;
+ __u32 id;
+};
+
#endif /* _UAPI__LINUX_BPF_H__ */
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index e729c9e5..33ddb31 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -374,9 +374,13 @@ static void bpf_trace_copy_string(char *buf, void *unsafe_ptr, char fmt_ptype,
}
}
+/* Unsafe BTF display ('u' modifier) is absent here. */
+#define is_btf_safe_modifier(c) \
+ (c == 'c' || c == 'N' || c == 'x' || c == '0')
+
/*
* Only limited trace_printk() conversion specifiers allowed:
- * %d %i %u %x %ld %li %lu %lx %lld %lli %llu %llx %p %pks %pus %s
+ * %d %i %u %x %ld %li %lu %lx %lld %lli %llu %llx %p %pks %pus %s %pT
*/
BPF_CALL_5(bpf_trace_printk, char *, fmt, u32, fmt_size, u64, arg1,
u64, arg2, u64, arg3)
@@ -412,6 +416,24 @@ static void bpf_trace_copy_string(char *buf, void *unsafe_ptr, char fmt_ptype,
i++;
} else if (fmt[i] == 'p') {
mod[fmt_cnt]++;
+
+ /*
+ * allow BTF type-based printing, but disallow unsafe
+ * mode - this ensures the data is copied safely
+ * using probe_kernel_read() prior to traversing it.
+ */
+ if (fmt[i + 1] == 'T') {
+ int ret;
+
+ ret = security_locked_down(LOCKDOWN_BPF_READ);
+ if (unlikely(ret < 0))
+ return ret;
+ i += 2;
+ while (is_btf_safe_modifier(fmt[i]))
+ i++;
+ goto fmt_next;
+ }
+
if ((fmt[i + 1] == 'k' ||
fmt[i + 1] == 'u') &&
fmt[i + 2] == 's') {
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 1968481..ea4fbf3 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -702,7 +702,12 @@ struct bpf_stack_build_id {
* to file *\/sys/kernel/debug/tracing/trace* from DebugFS, if
* available. It can take up to three additional **u64**
* arguments (as an eBPF helpers, the total number of arguments is
- * limited to five).
+ * limited to five), and also supports %pT (BTF-based type
+ * printing), as long as BPF_READ lockdown is not active.
+ * "%pT" takes a "struct __btf_ptr *" as an argument; it
+ * consists of a pointer value and specified BTF type string or id
+ * used to select the type for display. For more details, see
+ * Documentation/core-api/printk-formats.rst.
*
* Each time the helper is called, it appends a line to the trace.
* Lines are discarded while *\/sys/kernel/debug/tracing/trace* is
@@ -738,10 +743,10 @@ struct bpf_stack_build_id {
* The conversion specifiers supported by *fmt* are similar, but
* more limited than for printk(). They are **%d**, **%i**,
* **%u**, **%x**, **%ld**, **%li**, **%lu**, **%lx**, **%lld**,
- * **%lli**, **%llu**, **%llx**, **%p**, **%s**. No modifier (size
- * of field, padding with zeroes, etc.) is available, and the
- * helper will return **-EINVAL** (but print nothing) if it
- * encounters an unknown specifier.
+ * **%lli**, **%llu**, **%llx**, **%p**, **%pT[cNx0], **%s**.
+ * Only %pT supports modifiers, and the helper will return
+ * **-EINVAL** (but print nothing) if it encouters an unknown
+ * specifier.
*
* Also, note that **bpf_trace_printk**\ () is slow, and should
* only be used for debugging purposes. For this reason, a notice
@@ -4260,4 +4265,16 @@ struct bpf_pidns_info {
__u32 pid;
__u32 tgid;
};
+
+/*
+ * struct __btf_ptr is used for %pT (typed pointer) display; the
+ * additional type string/BTF id are used to render the pointer
+ * data as the appropriate type.
+ */
+struct __btf_ptr {
+ void *ptr;
+ const char *type;
+ __u32 id;
+};
+
#endif /* _UAPI__LINUX_BPF_H__ */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v3 bpf-next 7/8] bpf: add support for %pT format specifier for bpf_trace_printk() helper
2020-06-23 12:07 ` [PATCH v3 bpf-next 7/8] bpf: add support for %pT format specifier for bpf_trace_printk() helper Alan Maguire
@ 2020-06-23 13:04 ` Andy Shevchenko
0 siblings, 0 replies; 18+ messages in thread
From: Andy Shevchenko @ 2020-06-23 13:04 UTC (permalink / raw)
To: Alan Maguire
Cc: ast, daniel, yhs, andriin, arnaldo.melo, kafai, songliubraving,
john.fastabend, kpsingh, linux, joe, pmladek, rostedt,
sergey.senozhatsky, corbet, bpf, netdev, linux-kernel, linux-doc
On Tue, Jun 23, 2020 at 01:07:10PM +0100, Alan Maguire wrote:
> Allow %pT[cNx0] format specifier for BTF-based display of data associated
> with pointer. The unsafe data modifier 'u' - where the source data
> is traversed without copying it to a safe buffer via probe_kernel_read() -
> is not supported.
I think I already saw this data structure definition...
> +/*
> + * struct __btf_ptr is used for %pT (typed pointer) display; the
> + * additional type string/BTF id are used to render the pointer
> + * data as the appropriate type.
> + */
> +struct __btf_ptr {
> + void *ptr;
> + const char *type;
> + __u32 id;
> +};
So, this is 2nd...
> +/*
> + * struct __btf_ptr is used for %pT (typed pointer) display; the
> + * additional type string/BTF id are used to render the pointer
> + * data as the appropriate type.
> + */
> +struct __btf_ptr {
> + void *ptr;
> + const char *type;
> + __u32 id;
> +};
,..and this is 3rd copy. Why?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 bpf-next 8/8] bpf/selftests: add tests for %pT format specifier
2020-06-23 12:07 [PATCH v3 bpf-next 0/8] bpf, printk: add BTF-based type printing Alan Maguire
` (6 preceding siblings ...)
2020-06-23 12:07 ` [PATCH v3 bpf-next 7/8] bpf: add support for %pT format specifier for bpf_trace_printk() helper Alan Maguire
@ 2020-06-23 12:07 ` Alan Maguire
2020-06-23 18:16 ` Andrii Nakryiko
2020-06-30 11:31 ` [PATCH v3 bpf-next 0/8] bpf, printk: add BTF-based type printing Sergey Senozhatsky
8 siblings, 1 reply; 18+ messages in thread
From: Alan Maguire @ 2020-06-23 12:07 UTC (permalink / raw)
To: ast, daniel, yhs, andriin, arnaldo.melo
Cc: kafai, songliubraving, john.fastabend, kpsingh, linux, joe,
pmladek, rostedt, sergey.senozhatsky, andriy.shevchenko, corbet,
bpf, netdev, linux-kernel, linux-doc
tests verify we get 0 return value from bpf_trace_print()
using %pT format specifier with various modifiers/pointer
values.
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
.../selftests/bpf/prog_tests/trace_printk_btf.c | 45 +++++++++++++++++++++
.../selftests/bpf/progs/netif_receive_skb.c | 47 ++++++++++++++++++++++
2 files changed, 92 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/trace_printk_btf.c
create mode 100644 tools/testing/selftests/bpf/progs/netif_receive_skb.c
diff --git a/tools/testing/selftests/bpf/prog_tests/trace_printk_btf.c b/tools/testing/selftests/bpf/prog_tests/trace_printk_btf.c
new file mode 100644
index 0000000..791eb97
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/trace_printk_btf.c
@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+
+#include "netif_receive_skb.skel.h"
+
+void test_trace_printk_btf(void)
+{
+ struct netif_receive_skb *skel;
+ struct netif_receive_skb__bss *bss;
+ int err, duration = 0;
+
+ skel = netif_receive_skb__open();
+ if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
+ return;
+
+ err = netif_receive_skb__load(skel);
+ if (CHECK(err, "skel_load", "failed to load skeleton: %d\n", err))
+ goto cleanup;
+
+ bss = skel->bss;
+
+ err = netif_receive_skb__attach(skel);
+ if (CHECK(err, "skel_attach", "skeleton attach failed: %d\n", err))
+ goto cleanup;
+
+ /* generate receive event */
+ system("ping -c 1 127.0.0.1 >/dev/null");
+
+ /*
+ * Make sure netif_receive_skb program was triggered
+ * and it set expected return values from bpf_trace_printk()s
+ * and all tests ran.
+ */
+ if (CHECK(bss->ret <= 0,
+ "bpf_trace_printk: got return value",
+ "ret <= 0 %d test %d\n", bss->ret, bss->num_subtests))
+ goto cleanup;
+
+ CHECK(bss->num_subtests != bss->ran_subtests, "check all subtests ran",
+ "only ran %d of %d tests\n", bss->num_subtests,
+ bss->ran_subtests);
+
+cleanup:
+ netif_receive_skb__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/netif_receive_skb.c b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
new file mode 100644
index 0000000..03ca1d8
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2020, Oracle and/or its affiliates. */
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+char _license[] SEC("license") = "GPL";
+
+int ret;
+int num_subtests;
+int ran_subtests;
+
+#define CHECK_PRINTK(_fmt, _p, res) \
+ do { \
+ char fmt[] = _fmt; \
+ ++num_subtests; \
+ if (ret >= 0) { \
+ ++ran_subtests; \
+ ret = bpf_trace_printk(fmt, sizeof(fmt), (_p)); \
+ } \
+ } while (0)
+
+/* TRACE_EVENT(netif_receive_skb,
+ * TP_PROTO(struct sk_buff *skb),
+ */
+SEC("tp_btf/netif_receive_skb")
+int BPF_PROG(trace_netif_receive_skb, struct sk_buff *skb)
+{
+ char skb_type[] = "struct sk_buff";
+ struct btf_ptr nullp = { .ptr = 0, .type = skb_type };
+ struct btf_ptr p = { .ptr = skb, .type = skb_type };
+
+ CHECK_PRINTK("%pT\n", &p, &res);
+ CHECK_PRINTK("%pTc\n", &p, &res);
+ CHECK_PRINTK("%pTN\n", &p, &res);
+ CHECK_PRINTK("%pTx\n", &p, &res);
+ CHECK_PRINTK("%pT0\n", &p, &res);
+ CHECK_PRINTK("%pTcNx0\n", &p, &res);
+ CHECK_PRINTK("%pT\n", &nullp, &res);
+ CHECK_PRINTK("%pTc\n", &nullp, &res);
+ CHECK_PRINTK("%pTN\n", &nullp, &res);
+ CHECK_PRINTK("%pTx\n", &nullp, &res);
+ CHECK_PRINTK("%pT0\n", &nullp, &res);
+ CHECK_PRINTK("%pTcNx0\n", &nullp, &res);
+
+ return 0;
+}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v3 bpf-next 8/8] bpf/selftests: add tests for %pT format specifier
2020-06-23 12:07 ` [PATCH v3 bpf-next 8/8] bpf/selftests: add tests for %pT format specifier Alan Maguire
@ 2020-06-23 18:16 ` Andrii Nakryiko
0 siblings, 0 replies; 18+ messages in thread
From: Andrii Nakryiko @ 2020-06-23 18:16 UTC (permalink / raw)
To: Alan Maguire
Cc: Alexei Starovoitov, Daniel Borkmann, Yonghong Song,
Andrii Nakryiko, Arnaldo Carvalho de Melo, Martin Lau, Song Liu,
john fastabend, KP Singh, Rasmus Villemoes, Joe Perches,
Petr Mladek, Steven Rostedt, Sergey Senozhatsky,
andriy.shevchenko, corbet, bpf, Networking, open list, linux-doc
On Tue, Jun 23, 2020 at 5:12 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> tests verify we get 0 return value from bpf_trace_print()
> using %pT format specifier with various modifiers/pointer
> values.
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
Acked-by: Andrii Nakryiko <andriin@fb.com>
> .../selftests/bpf/prog_tests/trace_printk_btf.c | 45 +++++++++++++++++++++
> .../selftests/bpf/progs/netif_receive_skb.c | 47 ++++++++++++++++++++++
> 2 files changed, 92 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/trace_printk_btf.c
> create mode 100644 tools/testing/selftests/bpf/progs/netif_receive_skb.c
>
[...]
> diff --git a/tools/testing/selftests/bpf/progs/netif_receive_skb.c b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> new file mode 100644
> index 0000000..03ca1d8
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2020, Oracle and/or its affiliates. */
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +
> +char _license[] SEC("license") = "GPL";
> +
> +int ret;
> +int num_subtests;
> +int ran_subtests;
oh, interesting, so Clang doesn't put these into the COM section
anymore? We used to need to explicitly zero-initialize these global
vars because of that.
> +
> +#define CHECK_PRINTK(_fmt, _p, res) \
> + do { \
> + char fmt[] = _fmt; \
pro tip: you can use `static const char fmt[] = _fmt` and it will just work.
> + ++num_subtests; \
> + if (ret >= 0) { \
> + ++ran_subtests; \
> + ret = bpf_trace_printk(fmt, sizeof(fmt), (_p)); \
> + } \
> + } while (0)
> +
> +/* TRACE_EVENT(netif_receive_skb,
> + * TP_PROTO(struct sk_buff *skb),
> + */
> +SEC("tp_btf/netif_receive_skb")
> +int BPF_PROG(trace_netif_receive_skb, struct sk_buff *skb)
> +{
> + char skb_type[] = "struct sk_buff";
same, `static const char` will generate more optimal code (good to
have good examples in selftests for people to follow). But don't
bother re-spinning just for this.
> + struct btf_ptr nullp = { .ptr = 0, .type = skb_type };
> + struct btf_ptr p = { .ptr = skb, .type = skb_type };
> +
> + CHECK_PRINTK("%pT\n", &p, &res);
> + CHECK_PRINTK("%pTc\n", &p, &res);
> + CHECK_PRINTK("%pTN\n", &p, &res);
> + CHECK_PRINTK("%pTx\n", &p, &res);
> + CHECK_PRINTK("%pT0\n", &p, &res);
> + CHECK_PRINTK("%pTcNx0\n", &p, &res);
> + CHECK_PRINTK("%pT\n", &nullp, &res);
> + CHECK_PRINTK("%pTc\n", &nullp, &res);
> + CHECK_PRINTK("%pTN\n", &nullp, &res);
> + CHECK_PRINTK("%pTx\n", &nullp, &res);
> + CHECK_PRINTK("%pT0\n", &nullp, &res);
> + CHECK_PRINTK("%pTcNx0\n", &nullp, &res);
> +
> + return 0;
> +}
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 bpf-next 0/8] bpf, printk: add BTF-based type printing
2020-06-23 12:07 [PATCH v3 bpf-next 0/8] bpf, printk: add BTF-based type printing Alan Maguire
` (7 preceding siblings ...)
2020-06-23 12:07 ` [PATCH v3 bpf-next 8/8] bpf/selftests: add tests for %pT format specifier Alan Maguire
@ 2020-06-30 11:31 ` Sergey Senozhatsky
8 siblings, 0 replies; 18+ messages in thread
From: Sergey Senozhatsky @ 2020-06-30 11:31 UTC (permalink / raw)
To: Alan Maguire
Cc: ast, daniel, yhs, andriin, arnaldo.melo, kafai, songliubraving,
john.fastabend, kpsingh, linux, joe, pmladek, rostedt,
sergey.senozhatsky, andriy.shevchenko, corbet, bpf, netdev,
linux-kernel, linux-doc
On (20/06/23 13:07), Alan Maguire wrote:
> struct sk_buff *skb = alloc_skb(64, GFP_KERNEL);
>
> pr_info("%pT", BTF_PTR_TYPE(skb, "struct sk_buff"));
>
> ...gives us:
>
> (struct sk_buff){
> .transport_header = (__u16)65535,
> .mac_header = (__u16)65535,
> .end = (sk_buff_data_t)192,
> .head = (unsigned char *)000000007524fd8b,
> .data = (unsigned char *)000000007524fd8b,
> .truesize = (unsigned int)768,
> .users = (refcount_t){
> .refs = (atomic_t){
> .counter = (int)1,
> },
> },
> }
Hmm. So this can expose the kernel memory layout (IOW do you print out real
%px pointers and so on)? If so, then I'd suggest not to use printk.
Unprivileged /dev/kmsg or /proc/kmsg reads are really OK thing for printk()
log buffer. And if you are going to print pointer hashes instead,
.transport_header = (__u16)65535,
.mac_header = (__u16)65535,
.end = (sk_buff_data_t)192,
.head = (unsigned char *)34897918740, // pointer_hash
.data = (unsigned char *)23942384983, // pointer hash
.truesize = (unsigned int)768,
.users = (refcount_t){
.refs = (atomic_t){
.counter = (int)1,
},
},
then the value of such printouts becomes a bit unclear to me, sorry.
Probably, something like a seq print into a file somewhere in
/sys/kernel/debug/foo, from which only privileged processes can
read, would be a better approach? My 5 cents.
-ss
^ permalink raw reply [flat|nested] 18+ messages in thread