If the last ovn-controller main loop run started a transaction to the
Southbound DB and the transaction is still in progress, ovn-controller
will not call engine_run(). In case there were changes to the DB,
engine_need_run() would return true which would trigger an immediate
forced recompute in the next processing loop iteration.

However, there are scenarios when updates can be processed incrementally
even if no Southbound DB transaction is available. One example, often
seen in the field, is when the MAC_Binding table is updated. Currently
such an update received while a transaction is still committing would
trigger a forced recompute.

To minimize the number of forced recomputes, we now call
engine_run(false), i.e., try to process updates incrementally without
allowing recompute, also when ovnsb_idl_txn == NULL. This is safe
because ovnsb_idl_txn is not used by change_handlers and run handlers
are not allowed to execute when calling engine_run(false).

To make sure that other users of ovnsb_idl_txn, like pinctrl_run(),
get a chance to run as soon as the transaction is completed, if the
engine has successfully run and ovnsb_idl_txn is NULL we trigger an
immediate wake and a new iteration of the processing loop.

CC: Han Zhou <[email protected]>
CC: Numan Siddique <[email protected]>
Signed-off-by: Dumitru Ceara <[email protected]>
---
 controller/ovn-controller.c | 13 +++++++++++++
 lib/inc-proc-eng.h          |  8 +++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 5874776..4a27d5f 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -2100,6 +2100,14 @@ main(int argc, char *argv[])
                         } else {
                             engine_run(true);
                         }
+                    } else {
+                        /* Even if there's no SB DB transaction available,
+                         * try to run the engine so that we can handle any
+                         * incremental changes that don't require a recompute.
+                         * If a recompute is required, the engine will abort,
+                         * triggerring a full run in the next iteration.
+                         */
+                        engine_run(false);
                     }
                     stopwatch_stop(CONTROLLER_LOOP_STOPWATCH_NAME,
                                    time_msec());
@@ -2168,6 +2176,11 @@ main(int argc, char *argv[])
                          "br_int %p, chassis %p", br_int, chassis);
                 engine_set_force_recompute(true);
                 poll_immediate_wake();
+            } else if (!ovnsb_idl_txn) {
+                VLOG_DBG("engine ran, no SB DB transaction available, "
+                         "trigger an immediate loop run: "
+                         "br_int %p, chassis %p", br_int, chassis);
+                poll_immediate_wake();
             } else {
                 engine_set_force_recompute(false);
             }
diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h
index 5b92971..2f90b0a 100644
--- a/lib/inc-proc-eng.h
+++ b/lib/inc-proc-eng.h
@@ -189,7 +189,13 @@ void engine_add_input(struct engine_node *node, struct 
engine_node *input,
  * iteration, and the change can't be tracked across iterations */
 void engine_set_force_recompute(bool val);
 
-const struct engine_context * engine_get_context(void);
+/* Return the current engine_context. The values in the context can be NULL
+ * if the engine is run with allow_recompute == false in the current
+ * iteration.
+ * Therefore, it is the responsibility of the caller to check the context
+ * values when called from change_handlers.
+ */
+const struct engine_context *engine_get_context(void);
 
 void engine_set_context(const struct engine_context *);
 
-- 
1.8.3.1

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to