[PATCH nft 0/3] shell/tests: cleanups and skip tests on Fedora 38

[PATCH nft 0/3] shell/tests: cleanups and skip tests on Fedora 38

[Thomas Haller]

- cleanup dummy interface handling
- adjust 2 tests so that they are skipped on Fedora 38 kernel
  (6.4.14-200.fc38.x86_64). The end goal is to run the test suite
  without failures on Fedora 38.

    - tests/shell/testcases/sets/reset_command_0
    - tests/shell/testcases/sets/0030add_many_elements_interval_0

Thomas Haller (3):
  tests/shell: cleanup creating dummy interfaces in tests
  tests/shell: skip "sets/reset_command_0" on unsupported reset command
  tests/shell: suggest 4Mb /proc/sys/net/core/{wmem_max,rmem_max} for
    rootless

 tests/shell/run-tests.sh                      |  6 ++---
 .../testcases/chains/dumps/netdev_chain_0.nft |  3 ---
 tests/shell/testcases/chains/netdev_chain_0   | 26 +++++++------------
 .../flowtable/0012flowtable_variable_0        |  6 +++++
 .../dumps/0012flowtable_variable_0.nft        |  4 +--
 tests/shell/testcases/json/netdev             | 12 +++++----
 tests/shell/testcases/listing/0020flowtable_0 | 12 +++++----
 tests/shell/testcases/sets/reset_command_0    | 20 ++++++++++----
 8 files changed, 50 insertions(+), 39 deletions(-)

-- 
2.41.0

[PATCH nft 1/3] tests/shell: cleanup creating dummy interfaces in tests

[Thomas Haller]

In "tests/shell/testcases/chains/netdev_chain_0", calling "trap ...
EXIT" multiple times does not work. Fix it, by calling one cleanup
function.

Note that we run in separate namespaces, so the cleanup is usually not
necessary. Still do it, we might want to run without unshare (via
NFT_TEST_UNSHARE_CMD=""). Without unshare, it's important that the
cleanup always works. In practice it might not, for example, "trap ...
EXIT" does not run for SIGTERM. A leaked interface might break the
follow up test and tests interfere with each other.

Try to workaround that by first trying to delete the interface.

Also failures to create the interfaces are not considered fatal. I don't
understand under what circumstances this might fail, note that there are
other tests that create dummy interface and don't "exit 77" on failure.
We want to know when something odd is going on.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 .../testcases/chains/dumps/netdev_chain_0.nft |  3 ---
 tests/shell/testcases/chains/netdev_chain_0   | 26 +++++++------------
 .../flowtable/0012flowtable_variable_0        |  6 +++++
 .../dumps/0012flowtable_variable_0.nft        |  4 +--
 tests/shell/testcases/json/netdev             | 12 +++++----
 tests/shell/testcases/listing/0020flowtable_0 | 12 +++++----
 6 files changed, 32 insertions(+), 31 deletions(-)

diff --git a/tests/shell/testcases/chains/dumps/netdev_chain_0.nft b/tests/shell/testcases/chains/dumps/netdev_chain_0.nft
index bc02dc18692d..aa571e00885f 100644
--- a/tests/shell/testcases/chains/dumps/netdev_chain_0.nft
+++ b/tests/shell/testcases/chains/dumps/netdev_chain_0.nft
@@ -1,5 +1,2 @@
 table netdev x {
-	chain y {
-		type filter hook ingress devices = { d0, d1 } priority filter; policy accept;
-	}
 }
diff --git a/tests/shell/testcases/chains/netdev_chain_0 b/tests/shell/testcases/chains/netdev_chain_0
index 41e724413528..88bbc437d471 100755
--- a/tests/shell/testcases/chains/netdev_chain_0
+++ b/tests/shell/testcases/chains/netdev_chain_0
@@ -1,24 +1,18 @@
 #!/bin/bash
 
-ip link add d0 type dummy || {
-        echo "Skipping, no dummy interface available"
-        exit 77
-}
-trap "ip link del d0" EXIT
-
-ip link add d1 type dummy || {
-        echo "Skipping, no dummy interface available"
-        exit 77
-}
-trap "ip link del d1" EXIT
+set -e
 
-ip link add d2 type dummy || {
-        echo "Skipping, no dummy interface available"
-        exit 77
+iface_cleanup() {
+	ip link del d0 &>/dev/null || :
+	ip link del d1 &>/dev/null || :
+	ip link del d2 &>/dev/null || :
 }
-trap "ip link del d2" EXIT
+trap 'iface_cleanup' EXIT
+iface_cleanup
 
-set -e
+ip link add d0 type dummy
+ip link add d1 type dummy
+ip link add d2 type dummy
 
 RULESET="table netdev x {
 	chain y {
diff --git a/tests/shell/testcases/flowtable/0012flowtable_variable_0 b/tests/shell/testcases/flowtable/0012flowtable_variable_0
index 8e334224ac66..080059d24935 100755
--- a/tests/shell/testcases/flowtable/0012flowtable_variable_0
+++ b/tests/shell/testcases/flowtable/0012flowtable_variable_0
@@ -2,6 +2,12 @@
 
 set -e
 
+iface_cleanup() {
+	ip link del dummy1 &>/dev/null || :
+}
+trap 'iface_cleanup' EXIT
+iface_cleanup
+
 ip link add name dummy1 type dummy
 
 EXPECTED="define if_main = { lo, dummy1 }
diff --git a/tests/shell/testcases/flowtable/dumps/0012flowtable_variable_0.nft b/tests/shell/testcases/flowtable/dumps/0012flowtable_variable_0.nft
index 1cbb2f1103f0..df1c51a24703 100644
--- a/tests/shell/testcases/flowtable/dumps/0012flowtable_variable_0.nft
+++ b/tests/shell/testcases/flowtable/dumps/0012flowtable_variable_0.nft
@@ -1,14 +1,14 @@
 table ip filter1 {
 	flowtable Main_ft1 {
 		hook ingress priority filter
-		devices = { dummy1, lo }
+		devices = { lo }
 		counter
 	}
 }
 table ip filter2 {
 	flowtable Main_ft2 {
 		hook ingress priority filter
-		devices = { dummy1, lo }
+		devices = { lo }
 		counter
 	}
 }
diff --git a/tests/shell/testcases/json/netdev b/tests/shell/testcases/json/netdev
index dad7afcdc020..8c16cf42baa0 100755
--- a/tests/shell/testcases/json/netdev
+++ b/tests/shell/testcases/json/netdev
@@ -1,12 +1,14 @@
 #!/bin/bash
 
-ip link add d0 type dummy || {
-        echo "Skipping, no dummy interface available"
-        exit 77
+set -e
+
+iface_cleanup() {
+	ip link del d0 &>/dev/null || :
 }
-trap "ip link del d0" EXIT
+trap 'iface_cleanup' EXIT
+iface_cleanup
 
-set -e
+ip link add d0 type dummy
 
 $NFT flush ruleset
 $NFT add table inet test
diff --git a/tests/shell/testcases/listing/0020flowtable_0 b/tests/shell/testcases/listing/0020flowtable_0
index 210289d70415..6eb82cfeabc3 100755
--- a/tests/shell/testcases/listing/0020flowtable_0
+++ b/tests/shell/testcases/listing/0020flowtable_0
@@ -2,6 +2,8 @@
 
 # list only the flowtable asked for with table
 
+set -e
+
 FLOWTABLES="flowtable f {
 	hook ingress priority filter
 	devices = { lo }
@@ -41,13 +43,13 @@ EXPECTED3="table ip filter {
 	}
 }"
 
-ip link add d0 type dummy || {
-	echo "Skipping, no dummy interface available"
-	exit 77
+iface_cleanup() {
+	ip link del d0 &>/dev/null || :
 }
-trap "ip link del d0" EXIT
+trap 'iface_cleanup' EXIT
+iface_cleanup
 
-set -e
+ip link add d0 type dummy
 
 $NFT -f - <<< "$RULESET"
 
-- 
2.41.0

[PATCH nft 2/3] tests/shell: skip "sets/reset_command_0" on unsupported reset command

[Thomas Haller]

The NFT_MSG_GETSETELEM_RESET command was only added to kernel
v6.4-rc3-764-g079cd633219d. Also, it doesn't work on Fedora 38
(6.4.14-200.fc38.x86_64), although that would appear to have the
feature. On CentOS-Stream-9 (5.14.0-354.el9.x86_64) the test passes.

Note that this is not implemented via a re-usable feature detection.
Instead, we just in the middle of the test notice that it appears not to
work, and abort (skip).

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=079cd633219d7298d087cd115c17682264244c18

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/testcases/sets/reset_command_0 | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/tests/shell/testcases/sets/reset_command_0 b/tests/shell/testcases/sets/reset_command_0
index ad2e16a7d274..a0f5ca017b0f 100755
--- a/tests/shell/testcases/sets/reset_command_0
+++ b/tests/shell/testcases/sets/reset_command_0
@@ -2,7 +2,7 @@
 
 set -e
 
-trap '[[ $? -eq 0 ]] || echo FAIL' EXIT
+trap 'rc="$?"; [ "$rc" -ne 0 -a "$rc" -ne 77 ] && echo FAIL' EXIT
 
 RULESET="table t {
 	set s {
@@ -36,11 +36,21 @@ expires_minutes() {
 	sed -n 's/.*expires \([0-9]*\)m.*/\1/p'
 }
 
-echo -n "get set elem matches reset set elem: "
 elem='element t s { 1.0.0.1 . udp . 53 }'
-[[ $($NFT "get $elem ; reset $elem" | \
-	grep 'elements = ' | drop_seconds | uniq | wc -l) == 1 ]]
-echo OK
+
+rc=0
+OUT="$( $NFT "get $elem ; reset $elem" )" || rc=$?
+if [ "$rc" -ne 0 ] ; then
+	echo "Command \`nft \"get $elem ; reset $elem\"\` failed. Assume reset is not supported. SKIP"
+	exit 77
+fi
+
+[ "$(printf '%s\n' "$OUT" | \
+      grep 'elements = ' | \
+      drop_seconds | \
+      uniq | \
+      wc -l)" = 1 ] || die "Unexpected output getting elements: \`nft \"get $elem ; reset $elem\"\`"$'\nOutput\n>'"$OUT"'<'
+echo "get set elem matches reset set elem: OK"
 
 echo -n "counters and expiry are reset: "
 NEW=$($NFT "get $elem")
-- 
2.41.0

[PATCH nft 3/3] tests/shell: suggest 4Mb /proc/sys/net/core/{wmem_max,rmem_max} for rootless

[Thomas Haller]

2Mb was not enough to pass "tests/shell/testcases/sets/0030add_many_elements_interval_0"
in an unprivileged/rootless namespace.

Instead, bump the suggestion to 4Mb, which lets the test pass.

Note that the 4Mb are only the recommended value when running the test
as rootless, and is used to autodetect NFT_TEST_HAS_SOCKET_LIMITS=y.
You can set whatever values are suitable for your environment, and
explicitly indicate whether the limits are appropriate or not via
NFT_TEST_HAS_SOCKET_LIMITS=n|y.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/run-tests.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index 1527b2a6455c..d11b4a63b6d1 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -180,7 +180,7 @@ usage() {
 	echo "                 with rootless the test would fail. Tests will check for [ "\$NFT_TEST_HAS_SOCKET_LIMITS" = y ]"
 	echo "                 and skip. You may set NFT_TEST_HAS_SOCKET_LIMITS=n if you ensure those limits are"
 	echo "                 suitable to run the test rootless. Otherwise will be autodetected."
-	echo "                 Set /proc/sys/net/core/{wmem_max,rmem_max} to at least 2MB to get them to pass automatically."
+	echo "                 Set /proc/sys/net/core/{wmem_max,rmem_max} to at least 4MB to get them to pass automatically."
 	echo " NFT_TEST_UNSHARE_CMD=cmd : when set, this is the command line for an unshare"
 	echo "                 command, which is used to sandbox each test invocation. By"
 	echo "                 setting it to empty, no unsharing is done."
@@ -391,8 +391,8 @@ export NFT_TEST_HAS_REALROOT
 if [ "$NFT_TEST_HAS_SOCKET_LIMITS" = "" ] ; then
 	if [ "$NFT_TEST_HAS_REALROOT" = y ] ; then
 		NFT_TEST_HAS_SOCKET_LIMITS=n
-	elif [ "$(cat /proc/sys/net/core/wmem_max 2>/dev/null)" -ge $((2000*1024)) ] 2>/dev/null && \
-	     [ "$(cat /proc/sys/net/core/rmem_max 2>/dev/null)" -ge $((2000*1024)) ] 2>/dev/null ; then
+	elif [ "$(cat /proc/sys/net/core/wmem_max 2>/dev/null)" -ge $((4000*1024)) ] 2>/dev/null && \
+	     [ "$(cat /proc/sys/net/core/rmem_max 2>/dev/null)" -ge $((4000*1024)) ] 2>/dev/null ; then
 		NFT_TEST_HAS_SOCKET_LIMITS=n
 	else
 		NFT_TEST_HAS_SOCKET_LIMITS=y
-- 
2.41.0

Re: [PATCH nft 2/3] tests/shell: skip "sets/reset_command_0" on unsupported reset command

[Thomas Haller]

This patch (#2 of 3) should be dropped. Don't apply.

It will be solved differently by a patch from Florian.


Thomas


On Fri, 2023-09-15 at 17:54 +0200, Thomas Haller wrote:
> The NFT_MSG_GETSETELEM_RESET command was only added to kernel
> v6.4-rc3-764-g079cd633219d. Also, it doesn't work on Fedora 38
> (6.4.14-200.fc38.x86_64), although that would appear to have the
> feature. On CentOS-Stream-9 (5.14.0-354.el9.x86_64) the test passes.
> 
> Note that this is not implemented via a re-usable feature detection.
> Instead, we just in the middle of the test notice that it appears not
> to
> work, and abort (skip).
> 
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=079cd633219d7298d087cd115c17682264244c18
> 
> Signed-off-by: Thomas Haller <thaller@redhat.com>
> ---
>  tests/shell/testcases/sets/reset_command_0 | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/shell/testcases/sets/reset_command_0
> b/tests/shell/testcases/sets/reset_command_0
> index ad2e16a7d274..a0f5ca017b0f 100755
> --- a/tests/shell/testcases/sets/reset_command_0
> +++ b/tests/shell/testcases/sets/reset_command_0
> @@ -2,7 +2,7 @@
>  
>  set -e
>  
> -trap '[[ $? -eq 0 ]] || echo FAIL' EXIT
> +trap 'rc="$?"; [ "$rc" -ne 0 -a "$rc" -ne 77 ] && echo FAIL' EXIT
>  
>  RULESET="table t {
>         set s {
> @@ -36,11 +36,21 @@ expires_minutes() {
>         sed -n 's/.*expires \([0-9]*\)m.*/\1/p'
>  }
>  
> -echo -n "get set elem matches reset set elem: "
>  elem='element t s { 1.0.0.1 . udp . 53 }'
> -[[ $($NFT "get $elem ; reset $elem" | \
> -       grep 'elements = ' | drop_seconds | uniq | wc -l) == 1 ]]
> -echo OK
> +
> +rc=0
> +OUT="$( $NFT "get $elem ; reset $elem" )" || rc=$?
> +if [ "$rc" -ne 0 ] ; then
> +       echo "Command \`nft \"get $elem ; reset $elem\"\` failed.
> Assume reset is not supported. SKIP"
> +       exit 77
> +fi
> +
> +[ "$(printf '%s\n' "$OUT" | \
> +      grep 'elements = ' | \
> +      drop_seconds | \
> +      uniq | \
> +      wc -l)" = 1 ] || die "Unexpected output getting elements:
> \`nft \"get $elem ; reset $elem\"\`"$'\nOutput\n>'"$OUT"'<'
> +echo "get set elem matches reset set elem: OK"
>  
>  echo -n "counters and expiry are reset: "
>  NEW=$($NFT "get $elem")