Re: [PATCH bpf-next 1/2] bpftool: add ability to dump BTF types

* Re: [PATCH bpf-next 1/2] bpftool: add ability to dump BTF types
       [not found] ` <20190424052200.1625142-2-andrii.nakryiko@gmail.com>
@ 2019-04-24 17:12   ` Yonghong Song
  2019-04-24 18:12     ` Andrii Nakryiko
  2019-04-24 17:17   ` Yonghong Song
  1 sibling, 1 reply; 7+ messages in thread
From: Yonghong Song @ 2019-04-24 17:12 UTC (permalink / raw)
  To: andrii.nakryiko, Kernel Team, netdev, bpf, Alexei Starovoitov,
	daniel, Song Liu, Martin Lau, acme
  Cc: Andrii Nakryiko, Arnaldo Carvalho de Melo

On 4/23/19 10:21 PM, andrii.nakryiko@gmail.com wrote:
> From: Andrii Nakryiko <andriin@fb.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).
This is great. Thanks!
> 
> 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@iogearbox.net>
> Cc: Alexei Starovoitov <ast@fb.com>
> Cc: Yonghong Song <yhs@fb.com>
> Cc: Martin KaFai Lau <kafai@fb.com>
> Cc: Song Liu <songliubraving@fb.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>   tools/bpf/bpftool/btf.c  | 576 +++++++++++++++++++++++++++++++++++++++
>   tools/bpf/bpftool/main.c |   1 +
>   tools/bpf/bpftool/main.h |   1 +
>   3 files changed, 578 insertions(+)
>   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..afe5cb7bab0c
> --- /dev/null
> +++ b/tools/bpf/bpftool/btf.c
> @@ -0,0 +1,576 @@
> +// 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>
> +
[.....]
> +
> +static int do_dump(int argc, char **argv)
> +{
> +	struct btf *btf = NULL;
> +	__u32 root_type_ids[2];
> +	int root_type_cnt = 0;
> +	__u32 btf_id = -1;
> +	const char *src;
> +	int fd = -1;
> +	int err;
> +
> +	if (!REQ_ARGS(2)) {
> +		usage();
> +		return -1;
> +	}
> +	src = GET_ARG();
> +
> +	if (is_prefix(src, "map")) {
> +		struct bpf_map_info info = {};
> +		__u32 len = sizeof(info);
> +
> +		if (!REQ_ARGS(2)) {
> +			usage();
> +			return -1;
> +		}
> +
> +		fd = map_parse_fd_and_info(&argc, &argv, &info, &len);
> +		if (fd < 0)
> +			return -1;
> +
> +		btf_id = info.btf_id;
> +		if (argc && is_prefix(*argv, "key")) {
> +			root_type_ids[root_type_cnt++] = info.btf_key_type_id;
> +			NEXT_ARG();
> +		} else if (argc && is_prefix(*argv, "value")) {
> +			root_type_ids[root_type_cnt++] = info.btf_value_type_id;
> +			NEXT_ARG();
> +		} else if (argc && is_prefix(*argv, "all")) {
> +			NEXT_ARG();
> +		} else if (argc && is_prefix(*argv, "kv")) {
> +			root_type_ids[root_type_cnt++] = info.btf_key_type_id;
> +			root_type_ids[root_type_cnt++] = info.btf_value_type_id;
> +			NEXT_ARG();
> +		} else {
> +			root_type_ids[root_type_cnt++] = info.btf_key_type_id;
> +			root_type_ids[root_type_cnt++] = info.btf_value_type_id;
> +		}
> +	} else if (is_prefix(src, "prog")) {
> +		struct bpf_prog_info info = {};
> +		__u32 len = sizeof(info);
> +
> +		if (!REQ_ARGS(2)) {
> +			usage();
> +			return -1;
> +		}
> +
> +		fd = prog_parse_fd(&argc, &argv);
> +		if (fd < 0)
> +			return -1;
> +
> +		err = bpf_obj_get_info_by_fd(fd, &info, &len);
> +		if (err) {
> +			p_err("can't get prog info: %s", strerror(errno));
> +			goto done;
> +		}
> +
> +		btf_id = info.btf_id;
> +	} else if (is_prefix(src, "id")) {
> +		char *endptr;
> +
> +		btf_id = strtoul(*argv, &endptr, 0);
> +		if (*endptr) {
> +			p_err("can't parse %s as ID", **argv);
> +			return -1;
> +		}
> +		NEXT_ARG();
> +	} else if (is_prefix(src, "file")) {
> +		err = btf_load_from_elf(*argv, &btf);
> +		if (err)
> +			goto done;
> +		NEXT_ARG();
> +	}

Another "else" branch here to handle wrong prefix? Otherwise, it will 
fall down to "can't find btf with ID (4294967295)" where "4294967295 == 
-1) which is a little misleading for users.

> +
> +	if (!btf) {
> +		err = btf__get_from_id(btf_id, &btf);
> +		if (err) {
> +			p_err("get btf by id (%u): %s", btf_id, strerror(err));
> +			goto done;
> +		}
> +		if (!btf) {
> +			err = ENOENT;
> +			p_err("can't find btf with ID (%u)", btf_id);
> +			goto done;
> +		}
> +	}
> +
> +	dump_btf_raw(btf, root_type_ids, root_type_cnt);
> +
> +done:
> +	close(fd);
> +	btf__free(btf);
> +	return err;
> +}
> +
> +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"
> +		"       %s btf help\n"
> +		"\n"
> +		"       BTF_SRC := { id BTF_ID | prog PROG | map MAP [{key | value | kv | all}] | elf FILE }\n"

"elf FILE" is not consistent with implementation. The implementation 
uses "file FILE". I prefer to use "file FILE".

> +		"       " HELP_SPEC_MAP "\n"
> +		"       " HELP_SPEC_PROGRAM "\n"
> +		"       " HELP_SPEC_OPTIONS "\n"
> +		"",
> +		bin_name, bin_name);
> +
> +	return 0;
> +}
> +
> +static const struct cmd cmds[] = {
> +	{ "help",	do_help },
> +	{ "dump",	do_dump },
> +	{ 0 }
> +};
> +
> +int do_btf(int argc, char **argv)
> +{
> +	return cmd_select(cmds, argc, argv, do_help);
> +}
> diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
> index a9d5e9e6a732..eba56edd7c77 100644
> --- a/tools/bpf/bpftool/main.c
> +++ b/tools/bpf/bpftool/main.c
> @@ -188,6 +188,7 @@ static const struct cmd cmds[] = {
>   	{ "perf",	do_perf },
>   	{ "net",	do_net },
>   	{ "feature",	do_feature },
> +	{ "btf",	do_btf },
>   	{ "version",	do_version },
>   	{ 0 }
>   };
> diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> index 1ccc46169a19..3d63feb7f852 100644
> --- a/tools/bpf/bpftool/main.h
> +++ b/tools/bpf/bpftool/main.h
> @@ -150,6 +150,7 @@ int do_perf(int argc, char **arg);
>   int do_net(int argc, char **arg);
>   int do_tracelog(int argc, char **arg);
>   int do_feature(int argc, char **argv);
> +int do_btf(int argc, char **argv);
>   
>   int parse_u32_arg(int *argc, char ***argv, __u32 *val, const char *what);
>   int prog_parse_fd(int *argc, char ***argv);
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread