selftests: netfilter: nft_concat_range.sh: prefer per element counters for testing

The selftest uses following rule:
  ... @test counter name "test"

Then sends a packet, then checks if the named counter did increment or
not.

This is fine for the 'no-match' test case: If anything matches the
counter increments and the test fails as expected.

But for the 'should match' test cases this isn't optimal.
Consider buggy matching, where the packet matches entry x, but it
should have matched entry y.

In that case the test would erronously pass.

Rework the selftest to use per-element counters to avoid this.

After sending packet that should have matched entry x, query the
relevant element via 'nft reset element' and check that its counter
had incremented.

The 'nomatch' case isn't altered, no entry should match so the named
counter must be 0, changing it to the per-element counter would then
pass if another entry matches.

The downside of this change is a slight increase in test run-time by
a few seconds.

Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/tools/testing/selftests/net/netfilter/nft_concat_range.sh b/tools/testing/selftests/net/netfilter/nft_concat_range.sh
index efea93c..86b8ce7 100755 (executable)
--- a/tools/testing/selftests/net/netfilter/nft_concat_range.sh
+++ b/tools/testing/selftests/net/netfilter/nft_concat_range.sh
@@ -419,6 +419,7 @@ table inet filter {
 
        set test {
                type ${type_spec}
+               counter
                flags interval,timeout
        }
 
@@ -1158,8 +1159,17 @@ del() {
        fi
 }
 
-# Return packet count from 'test' counter in 'inet filter' table
+# Return packet count for elem $1 from 'test' counter in 'inet filter' table
 count_packets() {
+       found=0
+       for token in $(nft reset element inet filter test "${1}" ); do
+               [ ${found} -eq 1 ] && echo "${token}" && return
+               [ "${token}" = "packets" ] && found=1
+       done
+}
+
+# Return packet count from 'test' counter in 'inet filter' table
+count_packets_nomatch() {
        found=0
        for token in $(nft list counter inet filter test); do
                [ ${found} -eq 1 ] && echo "${token}" && return
@@ -1206,6 +1216,10 @@ perf() {
 
 # Set MAC addresses, send single packet, check that it matches, reset counter
 send_match() {
+       local elem="$1"
+
+       shift
+
        ip link set veth_a address "$(format_mac "${1}")"
        ip -n B link set veth_b address "$(format_mac "${2}")"
 
@@ -1216,7 +1230,7 @@ send_match() {
                eval src_"$f"=\$\(format_\$f "${2}"\)
        done
        eval send_\$proto
-       if [ "$(count_packets)" != "1" ]; then
+       if [ "$(count_packets "$elem")" != "1" ]; then
                err "${proto} packet to:"
                err "  $(for f in ${dst}; do
                         eval format_\$f "${1}"; printf ' '; done)"
@@ -1242,7 +1256,7 @@ send_nomatch() {
                eval src_"$f"=\$\(format_\$f "${2}"\)
        done
        eval send_\$proto
-       if [ "$(count_packets)" != "0" ]; then
+       if [ "$(count_packets_nomatch)" != "0" ]; then
                err "${proto} packet to:"
                err "  $(for f in ${dst}; do
                         eval format_\$f "${1}"; printf ' '; done)"
@@ -1262,6 +1276,8 @@ send_nomatch() {
 test_correctness_main() {
        range_size=1
        for i in $(seq "${start}" $((start + count))); do
+               local elem=""
+
                end=$((start + range_size))
 
                # Avoid negative or zero-sized port ranges
@@ -1272,15 +1288,16 @@ test_correctness_main() {
                srcstart=$((start + src_delta))
                srcend=$((end + src_delta))
 
-               add "$(format)" || return 1
+               elem="$(format)"
+               add "$elem" || return 1
                for j in $(seq "$start" $((range_size / 2 + 1)) ${end}); do
-                       send_match "${j}" $((j + src_delta)) || return 1
+                       send_match "$elem" "${j}" $((j + src_delta)) || return 1
                done
                send_nomatch $((end + 1)) $((end + 1 + src_delta)) || return 1
 
                # Delete elements now and then
                if [ $((i % 3)) -eq 0 ]; then
-                       del "$(format)" || return 1
+                       del "$elem" || return 1
                        for j in $(seq "$start" \
                                   $((range_size / 2 + 1)) ${end}); do
                                send_nomatch "${j}" $((j + src_delta)) \
@@ -1572,14 +1589,17 @@ test_timeout() {
 
        range_size=1
        for i in $(seq "$start" $((start + count))); do
+               local elem=""
+
                end=$((start + range_size))
                srcstart=$((start + src_delta))
                srcend=$((end + src_delta))
 
-               add "$(format)" || return 1
+               elem="$(format)"
+               add "$elem" || return 1
 
                for j in $(seq "$start" $((range_size / 2 + 1)) ${end}); do
-                       send_match "${j}" $((j + src_delta)) || return 1
+                       send_match "$elem" "${j}" $((j + src_delta)) || return 1
                done
 
                range_size=$((range_size + 1))
@@ -1737,7 +1757,7 @@ test_bug_reload() {
                srcend=$((end + src_delta))
 
                for j in $(seq "$start" $((range_size / 2 + 1)) ${end}); do
-                       send_match "${j}" $((j + src_delta)) || return 1
+                       send_match "$(format)" "${j}" $((j + src_delta)) || return 1
                done
 
                range_size=$((range_size + 1))
@@ -1817,7 +1837,7 @@ test_bug_avx2_mismatch()
        dst_addr6="$a2"
        send_icmp6
 
-       if [ "$(count_packets)" -gt "0" ]; then
+       if [ "$(count_packets "{ icmpv6 . $a1 }")" -gt "0" ]; then
                err "False match for $a2"
                return 1
        fi