netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH nft v2 0/3] add "eval-exit-code" and skip tests based on kernel version
@ 2023-10-17  8:49 Thomas Haller
  2023-10-17  8:49 ` [PATCH nft v2 1/3] tests/shell: add "tests/shell/helpers/eval-exit-code" Thomas Haller
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Thomas Haller @ 2023-10-17  8:49 UTC (permalink / raw)
  To: NetFilter; +Cc: Thomas Haller

This is a follow-up and replaces the two patches:

  [PATCH nft 1/3] tests/shell: skip "table_onoff" test if kernel patch is missing
  [PATCH nft 2/3] tests/shell: skip "vlan_8021ad_tag" test instead of failing

Instead, add a helper script "eval-exit-code" which makes it easy(?) to
conditionally downgrade a test failure to a SKIP (exit 77) based on the
kernel version.

Thomas Haller (3):
  tests/shell: add "tests/shell/helpers/eval-exit-code"
  tests/shell: skip "table_onoff" test on older kernels
  tests/shell: skip "vlan_8021ad_tag" test on older kernels

 tests/shell/helpers/eval-exit-code            | 116 ++++++++++++++++++
 .../testcases/packetpath/vlan_8021ad_tag      |   8 +-
 .../shell/testcases/transactions/table_onoff  |   4 +-
 3 files changed, 126 insertions(+), 2 deletions(-)
 create mode 100755 tests/shell/helpers/eval-exit-code

-- 
2.41.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH nft v2 1/3] tests/shell: add "tests/shell/helpers/eval-exit-code"
  2023-10-17  8:49 [PATCH nft v2 0/3] add "eval-exit-code" and skip tests based on kernel version Thomas Haller
@ 2023-10-17  8:49 ` Thomas Haller
  2023-10-17  8:49 ` [PATCH nft v2 2/3] tests/shell: skip "table_onoff" test on older kernels Thomas Haller
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Thomas Haller @ 2023-10-17  8:49 UTC (permalink / raw)
  To: NetFilter; +Cc: Thomas Haller

This script is used to compare the running kernel version (`ulimit -r`)
against a list of versions, to find out whether a test should be skipped
or not.

If kernel has a bug, a test might not pass. In that case, the test
should exit with error code 77 as we also want to pass the test suite on
distro kernels. However, if we know that the bug was fixed in a certain
kernel version, then we the failure to be fatal and noticeable.

Example usage:

    if ! _check_some_condition ; then
        echo "The condition to check for kernel bug https://git.kernel.org/XYZ failed"
        "$NFT_TEST_BASEDIR/helpers/eval-exit-code" kernel 6.5.6 6.6
        exit $?
    fi

Note that "eval-exit-code" will always exit with a non-zero exit code. It will
also print a message about the comparison, which ends up in the test output.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/helpers/eval-exit-code | 116 +++++++++++++++++++++++++++++
 1 file changed, 116 insertions(+)
 create mode 100755 tests/shell/helpers/eval-exit-code

diff --git a/tests/shell/helpers/eval-exit-code b/tests/shell/helpers/eval-exit-code
new file mode 100755
index 000000000000..d007c0fc0fd6
--- /dev/null
+++ b/tests/shell/helpers/eval-exit-code
@@ -0,0 +1,116 @@
+#!/bin/bash -e
+
+die() {
+	printf "%s\n" "$*"
+	exit 2
+}
+
+usage() {
+	echo "usage: $0 [MODE] [ARGS...]"
+	echo ""
+	echo "Evaluates whether to exit a test with skip or failure reason."
+	echo ""
+	echo "The use case is for example a kernel bug, which prevents a test from passing. We"
+	echo "know that certain newer kernel versions have the fix, and we require the test to"
+	echo "pass there. When an assertion fails, The script will determine that the test should"
+	echo "have passed and exit with code \"1\". When running against an older kernel, the failure"
+	echo "is expected and the script exits with \"77\" to indicate a skip."
+	echo ""
+	echo "The script always either exits with:"
+	echo "  1: the check determined that we have a hard failure (a message is printed)"
+	echo "  77: the check determined that we skip (a message is printed)"
+	echo "  2: an error in the script happend (invalid arguments?)"
+	echo ""
+	echo "MODE can be one of:"
+	echo " \"kernel\": compares \`ulimit -r\` against the arguments. The arguments are"
+	echo "      kernel versions for which we expect to support the feature and when called"
+	echo "      on such a kernel, the script returns \"1\" to indicate a hard failure. Against"
+	echo "      older kernels, \"77\" is returned. Multiple kernel version can be provided for"
+	echo "      example \`$0 kernel 6.5.6 6.6\`."
+	echo ""
+	echo "USAGE:"
+	echo "    if ! _check_some_condition ; then"
+	echo "        echo \"The condition to check for kernel bug https://git.kernel.org/XYZ failed\""
+	echo "        \"\$NFT_TEST_BASEDIR/helpers/eval-exit-code\" kernel 6.5.6 6.6"
+	echo "        exit \$?"
+	echo "    fi"
+	exit 2
+}
+
+[ "$#" -eq 0 ] && usage
+
+_kernel_check_skip() {
+	local kversion="$1"
+	local compare="$2"
+
+	if [ "$kversion" = "$compare" ] ; then
+		return 1
+	fi
+	if [[ "$kversion" == "$compare"[.-]* ]] ; then
+		return 1
+	fi
+
+	local a1="$(printf '%s\n' 0 "$kversion" "$compare" 100000)"
+	local a2="$(printf '%s' "$a1" | sort -V)"
+
+	if [ "$a1" != "$a2" ] ; then
+		return 1
+	fi
+	return 0
+}
+
+_PRINT_ARG="???"
+
+eval_kernel() {
+	local compare
+	local kversion
+
+	if [ -n "$_EVAL_EXIT_CODE_UNAME" ] ; then
+		# Only for testing.
+		kversion="$_EVAL_EXIT_CODE_UNAME"
+	else
+		kversion="$(uname -r)" || die "uname -r failed"
+	fi
+
+	[ $# -ge 1 ] || die "Operation kernel expects one or more kernel versions to be compared with >= against $kversion"
+
+	_PRINT_ARG="$kversion"
+
+	local all_skip=1
+	for compare; do
+		_kernel_check_skip "$kversion" "$compare" || all_skip=0
+	done
+	if [ "$all_skip" -eq 1 ] ; then
+		return 77
+	fi
+	return 1
+}
+
+run_op() {
+	local mode="$1"
+	shift
+
+	rc=2
+	"eval_$mode" "$@" || rc=$?
+	if [ "$rc" -eq 77 ] ; then
+		echo "Checking \"$mode\" ($@) against $_PRINT_ARG indicates to skip the test"
+	else
+		echo "Checking \"$mode\" ($@) against $_PRINT_ARG indicates a hard failure"
+	fi
+	exit "$rc"
+}
+
+mode="$1"
+shift
+
+case "$mode" in
+	kernel)
+		run_op "$mode" "$@"
+		;;
+	-h|--help)
+		usage
+		;;
+	*)
+		die "Invalid mode \"$mode\". Try $0 --help"
+		;;
+esac
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH nft v2 2/3] tests/shell: skip "table_onoff" test on older kernels
  2023-10-17  8:49 [PATCH nft v2 0/3] add "eval-exit-code" and skip tests based on kernel version Thomas Haller
  2023-10-17  8:49 ` [PATCH nft v2 1/3] tests/shell: add "tests/shell/helpers/eval-exit-code" Thomas Haller
@ 2023-10-17  8:49 ` Thomas Haller
  2023-10-17  9:04   ` Pablo Neira Ayuso
  2023-10-17  8:49 ` [PATCH nft v2 3/3] tests/shell: skip "vlan_8021ad_tag" " Thomas Haller
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Thomas Haller @ 2023-10-17  8:49 UTC (permalink / raw)
  To: NetFilter; +Cc: Thomas Haller

The "table_onoff" test can only pass with certain (recent) kernels.
Conditionally exit with status 77, if "eval-exit-code" determines that
we don't have a suitable kernel version.

In this case, we can find the fixes in:

 v6.6      : https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c9bd26513b3a11b3adb3c2ed8a31a01a87173ff1
 v6.5.6    : https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5e5754e9e77ce400d70ff3c30fea466c8dfe9a9f
 v6.1.56   : https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c4b0facd5c20ceae3d07018a3417f06302fa9cd1
 v5.15.135 : https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0dcc9b4097d860d9af52db5366a8755c13468d13

Fixes: bcca2d67656f ('tests: add test for dormant on/off/on bug')
Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/testcases/transactions/table_onoff | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/shell/testcases/transactions/table_onoff b/tests/shell/testcases/transactions/table_onoff
index 831d4614c1f2..0e70ad2cc3f4 100755
--- a/tests/shell/testcases/transactions/table_onoff
+++ b/tests/shell/testcases/transactions/table_onoff
@@ -11,7 +11,9 @@ delete table ip t
 EOF
 
 if [ $? -eq 0 ]; then
-	exit 1
+	echo "Command to re-awaken a dormant table did not fail. Lacking https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c9bd26513b3a11b3adb3c2ed8a31a01a87173ff1 ?"
+	"$NFT_TEST_BASEDIR/helpers/eval-exit-code" kernel  6.6  6.5.6  6.1.56  5.15.135
+	exit $?
 fi
 
 set -e
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH nft v2 3/3] tests/shell: skip "vlan_8021ad_tag" test on older kernels
  2023-10-17  8:49 [PATCH nft v2 0/3] add "eval-exit-code" and skip tests based on kernel version Thomas Haller
  2023-10-17  8:49 ` [PATCH nft v2 1/3] tests/shell: add "tests/shell/helpers/eval-exit-code" Thomas Haller
  2023-10-17  8:49 ` [PATCH nft v2 2/3] tests/shell: skip "table_onoff" test on older kernels Thomas Haller
@ 2023-10-17  8:49 ` Thomas Haller
  2023-10-17  9:24 ` [PATCH nft v2 0/3] add "eval-exit-code" and skip tests based on kernel version Florian Westphal
  2023-10-17 20:41 ` Florian Westphal
  4 siblings, 0 replies; 11+ messages in thread
From: Thomas Haller @ 2023-10-17  8:49 UTC (permalink / raw)
  To: NetFilter; +Cc: Thomas Haller

The "vlan_8021ad_tag" test can only pass with certain (recent) kernels.
Conditionally exit with status 77, if "eval-exit-code" determines that
we don't have a suitable kernel version.

In this case, we can find the fixes in:

 v6.6      : https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=af84f9e447a65b4b9f79e7e5d69e19039b431c56
 v6.5.7    : https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3f6a5636a96687381b329649950f21258bae380d

Fixes: 74cf3d16d8e9 ('tests: shell: add vlan match test case')
Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/testcases/packetpath/vlan_8021ad_tag | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tests/shell/testcases/packetpath/vlan_8021ad_tag b/tests/shell/testcases/packetpath/vlan_8021ad_tag
index 379a5710c1cb..6d908fe330df 100755
--- a/tests/shell/testcases/packetpath/vlan_8021ad_tag
+++ b/tests/shell/testcases/packetpath/vlan_8021ad_tag
@@ -47,4 +47,10 @@ EOF
 ip netns exec "$ns1" ping -c 1 10.1.1.2
 
 ip netns exec "$ns2" $NFT list ruleset
-ip netns exec "$ns2" $NFT list chain netdev t c | grep 'counter packets 1 bytes 84'
+OUT="$(ip netns exec "$ns2" $NFT list chain netdev t c)"
+
+if ! printf "%s" "$OUT" | grep -q 'counter packets 1 bytes 84' ; then
+	echo "Filter did not match. Missing https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=af84f9e447a65b4b9f79e7e5d69e19039b431c56 ?"
+	"$NFT_TEST_BASEDIR/helpers/eval-exit-code" kernel  6.6  6.5.7
+	exit $?
+fi
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH nft v2 2/3] tests/shell: skip "table_onoff" test on older kernels
  2023-10-17  8:49 ` [PATCH nft v2 2/3] tests/shell: skip "table_onoff" test on older kernels Thomas Haller
@ 2023-10-17  9:04   ` Pablo Neira Ayuso
  2023-10-17 11:21     ` Thomas Haller
  0 siblings, 1 reply; 11+ messages in thread
From: Pablo Neira Ayuso @ 2023-10-17  9:04 UTC (permalink / raw)
  To: Thomas Haller; +Cc: NetFilter

On Tue, Oct 17, 2023 at 10:49:07AM +0200, Thomas Haller wrote:
> The "table_onoff" test can only pass with certain (recent) kernels.
> Conditionally exit with status 77, if "eval-exit-code" determines that
> we don't have a suitable kernel version.
> 
> In this case, we can find the fixes in:
> 
>  v6.6      : https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c9bd26513b3a11b3adb3c2ed8a31a01a87173ff1
>  v6.5.6    : https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5e5754e9e77ce400d70ff3c30fea466c8dfe9a9f
>  v6.1.56   : https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c4b0facd5c20ceae3d07018a3417f06302fa9cd1
>  v5.15.135 : https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0dcc9b4097d860d9af52db5366a8755c13468d13

I am not sure it worth this level of tracking.

Soon these patches will be in upstream stable and this extra shell
code will be simply deadcode in little time.

> Fixes: bcca2d67656f ('tests: add test for dormant on/off/on bug')
> Signed-off-by: Thomas Haller <thaller@redhat.com>
> ---
>  tests/shell/testcases/transactions/table_onoff | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/shell/testcases/transactions/table_onoff b/tests/shell/testcases/transactions/table_onoff
> index 831d4614c1f2..0e70ad2cc3f4 100755
> --- a/tests/shell/testcases/transactions/table_onoff
> +++ b/tests/shell/testcases/transactions/table_onoff
> @@ -11,7 +11,9 @@ delete table ip t
>  EOF
>  
>  if [ $? -eq 0 ]; then
> -	exit 1
> +	echo "Command to re-awaken a dormant table did not fail. Lacking https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c9bd26513b3a11b3adb3c2ed8a31a01a87173ff1 ?"
> +	"$NFT_TEST_BASEDIR/helpers/eval-exit-code" kernel  6.6  6.5.6  6.1.56  5.15.135
> +	exit $?
>  fi
>  
>  set -e
> -- 
> 2.41.0
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH nft v2 0/3] add "eval-exit-code" and skip tests based on kernel version
  2023-10-17  8:49 [PATCH nft v2 0/3] add "eval-exit-code" and skip tests based on kernel version Thomas Haller
                   ` (2 preceding siblings ...)
  2023-10-17  8:49 ` [PATCH nft v2 3/3] tests/shell: skip "vlan_8021ad_tag" " Thomas Haller
@ 2023-10-17  9:24 ` Florian Westphal
  2023-10-17 20:41 ` Florian Westphal
  4 siblings, 0 replies; 11+ messages in thread
From: Florian Westphal @ 2023-10-17  9:24 UTC (permalink / raw)
  To: Thomas Haller; +Cc: NetFilter

Thomas Haller <thaller@redhat.com> wrote:
> This is a follow-up and replaces the two patches:
> 
>   [PATCH nft 1/3] tests/shell: skip "table_onoff" test if kernel patch is missing
>   [PATCH nft 2/3] tests/shell: skip "vlan_8021ad_tag" test instead of failing
> 
> Instead, add a helper script "eval-exit-code" which makes it easy(?) to
> conditionally downgrade a test failure to a SKIP (exit 77) based on the
> kernel version.

Unfortunately kernel versions are meaningless, especially for the
average commericial frankenkernels.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH nft v2 2/3] tests/shell: skip "table_onoff" test on older kernels
  2023-10-17  9:04   ` Pablo Neira Ayuso
@ 2023-10-17 11:21     ` Thomas Haller
  2023-10-17 11:40       ` Pablo Neira Ayuso
  0 siblings, 1 reply; 11+ messages in thread
From: Thomas Haller @ 2023-10-17 11:21 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: NetFilter

On Tue, 2023-10-17 at 11:04 +0200, Pablo Neira Ayuso wrote:
> On Tue, Oct 17, 2023 at 10:49:07AM +0200, Thomas Haller wrote:
> > The "table_onoff" test can only pass with certain (recent) kernels.
> > Conditionally exit with status 77, if "eval-exit-code" determines
> > that
> > we don't have a suitable kernel version.
> > 
> > In this case, we can find the fixes in:
> > 
> >  v6.6      :
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c9bd26513b3a11b3adb3c2ed8a31a01a87173ff1
> >  v6.5.6    :
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5e5754e9e77ce400d70ff3c30fea466c8dfe9a9f
> >  v6.1.56   :
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c4b0facd5c20ceae3d07018a3417f06302fa9cd1
> >  v5.15.135 :
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0dcc9b4097d860d9af52db5366a8755c13468d13
> 
> I am not sure it worth this level of tracking.
> 
> Soon these patches will be in upstream stable and this extra shell
> code will be simply deadcode in little time.

I am not concerned about dead code in old tests that keep passing.
The code was useful once, now the test passes. No need to revisit them,
unless you see a real problem with them.

If it would be only little time, the tests should wait. But how much is
the right time? You are not waiting for your use-case, you are holding
back to not to break the unknown use cases of others.

IMO merging tests is good. The problem just needs a good solution.


Thomas


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH nft v2 2/3] tests/shell: skip "table_onoff" test on older kernels
  2023-10-17 11:21     ` Thomas Haller
@ 2023-10-17 11:40       ` Pablo Neira Ayuso
  2023-10-17 12:43         ` Thomas Haller
  0 siblings, 1 reply; 11+ messages in thread
From: Pablo Neira Ayuso @ 2023-10-17 11:40 UTC (permalink / raw)
  To: Thomas Haller; +Cc: NetFilter

On Tue, Oct 17, 2023 at 01:21:54PM +0200, Thomas Haller wrote:
> On Tue, 2023-10-17 at 11:04 +0200, Pablo Neira Ayuso wrote:
> > On Tue, Oct 17, 2023 at 10:49:07AM +0200, Thomas Haller wrote:
> > > The "table_onoff" test can only pass with certain (recent) kernels.
> > > Conditionally exit with status 77, if "eval-exit-code" determines
> > > that
> > > we don't have a suitable kernel version.
> > > 
> > > In this case, we can find the fixes in:
> > > 
> > >  v6.6      :
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c9bd26513b3a11b3adb3c2ed8a31a01a87173ff1
> > >  v6.5.6    :
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5e5754e9e77ce400d70ff3c30fea466c8dfe9a9f
> > >  v6.1.56   :
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c4b0facd5c20ceae3d07018a3417f06302fa9cd1
> > >  v5.15.135 :
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0dcc9b4097d860d9af52db5366a8755c13468d13
> > 
> > I am not sure it worth this level of tracking.
> > 
> > Soon these patches will be in upstream stable and this extra shell
> > code will be simply deadcode in little time.
> 
> I am not concerned about dead code in old tests that keep passing.
> The code was useful once, now the test passes. No need to revisit them,
> unless you see a real problem with them.
> 
> If it would be only little time, the tests should wait. But how much is
> the right time? You are not waiting for your use-case, you are holding
> back to not to break the unknown use cases of others.
> 
> IMO merging tests is good. The problem just needs a good solution.

Apologies, I don't think this kind of hints is worth.

It might be a bug that resurrects in the future and then the
suggestion that a commit is missing will be misleading.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH nft v2 2/3] tests/shell: skip "table_onoff" test on older kernels
  2023-10-17 11:40       ` Pablo Neira Ayuso
@ 2023-10-17 12:43         ` Thomas Haller
  2023-10-17 13:58           ` Pablo Neira Ayuso
  0 siblings, 1 reply; 11+ messages in thread
From: Thomas Haller @ 2023-10-17 12:43 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: NetFilter

On Tue, 2023-10-17 at 13:40 +0200, Pablo Neira Ayuso wrote:
> On Tue, Oct 17, 2023 at 01:21:54PM +0200, Thomas Haller wrote:
> > On Tue, 2023-10-17 at 11:04 +0200, Pablo Neira Ayuso wrote:
> > > On Tue, Oct 17, 2023 at 10:49:07AM +0200, Thomas Haller wrote:
> > > > The "table_onoff" test can only pass with certain (recent)
> > > > kernels.
> > > > Conditionally exit with status 77, if "eval-exit-code"
> > > > determines
> > > > that
> > > > we don't have a suitable kernel version.
> > > > 
> > > > In this case, we can find the fixes in:
> > > > 
> > > >  v6.6      :
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c9bd26513b3a11b3adb3c2ed8a31a01a87173ff1
> > > >  v6.5.6    :
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5e5754e9e77ce400d70ff3c30fea466c8dfe9a9f
> > > >  v6.1.56   :
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c4b0facd5c20ceae3d07018a3417f06302fa9cd1
> > > >  v5.15.135 :
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0dcc9b4097d860d9af52db5366a8755c13468d13
> > > 
> > > I am not sure it worth this level of tracking.
> > > 
> > > Soon these patches will be in upstream stable and this extra
> > > shell
> > > code will be simply deadcode in little time.
> > 
> > I am not concerned about dead code in old tests that keep passing.
> > The code was useful once, now the test passes. No need to revisit
> > them,
> > unless you see a real problem with them.
> > 
> > If it would be only little time, the tests should wait. But how
> > much is
> > the right time? You are not waiting for your use-case, you are
> > holding
> > back to not to break the unknown use cases of others.
> > 
> > IMO merging tests is good. The problem just needs a good solution.
> 
> Apologies, I don't think this kind of hints is worth.


Which hint do you mean?

Do you mean the commit message, or the

    echo "Command to re-awaken a dormant table did not fail. Lacking https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c9bd26513b3a11b3adb3c2ed8a31a01a87173ff1 ?"

The echo is really just an elaborate code-comment. But it also ends up
in "testout.log", which makes it better.


I don't mind rewording commit message or the echo statement (or even
dropping it entirely). But I find this information more useful than
not.


Thomas


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH nft v2 2/3] tests/shell: skip "table_onoff" test on older kernels
  2023-10-17 12:43         ` Thomas Haller
@ 2023-10-17 13:58           ` Pablo Neira Ayuso
  0 siblings, 0 replies; 11+ messages in thread
From: Pablo Neira Ayuso @ 2023-10-17 13:58 UTC (permalink / raw)
  To: Thomas Haller; +Cc: NetFilter

On Tue, Oct 17, 2023 at 02:43:56PM +0200, Thomas Haller wrote:
> On Tue, 2023-10-17 at 13:40 +0200, Pablo Neira Ayuso wrote:
> > On Tue, Oct 17, 2023 at 01:21:54PM +0200, Thomas Haller wrote:
> > > On Tue, 2023-10-17 at 11:04 +0200, Pablo Neira Ayuso wrote:
> > > > On Tue, Oct 17, 2023 at 10:49:07AM +0200, Thomas Haller wrote:
> > > > > The "table_onoff" test can only pass with certain (recent)
> > > > > kernels.
> > > > > Conditionally exit with status 77, if "eval-exit-code"
> > > > > determines
> > > > > that
> > > > > we don't have a suitable kernel version.
> > > > > 
> > > > > In this case, we can find the fixes in:
> > > > > 
> > > > >  v6.6      :
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c9bd26513b3a11b3adb3c2ed8a31a01a87173ff1
> > > > >  v6.5.6    :
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5e5754e9e77ce400d70ff3c30fea466c8dfe9a9f
> > > > >  v6.1.56   :
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c4b0facd5c20ceae3d07018a3417f06302fa9cd1
> > > > >  v5.15.135 :
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0dcc9b4097d860d9af52db5366a8755c13468d13
> > > > 
> > > > I am not sure it worth this level of tracking.
> > > > 
> > > > Soon these patches will be in upstream stable and this extra
> > > > shell
> > > > code will be simply deadcode in little time.
> > > 
> > > I am not concerned about dead code in old tests that keep passing.
> > > The code was useful once, now the test passes. No need to revisit
> > > them,
> > > unless you see a real problem with them.
> > > 
> > > If it would be only little time, the tests should wait. But how
> > > much is
> > > the right time? You are not waiting for your use-case, you are
> > > holding
> > > back to not to break the unknown use cases of others.
> > > 
> > > IMO merging tests is good. The problem just needs a good solution.
> > 
> > Apologies, I don't think this kind of hints is worth.
> 
> 
> Which hint do you mean?
> 
> Do you mean the commit message, or the
> 
>     echo "Command to re-awaken a dormant table did not fail. Lacking https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c9bd26513b3a11b3adb3c2ed8a31a01a87173ff1 ?"
> 
> The echo is really just an elaborate code-comment. But it also ends up
> in "testout.log", which makes it better.
> 
> 
> I don't mind rewording commit message or the echo statement (or even
> dropping it entirely). But I find this information more useful than
> not.

But I still don't get the point on skipping this test in older kernels,
this test tells you your kernel is buggy.

Who is going to run this in older kernels? We only run these tests
these days.

Skipping a test that tells you that you kernel is buggy is misleading?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH nft v2 0/3] add "eval-exit-code" and skip tests based on kernel version
  2023-10-17  8:49 [PATCH nft v2 0/3] add "eval-exit-code" and skip tests based on kernel version Thomas Haller
                   ` (3 preceding siblings ...)
  2023-10-17  9:24 ` [PATCH nft v2 0/3] add "eval-exit-code" and skip tests based on kernel version Florian Westphal
@ 2023-10-17 20:41 ` Florian Westphal
  4 siblings, 0 replies; 11+ messages in thread
From: Florian Westphal @ 2023-10-17 20:41 UTC (permalink / raw)
  To: Thomas Haller; +Cc: NetFilter

Thomas Haller <thaller@redhat.com> wrote:
> This is a follow-up and replaces the two patches:
> 
>   [PATCH nft 1/3] tests/shell: skip "table_onoff" test if kernel patch is missing
>   [PATCH nft 2/3] tests/shell: skip "vlan_8021ad_tag" test instead of failing
> 
> Instead, add a helper script "eval-exit-code" which makes it easy(?) to
> conditionally downgrade a test failure to a SKIP (exit 77) based on the
> kernel version.

I think we should leave things as-is.

If this proves to be an issue (esp. if we have crasher-tests...),
we could make a new subdir, e.g. tests/shell/testcases/kernel, place
thse tests there and then have the 'make check' export a new environment
variable, e.g. 'SKIP_KERNEL_TESTS" or whatever.

But for now I think things are fine, Fedora should catch up soon with
the vlan test case (its already in 6.5.y stable) so this will go away.

And for others this hints that they need to complain to their vendor,
or backport a the fix to resolve this bug.

I think SKIP should be reseved only for tests that fail a feature-probe
dependency.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-10-17 20:41 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-17  8:49 [PATCH nft v2 0/3] add "eval-exit-code" and skip tests based on kernel version Thomas Haller
2023-10-17  8:49 ` [PATCH nft v2 1/3] tests/shell: add "tests/shell/helpers/eval-exit-code" Thomas Haller
2023-10-17  8:49 ` [PATCH nft v2 2/3] tests/shell: skip "table_onoff" test on older kernels Thomas Haller
2023-10-17  9:04   ` Pablo Neira Ayuso
2023-10-17 11:21     ` Thomas Haller
2023-10-17 11:40       ` Pablo Neira Ayuso
2023-10-17 12:43         ` Thomas Haller
2023-10-17 13:58           ` Pablo Neira Ayuso
2023-10-17  8:49 ` [PATCH nft v2 3/3] tests/shell: skip "vlan_8021ad_tag" " Thomas Haller
2023-10-17  9:24 ` [PATCH nft v2 0/3] add "eval-exit-code" and skip tests based on kernel version Florian Westphal
2023-10-17 20:41 ` Florian Westphal

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).