[ovs-dev] [PATCH v2] upcall: Don't start new revalidation round too soon after the last one.

[Jarno Rajahalme]

> On Sep 26, 2016, at 5:39 PM, Ben Pfaff <blp at ovn.org> wrote:
> 
> On Tue, Sep 20, 2016 at 11:42:45AM -0700, Jarno Rajahalme wrote:
>> The execution time of 'ovs-ofctl add-flows' with a large number of
>> flows can be more than halved if revalidators are not running after
>> each flow mod separately.  This was first suspected when it was found
>> that 'ovs-ofctl --bundle add-flows' is about 10 times faster than the
>> same command without the '--bundle' option in a scenario where there
>> is a large set of flows being added and no datapath flows at all.  One
>> of the differences caused by the '--bundle' option is that the
>> revalidators are woken up only once, at the end of the whole set of
>> flow table changes, rather than after each flow table change
>> individually.
>> 
>> This patch limits the revalidation to run at most 200 times a second
>> by enforcing a minimum of 5ms time gap between the start times of
>> revalidation rounds.  If nothing happens in, say 6 milliseconds, and
>> then a new flow table change is signaled, the revalidator threads wake
>> up immediately without any further delay.  Values smaller than 5 were
>> found to increase the 'ovs-ofctl add-flows' execution time noticeably.
>> 
>> Since the revalidators are not running after each flow mod, the
>> overall OVS CPU utilization during the 'ovs-ofctl add-flows' run time
>> is reduced roughly by one core on a four core machine.
>> 
>> In testing the 'ovs-ofctl add-flows' execution time is not
>> significantly improved from this even if the revalidators are not
>> notified about the flow table changes at all.
>> 
>> Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
> 
> Is this the patch you said you wanted to squeak into branch-2.6?
> 
> Acked-by: Ben Pfaff <blp at ovn.org>

Yes, thank you. Running the testsuite once more I noticed that I needed to add this incremental to wait for the added delay in a test case that changes the flow tables in a loop and expected the changes to be instantly visible in the datapath:

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index e2b983f..025c923 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -4609,6 +4609,9 @@ m4_define([CHECK_CONTINUATION], [dnl
         m4_if([$3], [0], [],
             [AT_CHECK([echo "$actions1" | sed 's/pause/controller(pause)/g' | ovs-ofctl -O OpenFlow13 add-flows br1 -])])
 
+        # Make sure the datapath is up-to-date before sending the packet.
+        ovs-appctl revalidator/wait
+
         # Run a packet through the switch.
         AT_CHECK([ovs-appctl netdev-dummy/receive p1 "$flow"], [0], [stdout])

Pushed to master and branch-2.6.

  Jarno