Messages in this thread | | | Subject | Re: [PATCH v2 bpf-next 4/7] printk: add type-printing %pT format specifier which uses BTF | From | Yonghong Song <> | Date | Thu, 14 May 2020 17:39:55 -0700 |
| |
On 5/14/20 3:37 PM, Alan Maguire wrote: > > > On Wed, 13 May 2020, Yonghong Song wrote: > >> >> >> On 5/11/20 10:56 PM, 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 *)000000006b71155a, >>> .data = (unsigned char *)000000006b71155a, >>> .truesize = (unsigned int)768, >>> .users = (refcount_t){ >>> .refs = (atomic_t){ >>> .counter = (int)1, >>> }, >>> }, >>> .extensions = (struct skb_ext *)00000000f486a130, >>> } >>> >>> 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> >>> --- >>> Documentation/core-api/printk-formats.rst | 15 ++++ >>> include/linux/btf.h | 3 +- >>> include/linux/printk.h | 16 +++++ >>> lib/Kconfig | 16 +++++ >>> lib/vsprintf.c | 113 >>> ++++++++++++++++++++++++++++++ >>> 5 files changed, 162 insertions(+), 1 deletion(-) >>> >>> diff --git a/Documentation/core-api/printk-formats.rst >>> b/Documentation/core-api/printk-formats.rst >>> index 8ebe46b1..5c66097 100644 >>> --- a/Documentation/core-api/printk-formats.rst >>> +++ b/Documentation/core-api/printk-formats.rst >>> @@ -545,6 +545,21 @@ 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 >>> + '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 d571125..7b585ab 100644 >>> --- a/include/linux/btf.h >>> +++ b/include/linux/btf.h >>> @@ -169,10 +169,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 fcde0772..3c3ea53 100644 >>> --- a/include/linux/printk.h >>> +++ b/include/linux/printk.h >>> @@ -528,4 +528,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 7c488a1..f9276f8 100644 >>> --- a/lib/vsprintf.c >>> +++ b/lib/vsprintf.c >>> @@ -43,6 +43,7 @@ >>> #ifdef CONFIG_BLOCK >>> #include <linux/blkdev.h> >>> #endif >>> +#include <linux/btf.h> >>> >>> #include "../mm/internal.h" /* For the trace_print_flags arrays */ >>> >>> @@ -2059,6 +2060,103 @@ char *fwnode_string(char *buf, char *end, struct >>> fwnode_handle *fwnode, >>> return widen_string(buf, buf - buf_start, end, spec); >>> } >>> >>> +#if IS_ENABLED(CONFIG_BTF_PRINTF) >>> +#define btf_modifier_flag(c) (c == 'c' ? BTF_SHOW_COMPACT : \ >>> + c == 'N' ? BTF_SHOW_NONAME : \ >>> + c == 'x' ? BTF_SHOW_PTR_RAW : \ >>> + 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 "); >>> + } >> >> I think typedef should be supported here. >> In kernel, we have some structure directly defined as typedef's. >> A lot of internal int types also typedefs, like u32, atomic_t, >> possible_net_t, etc. >> >> A type name without prefix "struct", "union", "enum" can be >> treated as a typedef first. >> > > That's how the code works today; we start with a typedef assumption. > See the comment below starting "Assume type specified is a typedef"; > we initialize btf_kind to be a typedef above; it's only changed > to an BTF_KIND_INT if we find a struct/enum/union prefix or if lookup > using the typedef kind fails. I should probably make this clearer > though (move the comment up maybe?). Thanks for taking a look!
Thanks for explanation. I missed it. Move the comments up about what to support explicitly will be good.
> >> If the type name is not a typedef, it is then compared to a limited >> number of C basic int types like "char", "unsigned char", "short", >> "unsigned short", ... >> >>> + >>> + 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); >>> +} [...]
| |