From ab959077c3dce4bd856f3d6bf633779a41dccdff Mon Sep 17 00:00:00 2001

From: Dumitru Ceara <dceara@redhat.com>

Date: Mon, 9 Dec 2019 09:12:18 +0100

Subject: [PATCH ovn] ovn-controller: Run I-P engine even when no SB txn is

 available.

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.

This patch performs only incremental processing when the SB DB txn

is NULL, which means executing change_handler() methods

only, without calling the run() methods of the I-P engine nodes.

Assuming that some change handlers need to update the DB and that

some don't, if the SB DB txn is NULL:

- if the incoming change doesn't trigger a SB DB update (currently true

  for all existing change handlers) then the change handler might

  complete without aborting if it doesn't trigger a run() of the node.

- if the incoming change tries to perform a SB DB update, the change

  handler should return false (SB DB txn is NULL) triggering the engine

  to call run() and abort computation.

CC: Han Zhou <hzhou@ovn.org>

CC: Numan Siddique <numans@ovn.org>

Signed-off-by: Dumitru Ceara <dceara@redhat.com>

Signed-off-by: Han Zhou <hzhou@ovn.org>

(cherry picked from upstream commit e2ab60e3a7c60f3adb8da40e4d1cfeb890d6f80e)

Change-Id: Ifdc87b925fc3e9c8b3c22d9b853ac27f2480f388

---

 ovn/controller/ovn-controller.c |  8 ++++++++

 ovn/lib/inc-proc-eng.h          | 15 ++++++++++++++-

 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c

index cb23bc8..c8470ce 100644

--- a/ovn/controller/ovn-controller.c

+++ b/ovn/controller/ovn-controller.c

@@ -2059,6 +2059,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());

diff --git a/ovn/lib/inc-proc-eng.h b/ovn/lib/inc-proc-eng.h

index 9cc99a3..c62f3bc 100644

--- a/ovn/lib/inc-proc-eng.h

+++ b/ovn/lib/inc-proc-eng.h

@@ -84,6 +84,10 @@ struct engine_node_input {

      * evaluated against all the other inputs. Returns:

      *  - true: if change can be handled

      *  - false: if change cannot be handled (indicating full recompute needed)

+     * A change handler can also call engine_get_context() but it must make

+     * sure the txn pointers returned by it are non-NULL. In case the change

+     * handler needs to use the txn pointers returned by engine_get_context(),

+     * and the pointers are NULL, the change handler MUST return false.

      */

     bool (*change_handler)(struct engine_node *node, void *data);

 };

@@ -133,6 +137,9 @@ struct engine_node {

     /* Fully processes all inputs of this node and regenerates the data

      * of this node. The pointer to the node's data is passed as argument.

+     * 'run' handlers can also call engine_get_context() and the

+     * implementation guarantees that the txn pointers returned

+     * engine_get_context() are not NULL and valid.

      */

     void (*run)(struct engine_node *node, void *data);

@@ -189,7 +196,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