diff mbox series

[ovs-dev] ovn.at: Skip ACL rate-limiting test on slow/overloaded systems.

Message ID 20180906043101.40003-1-jpettit@ovn.org
State Accepted
Headers show
Series [ovs-dev] ovn.at: Skip ACL rate-limiting test on slow/overloaded systems. | expand

Commit Message

Justin Pettit Sept. 6, 2018, 4:31 a.m. UTC
In ACL rate-limiting test, we send three sets of 100 packets.  One of
the sets drops packets at a rate of 10 per second, one at a rate of 5
per second, and one not at all.  On my setup, it takes roughly 0.67
seconds to send those 300 packets, but we have reports of it taking over
15 seconds on others.  The test was intended to allow some flexibility
in run-time, but it's very difficult to design a mechanism that can all
possibilities.

To prevent false test failures, this patch changes the test to check
the duration count of the meter, and if it's greater than nine seconds,
just skip the test.

Signed-off-by: Justin Pettit <jpettit@ovn.org>
Reported-by: Thomas Goirand <zigo@debian.org>
---
 tests/ovn.at | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

Comments

Ben Pfaff Sept. 6, 2018, 9:45 p.m. UTC | #1
On Wed, Sep 05, 2018 at 09:31:01PM -0700, Justin Pettit wrote:
> In ACL rate-limiting test, we send three sets of 100 packets.  One of
> the sets drops packets at a rate of 10 per second, one at a rate of 5
> per second, and one not at all.  On my setup, it takes roughly 0.67
> seconds to send those 300 packets, but we have reports of it taking over
> 15 seconds on others.  The test was intended to allow some flexibility
> in run-time, but it's very difficult to design a mechanism that can all
> possibilities.
> 
> To prevent false test failures, this patch changes the test to check
> the duration count of the meter, and if it's greater than nine seconds,
> just skip the test.
> 
> Signed-off-by: Justin Pettit <jpettit@ovn.org>
> Reported-by: Thomas Goirand <zigo@debian.org>

Around here, I recommend adding an "echo" something like this:
        echo "meter duration $d_secs"

That way, if the meter gets skipped, it's easier to find out how long it
actually took.

> +# On particularly slow or overloaded systems, the transmission rate may
> +# be lower than the configured meter rate.  To prevent false test
> +# failures, we check the duration count of the meter, and if it's
> +# greater than nine seconds, just skip the test.
> +d_secs=$(as hv ovs-ofctl -O OpenFlow13 meter-stats br-int | grep "meter:1" | sed 's/.* duration:\([[0-9]]\{1,\}\)\.[[0-9]]\+s .*/\1/')
> +AT_SKIP_IF([test $d_secs -gt 9])

I didn't test this.
Justin Pettit Sept. 6, 2018, 10:45 p.m. UTC | #2
> On Sep 6, 2018, at 2:45 PM, Ben Pfaff <blp@ovn.org> wrote:
> 
> On Wed, Sep 05, 2018 at 09:31:01PM -0700, Justin Pettit wrote:
>> In ACL rate-limiting test, we send three sets of 100 packets.  One of
>> the sets drops packets at a rate of 10 per second, one at a rate of 5
>> per second, and one not at all.  On my setup, it takes roughly 0.67
>> seconds to send those 300 packets, but we have reports of it taking over
>> 15 seconds on others.  The test was intended to allow some flexibility
>> in run-time, but it's very difficult to design a mechanism that can all
>> possibilities.
>> 
>> To prevent false test failures, this patch changes the test to check
>> the duration count of the meter, and if it's greater than nine seconds,
>> just skip the test.
>> 
>> Signed-off-by: Justin Pettit <jpettit@ovn.org>
>> Reported-by: Thomas Goirand <zigo@debian.org>
> 
> Around here, I recommend adding an "echo" something like this:
>        echo "meter duration $d_secs"
> 
> That way, if the meter gets skipped, it's easier to find out how long it
> actually took.

Sounds good.  Added.

>> +# On particularly slow or overloaded systems, the transmission rate may
>> +# be lower than the configured meter rate.  To prevent false test
>> +# failures, we check the duration count of the meter, and if it's
>> +# greater than nine seconds, just skip the test.
>> +d_secs=$(as hv ovs-ofctl -O OpenFlow13 meter-stats br-int | grep "meter:1" | sed 's/.* duration:\([[0-9]]\{1,\}\)\.[[0-9]]\+s .*/\1/')
>> +AT_SKIP_IF([test $d_secs -gt 9])
> 
> I didn't test this.

I've tried to test it in a number of scenarios, and it seems to work okay.

I pushed this to master and branch-2.10.

Thanks,

--Justin
diff mbox series

Patch

diff --git a/tests/ovn.at b/tests/ovn.at
index e10a7f9bab59..1fe4a4d4bf1c 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -6500,15 +6500,23 @@  for i in `seq 1 100`; do
     ovs-appctl netdev-dummy/receive lp1 'in_port(1),eth(src=f0:00:00:00:00:01,dst=f0:00:00:00:00:02),eth_type(0x0800),ipv4(src=192.168.1.2,dst=192.168.1.3,proto=6,tos=0,ttl=64,frag=no),tcp(src=7777,dst=82)'
 done
 
+# The rate at which packets are sent is highly system-dependent, so we
+# can't count on precise drop counts.  To work around that, we just
+# check that exactly 100 "http-acl3" actions were logged and that there
+# were more "http-acl1" actions than "http-acl2" ones.
+OVS_WAIT_UNTIL([ test 100 = $(grep -c 'http-acl3' hv/ovn-controller.log) ])
+
+# On particularly slow or overloaded systems, the transmission rate may
+# be lower than the configured meter rate.  To prevent false test
+# failures, we check the duration count of the meter, and if it's
+# greater than nine seconds, just skip the test.
+d_secs=$(as hv ovs-ofctl -O OpenFlow13 meter-stats br-int | grep "meter:1" | sed 's/.* duration:\([[0-9]]\{1,\}\)\.[[0-9]]\+s .*/\1/')
+AT_SKIP_IF([test $d_secs -gt 9])
+
 # Print some information that may help debugging.
 as hv ovs-appctl -t ovn-controller meter-table-list
 as hv ovs-ofctl -O OpenFlow13 meter-stats br-int
 
-# The userspace meter implementation doesn't precisely enforce counts,
-# so we just check that exactly 100 "http-acl3" actions were logged and
-# that there were more "http-acl1" actions than "http-acl2" ones.
-OVS_WAIT_UNTIL([ test 100 = $(grep -c 'http-acl3' hv/ovn-controller.log) ])
-
 n_acl1=$(grep -c 'http-acl1' hv/ovn-controller.log)
 n_acl2=$(grep -c 'http-acl2' hv/ovn-controller.log)
 n_acl3=$(grep -c 'http-acl3' hv/ovn-controller.log)