From: Numan Siddique <num...@ovn.org>

When a logical switch has only router ports and if a new VIF port is
added, both northd engine and lflow engine handle this change
incrementally, but it misses out on adding a few logical flows
where we have checks like :
   if (od->n_router_ports != od->nbs->n_ports) {
        ds_put_format(&actions, "clone {outport = %s; output; }; "
                                "outport = \""MC_FLOOD_L2"\"; output;",
                      patch_op->json_key);
        ....
   } else {
       ds_put_format(&actions, "outport = %s; output;", patch_op->json_key);
   }

The same issue is seen when a VIF port is deleted and after which the
logical switch has only router ports.

This patch fixes this issue by falling back to full recompute of northd
engine node.  It is possible to handle these changes incrementally
in northd engine node but fall back to full recompute in lflow engine
node.  But this patch goes for a simpler fix.  This can be optimized
later if required.

Fixes: b337750e45be ("northd: Incremental processing of VIF changes in 'northd' 
node.")
CC: Han Zhou <hz...@ovn.org>
Signed-off-by: Numan Siddique <num...@ovn.org>
---
 northd/northd.c     | 10 ++++++++++
 tests/ovn-northd.at | 28 ++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/northd/northd.c b/northd/northd.c
index b9605862e..462fa83ca 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -5120,6 +5120,11 @@ northd_handle_ls_changes(struct ovsdb_idl_txn 
*ovnsb_idl_txn,
             op->visited = false;
         }
 
+        bool only_rports = (od->n_router_ports == hmap_count(&od->ports));
+        if (only_rports) {
+            goto fail;
+        }
+
         /* Compare the individual ports in the old and new Logical Switches */
         for (size_t j = 0; j < changed_ls->n_ports; ++j) {
             struct nbrec_logical_switch_port *new_nbsp = changed_ls->ports[j];
@@ -5201,6 +5206,11 @@ northd_handle_ls_changes(struct ovsdb_idl_txn 
*ovnsb_idl_txn,
             }
         }
 
+        only_rports = (od->n_router_ports == hmap_count(&od->ports));
+        if (only_rports) {
+            goto fail_clean_deleted;
+        }
+
         if (!ovs_list_is_empty(&ls_change->added_ports) ||
             !ovs_list_is_empty(&ls_change->updated_ports) ||
             !ovs_list_is_empty(&ls_change->deleted_ports)) {
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 3e06f14c9..912aa5431 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -9589,6 +9589,34 @@ check_recompute_counter 0 0
 
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
+check ovn-nbctl --wait=hv ls-del ls0
+check ovn-nbctl ls-add ls0
+check ovn-nbctl --wait=sb lr-add lr0
+ovn-nbctl lrp-add lr0 lr0-ls0 00:00:00:00:ff:01 192.168.0.1/24
+ovn-nbctl lsp-add ls0 ls0-lr0
+ovn-nbctl lsp-set-type ls0-lr0 router
+ovn-nbctl lsp-set-addresses ls0-lr0 router
+check ovn-nbctl --wait=sb lsp-set-options ls0-lr0 router-port=lr0-ls0
+
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+
+ovn-nbctl lb-add lb0 192.168.0.10:80 10.0.0.10:8080
+check ovn-nbctl --wait=sb ls-lb-add ls0 lb0
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+
+check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
+# Add a lsp.  northd and lflow engine should recompute since this is
+# the first lsp added after the router ports.
+check ovn-nbctl --wait=hv lsp-add ls0 lsp0-1 -- lsp-set-addresses lsp0-1 
"aa:aa:aa:00:00:01 192.168.0.11"
+check_recompute_counter 1 1
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+
+# Delete the lsp.  northd and lflow engine should recompute.
+check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
+check ovn-nbctl lsp-del lsp0-1
+check_recompute_counter 1 1
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+
 OVN_CLEANUP([hv1])
 AT_CLEANUP
 ])
-- 
2.40.1

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to