netdev - Re: [PATCH net-next 1/1] tc-testing: initial version of tunnel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 26 Jun 2018 16:51:46 +0200
From:   Davide Caratti <dcaratti@...hat.com>
To:     Keara Leibovitz <kleib@...atatu.com>, davem@...emloft.net
Cc:     netdev@...r.kernel.org, jhs@...atatu.com, xiyou.wangcong@...il.com,
        jiri@...nulli.us, lucasb@...atatu.com
Subject: Re: [PATCH net-next 1/1] tc-testing: initial version of tunnel_key
 unit tests

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@...atatu.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@...l tc-testing]# modprobe  act_tunnel_key
[root@...l 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@...l 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