lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <zf62rqtgvl63sawxltmpgcnpek5bt3w5pleznby3zqb7ezhvdz@wqlwxy2f43wt>
Date: Thu, 12 Dec 2024 16:41:43 -0700
From: Daniel Xu <dxu@...uu.xyz>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: hawk@...nel.org, john.fastabend@...il.com, ast@...nel.org, 
	qmo@...nel.org, davem@...emloft.net, daniel@...earbox.net, andrii@...nel.org, 
	kuba@...nel.org, martin.lau@...ux.dev, eddyz87@...il.com, song@...nel.org, 
	yonghong.song@...ux.dev, kpsingh@...nel.org, sdf@...ichev.me, haoluo@...gle.com, 
	jolsa@...nel.org, bpf@...r.kernel.org, linux-kernel@...r.kernel.org, 
	netdev@...r.kernel.org, antony@...nome.org, toke@...nel.org
Subject: Re: [PATCH bpf-next v3 3/4] bpftool: btf: Support dumping a single
 type from file

On Thu, Dec 12, 2024 at 11:09:34AM GMT, Andrii Nakryiko wrote:
> On Mon, Dec 9, 2024 at 3:45 PM Daniel Xu <dxu@...uu.xyz> 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 <dxu@...uu.xyz>
> > ---
> >  .../bpf/bpftool/Documentation/bpftool-btf.rst |  7 +++++--
> >  tools/bpf/bpftool/btf.c                       | 21 ++++++++++++++++++-
> >  2 files changed, 25 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/bpf/bpftool/Documentation/bpftool-btf.rst b/tools/bpf/bpftool/Documentation/bpftool-btf.rst
> > index 245569f43035..4899b2c10777 100644
> > --- a/tools/bpf/bpftool/Documentation/bpftool-btf.rst
> > +++ b/tools/bpf/bpftool/Documentation/bpftool-btf.rst
> > @@ -24,7 +24,7 @@ BTF COMMANDS
> >  =============
> >
> >  | **bpftool** **btf** { **show** | **list** } [**id** *BTF_ID*]
> > -| **bpftool** **btf dump** *BTF_SRC* [**format** *FORMAT*]
> > +| **bpftool** **btf dump** *BTF_SRC* [**format** *FORMAT*] [**root_id** *ROOT_ID*]
> >  | **bpftool** **btf help**
> >  |
> >  | *BTF_SRC* := { **id** *BTF_ID* | **prog** *PROG* | **map** *MAP* [{**key** | **value** | **kv** | **all**}] | **file** *FILE* }
> > @@ -43,7 +43,7 @@ bpftool btf { show | list } [id *BTF_ID*]
> >      that hold open file descriptors (FDs) against BTF objects. On such kernels
> >      bpftool will automatically emit this information as well.
> >
> > -bpftool btf dump *BTF_SRC* [format *FORMAT*]
> > +bpftool btf dump *BTF_SRC* [format *FORMAT*] [root_id *ROOT_ID*]
> >      Dump BTF entries from a given *BTF_SRC*.
> >
> >      When **id** is specified, BTF object with that ID will be loaded and all
> > @@ -67,6 +67,9 @@ bpftool btf dump *BTF_SRC* [format *FORMAT*]
> >      formatting, the output is sorted by default. Use the **unsorted** option
> >      to avoid sorting the output.
> >
> > +    **root_id** option can be used to filter a dump to a single type and all
> > +    its dependent types. It cannot be used with any other types of filtering.
> > +
> >  bpftool btf help
> >      Print short help message.
> >
> > diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
> > index 3e995faf9efa..18b037a1414b 100644
> > --- a/tools/bpf/bpftool/btf.c
> > +++ b/tools/bpf/bpftool/btf.c
> > @@ -993,6 +993,25 @@ static int do_dump(int argc, char **argv)
> >                                 goto done;
> >                         }
> >                         NEXT_ARG();
> > +               } else if (is_prefix(*argv, "root_id")) {
> > +                       __u32 root_id;
> > +                       char *end;
> > +
> > +                       if (root_type_cnt) {
> > +                               p_err("cannot use root_id with other type filtering");
> 
> this is a confusing error if the user just wanted to provide two
> root_id arguments... Also, why don't we allow multiple root_ids?
> 
> I'd bump root_type_ids[] to have something like 16 elements or
> something (though we can always do dynamic realloc as well, probably),
> and allow multiple types to be specified.
> 
> Thoughts?

That's a good point. I added this check b/c I didn't think it would
make sense to allow `root_id` filtering in combination with map dump
filtering (which uses same root_type_ids param):

        map MAP [{key | value | kv | all}]

But code can easily be tweaked to still block combination but allow
multiple `root_id`s when used alone. 16 seems sufficient to me.

Do you think it'd be more bpftool-y to require "root_id" each time or to
just take a comma separated value?

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ