controller: Clear tunnels from old integration bridge

After integration bridge change the tunnels would
stay on the old bridge preventing new tunnels creation
and disrupting traffic. Detect the bridge change
and clear the tunnels from the old integration bridge.

Reported-at: https://bugzilla.redhat.com/2173635
Signed-off-by: Ales Musil <amusil@redhat.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
(cherry picked from commit 2afc098)

controller/encaps.c

@@ -356,6 +356,20 @@ chassis_tzones_overlap(const struct sset *transport_zones,
     return false;
 }
 
+static void
+clear_old_tunnels(const struct ovsrec_bridge *old_br_int)
+{
+    for (size_t i = 0; i < old_br_int->n_ports; i++) {
+        const struct ovsrec_port *port = old_br_int->ports[i];
+        const char *id = smap_get(&port->external_ids, "ovn-chassis-id");
+        if (id) {
+            VLOG_DBG("Clearing old tunnel port \"%s\" (%s) from bridge "
+                     "\"%s\".", port->name, id, old_br_int->name);
+            ovsrec_bridge_update_ports_delvalue(old_br_int, port);
+        }
+    }
+}
+
 void
 encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
            const struct ovsrec_bridge_table *bridge_table,
@@ -364,12 +378,33 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
            const struct sbrec_chassis *this_chassis,
            const struct sbrec_sb_global *sbg,
            const struct ovsrec_open_vswitch_table *ovs_table,
-           const struct sset *transport_zones)
+           const struct sset *transport_zones,
+           const char *br_int_name)
 {
     if (!ovs_idl_txn || !br_int) {
         return;
     }
 
+    if (!br_int_name) {
+        /* The controller has just started, we need to look through all
+         * bridges for old tunnel ports. */
+       const struct ovsrec_bridge *br;
+        OVSREC_BRIDGE_TABLE_FOR_EACH (br, bridge_table) {
+            if (!strcmp(br->name, br_int->name)) {
+                continue;
+            }
+            clear_old_tunnels(br);
+        }
+    } else if (strcmp(br_int_name, br_int->name)) {
+        /* The integration bridge was changed, clear tunnel ports from
+         * the old one. */
+        const struct ovsrec_bridge *old_br_int =
+            get_bridge(bridge_table, br_int_name);
+        if (old_br_int) {
+            clear_old_tunnels(old_br_int);
+        }
+    }
+
     const struct sbrec_chassis *chassis_rec;
     const struct ovsrec_bridge *br;
 

controller/encaps.h

@@ -36,7 +36,8 @@ void encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
                 const struct sbrec_chassis *,
                 const struct sbrec_sb_global *,
                 const struct ovsrec_open_vswitch_table *,
-                const struct sset *transport_zones);
+                const struct sset *transport_zones,
+                const char *br_int_name);
 
 bool encaps_cleanup(struct ovsdb_idl_txn *ovs_idl_txn,
                     const struct ovsrec_bridge *br_int);

controller/ovn-controller.c

@@ -497,6 +497,21 @@ get_ovs_chassis_id(const struct ovsrec_open_vswitch_table *ovs_table)
     return chassis_id;
 }
 
+static void
+consider_br_int_change(const struct ovsrec_bridge *br_int, char **current_name)
+{
+    ovs_assert(current_name);
+
+    if (!*current_name) {
+        *current_name = xstrdup(br_int->name);
+    }
+
+    if (strcmp(*current_name, br_int->name)) {
+        free(*current_name);
+        *current_name = xstrdup(br_int->name);
+    }
+}
+
 static void
 update_ssl_config(const struct ovsrec_ssl_table *ssl_table)
 {
@@ -3745,6 +3760,8 @@ main(int argc, char *argv[])
     char *ovn_version = ovn_get_internal_version();
     VLOG_INFO("OVN internal version is : [%s]", ovn_version);
 
+    char *current_br_int_name = NULL;
+
     /* Main loop. */
     exiting = false;
     restart = false;
@@ -3894,7 +3911,8 @@ main(int argc, char *argv[])
                                chassis,
                                sbrec_sb_global_first(ovnsb_idl_loop.idl),
                                ovs_table,
-                               &transport_zones);
+                               &transport_zones,
+                               current_br_int_name);
 
                     stopwatch_start(CONTROLLER_LOOP_STOPWATCH_NAME,
                                     time_msec());
@@ -4066,7 +4084,10 @@ main(int argc, char *argv[])
                     stopwatch_stop(IF_STATUS_MGR_RUN_STOPWATCH_NAME,
                                    time_msec());
                 }
-
+                /* The name needs to be reflected at the end of the block.
+                 * This allows us to detect br-int changes and act
+                 * accordingly. */
+                consider_br_int_change(br_int, &current_br_int_name);
             }
 
             if (!engine_has_run()) {
@@ -4248,6 +4269,7 @@ main(int argc, char *argv[])
     }
 
     free(ovn_version);
+    free(current_br_int_name);
     unixctl_server_destroy(unixctl);
     lflow_destroy();
     ofctrl_destroy();

tests/ovn.at

@@ -30918,3 +30918,86 @@ AT_CHECK([test -z "`grep disconnecting hv1/ovs-vswitchd.log`"])
 OVN_CLEANUP([hv1])
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([Re-create encap tunnels during integration bridge migration])
+ovn_start
+net_add n1
+
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+
+sim_add hv2
+as hv2
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.2
+
+check ovn-nbctl --wait=hv sync
+
+check_tunnel_port() {
+    local hv=$1
+    local br=$2
+    local id=$3
+
+    as $hv
+    OVS_WAIT_UNTIL([
+        test "$(ovs-vsctl --format=table --no-headings find port external_ids:ovn-chassis-id="$id" | wc -l)" = "1"
+    ])
+    local tunnel_id=$(ovs-vsctl --bare --columns _uuid find port external_ids:ovn-chassis-id="$id")
+    AT_CHECK([ovs-vsctl --bare --columns ports find bridge name="$br" | grep -q "$tunnel_id"])
+}
+
+# Check that both chassis have tunnel
+check_tunnel_port hv1 br-int hv2@192.168.0.2
+check_tunnel_port hv2 br-int hv1@192.168.0.1
+
+# Stop ovn-controller on hv1
+check as hv1 ovn-appctl -t ovn-controller exit --restart
+
+# The tunnel should remain intact
+check_tunnel_port hv1 br-int hv2@192.168.0.2
+
+# Change the bridge to br-int1 on hv1
+as hv1
+check ovs-vsctl add-br br-int1
+check ovs-vsctl set open . external_ids:ovn-bridge="br-int1"
+start_daemon ovn-controller --verbose="encaps:dbg"
+check ovn-nbctl --wait=hv sync
+
+# Check that the tunnel was created on br-int1 instead
+check_tunnel_port hv1 br-int1 hv2@192.168.0.2
+check grep -q "Clearing old tunnel port \"ovn-hv2-0\" (hv2@192.168.0.2) from bridge \"br-int\"" hv1/ovn-controller.log
+
+# Change the bridge to br-int1 on hv2
+as hv2
+check ovn-appctl vlog/set encaps:dbg
+check ovs-vsctl add-br br-int1
+check ovs-vsctl set open . external_ids:ovn-bridge="br-int1"
+check ovn-nbctl --wait=hv sync
+
+
+# Check that the tunnel was created on br-int1 instead
+check_tunnel_port hv2 br-int1 hv1@192.168.0.1
+check grep -q "Clearing old tunnel port \"ovn-hv1-0\" (hv1@192.168.0.1) from bridge \"br-int\"" hv2/ovn-controller.log
+
+# Stop ovn-controller on hv1
+check as hv1 ovn-appctl -t ovn-controller exit --restart
+
+# The tunnel should remain intact
+check_tunnel_port hv1 br-int1 hv2@192.168.0.2
+prev_id=$(ovs-vsctl --bare --columns _uuid find port external_ids:ovn-chassis-id="hv2@192.168.0.2")
+
+# Start the controller again
+start_daemon ovn-controller --verbose="encaps:dbg"
+check ovn-nbctl --wait=hv sync
+check_tunnel_port hv1 br-int1 hv2@192.168.0.2
+current_id=$(ovs-vsctl --bare --columns _uuid find port external_ids:ovn-chassis-id="hv2@192.168.0.2")
+
+# The tunnel should be the same after restart
+check test "$current_id" = "$prev_id"
+
+OVN_CLEANUP([hv1],[hv2])
+AT_CLEANUP
+])