[ovs-dev] [PATCH v4 ovn] Allow to run multiple controllers on the same machine

Ihar Hrachyshka ihrachys at redhat.com
Tue Sep 15 03:42:42 UTC 2020


User stories:
1) NFV: an admin wants to run two separate instances of OVN controller
   using the same database but configuring ports on different bridges.
   Some of these bridges may use DPDK while others may not.

2) Parallel OVN instances: an admin wants to run two separate
   instances of OVN controller using different databases. The
   instances are completely independent and serve different consumers.
   For example, the same machine runs both OpenStack and OpenShift
   stacks, each running its own separate OVN stack.

To serve these use cases, several features should be added to
ovn-controller:

- use different database configuration for multiple controllers;
- customize chassis name used by controller.

=====

For each of the following database configuration options, their
extended chassis specific counterparts are introduced:

external_ids:hostname
external_ids:ovn-bridge
external_ids:ovn-bridge-datapath-type
external_ids:ovn-bridge-mappings
external_ids:ovn-chassis-mac-mappings
external_ids:ovn-cms-options
external_ids:ovn-encap-csum
external_ids:ovn-encap-ip
external_ids:ovn-encap-type
external_ids:ovn-is-interconn
external_ids:ovn-monitor-all
external_ids:ovn-openflow-probe-interval
external_ids:ovn-remote
external_ids:ovn-remote-probe-interval

For example,

external_ids:ovn-bridge -> external_ids:ovn-bridge-<chassis-name>=
external_ids:ovn-encap-type -> external_ids:ovn-encap-type-<chassis-name>=
external_ids:ovn-encap-ip -> external_ids:ovn-encap-ip-<chassis-name>=
external_ids:ovn-remote -> external_ids:ovn-remote-<chassis-name>=

Priority wise, <chassis-name> specific options take precedence.

=====

For system-id,

You can now pass intended chassis name via CLI argument:

  $ ovn-controller ... -n <chassis_name>

Alternatively, you can configure a chassis name by putting it into the
${ovs_sysconfdir}/system-id file before running the controller.

The latter option may be more useful in container environment where
the same image may be reused for multiple controller instances, where
ovs_sysconfigdir/system-id is a volume mounted into this generic
image.

Priority wise, this is the order in which different means to configure
the chassis name are used:

- ovn-controller ... -n <chassis_name> CLI argument.
- ${ovs_sysconfdir}/system-id file;
- external_ids:system-id= ovsdb option;

=====

Concurrent chassis running on the same host may inadvertantly remove patch
ports that belong to their peer chassis. To avoid that, a new -M
(--concurrent) CLI argument is introduced. Right now it avoids removing
"unneeded" patch ports but it may later be used for related accommodations.

=====

Note: this patch assumes that each chassis has its own unique IP. Future work
may consider adding support to specify custom port numbers for tunneling that
would allow to reuse the same IP address for multiple chassis running on the
same host. This work is out of scope for this patch.

Signed-off-by: Ihar Hrachyshka <ihrachys at redhat.com>

---

v1: initial implementation.
v2: fixed test case to check ports are claimed by proper chassis.
v2: added NEWS entry.
v2: fixed some compiler warnings.
v2: moved file_system_id declaration inside a function that uses it.
v2: removed unneeded binding.h #include.
v2: docs: better explanation of alternatives to select chassis name.
v3: reverted priority order for chassis configuration: first CLI, then
    system-id file, then ovsdb.
v4: introduce helpers to extract external-ids (per-chassis or global).
v4: introduce per-chassis config options for all keys.
v4: introduce -M (--concurrent) CLI argument to avoid patch ports removed by
    concurrent chassis.
---
 NEWS                            |   5 ++
 controller/binding.h            |   1 +
 controller/chassis.c            |  69 ++++++++++++---------
 controller/chassis.h            |   3 +-
 controller/ovn-controller.8.xml |  13 ++++
 controller/ovn-controller.c     | 102 +++++++++++++++++++++++++-------
 controller/ovn-controller.h     |   3 +
 controller/patch.c              |  23 ++++---
 controller/patch.h              |   3 +-
 controller/physical.c           |   2 +-
 lib/ovn-util.c                  |  50 ++++++++++++++++
 lib/ovn-util.h                  |  18 ++++++
 ovn-sb.xml                      |  10 ++--
 tests/ovn-controller.at         |   9 ++-
 tests/ovn-macros.at             |  49 ++++++++++++---
 tests/ovn.at                    |  55 ++++++++++++++++-
 16 files changed, 336 insertions(+), 79 deletions(-)

diff --git a/NEWS b/NEWS
index a1ce4e8ec..046773330 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,11 @@ Post-v20.06.0
      called Chassis_Private now contains the nb_cfg column which is updated
      by incrementing the value in the NB_Global table, CMSes relying on
      this mechanism should update their code to use this new table.
+   - Added support for multiple ovn-controller instances on the same host
+     (virtual chassis). Now all external-ids:* configuration options can be
+     customized for each controller instance running on the same host. The only
+     option that is not available per chassis is external-ids:system-id, which
+     stands for the chassis name and can be passed via config file or CLI (-n).
 
 OVN v20.06.0
 --------------------------
diff --git a/controller/binding.h b/controller/binding.h
index c9740560f..12557b3b9 100644
--- a/controller/binding.h
+++ b/controller/binding.h
@@ -32,6 +32,7 @@ struct ovsrec_port_table;
 struct ovsrec_qos_table;
 struct ovsrec_bridge_table;
 struct ovsrec_open_vswitch_table;
+struct ovsrec_open_vswitch;
 struct sbrec_chassis;
 struct sbrec_port_binding_table;
 struct sset;
diff --git a/controller/chassis.c b/controller/chassis.c
index d392d4f20..40fc3696d 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -124,9 +124,10 @@ chassis_register_ovs_idl(struct ovsdb_idl *ovs_idl)
 }
 
 static const char *
-get_hostname(const struct smap *ext_ids)
+get_hostname(const struct smap *ext_ids, const char *chassis_id)
 {
-    const char *hostname = smap_get_def(ext_ids, "hostname", "");
+    const char *hostname = get_chassis_external_id_value(
+        ext_ids, chassis_id, "hostname", "");
 
     if (strlen(hostname) == 0) {
         static char hostname_[HOST_NAME_MAX + 1];
@@ -142,33 +143,38 @@ get_hostname(const struct smap *ext_ids)
 }
 
 static const char *
-get_bridge_mappings(const struct smap *ext_ids)
+get_bridge_mappings(const struct smap *ext_ids, const char *chassis_id)
 {
-    return smap_get_def(ext_ids, "ovn-bridge-mappings", "");
+    return get_chassis_external_id_value(
+        ext_ids, chassis_id, "ovn-bridge-mappings", "");
 }
 
 const char *
-get_chassis_mac_mappings(const struct smap *ext_ids)
+get_chassis_mac_mappings(const struct smap *ext_ids, const char *chassis_id)
 {
-    return smap_get_def(ext_ids, "ovn-chassis-mac-mappings", "");
+    return get_chassis_external_id_value(
+        ext_ids, chassis_id, "ovn-chassis-mac-mappings", "");
 }
 
 static const char *
-get_cms_options(const struct smap *ext_ids)
+get_cms_options(const struct smap *ext_ids, const char *chassis_id)
 {
-    return smap_get_def(ext_ids, "ovn-cms-options", "");
+    return get_chassis_external_id_value(
+        ext_ids, chassis_id, "ovn-cms-options", "");
 }
 
 static const char *
-get_monitor_all(const struct smap *ext_ids)
+get_monitor_all(const struct smap *ext_ids, const char *chassis_id)
 {
-    return smap_get_def(ext_ids, "ovn-monitor-all", "false");
+    return get_chassis_external_id_value(
+        ext_ids, chassis_id, "ovn-monitor-all", "false");
 }
 
 static const char *
-get_encap_csum(const struct smap *ext_ids)
+get_encap_csum(const struct smap *ext_ids, const char *chassis_id)
 {
-    return smap_get_def(ext_ids, "ovn-encap-csum", "true");
+    return get_chassis_external_id_value(
+        ext_ids, chassis_id, "ovn-encap-csum", "true");
 }
 
 static const char *
@@ -182,9 +188,10 @@ get_datapath_type(const struct ovsrec_bridge *br_int)
 }
 
 static bool
-get_is_interconn(const struct smap *ext_ids)
+get_is_interconn(const struct smap *ext_ids, const char *chassis_id)
 {
-    return smap_get_bool(ext_ids, "ovn-is-interconn", false);
+    return get_chassis_external_id_value_bool(
+        ext_ids, chassis_id, "ovn-is-interconn", false);
 }
 
 static void
@@ -271,21 +278,27 @@ chassis_parse_ovs_config(const struct ovsrec_open_vswitch_table *ovs_table,
         return false;
     }
 
-    const char *encap_type = smap_get(&cfg->external_ids, "ovn-encap-type");
-    const char *encap_ips = smap_get(&cfg->external_ids, "ovn-encap-ip");
+    const char *chassis_id = get_ovs_chassis_id(cfg);
+
+    const char *encap_type = get_chassis_external_id_value(
+        &cfg->external_ids, chassis_id, "ovn-encap-type", NULL);
+    const char *encap_ips = get_chassis_external_id_value(
+        &cfg->external_ids, chassis_id, "ovn-encap-ip", NULL);
     if (!encap_type || !encap_ips) {
         static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
         VLOG_INFO_RL(&rl, "Need to specify an encap type and ip");
         return false;
     }
 
-    ovs_cfg->hostname = get_hostname(&cfg->external_ids);
-    ovs_cfg->bridge_mappings = get_bridge_mappings(&cfg->external_ids);
+    ovs_cfg->hostname = get_hostname(&cfg->external_ids, chassis_id);
+    ovs_cfg->bridge_mappings = get_bridge_mappings(&cfg->external_ids,
+                                                   chassis_id);
     ovs_cfg->datapath_type = get_datapath_type(br_int);
-    ovs_cfg->encap_csum = get_encap_csum(&cfg->external_ids);
-    ovs_cfg->cms_options = get_cms_options(&cfg->external_ids);
-    ovs_cfg->monitor_all = get_monitor_all(&cfg->external_ids);
-    ovs_cfg->chassis_macs = get_chassis_mac_mappings(&cfg->external_ids);
+    ovs_cfg->encap_csum = get_encap_csum(&cfg->external_ids, chassis_id);
+    ovs_cfg->cms_options = get_cms_options(&cfg->external_ids, chassis_id);
+    ovs_cfg->monitor_all = get_monitor_all(&cfg->external_ids, chassis_id);
+    ovs_cfg->chassis_macs = get_chassis_mac_mappings(&cfg->external_ids,
+                                                     chassis_id);
 
     if (!chassis_parse_ovs_encap_type(encap_type, &ovs_cfg->encap_type_set)) {
         return false;
@@ -303,7 +316,7 @@ chassis_parse_ovs_config(const struct ovsrec_open_vswitch_table *ovs_table,
         sset_destroy(&ovs_cfg->encap_ip_set);
     }
 
-    ovs_cfg->is_interconn = get_is_interconn(&cfg->external_ids);
+    ovs_cfg->is_interconn = get_is_interconn(&cfg->external_ids, chassis_id);
 
     return true;
 }
@@ -337,7 +350,7 @@ chassis_other_config_changed(const char *bridge_mappings,
                              const struct sbrec_chassis *chassis_rec)
 {
     const char *chassis_bridge_mappings =
-        get_bridge_mappings(&chassis_rec->other_config);
+        get_bridge_mappings(&chassis_rec->other_config, NULL);
 
     if (strcmp(bridge_mappings, chassis_bridge_mappings)) {
         return true;
@@ -351,21 +364,21 @@ chassis_other_config_changed(const char *bridge_mappings,
     }
 
     const char *chassis_cms_options =
-        get_cms_options(&chassis_rec->other_config);
+        get_cms_options(&chassis_rec->other_config, NULL);
 
     if (strcmp(cms_options, chassis_cms_options)) {
         return true;
     }
 
     const char *chassis_monitor_all =
-        get_monitor_all(&chassis_rec->other_config);
+        get_monitor_all(&chassis_rec->other_config, NULL);
 
     if (strcmp(monitor_all, chassis_monitor_all)) {
         return true;
     }
 
     const char *chassis_mac_mappings =
-        get_chassis_mac_mappings(&chassis_rec->other_config);
+        get_chassis_mac_mappings(&chassis_rec->other_config, NULL);
     if (strcmp(chassis_macs, chassis_mac_mappings)) {
         return true;
     }
@@ -685,7 +698,7 @@ chassis_get_mac(const struct sbrec_chassis *chassis_rec,
                 struct eth_addr *chassis_mac)
 {
     const char *tokens
-        = get_chassis_mac_mappings(&chassis_rec->other_config);
+        = get_chassis_mac_mappings(&chassis_rec->other_config, NULL);
     if (!tokens[0]) {
        return false;
     }
diff --git a/controller/chassis.h b/controller/chassis.h
index 81055b403..62e82bead 100644
--- a/controller/chassis.h
+++ b/controller/chassis.h
@@ -47,7 +47,8 @@ bool chassis_get_mac(const struct sbrec_chassis *chassis,
                      const char *bridge_mapping,
                      struct eth_addr *chassis_mac);
 const char *chassis_get_id(void);
-const char * get_chassis_mac_mappings(const struct smap *ext_ids);
+const char * get_chassis_mac_mappings(const struct smap *ext_ids,
+                                      const char *chassis_id);
 
 
 #endif /* controller/chassis.h */
diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml
index 66877314c..9bd5e6592 100644
--- a/controller/ovn-controller.8.xml
+++ b/controller/ovn-controller.8.xml
@@ -235,6 +235,19 @@
       </dd>
     </dl>
 
+    <p>
+      Note that every <code>external_ids:*</code> key listed above has its
+      <code>external_ids:*-chassis_name</code> counterpart keys that allow to
+      configure values specific to chassis running on the same OVSDB. For
+      example, if two chassis named <code>blue</code> and <code>red</code> are
+      available on the same host, then an admin may configure different
+      <code>ovn-cms-options</code> for each of them by setting
+      <code>external_ids:ovn-cms-options-blue</code> and
+      <code>external_ids:ovn-cms-options-red</code> keys in the database. The
+      only key that is not available for per-chassis configuration is
+      <code>external_ids:system-id</code>.
+    </p>
+
     <p>
       <code>ovn-controller</code> reads the following values from the
       <code>Open_vSwitch</code> database of the local OVS instance:
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index d299c6153..0027f717c 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -18,10 +18,14 @@
 #include "ovn-controller.h"
 
 #include <errno.h>
+#include <fcntl.h>
 #include <getopt.h>
 #include <signal.h>
 #include <stdlib.h>
 #include <string.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
 
 #include "bfd.h"
 #include "binding.h"
@@ -80,6 +84,9 @@ static unixctl_cb_func cluster_state_reset_cmd;
 
 #define CONTROLLER_LOOP_STOPWATCH_NAME "ovn-controller-flow-generation"
 
+static const char *controller_chassis = NULL;
+static int is_multiple_chassis = 0;
+
 static char *parse_options(int argc, char *argv[]);
 OVS_NO_RETURN static void usage(void);
 
@@ -255,7 +262,9 @@ out:
 static const char *
 br_int_name(const struct ovsrec_open_vswitch *cfg)
 {
-    return smap_get_def(&cfg->external_ids, "ovn-bridge", DEFAULT_BRIDGE_NAME);
+    return get_chassis_external_id_value(
+        &cfg->external_ids, get_ovs_chassis_id(cfg),
+        "ovn-bridge", DEFAULT_BRIDGE_NAME);
 }
 
 static const struct ovsrec_bridge *
@@ -356,8 +365,9 @@ process_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
         const struct ovsrec_open_vswitch *cfg;
         cfg = ovsrec_open_vswitch_table_first(ovs_table);
         ovs_assert(cfg);
-        const char *datapath_type = smap_get(&cfg->external_ids,
-                                             "ovn-bridge-datapath-type");
+        const char *datapath_type = get_chassis_external_id_value(
+            &cfg->external_ids, get_ovs_chassis_id(cfg),
+            "ovn-bridge-datapath-type", NULL);
         /* Check for the datapath_type and set it only if it is defined in
          * cfg. */
         if (datapath_type && strcmp(br_int->datapath_type, datapath_type)) {
@@ -367,17 +377,47 @@ process_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
     return br_int;
 }
 
-static const char *
-get_ovs_chassis_id(const struct ovsrec_open_vswitch_table *ovs_table)
+static const char *get_file_system_id(void)
 {
-    const struct ovsrec_open_vswitch *cfg
-        = ovsrec_open_vswitch_table_first(ovs_table);
+    const char *ret = NULL;
+    char *filename = xasprintf("%s/system-id", ovs_sysconfdir());
+    errno = 0;
+    int fd = open(filename, O_RDONLY);
+    if (fd != -1) {
+        static char file_system_id[64];
+        int nread = read(fd, file_system_id, sizeof file_system_id);
+        if (nread) {
+            file_system_id[nread] = '\0';
+            if (file_system_id[nread - 1] == '\n') {
+                file_system_id[nread - 1] = '\0';
+            }
+            ret = file_system_id;
+        }
+        close(fd);
+    }
+
+    free(filename);
+    return ret;
+}
+
+const char *
+get_ovs_chassis_id(const struct ovsrec_open_vswitch *cfg)
+{
+    if (controller_chassis) {
+        return controller_chassis;
+    }
+
+    const char *id_from_file = get_file_system_id();
+    if (id_from_file) {
+        return id_from_file;
+    }
+
     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.");
+        VLOG_WARN_RL(&rl, "Failed to detect system-id, "
+                          "configuration not found.");
     }
 
     return chassis_id;
@@ -472,10 +512,12 @@ static int
 get_ofctrl_probe_interval(struct ovsdb_idl *ovs_idl)
 {
     const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(ovs_idl);
-    return !cfg ? OFCTRL_DEFAULT_PROBE_INTERVAL_SEC :
-                  smap_get_int(&cfg->external_ids,
-                               "ovn-openflow-probe-interval",
-                               OFCTRL_DEFAULT_PROBE_INTERVAL_SEC);
+    if (!cfg) {
+        return OFCTRL_DEFAULT_PROBE_INTERVAL_SEC;
+    }
+    return get_chassis_external_id_value_int(
+        &cfg->external_ids, get_ovs_chassis_id(cfg),
+        "ovn-openflow-probe-interval", OFCTRL_DEFAULT_PROBE_INTERVAL_SEC);
 }
 
 /* Retrieves the pointer to the OVN Southbound database from 'ovs_idl' and
@@ -490,18 +532,21 @@ update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl,
     }
 
     /* Set remote based on user configuration. */
-    const char *remote = smap_get(&cfg->external_ids, "ovn-remote");
+    const char *chassis_id = get_ovs_chassis_id(cfg);
+    const char *remote = get_chassis_external_id_value(
+        &cfg->external_ids, chassis_id, "ovn-remote", NULL);
     ovsdb_idl_set_remote(ovnsb_idl, remote, true);
 
     /* Set probe interval, based on user configuration and the remote. */
     int default_interval = (remote && !stream_or_pstream_needs_probes(remote)
                             ? 0 : DEFAULT_PROBE_INTERVAL_MSEC);
-    int interval = smap_get_int(&cfg->external_ids,
-                                "ovn-remote-probe-interval", default_interval);
+    int interval = get_chassis_external_id_value_int(
+        &cfg->external_ids, chassis_id, "ovn-remote-probe-interval",
+        default_interval);
     ovsdb_idl_set_probe_interval(ovnsb_idl, interval);
 
-    bool monitor_all = smap_get_bool(&cfg->external_ids, "ovn-monitor-all",
-                                     false);
+    bool monitor_all = get_chassis_external_id_value_bool(
+        &cfg->external_ids, chassis_id, "ovn-monitor-all", false);
     if (monitor_all) {
         /* Always call update_sb_monitors when monitor_all is true.
          * Otherwise, don't call it here, because there would be unnecessary
@@ -1151,7 +1196,9 @@ init_binding_ctx(struct engine_node *node,
     struct ovsrec_bridge_table *bridge_table =
         (struct ovsrec_bridge_table *)EN_OVSDB_GET(
             engine_get_input("OVS_bridge", node));
-    const char *chassis_id = get_ovs_chassis_id(ovs_table);
+    const struct ovsrec_open_vswitch *cfg =
+        ovsrec_open_vswitch_table_first(ovs_table);
+    const char *chassis_id = get_ovs_chassis_id(cfg);
     const struct ovsrec_bridge *br_int = get_br_int(bridge_table, ovs_table);
 
     ovs_assert(br_int && chassis_id);
@@ -2402,7 +2449,9 @@ main(int argc, char *argv[])
                 sbrec_chassis_table_get(ovnsb_idl_loop.idl);
             const struct ovsrec_bridge *br_int =
                 process_br_int(ovs_idl_txn, bridge_table, ovs_table);
-            const char *chassis_id = get_ovs_chassis_id(ovs_table);
+            const struct ovsrec_open_vswitch *cfg =
+                ovsrec_open_vswitch_table_first(ovs_table);
+            const char *chassis_id = get_ovs_chassis_id(cfg);
             const struct sbrec_chassis *chassis = NULL;
             const struct sbrec_chassis_private *chassis_private = NULL;
             if (chassis_id) {
@@ -2487,7 +2536,8 @@ main(int argc, char *argv[])
                             ovsrec_open_vswitch_table_get(ovs_idl_loop.idl),
                             ovsrec_port_table_get(ovs_idl_loop.idl),
                             sbrec_port_binding_table_get(ovnsb_idl_loop.idl),
-                            br_int, chassis, &runtime_data->local_datapaths);
+                            br_int, chassis, &runtime_data->local_datapaths,
+                            is_multiple_chassis);
                         pinctrl_run(ovnsb_idl_txn,
                                     sbrec_datapath_binding_by_key,
                                     sbrec_port_binding_by_datapath,
@@ -2698,6 +2748,8 @@ parse_options(int argc, char *argv[])
         STREAM_SSL_LONG_OPTIONS,
         {"peer-ca-cert", required_argument, NULL, OPT_PEER_CA_CERT},
         {"bootstrap-ca-cert", required_argument, NULL, OPT_BOOTSTRAP_CA_CERT},
+        {"chassis", required_argument, NULL, 'n'},
+        {"concurrent", no_argument, NULL, 'M'},
         {NULL, 0, NULL, 0}
     };
     char *short_options = ovs_cmdl_long_options_to_short_options(long_options);
@@ -2730,6 +2782,14 @@ parse_options(int argc, char *argv[])
             stream_ssl_set_ca_cert_file(optarg, true);
             break;
 
+        case 'n':
+            controller_chassis = xstrdup(optarg);
+            break;
+
+        case 'M':
+            is_multiple_chassis = 1;
+            break;
+
         case '?':
             exit(EXIT_FAILURE);
 
diff --git a/controller/ovn-controller.h b/controller/ovn-controller.h
index 5d9466880..df962b8e9 100644
--- a/controller/ovn-controller.h
+++ b/controller/ovn-controller.h
@@ -21,6 +21,7 @@
 #include "lib/ovn-sb-idl.h"
 
 struct ovsrec_bridge_table;
+struct ovsrec_open_vswitch;
 
 /* Linux supports a maximum of 64K zones, which seems like a fine default. */
 #define MAX_CT_ZONES 65535
@@ -87,4 +88,6 @@ enum chassis_tunnel_type {
 
 uint32_t get_tunnel_type(const char *name);
 
+const char *get_ovs_chassis_id(const struct ovsrec_open_vswitch *cfg);
+
 #endif /* controller/ovn-controller.h */
diff --git a/controller/patch.c b/controller/patch.c
index a2a7bcd79..bf40640e3 100644
--- a/controller/patch.c
+++ b/controller/patch.c
@@ -157,7 +157,9 @@ add_ovs_bridge_mappings(const struct ovsrec_open_vswitch_table *ovs_table,
         const char *mappings_cfg;
         char *cur, *next, *start;
 
-        mappings_cfg = smap_get(&cfg->external_ids, "ovn-bridge-mappings");
+        mappings_cfg = get_chassis_external_id_value(
+            &cfg->external_ids, get_ovs_chassis_id(cfg),
+            "ovn-bridge-mappings", NULL);
         if (!mappings_cfg || !mappings_cfg[0]) {
             return;
         }
@@ -287,7 +289,8 @@ patch_run(struct ovsdb_idl_txn *ovs_idl_txn,
           const struct sbrec_port_binding_table *port_binding_table,
           const struct ovsrec_bridge *br_int,
           const struct sbrec_chassis *chassis,
-          const struct hmap *local_datapaths)
+          const struct hmap *local_datapaths,
+          int is_multiple_chassis)
 {
     if (!ovs_idl_txn) {
         return;
@@ -318,12 +321,14 @@ patch_run(struct ovsdb_idl_txn *ovs_idl_txn,
                         local_datapaths);
 
     /* Now 'existing_ports' only still contains patch ports that exist in the
-     * database but shouldn't.  Delete them from the database. */
-    struct shash_node *port_node, *port_next_node;
-    SHASH_FOR_EACH_SAFE (port_node, port_next_node, &existing_ports) {
-        port = port_node->data;
-        shash_delete(&existing_ports, port_node);
-        remove_port(bridge_table, port);
+     * database but shouldn't.  Delete them from the database if it's safe. */
+    if (!is_multiple_chassis) {
+        struct shash_node *port_node, *port_next_node;
+        SHASH_FOR_EACH_SAFE (port_node, port_next_node, &existing_ports) {
+            port = port_node->data;
+            shash_delete(&existing_ports, port_node);
+            remove_port(bridge_table, port);
+        }
+        shash_destroy(&existing_ports);
     }
-    shash_destroy(&existing_ports);
 }
diff --git a/controller/patch.h b/controller/patch.h
index e470d502c..a3f4cd71e 100644
--- a/controller/patch.h
+++ b/controller/patch.h
@@ -42,7 +42,8 @@ void patch_run(struct ovsdb_idl_txn *ovs_idl_txn,
                const struct sbrec_port_binding_table *,
                const struct ovsrec_bridge *br_int,
                const struct sbrec_chassis *,
-               const struct hmap *local_datapaths);
+               const struct hmap *local_datapaths,
+               int is_multiple_chassis);
 void patch_init(void);
 void patch_destroy(void);
 
diff --git a/controller/physical.c b/controller/physical.c
index 535c77730..a312e6c27 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -424,7 +424,7 @@ populate_remote_chassis_macs(const struct sbrec_chassis *my_chassis,
         }
 
         const char *tokens
-            = get_chassis_mac_mappings(&chassis->other_config);
+            = get_chassis_mac_mappings(&chassis->other_config, NULL);
 
         if (!strlen(tokens)) {
             continue;
diff --git a/lib/ovn-util.c b/lib/ovn-util.c
index cdb5e18fb..3193b73db 100644
--- a/lib/ovn-util.c
+++ b/lib/ovn-util.c
@@ -641,3 +641,53 @@ str_tolower(const char *orig)
 
     return copy;
 }
+
+const char *
+get_chassis_external_id_value(const struct smap *external_ids,
+                              const char *chassis_id, const char *option_key,
+                              const char *def)
+{
+    const char *option_value = NULL;
+    if (chassis_id != NULL) {
+        char *chassis_option_key = xasprintf("%s-%s", option_key, chassis_id);
+        option_value = smap_get(external_ids, chassis_option_key);
+        free(chassis_option_key);
+    }
+    if (!option_value) {
+        option_value = smap_get_def(external_ids, option_key, def);
+    }
+    return option_value;
+}
+
+int
+get_chassis_external_id_value_int(const struct smap *external_ids,
+                                  const char *chassis_id,
+                                  const char *option_key,
+                                  int def)
+{
+    const char *value = get_chassis_external_id_value(
+        external_ids, chassis_id, option_key, NULL);
+
+    int i_value;
+    if (!value || !str_to_int(value, 10, &i_value)) {
+        return def;
+    }
+
+    return i_value;
+}
+
+bool
+get_chassis_external_id_value_bool(const struct smap *external_ids,
+                                   const char *chassis_id,
+                                   const char *option_key,
+                                   bool def)
+{
+    const char *value = get_chassis_external_id_value(
+        external_ids, chassis_id, option_key, "");
+
+    if (def) {
+        return strcasecmp("false", value) != 0;
+    } else {
+        return !strcasecmp("true", value);
+    }
+}
diff --git a/lib/ovn-util.h b/lib/ovn-util.h
index 0f7b501f1..7bca71e86 100644
--- a/lib/ovn-util.h
+++ b/lib/ovn-util.h
@@ -18,6 +18,7 @@
 
 #include "lib/packets.h"
 #include "include/ovn/version.h"
+#include "smap.h"
 
 #define ovn_set_program_name(name) \
     ovs_set_program_name(name, OVN_PACKAGE_VERSION)
@@ -148,6 +149,23 @@ char *normalize_ipv4_prefix(ovs_be32 ipv4, unsigned int plen);
 char *normalize_ipv6_prefix(struct in6_addr ipv6, unsigned int plen);
 char *normalize_v46_prefix(const struct v46_ip *prefix, unsigned int plen);
 
+const char *
+get_chassis_external_id_value(const struct smap *external_ids,
+                              const char *chassis_id, const char *option_key,
+                              const char *def);
+
+int
+get_chassis_external_id_value_int(const struct smap *external_ids,
+                                  const char *chassis_id,
+                                  const char *option_key,
+                                  int def);
+
+bool
+get_chassis_external_id_value_bool(const struct smap *external_ids,
+                                   const char *chassis_id,
+                                   const char *option_key,
+                                   bool def);
+
 /* Returns a lowercase copy of orig.
  * Caller must free the returned string.
  */
diff --git a/ovn-sb.xml b/ovn-sb.xml
index 59b21711b..f84bd5302 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -240,10 +240,12 @@
 
     <column name="name">
       OVN does not prescribe a particular format for chassis names.
-      ovn-controller populates this column using <ref key="system-id"
-      table="Open_vSwitch" column="external_ids" db="Open_vSwitch"/>
-      in the Open_vSwitch database's <ref table="Open_vSwitch"
-      db="Open_vSwitch"/> table.  ovn-controller-vtep populates this
+      ovn-controller populates this column using the <code>system-id</code>
+      configuration file, or <code>-n</code> CLI argument, or
+      <ref key="system-id" table="Open_vSwitch" column="external_ids"
+      db="Open_vSwitch"/> in the Open_vSwitch database's
+      <ref table="Open_vSwitch" db="Open_vSwitch"/> table.
+      ovn-controller-vtep populates this
       column with <ref table="Physical_Switch" column="name"
       db="hardware_vtep"/> in the hardware_vtep database's
       <ref table="Physical_Switch" db="hardware_vtep"/> table.
diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index 1b96934b1..2f1fe7387 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -50,8 +50,7 @@ patch
 # is mirrored into the Chassis record in the OVN_Southbound db.
 check_bridge_mappings () {
     local_mappings=$1
-    sysid=$(ovs-vsctl get Open_vSwitch . external_ids:system-id)
-    OVS_WAIT_UNTIL([test x"${local_mappings}" = x$(ovn-sbctl get Chassis ${sysid} other_config:ovn-bridge-mappings | sed -e 's/\"//g')])
+    OVS_WAIT_UNTIL([test x"${local_mappings}" = x$(ovn-sbctl get Chassis ${sandbox} other_config:ovn-bridge-mappings | sed -e 's/\"//g')])
 }
 
 # Initially there should be no patch ports.
@@ -133,13 +132,13 @@ ovs-vsctl \
     -- add-br br-eth2
 ovn_attach n1 br-phys 192.168.0.1
 
-sysid=$(ovs-vsctl get Open_vSwitch . external_ids:system-id)
+sysid=${sandbox}
 
 # Make sure that the datapath_type set in the Bridge table
 # is mirrored into the Chassis record in the OVN_Southbound db.
 check_datapath_type () {
     datapath_type=$1
-    chassis_datapath_type=$(ovn-sbctl get Chassis ${sysid} other_config:datapath-type | sed -e 's/"//g') #"
+    chassis_datapath_type=$(ovn-sbctl get Chassis ${sandbox} other_config:datapath-type | sed -e 's/"//g') #"
     test "${datapath_type}" = "${chassis_datapath_type}"
 }
 
@@ -187,7 +186,7 @@ OVS_WAIT_UNTIL([
     test "${expected_iface_types}" = "${chassis_iface_types}"
 ])
 
-# Change the value of external_ids:system-id and make sure it's mirrored
+# Set the value of external_ids:system-id and make sure it's mirrored
 # in the Chassis record in the OVN_Southbound database.
 sysid=${sysid}-foo
 ovs-vsctl set Open_vSwitch . external-ids:system-id="${sysid}"
diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
index c639c0ceb..1005350ab 100644
--- a/tests/ovn-macros.at
+++ b/tests/ovn-macros.at
@@ -215,7 +215,7 @@ net_attach () {
 
 # ovn_az_attach AZ NETWORK BRIDGE IP [MASKLEN]
 ovn_az_attach() {
-    local az=$1 net=$2 bridge=$3 ip=$4 masklen=${5-24}
+    local az=$1 net=$2 bridge=$3 ip=$4 masklen=${5-24} intbr=${6-br-int} chassis=$7
     net_attach $net $bridge || return 1
 
     mac=`ovs-vsctl get Interface $bridge mac_in_use | sed s/\"//g`
@@ -229,15 +229,48 @@ ovn_az_attach() {
     else
         ovn_remote=unix:$ovs_base/$az/ovn-sb/ovn-sb.sock
     fi
+
+    if [[ -n "${chassis}" ]]; then
+        bridge_key=ovn-bridge-${chassis}
+        remote_key=ovn-remote-${chassis}
+        encap_type_key=ovn-encap-type-${chassis}
+        encap_ip_key=ovn-encap-ip-${chassis}
+        chassis_args="-n $chassis"
+        chassis_vsctl_args=
+    else
+        bridge_key=ovn-bridge
+        remote_key=ovn-remote
+        encap_type_key=ovn-encap-type
+        encap_ip_key=ovn-encap-ip
+        chassis=$sandbox
+        chassis_args=
+        chassis_vsctl_args="-- set Open_vSwitch . external-ids:system-id=$chassis"
+    fi
+
     ovs-vsctl \
-        -- set Open_vSwitch . external-ids:system-id=$sandbox \
-        -- set Open_vSwitch . external-ids:ovn-remote=$ovn_remote \
-        -- set Open_vSwitch . external-ids:ovn-encap-type=geneve,vxlan \
-        -- set Open_vSwitch . external-ids:ovn-encap-ip=$ip \
-        -- --may-exist add-br br-int \
-        -- set bridge br-int fail-mode=secure other-config:disable-in-band=true \
+        $chassis_vsctl_args \
+        -- set Open_vSwitch . external-ids:$bridge_key=$intbr \
+        -- set Open_vSwitch . external-ids:$remote_key=$ovn_remote \
+        -- set Open_vSwitch . external-ids:$encap_type_key=geneve,vxlan \
+        -- set Open_vSwitch . external-ids:$encap_ip_key=$ip \
+        -- --may-exist add-br ${intbr} \
+        -- set bridge ${intbr} fail-mode=secure other-config:disable-in-band=true \
         || return 1
-    start_daemon ovn-controller || return 1
+
+    if [[ "${intbr}" = br-int ]]; then
+        pidfile="${OVS_RUNDIR}/ovn-controller.pid"
+        logfile="${OVS_LOGDIR}/ovn-controller.log"
+    else
+        pidfile="${OVS_RUNDIR}/ovn-controller-${intbr}.pid"
+        logfile="${OVS_LOGDIR}/ovn-controller-${chassis}.log"
+    fi
+
+    ovn-controller \
+        ${chassis_args} \
+        -vconsole:off --detach --no-chdir \
+        --pidfile=${pidfile} \
+        --log-file=${logfile} || return 1
+    on_exit "test -e \"$pidfile\" && kill \`cat \"$pidfile\"\`"
 }
 
 # ovn_attach NETWORK BRIDGE IP [MASKLEN]
diff --git a/tests/ovn.at b/tests/ovn.at
index c4edbd9e1..0d6f43e4b 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1727,7 +1727,60 @@ AT_CLEANUP
 
 AT_BANNER([OVN end-to-end tests])
 
-# 3 hypervisors, one logical switch, 3 logical ports per hypervisor
+AT_SETUP([ovn -- 3 virtual hosts, same node])
+AT_KEYWORDS([ovn])
+ovn_start
+ovn-nbctl ls-add lsw0
+net_add n1
+sim_add hv
+
+as hv
+for i in 1 2 3; do
+    chassis=host-$i
+    ovs-vsctl add-br br-phys-$i
+    ovn_attach n1 br-phys-$i 192.168.0.$i 24 br-int-$i $chassis
+
+    for j in 1 2 3; do
+        lpname=lp$i$j
+        ovs-vsctl add-port br-int-$i vif$i$j -- set Interface vif$i$j external-ids:iface-id=$lpname options:tx_pcap=hv$i/vif$i$j-tx.pcap options:rxq_pcap=hv$i/vif$i$j-rx.pcap ofport-request=$i$j
+        ovn-nbctl lsp-add lsw0 $lpname
+        OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up $lpname` = xup])
+
+        pb_chassis_id=$(ovn-sbctl --bare --columns chassis list port_binding $lpname)
+        pb_chassis_name=$(ovn-sbctl get chassis $pb_chassis_id name)
+        AT_FAIL_IF([test x$pb_chassis_name != x$chassis])
+    done
+done
+
+for i in 1 2 3; do
+    > expout
+    for vif in 1 2 3; do
+        echo vif$i$vif >> expout
+    done
+    AT_CHECK([ovs-vsctl list-ports br-int-$i | grep vif], [0], [expout])
+done
+
+AT_CLEANUP
+
+AT_SETUP([ovn -- system-id in file])
+AT_KEYWORDS([ovn])
+
+ovn_start
+net_add n1
+sim_add hv
+
+as hv
+
+echo otherid > ${OVS_SYSCONFDIR}/system-id
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+
+# system-id file overrides chassis name selected via cli
+echo otherid > expout
+AT_CHECK([ovn-sbctl --bare --columns name list chassis], [0], [expout])
+
+AT_CLEANUP
+
 AT_SETUP([ovn -- 3 HVs, 1 LS, 3 lports/HV])
 AT_KEYWORDS([ovnarp])
 ovn_start
-- 
2.26.2



More information about the dev mailing list