Re: [PATCH net-next 1/1] tc-testing: initial version of tunnel_key unit tests

[Davide Caratti <dcaratti@redhat.com>]

On Tue, 2018-06-26 at 09:17 -0400, Keara Leibovitz wrote:
> Create unittests for the tc tunnel_key action.
> 
> 
> Signed-off-by: Keara Leibovitz <kleib@mojatatu.com>
> ---
>  .../tc-testing/tc-tests/actions/tunnel_key.json    | 676 +++++++++++++++++++++
>  1 file changed, 676 insertions(+)
>  create mode 100644 tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json
> 
> diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json b/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json
> new file mode 100644
> index 000000000000..bfe522ac8177

hello Keara!

I think the 'teardown' stage in some of these tests should be reviewed.
Those that are meant to test invalid configurations (like dc6b) should
allow non-zero exit codes in the teardown stage, if the wrong
configuration is catched by the userspace TC tool, before talking to the
kernel. 

Otherwise, those tests will fail when they are invoked one by one with the
act_tunnel_key module unloaded.

> --- /dev/null
> +++ b/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json
> @@ -0,0 +1,676 @@
> 
...

> +    {
> +        "id": "dc6b",
> +        "name": "Add tunnel_key set action with missing mandatory src_ip parameter",
> +        "category": [
> +            "actions",
> +            "tunnel_key"
> +        ],
> +        "setup": [
> +            [
> +                "$TC actions flush action tunnel_key",
> +                0,
> +                1,
> +                255
> +            ]
> +        ],
> +        "cmdUnderTest": "$TC actions add action tunnel_key set dst_ip 20.20.20.2 id 100",
> +        "expExitCode": "255",
> +        "verifyCmd": "$TC actions list action tunnel_key",
> +        "matchPattern": "action order [0-9]+: tunnel_key set.*dst_ip 20.20.20.2.*key_id 100",
> +        "matchCount": "0",
> +        "teardown": [
> +            "$TC actions flush action tunnel_key"
> +        ]
> +    },

example: try the test above as follows:

[root@rhel tc-testing]# modprobe  act_tunnel_key
[root@rhel tc-testing]# ./tdc.py -e dc6b
Test dc6b: Add tunnel_key set action with missing mandatory src_ip parameter
All test results: 

1..1
ok 1 - dc6b # Add tunnel_key set action with missing mandatory src_ip parameter
about to flush the tap output if tests need to be skipped
done flushing skipped test tap output

[root@rhel tc-testing]# modprobe -r act_tunnel_key ; ./tdc.py -p /usr/local/src/iproute2/tc/tc -e dc6b
Test dc6b: Add tunnel_key set action with missing mandatory src_ip parameter

-----> teardown stage *** Could not execute: "$TC actions flush action tunnel_key"

-----> teardown stage *** Error message: "Error: Cannot flush unknown TC action.
We have an error flushing
"
[...]
---------------
accumulated output for this test:
---------------
All test results: 

1..1
about to flush the tap output if tests need to be skipped
ok 1 - dc6b # skipped - previous teardown failed 1 dc6b
done flushing skipped test tap output

(BTW: I'm fixing the bpf test suite for a similar problem, I forgot to fix
it when I posted commit f7017cafcdd ("tc-testing: fix tdc tests for 'bpf'
action") . Sorry for that.)


WDYT?

regards,
-- 
davide