netdev - Re: [PATCH bpf-next] tools: bpftool: add map create command

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: <20181015155302.60eb001f@cakuba.netronome.com>
Date:   Mon, 15 Oct 2018 15:53:02 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     daniel@...earbox.net, netdev@...r.kernel.org,
        oss-drivers@...ronome.com
Subject: Re: [PATCH bpf-next] tools: bpftool: add map create command

On Mon, 15 Oct 2018 12:58:07 -0700, Alexei Starovoitov wrote:
> > > >  	fprintf(stderr,
> > > >  		"Usage: %s %s { show | list }   [MAP]\n"
> > > > +		"       %s %s create     FILE type TYPE key KEY_SIZE value VALUE_SIZE \\\n"
> > > > +		"                              entries MAX_ENTRIES [name NAME] [flags FLAGS] \\\n"
> > > > +		"                              [dev NAME]\n"    
> > > 
> > > I suspect as soon as bpftool has an ability to create standalone maps
> > > some folks will start relying on such interface.  
> > 
> > That'd be cool, do you see any real life use cases where its useful
> > outside of corner case testing?  
> 
> In our XDP use case we have an odd protocol for different apps to share
> common prog_array that is pinned in bpffs.
> If cmdline creation of it via bpftool was available that would have been
> an option to consider. Not saying that it would have been a better option.
> Just another option.

I see, I didn't think of prog arrays.

> > > Therefore I'd like to request to make 'name' argument to be mandatory.  
> > 
> > Will do in v2!  
> 
> thx!
>  
> > > I think in the future we will require BTF to be mandatory too.
> > > We need to move towards more transparent and debuggable infra.
> > > Do you think requiring json description of key/value would be managable to implement?
> > > Then bpftool could convert it to BTF and the map full be fully defined.
> > > I certainly understand that bpf prog can disregard the key/value layout today,
> > > but we will make verifier to enforce that in the future too.  
> > 
> > I was hoping that we can leave BTF support as a future extension, and
> > then once we have the option for the verifier to enforce BTF (a sysctl?)
> > the bpftool map create without a BTF will get rejected as one would
> > expect.    
> 
> right. something like sysctl in the future.
> 
> > IOW it's fine not to make BTF required at bpftool level and
> > leave it to system configuration.
> > 
> > I'd love to implement the BTF support right away, but I'm not sure I
> > can afford that right now time-wise.  The whole map create command is
> > pretty trivial, but for BTF we don't even have a way of dumping it
> > AFAICT.  We can pretty print values, but what is the format in which to
> > express the BTF itself?  We could do JSON, do we use an external
> > library?  Should we have a separate BTF command for that?  
> 
> I prefer standard C type description for both input and output :)
> Anyway that wasn't a request for you to do it now. More of the feature
> request for somebody to put on todo list :)

Oh, okay :)  

I will wait for John's patches to get merged and post v2, otherwise
we'd conflict on the man page.