BPF Archive mirror
 help / color / mirror / Atom feed
From: Quentin Monnet <qmo@kernel.org>
To: Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>,
	bpf@vger.kernel.org, ast@kernel.org, andrii@kernel.org,
	daniel@iogearbox.net, kafai@meta.com, kernel-team@meta.com
Cc: Mykyta Yatsenko <yatsenko@meta.com>
Subject: Re: [PATCH v3 bpf-next] bpftool: introduce btf c dump sorting
Date: Tue, 14 May 2024 11:27:25 +0100	[thread overview]
Message-ID: <91c91557-6fc7-4228-b003-32e09d52c1e4@kernel.org> (raw)
In-Reply-To: <20240513192927.99189-1-yatsenko@meta.com>

2024-05-13 20:29 UTC+0100 ~ Mykyta Yatsenko mykyta.yatsenko5@gmail.com
> From: Mykyta Yatsenko <yatsenko@meta.com>
> 
> Sort bpftool c dump output; aiming to simplify vmlinux.h diffing and
> forcing more natural type definitions ordering.
> 
> Definitions are sorted first by their BTF kind ranks, then by their base
> type name and by their own name.
> 
> Type ranks
> 
> Assign ranks to btf kinds (defined in function btf_type_rank) to set
> next order:
> 1. Anonymous enums/enums64
> 2. Named enums/enums64
> 3. Trivial types typedefs (ints, then floats)
> 4. Structs/Unions
> 5. Function prototypes
> 6. Forward declarations
> 
> Type rank is set to maximum for unnamed reference types, structs and
> unions to avoid emitting those types early. They will be emitted as
> part of the type chain starting with named type.
> 
> Lexicographical ordering
> 
> Each type is assigned a sort_name and own_name.
> sort_name is the resolved name of the final base type for reference
> types (typedef, pointer, array etc). Sorting by sort_name allows to
> group typedefs of the same base type. sort_name for non-reference type
> is the same as own_name. own_name is a direct name of particular type,
> is used as final sorting step.
> 
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> ---
>  .../bpf/bpftool/Documentation/bpftool-btf.rst |   5 +-
>  tools/bpf/bpftool/bash-completion/bpftool     |   3 +
>  tools/bpf/bpftool/btf.c                       | 138 +++++++++++++++++-
>  3 files changed, 139 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-btf.rst b/tools/bpf/bpftool/Documentation/bpftool-btf.rst
> index eaba24320fb2..65eeb3d905f0 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-btf.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-btf.rst
> @@ -28,7 +28,7 @@ BTF COMMANDS
>  | **bpftool** **btf help**
>  |
>  | *BTF_SRC* := { **id** *BTF_ID* | **prog** *PROG* | **map** *MAP* [{**key** | **value** | **kv** | **all**}] | **file** *FILE* }
> -| *FORMAT* := { **raw** | **c** }
> +| *FORMAT* := { **raw** | **c** [**unsorted**]}

Missing space at the end, "[**unsorted**] }"

>  | *MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
>  | *PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* | **name** *PROG_NAME* }
>  
> @@ -63,7 +63,8 @@ bpftool btf dump *BTF_SRC*
>      pahole.
>  
>      **format** option can be used to override default (raw) output format. Raw
> -    (**raw**) or C-syntax (**c**) output formats are supported.
> +    (**raw**) or C-syntax (**c**) output formats are supported. (**unsorted**)
> +    option can be used with (**c**) to avoid sorting the output.

The parenthesis are not part of the formatting here, they are used in
the previous sentence to mention the keyword related to the options
mentioned in the sentence. So here we could have:

"With C-style formatting, the output is sorted by default. Use the
**unsorted** option to avoid sorting the output."

>  
>  bpftool btf help
>      Print short help message.
> diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
> index 04afe2ac2228..be99d49b8714 100644
> --- a/tools/bpf/bpftool/bash-completion/bpftool
> +++ b/tools/bpf/bpftool/bash-completion/bpftool
> @@ -930,6 +930,9 @@ _bpftool()
>                          format)
>                              COMPREPLY=( $( compgen -W "c raw" -- "$cur" ) )
>                              ;;
> +                        c)
> +                            COMPREPLY=( $( compgen -W "unsorted" -- "$cur" ) )
> +                            ;;
>                          *)
>                              # emit extra options
>                              case ${words[3]} in
> diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
> index 91fcb75babe3..7e7071d301df 100644
> --- a/tools/bpf/bpftool/btf.c
> +++ b/tools/bpf/bpftool/btf.c

[...]

> @@ -1063,7 +1191,7 @@ static int do_help(int argc, char **argv)
>  		"       %1$s %2$s help\n"
>  		"\n"
>  		"       BTF_SRC := { id BTF_ID | prog PROG | map MAP [{key | value | kv | all}] | file FILE }\n"
> -		"       FORMAT  := { raw | c }\n"
> +		"       FORMAT  := { raw | c [unsorted]}\n"

Missing space as well: "[unsorted] }\n"

>  		"       " HELP_SPEC_MAP "\n"
>  		"       " HELP_SPEC_PROGRAM "\n"
>  		"       " HELP_SPEC_OPTIONS " |\n"

With the above addressed:

Reviewed-by: Quentin Monnet <qmo@kernel.org>

      parent reply	other threads:[~2024-05-14 10:27 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-13 19:29 [PATCH v3 bpf-next] bpftool: introduce btf c dump sorting Mykyta
2024-05-13 23:08 ` Andrii Nakryiko
2024-05-13 23:34   ` Alexei Starovoitov
2024-05-14  3:51     ` Andrii Nakryiko
2024-05-14 10:27 ` Quentin Monnet [this message]

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=91c91557-6fc7-4228-b003-32e09d52c1e4@kernel.org \
    --to=qmo@kernel.org \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kafai@meta.com \
    --cc=kernel-team@meta.com \
    --cc=mykyta.yatsenko5@gmail.com \
    --cc=yatsenko@meta.com \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).