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

[John Fastabend <john.fastabend@gmail.com>]

Yonghong Song wrote:
> 
> 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>

LGTM.

Acked-by: John Fastabend <john.fastabend@gmail.com>