Re: [PATCH bpf-next v11 09/13] bpf: validate value_type

[Kui-Feng Lee <sinquersw@gmail.com>]

On 11/9/23 18:11, Martin KaFai Lau wrote:
> On 11/6/23 12:12 PM, thinker.li@gmail.com wrote:
>> From: Kui-Feng Lee <thinker.li@gmail.com>
>>
>> A value_type should consist of three components: refcnt, state, and data.
>> refcnt and state has been move to struct bpf_struct_ops_common_value to
>> make it easier to check the value type.
>>
>> Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
>> ---
>>   include/linux/bpf.h         | 14 ++++++
>>   kernel/bpf/bpf_struct_ops.c | 93 ++++++++++++++++++++++++-------------
>>   2 files changed, 74 insertions(+), 33 deletions(-)
>>
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index c287f42b2e48..48e97a255945 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -3231,4 +3231,18 @@ static inline bool bpf_is_subprog(const struct 
>> bpf_prog *prog)
>>       return prog->aux->func_idx != 0;
>>   }
>> +#ifdef CONFIG_BPF_JIT
> 
> There is an existing "#if defined(CONFIG_BPF_JIT) && 
> defined(CONFIG_BPF_SYSCALL)" above and a few bpf_struct_ops_*() has 
> already been there. Does it need another separate one which is only 
> CONFIG_BPF_JIT here?
> 
>> +enum bpf_struct_ops_state {
>> +    BPF_STRUCT_OPS_STATE_INIT,
>> +    BPF_STRUCT_OPS_STATE_INUSE,
>> +    BPF_STRUCT_OPS_STATE_TOBEFREE,
>> +    BPF_STRUCT_OPS_STATE_READY,
>> +};
>> +
>> +struct bpf_struct_ops_common_value {
>> +    refcount_t refcnt;
>> +    enum bpf_struct_ops_state state;
>> +};
> 
> Do the struct and enum really need to be in ifdef?
> 
>> +#endif /* CONFIG_BPF_JIT */
>> +
> 
I just removed this pair of #if-else.
You are right! They are not necessary.