Re: [PATCH bpf-next v5 2/9] bpf: Introduce internal bpf_map_check_op_flags helper function

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

On 9/9/25 01:36, Alexei Starovoitov wrote:
> On Mon, Sep 8, 2025 at 7:37 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>>
>> It is to unify map flags checking for lookup_elem, update_elem,
>> lookup_batch and update_batch APIs.
>>
>> Therefore, it will be convenient to check BPF_F_CPU and BPF_F_ALL_CPUS
>> flags in it for these APIs in next patch.
>>
>> Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
>> ---
>>  include/linux/bpf.h  | 31 +++++++++++++++++++++++++++++++
>>  kernel/bpf/syscall.c | 34 +++++++++++-----------------------
>>  2 files changed, 42 insertions(+), 23 deletions(-)
>>
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index ce523a49dc20c..55c98c7d52510 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -3735,4 +3735,35 @@ int bpf_prog_get_file_line(struct bpf_prog *prog, unsigned long ip, const char *
>>                            const char **linep, int *nump);
>>  struct bpf_prog *bpf_prog_find_from_stack(void);
>>
>> +static inline int bpf_map_check_op_flags(struct bpf_map *map, u64 flags, u64 allowed_flags)
>> +{
>> +       if (flags & ~allowed_flags)
>> +               return -EINVAL;
>> +
>> +       if ((flags & BPF_F_LOCK) && !btf_record_has_field(map->record, BPF_SPIN_LOCK))
>> +               return -EINVAL;
>> +
>> +       return 0;
>> +}
>> +
>> +static inline int bpf_map_check_lookup_flags(struct bpf_map *map, u64 flags)
>> +{
>> +       return bpf_map_check_op_flags(map, flags, BPF_F_LOCK);
>> +}
>> +
>> +static inline int bpf_map_check_update_flags(struct bpf_map *map, u64 flags)
>> +{
>> +       return bpf_map_check_op_flags(map, flags, ~0);
>> +}
>> +
>> +static inline int bpf_map_check_lookup_batch_flags(struct bpf_map *map, u64 flags)
>> +{
>> +       return bpf_map_check_lookup_flags(map, flags);
>> +}
>> +
>> +static inline int bpf_map_check_update_batch_flags(struct bpf_map *map, u64 flags)
>> +{
>> +       return bpf_map_check_op_flags(map, flags, BPF_F_LOCK);
>> +}
> 
> I don't like these pointless wrappers.
> They make the code less readable.

Thanks for the feedback.

My intent was to keep the helpers close in style to
bpf_map_check_op_flags(), so that lookup/update (single or batch) would
follow a consistent pattern. This way it’s easier to see the relation
between map ops and their corresponding flag checks.

That said, I understand your point about readability. I will drop these
wrappers and just call bpf_map_check_op_flags() directly at each site.

Thanks,
Leon