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: 0-day Robot <robot@bytheb.org>

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());

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 *);