diff mbox series

[v2,bpf-next,1/2] bpf: introduce BPF_F_PRESERVE_ELEMS for perf event array

Message ID 20200929215659.3938706-2-songliubraving@fb.com (mailing list archive)
State Superseded
Headers show
Series introduce BPF_F_PRESERVE_ELEMS | expand

Commit Message

Song Liu Sept. 29, 2020, 9:56 p.m. UTC
Currently, perf event in perf event array is removed from the array when
the map fd used to add the event is closed. This behavior makes it
difficult to the share perf events with perf event array.

Introduce perf event map that keeps the perf event open with a new flag
BPF_F_PRESERVE_ELEMS. With this flag set, perf events in the array are not
removed when the original map fd is closed. Instead, the perf event will
stay in the map until 1) it is explicitly removed from the array; or 2)
the array is freed.

Signed-off-by: Song Liu <songliubraving@fb.com>
---
 include/uapi/linux/bpf.h       |  3 +++
 kernel/bpf/arraymap.c          | 21 +++++++++++++++++++--
 tools/include/uapi/linux/bpf.h |  3 +++
 3 files changed, 25 insertions(+), 2 deletions(-)

Comments

Daniel Borkmann Sept. 30, 2020, 2:49 p.m. UTC | #1
On 9/29/20 11:56 PM, Song Liu wrote:
[...]
>   
> +static void bpf_fd_array_map_clear(struct bpf_map *map);
> +
> +static void perf_event_fd_array_map_free(struct bpf_map *map)
> +{
> +	if (map->map_flags & BPF_F_PRESERVE_ELEMS)
> +		bpf_fd_array_map_clear(map);
> +	fd_array_map_free(map);
> +}

Not quite sure why you place that here and added the fwd declaration? If you
place perf_event_fd_array_map_free() near perf_event_array_map_ops, then you
also don't need the additional bpf_fd_array_map_clear declaration.

>   static void *prog_fd_array_get_ptr(struct bpf_map *map,
>   				   struct file *map_file, int fd)
>   {
> @@ -1134,6 +1148,9 @@ static void perf_event_fd_array_release(struct bpf_map *map,
>   	struct bpf_event_entry *ee;
>   	int i;
>   
> +	if (map->map_flags & BPF_F_PRESERVE_ELEMS)
> +		return;
> +
>   	rcu_read_lock();
>   	for (i = 0; i < array->map.max_entries; i++) {
>   		ee = READ_ONCE(array->ptrs[i]);
> @@ -1148,7 +1165,7 @@ const struct bpf_map_ops perf_event_array_map_ops = {
>   	.map_meta_equal = bpf_map_meta_equal,
>   	.map_alloc_check = fd_array_map_alloc_check,
>   	.map_alloc = array_map_alloc,
> -	.map_free = fd_array_map_free,
> +	.map_free = perf_event_fd_array_map_free,
>   	.map_get_next_key = array_map_get_next_key,
>   	.map_lookup_elem = fd_array_map_lookup_elem,
>   	.map_delete_elem = fd_array_map_delete_elem,
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 82522f05c0213..ea78eb89f8d67 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -414,6 +414,9 @@ enum {
>   
>   /* Enable memory-mapping BPF map */
>   	BPF_F_MMAPABLE		= (1U << 10),
> +
> +/* Share perf_event among processes */
> +	BPF_F_PRESERVE_ELEMS	= (1U << 11),
>   };
>   
>   /* Flags for BPF_PROG_QUERY. */
>
Song Liu Sept. 30, 2020, 3:04 p.m. UTC | #2
> On Sep 30, 2020, at 7:49 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> 
> On 9/29/20 11:56 PM, Song Liu wrote:
> [...]
>>  +static void bpf_fd_array_map_clear(struct bpf_map *map);
>> +
>> +static void perf_event_fd_array_map_free(struct bpf_map *map)
>> +{
>> +	if (map->map_flags & BPF_F_PRESERVE_ELEMS)
>> +		bpf_fd_array_map_clear(map);
>> +	fd_array_map_free(map);
>> +}
> 
> Not quite sure why you place that here and added the fwd declaration? If you
> place perf_event_fd_array_map_free() near perf_event_array_map_ops, then you
> also don't need the additional bpf_fd_array_map_clear declaration.

Yeah.. I misread the line number...

Fixing it in v3.

Thanks,
Song
diff mbox series

Patch

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 82522f05c0213..ea78eb89f8d67 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -414,6 +414,9 @@  enum {
 
 /* Enable memory-mapping BPF map */
 	BPF_F_MMAPABLE		= (1U << 10),
+
+/* Share perf_event among processes */
+	BPF_F_PRESERVE_ELEMS	= (1U << 11),
 };
 
 /* Flags for BPF_PROG_QUERY. */
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index e5fd31268ae02..644ceb67350a6 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -15,7 +15,8 @@ 
 #include "map_in_map.h"
 
 #define ARRAY_CREATE_FLAG_MASK \
-	(BPF_F_NUMA_NODE | BPF_F_MMAPABLE | BPF_F_ACCESS_MASK)
+	(BPF_F_NUMA_NODE | BPF_F_MMAPABLE | BPF_F_ACCESS_MASK | \
+	 BPF_F_PRESERVE_ELEMS)
 
 static void bpf_array_free_percpu(struct bpf_array *array)
 {
@@ -64,6 +65,10 @@  int array_map_alloc_check(union bpf_attr *attr)
 	    attr->map_flags & BPF_F_MMAPABLE)
 		return -EINVAL;
 
+	if (attr->map_type != BPF_MAP_TYPE_PERF_EVENT_ARRAY &&
+	    attr->map_flags & BPF_F_PRESERVE_ELEMS)
+		return -EINVAL;
+
 	if (attr->value_size > KMALLOC_MAX_SIZE)
 		/* if value_size is bigger, the user space won't be able to
 		 * access the elements.
@@ -778,6 +783,15 @@  static int fd_array_map_delete_elem(struct bpf_map *map, void *key)
 	}
 }
 
+static void bpf_fd_array_map_clear(struct bpf_map *map);
+
+static void perf_event_fd_array_map_free(struct bpf_map *map)
+{
+	if (map->map_flags & BPF_F_PRESERVE_ELEMS)
+		bpf_fd_array_map_clear(map);
+	fd_array_map_free(map);
+}
+
 static void *prog_fd_array_get_ptr(struct bpf_map *map,
 				   struct file *map_file, int fd)
 {
@@ -1134,6 +1148,9 @@  static void perf_event_fd_array_release(struct bpf_map *map,
 	struct bpf_event_entry *ee;
 	int i;
 
+	if (map->map_flags & BPF_F_PRESERVE_ELEMS)
+		return;
+
 	rcu_read_lock();
 	for (i = 0; i < array->map.max_entries; i++) {
 		ee = READ_ONCE(array->ptrs[i]);
@@ -1148,7 +1165,7 @@  const struct bpf_map_ops perf_event_array_map_ops = {
 	.map_meta_equal = bpf_map_meta_equal,
 	.map_alloc_check = fd_array_map_alloc_check,
 	.map_alloc = array_map_alloc,
-	.map_free = fd_array_map_free,
+	.map_free = perf_event_fd_array_map_free,
 	.map_get_next_key = array_map_get_next_key,
 	.map_lookup_elem = fd_array_map_lookup_elem,
 	.map_delete_elem = fd_array_map_delete_elem,
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 82522f05c0213..ea78eb89f8d67 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -414,6 +414,9 @@  enum {
 
 /* Enable memory-mapping BPF map */
 	BPF_F_MMAPABLE		= (1U << 10),
+
+/* Share perf_event among processes */
+	BPF_F_PRESERVE_ELEMS	= (1U << 11),
 };
 
 /* Flags for BPF_PROG_QUERY. */