binding: fixed ovn-installed not properly removed (migration)

Fixed another potential race condition causing ovn-controller to not properly remove ovn-install. Before this patch, ovn-installed was not removed if an hypervisor was receiving the port_binding->chassis change (to another hv) within the same idl as the requested-chassis change. This caused random failures to test "options:multiple requested-chassis for logical port" Signed-off-by: Xavier Simonart <[email protected]> Signed-off-by: Mark Michelson <[email protected]> Acked-by: Ales Musil <[email protected]> Acked-by: Mark Michelson <[email protected]>
ovn-org · Jul 19, 2023 · ca54c2f · ca54c2f
1 parent a4ad748
commit ca54c2f
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 5 deletions.
diff --git a/controller/binding.c b/controller/binding.c
@@ -1506,8 +1506,9 @@ release_lport(const struct sbrec_port_binding *pb,
         if (!release_lport_additional_chassis(pb, chassis_rec, sb_readonly)) {
             return false;
         }
+    } else {
+        VLOG_INFO("Releasing lport %s", pb->logical_port);
     }
-
     update_lport_tracking(pb, tracked_datapaths, false);
     if_status_mgr_release_iface(if_mgr, pb->logical_port);
     return true;
@@ -1616,8 +1617,16 @@ consider_vif_lport_(const struct sbrec_port_binding *pb,
     }
 
     if (pb->chassis == b_ctx_in->chassis_rec
-            || is_additional_chassis(pb, b_ctx_in->chassis_rec)) {
+            || is_additional_chassis(pb, b_ctx_in->chassis_rec)
+            || if_status_is_port_claimed(b_ctx_out->if_mgr,
+                                         pb->logical_port)) {
         /* Release the lport if there is no lbinding. */
+       if (lbinding_set && !can_bind) {
+            if_status_mgr_remove_ovn_installed(b_ctx_out->if_mgr,
+                               b_lport->lbinding->iface->name,
+                               &b_lport->lbinding->iface->header_.uuid);
+        }
+
         if (!lbinding_set || !can_bind) {
             return release_lport(pb, b_ctx_in->chassis_rec,
                                  !b_ctx_in->ovnsb_idl_txn,

diff --git a/controller/if-status.c b/controller/if-status.c
@@ -71,12 +71,12 @@ enum if_state {
                            * but not yet marked "up" in the binding module (in
                            * SB and OVS databases).
                            */
-    OIF_MARK_DOWN,        /* Released interface but not yet marked "down" in
-                           * the binding module (in SB and/or OVS databases).
-                           */
     OIF_INSTALLED,        /* Interface flows programmed in OVS and binding
                            * marked "up" in the binding module.
                            */
+    OIF_MARK_DOWN,        /* Released interface but not yet marked "down" in
+                           * the binding module (in SB and/or OVS databases).
+                           */
     OIF_UPDATE_PORT,      /* Logical ports need to be set down, and pb->chassis
                            * removed.
                            */
@@ -820,3 +820,16 @@ if_status_mgr_get_memory_usage(struct if_status_mgr *mgr,
     simap_increase(usage, "if_status_mgr_ifaces_state_usage-KB",
                    ROUND_UP(ifaces_state_usage, 1024) / 1024);
 }
+
+bool
+if_status_is_port_claimed(const struct if_status_mgr *mgr,
+                          const char *iface_id)
+{
+    struct ovs_iface *iface = shash_find_data(&mgr->ifaces, iface_id);
+    if (!iface || (iface->state > OIF_INSTALLED)) {
+        return false;
+    } else {
+        return true;
+    }
+}
+
diff --git a/controller/if-status.h b/controller/if-status.h
@@ -62,5 +62,7 @@ uint16_t if_status_mgr_iface_get_mtu(const struct if_status_mgr *mgr,
                                      const char *iface_id);
 bool if_status_mgr_iface_update(const struct if_status_mgr *mgr,
                                 const struct ovsrec_interface *iface_rec);
+bool if_status_is_port_claimed(const struct if_status_mgr *mgr,
+                               const char *iface_id);
 
 # endif /* controller/if-status.h */
diff --git a/tests/ovn.at b/tests/ovn.at
@@ -14499,6 +14499,35 @@ wait_column "$hv1_uuid" Port_Binding requested_additional_chassis logical_port=l
 check ovn-nbctl lsp-set-options lsp0 requested-chassis=hv2
 wait_column "" Port_Binding additional_encap logical_port=lsp0
 
+# Migration with some race conditions
+check ovn-nbctl lsp-set-options lsp0 \
+    requested-chassis=hv2,hv1
+wait_column "$hv2_uuid" Port_Binding chassis logical_port=lsp0
+wait_column "$hv2_uuid" Port_Binding requested_chassis logical_port=lsp0
+wait_column "$hv1_uuid" Port_Binding additional_chassis logical_port=lsp0
+wait_column "$hv1_uuid" Port_Binding requested_additional_chassis logical_port=lsp0
+
+# Check ovn-installed updated for both chassis
+wait_for_ports_up
+
+for hv in hv1 hv2; do
+    OVS_WAIT_UNTIL([test `as $hv ovs-vsctl get Interface lsp0 external_ids:ovn-installed` = '"true"'])
+done
+
+# Complete moving the binding to the new location
+sleep_controller hv2
+check ovn-nbctl lsp-set-options lsp0 requested-chassis=hv1
+
+wait_column "$hv1_uuid" Port_Binding chassis logical_port=lsp0
+wait_column "$hv1_uuid" Port_Binding requested_chassis logical_port=lsp0
+wait_column "" Port_Binding additional_chassis logical_port=lsp0
+wait_column "" Port_Binding requested_additional_chassis logical_port=lsp0
+wake_up_controller hv2
+# Check ovn-installed updated for main chassis and removed from additional chassis
+wait_for_ports_up
+OVS_WAIT_UNTIL([test `as hv1 ovs-vsctl get Interface lsp0 external_ids:ovn-installed` = '"true"'])
+OVS_WAIT_UNTIL([test x`as hv2 ovs-vsctl get Interface lsp0 external_ids:ovn-installed` = x])
+
 OVN_CLEANUP([hv1],[hv2])
 
 AT_CLEANUP