On Wed, Aug 27, 2025 at 9:45 AM Leon Hwang <leon.hwang@xxxxxxxxx> wrote:
>
> Introduce BPF_F_CPU and BPF_F_ALL_CPUS flags and the following internal
> helper functions for percpu maps:
>
> * bpf_percpu_copy_to_user: For lookup_elem and lookup_batch user APIs,
> copy data to user-provided value pointer.
> * bpf_percpu_copy_from_user: For update_elem and update_batch user APIs,
> copy data from user-provided value pointer.
> * bpf_map_check_cpu_flags: Check BPF_F_CPU, BPF_F_ALL_CPUS and cpu info in
> flags.
>
> And, get the correct value size for these user APIs.
>
> Signed-off-by: Leon Hwang <leon.hwang@xxxxxxxxx>
> ---
> include/linux/bpf.h | 89 ++++++++++++++++++++++++++++++++--
> include/uapi/linux/bpf.h | 2 +
> kernel/bpf/syscall.c | 24 ++++-----
> tools/include/uapi/linux/bpf.h | 2 +
> 4 files changed, 103 insertions(+), 14 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 512717d442c09..a83364949b64c 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -547,6 +547,56 @@ static inline void copy_map_value_long(struct bpf_map *map, void *dst, void *src
> bpf_obj_memcpy(map->record, dst, src, map->value_size, true);
> }
>
> +#ifdef CONFIG_BPF_SYSCALL
> +static inline void bpf_percpu_copy_to_user(struct bpf_map *map, void __percpu *pptr, void *value,
> + u32 size, u64 flags)
> +{
> + int current_cpu = raw_smp_processor_id();
> + int cpu, off = 0;
> +
> + if (flags & BPF_F_CPU) {
> + cpu = flags >> 32;
> + copy_map_value_long(map, value, cpu != current_cpu ? per_cpu_ptr(pptr, cpu) :
> + this_cpu_ptr(pptr));
> + check_and_init_map_value(map, value);
I'm not sure it's the question to you, but why would we
"check_and_init_map_value" when copying data to user space?... this is
so confusing...
> + } else {
> + 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;
> + }
> + }
> +}
> +
> +void bpf_obj_free_fields(const struct btf_record *rec, void *obj);
> +
> +static inline void bpf_percpu_copy_from_user(struct bpf_map *map, void __percpu *pptr, void *value,
> + u32 size, u64 flags)
> +{
> + int current_cpu = raw_smp_processor_id();
> + int cpu, off = 0;
> + void *ptr;
> +
> + if (flags & BPF_F_CPU) {
> + cpu = flags >> 32;
> + ptr = cpu == current_cpu ? this_cpu_ptr(pptr) : per_cpu_ptr(pptr, cpu);
> + copy_map_value_long(map, ptr, value);
> + bpf_obj_free_fields(map->record, ptr);
> + } else {
> + for_each_possible_cpu(cpu) {
> + copy_map_value_long(map, per_cpu_ptr(pptr, cpu), value + off);
> + /* same user-provided value is used if
> + * BPF_F_ALL_CPUS is specified, otherwise value is
> + * an array of per-cpu values.
> + */
> + if (!(flags & BPF_F_ALL_CPUS))
> + off += size;
> + bpf_obj_free_fields(map->record, per_cpu_ptr(pptr, cpu));
> + }
> + }
> +}
> +#endif
hm... these helpers are just here with no way to validate that they
generalize existing logic correctly... Do a separate patch where you
introduce this helper before adding per-CPU flags *and* make use of
them in existing code? Then we can check that you didn't introduce any
subtle differences? Then in this patch you can adjust helpers to
handle BPF_F_CPU and BPF_F_ALL_CPUS?
pw-bot: cr
[...]
