Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Alan Maguire <alan.maguire@oracle.com>
To:     ast@kernel.org, daniel@iogearbox.net, yhs@fb.com
Cc:     kafai@fb.com, songliubraving@fb.com, andriin@fb.com,
        john.fastabend@gmail.com, kpsingh@chromium.org,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        bpf@vger.kernel.org, Alan Maguire <alan.maguire@oracle.com>
Subject: [RFC PATCH bpf-next 0/6] bpf, printk: add BTF-based type printing
Date:   Fri, 17 Apr 2020 11:42:34 +0100
Message-Id: <1587120160-3030-1-git-send-email-alan.maguire@oracle.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

The printk family of functions support printing specific pointer types
using %p format specifiers (MAC addresses, IP addresses, etc).  For
full details see Documentation/core-api/printk-formats.rst.

This RFC patchset proposes introducing a "print typed pointer" format
specifier "%pT<type>"; the type specified is then looked up in the BPF
Type Format (BTF) information provided for vmlinux to support display.

There is already support in kernel/bpf/btf.c for "show" functionality;
the changes here generalize that support from seq-file specific
verifier display to the more generic case and add another specific
use case; snprintf()-style rendering of type information to a
provided buffer.  This support is then used to support printk
rendering of types, but the intent is to provide a function
that might be useful in other in-kernel scenarios; for example:

- ftrace could possibly utilize the function to support typed
  display of function arguments by cross-referencing BTF function
  information to derive the types of arguments
- oops/panic messaging could extend the information displayed to
  dig into data structures associated with failing functions

The above potential use cases hint at a potential reply to
a reasonable objection that such typed display should be
solved by tracing programs, where the in kernel tracing records
data and the userspace program prints it out.  While this
is certainly the recommended approach for most cases, I
believe having an in-kernel mechanism would be valuable
also.

The function the printk() family of functions rely on
could potentially be used directly for other use cases
like ftrace where we might have the BTF ids of the
pointers we wish to display; its signature is as follows:

int btf_type_snprintf_show(const struct btf *btf, u32 type_id, void *obj,
                           char *buf, int len, u64 flags);

So if ftrace say had the BTF ids of the types of arguments,
we see that the above would allow us to convert the
pointer data into displayable form.

To give a flavour for what the printed-out data looks like,
here we use pr_info() to display a struct sk_buff *.  Note
we specify the 'N' modifier to show type field names:

  struct sk_buff *skb = alloc_skb(64, GFP_KERNEL);

  pr_info("%pTN<struct sk_buff>", skb);

...gives us:

{{{.next=00000000c7916e9c,.prev=00000000c7916e9c,{.dev=00000000c7916e9c|.dev_scratch=0}}|.rbnode={.__rb_parent_color=0,.rb_right=00000000c7916e9c,.rb_left=00000000c7916e9c}|.list={.next=00000000c7916e9c,.prev=00000000c7916e9c}},{.sk=00000000c7916e9c|.ip_defrag_offset=0},{.tstamp=0|.skb_mstamp_ns=0},.cb=['\0'],{{._skb_refdst=0,.destructor=00000000c7916e9c}|.tcp_tsorted_anchor={.next=00000000c7916e9c,.prev=00000000c7916e9c}},._nfct=0,.len=0,.data_len=0,.mac_len=0,.hdr_len=0,.queue_mapping=0,.__cloned_offset=[],.cloned=0x0,.nohdr=0x0,.fclone=0x0,.peeked=0x0,.head_frag=0x0,.pfmemalloc=0x0,.active_extensions=0,.headers_start=[],.__pkt_type_offset=[],.pkt_type=0x0,.ignore_df=0x0,.nf_trace=0x0,.ip_summed=0x0,.ooo_okay=0x0,.l4_hash=0x0,.sw_hash=0x0,.wifi_acked_valid=0x0,.wifi_acked=0x0,.no_fcs=0x0,.encapsulation=0x0,.encap_hdr_csum=0x0,.csum_valid=0x0,.__pkt_vlan_present_offset=[],.vlan_present=0x0,.csum_complete_sw=0x0,.csum_level=0x0,.csum_not_inet=0x0,.dst_pending_co

printk output is truncated at 1024 bytes.  For such cases, the compact
display mode (minus the field info 'N') may be used at the expense of
readability. "|" differentiates between different union members, but
aside from that the format is intended to minic a valid C initiailizer
for the given type.

The hope is that this functionality will be useful for debugging,
and possibly help facilitate the cases mentioned above in the future.

The patches are marked RFC for several reasons

- There's already an RFC patchset in flight dealing with BTF dumping;

https://www.spinics.net/lists/netdev/msg644412.html

  The reason I'm posting this is the approach is a bit different 
  and there may be ways of synthesizing the approaches.
- The mechanism of vmlinux BTF initialization is not fit for purpose
  in a printk() setting as I understand it (it uses mutex locking
  to prevent multiple initializations of the BTF info).  A simple
  approach to support printk might be to simply initialize the
  BTF vmlinux case early in boot; it only needs to happen once.
  Any suggestions here would be great.
- BTF-based rendering is more complex than other printk() format
  specifier-driven methods; that said, because of its generality it
  does provide significant value I think
- More tests are needed.

Alan Maguire (6):
  bpf: provide function to get vmlinux BTF information
  bpf: btf->resolved_[ids,sizes] should not be used for vmlinux BTF
  bpf: move to generic BTF show support, apply it to seq files/strings
  checkpatch: add new BTF pointer format specifier
  printk: add type-printing %pT<type> format specifier which uses BTF
  printk: extend test_printf to test %pT BTF-based format specifier

 Documentation/core-api/printk-formats.rst |   8 +
 include/linux/bpf.h                       |   2 +
 include/linux/btf.h                       |  35 ++-
 kernel/bpf/btf.c                          | 466 ++++++++++++++++++++++++------
 kernel/bpf/verifier.c                     |  18 +-
 lib/Kconfig                               |  16 +
 lib/test_printf.c                         | 118 ++++++++
 lib/vsprintf.c                            | 145 +++++++++-
 scripts/checkpatch.pl                     |   2 +-
 9 files changed, 704 insertions(+), 106 deletions(-)

-- 
1.8.3.1