Until now there was no good way to find how many elements are allowed
by the server schema in a particular database column. To support
working with different versions of OVS, we had to have a separate
database connection just to listen for schema changes, in order to know
how many flow table prefixes are allowed to be configured.
The newly introduced OVSDB IDL API allows to get the server-side
column type directly from the IDL. So, let's drop the schema parsing
code and use this new API instead.
OVS submodule is updated accordingly to include this and some other
internal OVS changes. As such, this bump also brings JSON performance
optimizations (storing of short strings and arrays in-place) that can
make JSON-RPC slightly more efficient.
Signed-off-by: Ilya Maximets <i.maxim...@ovn.org>
---
TODO.rst | 3 --
controller/ovn-controller.c | 15 +++---
include/ovn/features.h | 4 +-
lib/features.c | 104 +-----------------------------------
lib/test-ovn-features.c | 6 +--
ovs | 2 +-
6 files changed, 15 insertions(+), 119 deletions(-)
diff --git a/TODO.rst b/TODO.rst
index e50b1bd76..85208bfe3 100644
--- a/TODO.rst
+++ b/TODO.rst
@@ -138,9 +138,6 @@ OVN To-do List
* Remove ssl_ciphersuites workaround for clustered databases from ovn-ctl
after 26.03 release, assuming it will be an LTS release.
-* Remove OVS database schema parsing from features.c once server side column
- types are available through IDL.
-
* Dynamic Routing
* Add incremental processing of en_dynamic_routes for stateful configuration
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 4cbf1d858..477b3b089 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -43,6 +43,7 @@
#include "lflow-cache.h"
#include "lflow-conj-ids.h"
#include "lib/vswitch-idl.h"
+#include "lib/ovsdb-types.h"
#include "local_data.h"
#include "lport.h"
#include "memory.h"
@@ -546,12 +547,13 @@ static void
update_flow_table_prefixes(struct ovsdb_idl_txn *ovs_idl_txn,
const struct ovsrec_bridge *br_int)
{
- size_t max_prefixes = ovs_features_max_flow_table_prefixes_get();
+ const struct ovsdb_type *server_type;
struct ds ds = DS_EMPTY_INITIALIZER;
const char *prefixes[] = {
"ip_src", "ip_dst", "ipv6_src", "ipv6_dst",
};
struct ovsrec_flow_table *ft;
+ size_t max_prefixes;
size_t i;
/* We must not attempt setting more prefixes than our IDL supports.
@@ -561,12 +563,14 @@ update_flow_table_prefixes(struct ovsdb_idl_txn
*ovs_idl_txn,
ARRAY_SIZE(prefixes) <=
ovsrec_flow_table_columns[OVSREC_FLOW_TABLE_COL_PREFIXES].type.n_max);
- if (!max_prefixes) {
- /* Not discovered yet. */
+ server_type = ovsrec_flow_table_prefixes_server_type(
+ ovsdb_idl_txn_get_idl(ovs_idl_txn));
+ if (!server_type) {
+ /* Not connected or not in the server's schema somehow. */
return;
}
- max_prefixes = MIN(max_prefixes, ARRAY_SIZE(prefixes));
+ max_prefixes = MIN(server_type->n_max, ARRAY_SIZE(prefixes));
if (br_int->n_flow_tables == N_FLOW_TABLES &&
br_int->value_flow_tables[0]->n_prefixes == max_prefixes) {
/* Already up to date. Ideally, we would check every table,
@@ -6499,8 +6503,7 @@ main(int argc, char *argv[])
&& ovs_feature_support_run(br_int_dp ?
&br_int_dp->capabilities : NULL,
br_int_remote.target,
- br_int_remote.probe_interval,
- ovs_remote)) {
+ br_int_remote.probe_interval)) {
VLOG_INFO("OVS feature set changed, force recompute.");
engine_set_force_recompute();
diff --git a/include/ovn/features.h b/include/ovn/features.h
index 6e9c05e80..df24f6c06 100644
--- a/include/ovn/features.h
+++ b/include/ovn/features.h
@@ -60,11 +60,9 @@ enum ovs_feature_value {
void ovs_feature_support_destroy(void);
bool ovs_feature_is_supported(enum ovs_feature_value feature);
bool ovs_feature_support_run(const struct smap *ovs_capabilities,
- const char *conn_target, int probe_interval,
- const char *db_target);
+ const char *conn_target, int probe_interval);
bool ovs_feature_set_discovered(void);
uint32_t ovs_feature_max_meters_get(void);
uint32_t ovs_feature_max_select_groups_get(void);
-size_t ovs_features_max_flow_table_prefixes_get(void);
#endif
diff --git a/lib/features.c b/lib/features.c
index 4afb58a2a..7bee1c380 100644
--- a/lib/features.c
+++ b/lib/features.c
@@ -20,9 +20,6 @@
#include "lib/util.h"
#include "lib/dirs.h"
#include "socket-util.h"
-#include "lib/ovsdb-cs.h"
-#include "lib/ovsdb-error.h"
-#include "lib/ovsdb-types.h"
#include "lib/vswitch-idl.h"
#include "odp-netlink.h"
#include "openvswitch/vlog.h"
@@ -477,99 +474,17 @@ ovs_feature_get_openflow_cap(void)
return ret;
}
-/* Monitoring how many prefixes flow tables can have.
- * TODO: Remove this code once the server column type is available via IDL. */
-static struct ovsdb_cs *vswitch_cs;
-static size_t max_flow_table_prefixes;
-
-static struct json *
-vswitch_schema_prefixes_find(const struct json *schema_json,
- void *ctx OVS_UNUSED)
-{
- /* We must return a monitor request object, but we do not actually want
- * to monitor the data, so returning an empty one. */
- struct json *monitor_request = json_object_create();
- struct ovsdb_idl_column *prefixes_col;
-
- prefixes_col = &ovsrec_flow_table_columns[OVSREC_FLOW_TABLE_COL_PREFIXES];
-
- if (schema_json->type != JSON_OBJECT) {
- VLOG_WARN_RL(&rl, "OVS database schema is not a JSON object");
- return monitor_request;
- }
-
- const char *ft_name = ovsrec_table_classes[OVSREC_TABLE_FLOW_TABLE].name;
- const char *nested_objects[] = {
- "tables", ft_name, "columns", prefixes_col->name, "type",
- };
- const struct json *nested = schema_json;
-
- for (size_t i = 0; i < ARRAY_SIZE(nested_objects); i++) {
- nested = shash_find_data(json_object(nested), nested_objects[i]);
- if (!nested || nested->type != JSON_OBJECT) {
- VLOG_WARN_RL(&rl, "OVS database schema has no valid '%s'.",
- nested_objects[i]);
- return monitor_request;
- }
- }
-
- struct ovsdb_type server_type;
- struct ovsdb_error *error;
-
- error = ovsdb_type_from_json(&server_type, nested);
- if (error) {
- char *msg = ovsdb_error_to_string_free(error);
-
- VLOG_WARN_RL(&rl, "Failed to parse type of %s column in %s table: %s",
- prefixes_col->name, ft_name, msg);
- free(msg);
- return monitor_request;
- }
-
- if (max_flow_table_prefixes != server_type.n_max) {
- VLOG_INFO_RL(&rl, "OVS DB schema supports %d flow table prefixes, "
- "our IDL supports: %d",
- server_type.n_max, prefixes_col->type.n_max);
- max_flow_table_prefixes = server_type.n_max;
- }
-
- return monitor_request;
-}
-
-static struct ovsdb_cs_ops feature_cs_ops = {
- .compose_monitor_requests = vswitch_schema_prefixes_find,
-};
-
-static void
-vswitch_schema_prefixes_run(void)
-{
- struct ovsdb_cs_event *event;
- struct ovs_list events;
-
- ovsdb_cs_run(vswitch_cs, &events);
- LIST_FOR_EACH_POP (event, list_node, &events) {
- /* We're not really interested in events. We only care about the
- * monitor request callback being invoked on re-connections or
- * schema changes. */
- ovsdb_cs_event_destroy(event);
- }
- ovsdb_cs_wait(vswitch_cs);
-}
-
void
ovs_feature_support_destroy(void)
{
rconn_destroy(swconn);
swconn = NULL;
- ovsdb_cs_destroy(vswitch_cs);
- vswitch_cs = NULL;
}
/* Returns 'true' if the set of tracked OVS features has been updated. */
bool
ovs_feature_support_run(const struct smap *ovs_capabilities,
- const char *conn_target, int probe_interval,
- const char *db_target)
+ const char *conn_target, int probe_interval)
{
static struct smap empty_caps = SMAP_INITIALIZER(&empty_caps);
@@ -591,15 +506,6 @@ ovs_feature_support_run(const struct smap *ovs_capabilities,
feature->name);
}
- if (!vswitch_cs) {
- vswitch_cs = ovsdb_cs_create(ovsrec_idl_class.database, 3,
- &feature_cs_ops, NULL);
- }
- ovsdb_cs_set_remote(vswitch_cs, db_target, true);
- /* This feature doesn't affect OpenFlow rules, so not touching 'updated'
- * even if changed. */
- vswitch_schema_prefixes_run();
-
return updated;
}
@@ -630,11 +536,3 @@ ovs_feature_max_select_groups_get(void)
{
return ovs_group_features.max_groups[OFPGT11_SELECT];
}
-
-/* Returns the number of flow table prefixes that can be configured in
- * the OVS database. */
-size_t
-ovs_features_max_flow_table_prefixes_get(void)
-{
- return max_flow_table_prefixes;
-}
diff --git a/lib/test-ovn-features.c b/lib/test-ovn-features.c
index 40fba6a43..cddeae779 100644
--- a/lib/test-ovn-features.c
+++ b/lib/test-ovn-features.c
@@ -26,15 +26,15 @@ test_ovn_features(struct ovs_cmdl_context *ctx OVS_UNUSED)
struct smap features = SMAP_INITIALIZER(&features);
smap_add(&features, "ct_zero_snat", "false");
- ovs_assert(!ovs_feature_support_run(&features, NULL, 0, NULL));
+ ovs_assert(!ovs_feature_support_run(&features, NULL, 0));
ovs_assert(!ovs_feature_is_supported(OVS_CT_ZERO_SNAT_SUPPORT));
smap_replace(&features, "ct_zero_snat", "true");
- ovs_assert(ovs_feature_support_run(&features, NULL, 0, NULL));
+ ovs_assert(ovs_feature_support_run(&features, NULL, 0));
ovs_assert(ovs_feature_is_supported(OVS_CT_ZERO_SNAT_SUPPORT));
smap_add(&features, "unknown_feature", "true");
- ovs_assert(!ovs_feature_support_run(&features, NULL, 0, NULL));
+ ovs_assert(!ovs_feature_support_run(&features, NULL, 0));
smap_destroy(&features);
}
diff --git a/ovs b/ovs
index d28382947..0d5eece55 160000
--- a/ovs
+++ b/ovs
@@ -1 +1 @@
-Subproject commit d2838294774aa2457acff3aa3f372054719f73f3
+Subproject commit 0d5eece55c5a0d9fa82edeb0aafbf1003e1ebbff
