[bpf-next,v2] bpftool: btf: Support dumping a single type from file

Commit Message

Daniel Xu Dec. 7, 2024, 11:24 p.m. UTC

Some projects, for example xdp-tools [0], prefer to check in a minimized
vmlinux.h rather than the complete file which can get rather large.

However, when you try to add a minimized version of a complex struct (eg
struct xfrm_state), things can get quite complex if you're trying to
manually untangle and deduplicate the dependencies.

This commit teaches bpftool to do a minimized dump of a single type by
providing an optional root_id argument.

Example usage:

    $ ./bpftool btf dump file ~/dev/linux/vmlinux | rg "STRUCT 'xfrm_state'"
    [12643] STRUCT 'xfrm_state' size=912 vlen=58

    $ ./bpftool btf dump file ~/dev/linux/vmlinux root_id 12643 format c
    #ifndef __VMLINUX_H__
    #define __VMLINUX_H__

    [..]

    struct xfrm_type_offload;

    struct xfrm_sec_ctx;

    struct xfrm_state {
            possible_net_t xs_net;
            union {
                    struct hlist_node gclist;
                    struct hlist_node bydst;
            };
            union {
                    struct hlist_node dev_gclist;
                    struct hlist_node bysrc;
            };
            struct hlist_node byspi;
    [..]

[0]: https://github.com/xdp-project/xdp-tools/blob/master/headers/bpf/vmlinux.h

Signed-off-by: Daniel Xu <[e-mail]>
---
Changes in v2:
* Add early error check for invalid BTF ID

 .../bpf/bpftool/Documentation/bpftool-btf.rst |  5 +++--
 tools/bpf/bpftool/btf.c                       | 19 +++++++++++++++++++
 2 files changed, 22 insertions(+), 2 deletions(-)

Comments

Quentin Monnet Dec. 9, 2024, 12:26 p.m. UTC | #1

On 07/12/2024 23:24, Daniel Xu wrote:
> Some projects, for example xdp-tools [0], prefer to check in a minimized
> vmlinux.h rather than the complete file which can get rather large.
> 
> However, when you try to add a minimized version of a complex struct (eg
> struct xfrm_state), things can get quite complex if you're trying to
> manually untangle and deduplicate the dependencies.
> 
> This commit teaches bpftool to do a minimized dump of a single type by
> providing an optional root_id argument.
> 
> Example usage:
> 
>     $ ./bpftool btf dump file ~/dev/linux/vmlinux | rg "STRUCT 'xfrm_state'"
>     [12643] STRUCT 'xfrm_state' size=912 vlen=58
> 
>     $ ./bpftool btf dump file ~/dev/linux/vmlinux root_id 12643 format c
>     #ifndef __VMLINUX_H__
>     #define __VMLINUX_H__
> 
>     [..]
> 
>     struct xfrm_type_offload;
> 
>     struct xfrm_sec_ctx;
> 
>     struct xfrm_state {
>             possible_net_t xs_net;
>             union {
>                     struct hlist_node gclist;
>                     struct hlist_node bydst;
>             };
>             union {
>                     struct hlist_node dev_gclist;
>                     struct hlist_node bysrc;
>             };
>             struct hlist_node byspi;
>     [..]
> 
> [0]: https://github.com/xdp-project/xdp-tools/blob/master/headers/bpf/vmlinux.h
> 
> Signed-off-by: Daniel Xu <[e-mail]>
> ---
> Changes in v2:
> * Add early error check for invalid BTF ID
> 
>  .../bpf/bpftool/Documentation/bpftool-btf.rst |  5 +++--
>  tools/bpf/bpftool/btf.c                       | 19 +++++++++++++++++++
>  2 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-btf.rst b/tools/bpf/bpftool/Documentation/bpftool-btf.rst
> index 3f6bca03ad2e..5abd0e99022f 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-btf.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-btf.rst
> @@ -27,7 +27,7 @@ BTF COMMANDS
>  | **bpftool** **btf dump** *BTF_SRC* [**format** *FORMAT*]
>  | **bpftool** **btf help**
>  |
> -| *BTF_SRC* := { **id** *BTF_ID* | **prog** *PROG* | **map** *MAP* [{**key** | **value** | **kv** | **all**}] | **file** *FILE* }
> +| *BTF_SRC* := { **id** *BTF_ID* | **prog** *PROG* | **map** *MAP* [{**key** | **value** | **kv** | **all**}] | **file** *FILE* [**root_id** *ROOT_ID*] }


Thanks for this!

root_id is not part of the BTF_SRC, I think it should be an option on
the command line itself (3 lines above), after "format". And the change
should also be repeated below in the description (the "format" option is
missing, by the way, let's fix it too).

Can you please also update the interactive help message at the end of
btf.c?

Can you please also update the bash completion file? I think it should
look like this:


------
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 0c541498c301..097d406ee21f 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" ) )
                             ;;
+                        root_id)
+                            return 0;
+                            ;;
                         c)
                             COMPREPLY=( $( compgen -W "unsorted" -- "$cur" ) )
                             ;;
@@ -937,13 +940,13 @@ _bpftool()
                             # emit extra options
                             case ${words[3]} in
                                 id|file)
-                                    _bpftool_once_attr 'format'
+                                    _bpftool_once_attr 'format root_id'
                                     ;;
                                 map|prog)
                                     if [[ ${words[3]} == "map" ]] && [[ $cword == 6 ]]; then
                                         COMPREPLY+=( $( compgen -W "key value kv all" -- "$cur" ) )
                                     fi
-                                    _bpftool_once_attr 'format'
+                                    _bpftool_once_attr 'format root_id'
                                     ;;
                                 *)
                                     ;;
------


>  | *FORMAT* := { **raw** | **c** [**unsorted**] }
>  | *MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
>  | *PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* | **name** *PROG_NAME* }
> @@ -60,7 +60,8 @@ bpftool btf dump *BTF_SRC*
>  
>      When specifying *FILE*, an ELF file is expected, containing .BTF section
>      with well-defined BTF binary format data, typically produced by clang or
> -    pahole.
> +    pahole. You can choose to dump a single type and all its dependent types
> +    by providing an optional *ROOT_ID*.
>  
>      **format** option can be used to override default (raw) output format. Raw
>      (**raw**) or C-syntax (**c**) output formats are supported. With C-style
> diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
> index d005e4fd6128..a75e17efaf5e 100644
> --- a/tools/bpf/bpftool/btf.c
> +++ b/tools/bpf/bpftool/btf.c
> @@ -953,6 +953,8 @@ static int do_dump(int argc, char **argv)
>  		NEXT_ARG();
>  	} else if (is_prefix(src, "file")) {
>  		const char sysfs_prefix[] = "/sys/kernel/btf/";
> +		__u32 root_id;
> +		char *end;


I think we could move these declarations to a lower scope, under your
"if (argc && is_prefix(...))".


>  
>  		if (!base_btf &&
>  		    strncmp(*argv, sysfs_prefix, sizeof(sysfs_prefix) - 1) == 0 &&
> @@ -967,6 +969,23 @@ static int do_dump(int argc, char **argv)
>  			goto done;
>  		}
>  		NEXT_ARG();
> +
> +		if (argc && is_prefix(*argv, "root_id")) {
> +			NEXT_ARG();
> +			root_id = strtoul(*argv, &end, 0);
> +			if (*end) {
> +				err = -1;
> +				p_err("can't parse %s as root ID", *argv);
> +				goto done;
> +			}
> +			if (root_id >= btf__type_cnt(btf)) {
> +				err = -EINVAL;
> +				p_err("invalid root ID: %u", root_id);
> +				goto done;
> +			}
> +			root_type_ids[root_type_cnt++] = root_id;
> +			NEXT_ARG();
> +		}
>  	} else {
>  		err = -1;
>  		p_err("unrecognized BTF source specifier: '%s'", src);


Same comment as above, it seems to be that the root_id controls the
output for the command rather than the source, and I'd rather move this
to the "while (argc)" loop where we process the "format" option rather
than when parsing the source.


pw-bot: cr

Daniel Xu Dec. 9, 2024, 10:56 p.m. UTC | #2

Hi Quentin,

On Mon, Dec 09, 2024 at 12:26:50PM GMT, Quentin Monnet wrote:
> On 07/12/2024 23:24, Daniel Xu wrote:
> > Some projects, for example xdp-tools [0], prefer to check in a minimized
> > vmlinux.h rather than the complete file which can get rather large.
> > 
> > However, when you try to add a minimized version of a complex struct (eg
> > struct xfrm_state), things can get quite complex if you're trying to
> > manually untangle and deduplicate the dependencies.
> > 
> > This commit teaches bpftool to do a minimized dump of a single type by
> > providing an optional root_id argument.
> > 
> > Example usage:
> > 
> >     $ ./bpftool btf dump file ~/dev/linux/vmlinux | rg "STRUCT 'xfrm_state'"
> >     [12643] STRUCT 'xfrm_state' size=912 vlen=58
> > 
> >     $ ./bpftool btf dump file ~/dev/linux/vmlinux root_id 12643 format c
> >     #ifndef __VMLINUX_H__
> >     #define __VMLINUX_H__
> > 
> >     [..]
> > 
> >     struct xfrm_type_offload;
> > 
> >     struct xfrm_sec_ctx;
> > 
> >     struct xfrm_state {
> >             possible_net_t xs_net;
> >             union {
> >                     struct hlist_node gclist;
> >                     struct hlist_node bydst;
> >             };
> >             union {
> >                     struct hlist_node dev_gclist;
> >                     struct hlist_node bysrc;
> >             };
> >             struct hlist_node byspi;
> >     [..]
> > 
> > [0]: https://github.com/xdp-project/xdp-tools/blob/master/headers/bpf/vmlinux.h
> > 
> > Signed-off-by: Daniel Xu <[e-mail]>
> > ---
> > Changes in v2:
> > * Add early error check for invalid BTF ID
> > 
> >  .../bpf/bpftool/Documentation/bpftool-btf.rst |  5 +++--
> >  tools/bpf/bpftool/btf.c                       | 19 +++++++++++++++++++
> >  2 files changed, 22 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/bpf/bpftool/Documentation/bpftool-btf.rst b/tools/bpf/bpftool/Documentation/bpftool-btf.rst
> > index 3f6bca03ad2e..5abd0e99022f 100644
> > --- a/tools/bpf/bpftool/Documentation/bpftool-btf.rst
> > +++ b/tools/bpf/bpftool/Documentation/bpftool-btf.rst
> > @@ -27,7 +27,7 @@ BTF COMMANDS
> >  | **bpftool** **btf dump** *BTF_SRC* [**format** *FORMAT*]
> >  | **bpftool** **btf help**
> >  |
> > -| *BTF_SRC* := { **id** *BTF_ID* | **prog** *PROG* | **map** *MAP* [{**key** | **value** | **kv** | **all**}] | **file** *FILE* }
> > +| *BTF_SRC* := { **id** *BTF_ID* | **prog** *PROG* | **map** *MAP* [{**key** | **value** | **kv** | **all**}] | **file** *FILE* [**root_id** *ROOT_ID*] }
> 
> 
> Thanks for this!

No problem!

> 
> root_id is not part of the BTF_SRC, I think it should be an option on
> the command line itself (3 lines above), after "format". And the change
> should also be repeated below in the description (the "format" option is
> missing, by the way, let's fix it too).

Sure, will do. I was originally thinking it would conflict with `map`
subcommand, as it also sets the root_type_ids, but we can just error out
if root_type_cnt > 0 and root_id is pased.

Will send v3 with below suggestions as well.

> 
> Can you please also update the interactive help message at the end of
> btf.c?
> 
> Can you please also update the bash completion file? I think it should
> look like this:
> 
> 
> ------
> diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
> index 0c541498c301..097d406ee21f 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" ) )
>                              ;;
> +                        root_id)
> +                            return 0;
> +                            ;;
>                          c)
>                              COMPREPLY=( $( compgen -W "unsorted" -- "$cur" ) )
>                              ;;
> @@ -937,13 +940,13 @@ _bpftool()
>                              # emit extra options
>                              case ${words[3]} in
>                                  id|file)
> -                                    _bpftool_once_attr 'format'
> +                                    _bpftool_once_attr 'format root_id'
>                                      ;;
>                                  map|prog)
>                                      if [[ ${words[3]} == "map" ]] && [[ $cword == 6 ]]; then
>                                          COMPREPLY+=( $( compgen -W "key value kv all" -- "$cur" ) )
>                                      fi
> -                                    _bpftool_once_attr 'format'
> +                                    _bpftool_once_attr 'format root_id'
>                                      ;;
>                                  *)
>                                      ;;
> ------
> 
> 
> >  | *FORMAT* := { **raw** | **c** [**unsorted**] }
> >  | *MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
> >  | *PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* | **name** *PROG_NAME* }
> > @@ -60,7 +60,8 @@ bpftool btf dump *BTF_SRC*
> >  
> >      When specifying *FILE*, an ELF file is expected, containing .BTF section
> >      with well-defined BTF binary format data, typically produced by clang or
> > -    pahole.
> > +    pahole. You can choose to dump a single type and all its dependent types
> > +    by providing an optional *ROOT_ID*.
> >  
> >      **format** option can be used to override default (raw) output format. Raw
> >      (**raw**) or C-syntax (**c**) output formats are supported. With C-style
> > diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
> > index d005e4fd6128..a75e17efaf5e 100644
> > --- a/tools/bpf/bpftool/btf.c
> > +++ b/tools/bpf/bpftool/btf.c
> > @@ -953,6 +953,8 @@ static int do_dump(int argc, char **argv)
> >  		NEXT_ARG();
> >  	} else if (is_prefix(src, "file")) {
> >  		const char sysfs_prefix[] = "/sys/kernel/btf/";
> > +		__u32 root_id;
> > +		char *end;
> 
> 
> I think we could move these declarations to a lower scope, under your
> "if (argc && is_prefix(...))".
> 
> 
> >  
> >  		if (!base_btf &&
> >  		    strncmp(*argv, sysfs_prefix, sizeof(sysfs_prefix) - 1) == 0 &&
> > @@ -967,6 +969,23 @@ static int do_dump(int argc, char **argv)
> >  			goto done;
> >  		}
> >  		NEXT_ARG();
> > +
> > +		if (argc && is_prefix(*argv, "root_id")) {
> > +			NEXT_ARG();
> > +			root_id = strtoul(*argv, &end, 0);
> > +			if (*end) {
> > +				err = -1;
> > +				p_err("can't parse %s as root ID", *argv);
> > +				goto done;
> > +			}
> > +			if (root_id >= btf__type_cnt(btf)) {
> > +				err = -EINVAL;
> > +				p_err("invalid root ID: %u", root_id);
> > +				goto done;
> > +			}
> > +			root_type_ids[root_type_cnt++] = root_id;
> > +			NEXT_ARG();
> > +		}
> >  	} else {
> >  		err = -1;
> >  		p_err("unrecognized BTF source specifier: '%s'", src);
> 
> 
> Same comment as above, it seems to be that the root_id controls the
> output for the command rather than the source, and I'd rather move this
> to the "while (argc)" loop where we process the "format" option rather
> than when parsing the source.
> 
> 
> pw-bot: cr

Patch

diff --git a/tools/bpf/bpftool/Documentation/bpftool-btf.rst b/tools/bpf/bpftool/Documentation/bpftool-btf.rst
index 3f6bca03ad2e..5abd0e99022f 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-btf.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-btf.rst
@@ -27,7 +27,7 @@  BTF COMMANDS
 | **bpftool** **btf dump** *BTF_SRC* [**format** *FORMAT*]
 | **bpftool** **btf help**
 |
-| *BTF_SRC* := { **id** *BTF_ID* | **prog** *PROG* | **map** *MAP* [{**key** | **value** | **kv** | **all**}] | **file** *FILE* }
+| *BTF_SRC* := { **id** *BTF_ID* | **prog** *PROG* | **map** *MAP* [{**key** | **value** | **kv** | **all**}] | **file** *FILE* [**root_id** *ROOT_ID*] }
 | *FORMAT* := { **raw** | **c** [**unsorted**] }
 | *MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
 | *PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* | **name** *PROG_NAME* }
@@ -60,7 +60,8 @@  bpftool btf dump *BTF_SRC*
 
     When specifying *FILE*, an ELF file is expected, containing .BTF section
     with well-defined BTF binary format data, typically produced by clang or
-    pahole.
+    pahole. You can choose to dump a single type and all its dependent types
+    by providing an optional *ROOT_ID*.
 
     **format** option can be used to override default (raw) output format. Raw
     (**raw**) or C-syntax (**c**) output formats are supported. With C-style
diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
index d005e4fd6128..a75e17efaf5e 100644
--- a/tools/bpf/bpftool/btf.c
+++ b/tools/bpf/bpftool/btf.c
@@ -953,6 +953,8 @@  static int do_dump(int argc, char **argv)
 		NEXT_ARG();
 	} else if (is_prefix(src, "file")) {
 		const char sysfs_prefix[] = "/sys/kernel/btf/";
+		__u32 root_id;
+		char *end;
 
 		if (!base_btf &&
 		    strncmp(*argv, sysfs_prefix, sizeof(sysfs_prefix) - 1) == 0 &&
@@ -967,6 +969,23 @@  static int do_dump(int argc, char **argv)
 			goto done;
 		}
 		NEXT_ARG();
+
+		if (argc && is_prefix(*argv, "root_id")) {
+			NEXT_ARG();
+			root_id = strtoul(*argv, &end, 0);
+			if (*end) {
+				err = -1;
+				p_err("can't parse %s as root ID", *argv);
+				goto done;
+			}
+			if (root_id >= btf__type_cnt(btf)) {
+				err = -EINVAL;
+				p_err("invalid root ID: %u", root_id);
+				goto done;
+			}
+			root_type_ids[root_type_cnt++] = root_id;
+			NEXT_ARG();
+		}
 	} else {
 		err = -1;
 		p_err("unrecognized BTF source specifier: '%s'", src);