Re: [nft PATCH] tests/shell: add chain validations tests

[Mart Frauenlob <mart.frauenlob@chello.at>]

Good day,

On 22.03.2016 14:06, Arturo Borrero Gonzalez wrote:
> Some basic test regarding chains: jumps and validations.
>
> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> ---
> NOTE: the testcases/chains/0009masquerade_jump_1 file fails, seems like a bug
> in the kernel validation. Needs more investigation.
>
>   tests/shell/testcases/chains/0001jumps_0           |   17 +++++++++++++++
>   tests/shell/testcases/chains/0002jumps_1           |   22 ++++++++++++++++++++
>   tests/shell/testcases/chains/0003jump_loop_1       |   21 +++++++++++++++++++
>   tests/shell/testcases/chains/0004busy_1            |   11 ++++++++++
>   tests/shell/testcases/chains/0005busy_map_1        |   11 ++++++++++
>   tests/shell/testcases/chains/0006masquerade_0      |    7 ++++++
>   tests/shell/testcases/chains/0007masquerade_1      |    9 ++++++++
>   tests/shell/testcases/chains/0008masquerade_jump_1 |   11 ++++++++++
>   tests/shell/testcases/chains/0009masquerade_jump_1 |   11 ++++++++++
>   9 files changed, 120 insertions(+)
>   create mode 100755 tests/shell/testcases/chains/0001jumps_0
>   create mode 100755 tests/shell/testcases/chains/0002jumps_1
>   create mode 100755 tests/shell/testcases/chains/0003jump_loop_1
>   create mode 100755 tests/shell/testcases/chains/0004busy_1
>   create mode 100755 tests/shell/testcases/chains/0005busy_map_1
>   create mode 100755 tests/shell/testcases/chains/0006masquerade_0
>   create mode 100755 tests/shell/testcases/chains/0007masquerade_1
>   create mode 100755 tests/shell/testcases/chains/0008masquerade_jump_1
>   create mode 100755 tests/shell/testcases/chains/0009masquerade_jump_1
>
> diff --git a/tests/shell/testcases/chains/0001jumps_0 b/tests/shell/testcases/chains/0001jumps_0
> new file mode 100755
> index 0000000..b39df38
> --- /dev/null
> +++ b/tests/shell/testcases/chains/0001jumps_0
> @@ -0,0 +1,17 @@
> +#!/bin/bash

I've not looked up the code calling this, but:
First: bash only?
Second: It's not granted to be in /bin.
Third: May not be the wanted version.

So a shebang like:
#!/usr/bin/env bash
or
#!/urs/bin/env sh
should be more compatible and fail proof.

> +
> +set -e
> +
> +MAX_JUMPS=16
> +
> +$NFT add table t
Unquoted variable, may fail if, unlikely but possible, the name contains 
i.e. spaces.

> +
> +for i in $(seq 1 $MAX_JUMPS)
> +do
> +	$NFT add chain t c${i}
> +done
Requires `seq' binary.
I think for ((i=1; i<=$MAX_JUMPS; i++)) is more portable.

> +
> +for i in $(seq 1 $((MAX_JUMPS - 1)))
> +do
> +	$NFT add rule t c${i} jump c$((i + 1))
> +done

Why not add functions? i.e.

runft() {
"$NFT" "$@"
}

nfat() {
runft add table "$@"
}

nfac() {
runft add chain "$@"
}
....

[...]

Best regards,
Mart