[ovs-dev] [PATCH ovn] ovn-controller: Don't clear the lflow resources ref during flow_output_run

[numans at ovn.org]

From: Numan Siddique <numans at ovn.org>

After the patch [1], which added caching of lflow expr, the lflow_resource_ref
is not rebuilt properly when lflow_run() is called. If a lflow is already cached
in lflow expr cache, then the lflow_resource_ref is not updated.
But flow_output_run() clears the lflow_resource_ref cache before calling lflow_run().

This results in incorrect OF flows present in the switch even if the
address set/port group references have changed.

This patch fixes this issue by not clearing the lflow_resource_ref cache.
This cache gets cleaned up during lflow change handler or in lflow_run().

[1] - 8795bec737b9("ovn-controller: Cache logical flow expr tree for each lflow.")
Fixes: 8795bec737b9("ovn-controller: Cache logical flow expr tree for each lflow.")

Signed-off-by: Numan Siddique <numans at ovn.org>
---
 controller/lflow.c          | 3 +++
 controller/ovn-controller.c | 2 --
 tests/ovn.at                | 4 ++++
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/controller/lflow.c b/controller/lflow.c
index 79d89131a..072b2f1f1 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -924,6 +924,9 @@ lflow_run(struct lflow_ctx_in *l_ctx_in, struct lflow_ctx_out *l_ctx_out)
     SBREC_LOGICAL_FLOW_TABLE_FOR_EACH_TRACKED (lflow,
                                                l_ctx_in->logical_flow_table) {
         if (sbrec_logical_flow_is_deleted(lflow)) {
+            /* Delete entries from lflow resource reference. */
+            lflow_resource_destroy_lflow(l_ctx_out->lfrr,
+                                         &lflow->header_.uuid);
             struct lflow_expr *le =
                 lflow_expr_get(l_ctx_out->lflow_expr_cache, lflow);
             if (le) {
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 4d245ca28..b3be5c2db 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -1441,7 +1441,6 @@ en_flow_output_run(struct engine_node *node, void *data)
     struct ovn_extend_table *group_table = &fo->group_table;
     struct ovn_extend_table *meter_table = &fo->meter_table;
     uint32_t *conj_id_ofs = &fo->conj_id_ofs;
-    struct lflow_resource_ref *lfrr = &fo->lflow_resource_ref;
 
     static bool first_run = true;
     if (first_run) {
@@ -1450,7 +1449,6 @@ en_flow_output_run(struct engine_node *node, void *data)
         ovn_desired_flow_table_clear(flow_table);
         ovn_extend_table_clear(group_table, false /* desired */);
         ovn_extend_table_clear(meter_table, false /* desired */);
-        lflow_resource_clear(lfrr);
     }
 
     *conj_id_ofs = 1;
diff --git a/tests/ovn.at b/tests/ovn.at
index ea6760e1f..254645a3a 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -13778,6 +13778,10 @@ for i in 1 2 3; do
     n_flows_after=`ovs-ofctl dump-flows br-int | grep "10.1.2.10" | wc -l`
     AT_CHECK([test $(expr $n_flows_before \* 2) = $n_flows_after], [0], [ignore])
 
+    # Trigger full recompute. Creating a chassis would trigger full recompute.
+    ovn-sbctl chassis-add tst geneve 127.0.0.4
+    ovn-sbctl chassis-del tst
+
     # Remove an ACL
     ovn-nbctl --wait=hv acl-del ls1 to-lport 200 \
             'outport=="lp2" && ip4 && ip4.src == $as1'
-- 
2.24.1