Re: RFC Re: [PATCH v2 04/13] perf tools: Add API to config maps in bpf object

[Namhyung Kim <namhyung@kernel.org>]

Hi Arnaldo,

On Fri, Nov 27, 2015 at 10:10:54PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Nov 27, 2015 at 08:47:38AM +0000, Wang Nan escreveu:
> > bpf__config_obj() is introduced as a core API to config BPF object
> > after loading. One configuration option of maps is introduced. After
> > this patch BPF object can accept configuration like:
> > 
> >  maps:my_map:value=1234
> > 
> > (maps.my_map.value looks pretty. However, there's a small but hard
> > to fixed problem related to flex's greedy matching. Please see [1].
> > Choose ':' to avoid it in a simpler way.)
> 
> Understood the issues, but I would like to hear from Ingo, Jiri,
> Namhyung, Brian and others here, since we're setting up syntax, and yes,
> using:
>   
>   maps.my_map.value[0,3...6]=1234;
> 
> or even:
> 
>   maps->my_map->value[0,3...6]=1234;
> 
> Looks more natural than:
> 
>   maps:my_map:value[0,3...6]=1234;

Do we have to use only single separator?  What about this?

    maps:my_map.value[0,3...6]=1234;

Thanks,
Namhyung


> 
> I'll think harder about this, maybe we can find a way to use the more
> familiar dot notation somehow.
> 
> And I have to anyway test this more extensively, so I'll push the
> patches I have and try this as soon as I can.
> 
> - Arnaldo
> > 
> > This patch is more complex than the work it really does because the
> > consideration of extension. In designing of BPF map configuration,
> > following things should be considered:
> > 
> >  1. Array indics selection: perf should allow user setting different
> 
> indices :-)
> 
> >     value to different slots in an array, with syntax like:
> >     maps:my_map:value[0,3...6]=1234;
> > 
> >  2. A map can be config by different config terms, each for a part
> >     of it. For example, set each slot to pid of a thread;
> > 
> >  3. Type of value: integer is not the only valid value type. Perf
> >     event can also be put into a map after commit 35578d7984003097af2b1e3
> >     (bpf: Implement function bpf_perf_event_read() that get the selected
> >     hardware PMU conuter);
> > 
> >  4. For hash table, it is possible to use string or other as key;
> > 
> >  5. It is possible that map configuration is unable to be setup
> >     during parsing. Perf event is an example.
> > 
> > Therefore, this patch does following:
> > 
> >  1. Instead of updating map element during parsing, this patch stores
> >     map config options in 'struct bpf_map_priv'. Following patches
> >     would apply those configs at proper time;
> > 
> >  2. Link map operations to a list so a map can have multiple config
> >     terms attached, so different parts can be configured separately;
> > 
> >  3. Make 'struct bpf_map_priv' extensible so following patches can
> >     add new types of keys and operations;
> > 
> >  4. Use bpf_config_map_funcs array to support more maps config options.
> > 
> > Since the patch changing event parser to parse BPF object config is
> > relative large, I put in another commit. Code in this patch
> > could be tested after applying next patch.
> > 
> > [1] http://lkml.kernel.org/g/564ED621.4050500@huawei.com
>  
> > Signed-off-by: Wang Nan <wangnan0@huawei.com>
> > Signed-off-by: He Kuang <hekuang@huawei.com>
> > Cc: Alexei Starovoitov <ast@kernel.org>
> > Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > Cc: Zefan Li <lizefan@huawei.com>
> > Cc: pi3orama@163.com