From e5113a7fd91b42ff16ff675660cdc27fae0bd80c Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Wed, 2 Dec 2020 17:30:24 +0100
Subject: [PATCH] ovs: clean up interfaces from ovsdb at startup

During shutdown, NM always tries to remove from ovsdb all bridges,
ports, interfaces that it previously added. Currently NM doesn't run
the main loop during shutdown and so it's not possible to perform
asynchronous operations. In particular, the NMOvsdb singleton is
disposed in a destructor where it's not possible to send out all the
queued deletions.

The result is that NM deletes only one OVS interface, keeping the
others. This needs to be fixed, but requires a rework of the shutdown
procedure that involves many parts of NM.

Even when a better shutdown procedure will be implemented, we should
support an unclean shutdown caused by e.g. a kernel panic or a NM
crash. In these cases, the interfaces added by NM would still linger
in the ovsdb.

Delete all those interface at NM startup. If there are connections
profiles for them, NM will create them again.

https://bugzilla.redhat.com/show_bug.cgi?id=1861296
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/700
---
 src/devices/ovs/nm-ovsdb.c | 46 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c
index 08bf57388..78152cd8c 100644
--- a/src/devices/ovs/nm-ovsdb.c
+++ b/src/devices/ovs/nm-ovsdb.c
@@ -127,6 +127,7 @@ typedef struct {
     GHashTable *bridges;    /* bridge uuid => OpenvswitchBridge */
     char *      db_uuid;
     guint       num_failures;
+    bool        cleanup_done : 1;
 } NMOvsdbPrivate;
 
 struct _NMOvsdb {
@@ -2222,6 +2223,9 @@ ovsdb_disconnect(NMOvsdb *self, gboolean retry, gboolean is_disposing)
 
     _LOGD("disconnecting from ovsdb, retry %d", retry);
 
+    /* FIXME(shutdown): NMOvsdb should process the pending calls before
+     * shutting down, and cancel the remaining calls after the timeout. */
+
     if (retry) {
         if (!c_list_is_empty(&priv->calls_lst_head)) {
             call          = c_list_first_entry(&priv->calls_lst_head, OvsdbMethodCall, calls_lst);
@@ -2250,6 +2254,46 @@ ovsdb_disconnect(NMOvsdb *self, gboolean retry, gboolean is_disposing)
         ovsdb_try_connect(self);
 }
 
+static void
+_del_initial_iface_cb(GError *error, gpointer user_data)
+{
+    NMOvsdb *     self;
+    gs_free char *ifname = NULL;
+
+    nm_utils_user_data_unpack(user_data, &self, &ifname);
+
+    if (nm_utils_error_is_cancelled_or_disposing(error))
+        return;
+
+    if (error)
+        _LOGD("failed to delete initial interface '%s': %s", ifname, error->message);
+}
+
+static void
+ovsdb_cleanup_initial_interfaces(NMOvsdb *self)
+{
+    NMOvsdbPrivate *            priv = NM_OVSDB_GET_PRIVATE(self);
+    const OpenvswitchInterface *interface;
+    NMUtilsUserData *           data;
+    GHashTableIter              iter;
+
+    if (priv->cleanup_done)
+        return;
+    priv->cleanup_done = TRUE;
+
+    /* Delete OVS interfaces added by NM. Bridges and ports and
+     * not considered because they are deleted automatically
+     * when no interface is present. */
+    g_hash_table_iter_init(&iter, self->_priv.interfaces);
+    while (g_hash_table_iter_next(&iter, NULL, (gpointer *) &interface)) {
+        if (interface->connection_uuid) {
+            _LOGT("deleting initial interface '%s'", interface->name);
+            data = nm_utils_user_data_pack(self, g_strdup(interface->name));
+            nm_ovsdb_del_interface(self, interface->name, _del_initial_iface_cb, data);
+        }
+    }
+}
+
 static void
 _monitor_bridges_cb(NMOvsdb *self, json_t *result, GError *error, gpointer user_data)
 {
@@ -2264,6 +2308,8 @@ _monitor_bridges_cb(NMOvsdb *self, json_t *result, GError *error, gpointer user_
     /* Treat the first response the same as the subsequent "update"
      * messages we eventually get. */
     ovsdb_got_update(self, result);
+
+    ovsdb_cleanup_initial_interfaces(self);
 }
 
 static void