Re: [PATCH bpf-next v3 1/2] bpf: Support struct btf_struct_meta via KF_IMPLICIT_ARGS

[Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>]

Ihor Solodrai <ihor.solodrai@linux.dev> writes:

> On 3/20/26 8:49 AM, Mykyta Yatsenko wrote:
>> Ihor Solodrai <ihor.solodrai@linux.dev> writes:
>> 
>>> [...]
>>> +/*
>>> + * A kfunc with KF_IMPLICIT_ARGS has two prototypes in BTF:
>>> + *   - the _impl prototype with full arg list (this is meta->func_proto)
>>> + *   - the BPF API prototype w/o implicit args (func->type in BTF)
>>> + * To determine whether an argument is implicit, we compare its position
>>> + * against the number of arguments of both prototypes.
>>> + */
>>> +static bool is_kfunc_arg_implicit(const struct bpf_kfunc_call_arg_meta *meta, u32 arg_idx)
>>> +{
>>> +	const struct btf_type *func, *func_proto;
>>> +	u32 argn, full_argn;
>>> +
>>> +	if (!(meta->kfunc_flags & KF_IMPLICIT_ARGS))
>>> +		return false;
>>> +
>>> +	full_argn = btf_type_vlen(meta->func_proto);
>>> +
>>> +	func = btf_type_by_id(meta->btf, meta->func_id);
>>> +	func_proto = btf_type_by_id(meta->btf, func->type);
>>> +	argn = btf_type_vlen(func_proto);
>>> +
>>> +	return argn <= arg_idx && arg_idx < full_argn;
>> The `arg_idx < full_argn` condition is not necessary, is it?
>> arg_idx is always less than full_argn because full_argn is the number of
>> arguments in the _impl variant of the function, which is supposed to be
>> greater than non-_impl variant that arg_idx tracks.
>> arg_idx >= full_argn is an invariant violation, not the implicit
>> argument condition, if I understand this right.
>
> Hi Mykyta, thanks for the review.
>
> Yes, I think you're right.
>
> I don't think this helper should error out on arg_idx >= full_argn,
> since that would be an error caught in check_kfunc_call(). And returning
> an error will make the helper a little more complicated.
>
> At the same time, it seems logical to me that the answer to
> "is arg 6 implicit for kfunc with 5 args in _impl proto?" 
> should be false.
>
> Do you have a specific suggestion?
>
Sorry for the delayed reply, just noticed it. I would drop the check, as
you mentioned it should be caught earlier, so it's there just as
a defensive programming artifact. Argument index is never greater than
the number of arguments in the function.
>> 
>> The rest of the refactoring looks good to me.
>>> +}
>>> +
>>>  [...]
>>> -- 
>>> 2.53.0