Re: [PATCH bpf-next v4 1/7] bpf: Introduce internal bpf_map_check_op_flags helper function

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

On Thu Aug 28, 2025 at 7:17 AM +08, Andrii Nakryiko wrote:
> On Wed, Aug 27, 2025 at 9:45 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  | 28 ++++++++++++++++++++++++++++
>>  kernel/bpf/syscall.c | 34 +++++++++++-----------------------
>>  2 files changed, 39 insertions(+), 23 deletions(-)
>>
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index 8f6e87f0f3a89..512717d442c09 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -3709,4 +3709,32 @@ 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 extra_flags_mask)
>> +{
>> +       if (extra_flags_mask && (flags & extra_flags_mask))
>
> doh, Leon... when extra_flags_mask == 0, `flags & extra_flags_mask` is
> always false, so just:
>
> if (flags & extra_flags_mask)
>     return -EINVAL;
>
> But it feels more natural to reverse the meaning of this and treat it
> as extra *allowed flags*. So zero would mean no extra flags should be
> there (most strict case) and ~0 would mean "we don't care or will
> check later". And so in the code you'd have
>
> if (flags & ~extra_flags) /* check for any unsupported flags */
>     return -EINVAL;
>
> But I need someone else to do a reality check on me here at this point.
>

It seems clearer to handle this as additional *allowed flags*. That would
make it more understandable.

Thanks,
Leon