Hi,
On 2/17/2025 9:41 AM, Xiaomeng Zhang wrote:
> Remove redundant map_get_next_key callbacks with return -EOPNOTSUPP. We can
> directly return -EOPNOTSUPP when calling the unimplemented callbacks.
>
> Signed-off-by: Xiaomeng Zhang <zhangxiaomeng13@xxxxxxxxxx>
> ---
> kernel/bpf/arena.c | 6 ------
> kernel/bpf/bloom_filter.c | 6 ------
> kernel/bpf/bpf_cgrp_storage.c | 6 ------
> kernel/bpf/bpf_inode_storage.c | 7 -------
> kernel/bpf/bpf_task_storage.c | 6 ------
> kernel/bpf/ringbuf.c | 8 --------
> kernel/bpf/syscall.c | 10 ++++++++--
> 7 files changed, 8 insertions(+), 41 deletions(-)
>
> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
> index 50f07bbd33fa..7f37ef1b72ce 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;
> }
>
SNIP
> diff --git a/kernel/bpf/ringbuf.c b/kernel/bpf/ringbuf.c
> index e2d22f10a11f..932a6c06e3e0 100644
> --- a/kernel/bpf/ringbuf.c
> +++ b/kernel/bpf/ringbuf.c
> @@ -247,12 +247,6 @@ static long ringbuf_map_update_elem(struct bpf_map *map, void *key, void *value,
> return -ENOTSUPP;
> }
>
> -static int ringbuf_map_get_next_key(struct bpf_map *map, void *key,
> - void *next_key)
> -{
> - return -ENOTSUPP;
> -}
> -
> static int ringbuf_map_mmap_kern(struct bpf_map *map, struct vm_area_struct *vma)
> {
> struct bpf_ringbuf_map *rb_map;
> @@ -351,7 +345,6 @@ const struct bpf_map_ops ringbuf_map_ops = {
> .map_poll = ringbuf_map_poll_kern,
> .map_lookup_elem = ringbuf_map_lookup_elem,
> .map_update_elem = ringbuf_map_update_elem,
> - .map_get_next_key = ringbuf_map_get_next_key,
> .map_mem_usage = ringbuf_map_mem_usage,
> .map_btf_id = &ringbuf_map_btf_ids[0],
> };
> @@ -365,7 +358,6 @@ const struct bpf_map_ops user_ringbuf_map_ops = {
> .map_poll = ringbuf_map_poll_user,
> .map_lookup_elem = ringbuf_map_lookup_elem,
> .map_update_elem = ringbuf_map_update_elem,
> - .map_get_next_key = ringbuf_map_get_next_key,
> .map_mem_usage = ringbuf_map_mem_usage,
> .map_btf_id = &user_ringbuf_map_btf_ids[0],
> };
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 36eed7016da4..087abbacbd05 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1850,7 +1850,10 @@ static int map_get_next_key(union bpf_attr *attr)
> }
>
> rcu_read_lock();
> - err = map->ops->map_get_next_key(map, key, next_key);
> + if (map->ops->map_get_next_key)
> + err = map->ops->map_get_next_key(map, key, next_key);
> + else
> + err = -EOPNOTSUPP;
The old return value is ENOTSUPP. For consistency, I think it is better
to still return ENOTSUPP.
> rcu_read_unlock();
> out:
> if (err)
> @@ -2037,7 +2040,10 @@ int generic_map_lookup_batch(struct bpf_map *map,
>
> for (cp = 0; cp < max_count;) {
> rcu_read_lock();
> - err = map->ops->map_get_next_key(map, prev_key, key);
> + if (map->ops->map_get_next_key)
> + err = map->ops->map_get_next_key(map, prev_key, key);
> + else
> + err = -EOPNOTSUPP;
> rcu_read_unlock();
How about only check ->map_get_next_key() once instead of checking it
every time ? And there are another two invocations of
->map_get_next_key(), please update these invocations as well.
> if (err)
> break;
