Re: [PATCH bpf 2/3] selftests/bpf: Double the size of test_loader log

[Yonghong Song <yonghong.song@linux.dev>]

On 1/2/24 11:05 PM, Ilya Leoshkevich wrote:
> On Tue, 2024-01-02 at 16:41 -0800, Yonghong Song wrote:
>> On 1/2/24 11:30 AM, Ilya Leoshkevich wrote:
>>> Testing long jumps requires having >32k instructions. That many
>>> instructions require the verifier log buffer of 2 megabytes.
>>>
>>> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>>> ---
>>>    tools/testing/selftests/bpf/test_loader.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tools/testing/selftests/bpf/test_loader.c
>>> b/tools/testing/selftests/bpf/test_loader.c
>>> index 37ffa57f28a1..b0bfcc8d4638 100644
>>> --- a/tools/testing/selftests/bpf/test_loader.c
>>> +++ b/tools/testing/selftests/bpf/test_loader.c
>>> @@ -12,7 +12,7 @@
>>>    #define str_has_pfx(str, pfx) \
>>>          (strncmp(str, pfx, __builtin_constant_p(pfx) ? sizeof(pfx)
>>> - 1 : strlen(pfx)) == 0)
>>>    
>>> -#define TEST_LOADER_LOG_BUF_SZ 1048576
>>> +#define TEST_LOADER_LOG_BUF_SZ 2097152
>> I think this patch is not necessary.
>> If the log buffer size is not enough, the kernel
>> verifier will wrap around and overwrite some initial states,
>> but all later states are still preserved. In my opinion,
>> there is really no need to increase the buffer size in this case,
>> esp. it is a verification success case.
> What I observed in this case was that bpf_check() still returned
> -ENOSPC and failed the prog load. IIUC you are referring to the
> functionality introduced by the following commit:
>
> commit 1216640938035e63bdbd32438e91c9bcc1fd8ee1
> Author: Andrii Nakryiko <andrii@kernel.org>
> Date:   Thu Apr 6 16:41:49 2023 -0700
>
>      bpf: Switch BPF verifier log to be a rotating log by default
>
> The commit message says, among other things:
>
>      The only user-visible change is which portion of verifier log user
>      ends up seeing *if buffer is too small*.
>
> So if we don't increase the log size, we would still have to deal with
> -ENOSPC. An alternative would be to reallocate the log buffer and try
> again. But I thought that for the test code we better keep it as simple
> as possible.

Okay, thanks for the explanation. I applied the patch set to
my local env and indeed, with this patch, I can see libbpf returns
an error. So as you suggested, let us increase the buffer size to
avoid extra handling in test_progs. So

Acked-by: Yonghong Song <yonghong.song@linux.dev>

>   
>>>    #define TEST_TAG_EXPECT_FAILURE "comment:test_expect_failure"
>>>    #define TEST_TAG_EXPECT_SUCCESS "comment:test_expect_success"