Skip to content

Commit febe7ed

Browse files
Florian Westphalummakynes
authored andcommitted
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 <[email protected]> Reviewed-by: Stefano Brivio <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]>
1 parent ea77c39 commit febe7ed

File tree

1 file changed

+30
-10
lines changed

1 file changed

+30
-10
lines changed

tools/testing/selftests/net/netfilter/nft_concat_range.sh

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,7 @@ table inet filter {
419419
420420
set test {
421421
type ${type_spec}
422+
counter
422423
flags interval,timeout
423424
}
424425
@@ -1158,8 +1159,17 @@ del() {
11581159
fi
11591160
}
11601161

1161-
# Return packet count from 'test' counter in 'inet filter' table
1162+
# Return packet count for elem $1 from 'test' counter in 'inet filter' table
11621163
count_packets() {
1164+
found=0
1165+
for token in $(nft reset element inet filter test "${1}" ); do
1166+
[ ${found} -eq 1 ] && echo "${token}" && return
1167+
[ "${token}" = "packets" ] && found=1
1168+
done
1169+
}
1170+
1171+
# Return packet count from 'test' counter in 'inet filter' table
1172+
count_packets_nomatch() {
11631173
found=0
11641174
for token in $(nft list counter inet filter test); do
11651175
[ ${found} -eq 1 ] && echo "${token}" && return
@@ -1206,6 +1216,10 @@ perf() {
12061216

12071217
# Set MAC addresses, send single packet, check that it matches, reset counter
12081218
send_match() {
1219+
local elem="$1"
1220+
1221+
shift
1222+
12091223
ip link set veth_a address "$(format_mac "${1}")"
12101224
ip -n B link set veth_b address "$(format_mac "${2}")"
12111225

@@ -1216,7 +1230,7 @@ send_match() {
12161230
eval src_"$f"=\$\(format_\$f "${2}"\)
12171231
done
12181232
eval send_\$proto
1219-
if [ "$(count_packets)" != "1" ]; then
1233+
if [ "$(count_packets "$elem")" != "1" ]; then
12201234
err "${proto} packet to:"
12211235
err " $(for f in ${dst}; do
12221236
eval format_\$f "${1}"; printf ' '; done)"
@@ -1242,7 +1256,7 @@ send_nomatch() {
12421256
eval src_"$f"=\$\(format_\$f "${2}"\)
12431257
done
12441258
eval send_\$proto
1245-
if [ "$(count_packets)" != "0" ]; then
1259+
if [ "$(count_packets_nomatch)" != "0" ]; then
12461260
err "${proto} packet to:"
12471261
err " $(for f in ${dst}; do
12481262
eval format_\$f "${1}"; printf ' '; done)"
@@ -1262,6 +1276,8 @@ send_nomatch() {
12621276
test_correctness_main() {
12631277
range_size=1
12641278
for i in $(seq "${start}" $((start + count))); do
1279+
local elem=""
1280+
12651281
end=$((start + range_size))
12661282

12671283
# Avoid negative or zero-sized port ranges
@@ -1272,15 +1288,16 @@ test_correctness_main() {
12721288
srcstart=$((start + src_delta))
12731289
srcend=$((end + src_delta))
12741290

1275-
add "$(format)" || return 1
1291+
elem="$(format)"
1292+
add "$elem" || return 1
12761293
for j in $(seq "$start" $((range_size / 2 + 1)) ${end}); do
1277-
send_match "${j}" $((j + src_delta)) || return 1
1294+
send_match "$elem" "${j}" $((j + src_delta)) || return 1
12781295
done
12791296
send_nomatch $((end + 1)) $((end + 1 + src_delta)) || return 1
12801297

12811298
# Delete elements now and then
12821299
if [ $((i % 3)) -eq 0 ]; then
1283-
del "$(format)" || return 1
1300+
del "$elem" || return 1
12841301
for j in $(seq "$start" \
12851302
$((range_size / 2 + 1)) ${end}); do
12861303
send_nomatch "${j}" $((j + src_delta)) \
@@ -1572,14 +1589,17 @@ test_timeout() {
15721589

15731590
range_size=1
15741591
for i in $(seq "$start" $((start + count))); do
1592+
local elem=""
1593+
15751594
end=$((start + range_size))
15761595
srcstart=$((start + src_delta))
15771596
srcend=$((end + src_delta))
15781597

1579-
add "$(format)" || return 1
1598+
elem="$(format)"
1599+
add "$elem" || return 1
15801600

15811601
for j in $(seq "$start" $((range_size / 2 + 1)) ${end}); do
1582-
send_match "${j}" $((j + src_delta)) || return 1
1602+
send_match "$elem" "${j}" $((j + src_delta)) || return 1
15831603
done
15841604

15851605
range_size=$((range_size + 1))
@@ -1737,7 +1757,7 @@ test_bug_reload() {
17371757
srcend=$((end + src_delta))
17381758

17391759
for j in $(seq "$start" $((range_size / 2 + 1)) ${end}); do
1740-
send_match "${j}" $((j + src_delta)) || return 1
1760+
send_match "$(format)" "${j}" $((j + src_delta)) || return 1
17411761
done
17421762

17431763
range_size=$((range_size + 1))
@@ -1817,7 +1837,7 @@ test_bug_avx2_mismatch()
18171837
dst_addr6="$a2"
18181838
send_icmp6
18191839

1820-
if [ "$(count_packets)" -gt "0" ]; then
1840+
if [ "$(count_packets "{ icmpv6 . $a1 }")" -gt "0" ]; then
18211841
err "False match for $a2"
18221842
return 1
18231843
fi

0 commit comments

Comments
 (0)