On Thu, Jul 11, 2019 at 3:08 AM Andrii Nakryiko <andriin@xxxxxx> wrote:
>
> test_verifier tests can specify single- and multi-runs tests. Internally
> logic of handling them is duplicated. Get rid of it by making single run
> retval specification to be a first retvals spec.
>
> Cc: Krzesimir Nowak <krzesimir@xxxxxxxxxx>
> Signed-off-by: Andrii Nakryiko <andriin@xxxxxx>
Looks good, one nit below.
Acked-by: Krzesimir Nowak <krzesimir@xxxxxxxxxx>
> ---
> tools/testing/selftests/bpf/test_verifier.c | 37 ++++++++++-----------
> 1 file changed, 18 insertions(+), 19 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
> index b0773291012a..120ecdf4a7db 100644
> --- a/tools/testing/selftests/bpf/test_verifier.c
> +++ b/tools/testing/selftests/bpf/test_verifier.c
> @@ -86,7 +86,7 @@ struct bpf_test {
> int fixup_sk_storage_map[MAX_FIXUPS];
> const char *errstr;
> const char *errstr_unpriv;
> - uint32_t retval, retval_unpriv, insn_processed;
> + uint32_t insn_processed;
> int prog_len;
> enum {
> UNDEF,
> @@ -95,16 +95,24 @@ struct bpf_test {
> } result, result_unpriv;
> enum bpf_prog_type prog_type;
> uint8_t flags;
> - __u8 data[TEST_DATA_LEN];
> void (*fill_helper)(struct bpf_test *self);
> uint8_t runs;
> - struct {
> - uint32_t retval, retval_unpriv;
> - union {
> - __u8 data[TEST_DATA_LEN];
> - __u64 data64[TEST_DATA_LEN / 8];
> + union {
> + struct {
Maybe consider moving the struct definition outside to further the
removal of the duplication?
> + uint32_t retval, retval_unpriv;
> + union {
> + __u8 data[TEST_DATA_LEN];
> + __u64 data64[TEST_DATA_LEN / 8];
> + };
> };
> - } retvals[MAX_TEST_RUNS];
> + struct {
> + uint32_t retval, retval_unpriv;
> + union {
> + __u8 data[TEST_DATA_LEN];
> + __u64 data64[TEST_DATA_LEN / 8];
> + };
> + } retvals[MAX_TEST_RUNS];
> + };
> enum bpf_attach_type expected_attach_type;
> };
>
> @@ -949,17 +957,8 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
> uint32_t expected_val;
> int i;
>
> - if (!test->runs) {
> - expected_val = unpriv && test->retval_unpriv ?
> - test->retval_unpriv : test->retval;
> -
> - err = do_prog_test_run(fd_prog, unpriv, expected_val,
> - test->data, sizeof(test->data));
> - if (err)
> - run_errs++;
> - else
> - run_successes++;
> - }
> + if (!test->runs)
> + test->runs = 1;
>
> for (i = 0; i < test->runs; i++) {
> if (unpriv && test->retvals[i].retval_unpriv)
> --
> 2.17.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
