Message-ID: <e6654ecd-c102-858f-2d9e-a9394ec28895@netronome.com>
Date:   Thu, 25 Apr 2019 20:20:32 +0100
From:   Quentin Monnet <quentin.monnet@...ronome.com>
To:     Andrii Nakryiko <andriin@...com>, andrii.nakryiko@...il.com,
        kernel-team@...com, netdev@...r.kernel.org, bpf@...r.kernel.org,
        ast@...com, daniel@...earbox.net, yhs@...com,
        songliubraving@...com, kafai@...com, acme@...nel.org
Cc:     Arnaldo Carvalho de Melo <acme@...hat.com>
Subject: Re: [PATCH v3 bpf-next 1/4] bpftool: add ability to dump BTF types

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?

> +
> +#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.

> +
> +	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.

> +
> +		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"?

Powered by blists - more mailing lists