[nft PATCH] tests/shell: unload modules between tests

[nft PATCH] tests/shell: unload modules between tests

[Arturo Borrero Gonzalez]

This patch adjusts the main test script so it unload all nftables
kernel modules between tests.

This way we achieve two interesting things:
 * avoid false errors in some testcases due to module loading order
 * the module loading/unloading path itself

The false positives is for example, listing ruleset per families, which depends
on the loading order of nf_tables_xx modules.

We can later add more modules to unload incrementally (for
example nf_tables_switchdev).

This patch assumes we are working with a kernel which is compiled with
nf_tables =m, the case using =y is not supported and can still produce false
positives in some testcases due to module ordering.

Reported-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 tests/shell/run-tests.sh |   25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index df2670b..c08a3eb 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -37,16 +37,37 @@ if [ ! -x "$FIND" ] ; then
 	msg_error "no find binary found"
 fi
 
+MODPROBE="$(which modprobe)"
+if [ ! -x "$MODPROBE" ] ; then
+	msg_error "no modprobe binary found"
+fi
+
 if [ "$1" == "-v" ] ; then
 	VERBOSE=y
 fi
 
+kernel_cleanup() {
+	$NFT flush ruleset
+	$MODPROBE -rq \
+	nft_reject_ipv4 nft_reject_ipv6 nft_bridge_reject \
+	nft_reject_ipv6 nft_reject \
+	nft_redir_ipv4 nft_redir_ipv6 nft_redir \
+	nft_dup_ipv4 nft_dup_ipv6 nft_dup \
+	nft_nat_ipv4 nft_nat_ipv6 nft_nat \
+	nft_masq_ipv4 nft_masq_ipv6 nft_masq \
+	nft_exthdr nft_payload nft_cmp \
+	nft_meta nft_bridge_meta nft_counter nft_log nft_limit \
+	nft_hash nft_rbtree nft_ct nft_compat \
+	nf_tables_inet nf_tables_bridge nf_tables_arp \
+	nf_tables_ipv4 nf_tables_ipv6 nf_tables
+}
+
 echo ""
 ok=0
 failed=0
 for testfile in $(${FIND} ${TESTDIR} -executable -regex .*${RETURNCODE_SEPARATOR}[0-9]+)
 do
-	$NFT flush ruleset
+	kernel_cleanup
 
 	rc_spec=$(awk -F${RETURNCODE_SEPARATOR} '{print $NF}' <<< $testfile)
 	test_output=$(NFT=$NFT ${testfile} ${TESTS_OUTPUT} 2>&1)
@@ -69,4 +90,4 @@ done
 echo ""
 msg_info "results: [OK] $ok [FAILED] $failed [TOTAL] $((ok+failed))"
 
-$NFT flush ruleset
+kernel_cleanup

Re: [nft PATCH] tests/shell: unload modules between tests

[Pablo Neira Ayuso]

On Thu, Mar 17, 2016 at 09:34:47AM +0100, Arturo Borrero Gonzalez wrote:
> This patch adjusts the main test script so it unload all nftables
> kernel modules between tests.
> 
> This way we achieve two interesting things:
>  * avoid false errors in some testcases due to module loading order
>  * the module loading/unloading path itself
> 
> The false positives is for example, listing ruleset per families, which depends
> on the loading order of nf_tables_xx modules.
> 
> We can later add more modules to unload incrementally (for
> example nf_tables_switchdev).
> 
> This patch assumes we are working with a kernel which is compiled with
> nf_tables =m, the case using =y is not supported and can still produce false
> positives in some testcases due to module ordering.

Applied, thanks Arturo.

Re: [nft PATCH] tests/shell: unload modules between tests

[Piyush Pangtey]

On Thursday 17 March 2016 02:04 PM, Arturo Borrero Gonzalez wrote:
> This patch adjusts the main test script so it unload all nftables
> kernel modules between tests.
> 
> This way we achieve two interesting things:
>  * avoid false errors in some testcases due to module loading order
>  * the module loading/unloading path itself
> 
[....]
> 	test_output=$(NFT=$NFT ${testfile} ${TESTS_OUTPUT} 2>&1)
> @@ -69,4 +90,4 @@ done
>  echo ""
>  msg_info "results: [OK] $ok [FAILED] $failed [TOTAL] $((ok+failed))"
>  
> -$NFT flush ruleset
> +kernel_cleanup
>

I'm getting nonzero return code(1) by this last kernel_cleanup call, maybe.

> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [nft PATCH] tests/shell: unload modules between tests

[Arturo Borrero Gonzalez]

[-- Attachment #1: Type: text/plain, Size: 279 bytes --]

On 17 March 2016 at 19:02, Piyush Pangtey <gokuvsvegita@gmail.com> wrote:
> I'm getting nonzero return code(1) by this last kernel_cleanup call, maybe.
>

Hi Piyush,

find attached a patch, please test it and let me know.

best regards.

-- 
Arturo Borrero González

[-- Attachment #2: tests-shell-run-tests-sh-force.patch --]
[-- Type: text/x-patch, Size: 1780 bytes --]

tests/shell/run-tests.sh: tune kernel cleanup

From: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>

The modprobe call can return != 0 if, for example, a module was builtin and
we are triying to remove it, so force return code of 0 at the end of the
script.

This patch also adds the '-a' switch to modprobe so it doesn't stop unloading
modules if one of them fails (for example, it was builtin).

While at it, fix several module names, for example: 'nft_bridge_reject' vs
'nft_reject_bridge', delete bogus module names.

Reported-by: Piyush Pangtey <gokuvsvegita@gmail.com>
Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 tests/shell/run-tests.sh |   11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index c08a3eb..620fe57 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -48,15 +48,13 @@ fi
 
 kernel_cleanup() {
 	$NFT flush ruleset
-	$MODPROBE -rq \
-	nft_reject_ipv4 nft_reject_ipv6 nft_bridge_reject \
-	nft_reject_ipv6 nft_reject \
+	$MODPROBE -raq \
+	nft_reject_ipv4 nft_reject_bridge nft_reject_ipv6 nft_reject \
 	nft_redir_ipv4 nft_redir_ipv6 nft_redir \
-	nft_dup_ipv4 nft_dup_ipv6 nft_dup \
-	nft_nat_ipv4 nft_nat_ipv6 nft_nat \
+	nft_dup_ipv4 nft_dup_ipv6 nft_dup nft_nat \
 	nft_masq_ipv4 nft_masq_ipv6 nft_masq \
 	nft_exthdr nft_payload nft_cmp \
-	nft_meta nft_bridge_meta nft_counter nft_log nft_limit \
+	nft_meta nft_meta_bridge nft_counter nft_log nft_limit \
 	nft_hash nft_rbtree nft_ct nft_compat \
 	nf_tables_inet nf_tables_bridge nf_tables_arp \
 	nf_tables_ipv4 nf_tables_ipv6 nf_tables
@@ -91,3 +89,4 @@ echo ""
 msg_info "results: [OK] $ok [FAILED] $failed [TOTAL] $((ok+failed))"
 
 kernel_cleanup
+exit 0

Re: [nft PATCH] tests/shell: unload modules between tests

[Pablo Neira Ayuso]

On Fri, Mar 18, 2016 at 09:41:31AM +0100, Arturo Borrero Gonzalez wrote:
> From: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> 
> The modprobe call can return != 0 if, for example, a module was builtin and
> we are triying to remove it, so force return code of 0 at the end of the
> script.
> 
> This patch also adds the '-a' switch to modprobe so it doesn't stop unloading
> modules if one of them fails (for example, it was builtin).
> 
> While at it, fix several module names, for example: 'nft_bridge_reject' vs
> 'nft_reject_bridge', delete bogus module names.

Applied, thanks.