[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <28145b05ee792b89ab9cb560f4f9989fd3d5d93b.camel@perches.com>
Date: Thu, 14 May 2020 16:43:24 -0700
From: Joe Perches <joe@...ches.com>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: Alan Maguire <alan.maguire@...cle.com>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
bpf <bpf@...r.kernel.org>,
Rasmus Villemoes <linux@...musvillemoes.dk>,
Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>,
Yonghong Song <yhs@...com>, Martin KaFai Lau <kafai@...com>,
Song Liu <songliubraving@...com>,
Andrii Nakryiko <andriin@...com>,
John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...omium.org>,
LKML <linux-kernel@...r.kernel.org>,
Network Development <netdev@...r.kernel.org>
Subject: Re: [PATCH v2 bpf-next 4/7] printk: add type-printing %pT format
specifier which uses BTF
On Wed, 2020-05-13 at 16:22 -0700, Joe Perches wrote:
> On Wed, 2020-05-13 at 16:07 -0700, Alexei Starovoitov wrote:
> > On Wed, May 13, 2020 at 4:05 PM Joe Perches <joe@...ches.com> wrote:
> > > On Tue, 2020-05-12 at 06:56 +0100, 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,
> > >
> > > Given
> > >
> > > #define BTF_INT_ENCODING(VAL) (((VAL) & 0x0f000000) >> 24)
> > >
> > > Maybe
> > >
> > > #define BTF_INT_SIGNED (1 << 0)
> > > #define BTF_INT_CHAR (1 << 1)
> > > #define BTF_INT_BOOL (1 << 2)
> > >
> > > could be extended to include
> > >
> > > #define BTF_INT_HEX (1 << 3)
> > >
> > > So hex values can be appropriately pretty-printed.
> >
> > Nack to that.
>
> why?
>
Tell me what's wrong with the idea.
Here's a possible implementation:
---
Documentation/bpf/btf.rst | 5 +++--
include/uapi/linux/btf.h | 1 +
kernel/bpf/btf.c | 5 ++++-
tools/bpf/bpftool/btf.c | 2 ++
tools/bpf/bpftool/btf_dumper.c | 13 +++++++++++++
tools/include/uapi/linux/btf.h | 1 +
6 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/Documentation/bpf/btf.rst b/Documentation/bpf/btf.rst
index 4d565d202ce3..56aaa189e7fb 100644
--- a/Documentation/bpf/btf.rst
+++ b/Documentation/bpf/btf.rst
@@ -139,10 +139,11 @@ The ``BTF_INT_ENCODING`` has the following attributes::
#define BTF_INT_SIGNED (1 << 0)
#define BTF_INT_CHAR (1 << 1)
#define BTF_INT_BOOL (1 << 2)
+ #define BTF_INT_HEX (1 << 3)
The ``BTF_INT_ENCODING()`` provides extra information: signedness, char, or
-bool, for the int type. The char and bool encoding are mostly useful for
-pretty print. At most one encoding can be specified for the int type.
+bool, for the int type. The char, bool and hex encodings are mostly useful
+for pretty print. At most one encoding can be specified for the int type.
The ``BTF_INT_BITS()`` specifies the number of actual bits held by this int
type. For example, a 4-bit bitfield encodes ``BTF_INT_BITS()`` equals to 4.
diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
index 5a667107ad2c..36f309209786 100644
--- a/include/uapi/linux/btf.h
+++ b/include/uapi/linux/btf.h
@@ -90,6 +90,7 @@ struct btf_type {
#define BTF_INT_SIGNED (1 << 0)
#define BTF_INT_CHAR (1 << 1)
#define BTF_INT_BOOL (1 << 2)
+#define BTF_INT_HEX (1 << 3)
/* BTF_KIND_ENUM is followed by multiple "struct btf_enum".
* The exact number of btf_enum is stored in the vlen (of the
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 58c9af1d4808..90bdc0635321 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -501,6 +501,8 @@ static const char *btf_int_encoding_str(u8 encoding)
return "CHAR";
else if (encoding == BTF_INT_BOOL)
return "BOOL";
+ else if (encoding == BTF_INT_HEX)
+ return "HEX";
else
return "UNKN";
}
@@ -1404,7 +1406,8 @@ static s32 btf_int_check_meta(struct btf_verifier_env *env,
if (encoding &&
encoding != BTF_INT_SIGNED &&
encoding != BTF_INT_CHAR &&
- encoding != BTF_INT_BOOL) {
+ encoding != BTF_INT_BOOL &&
+ encoding != BTF_INT_HEX) {
btf_verifier_log_type(env, t, "Unsupported encoding");
return -ENOTSUPP;
}
diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
index 41a1346934a1..44a129c40873 100644
--- a/tools/bpf/bpftool/btf.c
+++ b/tools/bpf/bpftool/btf.c
@@ -59,6 +59,8 @@ static const char *btf_int_enc_str(__u8 encoding)
return "CHAR";
case BTF_INT_BOOL:
return "BOOL";
+ case BTF_INT_HEX:
+ return "HEX";
default:
return "UNKN";
}
diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c
index ede162f83eea..96947ef92565 100644
--- a/tools/bpf/bpftool/btf_dumper.c
+++ b/tools/bpf/bpftool/btf_dumper.c
@@ -418,6 +418,19 @@ static int btf_dumper_int(const struct btf_type *t, __u8 bit_offset,
case BTF_INT_BOOL:
jsonw_bool(jw, *(int *)data);
break;
+ case BTF_INT_HEX:
+ if (BTF_INT_BITS(*int_type) == 64)
+ jsonw_printf(jw, "%llx", *(long long *)data);
+ else if (BTF_INT_BITS(*int_type) == 32)
+ jsonw_printf(jw, "%x", *(int *)data);
+ else if (BTF_INT_BITS(*int_type) == 16)
+ jsonw_printf(jw, "%hx", *(short *)data);
+ else if (BTF_INT_BITS(*int_type) == 8)
+ jsonw_printf(jw, "%hhx", *(char *)data);
+ else
+ btf_dumper_int_bits(*int_type, bit_offset, data, jw,
+ is_plain_text);
+ break;
default:
/* shouldn't happen */
return -EINVAL;
diff --git a/tools/include/uapi/linux/btf.h b/tools/include/uapi/linux/btf.h
index 5a667107ad2c..36f309209786 100644
--- a/tools/include/uapi/linux/btf.h
+++ b/tools/include/uapi/linux/btf.h
@@ -90,6 +90,7 @@ struct btf_type {
#define BTF_INT_SIGNED (1 << 0)
#define BTF_INT_CHAR (1 << 1)
#define BTF_INT_BOOL (1 << 2)
+#define BTF_INT_HEX (1 << 3)
/* BTF_KIND_ENUM is followed by multiple "struct btf_enum".
* The exact number of btf_enum is stored in the vlen (of the
Powered by blists - more mailing lists