| Message ID | 6f1dbaf2ab5a98b2616b14d93ee589a7e741e5f9.1580342294.git.sbrivio@redhat.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | Introduce support for concatenated ranges | expand |
Hi, On Thu, Jan 30, 2020 at 01:16:58AM +0100, Stefano Brivio wrote: > This test checks that set elements can be added, deleted, that > addition and deletion are refused when appropriate, that entries > time out properly, and that they can be fetched by matching values > in the given ranges. Not worth blocking this series, but I really think this test should be reduced in size. On my VM, it takes 3.5min to complete, out of which 96secs are spent in sleep. /o\ Cheers, Phil
On Thu, Jan 30, 2020 at 01:16:58AM +0100, Stefano Brivio wrote: > This test checks that set elements can be added, deleted, that > addition and deletion are refused when appropriate, that entries > time out properly, and that they can be fetched by matching values > in the given ranges. I'll keep this back so Phil doesn't have to do some knitting work meanwhile the tests finishes for those 3 minutes. If this can be shortened, better. Probably you can add a parameter to enable the extra torture test mode not that is away from the ./run-test.sh path.
On Fri, 7 Feb 2020 11:34:42 +0100 Pablo Neira Ayuso <pablo@netfilter.org> wrote: > On Thu, Jan 30, 2020 at 01:16:58AM +0100, Stefano Brivio wrote: > > This test checks that set elements can be added, deleted, that > > addition and deletion are refused when appropriate, that entries > > time out properly, and that they can be fetched by matching values > > in the given ranges. > > I'll keep this back so Phil doesn't have to do some knitting work > meanwhile the tests finishes for those 3 minutes. But I wanted to see his production :( > If this can be shortened, better. Probably you can add a parameter to > enable the extra torture test mode not that is away from the > ./run-test.sh path. I can't think of an easy way to remove that sleep(1), I could decrease the timeouts passed to nft but then there's no portable way to wait for less than one second. Probably a good way to make it faster and still retain coverage would be to decrease the amount of combinations. Right now, most of the 6 ^ 3 combinations (six "types", three values each to have: single, prefix, range -- where allowed) are tested. I could stop after the first 3 x 3 matrix instead, if we come from run-tests.sh. Let me know if you have other ideas, otherwise I'd send a patch doing this.
On Mon, Feb 10, 2020 at 04:08:40PM +0100, Stefano Brivio wrote: > On Fri, 7 Feb 2020 11:34:42 +0100 > Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > > On Thu, Jan 30, 2020 at 01:16:58AM +0100, Stefano Brivio wrote: > > > This test checks that set elements can be added, deleted, that > > > addition and deletion are refused when appropriate, that entries > > > time out properly, and that they can be fetched by matching values > > > in the given ranges. > > > > I'll keep this back so Phil doesn't have to do some knitting work > > meanwhile the tests finishes for those 3 minutes. > > But I wanted to see his production :( I'm not good at knitting, always liked crocheting more. That said, I like fast testsuites more than any of those. ;) > > If this can be shortened, better. Probably you can add a parameter to > > enable the extra torture test mode not that is away from the > > ./run-test.sh path. > > I can't think of an easy way to remove that sleep(1), I could decrease > the timeouts passed to nft but then there's no portable way to wait for > less than one second. > > Probably a good way to make it faster and still retain coverage would > be to decrease the amount of combinations. Right now, most of the 6 ^ 3 > combinations (six "types", three values each to have: single, prefix, > range -- where allowed) are tested. I could stop after the first 3 x 3 > matrix instead, if we come from run-tests.sh. > > Let me know if you have other ideas, otherwise I'd send a patch doing > this. You could test the timeout feature just once and for all? I doubt there will ever be a bug in that feature which only a certain data type exposes, but you may e.g. create all the sets with elements at the same time so waiting for the timeout once is enough. Cheers, Phil
Stefano Brivio <sbrivio@redhat.com> wrote: > On Fri, 7 Feb 2020 11:34:42 +0100 > Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > > On Thu, Jan 30, 2020 at 01:16:58AM +0100, Stefano Brivio wrote: > > > This test checks that set elements can be added, deleted, that > > > addition and deletion are refused when appropriate, that entries > > > time out properly, and that they can be fetched by matching values > > > in the given ranges. > > > > I'll keep this back so Phil doesn't have to do some knitting work > > meanwhile the tests finishes for those 3 minutes. > > But I wanted to see his production :( > > > If this can be shortened, better. Probably you can add a parameter to > > enable the extra torture test mode not that is away from the > > ./run-test.sh path. > > I can't think of an easy way to remove that sleep(1), I could decrease > the timeouts passed to nft but then there's no portable way to wait for > less than one second. Even busybox' sleep can do 'sleep 0.01', do we really need to be *that* portable?
On Mon, 10 Feb 2020 17:04:10 +0100 Florian Westphal <fw@strlen.de> wrote: > Stefano Brivio <sbrivio@redhat.com> wrote: > > On Fri, 7 Feb 2020 11:34:42 +0100 > > Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > > > > On Thu, Jan 30, 2020 at 01:16:58AM +0100, Stefano Brivio wrote: > > > > This test checks that set elements can be added, deleted, that > > > > addition and deletion are refused when appropriate, that entries > > > > time out properly, and that they can be fetched by matching values > > > > in the given ranges. > > > > > > I'll keep this back so Phil doesn't have to do some knitting work > > > meanwhile the tests finishes for those 3 minutes. > > > > But I wanted to see his production :( > > > > > If this can be shortened, better. Probably you can add a parameter to > > > enable the extra torture test mode not that is away from the > > > ./run-test.sh path. > > > > I can't think of an easy way to remove that sleep(1), I could decrease > > the timeouts passed to nft but then there's no portable way to wait for > > less than one second. > > Even busybox' sleep can do 'sleep 0.01' Wait, that's only if you build it with ENABLE_FEATURE_FANCY_SLEEP and ENABLE_FLOAT_DURATION. > do we really need to be *that* portable? I don't actually know :) However, with Phil's idea: On Mon, 10 Feb 2020 16:51:47 +0100 Phil Sutter <phil@nwl.cc> wrote: > You could test the timeout feature just once and for all? I doubt there > will ever be a bug in that feature which only a certain data type > exposes, but you may e.g. create all the sets with elements at the same > time so waiting for the timeout once is enough. which I think is entirely reasonable, this becomes a single one-second sleep, so it shouldn't be a problem anymore. I would propose that I try this first, see if it gets reasonable, if it's not enough I'd go on and just reduce the number of combinations depending on how the script is invoked.
diff --git a/tests/shell/testcases/sets/0042concatenated_ranges_0 b/tests/shell/testcases/sets/0042concatenated_ranges_0 new file mode 100755 index 000000000000..244c5ffe7c75 --- /dev/null +++ b/tests/shell/testcases/sets/0042concatenated_ranges_0 @@ -0,0 +1,162 @@ +#!/bin/sh -e +# +# 0042concatenated_ranges_0 - Add, get, list, timeout for concatenated ranges +# +# Cycle over supported data types, forming concatenations of three fields, for +# all possible permutations, and: +# - add entries to set +# - list them +# - check that they can't be added again +# - get entries by specifying a value matching ranges for all fields +# - delete them +# - add them with 1s timeout +# - check that they can't be added again right away +# - check that they are not listed after 1s +# - delete them +# - make sure they can't be deleted again + +TYPES="ipv4_addr ipv6_addr ether_addr inet_proto inet_service mark" + +RULESPEC_ipv4_addr="ip saddr" +ELEMS_ipv4_addr="192.0.2.1 198.51.100.0/25 203.0.113.0-203.0.113.129" +ADD_ipv4_addr="192.0.2.252/31" +GET_ipv4_addr="198.51.100.127 198.51.100.0/25" + +RULESPEC_ipv6_addr="ip6 daddr" +ELEMS_ipv6_addr="2001:db8:c0c:c0de::1-2001:db8:cacc::a 2001:db8::1 2001:db8:dada:da::/64" +ADD_ipv6_addr="2001:db8::d1ca:d1ca" +GET_ipv6_addr="2001:db8::1 2001:db8::1" + +RULESPEC_ether_addr="ether saddr" +ELEMS_ether_addr="00:0a:c1:d1:f1:ed-00:0a:c1:dd:ec:af 00:0b:0c:ca:cc:10-c1:a0:c1:cc:10:00 f0:ca:cc:1a:b0:1a" +ADD_ether_addr="00:be:1d:ed:ab:e1" +GET_ether_addr="ac:c1:ac:c0:ce:c0 00:0b:0c:ca:cc:10-c1:a0:c1:cc:10:00" + +RULESPEC_inet_proto="meta l4proto" +ELEMS_inet_proto="tcp udp icmp" +ADD_inet_proto="sctp" +GET_inet_proto="udp udp" + +RULESPEC_inet_service="tcp dport" +ELEMS_inet_service="22-23 1024-32768 31337" +ADD_inet_service="32769-65535" +GET_inet_service="32768 1024-32768" + +RULESPEC_mark="mark" +ELEMS_mark="0x00000064-0x000000c8 0x0000006f 0x0000fffd-0x0000ffff" +ADD_mark="0x0000002a" +GET_mark="0x0000006f 0x0000006f" + +tmp="$(mktemp)" +trap "rm -f ${tmp}" EXIT + +render() { + eval "echo \"$(cat ${1})\"" +} + +cat <<'EOF' > "${tmp}" +flush ruleset + +table inet filter { + set test { + type ${ta} . ${tb} . ${tc} + flags interval,timeout + elements = { ${a1} . ${b1} . ${c1} , + ${a2} . ${b2} . ${c2} , + ${a3} . ${b3} . ${c3} } + } + + chain output { + type filter hook output priority 0; policy accept; + ${sa} . ${sb} . ${sc} @test counter + } +} +EOF + +for ta in ${TYPES}; do + eval a=\$ELEMS_${ta} + a1=${a%% *}; a2=$(expr "$a" : ".* \(.*\) .*"); a3=${a##* } + eval sa=\$RULESPEC_${ta} + + for tb in ${TYPES}; do + [ "${tb}" = "${ta}" ] && continue + if [ "${tb}" = "ipv6_addr" ]; then + [ "${ta}" = "ipv4_addr" ] && continue + elif [ "${tb}" = "ipv4_addr" ]; then + [ "${ta}" = "ipv6_addr" ] && continue + fi + + eval b=\$ELEMS_${tb} + b1=${b%% *}; b2=$(expr "$b" : ".* \(.*\) .*"); b3=${b##* } + eval sb=\$RULESPEC_${tb} + + for tc in ${TYPES}; do + [ "${tc}" = "${ta}" ] && continue + [ "${tc}" = "${tb}" ] && continue + if [ "${tc}" = "ipv6_addr" ]; then + [ "${ta}" = "ipv4_addr" ] && continue + [ "${tb}" = "ipv4_addr" ] && continue + elif [ "${tc}" = "ipv4_addr" ]; then + [ "${ta}" = "ipv6_addr" ] && continue + [ "${tb}" = "ipv6_addr" ] && continue + fi + + eval c=\$ELEMS_${tc} + c1=${c%% *}; c2=$(expr "$c" : ".* \(.*\) .*"); c3=${c##* } + eval sc=\$RULESPEC_${tc} + + render ${tmp} | ${NFT} -f - + + [ $(${NFT} list set inet filter test | \ + grep -c -e "${a1} . ${b1} . ${c1}" \ + -e "${a2} . ${b2} . ${c2}" \ + -e "${a3} . ${b3} . ${c3}") -eq 3 ] + + ! ${NFT} add element inet filter test \ + "{ ${a1} . ${b1} . ${c1} }" 2>/dev/null + ! ${NFT} add element inet filter test \ + "{ ${a2} . ${b2} . ${c2} }" 2>/dev/null + ! ${NFT} add element inet filter test \ + "{ ${a3} . ${b3} . ${c3} }" 2>/dev/null + + ${NFT} delete element inet filter test \ + "{ ${a1} . ${b1} . ${c1} }" + ! ${NFT} delete element inet filter test \ + "{ ${a1} . ${b1} . ${c1} }" 2>/dev/null + + eval add_a=\$ADD_${ta} + eval add_b=\$ADD_${tb} + eval add_c=\$ADD_${tc} + ${NFT} add element inet filter test \ + "{ ${add_a} . ${add_b} . ${add_c} timeout 1s}" + [ $(${NFT} list set inet filter test | \ + grep -c "${add_a} . ${add_b} . ${add_c}") -eq 1 ] + ! ${NFT} add element inet filter test \ + "{ ${add_a} . ${add_b} . ${add_c} timeout 1s}" \ + 2>/dev/null + + eval get_a=\$GET_${ta} + eval get_b=\$GET_${tb} + eval get_c=\$GET_${tc} + exp_a=${get_a##* }; get_a=${get_a%% *} + exp_b=${get_b##* }; get_b=${get_b%% *} + exp_c=${get_c##* }; get_c=${get_c%% *} + [ $(${NFT} get element inet filter test \ + "{ ${get_a} . ${get_b} . ${get_c} }" | \ + grep -c "${exp_a} . ${exp_b} . ${exp_c}") -eq 1 ] + + sleep 1 + [ $(${NFT} list set inet filter test | \ + grep -c "${add_a} . ${add_b} . ${add_c}") -eq 0 ] + + ${NFT} delete element inet filter test \ + "{ ${a2} . ${b2} . ${c2} }" + ${NFT} delete element inet filter test \ + "{ ${a3} . ${b3} . ${c3} }" + ! ${NFT} delete element inet filter test \ + "{ ${a2} . ${b2} . ${c2} }" 2>/dev/null + ! ${NFT} delete element inet filter test \ + "{ ${a3} . ${b3} . ${c3} }" 2>/dev/null + done + done +done
This test checks that set elements can be added, deleted, that addition and deletion are refused when appropriate, that entries time out properly, and that they can be fetched by matching values in the given ranges. v4: No changes v3: - renumber test to 0042, 0041 was added meanwhile v2: - actually check an IPv6 prefix, instead of specifying everything as explicit ranges in ELEMS_ipv6_addr - renumber test to 0041, 0038 already exists Signed-off-by: Stefano Brivio <sbrivio@redhat.com> --- .../testcases/sets/0042concatenated_ranges_0 | 162 ++++++++++++++++++ 1 file changed, 162 insertions(+) create mode 100755 tests/shell/testcases/sets/0042concatenated_ranges_0