From: Alan Maguire <alan.maguire@oracle.com>
To: Quentin Monnet <quentin@isovalent.com>,
ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
jolsa@kernel.org
Cc: eddyz87@gmail.com, martin.lau@linux.dev, song@kernel.org,
yonghong.song@linux.dev, john.fastabend@gmail.com,
kpsingh@kernel.org, sdf@google.com, haoluo@google.com,
masahiroy@kernel.org, bpf@vger.kernel.org
Subject: Re: [PATCH v4 bpf-next 10/17] bpftool: add BTF dump "format meta" to dump header/metadata
Date: Wed, 15 Nov 2023 08:45:41 +0000 [thread overview]
Message-ID: <b6368b81-e141-13c0-7fde-c4cada3e242c@oracle.com> (raw)
In-Reply-To: <ebb1d463-68f5-4668-b930-f5dfe1f52230@isovalent.com>
On 14/11/2023 05:10, Quentin Monnet wrote:
> 2023-11-12 12:49 UTC+0000 ~ Alan Maguire <alan.maguire@oracle.com>
>> Provide a way to dump BTF metadata info via bpftool; this
>> consists of BTF size, header fields and kind layout info
>> (if available); for example
>>
>> $ bpftool btf dump file vmlinux format meta
>> size 5161076
>> magic 0xeb9f
>> version 1
>> flags 0x1
>> hdr_len 40
>> type_len 3036368
>> type_off 0
>> str_len 2124588
>> str_off 3036368
>> kind_layout_len 80
>> kind_layout_off 5160956
>> crc 0x64af901b
>> base_crc 0x0
>> kind 0 UNKNOWN flags 0x0 info_sz 0 elem_sz 0
>> kind 1 INT flags 0x0 info_sz 0 elem_sz 0
>> kind 2 PTR flags 0x0 info_sz 0 elem_sz 0
>> kind 3 ARRAY flags 0x0 info_sz 0 elem_sz 0
>> kind 4 STRUCT flags 0x35 info_sz 0 elem_sz 0
>> ...
>>
>> JSON output is also supported:
>>
>> $ bpftool -j btf dump file vmlinux format meta
>> {"size":5161076,"header":{"magic":60319,"version":1,"flags":1,"hdr_len":40,"type_len":3036368,"type_off":0,"str_len":2124588,"str_off":3036368,"kind_layout_len":80,"kind_layout_offset":5160956,"crc":1689227291,"base_crc":0},"kind_layouts":[{"kind":0,"name":"UNKNOWN","flags":0,"info_sz":0,"elem_sz":0},{"kind":1,"name":"INT","flags":0,"info_sz":0,"elem_sz":0},{"kind":2,"name":"PTR","flags":0,"info_sz":0,"elem_sz":0},{"kind":3,"name":"ARRAY","flags":0,"info_sz":0,"elem_sz":0},{"kind":4,"name":"STRUCT","flags":53,"info_sz":0,"elem_sz":0},{"kind":5,"name":"UNION","flags":0,"info_sz":0,"elem_sz":0},{"kind":6,"name":"ENUM","flags":60319,"info_sz":1,"elem_sz":1},{"kind":7,"name":"FWD","flags":40,"info_sz":0,"elem_sz":0},{"kind":8,"name":"TYPEDEF","flags":0,"info_sz":0,"elem_sz":0},{"kind":9,"name":"VOLATILE","flags":0,"info_sz":0,"elem_sz":0},{"kind":10,"name":"CONST","flags":0,"info_sz":0,"elem_sz":0},{"kind":11,"name":"RESTRICT","flags":1,"info_sz":0,"elem_sz":0},{"kind":12,"name":"FUNC","flags":0,"info_sz":0,"elem_sz":0},{"kind":13,"name":"FUNC_PROTO","flags":80,"info_sz":0,"elem_sz":0},{"kind":14,"name":"VAR","flags":0,"info_sz":0,"elem_sz":0},{"kind":15,"name":"DATASEC","flags":0,"info_sz":0,"elem_sz":0},{"kind":16,"name":"FLOAT","flags":53,"info_sz":0,"elem_sz":0},{"kind":17,"name":"DECL_TAG","flags":0,"info_sz":0,"elem_sz":0},{"kind":18,"name":"TYPE_TAG","flags":11441,"info_sz":3,"elem_sz":0},{"kind":19,"name":"ENUM64","flags":0,"info_sz":0,"elem_sz":0}]}
>>
>> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>> ---
>> tools/bpf/bpftool/bash-completion/bpftool | 2 +-
>> tools/bpf/bpftool/btf.c | 91 ++++++++++++++++++++++-
>> 2 files changed, 90 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
>> index 6e4f7ce6bc01..157c3afd8247 100644
>> --- a/tools/bpf/bpftool/bash-completion/bpftool
>> +++ b/tools/bpf/bpftool/bash-completion/bpftool
>> @@ -937,7 +937,7 @@ _bpftool()
>> return 0
>> ;;
>> format)
>> - COMPREPLY=( $( compgen -W "c raw" -- "$cur" ) )
>> + COMPREPLY=( $( compgen -W "c raw meta" -- "$cur" ) )
>> ;;
>> *)
>> # emit extra options
>> diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
>> index 91fcb75babe3..208f3a587534 100644
>> --- a/tools/bpf/bpftool/btf.c
>> +++ b/tools/bpf/bpftool/btf.c
>> @@ -504,6 +504,88 @@ static int dump_btf_c(const struct btf *btf,
>> return err;
>> }
>>
>> +static int dump_btf_meta(const struct btf *btf)
>> +{
>> + const struct btf_header *hdr;
>> + const struct btf_kind_layout *k;
>> + __u8 i, nr_kinds = 0;
>> + const void *data;
>> + __u32 data_sz;
>> +
>> + data = btf__raw_data(btf, &data_sz);
>> + if (!data)
>> + return -ENOMEM;
>> + hdr = data;
>> + if (json_output) {
>> + jsonw_start_object(json_wtr); /* btf metadata object */
>
> Nit: Please make sure to be consistent when aligning these comments:
> there are several occurrences with spaces (here three spaces), several
> ones with tabs. For these, I'd prefer tabs to align the start and end
> comments for a given object/array, although I don't really using a
> single space as well as long as we keep it consistent.
>
>> + jsonw_uint_field(json_wtr, "size", data_sz);
>> + jsonw_name(json_wtr, "header");
>> + jsonw_start_object(json_wtr); /* btf header object */
>> + jsonw_uint_field(json_wtr, "magic", hdr->magic);
>> + jsonw_uint_field(json_wtr, "version", hdr->version);
>> + jsonw_uint_field(json_wtr, "flags", hdr->flags);
>> + jsonw_uint_field(json_wtr, "hdr_len", hdr->hdr_len);
>> + jsonw_uint_field(json_wtr, "type_len", hdr->type_len);
>> + jsonw_uint_field(json_wtr, "type_off", hdr->type_off);
>> + jsonw_uint_field(json_wtr, "str_len", hdr->str_len);
>> + jsonw_uint_field(json_wtr, "str_off", hdr->str_off);
>> + } else {
>> + printf("size %-10d\n", data_sz);
>> + printf("magic 0x%-10x\nversion %-10d\nflags 0x%-10x\nhdr_len %-10d\n",
>> + hdr->magic, hdr->version, hdr->flags, hdr->hdr_len);
>> + printf("type_len %-10d\ntype_off %-10d\n", hdr->type_len, hdr->type_off);
>> + printf("str_len %-10d\nstr_off %-10d\n", hdr->str_len, hdr->str_off);
>> + }
>> +
>> + if (hdr->hdr_len < sizeof(struct btf_header)) {
>> + if (json_output) {
>> + jsonw_end_object(json_wtr); /* header object */
>> + jsonw_end_object(json_wtr); /* metadata object */
>
> Similarly, can you please keep consistent comment strings? "metadata
> object" here vs. "end metadata" below.
>
Sure, I'll fix indent consistency/naming and the docs issue in the
next revision. Thanks!
>> + }
>> + return 0;
>> + }
>> + if (hdr->kind_layout_len > 0 && hdr->kind_layout_off > 0) {
>> + k = (void *)hdr + hdr->hdr_len + hdr->kind_layout_off;
>> + nr_kinds = hdr->kind_layout_len / sizeof(*k);
>> + }
>> + if (json_output) {
>> + jsonw_uint_field(json_wtr, "kind_layout_len", hdr->kind_layout_len);
>> + jsonw_uint_field(json_wtr, "kind_layout_offset", hdr->kind_layout_off);
>> + jsonw_uint_field(json_wtr, "crc", hdr->crc);
>> + jsonw_uint_field(json_wtr, "base_crc", hdr->base_crc);
>> + jsonw_end_object(json_wtr); /* end header object */
>> +
>> + if (nr_kinds > 0) {
>> + jsonw_name(json_wtr, "kind_layouts");
>> + jsonw_start_array(json_wtr);
>> + for (i = 0; i < nr_kinds; i++) {
>> + jsonw_start_object(json_wtr);
>> + jsonw_uint_field(json_wtr, "kind", i);
>> + if (i < NR_BTF_KINDS)
>> + jsonw_string_field(json_wtr, "name", btf_kind_str[i]);
>
> I prefer to avoid conditional fields in JSON, especially in an array
> it's easier to process the JSON if all items have the same structure.
> Would it make sense to keep the "name" field, but use jsonw_null() (or
> "UNKNOWN") for the value when there's no name to print?
>
The only thing about UNKNOWN is that there is a BTF_KIND_UNKN that
is displayed as UNKNOWN; what about "?" to be consistent with the
non-json output (or if there's another option of course, we could
use that for both)? Thanks!
Alan
next prev parent reply other threads:[~2023-11-15 8:46 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-12 12:48 [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 01/17] btf: add kind layout encoding, crcs to UAPI Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 02/17] libbpf: support kind layout section handling in BTF Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 03/17] libbpf: use kind layout to compute an unknown kind size Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 04/17] libbpf: add kind layout encoding, crc support Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 05/17] libbpf: BTF validation can use kind layout for unknown kinds Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 06/17] btf: support kernel parsing of BTF with kind layout Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 07/17] bpf: add BTF CRC verification where present Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 08/17] bpf: verify base BTF CRC to ensure it matches module BTF Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 09/17] kbuild, bpf: switch to --btf_features, add crc,kind_layout features Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 10/17] bpftool: add BTF dump "format meta" to dump header/metadata Alan Maguire
2023-11-14 5:10 ` Quentin Monnet
2023-11-15 8:45 ` Alan Maguire [this message]
2023-11-15 14:51 ` Quentin Monnet
2023-11-16 9:16 ` Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 11/17] bpftool: update doc to describe bpftool btf dump .. format meta Alan Maguire
2023-11-14 5:12 ` Quentin Monnet
2023-11-12 12:48 ` [PATCH v4 bpf-next 12/17] selftests/bpf: test kind encoding/decoding Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 13/17] bpf: support standalone BTF in modules Alan Maguire
2023-11-12 15:35 ` kernel test robot
2023-11-12 20:00 ` kernel test robot
2023-11-12 12:48 ` [PATCH v4 bpf-next 14/17] kbuild, bpf: allow opt-out from using split BTF for modules Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 15/17] selftests/bpf: generalize module load to support specifying a module name Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 16/17] selftests/bpf: build separate bpf_testmod module with standalone BTF Alan Maguire
2023-11-12 12:48 ` [PATCH v4 bpf-next 17/17] selftests/bpf: update btf_module test to ensure standalone BTF works Alan Maguire
2023-11-14 20:16 ` [PATCH v4 bpf-next 00/17] Add kind layout, CRCs to BTF Alan Maguire
2023-11-21 19:44 ` Andrii Nakryiko
2023-11-22 17:00 ` Alan Maguire
2023-11-22 17:42 ` Andrii Nakryiko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b6368b81-e141-13c0-7fde-c4cada3e242c@oracle.com \
--to=alan.maguire@oracle.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kpsingh@kernel.org \
--cc=martin.lau@linux.dev \
--cc=masahiroy@kernel.org \
--cc=quentin@isovalent.com \
--cc=sdf@google.com \
--cc=song@kernel.org \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox