diff mbox series

[ovs-dev,v7,1/7] Include "chassis index" into tunnel port name

Message ID 20221130003306.898159-2-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-_ovn-kubernetes success github build: passed
ovsrobot/github-robot-_Build_and_Test fail github build: failed

Commit Message

Ihar Hrachyshka Nov. 30, 2022, 12:33 a.m. UTC
This is in preparation to support multiple separate controller instances
with distinct chassis names operating on the same vswitchd instance.

To avoid conflicts, this patch introduces a unique "index" (from 0-9a-z
range) into the port name. Each chassis allocates a separate index for
itself on startup. The index is then stored in
Open_vSwitch:other_config:ovn-chassis-idx-<chassis_name> key.

An alternative would be including source chassis name into the port
name, but the length is limited by IFNAMSIZ defined in kernel, which is
15.

Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com>
---
 controller/chassis.c        | 107 +++++++++++++++++++++++++++++++++++-
 controller/chassis.h        |   6 ++
 controller/encaps.c         |  15 +++--
 controller/ovn-controller.c |  24 +++-----
 tests/automake.mk           |   1 +
 tests/ovn.at                |  32 +++++++++++
 6 files changed, 160 insertions(+), 25 deletions(-)
diff mbox series

Patch

diff --git a/controller/chassis.c b/controller/chassis.c
index 685d9b2ae..2be14f3e1 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -794,21 +794,124 @@  chassis_get_mac(const struct sbrec_chassis *chassis_rec,
     return ret;
 }
 
+const char *
+get_ovs_chassis_id(const struct ovsrec_open_vswitch_table *ovs_table)
+{
+    const struct ovsrec_open_vswitch *cfg
+        = ovsrec_open_vswitch_table_first(ovs_table);
+    const char *chassis_id = cfg ? smap_get(&cfg->external_ids, "system-id")
+                                 : NULL;
+
+    if (!chassis_id) {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+        VLOG_WARN_RL(&rl, "'system-id' in Open_vSwitch database is missing.");
+    }
+
+    return chassis_id;
+}
+
+const char *get_chassis_idx(const struct ovsrec_open_vswitch_table *ovs_table)
+{
+    const struct ovsrec_open_vswitch *cfg =
+        ovsrec_open_vswitch_table_first(ovs_table);
+    const char *chassis_id = get_ovs_chassis_id(ovs_table);
+    if (!chassis_id) {
+        return "";
+    }
+    char *idx_key = xasprintf(CHASSIS_IDX_PREFIX "%s", chassis_id);
+    const char *idx = smap_get_def(&cfg->other_config, idx_key, "");
+    free(idx_key);
+    return idx;
+}
+
+void
+store_chassis_index_if_needed(
+        const struct ovsrec_open_vswitch_table *ovs_table)
+{
+    const struct ovsrec_open_vswitch *cfg =
+        ovsrec_open_vswitch_table_first(ovs_table);
+    const char *chassis_id = get_ovs_chassis_id(ovs_table);
+
+    char *idx_key = xasprintf(CHASSIS_IDX_PREFIX "%s", chassis_id);
+    const char *chassis_idx = smap_get(&cfg->other_config, idx_key);
+    if (!chassis_idx) {
+        /* Collect all indices so far consumed by other chassis. */
+        struct sset used_indices = SSET_INITIALIZER(&used_indices);
+        struct smap_node *node;
+        SMAP_FOR_EACH (node, &cfg->other_config) {
+            if (!strncmp(node->key, CHASSIS_IDX_PREFIX,
+                    sizeof(CHASSIS_IDX_PREFIX) - 1)) {
+                sset_add(&used_indices, node->value);
+            }
+        }
+        /* First chassis on the host: use an empty string to avoid adding an
+         * unnecessary index character to tunnel port names when a single
+         * controller is running on the host (the most common case). */
+        if (!sset_contains(&used_indices, "")) {
+            ovsrec_open_vswitch_update_other_config_setkey(
+                cfg, idx_key, "");
+            goto out;
+        }
+        /* Next chassis gets an alphanum index allocated. */
+        char idx[] = "0";
+        for (char i = '0'; i <= '9'; i++) {
+            idx[0] = i;
+            if (!sset_contains(&used_indices, idx)) {
+                ovsrec_open_vswitch_update_other_config_setkey(
+                    cfg, idx_key, idx);
+                goto out;
+            }
+        }
+        for (char i = 'a'; i <= 'z'; i++) {
+            idx[0] = i;
+            if (!sset_contains(&used_indices, idx)) {
+                ovsrec_open_vswitch_update_other_config_setkey(
+                    cfg, idx_key, idx);
+                goto out;
+            }
+        }
+        /* All indices consumed: it's safer to just abort. */
+        VLOG_ERR("All unique controller indices consumed. Exiting.");
+        exit(EXIT_FAILURE);
+    }
+out:
+    free(idx_key);
+}
+
+static void
+clear_chassis_index_if_needed(
+        const struct ovsrec_open_vswitch_table *ovs_table)
+{
+    const struct ovsrec_open_vswitch *cfg =
+        ovsrec_open_vswitch_table_first(ovs_table);
+    const char *chassis_id = get_ovs_chassis_id(ovs_table);
+    char *idx_key = xasprintf(CHASSIS_IDX_PREFIX "%s", chassis_id);
+    if (smap_get(&cfg->other_config, idx_key)) {
+        ovsrec_open_vswitch_update_other_config_delkey(cfg, idx_key);
+    }
+    free(idx_key);
+}
+
 /* Returns true if the database is all cleaned up, false if more work is
  * required. */
 bool
 chassis_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,
+                const struct ovsrec_open_vswitch_table *ovs_table,
                 const struct sbrec_chassis *chassis_rec,
                 const struct sbrec_chassis_private *chassis_private_rec)
 {
     if (!chassis_rec && !chassis_private_rec) {
         return true;
     }
+
+    const char *chassis_name = (
+        chassis_rec ? chassis_rec->name : chassis_private_rec->name);
+    clear_chassis_index_if_needed(ovs_table);
+
     if (ovnsb_idl_txn) {
         ovsdb_idl_txn_add_comment(ovnsb_idl_txn,
                                   "ovn-controller: unregistering chassis '%s'",
-                                  chassis_rec ? chassis_rec->name
-                                  : chassis_private_rec->name);
+                                  chassis_name);
         if (chassis_rec) {
             sbrec_chassis_delete(chassis_rec);
         }
diff --git a/controller/chassis.h b/controller/chassis.h
index 18b45a1c5..831b5bc94 100644
--- a/controller/chassis.h
+++ b/controller/chassis.h
@@ -19,6 +19,8 @@ 
 #include <stdbool.h>
 #include "lib/ovn-sb-idl.h"
 
+#define CHASSIS_IDX_PREFIX "ovn-chassis-idx-"
+
 struct ovsdb_idl;
 struct ovsdb_idl_index;
 struct ovsdb_idl_txn;
@@ -41,12 +43,16 @@  const struct sbrec_chassis *chassis_run(
     const struct sset *transport_zones,
     const struct sbrec_chassis_private **chassis_private);
 bool chassis_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,
+                     const struct ovsrec_open_vswitch_table *,
                      const struct sbrec_chassis *,
                      const struct sbrec_chassis_private *);
 bool chassis_get_mac(const struct sbrec_chassis *chassis,
                      const char *bridge_mapping,
                      struct eth_addr *chassis_mac);
 const char * get_chassis_mac_mappings(const struct smap *ext_ids);
+const char *get_ovs_chassis_id(const struct ovsrec_open_vswitch_table *);
+const char *get_chassis_idx(const struct ovsrec_open_vswitch_table *);
+void store_chassis_index_if_needed(const struct ovsrec_open_vswitch_table *);
 
 
 #endif /* controller/chassis.h */
diff --git a/controller/encaps.c b/controller/encaps.c
index 9647ba507..5d171882d 100644
--- a/controller/encaps.c
+++ b/controller/encaps.c
@@ -15,6 +15,7 @@ 
 
 #include <config.h>
 #include "encaps.h"
+#include "chassis.h"
 
 #include "lib/hash.h"
 #include "lib/sset.h"
@@ -22,6 +23,7 @@ 
 #include "lib/vswitch-idl.h"
 #include "openvswitch/vlog.h"
 #include "lib/ovn-sb-idl.h"
+#include "lib/ovsdb-idl.h"
 #include "ovn-controller.h"
 #include "smap.h"
 
@@ -59,6 +61,7 @@  struct tunnel_ctx {
     struct sset port_names;
 
     struct ovsdb_idl_txn *ovs_txn;
+    const struct ovsrec_open_vswitch_table *ovs_table;
     const struct ovsrec_bridge *br_int;
     const struct sbrec_chassis *this_chassis;
 };
@@ -71,11 +74,10 @@  struct chassis_node {
 static char *
 tunnel_create_name(struct tunnel_ctx *tc, const char *chassis_id)
 {
-    int i;
-
-    for (i = 0; i < UINT16_MAX; i++) {
-        char *port_name;
-        port_name = xasprintf("ovn-%.6s-%x", chassis_id, i);
+    for (int i = 0; i < UINT16_MAX; i++) {
+        const char *idx = get_chassis_idx(tc->ovs_table);
+        char *port_name = xasprintf(
+            "ovn%s-%.*s-%x", idx, strlen(idx) ? 5 : 6, chassis_id, i);
 
         if (!sset_contains(&tc->port_names, port_name)) {
             return port_name;
@@ -400,7 +402,8 @@  encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
         .chassis = SHASH_INITIALIZER(&tc.chassis),
         .port_names = SSET_INITIALIZER(&tc.port_names),
         .br_int = br_int,
-        .this_chassis = this_chassis
+        .this_chassis = this_chassis,
+        .ovs_table = ovs_table,
     };
 
     tc.ovs_txn = ovs_idl_txn;
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 0752a71ad..2c603bad6 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -478,22 +478,6 @@  process_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
     *br_int_ = br_int;
 }
 
-static const char *
-get_ovs_chassis_id(const struct ovsrec_open_vswitch_table *ovs_table)
-{
-    const struct ovsrec_open_vswitch *cfg
-        = ovsrec_open_vswitch_table_first(ovs_table);
-    const char *chassis_id = cfg ? smap_get(&cfg->external_ids, "system-id")
-                                 : NULL;
-
-    if (!chassis_id) {
-        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
-        VLOG_WARN_RL(&rl, "'system-id' in Open_vSwitch database is missing.");
-    }
-
-    return chassis_id;
-}
-
 static void
 update_ssl_config(const struct ovsrec_ssl_table *ssl_table)
 {
@@ -4138,6 +4122,12 @@  main(int argc, char *argv[])
             }
         }
 
+        static bool chassis_idx_stored = false;
+        if (ovs_idl_txn && !chassis_idx_stored) {
+            store_chassis_index_if_needed(ovs_table);
+            chassis_idx_stored = true;
+        }
+
         if (ovsdb_idl_has_ever_connected(ovnsb_idl_loop.idl) &&
             northd_version_match) {
 
@@ -4534,7 +4524,7 @@  loop_done:
             /* Run all of the cleanup functions, even if one of them returns
              * false. We're done if all of them return true. */
             done = binding_cleanup(ovnsb_idl_txn, port_binding_table, chassis);
-            done = chassis_cleanup(ovnsb_idl_txn,
+            done = chassis_cleanup(ovnsb_idl_txn, ovs_table,
                                    chassis, chassis_private) && done;
             done = encaps_cleanup(ovs_idl_txn, br_int) && done;
             done = igmp_group_cleanup(ovnsb_idl_txn, sbrec_igmp_group) && done;
diff --git a/tests/automake.mk b/tests/automake.mk
index dce9c9108..d9f2777f3 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -253,6 +253,7 @@  tests_ovstest_SOURCES = \
 tests_ovstest_LDADD = $(OVS_LIBDIR)/daemon.lo \
     $(OVS_LIBDIR)/libopenvswitch.la lib/libovn.la \
 	controller/binding.$(OBJEXT) \
+	controller/chassis.$(OBJEXT) \
 	controller/encaps.$(OBJEXT) \
 	controller/ha-chassis.$(OBJEXT) \
 	controller/if-status.$(OBJEXT) \
diff --git a/tests/ovn.at b/tests/ovn.at
index b540920b4..233d4e066 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -33423,3 +33423,35 @@  AT_CHECK([test -z "`grep disconnecting hv1/ovs-vswitchd.log`"])
 OVN_CLEANUP([hv1])
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn-chassis-idx maintenance in ovsdb])
+ovn_start
+net_add n1
+
+sim_add hv1
+as hv1
+check ovs-vsctl add-br br-phys
+
+# check that chassis index is unset before ovn-controller is started
+OVS_WAIT_UNTIL([test "x`ovs-vsctl get Open_vSwitch . other_config | grep ovn-chassis-idx-hv1`" = x])
+
+ovn_attach n1 br-phys 192.168.0.1
+
+# check that chassis index is set now that ovn-controller is running
+OVS_WAIT_UNTIL([test x`ovs-vsctl get Open_vSwitch . other_config:ovn-chassis-idx-hv1 | tr -d '""'` = x])
+
+# exit ovn-controller which should clean up allocated index in the database
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+
+# check that chassis index is unset since ovn-controller cleaned it up
+OVS_WAIT_UNTIL([test "x`ovs-vsctl get Open_vSwitch . other_config | grep ovn-chassis-idx-hv1`" = x])
+
+# clean up the rest of services on hv1
+OVN_CLEANUP_VSWITCH([hv1])
+
+# tear down main services
+OVN_CLEANUP
+
+AT_CLEANUP
+])