There was a potential ovs_assert when exiting ovn-controller:
    controller/if-status.c:261: assertion 
shash_is_empty(&mgr->ovn_uninstall_hash) failed in if_status_mgr_clear()

Fixes: 395eac485b87 ("ovn-controller: fixed ovn-installed not always properly 
added or removed.")

Signed-off-by: Xavier Simonart <xsimo...@redhat.com>
---
 controller/if-status.c | 24 +++++++++---------------
 tests/system-ovn.at    | 23 +++++++++++++++++++----
 2 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/controller/if-status.c b/controller/if-status.c
index 9a7488057..ada78a18b 100644
--- a/controller/if-status.c
+++ b/controller/if-status.c
@@ -219,7 +219,8 @@ ovs_iface_create(struct if_status_mgr *, const char 
*iface_id,
 static void add_to_ovn_uninstall_hash(struct if_status_mgr *, const char *,
                                       const struct uuid *);
 static void ovs_iface_destroy(struct if_status_mgr *, struct ovs_iface *);
-static void ovn_uninstall_hash_destroy(struct if_status_mgr *mgr, char *name);
+static void ovn_uninstall_hash_destroy(struct if_status_mgr *mgr,
+                                       struct shash_node *node);
 static void ovs_iface_set_state(struct if_status_mgr *, struct ovs_iface *,
                                 enum if_state);
 
@@ -256,7 +257,7 @@ if_status_mgr_clear(struct if_status_mgr *mgr)
     ovs_assert(shash_is_empty(&mgr->ifaces));
 
     SHASH_FOR_EACH_SAFE (node, &mgr->ovn_uninstall_hash) {
-        ovn_uninstall_hash_destroy(mgr, node->data);
+        ovn_uninstall_hash_destroy(mgr, node);
     }
     ovs_assert(shash_is_empty(&mgr->ovn_uninstall_hash));
 
@@ -789,20 +790,13 @@ ovs_iface_destroy(struct if_status_mgr *mgr, struct 
ovs_iface *iface)
 }
 
 static void
-ovn_uninstall_hash_destroy(struct if_status_mgr *mgr, char *name)
+ovn_uninstall_hash_destroy(struct if_status_mgr *mgr, struct shash_node *node)
 {
-    struct shash_node *node = shash_find(&mgr->ovn_uninstall_hash, name);
-    char *node_name = NULL;
-    if (node) {
-        free(node->data);
-        VLOG_DBG("Interface name %s destroy", name);
-        node_name = shash_steal(&mgr->ovn_uninstall_hash, node);
-        ovn_uninstall_hash_account_mem(name, true);
-        free(node_name);
-    } else {
-        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
-        VLOG_WARN_RL(&rl, "Interface name %s not found", name);
-    }
+    free(node->data);
+    VLOG_DBG("Interface name %s destroy", node->name);
+    char *node_name = shash_steal(&mgr->ovn_uninstall_hash, node);
+    ovn_uninstall_hash_account_mem(node_name, true);
+    free(node_name);
 }
 
 static void
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index ddb3d14e9..e9d889f43 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -11325,7 +11325,25 @@ check_ovn_installed
 check_ports_up
 check_ports_bound
 
-OVS_APP_EXIT_AND_WAIT([ovn-controller])
+AS_BOX(["Leave some ovn-installed while closing ovn-controller"])
+# Block IDL from ovn-controller to OVSDB
+stop_ovsdb_controller_updates $TCP_PORT
+remove_iface_id vif2
+ensure_controller_run
+
+# OVSDB should now be seen as read-only by ovn-controller
+remove_iface_id vif1
+check ovn-nbctl --wait=hv sync
+
+# Stop ovsdb before ovn-controller to ensure it's not updated
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
+/connection dropped.*/d"])
+
+# Don't use OVS_APP_EXIT... to use --restart to avoid cleaning up the 
databases.
+TMPPID=$(cat $OVS_RUNDIR/ovn-controller.pid)
+check ovs-appctl -t ovn-controller exit --restart
+OVS_WAIT_WHILE([kill -0 $TMPPID 2>/dev/null])
 
 as ovn-sb
 OVS_APP_EXIT_AND_WAIT([ovsdb-server])
@@ -11336,9 +11354,6 @@ OVS_APP_EXIT_AND_WAIT([ovsdb-server])
 as northd
 OVS_APP_EXIT_AND_WAIT([ovn-northd])
 
-as
-OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
-/connection dropped.*/d"])
 AT_CLEANUP
 ])
 
-- 
2.31.1

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

Reply via email to