On 15/11/2023 14:51, Quentin Monnet wrote:
>
>
> On 15 November 2023 03:45:41 GMT-05:00, Alan Maguire <alan.maguire@xxxxxxxxxx> wrote:
>> On 14/11/2023 05:10, Quentin Monnet wrote:
>>> 2023-11-12 12:49 UTC+0000 ~ Alan Maguire <alan.maguire@xxxxxxxxxx>
>>>> 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@xxxxxxxxxx>
>>>> ---
>>>> 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!
>
> Right, sorry, I realised just after sending my message.
> In that case we could just use a 'null' value in JSON with jsonw_null()? The object '{"name": null}' is valid. The question mark is another possibility but requires comparing strings when parsing the JSON.
>
Great idea! I'll use jsonw_null() for the next version.
I think given the fact we're not sure about the way forward on CRCs and
standalone BTF I'll probably just separate out the kind layout stuff +
bpftool dump meta parts since I _think_ at this point those pieces are
relatively uncontroversial.
Thanks again for the reviews Quentin!
Alan
> Thanks,
> Quentin
