[ovs-dev] [PATCH v2 ovn 3/3] ovn-northd: Remove Port_Binding stale references to HA_Chassis_Group.

Dumitru Ceara dceara at redhat.com
Thu Apr 30 13:14:33 UTC 2020


If a logical switch port of type "external" that has ha_chassis_group set
is deleted from a logical switch and a port with the same name is added
to a different logical switch in the same transaction, the SB Port_Binding
record corresponding to the logical port is reused but the ha_chassis_group
field is not cleared.

One way to reproduce the issue is:
$ ovn-nbctl ls-add ls1
$ ovn-nbctl ls-add ls2
$ ovn-nbctl lsp-add ls1 lsp1
$ ovn-nbctl lsp-add ls2 lsp2
$ ovn-nbctl lsp-set-type lsp2 external
$ ovn-nbctl ha-chassis-group-add chg1
$ ovn-nbctl ha-chassis-group-add-chassis chg1 chassis-1 30
$ chg1_uuid=$(ovn-nbctl --bare --columns _uuid list ha_Chassis_Group .)
$ ovn-nbctl set logical_switch_port lsp2 ha_chassis_group=${chg1_uuid}
$ ovn-nbctl lsp-del lsp2 -- lsp-add ls1 lsp2

To avoid that, we now clear the Port_Binding ha_chassis_group field when
the logical port corresponding to the port binding is not of type
"external" anymore.

Reported-by: Numan Siddique <numans at ovn.org>
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
---
 northd/ovn-northd.c |   18 ++++++++----------
 tests/ovn-northd.at |   26 ++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index dad6a45..c75a80b 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -2934,16 +2934,14 @@ ovn_port_update_sbrec(struct northd_context *ctx,
 
             sbrec_port_binding_set_nat_addresses(op->sb, NULL, 0);
 
-            if (!strcmp(op->nbsp->type, "external")) {
-                if (op->nbsp->ha_chassis_group) {
-                    sync_ha_chassis_group_for_sbpb(
-                        ctx, op->nbsp->ha_chassis_group,
-                        sbrec_chassis_by_name, op->sb);
-                    sset_add(active_ha_chassis_grps,
-                             op->nbsp->ha_chassis_group->name);
-                } else {
-                    sbrec_port_binding_set_ha_chassis_group(op->sb, NULL);
-                }
+            if (!strcmp(op->nbsp->type, "external") &&
+                    op->nbsp->ha_chassis_group) {
+                sync_ha_chassis_group_for_sbpb(ctx, op->nbsp->ha_chassis_group,
+                                               sbrec_chassis_by_name, op->sb);
+                sset_add(active_ha_chassis_grps,
+                         op->nbsp->ha_chassis_group->name);
+            } else {
+                sbrec_port_binding_set_ha_chassis_group(op->sb, NULL);
             }
         } else {
             const char *chassis = NULL;
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 4823d14..0f10689 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -1444,3 +1444,29 @@ logical_port=lsp2)
 AT_CHECK([test ${pb1_key} != ${pb2_key}])
 
 AT_CLEANUP
+
+AT_SETUP([ovn -- check reconcile stale Ha_Chassis_Group])
+ovn_start
+
+ovn-nbctl ls-add ls1
+ovn-nbctl ls-add ls2
+ovn-nbctl lsp-add ls1 lsp1
+ovn-nbctl lsp-add ls2 lsp2
+
+ovn-nbctl lsp-set-type lsp2 external
+
+ovn-nbctl ha-chassis-group-add chg1
+ovn-nbctl ha-chassis-group-add-chassis chg1 chassis-1 30
+
+chg1_uuid=$(ovn-nbctl --bare --columns _uuid list Ha_Chassis_Group .)
+ovn-nbctl set logical_switch_port lsp2 ha_chassis_group=${chg1_uuid}
+AT_CHECK([ovn-nbctl --wait=sb sync], [0])
+
+# Move lsp2 from ls2 to ls1. This should also remove the SB HA_Chassis_Group
+# record.
+ovn-nbctl lsp-del lsp2 -- lsp-add ls1 lsp2
+AT_CHECK([ovn-nbctl --wait=sb sync], [0])
+
+AT_CHECK([test 0 = $(ovn-sbctl list Ha_Chassis_Group | wc -l)])
+
+AT_CLEANUP



More information about the dev mailing list