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

[Ilya Leoshkevich <iii@linux.ibm.com>]

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.
 
> >   #define TEST_TAG_EXPECT_FAILURE "comment:test_expect_failure"
> >   #define TEST_TAG_EXPECT_SUCCESS "comment:test_expect_success"