Re: [PATCH bpf-next v7 3/7] bpf: Add BPF_F_CPU and BPF_F_ALL_CPUS flags support for percpu_array maps

["Leon Hwang" <leon.hwang@linux.dev>]

On Wed Sep 17, 2025 at 7:44 AM +08, Andrii Nakryiko wrote:
> On Wed, Sep 10, 2025 at 9:28 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>>
>> Introduce support for the BPF_F_ALL_CPUS flag in percpu_array maps to
>> allow updating values for all CPUs with a single value for both
>> update_elem and update_batch APIs.
>>
>> Introduce support for the BPF_F_CPU flag in percpu_array maps to allow:
>>
>> * update value for specified CPU for both update_elem and update_batch
>> APIs.
>> * lookup value for specified CPU for both lookup_elem and lookup_batch
>> APIs.
>>
>> The BPF_F_CPU flag is passed via:
>>
>> * map_flags of lookup_elem and update_elem APIs along with embedded cpu
>> info.
>> * elem_flags of lookup_batch and update_batch APIs along with embedded
>> cpu info.
>>
>> Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
>> ---
>>  include/linux/bpf.h   |  9 +++++++--
>>  kernel/bpf/arraymap.c | 24 +++++++++++++++++++++---
>>  kernel/bpf/syscall.c  |  2 +-
>>  3 files changed, 29 insertions(+), 6 deletions(-)
>>
>
> [...]
>
>>
>> -int bpf_percpu_array_copy(struct bpf_map *map, void *key, void *value)
>> +int bpf_percpu_array_copy(struct bpf_map *map, void *key, void *value, u64 map_flags)
>>  {
>>         struct bpf_array *array = container_of(map, struct bpf_array, map);
>>         u32 index = *(u32 *)key;
>> @@ -313,11 +313,18 @@ int bpf_percpu_array_copy(struct bpf_map *map, void *key, void *value)
>>         size = array->elem_size;
>>         rcu_read_lock();
>>         pptr = array->pptrs[index & array->index_mask];
>> +       if (map_flags & BPF_F_CPU) {
>> +               cpu = map_flags >> 32;
>> +               copy_map_value_long(map, value, per_cpu_ptr(pptr, cpu));
>> +               check_and_init_map_value(map, value);
>> +               goto unlock;
>
> goto is not how I'd structure this logic, I think if/else is a more
> logical structure here, but this works, I suppose...
>

My intention is to avoid putting the existing code inside a new 'else'
block, even if it would only affect indentation.

This way, the original code block stays intact, and git-blame will still
point to the commit that introduced it.

>> +       }
>>         for_each_possible_cpu(cpu) {
>>                 copy_map_value_long(map, value + off, per_cpu_ptr(pptr, cpu));
>>                 check_and_init_map_value(map, value + off);
>>                 off += size;
>>         }
>> +unlock:
>>         rcu_read_unlock();
>>         return 0;
>>  }
>> @@ -390,7 +397,7 @@ int bpf_percpu_array_update(struct bpf_map *map, void *key, void *value,
>>         int cpu, off = 0;
>>         u32 size;
>>
>> -       if (unlikely(map_flags > BPF_EXIST))
>> +       if (unlikely((u32)map_flags > BPF_F_ALL_CPUS))
>
> this will let through BPF_F_LOCK, no? which is not what you intended,
> right? So you need to check for
>
> (map_flags & BPF_F_LOCK) || (u32)map_flags > BPF_F_ALL_CPUS
>

Right.

I'll update it in next revision.

Thanks,
Leon