[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAEf4BzbboN0rLqNywMuR79+E9Gqm2OCpT8m8DmwABMRLwwUdMA@mail.gmail.com>
Date: Thu, 25 Apr 2019 15:00:46 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Quentin Monnet <quentin.monnet@...ronome.com>
Cc: Andrii Nakryiko <andriin@...com>, Kernel Team <kernel-team@...com>,
Networking <netdev@...r.kernel.org>, bpf@...r.kernel.org,
Alexei Starovoitov <ast@...com>,
Daniel Borkmann <daniel@...earbox.net>,
Yonghong Song <yhs@...com>, Song Liu <songliubraving@...com>,
Martin Lau <kafai@...com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Arnaldo Carvalho de Melo <acme@...hat.com>
Subject: Re: [PATCH v3 bpf-next 1/4] bpftool: add ability to dump BTF types
On Thu, Apr 25, 2019 at 12:20 PM Quentin Monnet
<quentin.monnet@...ronome.com> wrote:
>
> 2019-04-25 09:55 UTC-0700 ~ Andrii Nakryiko <andriin@...com>
> > Add new `btf dump` sub-command to bpftool. It allows to dump
> > human-readable low-level BTF types representation of BTF types. BTF can
> > be retrieved from few different sources:
> > - from BTF object by ID;
> > - from PROG, if it has associated BTF;
> > - from MAP, if it has associated BTF data; it's possible to narrow
> > down types to either key type, value type, both, or all BTF types;
> > - from ELF file (.BTF section).
> >
> > Output format mostly follows BPF verifier log format with few notable
> > exceptions:
> > - all the type/field/param/etc names are enclosed in single quotes to
> > allow easier grepping and to stand out a little bit more;
> > - FUNC_PROTO output follows STRUCT/UNION/ENUM format of having one
> > line per each argument; this is more uniform and allows easy
> > grepping, as opposed to succinct, but inconvenient format that BPF
> > verifier log is using.
> >
> > Cc: Daniel Borkmann <daniel@...earbox.net>
> > Cc: Alexei Starovoitov <ast@...com>
> > Cc: Yonghong Song <yhs@...com>
> > Cc: Martin KaFai Lau <kafai@...com>
> > Cc: Song Liu <songliubraving@...com>
> > Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
> > Acked-by: Yonghong Song <yhs@...com>
> > Signed-off-by: Andrii Nakryiko <andriin@...com>
> > ---
> > tools/bpf/bpftool/btf.c | 580 +++++++++++++++++++++++++++++++++++++++
> > tools/bpf/bpftool/main.c | 3 +-
> > tools/bpf/bpftool/main.h | 1 +
> > 3 files changed, 583 insertions(+), 1 deletion(-)
> > create mode 100644 tools/bpf/bpftool/btf.c
> >
> > diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
> > new file mode 100644
> > index 000000000000..cbf04850c798
> > --- /dev/null
> > +++ b/tools/bpf/bpftool/btf.c
> > @@ -0,0 +1,580 @@
> > +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +/* Copyright (C) 2019 Facebook */
> > +
> > +#include <errno.h>
> > +#include <fcntl.h>
> > +#include <linux/err.h>
> > +#include <stdbool.h>
> > +#include <stdio.h>
> > +#include <string.h>
> > +#include <unistd.h>
> > +#include <gelf.h>
> > +#include <bpf.h>
> > +#include <linux/btf.h>
>
> Can we have as in prog.c/map.c: standard includes sorted alphabetically,
> then linux/ includes, then bpf includes?
Sure, will do.
>
> > +
> > +#include "btf.h"
> > +#include "json_writer.h"
> > +#include "main.h"
> > +
> > +static const char * const btf_kind_str[NR_BTF_KINDS] = {
> > + [BTF_KIND_UNKN] = "UNKNOWN",
> > + [BTF_KIND_INT] = "INT",
> > + [BTF_KIND_PTR] = "PTR",
> > + [BTF_KIND_ARRAY] = "ARRAY",
> > + [BTF_KIND_STRUCT] = "STRUCT",
> > + [BTF_KIND_UNION] = "UNION",
> > + [BTF_KIND_ENUM] = "ENUM",
> > + [BTF_KIND_FWD] = "FWD",
> > + [BTF_KIND_TYPEDEF] = "TYPEDEF",
> > + [BTF_KIND_VOLATILE] = "VOLATILE",
> > + [BTF_KIND_CONST] = "CONST",
> > + [BTF_KIND_RESTRICT] = "RESTRICT",
> > + [BTF_KIND_FUNC] = "FUNC",
> > + [BTF_KIND_FUNC_PROTO] = "FUNC_PROTO",
> > + [BTF_KIND_VAR] = "VAR",
> > + [BTF_KIND_DATASEC] = "DATASEC",
> > +};
> > +
> > +static const char *btf_int_enc_str(__u8 encoding)
> > +{
> > + switch (encoding) {
> > + case 0:
> > + return "(none)";
> > + case BTF_INT_SIGNED:
> > + return "SIGNED";
> > + case BTF_INT_CHAR:
> > + return "CHAR";
> > + case BTF_INT_BOOL:
> > + return "BOOL";
> > + default:
> > + return "UNKN";
> > + }
> > +}
> > +
> > +static const char *btf_var_linkage_str(__u32 linkage)
> > +{
> > + switch (linkage) {
> > + case BTF_VAR_STATIC:
> > + return "static";
> > + case BTF_VAR_GLOBAL_ALLOCATED:
> > + return "global-alloc";
> > + default:
> > + return "(unknown)";
> > + }
> > +}
> > +
> > +static const char *btf_str(const struct btf *btf, __u32 off)
> > +{
> > + if (!off)
> > + return "(anon)";
> > + return btf__name_by_offset(btf, off) ? : "(invalid)";
> > +}
> > +
> > +static int dump_btf_type(const struct btf *btf, __u32 id,
> > + const struct btf_type *t)
> > +{
> > + int kind = BTF_INFO_KIND(t->info);
> > + int safe_kind = kind <= BTF_KIND_MAX ? kind : BTF_KIND_UNKN;
> > + json_writer_t *w = json_wtr;
>
> Can we keep reverse-Christmas tree style for declarations? Assigning the
> values can be done on its own after the declarations.
Ok.
>
> > +
> > + if (json_output) {
> > + jsonw_start_object(w);
> > + jsonw_uint_field(w, "id", id);
> > + jsonw_string_field(w, "kind", btf_kind_str[safe_kind]);
> > + jsonw_string_field(w, "name", btf_str(btf, t->name_off));
> > + } else {
> > + printf("[%u] %s '%s'", id, btf_kind_str[safe_kind],
> > + btf_str(btf, t->name_off));
> > + }
> > +
> > + switch (BTF_INFO_KIND(t->info)) {
> > + case BTF_KIND_INT: {
> > + __u32 v = *(__u32 *)(t + 1);
> > + const char *enc = btf_int_enc_str(BTF_INT_ENCODING(v));
>
> Same thing here.
Fixing.
>
> > +
> > + if (json_output) {
> > + jsonw_uint_field(w, "size", t->size);
> > + jsonw_uint_field(w, "bits_offset", BTF_INT_OFFSET(v));
> > + jsonw_uint_field(w, "nr_bits", BTF_INT_BITS(v));
> > + jsonw_string_field(w, "encoding", enc);
> > + } else {
> > + printf(" size=%u bits_offset=%u nr_bits=%u encoding=%s",
> > + t->size, BTF_INT_OFFSET(v), BTF_INT_BITS(v),
> > + enc);
> > + }
> > + break;
> > + }
> > + case BTF_KIND_PTR:
>
> [...]
>
> printf(" type_id=%u", t->type);
> > + break;
> > + case BTF_KIND_FUNC_PROTO: {
> > + const struct btf_param *p = (const void *)(t + 1);
> > + __u16 vlen = BTF_INFO_VLEN(t->info);
> > + int i;
> > +
> > + if (json_output) {
> > + jsonw_uint_field(w, "ret_type_id", t->type);
> > + jsonw_uint_field(w, "vlen", vlen);
> > + jsonw_name(w, "params");
> > + jsonw_start_array(w);
> > + } else {
> > + printf(" ret_type_id=%u vlen=%u", t->type, vlen);
> > + }
> > + for (i = 0; i < vlen; i++, p++) {
> > + const char *name = btf_str(btf, p->name_off);
> > +
> > + if (json_output) {
> > + jsonw_start_object(w);
> > + jsonw_string_field(w, "name", name);
> > + jsonw_uint_field(w, "type_id", p->type);
> > + jsonw_end_object(w);
> > + } else {
> > + printf("\n\t'%s' type_id=%u", name, p->type);
> > + }
> > + }
> > + if (json_output)
> > + jsonw_end_array(w);
> > + break;
> > + }
> > + case BTF_KIND_VAR: {
> > + const struct btf_var *v = (const void *)(t + 1);
> > + const char *linkage = btf_var_linkage_str(v->linkage);
>
> And here please.
>
> > +
> > + if (json_output) {
> > + jsonw_uint_field(w, "type_id", t->type);
> > + jsonw_string_field(w, "linkage", linkage);
> > + } else {
> > + printf(" type_id=%u, linkage=%s", t->type, linkage);
> > + }
> > + break;
> > + }
> > + case BTF_KIND_DATASEC: {
>
> [...]
>
> > +static int do_help(int argc, char **argv)
> > +{
> > + if (json_output) {
> > + jsonw_null(json_wtr);
> > + return 0;
> > + }
> > +
> > + fprintf(stderr,
> > + "Usage: %s btf dump BTF_SRC\n"
>
> Why so much space between "dump" and "BTF_SRC"?
Followed map.c's style, which has that much space (though in that case
it's aligned on longest command). But seems like prog.c doesn't follow
that. I'll make spacing tighter.
Powered by blists - more mailing lists