Re: [PATCH net-next 2/5] bpf: Add map_name to bpf_map_info

2017-09-29 Thread Jakub Kicinski
On Fri, 29 Sep 2017 19:35:18 -0700, Martin KaFai Lau wrote:
> On Sat, Sep 30, 2017 at 02:07:46AM +, Jakub Kicinski wrote:
> > Hi Martin!
> > 
> > On Wed, 27 Sep 2017 14:37:53 -0700, Martin KaFai Lau wrote:  
> > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > > index 33ccc474fb04..252f4bc9eb25 100644
> > > --- a/include/linux/bpf.h
> > > +++ b/include/linux/bpf.h
> > > @@ -56,6 +56,7 @@ struct bpf_map {
> > >   struct work_struct work;
> > >   atomic_t usercnt;
> > >   struct bpf_map *inner_map_meta;
> > > + u8 name[BPF_OBJ_NAME_LEN];  
> > 
> > Any reason not to use plain char?  I was looking at adding names to
> > bpftool and:  
> Happy to have early user :)

The newly exposed info is so useful I couldn't resist :)

> It was mostly due to my early idea on treating the name as
> a blob without much char checking, which was then trashed
> but did not reflect that here.
> 
> I will make a followup patch later.

Awesome, thanks!


Re: [PATCH net-next 2/5] bpf: Add map_name to bpf_map_info

2017-09-29 Thread Martin KaFai Lau
On Sat, Sep 30, 2017 at 02:07:46AM +, Jakub Kicinski wrote:
> Hi Martin!
> 
> On Wed, 27 Sep 2017 14:37:53 -0700, Martin KaFai Lau wrote:
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index 33ccc474fb04..252f4bc9eb25 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -56,6 +56,7 @@ struct bpf_map {
> > struct work_struct work;
> > atomic_t usercnt;
> > struct bpf_map *inner_map_meta;
> > +   u8 name[BPF_OBJ_NAME_LEN];
> 
> Any reason not to use plain char?  I was looking at adding names to
> bpftool and:
Happy to have early user :)

It was mostly due to my early idea on treating the name as
a blob without much char checking, which was then trashed
but did not reflect that here.

I will make a followup patch later.
> 
> map.c: In function ‘show_map_close’:
> map.c:386:13: warning: pointer targets in passing argument 1 of ‘strlen’ 
> differ in signedness [-Wpointer-sign]
>   if (strlen(info->name))
>  ^~~~
> In file included from map.c:43:0:
> /usr/include/string.h:399:15: note: expected ‘const char *’ but argument is 
> of type ‘__u8 * {aka unsigned char *}’
>  extern size_t strlen (const char *__s)
>^~
> >  };
> >  
> >  /* function argument constraints */
> 


Re: [PATCH net-next 2/5] bpf: Add map_name to bpf_map_info

2017-09-29 Thread Jakub Kicinski
Hi Martin!

On Wed, 27 Sep 2017 14:37:53 -0700, Martin KaFai Lau wrote:
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 33ccc474fb04..252f4bc9eb25 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -56,6 +56,7 @@ struct bpf_map {
>   struct work_struct work;
>   atomic_t usercnt;
>   struct bpf_map *inner_map_meta;
> + u8 name[BPF_OBJ_NAME_LEN];

Any reason not to use plain char?  I was looking at adding names to
bpftool and:

map.c: In function ‘show_map_close’:
map.c:386:13: warning: pointer targets in passing argument 1 of ‘strlen’ differ 
in signedness [-Wpointer-sign]
  if (strlen(info->name))
 ^~~~
In file included from map.c:43:0:
/usr/include/string.h:399:15: note: expected ‘const char *’ but argument is of 
type ‘__u8 * {aka unsigned char *}’
 extern size_t strlen (const char *__s)
   ^~
>  };
>  
>  /* function argument constraints */