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>
This commit is contained in:
committed by
Pablo Neira Ayuso
parent
ea77c397bf
commit
febe7eda74
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user