Re: [bpf-next v3 01/12] selftests/bpf: Print a message when tester could not run a program

From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Krzesimir Nowak <krzesimir@kinvolk.io>
Cc: "open list" <linux-kernel@vger.kernel.org>,
	"Alban Crequy" <alban@kinvolk.io>,
	"Iago López Galeiras" <iago@kinvolk.io>,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Martin KaFai Lau" <kafai@fb.com>,
	"Song Liu" <songliubraving@fb.com>, "Yonghong Song" <yhs@fb.com>,
	"David S. Miller" <davem@davemloft.net>,
	"Jakub Kicinski" <jakub.kicinski@netronome.com>,
	"Jesper Dangaard Brouer" <hawk@kernel.org>,
	"John Fastabend" <john.fastabend@gmail.com>,
	"Stanislav Fomichev" <sdf@google.com>,
	Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
	xdp-newbies@vger.kernel.org
Subject: Re: [bpf-next v3 01/12] selftests/bpf: Print a message when tester could not run a program
Date: Thu, 11 Jul 2019 17:10:16 -0700	[thread overview]
Message-ID: <CAEf4BzZBzreJOvEzm-OzVPNfwHOhuQ4nh4P04Nh5-u24sjFEcA@mail.gmail.com> (raw)
In-Reply-To: <CAGGp+cEaGphDCuZL+sbo2aCVumk2jrq9_Lshifg-Ewphfm40Wg@mail.gmail.com>

On Thu, Jul 11, 2019 at 4:36 AM Krzesimir Nowak <krzesimir@kinvolk.io> wrote:
>
> On Thu, Jul 11, 2019 at 1:45 AM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Mon, Jul 8, 2019 at 3:42 PM Krzesimir Nowak <krzesimir@kinvolk.io> wrote:
> > >
> > > This prints a message when the error is about program type being not
> > > supported by the test runner or because of permissions problem. This
> > > is to see if the program we expected to run was actually executed.
> > >
> > > The messages are open-coded because strerror(ENOTSUPP) returns
> > > "Unknown error 524".
> > >
> > > Changes since v2:
> > > - Also print "FAIL" on an unexpected bpf_prog_test_run error, so there
> > >   is a corresponding "FAIL" message for each failed test.
> > >
> > > Signed-off-by: Krzesimir Nowak <krzesimir@kinvolk.io>
> > > ---
> > >  tools/testing/selftests/bpf/test_verifier.c | 17 +++++++++++++----
> > >  1 file changed, 13 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
> > > index c5514daf8865..b8d065623ead 100644
> > > --- a/tools/testing/selftests/bpf/test_verifier.c
> > > +++ b/tools/testing/selftests/bpf/test_verifier.c
> > > @@ -831,11 +831,20 @@ static int do_prog_test_run(int fd_prog, bool unpriv, uint32_t expected_val,
> > >                                 tmp, &size_tmp, &retval, NULL);
> > >         if (unpriv)
> > >                 set_admin(false);
> > > -       if (err && errno != 524/*ENOTSUPP*/ && errno != EPERM) {
> > > -               printf("Unexpected bpf_prog_test_run error ");
> > > -               return err;
> > > +       if (err) {
> > > +               switch (errno) {
> > > +               case 524/*ENOTSUPP*/:
> > > +                       printf("Did not run the program (not supported) ");
> > > +                       return 0;
> > > +               case EPERM:
> > > +                       printf("Did not run the program (no permission) ");
> >
> > Let's add "SKIP: " prefix to these?
>
> Not sure about it. The important part of the test (the program being
> verified by the kernel's verifier) was still executed, so the test is
> not really skipped.

Ah, I see. So the program was loaded/verifierd, but wasn't test-run.

Acked-by: Andrii Nakryiko <andriin@fb.com>

>
>
> >
> > > +                       return 0;
> > > +               default:
> > > +                       printf("FAIL: Unexpected bpf_prog_test_run error (%s) ", strerror(saved_errno));
> > > +                       return err;
> > > +               }
> > >         }
> > > -       if (!err && retval != expected_val &&
> > > +       if (retval != expected_val &&
> > >             expected_val != POINTER_VALUE) {
> > >                 printf("FAIL retval %d != %d ", retval, expected_val);
> > >                 return 1;
> > > --
> > > 2.20.1
> > >
>
>
>
> --
> Kinvolk GmbH | Adalbertstr.6a, 10999 Berlin | tel: +491755589364
> Geschäftsführer/Directors: Alban Crequy, Chris Kühl, Iago López Galeiras
> Registergericht/Court of registration: Amtsgericht Charlottenburg
> Registernummer/Registration number: HRB 171414 B
> Ust-ID-Nummer/VAT ID number: DE302207000