lkml.org 
[lkml]   [2020]   [May]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v2 bpf-next 4/7] printk: add type-printing %pT format specifier which uses BTF
    From
    Date


    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);
    >>> +}
    [...]

    \
     
     \ /
      Last update: 2020-05-15 02:41    [W:2.327 / U:0.052 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site