netdev - Re: [PATCH iproute2 5/5] bpf: implement btf handling and map annotation

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] [day] [month] [year] [list]

Message-ID: <20180719163744.fxhtcdy4bg2b4bja@kafai-mbp.dhcp.thefacebook.com>
Date:   Thu, 19 Jul 2018 09:37:44 -0700
From:   Martin KaFai Lau <kafai@...com>
To:     Daniel Borkmann <daniel@...earbox.net>
CC:     Jakub Kicinski <jakub.kicinski@...ronome.com>, <dsahern@...il.com>,
        <alexei.starovoitov@...il.com>, <netdev@...r.kernel.org>
Subject: Re: [PATCH iproute2 5/5] bpf: implement btf handling and map
 annotation

On Thu, Jul 19, 2018 at 05:43:11PM +0200, Daniel Borkmann wrote:
> On 07/19/2018 02:11 AM, Martin KaFai Lau wrote:
> > On Wed, Jul 18, 2018 at 11:13:37AM -0700, Jakub Kicinski wrote:
> >> On Wed, 18 Jul 2018 11:33:22 +0200, Daniel Borkmann wrote:
> >>> On 07/18/2018 10:42 AM, Daniel Borkmann wrote:
> >>>> On 07/18/2018 02:27 AM, Jakub Kicinski wrote:  
> >>>>> On Wed, 18 Jul 2018 01:31:22 +0200, Daniel Borkmann wrote:  
> >>>>>>   # bpftool map dump id 386
> >>>>>>    [{
> >>>>>>         "key": 0,
> >>>>>>         "value": {
> >>>>>>             "": {
> >>>>>>                 "value": 0,
> >>>>>>                 "ifindex": 0,
> >>>>>>                 "mac": []
> >>>>>>             }
> >>>>>>         }
> >>>>>>     },{
> >>>>>>         "key": 1,
> >>>>>>         "value": {
> >>>>>>             "": {
> >>>>>>                 "value": 0,
> >>>>>>                 "ifindex": 0,
> >>>>>>                 "mac": []
> >>>>>>             }
> >>>>>>         }
> >>>>>>     },{
> >>>>>>   [...]  
> >>>>>
> >>>>> Ugh, the empty keys ("") look worrying, we should probably improve
> >>>>> handling of anonymous structs in bpftool :S  
> >>>>
> >>>> Yeah agree, I think it would be nice to see a more pahole style dump
> >>>> where we have types and member names along with the value as otherwise
> >>>> it might be a bit confusing.  
> >>>
> >>> Another feature that would be super useful imho would be in the /single/
> >>> map view e.g. 'bpftool map show id 123' to have a detailed BTF key+value
> >>> type dump, so in addition to the basic map info we show pahole like info
> >>> of the structs with length/offsets.
> >>
> >> That sounds good!  We could also consider adding a btf object and
> >> commands to interrogate BTF types in the kernel in general..  Perhaps
> >> then we could add something like bpftool btf describe map id 123.
> > +1 on the btf subcommand.
> 
> That would also work, I think both might be useful to have. Former would
> all sit under a single command to show map details.
Agree that both would be useful.  btf command could address the whole BTF
object which could include many maps/types while the map command is
focusing on its own map info.

> With 'bpftool btf' you
> would also allow for a full BTF dump when a specific BTF obj id is provided?
Right, I think the BTF obj id (or file) is needed for the btf command.  and then
it should allow to do full dump or only show a particular map/type id.

A little forward thinking, map here is a C type.  Hence, I think using the
name "type" like "btftool btf id 1 show _type_ id 123" may be better when
we later expand BTF usage beyond BPF program.

> 
> >> Having the single map view show more information seems interesting, but
> >> I wonder if it could be surprising.  Is there precedent for such
> >> behaviour?
> > Having everything in one page (map show id 123) could be interesting.
> > One thing is the pahole-like output may be quite long?
> > e.g. the member of a struct could itself be another struct.
> 
> Right, though probably fine when you want to see all information specific
> to one map. Of course the 'bpftool map' list view would need to hide this
> information.
> 
> > Not sure how the pahole-like output may look like in json though.
> 
> Would the existing map k/v dump have more or less the same 'issue'?
True, the existing map k/v dump of the map data is reusing the
json {}/[]/""/number convention.  I think that is ok and actually a
pretty condensed way since people are used to this convention when
reading "data" output.

For printing out C type, I think it is more natural to have it as
close to C syntax as possible in order to have it parsable by human
eyes.  However, yes, we could reuse a similar fashion to print type
in json as we do in printing data.  Just curious, the json type output
is more for script or mostly for people that can read everything from one
json output.

For plaintext, we can just print like pahole.