netdev - Re: [PATCH 1/1] bpftool: Allow referring to maps by its name

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: <20190315132523.74c69051@cakuba.netronome.com>
Date:   Fri, 15 Mar 2019 13:25:23 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>
Cc:     Quentin Monnet <quentin.monnet@...ronome.com>,
        Arnaldo Carvalho de Melo <acme@...hat.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Jiri Olsa <jolsa@...nel.org>, Martin KaFai Lau <kafai@...com>,
        Namhyung Kim <namhyung@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Song Liu <songliubraving@...com>,
        Stanislav Fomichev <sdf@...gle.com>,
        Yonghong Song <yhs@...com>, bpf@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [PATCH 1/1] bpftool: Allow referring to maps by its name

On Fri, 15 Mar 2019 16:48:00 -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Mar 15, 2019 at 07:41:35PM +0000, Quentin Monnet escreveu:
> > 2019-03-15 12:16 UTC-0700 ~ Jakub Kicinski <jakub.kicinski@...ronome.com>  
> > > On Fri, 15 Mar 2019 15:41:41 -0300, Arnaldo Carvalho de Melo wrote:  
> > > > While developing 'perf trace' and looking at BPF maps it puts in place I
> > > > noticed that one needs to first use 'bpftool map' to lookup a map id to
> > > > then use 'bpftool map dump map id <map-id-looked-up>'.
> > > > 
> > > > This is needed because everytime we restart 'perf trace' the map IDs
> > > > gets changed so we need to do the ID lookup again.
> > > > 
> > > > To speed up this sequence, allow specifying just the map name, look up
> > > > its ID and then use the existing routines as if the user had provided
> > > > the map id.  
> > > 
> > > I can see how it could be useful for quick debug.  Names are not
> > > guaranteed to be unique, though, do you not care about potential
> > > duplicates?
> > >   
> > 
> > Jakub beat me to it by a few seconds :). I discussed possible workarounds on
> > that point with him though, so here are my two cents. I see two options:
> > first one would be to return the ids of, and then dump info about all maps
> > having that name. The second one would be to simply return an error if
> > duplicate names are found.
> > 
> > We agreed that returning an error if there are duplicates is probably the
> > best way to go, because dealing with several maps for anything else than
> > "bpftool map show" would be problematic (clearly, we don't want to update
> > all maps with a given name at once). Of course if you have other
> > suggestions, feel free to propose :).  
> 
> So, if there are dups, just tell that and print the ids, i.e. the
> following, for any 'bpf map' command that finds dups, except for this
> one:
> 
> [root@...co ~]# bpftool map list pids_filtered
> 45: hash  name pids_filtered  flags 0x0
> 	key 4B  value 1B  max_entries 64  memlock 8192B
> 90: hash  name pids_filtered  flags 0x0
> 	key 4B  value 80B  max_entries 512  memlock do-the-mathB
> [root@...co ~]#
> 
> I.e. if there are multiple 'pids_filtered' maps, the output for, say,
> 'dump' would be:
> 
> [root@...co ~]# bpftool map dump pids_filtered
> There are multiple maps with this name, use 'bpftool map dump id NR' to disambiguate:
> -------------------------------------------------------------------------------------
> 45: hash  name pids_filtered  flags 0x0
> 	key 4B  value 1B  max_entries 64  memlock 8192B
> 90: hash  name pids_filtered  flags 0x0
> 	key 4B  value 80B  max_entries 512  memlock do-the-mathB
> [root@...co ~]#

Yes, this looks good (modulo JSON output, which should probably be
just a single "error" value).

For simplicity I'd personally go for just always printing an error
if name is duplicated and not bother with listing the options.