[-next,1/5] bpf: Remove map_peek_elem callbacks with -EOPNOTSUPP

Commit Message

Xiaomeng Zhang Feb. 17, 2025, 1:41 a.m. UTC

Remove redundant map_peek_elem callbacks with return -EOPNOTSUPP. We can
directly return -EOPNOTSUPP when calling the unimplemented callbacks.

Signed-off-by: Xiaomeng Zhang <[e-mail]>
---
 kernel/bpf/arena.c   | 6 ------
 kernel/bpf/helpers.c | 5 ++++-
 kernel/bpf/syscall.c | 5 ++++-
 3 files changed, 8 insertions(+), 8 deletions(-)

Comments

Hou Tao Feb. 17, 2025, 2:25 a.m. UTC | #1

Hi,

On 2/17/2025 9:41 AM, Xiaomeng Zhang wrote:
> Remove redundant map_peek_elem callbacks with return -EOPNOTSUPP. We can
> directly return -EOPNOTSUPP when calling the unimplemented callbacks.
>
> Signed-off-by: Xiaomeng Zhang <[e-mail]>
> ---
>  kernel/bpf/arena.c   | 6 ------
>  kernel/bpf/helpers.c | 5 ++++-
>  kernel/bpf/syscall.c | 5 ++++-
>  3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
> index 095a9554e1de..0aaefa5d6b09 100644
> --- a/kernel/bpf/arena.c
> +++ b/kernel/bpf/arena.c
> @@ -62,11 +62,6 @@ u64 bpf_arena_get_user_vm_start(struct bpf_arena *arena)
>  	return arena ? arena->user_vm_start : 0;
>  }
>  
> -static long arena_map_peek_elem(struct bpf_map *map, void *value)
> -{
> -	return -EOPNOTSUPP;
> -}
> -
>  static long arena_map_push_elem(struct bpf_map *map, void *value, u64 flags)
>  {
>  	return -EOPNOTSUPP;
> @@ -404,7 +399,6 @@ const struct bpf_map_ops arena_map_ops = {
>  	.map_get_unmapped_area = arena_get_unmapped_area,
>  	.map_get_next_key = arena_map_get_next_key,
>  	.map_push_elem = arena_map_push_elem,
> -	.map_peek_elem = arena_map_peek_elem,
>  	.map_pop_elem = arena_map_pop_elem,
>  	.map_lookup_elem = arena_map_lookup_elem,
>  	.map_update_elem = arena_map_update_elem,
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index f27ce162427a..0f429171de6d 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -116,7 +116,10 @@ const struct bpf_func_proto bpf_map_pop_elem_proto = {
>  
>  BPF_CALL_2(bpf_map_peek_elem, struct bpf_map *, map, void *, value)
>  {
> -	return map->ops->map_peek_elem(map, value);
> +	if (map->ops->map_peek_elem)
> +		return map->ops->map_peek_elem(map, value);
> +	else
> +		return -EOPNOTSUPP;
>  }

It is unnecessary. Because the verifier has ensured the
bpf_map_peek_elem helper is only available for specific maps in
check_map_func_compatibility().
>  
>  const struct bpf_func_proto bpf_map_peek_elem_proto = {
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index ff1f980bd59a..e6e859f71c5d 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -325,7 +325,10 @@ static int bpf_map_copy_value(struct bpf_map *map, void *key, void *value,
>  	} else if (map->map_type == BPF_MAP_TYPE_QUEUE ||
>  		   map->map_type == BPF_MAP_TYPE_STACK ||
>  		   map->map_type == BPF_MAP_TYPE_BLOOM_FILTER) {
> -		err = map->ops->map_peek_elem(map, value);
> +		if (map->ops->map_peek_elem)
> +			err = map->ops->map_peek_elem(map, value);
> +		else
> +			err = -EOPNOTSUPP;

It is also unnecessary due to the similar reason.
>  	} else if (map->map_type == BPF_MAP_TYPE_STRUCT_OPS) {
>  		/* struct_ops map requires directly updating "value" */
>  		err = bpf_struct_ops_map_sys_lookup_elem(map, key, value);

Patch

diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
index 095a9554e1de..0aaefa5d6b09 100644
--- a/kernel/bpf/arena.c
+++ b/kernel/bpf/arena.c
@@ -62,11 +62,6 @@  u64 bpf_arena_get_user_vm_start(struct bpf_arena *arena)
 	return arena ? arena->user_vm_start : 0;
 }
 
-static long arena_map_peek_elem(struct bpf_map *map, void *value)
-{
-	return -EOPNOTSUPP;
-}
-
 static long arena_map_push_elem(struct bpf_map *map, void *value, u64 flags)
 {
 	return -EOPNOTSUPP;
@@ -404,7 +399,6 @@  const struct bpf_map_ops arena_map_ops = {
 	.map_get_unmapped_area = arena_get_unmapped_area,
 	.map_get_next_key = arena_map_get_next_key,
 	.map_push_elem = arena_map_push_elem,
-	.map_peek_elem = arena_map_peek_elem,
 	.map_pop_elem = arena_map_pop_elem,
 	.map_lookup_elem = arena_map_lookup_elem,
 	.map_update_elem = arena_map_update_elem,
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index f27ce162427a..0f429171de6d 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -116,7 +116,10 @@  const struct bpf_func_proto bpf_map_pop_elem_proto = {
 
 BPF_CALL_2(bpf_map_peek_elem, struct bpf_map *, map, void *, value)
 {
-	return map->ops->map_peek_elem(map, value);
+	if (map->ops->map_peek_elem)
+		return map->ops->map_peek_elem(map, value);
+	else
+		return -EOPNOTSUPP;
 }
 
 const struct bpf_func_proto bpf_map_peek_elem_proto = {
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index ff1f980bd59a..e6e859f71c5d 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -325,7 +325,10 @@  static int bpf_map_copy_value(struct bpf_map *map, void *key, void *value,
 	} else if (map->map_type == BPF_MAP_TYPE_QUEUE ||
 		   map->map_type == BPF_MAP_TYPE_STACK ||
 		   map->map_type == BPF_MAP_TYPE_BLOOM_FILTER) {
-		err = map->ops->map_peek_elem(map, value);
+		if (map->ops->map_peek_elem)
+			err = map->ops->map_peek_elem(map, value);
+		else
+			err = -EOPNOTSUPP;
 	} else if (map->map_type == BPF_MAP_TYPE_STRUCT_OPS) {
 		/* struct_ops map requires directly updating "value" */
 		err = bpf_struct_ops_map_sys_lookup_elem(map, key, value);