Re: [PATCH v4 bpf-next 1/9] bpf: Move insn_buf[16] to bpf_verifier_env

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

On 8/29/24 8:26 AM, Alexei Starovoitov wrote:
> On Thu, Aug 29, 2024 at 8:20 AM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>>
>> On 8/28/24 6:46 PM, Alexei Starovoitov wrote:
>>> On Wed, Aug 28, 2024 at 5:41 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>>>>
>>>> On Tue, 2024-08-27 at 12:48 -0700, Martin KaFai Lau wrote:
>>>>> From: Martin KaFai Lau <martin.lau@kernel.org>
>>>>>
>>>>> This patch moves the 'struct bpf_insn insn_buf[16]' stack usage
>>>>> to the bpf_verifier_env. A '#define INSN_BUF_SIZE 16' is also added
>>>>> to replace the ARRAY_SIZE(insn_buf) usages.
>>>>>
>>>>> Both convert_ctx_accesses() and do_misc_fixup() are changed
>>>>> to use the env->insn_buf.
>>>>>
>>>>> It is a prep work for adding the epilogue_buf[16] in a later patch.
>>>>>
>>>>> Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
>>>>> ---
>>>>
>>>> Not sure if this refactoring is worth it but code looks correct.
>>>> Note that there is also inline_bpf_loop()
>>>> (it needs a slightly bigger buffer).
>>>
>>> Probably worth it in the follow up, since people complain that

I read it and Eduard's earlier comment together as the whole patch 1 as a follow 
up. :)

yep. I will keep this one and follow up with the insn_buf in the inline_bpf_loop().

I will update the commit message with the stack usage in the next respin.

>>> this or that function in verifier.c reaches stack size limit
>>> when compiled with sanitizers.
>>> These buffers on stack are the biggest consumers.
>>
>> ok. I will drop this patch for now. Redo it again as a followup and will
>> consider inline_bpf_loop() together at that time.
> 
> why? Keep it. It's an improvement already.