[ovs-dev] [PATCH ovn 3/4] ovn-controller: Detect OVS datapath capabilities.
Dumitru Ceara
dceara at redhat.com
Thu Jun 3 15:05:52 UTC 2021
Automatically create an OVS Datapath record if none exists for the
current br-int datapath type.
Add a 'features' module to track which OVS features are available in
the datapath currently being used. For now, only ct_zero_snat is
tracked, all other features are assumed to be on-par between all
datapaths.
A future commit will make use of the 'features' module to conditionally
program openflows based on available datapath features.
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
---
controller/ovn-controller.c | 125 ++++++++++++++++++++++++++++++++++---------
include/ovn/features.h | 16 ++++++
lib/automake.mk | 1
lib/features.c | 39 +++++++++++++
tests/ovn-controller.at | 11 ++--
5 files changed, 159 insertions(+), 33 deletions(-)
create mode 100644 lib/features.c
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index d48ddc7a2..15af7a13c 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -46,6 +46,7 @@
#include "openvswitch/vconn.h"
#include "openvswitch/vlog.h"
#include "ovn/actions.h"
+#include "ovn/features.h"
#include "lib/chassis-index.h"
#include "lib/extend-table.h"
#include "lib/ip-mcast-index.h"
@@ -88,6 +89,7 @@ static unixctl_cb_func lflow_cache_show_stats_cmd;
static unixctl_cb_func debug_delay_nb_cfg_report;
#define DEFAULT_BRIDGE_NAME "br-int"
+#define DEFAULT_DATAPATH "system"
#define DEFAULT_PROBE_INTERVAL_MSEC 5000
#define OFCTRL_DEFAULT_PROBE_INTERVAL_SEC 0
@@ -95,6 +97,8 @@ static unixctl_cb_func debug_delay_nb_cfg_report;
#define OVS_NB_CFG_NAME "ovn-nb-cfg"
+#define OVS_CT_ZERO_SNAT_FEATURE "ct_zero_snat"
+
static char *parse_options(int argc, char *argv[]);
OVS_NO_RETURN static void usage(void);
@@ -319,10 +323,6 @@ static const struct ovsrec_bridge *
create_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
const struct ovsrec_open_vswitch_table *ovs_table)
{
- if (!ovs_idl_txn) {
- return NULL;
- }
-
const struct ovsrec_open_vswitch *cfg;
cfg = ovsrec_open_vswitch_table_first(ovs_table);
if (!cfg) {
@@ -386,6 +386,21 @@ create_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
return bridge;
}
+static const struct ovsrec_datapath *
+create_br_datapath(struct ovsdb_idl_txn *ovs_idl_txn,
+ const struct ovsrec_open_vswitch *cfg,
+ const char *datapath_type)
+{
+ ovsdb_idl_txn_add_comment(ovs_idl_txn,
+ "ovn-controller: creating bridge datapath '%s'",
+ datapath_type);
+
+ struct ovsrec_datapath *dp = ovsrec_datapath_insert(ovs_idl_txn);
+ ovsrec_open_vswitch_verify_datapaths(cfg);
+ ovsrec_open_vswitch_update_datapaths_setkey(cfg, datapath_type, dp);
+ return dp;
+}
+
static const struct ovsrec_bridge *
get_br_int(const struct ovsrec_bridge_table *bridge_table,
const struct ovsrec_open_vswitch_table *ovs_table)
@@ -399,33 +414,78 @@ get_br_int(const struct ovsrec_bridge_table *bridge_table,
return get_bridge(bridge_table, br_int_name(cfg));
}
-static const struct ovsrec_bridge *
+static const struct ovsrec_datapath *
+get_br_datapath(const struct ovsrec_open_vswitch *cfg,
+ const char *datapath_type)
+{
+ for (size_t i = 0; i < cfg->n_datapaths; i++) {
+ if (!strcmp(cfg->key_datapaths[i], datapath_type)) {
+ return cfg->value_datapaths[i];
+ }
+ }
+ return NULL;
+}
+
+static void
process_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
const struct ovsrec_bridge_table *bridge_table,
- const struct ovsrec_open_vswitch_table *ovs_table)
+ const struct ovsrec_open_vswitch_table *ovs_table,
+ const struct ovsrec_bridge **br_int_,
+ const struct ovsrec_datapath **br_int_dp_)
{
- const struct ovsrec_bridge *br_int = get_br_int(bridge_table,
- ovs_table);
- if (!br_int) {
- br_int = create_br_int(ovs_idl_txn, ovs_table);
- }
- if (br_int && 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");
- /* 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)) {
- ovsrec_bridge_set_datapath_type(br_int, datapath_type);
+ const struct ovsrec_bridge *br_int = get_br_int(bridge_table, ovs_table);
+ const struct ovsrec_datapath *br_int_dp = NULL;
+ if (ovs_idl_txn) {
+ if (!br_int) {
+ br_int = create_br_int(ovs_idl_txn, ovs_table);
}
- if (!br_int->fail_mode || strcmp(br_int->fail_mode, "secure")) {
- ovsrec_bridge_set_fail_mode(br_int, "secure");
- VLOG_WARN("Integration bridge fail-mode changed to 'secure'.");
+
+ if (br_int) {
+ const struct ovsrec_open_vswitch *cfg =
+ ovsrec_open_vswitch_table_first(ovs_table);
+ ovs_assert(cfg);
+
+ /* Propagate "ovn-bridge-datapath-type" from OVS table, if any.
+ * Otherwise use the datapath-type set in br-int, if any.
+ * Finally, assume "system" datapath if none configured.
+ */
+ const char *datapath_type =
+ smap_get(&cfg->external_ids, "ovn-bridge-datapath-type");
+
+ if (!datapath_type) {
+ if (br_int->datapath_type[0]) {
+ datapath_type = br_int->datapath_type;
+ } else {
+ datapath_type = DEFAULT_DATAPATH;
+ }
+ }
+ if (strcmp(br_int->datapath_type, datapath_type)) {
+ ovsrec_bridge_set_datapath_type(br_int, datapath_type);
+ }
+ if (!br_int->fail_mode || strcmp(br_int->fail_mode, "secure")) {
+ ovsrec_bridge_set_fail_mode(br_int, "secure");
+ VLOG_WARN("Integration bridge fail-mode changed to 'secure'.");
+ }
+ br_int_dp = get_br_datapath(cfg, datapath_type);
+ if (!br_int_dp) {
+ br_int_dp = create_br_datapath(ovs_idl_txn, cfg,
+ datapath_type);
+ }
}
}
- return br_int;
+ *br_int_ = br_int;
+ *br_int_dp_ = br_int_dp;
+}
+
+/* Returns 'true' if the feature set supported by 'ovs_datapath' have been
+ * updated since the last call.
+ */
+static bool
+process_ovs_datapath(const struct ovsrec_datapath *ovs_datapath)
+{
+ bool ct_zero_snat = smap_get_bool(&ovs_datapath->capabilities,
+ OVS_CT_ZERO_SNAT_FEATURE, false);
+ return ovs_feature_support_update(ct_zero_snat);
}
static const char *
@@ -848,6 +908,7 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
ovsdb_idl_add_table(ovs_idl, &ovsrec_table_open_vswitch);
ovsdb_idl_add_column(ovs_idl, &ovsrec_open_vswitch_col_external_ids);
ovsdb_idl_add_column(ovs_idl, &ovsrec_open_vswitch_col_bridges);
+ ovsdb_idl_add_column(ovs_idl, &ovsrec_open_vswitch_col_datapaths);
ovsdb_idl_add_table(ovs_idl, &ovsrec_table_interface);
ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_name);
ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_bfd);
@@ -870,6 +931,8 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
ovsdb_idl_add_column(ovs_idl, &ovsrec_ssl_col_ca_cert);
ovsdb_idl_add_column(ovs_idl, &ovsrec_ssl_col_certificate);
ovsdb_idl_add_column(ovs_idl, &ovsrec_ssl_col_private_key);
+ ovsdb_idl_add_table(ovs_idl, &ovsrec_table_datapath);
+ ovsdb_idl_add_column(ovs_idl, &ovsrec_datapath_col_capabilities);
chassis_register_ovs_idl(ovs_idl);
encaps_register_ovs_idl(ovs_idl);
binding_register_ovs_idl(ovs_idl);
@@ -2977,8 +3040,10 @@ main(int argc, char *argv[])
ovsrec_bridge_table_get(ovs_idl_loop.idl);
const struct ovsrec_open_vswitch_table *ovs_table =
ovsrec_open_vswitch_table_get(ovs_idl_loop.idl);
- const struct ovsrec_bridge *br_int =
- process_br_int(ovs_idl_txn, bridge_table, ovs_table);
+ const struct ovsrec_bridge *br_int = NULL;
+ const struct ovsrec_datapath *br_int_dp = NULL;
+ process_br_int(ovs_idl_txn, bridge_table, ovs_table,
+ &br_int, &br_int_dp);
if (ovsdb_idl_has_ever_connected(ovnsb_idl_loop.idl) &&
northd_version_match) {
@@ -3009,6 +3074,12 @@ main(int argc, char *argv[])
&chassis_private);
}
+ /* If any OVS feature support changed, force a full recompute. */
+ if (br_int_dp && process_ovs_datapath(br_int_dp)) {
+ VLOG_INFO("OVS feature set changed, force recompute.");
+ engine_set_force_recompute(true);
+ }
+
if (br_int) {
ct_zones_data = engine_get_data(&en_ct_zones);
if (ct_zones_data) {
diff --git a/include/ovn/features.h b/include/ovn/features.h
index 10ee46fcd..8a728a847 100644
--- a/include/ovn/features.h
+++ b/include/ovn/features.h
@@ -16,7 +16,23 @@
#ifndef OVN_FEATURES_H
#define OVN_FEATURES_H 1
+#include <stdbool.h>
+
/* ovn-controller supported feature names. */
#define OVN_FEATURE_PORT_UP_NOTIF "port-up-notif"
+/* OVS datapath supported features. Based on availability OVN might generate
+ * different types of openflows.
+ */
+enum ovs_feature_support_bits {
+ OVS_CT_ZERO_SNAT_SUPPORT_BIT,
+};
+
+enum ovs_feature_support {
+ OVS_CT_ZERO_SNAT_SUPPORT = (1 << OVS_CT_ZERO_SNAT_SUPPORT_BIT),
+};
+
+enum ovs_feature_support ovs_feature_support_get(void);
+bool ovs_feature_support_update(enum ovs_feature_support features);
+
#endif
diff --git a/lib/automake.mk b/lib/automake.mk
index 781be2109..917b28e1e 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -13,6 +13,7 @@ lib_libovn_la_SOURCES = \
lib/expr.c \
lib/extend-table.h \
lib/extend-table.c \
+ lib/features.c \
lib/ovn-parallel-hmap.h \
lib/ovn-parallel-hmap.c \
lib/ip-mcast-index.c \
diff --git a/lib/features.c b/lib/features.c
new file mode 100644
index 000000000..697fae112
--- /dev/null
+++ b/lib/features.c
@@ -0,0 +1,39 @@
+/* Copyright (c) 2021, Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <config.h>
+
+#include "ovn/features.h"
+
+static enum ovs_feature_support ovs_features;
+
+enum ovs_feature_support
+ovs_feature_support_get(void)
+{
+ return ovs_features;
+}
+
+/* Returns 'true' if the OVS feature set has been updated since the last
+ * call.
+ */
+bool
+ovs_feature_support_update(enum ovs_feature_support features)
+{
+ if (features != ovs_features) {
+ ovs_features = features;
+ return true;
+ }
+ return false;
+}
diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index 72c07b3fa..9c25193e8 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -151,23 +151,24 @@ sysid=$(ovs-vsctl get Open_vSwitch . external_ids:system-id)
check_datapath_type () {
datapath_type=$1
chassis_datapath_type=$(ovn-sbctl get Chassis ${sysid} other_config:datapath-type | sed -e 's/"//g') #"
- test "${datapath_type}" = "${chassis_datapath_type}"
+ ovs_datapath_type=$(ovs-vsctl get Bridge br-int datapath-type)
+ test "${datapath_type}" = "${chassis_datapath_type}" && test "${datapath_type}" = "${ovs_datapath_type}"
}
-OVS_WAIT_UNTIL([check_datapath_type ""])
+OVS_WAIT_UNTIL([check_datapath_type system])
ovs-vsctl set Bridge br-int datapath-type=foo
OVS_WAIT_UNTIL([check_datapath_type foo])
# Change "ovn-bridge-mappings" value. It should not change the "datapath-type".
ovs-vsctl set Open_vSwitch . external_ids:ovn-bridge-mappings=foo-mapping
-check_datapath_type foo
+AT_CHECK([check_datapath_type foo])
ovs-vsctl set Bridge br-int datapath-type=bar
OVS_WAIT_UNTIL([check_datapath_type bar])
ovs-vsctl set Bridge br-int datapath-type=\"\"
-OVS_WAIT_UNTIL([check_datapath_type ""])
+OVS_WAIT_UNTIL([check_datapath_type system])
# Set the datapath_type in external_ids:ovn-bridge-datapath-type.
ovs-vsctl set Open_vSwitch . external_ids:ovn-bridge-datapath-type=foo
@@ -176,11 +177,9 @@ OVS_WAIT_UNTIL([check_datapath_type foo])
# Change the br-int's datapath type to bar.
# It should be reset to foo since ovn-bridge-datapath-type is configured.
ovs-vsctl set Bridge br-int datapath-type=bar
-OVS_WAIT_UNTIL([test foo = `ovs-vsctl get Bridge br-int datapath-type`])
OVS_WAIT_UNTIL([check_datapath_type foo])
ovs-vsctl set Open_vSwitch . external_ids:ovn-bridge-datapath-type=foobar
-OVS_WAIT_UNTIL([test foobar = `ovs-vsctl get Bridge br-int datapath-type`])
OVS_WAIT_UNTIL([check_datapath_type foobar])
expected_iface_types=$(ovs-vsctl get Open_vSwitch . iface_types | tr -d '[[]] ""')
More information about the dev
mailing list