Skip to content

Commit

Permalink
ovn-controller: Fix potential assert when exiting.
Browse files Browse the repository at this point in the history
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: 395eac4 ("ovn-controller: fixed ovn-installed not always properly added or removed.")

Signed-off-by: Xavier Simonart <xsimonar@redhat.com>
Acked-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
(cherry picked from commit f958cc7)
  • Loading branch information
simonartxavier authored and dceara committed Sep 19, 2024
1 parent 7195dcd commit 231d1ca
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 19 deletions.
24 changes: 9 additions & 15 deletions controller/if-status.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -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
Expand Down
23 changes: 19 additions & 4 deletions tests/system-ovn.at
Original file line number Diff line number Diff line change
Expand Up @@ -11327,7 +11327,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])
Expand All @@ -11338,9 +11356,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
])

Expand Down

0 comments on commit 231d1ca

Please sign in to comment.