| Message ID | 20241022152324.13554-1-fw@strlen.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c59d72d0a4fbaa5fd7a04b2d13cfc101d01310db |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] selftests: netfilter: nft_flowtable.sh: make first pass deterministic | expand |
On Tue, Oct 22, 2024 at 05:23:18PM +0200, Florian Westphal wrote: > The CI occasionaly encounters a failing test run. Example: > # PASS: ipsec tunnel mode for ns1/ns2 > # re-run with random mtus: -o 10966 -l 19499 -r 31322 > # PASS: flow offloaded for ns1/ns2 > [..] > # FAIL: ipsec tunnel ... counter 1157059 exceeds expected value 878489 > > This script will re-exec itself, on the second run, random MTUs are > chosen for the involved links. This is done so we can cover different > combinations (large mtu on client, small on server, link has lowest > mtu, etc). > > Furthermore, file size is random, even for the first run. > > Rework this script and always use the same file size on initial run so > that at least the first round can be expected to have reproducible > behavior. > > Second round will use random mtu/filesize. > > Raise the failure limit to that of the file size, this should avoid all > errneous test errors. Currently, first fin will remove the offload, so if > one peer is already closing remaining data is handled by classic path, > which result in larger-than-expected counter and a test failure. > > Given packet path also counts tcp/ip headers, in case offload is > completely broken this test will still fail (as expected). > > The test counter limit could be made more strict again in the future > once flowtable can keep a connection in offloaded state until FINs > in both directions were seen. > > Signed-off-by: Florian Westphal <fw@strlen.de> > --- > If you prefer you can also apply this to net-next instead. Hi Florian, No preference on my side. But if it is for net, then we'll need a fixes tag. Which you can simply add by responding with it to this email. (I think it has to start at the beginning of the line.) In any case, Reviewed-by: Simon Horman <horms@kernel.org> ...
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Tue, 22 Oct 2024 17:23:18 +0200 you wrote: > The CI occasionaly encounters a failing test run. Example: > # PASS: ipsec tunnel mode for ns1/ns2 > # re-run with random mtus: -o 10966 -l 19499 -r 31322 > # PASS: flow offloaded for ns1/ns2 > [..] > # FAIL: ipsec tunnel ... counter 1157059 exceeds expected value 878489 > > [...] Here is the summary with links: - [net] selftests: netfilter: nft_flowtable.sh: make first pass deterministic https://git.kernel.org/netdev/net/c/c59d72d0a4fb You are awesome, thank you!
diff --git a/tools/testing/selftests/net/netfilter/nft_flowtable.sh b/tools/testing/selftests/net/netfilter/nft_flowtable.sh index b3995550856a..a4ee5496f2a1 100755 --- a/tools/testing/selftests/net/netfilter/nft_flowtable.sh +++ b/tools/testing/selftests/net/netfilter/nft_flowtable.sh @@ -71,6 +71,8 @@ omtu=9000 lmtu=1500 rmtu=2000 +filesize=$((2 * 1024 * 1024)) + usage(){ echo "nft_flowtable.sh [OPTIONS]" echo @@ -81,12 +83,13 @@ usage(){ exit 1 } -while getopts "o:l:r:" o +while getopts "o:l:r:s:" o do case $o in o) omtu=$OPTARG;; l) lmtu=$OPTARG;; r) rmtu=$OPTARG;; + s) filesize=$OPTARG;; *) usage;; esac done @@ -217,18 +220,10 @@ ns2out=$(mktemp) make_file() { - name=$1 - - SIZE=$((RANDOM % (1024 * 128))) - SIZE=$((SIZE + (1024 * 8))) - TSIZE=$((SIZE * 1024)) - - dd if=/dev/urandom of="$name" bs=1024 count=$SIZE 2> /dev/null + name="$1" + sz="$2" - SIZE=$((RANDOM % 1024)) - SIZE=$((SIZE + 128)) - TSIZE=$((TSIZE + SIZE)) - dd if=/dev/urandom conf=notrunc of="$name" bs=1 count=$SIZE 2> /dev/null + head -c "$sz" < /dev/urandom > "$name" } check_counters() @@ -246,18 +241,18 @@ check_counters() local fs fs=$(du -sb "$nsin") local max_orig=${fs%%/*} - local max_repl=$((max_orig/4)) + local max_repl=$((max_orig)) # flowtable fastpath should bypass normal routing one, i.e. the counters in forward hook # should always be lower than the size of the transmitted file (max_orig). if [ "$orig_cnt" -gt "$max_orig" ];then - echo "FAIL: $what: original counter $orig_cnt exceeds expected value $max_orig" 1>&2 + echo "FAIL: $what: original counter $orig_cnt exceeds expected value $max_orig, reply counter $repl_cnt" 1>&2 ret=1 ok=0 fi if [ "$repl_cnt" -gt $max_repl ];then - echo "FAIL: $what: reply counter $repl_cnt exceeds expected value $max_repl" 1>&2 + echo "FAIL: $what: reply counter $repl_cnt exceeds expected value $max_repl, original counter $orig_cnt" 1>&2 ret=1 ok=0 fi @@ -455,7 +450,7 @@ test_tcp_forwarding_nat() return $lret } -make_file "$nsin" +make_file "$nsin" "$filesize" # First test: # No PMTU discovery, nsr1 is expected to fragment packets from ns1 to ns2 as needed. @@ -664,8 +659,16 @@ if [ "$1" = "" ]; then l=$(((RANDOM%mtu) + low)) r=$(((RANDOM%mtu) + low)) - echo "re-run with random mtus: -o $o -l $l -r $r" - $0 -o "$o" -l "$l" -r "$r" + MINSIZE=$((2 * 1000 * 1000)) + MAXSIZE=$((64 * 1000 * 1000)) + + filesize=$(((RANDOM * RANDOM) % MAXSIZE)) + if [ "$filesize" -lt "$MINSIZE" ]; then + filesize=$((filesize+MINSIZE)) + fi + + echo "re-run with random mtus and file size: -o $o -l $l -r $r -s $filesize" + $0 -o "$o" -l "$l" -r "$r" -s "$filesize" fi exit $ret
The CI occasionaly encounters a failing test run. Example: # PASS: ipsec tunnel mode for ns1/ns2 # re-run with random mtus: -o 10966 -l 19499 -r 31322 # PASS: flow offloaded for ns1/ns2 [..] # FAIL: ipsec tunnel ... counter 1157059 exceeds expected value 878489 This script will re-exec itself, on the second run, random MTUs are chosen for the involved links. This is done so we can cover different combinations (large mtu on client, small on server, link has lowest mtu, etc). Furthermore, file size is random, even for the first run. Rework this script and always use the same file size on initial run so that at least the first round can be expected to have reproducible behavior. Second round will use random mtu/filesize. Raise the failure limit to that of the file size, this should avoid all errneous test errors. Currently, first fin will remove the offload, so if one peer is already closing remaining data is handled by classic path, which result in larger-than-expected counter and a test failure. Given packet path also counts tcp/ip headers, in case offload is completely broken this test will still fail (as expected). The test counter limit could be made more strict again in the future once flowtable can keep a connection in offloaded state until FINs in both directions were seen. Signed-off-by: Florian Westphal <fw@strlen.de> --- If you prefer you can also apply this to net-next instead. .../selftests/net/netfilter/nft_flowtable.sh | 39 ++++++++++--------- 1 file changed, 21 insertions(+), 18 deletions(-)