[ovs-dev,v3,09/16] ovn-controller: Avoid forced recompute when not needed

Message ID 1527874926-40450-10-git-send-email-hzhou8@ebay.com
State Superseded
Headers show
Series
  • ovn-controller incremental processing
Related show

Commit Message

Han Zhou June 1, 2018, 5:41 p.m.
Signed-off-by: Han Zhou <hzhou8@ebay.com>
---
 ovn/controller/ovn-controller.c | 13 +++++++++----
 ovn/lib/inc-proc-eng.c          | 20 ++++++++++++++++++++
 ovn/lib/inc-proc-eng.h          |  4 ++++
 3 files changed, 33 insertions(+), 4 deletions(-)

Comments

Ben Pfaff June 4, 2018, 10:50 p.m. | #1
On Fri, Jun 01, 2018 at 10:41:59AM -0700, Han Zhou wrote:
> Signed-off-by: Han Zhou <hzhou8@ebay.com>

This seems like a refinement of the incremental processing engine
mainly.  Is there a reason it can only happen in this point in the
series?  In other words, can this be folded into patch 1?
Han Zhou June 4, 2018, 11:22 p.m. | #2
On Mon, Jun 4, 2018 at 3:50 PM, Ben Pfaff <blp@ovn.org> wrote:
>
> On Fri, Jun 01, 2018 at 10:41:59AM -0700, Han Zhou wrote:
> > Signed-off-by: Han Zhou <hzhou8@ebay.com>
>
> This seems like a refinement of the incremental processing engine
> mainly.  Is there a reason it can only happen in this point in the
> series?  In other words, can this be folded into patch 1?

Yes it is a refinement/improvement. The change in engine can be folded into
patch 1, and the change in ovn-controller.c that calls engine_need_run()
can be folded into patch 5: ovn-controller: split ovs_idl inputs in
incremental engine.

I will take care of this in v4, after you finish the review for v3.

Thanks,
Han

Patch

diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index e939d03..e4506c2 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -1157,10 +1157,15 @@  main(int argc, char *argv[])
 
         }
         if (old_engine_run_id == engine_run_id) {
-            VLOG_DBG("engine did not run, force recompute next time: "
-                     "br_int %p, chassis %p", br_int, chassis);
-            engine_set_force_recompute(true);
-            poll_immediate_wake();
+            if (engine_need_run(&en_flow_output)) {
+                VLOG_DBG("engine did not run, force recompute next time: "
+                         "br_int %p, chassis %p", br_int, chassis);
+                engine_set_force_recompute(true);
+                poll_immediate_wake();
+            } else {
+                VLOG_DBG("engine did not run, and it was not needed either: "
+                         "br_int %p, chassis %p", br_int, chassis);
+            }
         } else {
             engine_set_force_recompute(false);
         }
diff --git a/ovn/lib/inc-proc-eng.c b/ovn/lib/inc-proc-eng.c
index 8006acd..08f152e 100644
--- a/ovn/lib/inc-proc-eng.c
+++ b/ovn/lib/inc-proc-eng.c
@@ -139,3 +139,23 @@  engine_run(struct engine_node *node, uint64_t run_id)
 
     VLOG_DBG("node: %s, changed: %d", node->name, node->changed);
 }
+
+bool
+engine_need_run(struct engine_node *node)
+{
+    size_t i;
+
+    if (!node->n_inputs) {
+        node->run(node);
+        VLOG_DBG("input node: %s, changed: %d", node->name, node->changed);
+        return node->changed;
+    }
+
+    for (i = 0; i < node->n_inputs; i++) {
+        if (engine_need_run(node->inputs[i].node)) {
+            return true;
+        }
+    }
+
+    return false;
+}
diff --git a/ovn/lib/inc-proc-eng.h b/ovn/lib/inc-proc-eng.h
index 8399321..17006f1 100644
--- a/ovn/lib/inc-proc-eng.h
+++ b/ovn/lib/inc-proc-eng.h
@@ -126,6 +126,10 @@  void engine_run(struct engine_node *, uint64_t run_id);
  * terminates. */
 void engine_cleanup(struct engine_node *);
 
+/* Check if engine needs to run, i.e. any change to be processed. */
+bool
+engine_need_run(struct engine_node *);
+
 /* Get the input node with <name> for <node> */
 struct engine_node * engine_get_input(const char *input_name,
                                       struct engine_node *);