[ovs-dev] [PATCH ovn] ovn-controller: Fix IP engine run with in-flight messages

Dumitru Ceara dceara at redhat.com
Fri Aug 2 09:22:58 UTC 2019


When trying to incrementally process changes even if there are in-flight
messages we were incorrectly setting the engine_aborted variable to the
value returned by engine_run. However, engine_run returns true if the
run was completed.

This was causing discrepancies between logical flows and openflow flows
due to the fact that the rerun wasn't happening after an aborted run.

In order to avoid confusion the engine_aborted variable is now renamed to
engine_run_done thus avoiding the negated logic.

CC: Han Zhou <hzhou8 at ebay.com>
Fixes: a6b7d9f4f047 ("ovn-controller: Fix flow installation latency caused by recompute.")
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
---
 controller/ovn-controller.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index ad33d17..15b9f4e 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -1896,7 +1896,7 @@ main(int argc, char *argv[])
 
     uint64_t engine_run_id = 0;
     uint64_t old_engine_run_id = 0;
-    bool engine_aborted = false;
+    bool engine_run_done = true;
 
     unsigned int ovs_cond_seqno = UINT_MAX;
     unsigned int ovnsb_cond_seqno = UINT_MAX;
@@ -1997,14 +1997,14 @@ main(int argc, char *argv[])
                              * this round of engine_run and continue processing
                              * acculated changes incrementally later when
                              * ofctrl_can_put() returns true. */
-                            if (!engine_aborted) {
+                            if (engine_run_done) {
                                 engine_set_abort_recompute(true);
-                                engine_aborted = engine_run(&en_flow_output,
-                                                            ++engine_run_id);
+                                engine_run_done = engine_run(&en_flow_output,
+                                                             ++engine_run_id);
                             }
                         } else {
                             engine_set_abort_recompute(false);
-                            engine_aborted = false;
+                            engine_run_done = true;
                             engine_run(&en_flow_output, ++engine_run_id);
                         }
                     }
@@ -2048,8 +2048,8 @@ main(int argc, char *argv[])
                 }
 
             }
-            if (old_engine_run_id == engine_run_id || engine_aborted) {
-                if (engine_aborted || engine_need_run(&en_flow_output)) {
+            if (old_engine_run_id == engine_run_id || !engine_run_done) {
+                if (!engine_run_done || 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);
-- 
1.8.3.1



More information about the dev mailing list