Re: [PATCH v4 1/4] bpf: Make the bpf_prog_array_map more generic

2015-07-31 Thread Alexei Starovoitov

On 7/31/15 1:50 AM, xiakaixu wrote:

于 2015/7/30 9:44, Alexei Starovoitov 写道:

On 7/29/15 4:17 PM, Daniel Borkmann wrote:

-if (map->map_type == BPF_MAP_TYPE_PROG_ARRAY)
+if (map->map_type >= BPF_MAP_TYPE_PROG_ARRAY)
   /* prog_array stores refcnt-ed bpf_prog pointers
* release them all when user space closes prog_array_fd
*/
-bpf_prog_array_map_clear(map);
+bpf_fd_array_map_clear(map);


When we are going to add a new map type to the eBPF framework that is not
an fd_array_map thing, this assumption of map->map_type >=
BPF_MAP_TYPE_PROG_ARRAY
might not hold then ...


Also I think here changing == to >= is probably unnecessary.
prog_array needs to do it because of circular dependency
whereas perf_event_array cannot have it.
Even when we attach bpf prog to perf_event and then add it to
perf_event_array used by the same prog, right?
Please test such scenario just in case.


Not sure completely understand what you mean. You know, we can
attach bpf_prog to kprobe events. For now, we limit few event
types, only PERF_EVENT_RAW & PERF_EVENT_HARDWARE event can
be accessed in bpf_perf_event_read(). Seems like the dependency
scenario won't happen.


ahh, yes, you're correct. There is no circular dependency.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/4] bpf: Make the bpf_prog_array_map more generic

2015-07-31 Thread xiakaixu
于 2015/7/30 9:44, Alexei Starovoitov 写道:
> On 7/29/15 4:17 PM, Daniel Borkmann wrote:
>>> -if (map->map_type == BPF_MAP_TYPE_PROG_ARRAY)
>>> +if (map->map_type >= BPF_MAP_TYPE_PROG_ARRAY)
>>>   /* prog_array stores refcnt-ed bpf_prog pointers
>>>* release them all when user space closes prog_array_fd
>>>*/
>>> -bpf_prog_array_map_clear(map);
>>> +bpf_fd_array_map_clear(map);
>>
>> When we are going to add a new map type to the eBPF framework that is not
>> an fd_array_map thing, this assumption of map->map_type >=
>> BPF_MAP_TYPE_PROG_ARRAY
>> might not hold then ...
> 
> Also I think here changing == to >= is probably unnecessary.
> prog_array needs to do it because of circular dependency
> whereas perf_event_array cannot have it.
> Even when we attach bpf prog to perf_event and then add it to
> perf_event_array used by the same prog, right?
> Please test such scenario just in case.

Not sure completely understand what you mean. You know, we can
attach bpf_prog to kprobe events. For now, we limit few event
types, only PERF_EVENT_RAW & PERF_EVENT_HARDWARE event can
be accessed in bpf_perf_event_read(). Seems like the dependency
scenario won't happen.
I will add the event decrement refcnt function to map_free in V5.
right?
> 
> 
> .
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/4] bpf: Make the bpf_prog_array_map more generic

2015-07-31 Thread Alexei Starovoitov

On 7/31/15 1:50 AM, xiakaixu wrote:

于 2015/7/30 9:44, Alexei Starovoitov 写道:

On 7/29/15 4:17 PM, Daniel Borkmann wrote:

-if (map-map_type == BPF_MAP_TYPE_PROG_ARRAY)
+if (map-map_type = BPF_MAP_TYPE_PROG_ARRAY)
   /* prog_array stores refcnt-ed bpf_prog pointers
* release them all when user space closes prog_array_fd
*/
-bpf_prog_array_map_clear(map);
+bpf_fd_array_map_clear(map);


When we are going to add a new map type to the eBPF framework that is not
an fd_array_map thing, this assumption of map-map_type =
BPF_MAP_TYPE_PROG_ARRAY
might not hold then ...


Also I think here changing == to = is probably unnecessary.
prog_array needs to do it because of circular dependency
whereas perf_event_array cannot have it.
Even when we attach bpf prog to perf_event and then add it to
perf_event_array used by the same prog, right?
Please test such scenario just in case.


Not sure completely understand what you mean. You know, we can
attach bpf_prog to kprobe events. For now, we limit few event
types, only PERF_EVENT_RAW  PERF_EVENT_HARDWARE event can
be accessed in bpf_perf_event_read(). Seems like the dependency
scenario won't happen.


ahh, yes, you're correct. There is no circular dependency.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/4] bpf: Make the bpf_prog_array_map more generic

2015-07-31 Thread xiakaixu
于 2015/7/30 9:44, Alexei Starovoitov 写道:
 On 7/29/15 4:17 PM, Daniel Borkmann wrote:
 -if (map-map_type == BPF_MAP_TYPE_PROG_ARRAY)
 +if (map-map_type = BPF_MAP_TYPE_PROG_ARRAY)
   /* prog_array stores refcnt-ed bpf_prog pointers
* release them all when user space closes prog_array_fd
*/
 -bpf_prog_array_map_clear(map);
 +bpf_fd_array_map_clear(map);

 When we are going to add a new map type to the eBPF framework that is not
 an fd_array_map thing, this assumption of map-map_type =
 BPF_MAP_TYPE_PROG_ARRAY
 might not hold then ...
 
 Also I think here changing == to = is probably unnecessary.
 prog_array needs to do it because of circular dependency
 whereas perf_event_array cannot have it.
 Even when we attach bpf prog to perf_event and then add it to
 perf_event_array used by the same prog, right?
 Please test such scenario just in case.

Not sure completely understand what you mean. You know, we can
attach bpf_prog to kprobe events. For now, we limit few event
types, only PERF_EVENT_RAW  PERF_EVENT_HARDWARE event can
be accessed in bpf_perf_event_read(). Seems like the dependency
scenario won't happen.
I will add the event decrement refcnt function to map_free in V5.
right?
 
 
 .
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/4] bpf: Make the bpf_prog_array_map more generic

2015-07-29 Thread Alexei Starovoitov

On 7/29/15 4:17 PM, Daniel Borkmann wrote:

-if (map->map_type == BPF_MAP_TYPE_PROG_ARRAY)
+if (map->map_type >= BPF_MAP_TYPE_PROG_ARRAY)
  /* prog_array stores refcnt-ed bpf_prog pointers
   * release them all when user space closes prog_array_fd
   */
-bpf_prog_array_map_clear(map);
+bpf_fd_array_map_clear(map);


When we are going to add a new map type to the eBPF framework that is not
an fd_array_map thing, this assumption of map->map_type >=
BPF_MAP_TYPE_PROG_ARRAY
might not hold then ...


Also I think here changing == to >= is probably unnecessary.
prog_array needs to do it because of circular dependency
whereas perf_event_array cannot have it.
Even when we attach bpf prog to perf_event and then add it to
perf_event_array used by the same prog, right?
Please test such scenario just in case.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/4] bpf: Make the bpf_prog_array_map more generic

2015-07-29 Thread Daniel Borkmann

On 07/28/2015 01:17 PM, Kaixu Xia wrote:

From: Wang Nan 

According to the comments from Daniel, rewrite part of
the bpf_prog_array map code and make it more generic.
So the new perf_event_array map type can reuse most of
code with bpf_prog_array map and add fewer lines of
special code.

Tested the samples/bpf/tracex5 after this patch:
$ sudo ./tracex5
...
dd-1051  [000] d...26.682903: : mmap
dd-1051  [000] d...26.698348: : syscall=102 (one of get/set 
uid/pid/gid)
dd-1051  [000] d...26.703892: : read(fd=0, buf=0078c010, 
size=512)
dd-1051  [000] d...26.705847: : write(fd=1, buf=0078c010, 
size=512)
dd-1051  [000] d...26.707914: : read(fd=0, buf=0078c010, 
size=512)
dd-1051  [000] d...26.710988: : write(fd=1, buf=0078c010, 
size=512)
dd-1051  [000] d...26.711865: : read(fd=0, buf=0078c010, 
size=512)
dd-1051  [000] d...26.712704: : write(fd=1, buf=0078c010, 
size=512)
...

Signed-off-by: Wang Nan 
Signed-off-by: Kaixu Xia 
---
  include/linux/bpf.h   |   6 ++-
  kernel/bpf/arraymap.c | 104 +++---
  kernel/bpf/syscall.c  |   4 +-
  3 files changed, 80 insertions(+), 34 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 4383476..610b730 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -130,6 +130,8 @@ struct bpf_prog_aux {
};
  };

+struct fd_array_map_ops;
+
  struct bpf_array {
struct bpf_map map;
u32 elem_size;
@@ -140,15 +142,17 @@ struct bpf_array {
 */
enum bpf_prog_type owner_prog_type;
bool owner_jited;
+   const struct fd_array_map_ops* fd_ops;
union {
char value[0] __aligned(8);
+   void *ptrs[0] __aligned(8);
struct bpf_prog *prog[0] __aligned(8);


After your conversion, prog member from the union is not used here anymore
(only as offsetof(...) in JITs). We should probably get rid of it then.


};
  };
  #define MAX_TAIL_CALL_CNT 32

  u64 bpf_tail_call(u64 ctx, u64 r2, u64 index, u64 r4, u64 r5);
-void bpf_prog_array_map_clear(struct bpf_map *map);
+void bpf_fd_array_map_clear(struct bpf_map *map);
  bool bpf_prog_array_compatible(struct bpf_array *array, const struct bpf_prog 
*fp);
  const struct bpf_func_proto *bpf_get_trace_printk_proto(void);

diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index cb31229..4784cdc 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -150,15 +150,62 @@ static int __init register_array_map(void)
  }
  late_initcall(register_array_map);

-static struct bpf_map *prog_array_map_alloc(union bpf_attr *attr)
+struct fd_array_map_ops {
+   void *(*get_ptr)(struct bpf_array *array, int fd);
+   void (*put_ptr)(struct bpf_array *array, void *ptr);
+};
+
+static void *prog_fd_array_get_ptr(struct bpf_array *array, int fd)
+{
+   struct bpf_prog *prog = bpf_prog_get(fd);
+   if (IS_ERR(prog))
+   return prog;
+
+   if (!bpf_prog_array_compatible(array, prog)) {
+   bpf_prog_put(prog);
+   return ERR_PTR(-EINVAL);
+   }
+   return prog;
+}
+
+static void prog_fd_array_put_ptr(struct bpf_array *array __maybe_unused,
+ void *ptr)


array member seems not to be used in both implementations. It should then
probably not be part of the API?


+{
+   struct bpf_prog *prog = (struct bpf_prog *)ptr;


No cast on void * needed.


+
+   bpf_prog_put_rcu(prog);
+}
+
+static const struct fd_array_map_ops prog_fd_array_map_ops = {
+   .get_ptr= prog_fd_array_get_ptr,
+   .put_ptr= prog_fd_array_put_ptr,
+};
+
+static struct bpf_map *fd_array_map_alloc(union bpf_attr *attr,
+ const struct fd_array_map_ops *ops)
  {
-   /* only bpf_prog file descriptors can be stored in prog_array map */
+   struct bpf_map *map;
+   struct bpf_array *array;
+
+   /* only file descriptors can be stored in this type of map */
if (attr->value_size != sizeof(u32))
return ERR_PTR(-EINVAL);
-   return array_map_alloc(attr);
+
+   map = array_map_alloc(attr);
+   if (IS_ERR(map))
+   return map;
+
+   array = container_of(map, struct bpf_array, map);
+   array->fd_ops = ops;
+   return map;
+}
+
+static struct bpf_map *prog_array_map_alloc(union bpf_attr *attr)
+{
+   return fd_array_map_alloc(attr, _fd_array_map_ops);
  }

-static void prog_array_map_free(struct bpf_map *map)
+static void fd_array_map_free(struct bpf_map *map)
  {
struct bpf_array *array = container_of(map, struct bpf_array, map);
int i;
@@ -167,21 +214,21 @@ static void prog_array_map_free(struct bpf_map *map)

/* make sure it's empty */
for (i = 0; i < array->map.max_entries; i++)
- 

Re: [PATCH v4 1/4] bpf: Make the bpf_prog_array_map more generic

2015-07-29 Thread Daniel Borkmann

On 07/28/2015 01:17 PM, Kaixu Xia wrote:

From: Wang Nan wangn...@huawei.com

According to the comments from Daniel, rewrite part of
the bpf_prog_array map code and make it more generic.
So the new perf_event_array map type can reuse most of
code with bpf_prog_array map and add fewer lines of
special code.

Tested the samples/bpf/tracex5 after this patch:
$ sudo ./tracex5
...
dd-1051  [000] d...26.682903: : mmap
dd-1051  [000] d...26.698348: : syscall=102 (one of get/set 
uid/pid/gid)
dd-1051  [000] d...26.703892: : read(fd=0, buf=0078c010, 
size=512)
dd-1051  [000] d...26.705847: : write(fd=1, buf=0078c010, 
size=512)
dd-1051  [000] d...26.707914: : read(fd=0, buf=0078c010, 
size=512)
dd-1051  [000] d...26.710988: : write(fd=1, buf=0078c010, 
size=512)
dd-1051  [000] d...26.711865: : read(fd=0, buf=0078c010, 
size=512)
dd-1051  [000] d...26.712704: : write(fd=1, buf=0078c010, 
size=512)
...

Signed-off-by: Wang Nan wangn...@huawei.com
Signed-off-by: Kaixu Xia xiaka...@huawei.com
---
  include/linux/bpf.h   |   6 ++-
  kernel/bpf/arraymap.c | 104 +++---
  kernel/bpf/syscall.c  |   4 +-
  3 files changed, 80 insertions(+), 34 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 4383476..610b730 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -130,6 +130,8 @@ struct bpf_prog_aux {
};
  };

+struct fd_array_map_ops;
+
  struct bpf_array {
struct bpf_map map;
u32 elem_size;
@@ -140,15 +142,17 @@ struct bpf_array {
 */
enum bpf_prog_type owner_prog_type;
bool owner_jited;
+   const struct fd_array_map_ops* fd_ops;
union {
char value[0] __aligned(8);
+   void *ptrs[0] __aligned(8);
struct bpf_prog *prog[0] __aligned(8);


After your conversion, prog member from the union is not used here anymore
(only as offsetof(...) in JITs). We should probably get rid of it then.


};
  };
  #define MAX_TAIL_CALL_CNT 32

  u64 bpf_tail_call(u64 ctx, u64 r2, u64 index, u64 r4, u64 r5);
-void bpf_prog_array_map_clear(struct bpf_map *map);
+void bpf_fd_array_map_clear(struct bpf_map *map);
  bool bpf_prog_array_compatible(struct bpf_array *array, const struct bpf_prog 
*fp);
  const struct bpf_func_proto *bpf_get_trace_printk_proto(void);

diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index cb31229..4784cdc 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -150,15 +150,62 @@ static int __init register_array_map(void)
  }
  late_initcall(register_array_map);

-static struct bpf_map *prog_array_map_alloc(union bpf_attr *attr)
+struct fd_array_map_ops {
+   void *(*get_ptr)(struct bpf_array *array, int fd);
+   void (*put_ptr)(struct bpf_array *array, void *ptr);
+};
+
+static void *prog_fd_array_get_ptr(struct bpf_array *array, int fd)
+{
+   struct bpf_prog *prog = bpf_prog_get(fd);
+   if (IS_ERR(prog))
+   return prog;
+
+   if (!bpf_prog_array_compatible(array, prog)) {
+   bpf_prog_put(prog);
+   return ERR_PTR(-EINVAL);
+   }
+   return prog;
+}
+
+static void prog_fd_array_put_ptr(struct bpf_array *array __maybe_unused,
+ void *ptr)


array member seems not to be used in both implementations. It should then
probably not be part of the API?


+{
+   struct bpf_prog *prog = (struct bpf_prog *)ptr;


No cast on void * needed.


+
+   bpf_prog_put_rcu(prog);
+}
+
+static const struct fd_array_map_ops prog_fd_array_map_ops = {
+   .get_ptr= prog_fd_array_get_ptr,
+   .put_ptr= prog_fd_array_put_ptr,
+};
+
+static struct bpf_map *fd_array_map_alloc(union bpf_attr *attr,
+ const struct fd_array_map_ops *ops)
  {
-   /* only bpf_prog file descriptors can be stored in prog_array map */
+   struct bpf_map *map;
+   struct bpf_array *array;
+
+   /* only file descriptors can be stored in this type of map */
if (attr-value_size != sizeof(u32))
return ERR_PTR(-EINVAL);
-   return array_map_alloc(attr);
+
+   map = array_map_alloc(attr);
+   if (IS_ERR(map))
+   return map;
+
+   array = container_of(map, struct bpf_array, map);
+   array-fd_ops = ops;
+   return map;
+}
+
+static struct bpf_map *prog_array_map_alloc(union bpf_attr *attr)
+{
+   return fd_array_map_alloc(attr, prog_fd_array_map_ops);
  }

-static void prog_array_map_free(struct bpf_map *map)
+static void fd_array_map_free(struct bpf_map *map)
  {
struct bpf_array *array = container_of(map, struct bpf_array, map);
int i;
@@ -167,21 +214,21 @@ static void prog_array_map_free(struct bpf_map *map)

/* make sure it's empty */

Re: [PATCH v4 1/4] bpf: Make the bpf_prog_array_map more generic

2015-07-29 Thread Alexei Starovoitov

On 7/29/15 4:17 PM, Daniel Borkmann wrote:

-if (map-map_type == BPF_MAP_TYPE_PROG_ARRAY)
+if (map-map_type = BPF_MAP_TYPE_PROG_ARRAY)
  /* prog_array stores refcnt-ed bpf_prog pointers
   * release them all when user space closes prog_array_fd
   */
-bpf_prog_array_map_clear(map);
+bpf_fd_array_map_clear(map);


When we are going to add a new map type to the eBPF framework that is not
an fd_array_map thing, this assumption of map-map_type =
BPF_MAP_TYPE_PROG_ARRAY
might not hold then ...


Also I think here changing == to = is probably unnecessary.
prog_array needs to do it because of circular dependency
whereas perf_event_array cannot have it.
Even when we attach bpf prog to perf_event and then add it to
perf_event_array used by the same prog, right?
Please test such scenario just in case.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v4 1/4] bpf: Make the bpf_prog_array_map more generic

2015-07-28 Thread Kaixu Xia
From: Wang Nan 

According to the comments from Daniel, rewrite part of
the bpf_prog_array map code and make it more generic.
So the new perf_event_array map type can reuse most of
code with bpf_prog_array map and add fewer lines of
special code.

Tested the samples/bpf/tracex5 after this patch:
$ sudo ./tracex5
...
dd-1051  [000] d...26.682903: : mmap
dd-1051  [000] d...26.698348: : syscall=102 (one of get/set 
uid/pid/gid)
dd-1051  [000] d...26.703892: : read(fd=0, buf=0078c010, 
size=512)
dd-1051  [000] d...26.705847: : write(fd=1, buf=0078c010, 
size=512)
dd-1051  [000] d...26.707914: : read(fd=0, buf=0078c010, 
size=512)
dd-1051  [000] d...26.710988: : write(fd=1, buf=0078c010, 
size=512)
dd-1051  [000] d...26.711865: : read(fd=0, buf=0078c010, 
size=512)
dd-1051  [000] d...26.712704: : write(fd=1, buf=0078c010, 
size=512)
...

Signed-off-by: Wang Nan 
Signed-off-by: Kaixu Xia 
---
 include/linux/bpf.h   |   6 ++-
 kernel/bpf/arraymap.c | 104 +++---
 kernel/bpf/syscall.c  |   4 +-
 3 files changed, 80 insertions(+), 34 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 4383476..610b730 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -130,6 +130,8 @@ struct bpf_prog_aux {
};
 };
 
+struct fd_array_map_ops;
+
 struct bpf_array {
struct bpf_map map;
u32 elem_size;
@@ -140,15 +142,17 @@ struct bpf_array {
 */
enum bpf_prog_type owner_prog_type;
bool owner_jited;
+   const struct fd_array_map_ops* fd_ops;
union {
char value[0] __aligned(8);
+   void *ptrs[0] __aligned(8);
struct bpf_prog *prog[0] __aligned(8);
};
 };
 #define MAX_TAIL_CALL_CNT 32
 
 u64 bpf_tail_call(u64 ctx, u64 r2, u64 index, u64 r4, u64 r5);
-void bpf_prog_array_map_clear(struct bpf_map *map);
+void bpf_fd_array_map_clear(struct bpf_map *map);
 bool bpf_prog_array_compatible(struct bpf_array *array, const struct bpf_prog 
*fp);
 const struct bpf_func_proto *bpf_get_trace_printk_proto(void);
 
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index cb31229..4784cdc 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -150,15 +150,62 @@ static int __init register_array_map(void)
 }
 late_initcall(register_array_map);
 
-static struct bpf_map *prog_array_map_alloc(union bpf_attr *attr)
+struct fd_array_map_ops {
+   void *(*get_ptr)(struct bpf_array *array, int fd);
+   void (*put_ptr)(struct bpf_array *array, void *ptr);
+};
+
+static void *prog_fd_array_get_ptr(struct bpf_array *array, int fd)
+{
+   struct bpf_prog *prog = bpf_prog_get(fd);
+   if (IS_ERR(prog))
+   return prog;
+
+   if (!bpf_prog_array_compatible(array, prog)) {
+   bpf_prog_put(prog);
+   return ERR_PTR(-EINVAL);
+   }
+   return prog;
+}
+
+static void prog_fd_array_put_ptr(struct bpf_array *array __maybe_unused,
+ void *ptr)
+{
+   struct bpf_prog *prog = (struct bpf_prog *)ptr;
+
+   bpf_prog_put_rcu(prog);
+}
+
+static const struct fd_array_map_ops prog_fd_array_map_ops = {
+   .get_ptr= prog_fd_array_get_ptr,
+   .put_ptr= prog_fd_array_put_ptr,
+};
+
+static struct bpf_map *fd_array_map_alloc(union bpf_attr *attr,
+ const struct fd_array_map_ops *ops)
 {
-   /* only bpf_prog file descriptors can be stored in prog_array map */
+   struct bpf_map *map;
+   struct bpf_array *array;
+
+   /* only file descriptors can be stored in this type of map */
if (attr->value_size != sizeof(u32))
return ERR_PTR(-EINVAL);
-   return array_map_alloc(attr);
+
+   map = array_map_alloc(attr);
+   if (IS_ERR(map))
+   return map;
+
+   array = container_of(map, struct bpf_array, map);
+   array->fd_ops = ops;
+   return map;
+}
+
+static struct bpf_map *prog_array_map_alloc(union bpf_attr *attr)
+{
+   return fd_array_map_alloc(attr, _fd_array_map_ops);
 }
 
-static void prog_array_map_free(struct bpf_map *map)
+static void fd_array_map_free(struct bpf_map *map)
 {
struct bpf_array *array = container_of(map, struct bpf_array, map);
int i;
@@ -167,21 +214,21 @@ static void prog_array_map_free(struct bpf_map *map)
 
/* make sure it's empty */
for (i = 0; i < array->map.max_entries; i++)
-   BUG_ON(array->prog[i] != NULL);
+   BUG_ON(array->ptrs[i] != NULL);
kvfree(array);
 }
 
-static void *prog_array_map_lookup_elem(struct bpf_map *map, void *key)
+static void *fd_array_map_lookup_elem(struct bpf_map *map, void *key)
 {
return NULL;
 }
 
 /* only called from syscall */
-static int 

[PATCH v4 1/4] bpf: Make the bpf_prog_array_map more generic

2015-07-28 Thread Kaixu Xia
From: Wang Nan wangn...@huawei.com

According to the comments from Daniel, rewrite part of
the bpf_prog_array map code and make it more generic.
So the new perf_event_array map type can reuse most of
code with bpf_prog_array map and add fewer lines of
special code.

Tested the samples/bpf/tracex5 after this patch:
$ sudo ./tracex5
...
dd-1051  [000] d...26.682903: : mmap
dd-1051  [000] d...26.698348: : syscall=102 (one of get/set 
uid/pid/gid)
dd-1051  [000] d...26.703892: : read(fd=0, buf=0078c010, 
size=512)
dd-1051  [000] d...26.705847: : write(fd=1, buf=0078c010, 
size=512)
dd-1051  [000] d...26.707914: : read(fd=0, buf=0078c010, 
size=512)
dd-1051  [000] d...26.710988: : write(fd=1, buf=0078c010, 
size=512)
dd-1051  [000] d...26.711865: : read(fd=0, buf=0078c010, 
size=512)
dd-1051  [000] d...26.712704: : write(fd=1, buf=0078c010, 
size=512)
...

Signed-off-by: Wang Nan wangn...@huawei.com
Signed-off-by: Kaixu Xia xiaka...@huawei.com
---
 include/linux/bpf.h   |   6 ++-
 kernel/bpf/arraymap.c | 104 +++---
 kernel/bpf/syscall.c  |   4 +-
 3 files changed, 80 insertions(+), 34 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 4383476..610b730 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -130,6 +130,8 @@ struct bpf_prog_aux {
};
 };
 
+struct fd_array_map_ops;
+
 struct bpf_array {
struct bpf_map map;
u32 elem_size;
@@ -140,15 +142,17 @@ struct bpf_array {
 */
enum bpf_prog_type owner_prog_type;
bool owner_jited;
+   const struct fd_array_map_ops* fd_ops;
union {
char value[0] __aligned(8);
+   void *ptrs[0] __aligned(8);
struct bpf_prog *prog[0] __aligned(8);
};
 };
 #define MAX_TAIL_CALL_CNT 32
 
 u64 bpf_tail_call(u64 ctx, u64 r2, u64 index, u64 r4, u64 r5);
-void bpf_prog_array_map_clear(struct bpf_map *map);
+void bpf_fd_array_map_clear(struct bpf_map *map);
 bool bpf_prog_array_compatible(struct bpf_array *array, const struct bpf_prog 
*fp);
 const struct bpf_func_proto *bpf_get_trace_printk_proto(void);
 
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index cb31229..4784cdc 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -150,15 +150,62 @@ static int __init register_array_map(void)
 }
 late_initcall(register_array_map);
 
-static struct bpf_map *prog_array_map_alloc(union bpf_attr *attr)
+struct fd_array_map_ops {
+   void *(*get_ptr)(struct bpf_array *array, int fd);
+   void (*put_ptr)(struct bpf_array *array, void *ptr);
+};
+
+static void *prog_fd_array_get_ptr(struct bpf_array *array, int fd)
+{
+   struct bpf_prog *prog = bpf_prog_get(fd);
+   if (IS_ERR(prog))
+   return prog;
+
+   if (!bpf_prog_array_compatible(array, prog)) {
+   bpf_prog_put(prog);
+   return ERR_PTR(-EINVAL);
+   }
+   return prog;
+}
+
+static void prog_fd_array_put_ptr(struct bpf_array *array __maybe_unused,
+ void *ptr)
+{
+   struct bpf_prog *prog = (struct bpf_prog *)ptr;
+
+   bpf_prog_put_rcu(prog);
+}
+
+static const struct fd_array_map_ops prog_fd_array_map_ops = {
+   .get_ptr= prog_fd_array_get_ptr,
+   .put_ptr= prog_fd_array_put_ptr,
+};
+
+static struct bpf_map *fd_array_map_alloc(union bpf_attr *attr,
+ const struct fd_array_map_ops *ops)
 {
-   /* only bpf_prog file descriptors can be stored in prog_array map */
+   struct bpf_map *map;
+   struct bpf_array *array;
+
+   /* only file descriptors can be stored in this type of map */
if (attr-value_size != sizeof(u32))
return ERR_PTR(-EINVAL);
-   return array_map_alloc(attr);
+
+   map = array_map_alloc(attr);
+   if (IS_ERR(map))
+   return map;
+
+   array = container_of(map, struct bpf_array, map);
+   array-fd_ops = ops;
+   return map;
+}
+
+static struct bpf_map *prog_array_map_alloc(union bpf_attr *attr)
+{
+   return fd_array_map_alloc(attr, prog_fd_array_map_ops);
 }
 
-static void prog_array_map_free(struct bpf_map *map)
+static void fd_array_map_free(struct bpf_map *map)
 {
struct bpf_array *array = container_of(map, struct bpf_array, map);
int i;
@@ -167,21 +214,21 @@ static void prog_array_map_free(struct bpf_map *map)
 
/* make sure it's empty */
for (i = 0; i  array-map.max_entries; i++)
-   BUG_ON(array-prog[i] != NULL);
+   BUG_ON(array-ptrs[i] != NULL);
kvfree(array);
 }
 
-static void *prog_array_map_lookup_elem(struct bpf_map *map, void *key)
+static void *fd_array_map_lookup_elem(struct bpf_map *map, void *key)
 {
return NULL;
 }