Re: [RFC PATCH bpf-next 5/6] bpf: Allow pro/epilogue to call kfunc

[Martin KaFai Lau <martin.lau@linux.dev>]

On 8/14/24 3:17 PM, Eduard Zingerman wrote:
> On Tue, 2024-08-13 at 11:49 -0700, Martin KaFai Lau wrote:
>> From: Martin KaFai Lau <martin.lau@kernel.org>
>>
>> The existing prologue has been able to call bpf helper but not a kfunc.
>> This patch allows the prologue/epilogue to call the kfunc.
> 
> [...]
> 
>> Once the insn->off is determined (either reuse an existing one
>> or an unused one is found), it will call the existing add_kfunc_call()
>> and everything else should fall through.
>>
>> Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
>> ---
> 
> fwiw, don't see any obvious problems with this patch.
> 
> Reviewed-by: Eduard Zingerman <eddyz87@gmail.com>
> 
>>   kernel/bpf/verifier.c | 116 ++++++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 113 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 5e995b7884fb..2873e1083402 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -2787,6 +2787,61 @@ static struct btf *find_kfunc_desc_btf(struct bpf_verifier_env *env, s16 offset)
>>   	return btf_vmlinux ?: ERR_PTR(-ENOENT);
>>   }
>>   
>> +static int find_kfunc_desc_btf_offset(struct bpf_verifier_env *env, struct btf *btf,
>> +				      struct module *module, s16 *offset)
>> +{
>> +	struct bpf_kfunc_btf_tab *tab;
>> +	struct bpf_kfunc_btf *b;
>> +	s16 new_offset = S16_MAX;
>> +	u32 i;
>> +
>> +	if (btf_is_vmlinux(btf)) {
>> +		*offset = 0;
>> +		return 0;
>> +	}
>> +
>> +	tab = env->prog->aux->kfunc_btf_tab;
>> +	if (!tab) {
>> +		tab = kzalloc(sizeof(*tab), GFP_KERNEL);
>> +		if (!tab)
>> +			return -ENOMEM;
>> +		env->prog->aux->kfunc_btf_tab = tab;
>> +	}
>> +
>> +	b = tab->descs;
>> +	for (i = tab->nr_descs; i > 0; i--) {
> 
> Question: why iterating in reverse here?

Agreed. It is unnecessary. I will change it to iterate forward in the next re-spin.

Thanks for the review!