Messages in this thread Patch in this message | | | Subject | Re: [PATCH v2 bpf-next 2/7] bpf: move to generic BTF show support, apply it to seq files/strings | From | Yonghong Song <> | Date | Wed, 13 May 2020 16:04:26 -0700 |
| |
On 5/11/20 10:56 PM, Alan Maguire wrote: > 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. > > 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_ZERO - show zeroed values (by default they are not shown). > > Signed-off-by: Alan Maguire <alan.maguire@oracle.com> > --- > include/linux/btf.h | 33 +++ > kernel/bpf/btf.c | 759 +++++++++++++++++++++++++++++++++++++++++++++------- > 2 files changed, 690 insertions(+), 102 deletions(-) > > diff --git a/include/linux/btf.h b/include/linux/btf.h > index 5c1ea99..d571125 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,40 @@ 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 > + */ > +#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) > + > 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 dcd2331..edf6455 100644 > --- a/kernel/bpf/btf.c > +++ b/kernel/bpf/btf.c > @@ -281,6 +281,32 @@ static const char *btf_type_str(const struct btf_type *t) > return btf_kind_str[BTF_INFO_KIND(t->info)]; > } > > +/* > + * 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. > + */ > +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_shown; > + u8 depth_check;
I have some difficulties to understand the relationship between the above three variables. Could you add some comments here?
> + u8 array_member:1, > + array_terminated:1; > + u16 array_encoding; > + u32 type_id; > + const struct btf_type *type; > + const struct btf_member *member; > + char name[KSYM_NAME_LEN]; /* scratch space for name */ > + char type_name[KSYM_NAME_LEN]; /* scratch space for type */
KSYM_NAME_LEN is for symbol name, not for type name. But I guess in kernel we probably do not have > 128 bytes type name so we should be okay here.
> + } state; > +}; > + > struct btf_kind_operations { > s32 (*check_meta)(struct btf_verifier_env *env, > const struct btf_type *t, > @@ -297,9 +323,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 +702,340 @@ 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) > + > +static inline const char *btf_show_type_name(struct btf_show *show, > + const struct btf_type *t) > +{ > + const char *array_suffixes = "[][][][][][][][][][]";
Add a comment here saying length BTF_SHOW_MAX_ITER * 2 so later on if somebody changes the BTF_SHOW_MAX_ITER from 10 to 12, it won't miss here?
> + const char *array_suffix = &array_suffixes[strlen(array_suffixes)]; > + const char *ptr_suffixes = "**********";
The same here.
> + const char *ptr_suffix = &ptr_suffixes[strlen(ptr_suffixes)]; > + const char *type_name = NULL, *prefix = "", *parens = ""; > + const struct btf_array *array; > + u32 id = show->state.type_id; > + bool allow_anon = true; > + u64 kinds = 0; > + int i; > + > + show->state.type_name[0] = '\0'; > + > + /* > + * 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 show->state.type_name; > + > + /* > + * 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 (!type_name) > + type_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 = "["; > + array = btf_type_array(t); > + if (!array) > + return show->state.type_name; > + if (!t) > + return show->state.type_name; > + 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); > + if (!t) > + return show->state.type_name; > + }
Do we do pointer tracing here? For example struct t { int *m[5]; }
When trying to access memory, the above code may go through ptr->array and out of loop when hitting array element type "int"? > + /* We may not be able to represent this type; bail to be safe */ > + if (i == BTF_SHOW_MAX_ITER) > + return show->state.type_name; > + > + if (!type_name) > + type_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: > + allow_anon = false; > + 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 (!type_name || strlen(type_name) == 0) { > + if (allow_anon) > + type_name = ""; > + else > + return show->state.type_name; > + } > + > + /* Even if we don't want type name info, we want parentheses etc */ > + if (show->flags & BTF_SHOW_NONAME) > + snprintf(show->state.type_name, sizeof(show->state.type_name), > + "%s", parens); > + else > + snprintf(show->state.type_name, sizeof(show->state.type_name), > + "(%s%s%s%s%s%s)%s", > + prefix, > + strlen(prefix) > 0 && strlen(type_name) > 0 ? " " : "", > + type_name, > + strlen(ptr_suffix) > 0 ? " " : "", ptr_suffix, > + array_suffix, parens); > + > + return show->state.type_name; > +} > + > +static inline const char *btf_show_name(struct btf_show *show) > +{ > + const struct btf_type *t = show->state.type; > + const struct btf_member *m = show->state.member; > + const char *member = NULL; > + const char *type = ""; > + > + show->state.name[0] = '\0'; > + > + if ((!m && !t) || show->state.array_member) > + return show->state.name; > + > + if (m) > + t = btf_type_skip_qualifiers(show->btf, m->type); > + > + if (t) { > + type = btf_show_type_name(show, t); > + if (!type) > + return show->state.name; > + } > + > + if (m && !(show->flags & BTF_SHOW_NONAME)) { > + member = btf_name_by_offset(show->btf, m->name_off); > + if (member && strlen(member) > 0) { > + snprintf(show->state.name, sizeof(show->state.name), > + ".%s = %s", member, type); > + return show->state.name; > + } > + } > + > + snprintf(show->state.name, sizeof(show->state.name), "%s", type); > + > + return show->state.name; > +} > + > +#define btf_show(show, ...) \ > + do { \ > + if (!show->state.depth_check) \
As I mentioned above, some comments will be good to understand here.
> + 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_shown) \ > + show->state.depth_shown = 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_shown) \ > + show->state.depth_shown = show->state.depth; \ > + } while (0) > + [...] > > static int btf_array_check_member(struct btf_verifier_env *env, > @@ -2104,28 +2489,87 @@ 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;
Some char array may be printable and some may not be printable, how did you differentiate this?
> + } > + > + btf_show_start_array_type(show, t, type_id, encoding); > + > + 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); > +} > + [...]
| |