[ovs-dev] [PATCH ovn] ic: remove port_binding on ts deletion

Vladislav Odintsov odivlad at gmail.com
Tue Aug 24 18:44:42 UTC 2021


When IC port_binding exists and transit switch is deleted,
the orphan port_binding if left in the IC_SB_DB.

This patch fixes such situation and adds test for this case.

Signed-off-by: Vladislav Odintsov <odivlad at gmail.com>
---
 ic/ovn-ic.c     | 35 +++++++++++++++++++++++++++++++--
 tests/ovn-ic.at | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+), 2 deletions(-)

diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
index 75e798cd1..e80cd34ca 100644
--- a/ic/ovn-ic.c
+++ b/ic/ovn-ic.c
@@ -66,6 +66,7 @@ struct ic_context {
     struct ovsdb_idl_index *nbrec_port_by_name;
     struct ovsdb_idl_index *sbrec_chassis_by_name;
     struct ovsdb_idl_index *sbrec_port_binding_by_name;
+    struct ovsdb_idl_index *icsbrec_port_binding_by_az;
     struct ovsdb_idl_index *icsbrec_port_binding_by_ts;
     struct ovsdb_idl_index *icsbrec_route_by_ts;
 };
@@ -174,7 +175,7 @@ allocate_ts_dp_key(struct hmap *dp_tnlids)
 }
 
 static void
-ts_run(struct ic_context *ctx)
+ts_run(struct ic_context *ctx, const struct icsbrec_availability_zone *az)
 {
     const struct icnbrec_transit_switch *ts;
 
@@ -239,8 +240,25 @@ ts_run(struct ic_context *ctx)
      * SB, to avoid uncommitted ISB datapath tunnel key to be synced back to
      * AZ. */
     if (ctx->ovnisb_txn) {
+        struct shash isb_pbs = SHASH_INITIALIZER(&isb_pbs);
+        const struct icsbrec_port_binding *isb_pb;
+        const struct icsbrec_port_binding *isb_pb_key =
+            icsbrec_port_binding_index_init_row(
+                ctx->icsbrec_port_binding_by_az);
+
+        icsbrec_port_binding_index_set_availability_zone(isb_pb_key, az);
+
+        ICSBREC_PORT_BINDING_FOR_EACH (isb_pb, ctx->ovnisb_idl) {
+            shash_add(&isb_pbs, isb_pb->transit_switch, isb_pb);
+        }
+
         /* Create ISB Datapath_Binding */
         ICNBREC_TRANSIT_SWITCH_FOR_EACH (ts, ctx->ovninb_idl) {
+            while (shash_find_and_delete(&isb_pbs, ts->name)) {
+                /* There may be multiple Port_Bindings */
+                continue;
+            }
+
             isb_dp = shash_find_and_delete(&isb_dps, ts->name);
             if (!isb_dp) {
                 /* Allocate tunnel key */
@@ -260,6 +278,13 @@ ts_run(struct ic_context *ctx)
         SHASH_FOR_EACH (node, &isb_dps) {
             icsbrec_datapath_binding_delete(node->data);
         }
+
+        SHASH_FOR_EACH (node, &isb_pbs) {
+            icsbrec_port_binding_delete(node->data);
+        }
+
+        icsbrec_port_binding_index_destroy_row(isb_pb_key);
+        shash_destroy(&isb_pbs);
     }
     ovn_destroy_tnlids(&dp_tnlids);
     shash_destroy(&isb_dps);
@@ -1493,7 +1518,7 @@ ovn_db_run(struct ic_context *ctx)
         return;
     }
 
-    ts_run(ctx);
+    ts_run(ctx, az);
     gateway_run(ctx, az);
     port_binding_run(ctx, az);
     route_run(ctx, az);
@@ -1684,6 +1709,11 @@ main(int argc, char *argv[])
     struct ovsdb_idl_index *sbrec_chassis_by_name
         = ovsdb_idl_index_create1(ovnsb_idl_loop.idl,
                                   &sbrec_chassis_col_name);
+
+    struct ovsdb_idl_index *icsbrec_port_binding_by_az
+        = ovsdb_idl_index_create1(ovnisb_idl_loop.idl,
+                                  &icsbrec_port_binding_col_availability_zone);
+
     struct ovsdb_idl_index *icsbrec_port_binding_by_ts
         = ovsdb_idl_index_create1(ovnisb_idl_loop.idl,
                                   &icsbrec_port_binding_col_transit_switch);
@@ -1731,6 +1761,7 @@ main(int argc, char *argv[])
                 .nbrec_port_by_name = nbrec_port_by_name,
                 .sbrec_port_binding_by_name = sbrec_port_binding_by_name,
                 .sbrec_chassis_by_name = sbrec_chassis_by_name,
+                .icsbrec_port_binding_by_az = icsbrec_port_binding_by_az,
                 .icsbrec_port_binding_by_ts = icsbrec_port_binding_by_ts,
                 .icsbrec_route_by_ts = icsbrec_route_by_ts,
             };
diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
index ee78f4794..b6a8edb68 100644
--- a/tests/ovn-ic.at
+++ b/tests/ovn-ic.at
@@ -64,6 +64,58 @@ OVN_CLEANUP_IC([az1])
 AT_CLEANUP
 ])
 
+
+AT_SETUP([ovn-ic -- port bindings])
+
+ovn_init_ic_db
+net_add n1
+
+# 1 GW per AZ
+for i in 1 2; do
+    az=az$i
+    ovn_start $az
+    sim_add gw-$az
+    as gw-$az
+    check ovs-vsctl add-br br-phys
+    ovn_az_attach $az n1 br-phys 192.168.1.$i
+    check ovs-vsctl set open . external-ids:ovn-is-interconn=true
+done
+
+ovn_as az1
+
+# create transit switch and connect to LR
+check ovn-ic-nbctl ts-add ts1
+check ovn-nbctl lr-add lr1
+check ovn-nbctl lrp-add lr1 lrp1 00:00:00:00:00:01 10.0.0.1/24
+check ovn-nbctl lrp-set-gateway-chassis lrp1 gw-az1
+
+check ovn-nbctl lsp-add ts1 lsp1 -- \
+    lsp-set-addresses lsp1 router -- \
+    lsp-set-type lsp1 router -- \
+    lsp-set-options lsp1 router-port=lrp1
+
+OVS_WAIT_UNTIL([ovn-sbctl list datapath_binding | grep interconn-ts | grep ts1])
+
+# check port binding appeared
+AT_CHECK([ovn-ic-sbctl show | grep -A2 lsp1], [0], [dnl
+        port lsp1
+            transit switch: ts1
+            address: [["00:00:00:00:00:01 10.0.0.1/24"]]
+])
+
+# remove transit switch and check if port_binding is deleted
+check ovn-ic-nbctl ts-del ts1
+OVS_WAIT_UNTIL([test -z "$(ovn-ic-sbctl show | grep lsp1)"])
+
+for i in 1 2; do
+    az=az$i
+    OVN_CLEANUP_SBOX(gw-$az)
+    OVN_CLEANUP_AZ([$az])
+done
+OVN_CLEANUP_IC
+AT_CLEANUP
+
+
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([ovn-ic -- gateway sync])
 
-- 
2.30.0



More information about the dev mailing list