diff mbox series

[ovs-dev,v3,5/7] Don't touch tunnel ports from a different br-int

Message ID 20221004173103.2074411-6-ihrachys@redhat.com
State Superseded, archived
Headers show
Series Support 2+ controllers on the same vswitchd | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test fail github build: failed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed

Commit Message

Ihar Hrachyshka Oct. 4, 2022, 5:31 p.m. UTC
When multiple controllers are running using the same vswitchd,
controllers should delete only those tunnel ports that belong to the
integration bridge that is managed by the controller instance.

This makes sure multiple controllers don't step on each other when
running using the same vswitchd instance.

Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com>
---
 controller/encaps.c         | 42 ++++++++++------------
 controller/encaps.h         |  1 -
 controller/ovn-controller.c |  3 +-
 tests/ovn.at                | 70 +++++++++++++++++++++++++++++++++++++
 4 files changed, 90 insertions(+), 26 deletions(-)
diff mbox series

Patch

diff --git a/controller/encaps.c b/controller/encaps.c
index 06f498557..e29074c28 100644
--- a/controller/encaps.c
+++ b/controller/encaps.c
@@ -395,7 +395,6 @@  chassis_tzones_overlap(const struct sset *transport_zones,
 
 void
 encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
-           const struct ovsrec_bridge_table *bridge_table,
            const struct ovsrec_bridge *br_int,
            const struct sbrec_chassis_table *chassis_table,
            const struct sbrec_chassis *this_chassis,
@@ -408,7 +407,6 @@  encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
     }
 
     const struct sbrec_chassis *chassis_rec;
-    const struct ovsrec_bridge *br;
 
     struct tunnel_ctx tc = {
         .chassis = SHASH_INITIALIZER(&tc.chassis),
@@ -426,27 +424,25 @@  encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
     /* Collect all port names into tc.port_names.
      *
      * Collect all the OVN-created tunnels into tc.tunnel_hmap. */
-    OVSREC_BRIDGE_TABLE_FOR_EACH (br, bridge_table) {
-        for (size_t i = 0; i < br->n_ports; i++) {
-            const struct ovsrec_port *port = br->ports[i];
-            sset_add(&tc.port_names, port->name);
-
-            /*
-             * note that the id here is not just the chassis name, but the
-             * combination of <chassis_name><delim><encap_ip>
-             */
-            const char *id = smap_get(&port->external_ids, "ovn-chassis-id");
-            if (id) {
-                if (!shash_find(&tc.chassis, id)) {
-                    struct chassis_node *chassis = xzalloc(sizeof *chassis);
-                    chassis->bridge = br;
-                    chassis->port = port;
-                    shash_add_assert(&tc.chassis, id, chassis);
-                } else {
-                    /* Duplicate port for ovn-chassis-id.  Arbitrarily choose
-                     * to delete this one. */
-                    ovsrec_bridge_update_ports_delvalue(br, port);
-                }
+    for (size_t i = 0; i < br_int->n_ports; i++) {
+        const struct ovsrec_port *port = br_int->ports[i];
+        sset_add(&tc.port_names, port->name);
+
+        /*
+         * note that the id here is not just the chassis name, but the
+         * combination of <chassis_name><delim><encap_ip>
+         */
+        const char *id = smap_get(&port->external_ids, "ovn-chassis-id");
+        if (id) {
+            if (!shash_find(&tc.chassis, id)) {
+                struct chassis_node *chassis = xzalloc(sizeof *chassis);
+                chassis->bridge = br_int;
+                chassis->port = port;
+                shash_add_assert(&tc.chassis, id, chassis);
+            } else {
+                /* Duplicate port for ovn-chassis-id.  Arbitrarily choose
+                 * to delete this one. */
+                ovsrec_bridge_update_ports_delvalue(br_int, port);
             }
         }
     }
diff --git a/controller/encaps.h b/controller/encaps.h
index 16032f15b..7197050b6 100644
--- a/controller/encaps.h
+++ b/controller/encaps.h
@@ -32,7 +32,6 @@  struct sset;
 
 void encaps_register_ovs_idl(struct ovsdb_idl *);
 void encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
-                const struct ovsrec_bridge_table *,
                 const struct ovsrec_bridge *br_int,
                 const struct sbrec_chassis_table *,
                 const struct sbrec_chassis *,
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index e26d46535..6a3aec317 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -4230,8 +4230,7 @@  main(int argc, char *argv[])
                 }
 
                 if (chassis) {
-                    encaps_run(ovs_idl_txn,
-                               bridge_table, br_int,
+                    encaps_run(ovs_idl_txn, br_int,
                                sbrec_chassis_table_get(ovnsb_idl_loop.idl),
                                chassis,
                                sbrec_sb_global_first(ovnsb_idl_loop.idl),
diff --git a/tests/ovn.at b/tests/ovn.at
index 2508a224e..3c76ba49c 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -33100,3 +33100,73 @@  AT_CHECK(test x$encap_hv1_ip == x)
 OVN_CLEANUP([hv1],[hv2])
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([controllers don't touch tunnels that are not on br-int])
+ovn_start
+net_add n1
+
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys1
+ovn_attach n1 br-phys1 192.168.0.1
+
+# the file is read once at startup so it's safe to write it
+# here after the first ovn-controller has started
+echo hv2 > ${OVN_SYSCONFDIR}/system-id-override
+ovs-vsctl add-br br-phys2
+
+# This function is similar to ovn_attach but makes sure it doesn't
+# mess with another controller settings
+start_virtual_controller() {
+    local net=$1 bridge=$2 ip=$3 masklen=${4-24} encap=${5-geneve,vxlan} systemid=${6-$sandbox} cli_args=${@:7}
+    net_attach $net $bridge || return 1
+
+    mac=`ovs-vsctl get Interface $bridge mac_in_use | sed s/\"//g`
+    arp_table="$arp_table $sandbox,$bridge,$ip,$mac"
+    ovs-appctl netdev-dummy/ip4addr $bridge $ip/$masklen >/dev/null || return 1
+    ovs-appctl ovs/route/add $ip/$masklen $bridge >/dev/null || return 1
+
+    local ovn_remote
+    if test X$HAVE_OPENSSL = Xyes; then
+        ovn_remote=$SSL_OVN_SB_DB
+    else
+        ovn_remote=unix:$ovs_base/ovn-sb/ovn-sb.sock
+    fi
+    ovs-vsctl \
+        -- set Open_vSwitch . external-ids:ovn-remote-$systemid=$ovn_remote \
+        -- set Open_vSwitch . external-ids:ovn-encap-type-$systemid=$encap \
+        -- set Open_vSwitch . external-ids:ovn-encap-ip-$systemid=$ip \
+        -- set Open_vSwitch . external-ids:ovn-bridge-$systemid=br-int-2 \
+        -- --may-exist add-br br-int-2 \
+        -- set bridge br-int-2 fail-mode=secure other-config:disable-in-band=true \
+        || return 1
+
+    ovn-controller --enable-dummy-vif-plug ${cli_args} -vconsole:off --detach --no-chdir
+}
+
+# for some reason SSL ovsdb configuration overrides CLI, so
+# delete ssl config from ovsdb to give CLI arguments priority
+ovs-vsctl del-ssl
+
+start_virtual_controller n1 br-phys2 192.168.0.2 24 geneve,vxlan hv2 \
+    --pidfile=${OVS_RUNDIR}/ovn-controller-2.pid \
+    --log-file=${OVS_RUNDIR}/ovn-controller-2.log \
+    -p $PKIDIR/testpki-hv2-privkey.pem \
+    -c $PKIDIR/testpki-hv2-cert.pem \
+    -C $PKIDIR/testpki-cacert.pem
+pidfile="$OVS_RUNDIR"/ovn-controller-2.pid
+on_exit "test -e \"$pidfile\" && kill \`cat \"$pidfile\"\`"
+
+# check that both tunnel ports are present, meaning controllers
+# don't step on each other
+OVS_WAIT_UNTIL([ovs-vsctl --columns _uuid --bare find Port \
+    name=ovn-hv1-hv2-0 | wc -l], [0],[[1
+]])
+OVS_WAIT_UNTIL([ovs-vsctl --columns _uuid --bare find Port \
+    name=ovn-hv2-hv1-0 | wc -l], [0],[[1
+]])
+
+OVN_CLEANUP([hv1])
+AT_CLEANUP
+])