* [PATCH nft 1/5] tests: add feature probing
2023-09-04 9:06 [PATCH nft 0/5] tests: shell: add and use feature probing Florian Westphal
@ 2023-09-04 9:06 ` Florian Westphal
2023-09-05 13:00 ` Phil Sutter
2023-09-06 14:36 ` Thomas Haller
2023-09-04 9:06 ` [PATCH nft 2/5] tests: shell: let netdev_chain_0 test indicate SKIP if kernel requires netdev device Florian Westphal
` (3 subsequent siblings)
4 siblings, 2 replies; 16+ messages in thread
From: Florian Westphal @ 2023-09-04 9:06 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
Running selftests on older kernels makes some of them fail very early
because some tests use features that are not available on older
kernels, e.g. -stable releases.
Known examples:
- inner header matching
- anonymous chains
- elem delete from packet path
Also, some test cases might fail because a feature isn't
compiled in, such as netdev chains for example.
This adds a feature-probing to the shell tests.
Simply drop a 'nft -f' compatible file with a .nft suffix into
tests/shell/features.
run-tests.sh will load it via --check and will add
NFT_TESTS_HAVE_${filename}=$?
to the environment.
The test script can then either elide a part of the test
or replace it with a supported feature subset.
This adds the probing skeleton, a probe file for chain_binding
and alters 30s-stress to suppress anonon chains when its unuspported.
Note that 30s-stress is optionally be run standalone, so this adds
more code than needed, for tests that always run via run-tests.sh
its enough to do
[ $NFT_HAVE_chain_binding -eq 1 ] && test_chain_binding
Signed-off-by: Florian Westphal <fw@strlen.de>
---
tests/shell/features/chain_binding.nft | 7 +++
tests/shell/run-tests.sh | 24 +++++++++
tests/shell/testcases/transactions/30s-stress | 52 ++++++++++++++++---
3 files changed, 76 insertions(+), 7 deletions(-)
create mode 100644 tests/shell/features/chain_binding.nft
diff --git a/tests/shell/features/chain_binding.nft b/tests/shell/features/chain_binding.nft
new file mode 100644
index 000000000000..b381ec540fae
--- /dev/null
+++ b/tests/shell/features/chain_binding.nft
@@ -0,0 +1,7 @@
+# d0e2c7de92c7 ("netfilter: nf_tables: add NFT_CHAIN_BINDING")
+# v5.9-rc1~133^2~302^2~1
+table ip t {
+ chain c {
+ jump { counter; }
+ }
+}
diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index b66ef4fa4d1f..3113404de2b9 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -163,6 +163,24 @@ ok=0
failed=0
taint=0
+check_features()
+{
+ for ffilename in $TESTDIR/../features/*.nft; do
+ feature=$(basename $ffilename)
+
+ feature=${feature#*/}
+ feature=${feature%*.nft}
+
+ eval NFT_HAVE_${feature}=0
+ $NFT --check -f "$ffilename" 2>/dev/null
+ if [ $? -eq 0 ]; then
+ eval NFT_HAVE_${feature}=1
+ fi
+
+ export NFT_HAVE_${feature}
+ done
+}
+
check_taint()
{
read taint_now < /proc/sys/kernel/tainted
@@ -211,6 +229,7 @@ check_kmemleak()
fi
}
+check_features
check_taint
for testfile in $(find_tests)
@@ -277,5 +296,10 @@ check_kmemleak_force
msg_info "results: [OK] $ok [FAILED] $failed [TOTAL] $((ok+failed))"
+if [ "$VERBOSE" == "y" ] ; then
+ echo "Probed Features:"
+ env | grep NFT_HAVE_
+fi
+
kernel_cleanup
[ "$failed" -eq 0 ]
diff --git a/tests/shell/testcases/transactions/30s-stress b/tests/shell/testcases/transactions/30s-stress
index 4d3317e22b0c..924e7e28f97e 100755
--- a/tests/shell/testcases/transactions/30s-stress
+++ b/tests/shell/testcases/transactions/30s-stress
@@ -16,6 +16,18 @@ if [ x = x"$NFT" ] ; then
NFT=nft
fi
+if [ -z $NFT_HAVE_chain_binding ];then
+ NFT_HAVE_chain_binding=0
+ mydir=$(dirname $0)
+ $NFT --check -f $mydir/../../features/chain_binding.nft
+ if [ $? -eq 0 ];then
+ NFT_HAVE_chain_binding=1
+ else
+ echo "Assuming anonymous chains are not supported"
+ fi
+
+fi
+
testns=testns-$(mktemp -u "XXXXXXXX")
tmp=""
@@ -31,8 +43,8 @@ failslab_defaults() {
# allow all slabs to fail (if process is tagged).
find /sys/kernel/slab/ -wholename '*/kmalloc-[0-9]*/failslab' -type f -exec sh -c 'echo 1 > {}' \;
- # no limit on the number of failures
- echo -1 > /sys/kernel/debug/failslab/times
+ # no limit on the number of failures, or clause works around old kernels that reject negative integer.
+ echo -1 > /sys/kernel/debug/failslab/times 2>/dev/null || printf '%#x -1' > /sys/kernel/debug/failslab/times
# Set to 2 for full dmesg traces for each injected error
echo 0 > /sys/kernel/debug/failslab/verbose
@@ -91,6 +103,15 @@ nft_with_fault_inject()
trap cleanup EXIT
tmp=$(mktemp)
+jump_or_goto()
+{
+ if [ $((RANDOM & 1)) -eq 0 ] ;then
+ echo -n "jump"
+ else
+ echo -n "goto"
+ fi
+}
+
random_verdict()
{
max="$1"
@@ -102,7 +123,8 @@ random_verdict()
rnd=$((RANDOM%max))
if [ $rnd -gt 0 ];then
- printf "jump chain%03u" "$((rnd+1))"
+ jump_or_goto
+ printf " chain%03u" "$((rnd+1))"
return
fi
@@ -411,6 +433,21 @@ stress_all()
randmonitor &
}
+gen_anon_chain_jump()
+{
+ echo -n "insert rule inet $@ "
+ jump_or_goto
+
+ if [ $NFT_HAVE_chain_binding -ne 1 ];then
+ echo " defaultchain"
+ return
+ fi
+
+ echo -n " { "
+ jump_or_goto
+ echo " defaultchain; counter; }"
+}
+
gen_ruleset() {
echo > "$tmp"
for table in $tables; do
@@ -452,12 +489,13 @@ for table in $tables; do
echo "insert rule inet $table $chain ip6 saddr { ::1, dead::beef } counter" comment hash >> "$tmp"
echo "insert rule inet $table $chain ip saddr { 1.2.3.4 - 5.6.7.8, 127.0.0.1 } comment rbtree" >> "$tmp"
# bitmap 1byte, with anon chain jump
- echo "insert rule inet $table $chain ip protocol { 6, 17 } jump { jump defaultchain; counter; }" >> "$tmp"
+ gen_anon_chain_jump "$table $chain ip protocol { 6, 17 }" >> "$tmp"
+
# bitmap 2byte
echo "insert rule inet $table $chain tcp dport != { 22, 23, 80 } goto defaultchain" >> "$tmp"
echo "insert rule inet $table $chain tcp dport { 1-1024, 8000-8080 } jump defaultchain comment rbtree" >> "$tmp"
# pipapo (concat + set), with goto anonymous chain.
- echo "insert rule inet $table $chain ip saddr . tcp dport { 1.2.3.4 . 1-1024, 1.2.3.6 - 1.2.3.10 . 8000-8080, 1.2.3.4 . 8080, 1.2.3.6 - 1.2.3.10 . 22 } goto { jump defaultchain; counter; }" >> "$tmp"
+ gen_anon_chain_jump "$table $chain ip saddr . tcp dport { 1.2.3.4 . 1-1024, 1.2.3.6 - 1.2.3.10 . 8000-8080, 1.2.3.4 . 8080, 1.2.3.6 - 1.2.3.10 . 22 }" >> "$tmp"
# add a few anonymous sets. rhashtable is convered by named sets below.
c=$((RANDOM%$count))
@@ -466,12 +504,12 @@ for table in $tables; do
echo "insert rule inet $table $chain ip6 saddr { ::1, dead::beef } counter" comment hash >> "$tmp"
echo "insert rule inet $table $chain ip saddr { 1.2.3.4 - 5.6.7.8, 127.0.0.1 } comment rbtree" >> "$tmp"
# bitmap 1byte, with anon chain jump
- echo "insert rule inet $table $chain ip protocol { 6, 17 } jump { jump defaultchain; counter; }" >> "$tmp"
+ gen_anon_chain_jump "$table $chain ip protocol { 6, 17 }" >> "$tmp"
# bitmap 2byte
echo "insert rule inet $table $chain tcp dport != { 22, 23, 80 } goto defaultchain" >> "$tmp"
echo "insert rule inet $table $chain tcp dport { 1-1024, 8000-8080 } jump defaultchain comment rbtree" >> "$tmp"
# pipapo (concat + set), with goto anonymous chain.
- echo "insert rule inet $table $chain ip saddr . tcp dport { 1.2.3.4 . 1-1024, 1.2.3.6 - 1.2.3.10 . 8000-8080, 1.2.3.4 . 8080, 1.2.3.6 - 1.2.3.10 . 22 } goto { jump defaultchain; counter; }" >> "$tmp"
+ gen_anon_chain_jump "$table $chain ip saddr . tcp dport { 1.2.3.4 . 1-1024, 1.2.3.6 - 1.2.3.10 . 8000-8080, 1.2.3.4 . 8080, 1.2.3.6 - 1.2.3.10 . 22 }" >> "$tmp"
# add constant/immutable sets
size=$((RANDOM%5120000))
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH nft 1/5] tests: add feature probing
2023-09-04 9:06 ` [PATCH nft 1/5] tests: add " Florian Westphal
@ 2023-09-05 13:00 ` Phil Sutter
2023-09-05 13:44 ` Florian Westphal
2023-09-06 5:17 ` Thomas Haller
2023-09-06 14:36 ` Thomas Haller
1 sibling, 2 replies; 16+ messages in thread
From: Phil Sutter @ 2023-09-05 13:00 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel
On Mon, Sep 04, 2023 at 11:06:30AM +0200, Florian Westphal wrote:
> Running selftests on older kernels makes some of them fail very early
> because some tests use features that are not available on older
> kernels, e.g. -stable releases.
>
> Known examples:
> - inner header matching
> - anonymous chains
> - elem delete from packet path
>
> Also, some test cases might fail because a feature isn't
> compiled in, such as netdev chains for example.
>
> This adds a feature-probing to the shell tests.
>
> Simply drop a 'nft -f' compatible file with a .nft suffix into
> tests/shell/features.
>
> run-tests.sh will load it via --check and will add
>
> NFT_TESTS_HAVE_${filename}=$?
Maybe make this:
| truefalse=(true false)
| NFT_TESTS_HAVE_${filename}=${truefalse[$?]}
[...]
> [ $NFT_HAVE_chain_binding -eq 1 ] && test_chain_binding
So this becomes:
| $NFT_HAVE_chain_binding && test_chain_binding
Use of true/false appears to work in dash, so might be POSIX sh
compatible?
Cheers, Phil
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH nft 1/5] tests: add feature probing
2023-09-05 13:00 ` Phil Sutter
@ 2023-09-05 13:44 ` Florian Westphal
2023-09-05 14:01 ` Phil Sutter
2023-09-06 5:17 ` Thomas Haller
1 sibling, 1 reply; 16+ messages in thread
From: Florian Westphal @ 2023-09-05 13:44 UTC (permalink / raw)
To: Phil Sutter, Florian Westphal, netfilter-devel
Phil Sutter <phil@nwl.cc> wrote:
> On Mon, Sep 04, 2023 at 11:06:30AM +0200, Florian Westphal wrote:
> > Running selftests on older kernels makes some of them fail very early
> > because some tests use features that are not available on older
> > kernels, e.g. -stable releases.
> >
> > Known examples:
> > - inner header matching
> > - anonymous chains
> > - elem delete from packet path
> >
> > Also, some test cases might fail because a feature isn't
> > compiled in, such as netdev chains for example.
> >
> > This adds a feature-probing to the shell tests.
> >
> > Simply drop a 'nft -f' compatible file with a .nft suffix into
> > tests/shell/features.
> >
> > run-tests.sh will load it via --check and will add
> >
> > NFT_TESTS_HAVE_${filename}=$?
>
> Maybe make this:
>
> | truefalse=(true false)
> | NFT_TESTS_HAVE_${filename}=${truefalse[$?]}
>
> [...]
>
> > [ $NFT_HAVE_chain_binding -eq 1 ] && test_chain_binding
>
> So this becomes:
>
> | $NFT_HAVE_chain_binding && test_chain_binding
>
> Use of true/false appears to work in dash, so might be POSIX sh
> compatible?
Can do that, but if [ false ] evaluates to true...
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH nft 1/5] tests: add feature probing
2023-09-05 13:44 ` Florian Westphal
@ 2023-09-05 14:01 ` Phil Sutter
2023-09-05 14:09 ` Florian Westphal
0 siblings, 1 reply; 16+ messages in thread
From: Phil Sutter @ 2023-09-05 14:01 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel
On Tue, Sep 05, 2023 at 03:44:06PM +0200, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > On Mon, Sep 04, 2023 at 11:06:30AM +0200, Florian Westphal wrote:
> > > Running selftests on older kernels makes some of them fail very early
> > > because some tests use features that are not available on older
> > > kernels, e.g. -stable releases.
> > >
> > > Known examples:
> > > - inner header matching
> > > - anonymous chains
> > > - elem delete from packet path
> > >
> > > Also, some test cases might fail because a feature isn't
> > > compiled in, such as netdev chains for example.
> > >
> > > This adds a feature-probing to the shell tests.
> > >
> > > Simply drop a 'nft -f' compatible file with a .nft suffix into
> > > tests/shell/features.
> > >
> > > run-tests.sh will load it via --check and will add
> > >
> > > NFT_TESTS_HAVE_${filename}=$?
> >
> > Maybe make this:
> >
> > | truefalse=(true false)
> > | NFT_TESTS_HAVE_${filename}=${truefalse[$?]}
> >
> > [...]
> >
> > > [ $NFT_HAVE_chain_binding -eq 1 ] && test_chain_binding
> >
> > So this becomes:
> >
> > | $NFT_HAVE_chain_binding && test_chain_binding
> >
> > Use of true/false appears to work in dash, so might be POSIX sh
> > compatible?
>
> Can do that, but if [ false ] evaluates to true...
Sure, because that's a short-cut for '[ -n false ]'. In what context is
that problematic?
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH nft 1/5] tests: add feature probing
2023-09-05 14:01 ` Phil Sutter
@ 2023-09-05 14:09 ` Florian Westphal
2023-09-05 20:28 ` Phil Sutter
0 siblings, 1 reply; 16+ messages in thread
From: Florian Westphal @ 2023-09-05 14:09 UTC (permalink / raw)
To: Phil Sutter, Florian Westphal, netfilter-devel
Phil Sutter <phil@nwl.cc> wrote:
> Sure, because that's a short-cut for '[ -n false ]'. In what context is
> that problematic?
if [ $HAVE_NFT_foo ] ; then ...
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH nft 1/5] tests: add feature probing
2023-09-05 14:09 ` Florian Westphal
@ 2023-09-05 20:28 ` Phil Sutter
0 siblings, 0 replies; 16+ messages in thread
From: Phil Sutter @ 2023-09-05 20:28 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel
On Tue, Sep 05, 2023 at 04:09:20PM +0200, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > Sure, because that's a short-cut for '[ -n false ]'. In what context is
> > that problematic?
>
> if [ $HAVE_NFT_foo ] ; then ...
Same as if $HAVE_NFT_foo was either 0 or 1?! Obviously, with variables
holding the string "true" or "false", one has to test them either via:
| if $var; then ...
or
| if [ $var == true ]; then ...
I just find code more straightforward which does "if $have_foo; then
..." instead of "if [ $have_foo -ne 1 ]; then ...". The latter makes me
question whether that 1 is positive (as with C) or negative (as with
shell) and whether there are more possible values than two and any but 1
are OK.
Cheers, Phil
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH nft 1/5] tests: add feature probing
2023-09-05 13:00 ` Phil Sutter
2023-09-05 13:44 ` Florian Westphal
@ 2023-09-06 5:17 ` Thomas Haller
1 sibling, 0 replies; 16+ messages in thread
From: Thomas Haller @ 2023-09-06 5:17 UTC (permalink / raw)
To: Phil Sutter, Florian Westphal; +Cc: netfilter-devel
On Tue, 2023-09-05 at 15:00 +0200, Phil Sutter wrote:
> On Mon, Sep 04, 2023 at 11:06:30AM +0200, Florian Westphal wrote:
> > Running selftests on older kernels makes some of them fail very
> > early
> > because some tests use features that are not available on older
> > kernels, e.g. -stable releases.
> >
> > Known examples:
> > - inner header matching
> > - anonymous chains
> > - elem delete from packet path
> >
> > Also, some test cases might fail because a feature isn't
> > compiled in, such as netdev chains for example.
> >
> > This adds a feature-probing to the shell tests.
> >
> > Simply drop a 'nft -f' compatible file with a .nft suffix into
> > tests/shell/features.
> >
> > run-tests.sh will load it via --check and will add
> >
> > NFT_TESTS_HAVE_${filename}=$?
>
> Maybe make this:
>
> > truefalse=(true false)
> > NFT_TESTS_HAVE_${filename}=${truefalse[$?]}
>
> [...]
>
> > [ $NFT_HAVE_chain_binding -eq 1 ] && test_chain_binding
>
> So this becomes:
>
> > $NFT_HAVE_chain_binding && test_chain_binding
>
> Use of true/false appears to work in dash, so might be POSIX sh
> compatible?
>
> Cheers, Phil
>
That's possible. But I would not do such non-obvious magic.
The existing variables like VERBOSE,VALGRIND,DUMPGEN use "y" for true
and everything else is false. I'd stick with that form.
The form would be (exactly):
if [ "$NFT_TESTS_HAVE_chain_binding" = y ] ; then
Btw, as a matter of principle, I think that all variables in the patch
set should be quoted.
Thomas
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH nft 1/5] tests: add feature probing
2023-09-04 9:06 ` [PATCH nft 1/5] tests: add " Florian Westphal
2023-09-05 13:00 ` Phil Sutter
@ 2023-09-06 14:36 ` Thomas Haller
1 sibling, 0 replies; 16+ messages in thread
From: Thomas Haller @ 2023-09-06 14:36 UTC (permalink / raw)
To: Florian Westphal, netfilter-devel
On Mon, 2023-09-04 at 11:06 +0200, Florian Westphal wrote:
> Running selftests on older kernels makes some of them fail very early
> because some tests use features that are not available on older
> kernels, e.g. -stable releases.
>
> Known examples:
> - inner header matching
> - anonymous chains
> - elem delete from packet path
>
> Also, some test cases might fail because a feature isn't
> compiled in, such as netdev chains for example.
>
> This adds a feature-probing to the shell tests.
On my Fedora 38, I have currently 7 tests failing.
With this patchset, tests/shell/testcases/maps/typeof_maps_add_delete
is now skipped (6 failing left).
Nice!
> +check_features()
> +{
> + for ffilename in $TESTDIR/../features/*.nft; do
> + feature=$(basename $ffilename)
> +
> + feature=${feature#*/}
> + feature=${feature%*.nft}
> +
> + eval NFT_HAVE_${feature}=0
> + $NFT --check -f "$ffilename" 2>/dev/null
> + if [ $? -eq 0 ]; then
> + eval NFT_HAVE_${feature}=1
the existing variables like VERBOSE,VALGRIND use "y" for true and
everything else is false.
I think 0|1 looks better. But it should be consistent. If 0|1 is used,
the other variables should be adjusted.
Note that on the other branch I added normalization functions
bool_y()/bool_n() to accept values like true/1/y/yes from the user, and
normalize to y|n. This could be changed internally to 1|0 without
breaking user setups.
> for testfile in $(find_tests)
> @@ -277,5 +296,10 @@ check_kmemleak_force
>
> msg_info "results: [OK] $ok [FAILED] $failed [TOTAL] $((ok+failed))"
>
> +if [ "$VERBOSE" == "y" ] ; then
> + echo "Probed Features:"
> + env | grep NFT_HAVE_
> +fi
xxx=$'\nNFT_HAVE_XXXXX=bogus' ./tests/shell/run-tests.sh /bin/true -v
gives the wrong output. Could be instead:
for v in $(compgen -v | grep '^NFT_HAVE_') ; do
echo "$v=${!v}";
done
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH nft 2/5] tests: shell: let netdev_chain_0 test indicate SKIP if kernel requires netdev device
2023-09-04 9:06 [PATCH nft 0/5] tests: shell: add and use feature probing Florian Westphal
2023-09-04 9:06 ` [PATCH nft 1/5] tests: add " Florian Westphal
@ 2023-09-04 9:06 ` Florian Westphal
2023-09-05 13:03 ` Phil Sutter
2023-09-06 13:42 ` Thomas Haller
2023-09-04 9:06 ` [PATCH nft 3/5] tests: shell: typeof_integer/raw: prefer @nh for payload matching Florian Westphal
` (2 subsequent siblings)
4 siblings, 2 replies; 16+ messages in thread
From: Florian Westphal @ 2023-09-04 9:06 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
This test case only works on kernel 6.4+.
Add feature probe for this and then exit early.
We don't want to indicate a test failure, as this test doesn't apply
on older kernels.
But we should not indicate sucess either, else we might be fooled
in case something went wrong during feature probe.
Add a special return value, 123, and let run-tests.sh count this
as 'SKIPPED'.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
tests/shell/features/netdev_chain_without_device.nft | 7 +++++++
tests/shell/run-tests.sh | 11 ++++++++++-
tests/shell/testcases/chains/netdev_chain_0 | 2 ++
3 files changed, 19 insertions(+), 1 deletion(-)
create mode 100644 tests/shell/features/netdev_chain_without_device.nft
diff --git a/tests/shell/features/netdev_chain_without_device.nft b/tests/shell/features/netdev_chain_without_device.nft
new file mode 100644
index 000000000000..25eb200ffe31
--- /dev/null
+++ b/tests/shell/features/netdev_chain_without_device.nft
@@ -0,0 +1,7 @@
+# 207296f1a03b ("netfilter: nf_tables: allow to create netdev chain without device")
+# v6.4-rc1~132^2~14^2
+table netdev t {
+ chain c {
+ type filter hook ingress priority 0; policy accept;
+ }
+}
diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index 3113404de2b9..17cab3f11c9b 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -161,6 +161,7 @@ fi
echo ""
ok=0
failed=0
+skipped=0
taint=0
check_features()
@@ -270,6 +271,9 @@ do
msg_warn "[DUMP FAIL] $testfile"
fi
fi
+ elif [ "$rc_got" -eq 123 ]; then
+ ((skipped++))
+ msg_info "[SKIPPED] $testfile"
else
((failed++))
if [ "$VERBOSE" == "y" ] ; then
@@ -294,7 +298,12 @@ echo ""
kmemleak_found=0
check_kmemleak_force
-msg_info "results: [OK] $ok [FAILED] $failed [TOTAL] $((ok+failed))"
+msg_info "results: [OK] $ok [FAILED] $failed [SKIPPED] $skipped [TOTAL] $((ok+failed+skipped))"
+
+if [ $ok -eq 0 -a $failed -eq 0 ]; then
+ # no test cases were run, indicate a failure
+ failed=1
+fi
if [ "$VERBOSE" == "y" ] ; then
echo "Probed Features:"
diff --git a/tests/shell/testcases/chains/netdev_chain_0 b/tests/shell/testcases/chains/netdev_chain_0
index 67cd715fc59f..2e2a0c177fcb 100755
--- a/tests/shell/testcases/chains/netdev_chain_0
+++ b/tests/shell/testcases/chains/netdev_chain_0
@@ -1,5 +1,7 @@
#!/bin/bash
+[ "$NFT_HAVE_netdev_chain_without_device" -eq 0 ] && exit 123
+
ip link add d0 type dummy || {
echo "Skipping, no dummy interface available"
exit 0
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH nft 2/5] tests: shell: let netdev_chain_0 test indicate SKIP if kernel requires netdev device
2023-09-04 9:06 ` [PATCH nft 2/5] tests: shell: let netdev_chain_0 test indicate SKIP if kernel requires netdev device Florian Westphal
@ 2023-09-05 13:03 ` Phil Sutter
2023-09-06 13:42 ` Thomas Haller
1 sibling, 0 replies; 16+ messages in thread
From: Phil Sutter @ 2023-09-05 13:03 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel
On Mon, Sep 04, 2023 at 11:06:31AM +0200, Florian Westphal wrote:
> This test case only works on kernel 6.4+.
> Add feature probe for this and then exit early.
>
> We don't want to indicate a test failure, as this test doesn't apply
> on older kernels.
>
> But we should not indicate sucess either, else we might be fooled
> in case something went wrong during feature probe.
>
> Add a special return value, 123, and let run-tests.sh count this
> as 'SKIPPED'.
I suggest we adhere to Gnu automake convention:
"[...] an exit status of 0
from a test script will denote a success, an exit status of 77 a skipped
test, an exit status of 99 a hard error, and any other exit status will
denote a failure."[1]
Cheers, Phil
[1] https://www.gnu.org/software/automake/manual/html_node/Scripts_002dbased-Testsuites.html
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH nft 2/5] tests: shell: let netdev_chain_0 test indicate SKIP if kernel requires netdev device
2023-09-04 9:06 ` [PATCH nft 2/5] tests: shell: let netdev_chain_0 test indicate SKIP if kernel requires netdev device Florian Westphal
2023-09-05 13:03 ` Phil Sutter
@ 2023-09-06 13:42 ` Thomas Haller
1 sibling, 0 replies; 16+ messages in thread
From: Thomas Haller @ 2023-09-06 13:42 UTC (permalink / raw)
To: Florian Westphal, netfilter-devel
On Mon, 2023-09-04 at 11:06 +0200, Florian Westphal wrote:
> This test case only works on kernel 6.4+.
> Add feature probe for this and then exit early.
>
> We don't want to indicate a test failure, as this test doesn't apply
> on older kernels.
>
> But we should not indicate sucess either, else we might be fooled
> in case something went wrong during feature probe.
>
> Add a special return value, 123, and let run-tests.sh count this
> as 'SKIPPED'.
>
[...]
> failed=0
> +skipped=0
> taint=0
>
> check_features()
> @@ -270,6 +271,9 @@ do
> msg_warn "[DUMP FAIL] $testfile"
> fi
> fi
> + elif [ "$rc_got" -eq 123 ]; then
> + ((skipped++))
> + msg_info "[SKIPPED] $testfile"
I agree with Phil, I think this should return 77.
Btw, I did a similar patch on
[PATCH nft v5 08/19] tests/shell: interpret an exit code of 77 from scripts as "skipped"
Granted, you send your first version with this patch/idea a few hours
before mine. I just point out the overlap...
> else
> ((failed++))
> if [ "$VERBOSE" == "y" ] ; then
> @@ -294,7 +298,12 @@ echo ""
> kmemleak_found=0
> check_kmemleak_force
>
> -msg_info "results: [OK] $ok [FAILED] $failed [TOTAL] $((ok+failed))"
> +msg_info "results: [OK] $ok [FAILED] $failed [SKIPPED] $skipped
> [TOTAL] $((ok+failed+skipped))"
> +
> +if [ $ok -eq 0 -a $failed -eq 0 ]; then
> + # no test cases were run, indicate a failure
> + failed=1
> +fi
I think this should be dropped.
When I run
./tests/shell/run-tests.sh tests/shell/testcases/maps/typeof_maps_add_delete
on my Fedora38, the test gets skipped. But I think the command should
just give me a success. What would I do about the "failure" anyway?
Thomas
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH nft 3/5] tests: shell: typeof_integer/raw: prefer @nh for payload matching
2023-09-04 9:06 [PATCH nft 0/5] tests: shell: add and use feature probing Florian Westphal
2023-09-04 9:06 ` [PATCH nft 1/5] tests: add " Florian Westphal
2023-09-04 9:06 ` [PATCH nft 2/5] tests: shell: let netdev_chain_0 test indicate SKIP if kernel requires netdev device Florian Westphal
@ 2023-09-04 9:06 ` Florian Westphal
2023-09-04 9:06 ` [PATCH nft 4/5] tests: shell: add and use feature probe for map query like a set Florian Westphal
2023-09-04 9:06 ` [PATCH nft 5/5] tests: shell skip inner matching tests if unsupported Florian Westphal
4 siblings, 0 replies; 16+ messages in thread
From: Florian Westphal @ 2023-09-04 9:06 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
@ih fails on kernels where payload expression doesn't support the 'inner'
base offset.
This test isn't about inner headers, so just use @nh which is
universally available.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
tests/shell/testcases/maps/dumps/typeof_integer_0.nft | 4 ++--
tests/shell/testcases/maps/dumps/typeof_raw_0.nft | 4 ++--
tests/shell/testcases/maps/typeof_integer_0 | 4 ++--
tests/shell/testcases/maps/typeof_raw_0 | 4 ++--
tests/shell/testcases/sets/dumps/typeof_raw_0.nft | 4 ++--
tests/shell/testcases/sets/typeof_raw_0 | 4 ++--
6 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/tests/shell/testcases/maps/dumps/typeof_integer_0.nft b/tests/shell/testcases/maps/dumps/typeof_integer_0.nft
index 330415574c95..19c24febffcc 100644
--- a/tests/shell/testcases/maps/dumps/typeof_integer_0.nft
+++ b/tests/shell/testcases/maps/dumps/typeof_integer_0.nft
@@ -13,8 +13,8 @@ table inet t {
}
chain c {
- udp length . @ih,32,32 vmap @m1
- udp length . @ih,32,32 vmap @m2
+ udp length . @nh,32,32 vmap @m1
+ udp length . @nh,32,32 vmap @m2
udp length . @th,160,128 vmap { 47-63 . 0xe373135363130333131303735353203 : accept }
}
}
diff --git a/tests/shell/testcases/maps/dumps/typeof_raw_0.nft b/tests/shell/testcases/maps/dumps/typeof_raw_0.nft
index e876425b2bc6..476169f2943b 100644
--- a/tests/shell/testcases/maps/dumps/typeof_raw_0.nft
+++ b/tests/shell/testcases/maps/dumps/typeof_raw_0.nft
@@ -7,7 +7,7 @@ table ip x {
}
chain y {
- ip saddr . @ih,32,32 vmap @y
- ip saddr . @ih,32,32 vmap { 4.4.4.4 . 0x34 : accept, 5.5.5.5 . 0x45 : drop }
+ ip saddr . @nh,32,32 vmap @y
+ ip saddr . @nh,32,32 vmap { 4.4.4.4 . 0x34 : accept, 5.5.5.5 . 0x45 : drop }
}
}
diff --git a/tests/shell/testcases/maps/typeof_integer_0 b/tests/shell/testcases/maps/typeof_integer_0
index d51510af9073..0deff5eef67b 100755
--- a/tests/shell/testcases/maps/typeof_integer_0
+++ b/tests/shell/testcases/maps/typeof_integer_0
@@ -13,8 +13,8 @@ EXPECTED="table inet t {
}
chain c {
- udp length . @ih,32,32 vmap @m1
- udp length . @ih,32,32 vmap @m2
+ udp length . @nh,32,32 vmap @m1
+ udp length . @nh,32,32 vmap @m2
udp length . @th,160,128 vmap { 47-63 . 0xe373135363130333131303735353203 : accept }
}
}"
diff --git a/tests/shell/testcases/maps/typeof_raw_0 b/tests/shell/testcases/maps/typeof_raw_0
index e3da7825cb7b..bcd2c6d8c502 100755
--- a/tests/shell/testcases/maps/typeof_raw_0
+++ b/tests/shell/testcases/maps/typeof_raw_0
@@ -7,8 +7,8 @@ EXPECTED="table ip x {
}
chain y {
- ip saddr . @ih,32,32 vmap @y
- ip saddr . @ih,32,32 vmap { 4.4.4.4 . 0x34 : accept, 5.5.5.5 . 0x45 : drop}
+ ip saddr . @nh,32,32 vmap @y
+ ip saddr . @nh,32,32 vmap { 4.4.4.4 . 0x34 : accept, 5.5.5.5 . 0x45 : drop}
}
}"
diff --git a/tests/shell/testcases/sets/dumps/typeof_raw_0.nft b/tests/shell/testcases/sets/dumps/typeof_raw_0.nft
index 499ff167f51d..4d6abaaa151b 100644
--- a/tests/shell/testcases/sets/dumps/typeof_raw_0.nft
+++ b/tests/shell/testcases/sets/dumps/typeof_raw_0.nft
@@ -6,7 +6,7 @@ table inet t {
}
chain y {
- ip saddr . @ih,32,32 { 1.1.1.1 . 0x14, 2.2.2.2 . 0x1e }
- ip daddr . @ih,32,32 @y
+ ip saddr . @nh,32,32 { 1.1.1.1 . 0x14, 2.2.2.2 . 0x1e }
+ ip daddr . @nh,32,32 @y
}
}
diff --git a/tests/shell/testcases/sets/typeof_raw_0 b/tests/shell/testcases/sets/typeof_raw_0
index 36396b5c2e1d..66042eb4085a 100755
--- a/tests/shell/testcases/sets/typeof_raw_0
+++ b/tests/shell/testcases/sets/typeof_raw_0
@@ -7,8 +7,8 @@ EXPECTED="table inet t {
}
chain y {
- ip saddr . @ih,32,32 { 1.1.1.1 . 0x14, 2.2.2.2 . 0x1e }
- ip daddr . @ih,32,32 @y
+ ip saddr . @nh,32,32 { 1.1.1.1 . 0x14, 2.2.2.2 . 0x1e }
+ ip daddr . @nh,32,32 @y
}
}"
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH nft 4/5] tests: shell: add and use feature probe for map query like a set
2023-09-04 9:06 [PATCH nft 0/5] tests: shell: add and use feature probing Florian Westphal
` (2 preceding siblings ...)
2023-09-04 9:06 ` [PATCH nft 3/5] tests: shell: typeof_integer/raw: prefer @nh for payload matching Florian Westphal
@ 2023-09-04 9:06 ` Florian Westphal
2023-09-06 14:39 ` Thomas Haller
2023-09-04 9:06 ` [PATCH nft 5/5] tests: shell skip inner matching tests if unsupported Florian Westphal
4 siblings, 1 reply; 16+ messages in thread
From: Florian Westphal @ 2023-09-04 9:06 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
On recent kernels one can perform a lookup in a map without a destination
register (i.e., treat the map like a set -- pure existence check).
Add a feature probe and work around the missing feature in
typeof_maps_add_delete: do the test with a simplified ruleset,
Indicate skipped even though a reduced test was run (earlier errors
cause a failure) to not trigger dump validation error.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
tests/shell/features/map_lookup.nft | 11 ++++++
.../testcases/maps/typeof_maps_add_delete | 35 ++++++++++++++-----
2 files changed, 38 insertions(+), 8 deletions(-)
create mode 100644 tests/shell/features/map_lookup.nft
diff --git a/tests/shell/features/map_lookup.nft b/tests/shell/features/map_lookup.nft
new file mode 100644
index 000000000000..06c4c9d9c82d
--- /dev/null
+++ b/tests/shell/features/map_lookup.nft
@@ -0,0 +1,11 @@
+# a4878eeae390 ("netfilter: nf_tables: relax set/map validation checks")
+# v6.5-rc1~163^2~256^2~8
+table ip t {
+ map m {
+ typeof ip daddr : meta mark
+ }
+
+ chain c {
+ ip saddr @m
+ }
+}
diff --git a/tests/shell/testcases/maps/typeof_maps_add_delete b/tests/shell/testcases/maps/typeof_maps_add_delete
index 341de538e90e..579194b03372 100755
--- a/tests/shell/testcases/maps/typeof_maps_add_delete
+++ b/tests/shell/testcases/maps/typeof_maps_add_delete
@@ -1,6 +1,15 @@
#!/bin/bash
-EXPECTED='table ip dynset {
+CONDMATCH="ip saddr @dynmark"
+NCONDMATCH="ip saddr != @dynmark"
+
+# use reduced feature set
+if [ $NFT_HAVE_map_lookup -eq 0 ] ;then
+ CONDMATCH=""
+ NCONDMATCH=""
+fi
+
+EXPECTED="table ip dynset {
map dynmark {
typeof ip daddr : meta mark
counter
@@ -9,20 +18,20 @@ EXPECTED='table ip dynset {
}
chain test_ping {
- ip saddr @dynmark counter comment "should not increment"
- ip saddr != @dynmark add @dynmark { ip saddr : 0x1 } counter
- ip saddr @dynmark counter comment "should increment"
- ip saddr @dynmark delete @dynmark { ip saddr : 0x1 }
- ip saddr @dynmark counter comment "delete should be instant but might fail under memory pressure"
+ $CONDMATCH counter comment \"should not increment\"
+ $NCONDMATCH add @dynmark { ip saddr : 0x1 } counter
+ $CONDMATCH counter comment \"should increment\"
+ $CONDMATCH delete @dynmark { ip saddr : 0x1 }
+ $CONDMATCH counter comment \"delete should be instant but might fail under memory pressure\"
}
chain input {
type filter hook input priority 0; policy accept;
- add @dynmark { 10.2.3.4 timeout 1s : 0x2 } comment "also check timeout-gc"
+ add @dynmark { 10.2.3.4 timeout 1s : 0x2 } comment \"also check timeout-gc\"
meta l4proto icmp ip daddr 127.0.0.42 jump test_ping
}
-}'
+}"
set -e
$NFT -f - <<< $EXPECTED
@@ -31,5 +40,15 @@ $NFT list ruleset
ip link set lo up
ping -c 1 127.0.0.42
+$NFT get element ip dynset dynmark { 10.2.3.4 }
+
# wait so that 10.2.3.4 times out.
sleep 2
+
+set +e
+$NFT get element ip dynset dynmark { 10.2.3.4 } && exit 1
+
+# success, but indicate skip for reduced test to avoid dump validation error
+if [ $NFT_HAVE_map_lookup -eq 0 ];then
+ exit 123
+fi
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH nft 5/5] tests: shell skip inner matching tests if unsupported
2023-09-04 9:06 [PATCH nft 0/5] tests: shell: add and use feature probing Florian Westphal
` (3 preceding siblings ...)
2023-09-04 9:06 ` [PATCH nft 4/5] tests: shell: add and use feature probe for map query like a set Florian Westphal
@ 2023-09-04 9:06 ` Florian Westphal
4 siblings, 0 replies; 16+ messages in thread
From: Florian Westphal @ 2023-09-04 9:06 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
Signed-off-by: Florian Westphal <fw@strlen.de>
---
tests/shell/features/inner_matching.nft | 7 +++++++
tests/shell/testcases/sets/inner_0 | 2 ++
2 files changed, 9 insertions(+)
create mode 100644 tests/shell/features/inner_matching.nft
diff --git a/tests/shell/features/inner_matching.nft b/tests/shell/features/inner_matching.nft
new file mode 100644
index 000000000000..6c86fd3558ac
--- /dev/null
+++ b/tests/shell/features/inner_matching.nft
@@ -0,0 +1,7 @@
+# 3a07327d10a0 ("netfilter: nft_inner: support for inner tunnel header matching")
+# v6.2-rc1~99^2~350^2~4
+table ip t {
+ chain c {
+ udp dport 4789 vxlan ip saddr 1.2.3.4
+ }
+}
diff --git a/tests/shell/testcases/sets/inner_0 b/tests/shell/testcases/sets/inner_0
index 0eb172a8cf06..ceb35115cc7d 100755
--- a/tests/shell/testcases/sets/inner_0
+++ b/tests/shell/testcases/sets/inner_0
@@ -1,5 +1,7 @@
#!/bin/bash
+[ $NFT_HAVE_inner_matching -eq 0 ] && exit 123
+
set -e
RULESET="table netdev x {
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread