netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Haller <thaller@redhat.com>
To: NetFilter <netfilter-devel@vger.kernel.org>
Cc: Thomas Haller <thaller@redhat.com>
Subject: [PATCH nft 3/3] tests/shell: run `nft --check` on persisted dump files
Date: Mon, 18 Sep 2023 21:59:24 +0200	[thread overview]
Message-ID: <20230918195933.318893-3-thaller@redhat.com> (raw)
In-Reply-To: <20230918195933.318893-1-thaller@redhat.com>

"nft --check" will trigger a rollback in kernel. The existing dump files
might hit new code paths. Take the opportunity to call the command on
the existing files.

And alternative would be to write a separate tests, that iterates over
all files. However, then we can only run all the commands sequentially
(unless we do something smart). That might be slower than the
opportunity to run the checks in parallel. More importantly, it would be
nice if the check for the dump file is clearly tied to the file's test.
So run it right after the test, from the test wrapper.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/helpers/test-wrapper.sh | 31 +++++++++++++++++++++++++++++
 tests/shell/run-tests.sh            |  4 +++-
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/tests/shell/helpers/test-wrapper.sh b/tests/shell/helpers/test-wrapper.sh
index 165a944da2b1..e10360c9b266 100755
--- a/tests/shell/helpers/test-wrapper.sh
+++ b/tests/shell/helpers/test-wrapper.sh
@@ -134,6 +134,35 @@ if [ "$rc_dump" -ne 0 ] ; then
 	echo "$DUMPFILE" > "$NFT_TEST_TESTTMPDIR/rc-failed-dump"
 fi
 
+rc_chkdump=0
+# check that a flush after the test succeeds. We anyway need a clean ruleset
+# for the `nft --check` next.
+$NFT flush ruleset &> "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump" || rc_chkdump=1
+if [ -f "$DUMPFILE" ] ; then
+	# We have a dumpfile. Call `nft --check` to possibly cover new code
+	# paths.
+	if [ "$rc_test" -eq 77 ] ; then
+		# The test was skipped. Possibly we don't have the required
+		# features to process this file. Ignore any output and exit
+		# code, but still call the program (for valgrind or sanitizer
+		# issue we hope to find).
+		$NFT --check -f "$DUMPFILE" &>/dev/null || :
+	else
+		$NFT --check -f "$DUMPFILE" &>> "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump" || rc_chkdump=1
+	fi
+fi
+if [ -s "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump" ] ; then
+	# Non-empty output? That is wrong.
+	rc_chkdump=1
+elif [ "$rc_chkdump" -eq 0 ] ; then
+	rm -rf "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump"
+fi
+if [ "$rc_chkdump" -ne 0 ] ; then
+	# Ensure we don't have empty output files. Always write something, so
+	# that `grep ^ -R` lists the file.
+	echo -e "<<<<<\n\nCalling \`nft --check\` (or \`nft flush ruleset\`) failed for \"$DUMPFILE\"" >> "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump"
+fi
+
 rc_valgrind=0
 [ -f "$NFT_TEST_TESTTMPDIR/rc-failed-valgrind" ] && rc_valgrind=1
 
@@ -154,6 +183,8 @@ elif [ "$rc_test" -ne 0 ] ; then
 	rc_exit="$rc_test"
 elif [ "$rc_dump" -ne 0 ] ; then
 	rc_exit=124
+elif [ "$rc_chkdump" -ne 0 ] ; then
+	rc_exit=121
 else
 	rc_exit=0
 fi
diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index 528646f57eca..85aa498ca8ee 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -731,7 +731,9 @@ print_test_result() {
 	else
 		((failed++))
 		result_msg_level="W"
-		if [ "$rc_got" -eq 122 ] ; then
+		if [ "$rc_got" -eq 121 ] ; then
+			result_msg_status="CHK DUMP"
+		elif [ "$rc_got" -eq 122 ] ; then
 			result_msg_status="VALGRIND"
 		elif [ "$rc_got" -eq 123 ] ; then
 			result_msg_status="TAINTED"
-- 
2.41.0


  parent reply	other threads:[~2023-09-18 20:00 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-18 19:59 [PATCH nft 1/3] tests/shell: fix preserving ruleset diff after test Thomas Haller
2023-09-18 19:59 ` [PATCH nft 2/3] tests/shell: simplify collecting error result in "test-wrapper.sh" Thomas Haller
2023-09-18 19:59 ` Thomas Haller [this message]
2023-09-20 15:50   ` [PATCH nft 3/3] tests/shell: run `nft --check` on persisted dump files Pablo Neira Ayuso

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230918195933.318893-3-thaller@redhat.com \
    --to=thaller@redhat.com \
    --cc=netfilter-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).