On 10/18/22 14:33, Ihar Hrachyshka wrote:
Before the patch, all controller instances were reading configuration
from the same external-ids:ovn-* options. This patch adds support for
distinct config otions for different chassis names stored in the same
ovsdb global config object.
To configure an option for a distinct chassis name, an admin may add a
suffix with the desired chassis name to a config option. For example, if
the following is configured in ovsdb, only a controller with the
corresponding chassis name (either 'hv1' or 'hv2') would read just one
of the following options:
ovs-vsctl set open . external-ids:ovn-bridge-mappings-hv1=phys:br-phys-1
ovs-vsctl set open . external-ids:ovn-bridge-mappings-hv2=phys:br-phys-2
Chassis specific config options override any global settings, so for
example if the following configuration is used, then controller 'hv1'
will use the first setting but not the latter. Any other controllers
will use the global setting, which is the second setting..
ovs-vsctl set open . external-ids:ovn-bridge-mappings-hv1=phys:br-phys-1
ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys-2
This is supported for other options too.
This is in preparation to support running multiple controller instances
using the same vswitchd instance.
Signed-off-by: Ihar Hrachyshka <[email protected]>
---
controller/chassis.c | 153 +++++++++++++++++++++-----------
controller/chassis.h | 6 +-
controller/encaps.c | 19 ++--
controller/ovn-controller.8.xml | 12 +++
controller/ovn-controller.c | 123 ++++++++++++++-----------
controller/patch.c | 6 +-
controller/physical.c | 2 +-
lib/ovn-util.c | 87 ++++++++++++++++++
lib/ovn-util.h | 26 ++++++
tests/automake.mk | 1 +
tests/ovn-macros.at | 4 +-
tests/ovn.at | 41 +++++++++
12 files changed, 361 insertions(+), 119 deletions(-)
diff --git a/controller/chassis.c b/controller/chassis.c
index 685d9b2ae..c8a332786 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -93,9 +93,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", "");
hostname is an interesting field on the chassis. The "requested-chassis"
option for logical switch ports may refer to a chassis name or chassis
hostname. The "hostname" column is not indexed, so there currently isn't
anything stopping multiple chassis from having the same hostname.
However, I suspect this tends to just work because people don't set the
hostname in the OVSDB and instead rely on the fallback behavior of
get_hostname(), which calls the system's gethostname() function instead.
Since there is one chassis record per host, this results in each chassis
record having a unique hostname.
So now we introduce the situation where there can be multiple chassis
records on the same host. This means that if the hostname is not
configured in the OVSDB, then all chassis records on this host will have
the same hostname value.
This can be OK. If each chassis is running as part of a distinct OVN
deployment (i.e. each chassis record is in a different southbound
database), then there is no issue of ambiguity. Even though each chassis
will have the same hostname, each ovn northd will only be able to see
the chassis that is in its southbound DB. This is what I had assumed the
target use case for multiple chassis on the same host is.
However, if each chassis is part of the same OVN deployment (which is
what the tests introduced in patches 5 and 6 do), then having the same
hostname could be a problem, since "requested-chassis" can be ambiguous.
I'm not sure what the best way to handle this is. One way is to just
allow for the same hostname to be repeated across chassis records and
just document that chassis names *must* be used for "requested-chassis".
Another would be to append the chassis index to the hostname and store
that in the chassis record's hostname field. Both ideas have drawbacks,
and I'm not sure what is best.
if (strlen(hostname) == 0) {
static char hostname_[HOST_NAME_MAX + 1];
@@ -111,69 +112,81 @@ 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_enable_lflow_cache(const struct smap *ext_ids)
+get_enable_lflow_cache(const struct smap *ext_ids, const char *chassis_id)
{
- return smap_get_def(ext_ids, "ovn-enable-lflow-cache", "true");
+ return get_chassis_external_id_value(ext_ids, chassis_id,
+ "ovn-enable-lflow-cache", "true");
}
static const char *
-get_limit_lflow_cache(const struct smap *ext_ids)
+get_limit_lflow_cache(const struct smap *ext_ids, const char *chassis_id)
{
- return smap_get_def(ext_ids, "ovn-limit-lflow-cache", "");
+ return get_chassis_external_id_value(ext_ids, chassis_id,
+ "ovn-limit-lflow-cache", "");
}
static const char *
-get_memlimit_lflow_cache(const struct smap *ext_ids)
+get_memlimit_lflow_cache(const struct smap *ext_ids, const char *chassis_id)
{
- return smap_get_def(ext_ids, "ovn-memlimit-lflow-cache-kb", "");
+ return get_chassis_external_id_value(ext_ids, chassis_id,
+ "ovn-memlimit-lflow-cache-kb", "");
}
static const char *
-get_trim_limit_lflow_cache(const struct smap *ext_ids)
+get_trim_limit_lflow_cache(const struct smap *ext_ids, const char *chassis_id)
{
- return smap_get_def(ext_ids, "ovn-trim-limit-lflow-cache", "");
+ return get_chassis_external_id_value(ext_ids, chassis_id,
+ "ovn-trim-limit-lflow-cache", "");
}
static const char *
-get_trim_wmark_perc_lflow_cache(const struct smap *ext_ids)
+get_trim_wmark_perc_lflow_cache(const struct smap *ext_ids,
+ const char *chassis_id)
{
- return smap_get_def(ext_ids, "ovn-trim-wmark-perc-lflow-cache", "");
+ return get_chassis_external_id_value(
+ ext_ids, chassis_id, "ovn-trim-wmark-perc-lflow-cache", "");
}
static const char *
-get_trim_timeout(const struct smap *ext_ids)
+get_trim_timeout(const struct smap *ext_ids, const char *chassis_id)
{
- return smap_get_def(ext_ids, "ovn-trim-timeout-ms", "");
+ return get_chassis_external_id_value(ext_ids, chassis_id,
+ "ovn-trim-timeout-ms", "");
}
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 *
@@ -187,9 +200,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
@@ -260,6 +274,23 @@ chassis_parse_ovs_iface_types(char **iface_types, size_t
n_iface_types,
return true;
}
+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;
+}
+
/*
* Parse the 'ovs_table' entry and populate 'ovs_cfg'.
*/
@@ -276,30 +307,41 @@ 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(ovs_table);
+ 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->enable_lflow_cache = get_enable_lflow_cache(&cfg->external_ids);
- ovs_cfg->limit_lflow_cache = get_limit_lflow_cache(&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);
+ ovs_cfg->enable_lflow_cache =
+ get_enable_lflow_cache(&cfg->external_ids, chassis_id);
+ ovs_cfg->limit_lflow_cache =
+ get_limit_lflow_cache(&cfg->external_ids, chassis_id);
ovs_cfg->memlimit_lflow_cache =
- get_memlimit_lflow_cache(&cfg->external_ids);
+ get_memlimit_lflow_cache(&cfg->external_ids, chassis_id);
ovs_cfg->trim_limit_lflow_cache =
- get_trim_limit_lflow_cache(&cfg->external_ids);
+ get_trim_limit_lflow_cache(&cfg->external_ids, chassis_id);
ovs_cfg->trim_wmark_perc_lflow_cache =
- get_trim_wmark_perc_lflow_cache(&cfg->external_ids);
- ovs_cfg->trim_timeout_ms = get_trim_timeout(&cfg->external_ids);
+ get_trim_wmark_perc_lflow_cache(&cfg->external_ids, chassis_id);
+ ovs_cfg->trim_timeout_ms =
+ get_trim_timeout(&cfg->external_ids, chassis_id);
if (!chassis_parse_ovs_encap_type(encap_type, &ovs_cfg->encap_type_set)) {
return false;
@@ -322,7 +364,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;
}
@@ -362,7 +404,7 @@ chassis_other_config_changed(const struct ovs_chassis_cfg
*ovs_cfg,
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, chassis_rec->name);
if (strcmp(ovs_cfg->bridge_mappings, chassis_bridge_mappings)) {
return true;
@@ -376,42 +418,44 @@ chassis_other_config_changed(const struct ovs_chassis_cfg
*ovs_cfg,
}
const char *chassis_cms_options =
- get_cms_options(&chassis_rec->other_config);
+ get_cms_options(&chassis_rec->other_config, chassis_rec->name);
if (strcmp(ovs_cfg->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, chassis_rec->name);
if (strcmp(ovs_cfg->monitor_all, chassis_monitor_all)) {
return true;
}
const char *chassis_enable_lflow_cache =
- get_enable_lflow_cache(&chassis_rec->other_config);
+ get_enable_lflow_cache(&chassis_rec->other_config, chassis_rec->name);
if (strcmp(ovs_cfg->enable_lflow_cache, chassis_enable_lflow_cache)) {
return true;
}
const char *chassis_limit_lflow_cache =
- get_limit_lflow_cache(&chassis_rec->other_config);
+ get_limit_lflow_cache(&chassis_rec->other_config, chassis_rec->name);
if (strcmp(ovs_cfg->limit_lflow_cache, chassis_limit_lflow_cache)) {
return true;
}
const char *chassis_memlimit_lflow_cache =
- get_memlimit_lflow_cache(&chassis_rec->other_config);
+ get_memlimit_lflow_cache(&chassis_rec->other_config,
+ chassis_rec->name);
if (strcmp(ovs_cfg->memlimit_lflow_cache, chassis_memlimit_lflow_cache)) {
return true;
}
const char *chassis_trim_limit_lflow_cache =
- get_trim_limit_lflow_cache(&chassis_rec->other_config);
+ get_trim_limit_lflow_cache(&chassis_rec->other_config,
+ chassis_rec->name);
if (strcmp(ovs_cfg->trim_limit_lflow_cache,
chassis_trim_limit_lflow_cache)) {
@@ -419,7 +463,8 @@ chassis_other_config_changed(const struct ovs_chassis_cfg
*ovs_cfg,
}
const char *chassis_trim_wmark_perc_lflow_cache =
- get_trim_wmark_perc_lflow_cache(&chassis_rec->other_config);
+ get_trim_wmark_perc_lflow_cache(
+ &chassis_rec->other_config, chassis_rec->name);
if (strcmp(ovs_cfg->trim_wmark_perc_lflow_cache,
chassis_trim_wmark_perc_lflow_cache)) {
@@ -427,14 +472,15 @@ chassis_other_config_changed(const struct ovs_chassis_cfg
*ovs_cfg,
}
const char *chassis_trim_timeout_ms =
- get_trim_timeout(&chassis_rec->other_config);
+ get_trim_timeout(&chassis_rec->other_config, chassis_rec->name);
if (strcmp(ovs_cfg->trim_timeout_ms, chassis_trim_timeout_ms)) {
return true;
}
const char *chassis_mac_mappings =
- get_chassis_mac_mappings(&chassis_rec->other_config);
+ get_chassis_mac_mappings(&chassis_rec->other_config,
+ chassis_rec->name);
if (strcmp(ovs_cfg->chassis_macs, chassis_mac_mappings)) {
return true;
}
@@ -754,8 +800,9 @@ chassis_get_mac(const struct sbrec_chassis *chassis_rec,
const char *bridge_mapping,
struct eth_addr *chassis_mac)
{
- const char *tokens
- = get_chassis_mac_mappings(&chassis_rec->other_config);
+ const char *tokens =
+ get_chassis_mac_mappings(&chassis_rec->other_config,
+ chassis_rec->name);
if (!tokens[0]) {
return false;
}
diff --git a/controller/chassis.h b/controller/chassis.h
index 18b45a1c5..05a96bb0c 100644
--- a/controller/chassis.h
+++ b/controller/chassis.h
@@ -46,7 +46,9 @@ bool chassis_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,
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_chassis_mac_mappings(const struct smap *ext_ids,
+ const char *chassis_id);
+const char *
+get_ovs_chassis_id(const struct ovsrec_open_vswitch_table *ovs_table);
#endif /* controller/chassis.h */
diff --git a/controller/encaps.c b/controller/encaps.c
index a82088d42..0bc887983 100644
--- a/controller/encaps.c
+++ b/controller/encaps.c
@@ -194,24 +194,29 @@ tunnel_add(struct tunnel_ctx *tc, const struct
sbrec_sb_global *sbg,
bool set_local_ip = false;
if (cfg) {
/* If the tos option is configured, get it */
- const char *encap_tos = smap_get_def(&cfg->external_ids,
- "ovn-encap-tos", "none");
+ const char *encap_tos =
+ get_chassis_external_id_value(
+ &cfg->external_ids, tc->this_chassis->name,
+ "ovn-encap-tos", "none");
if (encap_tos && strcmp(encap_tos, "none")) {
smap_add(&options, "tos", encap_tos);
}
/* If the df_default option is configured, get it */
-
- const char *encap_df = smap_get(&cfg->external_ids,
- "ovn-encap-df_default");
+ const char *encap_df =
+ get_chassis_external_id_value(
+ &cfg->external_ids, tc->this_chassis->name,
+ "ovn-encap-df_default", NULL);
if (encap_df) {
smap_add(&options, "df_default", encap_df);
}
/* If ovn-set-local-ip option is configured, get it */
- set_local_ip = smap_get_bool(&cfg->external_ids, "ovn-set-local-ip",
- false);
+ set_local_ip =
+ get_chassis_external_id_value_bool(
+ &cfg->external_ids, tc->this_chassis->name,
+ "ovn-set-local-ip", false);
}
/* Add auth info if ipsec is enabled. */
diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml
index eb2950723..19dfa0ff5 100644
--- a/controller/ovn-controller.8.xml
+++ b/controller/ovn-controller.8.xml
@@ -354,6 +354,18 @@
</dd>
</dl>
+ <p>
+ Most of configuration options listed above can also be set for a
+ particular chassis name (see <code>external_ids:system-id </code>
+ for more information). This can be achieved by setting
+ <code>external_ids:option-[chassis]</code> instead of
+ <code>external_ids:option</code>. For example, set
+ <code>external_ids:ovn-encap-ip-otherhv</code> to use a particular
+ IP address for the controller instance named <code>otherhv</code>.
+ Name specific configuration options always override any global options
+ set in the database.
+ </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 5c2a4b6ef..f40633517 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -311,9 +311,13 @@ out:;
}
static const char *
-br_int_name(const struct ovsrec_open_vswitch *cfg)
+br_int_name(const struct ovsrec_open_vswitch_table *ovs_table)
{
- return smap_get_def(&cfg->external_ids, "ovn-bridge", DEFAULT_BRIDGE_NAME);
+ const struct ovsrec_open_vswitch *cfg =
+ ovsrec_open_vswitch_table_first(ovs_table);
+ const char *chassis_id = get_ovs_chassis_id(ovs_table);
+ return get_chassis_external_id_value(&cfg->external_ids, chassis_id,
+ "ovn-bridge", DEFAULT_BRIDGE_NAME);
It's probably a bad idea to allow for the same ovn-bridge to be used by
multiple chassis on a host. If the chassis are part of the same
deployment, then we may end up writing duplicate flows into the bridge.
If they are part of different deployments, then all hell will break loose :)
One way to fix this is to append the chassis index that is calculated in
patch 1 onto the configured bridge name so that we guarantee separate
bridges for each chassis. The resulting bridge would not match the
configured bridge name, but it will guarantee consistent behavior.
}
static const struct ovsrec_bridge *
@@ -325,7 +329,7 @@ create_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
if (!cfg) {
return NULL;
}
- const char *bridge_name = br_int_name(cfg);
+ const char *bridge_name = br_int_name(ovs_table);
ovsdb_idl_txn_add_comment(ovs_idl_txn,
"ovn-controller: creating integration bridge '%s'", bridge_name);
@@ -410,7 +414,7 @@ get_br_int(const struct ovsrec_bridge_table *bridge_table,
return NULL;
}
- return get_bridge(bridge_table, br_int_name(cfg));
+ return get_bridge(bridge_table, br_int_name(ovs_table));
}
static const struct ovsrec_datapath *
@@ -449,8 +453,11 @@ process_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
* Otherwise use the datapath-type set in br-int, if any.
* Finally, assume "system" datapath if none configured.
*/
+ const char *chassis_id = get_ovs_chassis_id(ovs_table);
const char *datapath_type =
- smap_get(&cfg->external_ids, "ovn-bridge-datapath-type");
+ get_chassis_external_id_value(
+ &cfg->external_ids, chassis_id,
+ "ovn-bridge-datapath-type", NULL);
if (!datapath_type) {
if (br_int->datapath_type[0]) {
@@ -478,22 +485,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)
{
@@ -517,10 +508,16 @@ 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;
+ }
+
+ const struct ovsrec_open_vswitch_table *ovs_table =
+ ovsrec_open_vswitch_table_get(ovs_idl);
+ const char *chassis_id = get_ovs_chassis_id(ovs_table);
+ return get_chassis_external_id_value_int(
+ &cfg->external_ids, chassis_id,
+ "ovn-openflow-probe-interval", OFCTRL_DEFAULT_PROBE_INTERVAL_SEC);
}
/* Retrieves the pointer to the OVN Southbound database from 'ovs_idl' and
@@ -537,18 +534,26 @@ 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 struct ovsrec_open_vswitch_table *ovs_table =
+ ovsrec_open_vswitch_table_get(ovs_idl);
+ const char *chassis_id = get_ovs_chassis_id(ovs_table);
+ 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
@@ -571,25 +576,31 @@ update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl
*ovnsb_idl,
}
if (ctx) {
- lflow_cache_enable(ctx->lflow_cache,
- smap_get_bool(&cfg->external_ids,
- "ovn-enable-lflow-cache",
- true),
- smap_get_uint(&cfg->external_ids,
- "ovn-limit-lflow-cache",
- DEFAULT_LFLOW_CACHE_MAX_ENTRIES),
- smap_get_ullong(&cfg->external_ids,
- "ovn-memlimit-lflow-cache-kb",
- DEFAULT_LFLOW_CACHE_MAX_MEM_KB),
- smap_get_uint(&cfg->external_ids,
- "ovn-trim-limit-lflow-cache",
- DEFAULT_LFLOW_CACHE_TRIM_LIMIT),
- smap_get_uint(&cfg->external_ids,
- "ovn-trim-wmark-perc-lflow-cache",
- DEFAULT_LFLOW_CACHE_WMARK_PERC),
- smap_get_uint(&cfg->external_ids,
- "ovn-trim-timeout-ms",
- DEFAULT_LFLOW_CACHE_TRIM_TO_MS));
+ lflow_cache_enable(
+ ctx->lflow_cache,
+ get_chassis_external_id_value_bool(
+ &cfg->external_ids, chassis_id,
+ "ovn-enable-lflow-cache", true),
+ get_chassis_external_id_value_uint(
+ &cfg->external_ids, chassis_id,
+ "ovn-limit-lflow-cache",
+ DEFAULT_LFLOW_CACHE_MAX_ENTRIES),
+ get_chassis_external_id_value_ullong(
+ &cfg->external_ids, chassis_id,
+ "ovn-memlimit-lflow-cache-kb",
+ DEFAULT_LFLOW_CACHE_MAX_MEM_KB),
+ get_chassis_external_id_value_uint(
+ &cfg->external_ids, chassis_id,
+ "ovn-trim-limit-lflow-cache",
+ DEFAULT_LFLOW_CACHE_TRIM_LIMIT),
+ get_chassis_external_id_value_uint(
+ &cfg->external_ids, chassis_id,
+ "ovn-trim-wmark-perc-lflow-cache",
+ DEFAULT_LFLOW_CACHE_WMARK_PERC),
+ get_chassis_external_id_value_uint(
+ &cfg->external_ids, chassis_id,
+ "ovn-trim-timeout-ms",
+ DEFAULT_LFLOW_CACHE_TRIM_TO_MS));
}
}
@@ -812,7 +823,7 @@ restore_ct_zones(const struct ovsrec_bridge_table *bridge_table,
}
const struct ovsrec_bridge *br_int;
- br_int = get_bridge(bridge_table, br_int_name(cfg));
+ br_int = get_bridge(bridge_table, br_int_name(ovs_table));
if (!br_int) {
/* If the integration bridge hasn't been defined, assume that
* any existing ct-zone definitions aren't valid. */
@@ -921,7 +932,9 @@ get_transport_zones(const struct ovsrec_open_vswitch_table
*ovs_table)
{
const struct ovsrec_open_vswitch *cfg
= ovsrec_open_vswitch_table_first(ovs_table);
- return smap_get_def(&cfg->external_ids, "ovn-transport-zones", "");
+ const char *chassis_id = get_ovs_chassis_id(ovs_table);
+ return get_chassis_external_id_value(&cfg->external_ids, chassis_id,
+ "ovn-transport-zones", "");
}
static void
@@ -3494,8 +3507,12 @@ check_northd_version(struct ovsdb_idl *ovs_idl, struct
ovsdb_idl *ovnsb_idl,
static bool version_mismatch;
const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(ovs_idl);
- if (!cfg || !smap_get_bool(&cfg->external_ids, "ovn-match-northd-version",
- false)) {
+ const struct ovsrec_open_vswitch_table *ovs_table =
+ ovsrec_open_vswitch_table_get(ovs_idl);
+ const char *chassis_id = get_ovs_chassis_id(ovs_table);
+ if (!cfg || !get_chassis_external_id_value_bool(
+ &cfg->external_ids, chassis_id,
+ "ovn-match-northd-version", false)) {
version_mismatch = false;
return true;
}
diff --git a/controller/patch.c b/controller/patch.c
index 12e0b6f7c..af4e8911f 100644
--- a/controller/patch.c
+++ b/controller/patch.c
@@ -15,6 +15,7 @@
#include <config.h>
+#include "chassis.h"
#include "patch.h"
#include "ovsport.h"
@@ -131,7 +132,10 @@ 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");
+ const char *chassis_id = get_ovs_chassis_id(ovs_table);
+ mappings_cfg =
+ get_chassis_external_id_value(
+ &cfg->external_ids, chassis_id, "ovn-bridge-mappings", NULL);
if (!mappings_cfg || !mappings_cfg[0]) {
return;
}
diff --git a/controller/physical.c b/controller/physical.c
index 705146316..a37e71f46 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -503,7 +503,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, chassis->name);
if (!strlen(tokens)) {
continue;
diff --git a/lib/ovn-util.c b/lib/ovn-util.c
index d80db179a..ae9481403 100644
--- a/lib/ovn-util.c
+++ b/lib/ovn-util.c
@@ -935,3 +935,90 @@ daemon_started_recently(void)
/* Ensure that at least an amount of time has passed. */
return time_wall_msec() - startup_ts <= DAEMON_STARTUP_DELAY_MS;
}
+
+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) {
+ 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;
+ }
These functions have an issue that they essentially copy logic that
already exists in ovs's smap library. This was in an attempt to re-use
the common get_chassis_external_id_value(), but I think copying the smap
logic defeats the purpose.
I think a better alternative would be to make a function that calculates
the chassis-specific key, like:
static char *
chassis_option_key(const char *option_key, const char *chassis-id)
{
return xasprintf("%s-%s", option_key, chassis_id);
}
Then in each of these handlers, you could do something like:
int
get_chassis_external_id_value_int(const struct smap *external_ids,
const char *chassis_id,
const char *option_key,
int def)
{
char *chassis_key = chassis_option_key(option_key, chassis_id);
int ret = smap_get_int(external_ids, chassis_key,
smap_get_int(external_ids, option_key, def));
free(chassis_key);
return ret;
}
I think this should work for each of the various
get_chassis_external_id_value_* functions.
+
+ return i_value;
+}
+
+unsigned int
+get_chassis_external_id_value_uint(const struct smap *external_ids,
+ const char *chassis_id,
+ const char *option_key,
+ unsigned int def)
+{
+ const char *value = get_chassis_external_id_value(
+ external_ids, chassis_id, option_key, NULL);
+
+ unsigned int u_value;
+
+ if (!value || !str_to_uint(value, 10, &u_value)) {
+ return def;
+ }
+
+ return u_value;
+}
+
+unsigned long long int
+get_chassis_external_id_value_ullong(const struct smap *external_ids,
+ const char *chassis_id,
+ const char *option_key,
+ unsigned long long int def)
+{
+ const char *value = get_chassis_external_id_value(
+ external_ids, chassis_id, option_key, NULL);
+
+ unsigned long long ull_value;
+
+ if (!value || !str_to_ullong(value, 10, &ull_value)) {
+ return def;
+ }
+
+ return ull_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 145f974ed..2a19faa43 100644
--- a/lib/ovn-util.h
+++ b/lib/ovn-util.h
@@ -314,4 +314,30 @@ void daemon_started_recently_ignore(void);
bool daemon_started_recently(void);
int64_t daemon_startup_ts(void);
+const char *
+get_chassis_external_id_value(const struct smap *,
+ const char *chassis_id,
+ const char *option_key,
+ const char *def);
+int
+get_chassis_external_id_value_int(const struct smap *,
+ const char *chassis_id,
+ const char *option_key,
+ int def);
+unsigned int
+get_chassis_external_id_value_uint(const struct smap *,
+ const char *chassis_id,
+ const char *option_key,
+ unsigned int def);
+unsigned long long int
+get_chassis_external_id_value_ullong(const struct smap *external_ids,
+ const char *chassis_id,
+ const char *option_key,
+ unsigned long long int def);
+bool
+get_chassis_external_id_value_bool(const struct smap *,
+ const char *chassis_id,
+ const char *option_key,
+ bool def);
+
#endif /* OVN_UTIL_H */
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-macros.at b/tests/ovn-macros.at
index a3c7f8125..35b7b3872 100644
--- a/tests/ovn-macros.at
+++ b/tests/ovn-macros.at
@@ -290,7 +290,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} encap=${6-geneve,vxlan}
+ local az=$1 net=$2 bridge=$3 ip=$4 masklen=${5-24} encap=${6-geneve,vxlan}
systemid=${7-$sandbox}
net_attach $net $bridge || return 1
mac=`ovs-vsctl get Interface $bridge mac_in_use | sed s/\"//g`
@@ -310,7 +310,7 @@ ovn_az_attach() {
fi
ovs-vsctl \
-- set Open_vSwitch . external-ids:hostname=$sandbox \
- -- set Open_vSwitch . external-ids:system-id=$sandbox \
+ -- set Open_vSwitch . external-ids:system-id=$systemid \
-- set Open_vSwitch . external-ids:ovn-remote=$ovn_remote \
-- set Open_vSwitch . external-ids:ovn-encap-type=$encap \
-- set Open_vSwitch . external-ids:ovn-encap-ip=$ip \
diff --git a/tests/ovn.at b/tests/ovn.at
index f8b8db4df..9aac857f4 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -32975,3 +32975,44 @@ check ovn-nbctl --wait=hv sync
OVN_CLEANUP([hv1])
AT_CLEANUP
])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([chassis-specific configuration options])
+ovn_start
+net_add n1
+
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+
+ovs-vsctl \
+ -- set Open_vSwitch . external-ids:ovn-encap-type-hv3=geneve \
+ -- set Open_vSwitch . external-ids:ovn-encap-ip-hv3=192.168.1.1
+
+as hv1 ovs-vsctl set-ssl \
+ $PKIDIR/testpki-hv3-privkey.pem \
+ $PKIDIR/testpki-hv3-cert.pem \
+ $PKIDIR/testpki-cacert.pem
+
+ovn_attach n1 br-phys 192.168.0.1 24 vxlan hv3
+
+sim_add hv2
+as hv2
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.2 24 geneve
+
+# despite that we configured ovn-encap-ip=192.168.0.1, this setting is
+# overridden by chassis specific ovn-encap-ip-hv3
+OVS_WAIT_UNTIL([
+ test "1" = "$(ovn-sbctl list Encap | grep -c '192.168.1.1')"
+])
+
+encap_hv3_ip=$(fetch_column Encap ip chassis_name=hv3 type=geneve)
+AT_CHECK(test x$encap_hv3_ip == x192.168.1.1)
You can replace everything from the OVS_WAIT_UNTIL up to this point with:
wait_column 192.168.1.1 Encap ip chassis_name=hv3 type=geneve
+
+encap_hv1_ip=$(fetch_column Encap ip chassis_name=hv1 type=vxlan)
+AT_CHECK(test x$encap_hv1_ip == x)
You can replace the above two lines with:
check_column "" Encap ip chassis_name=hv1 type=vxlan
+
+OVN_CLEANUP([hv1],[hv2])
+AT_CLEANUP
+])
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
