In large scale deployments it's expected that there are many port
bindings. Out of these only a small fraction is usually a
localnet/l2gateway port.
Instead of iterating on all SB port bindings at every ovn-controller
iteration in patch_run()/add_bridge_mappings(), use IDL indexes to only
iterate over relevant port bindings, based on 'type'.
For example, running perf on ovn-controller on such deployments we get:
Without using IDL indexed iteration:
Children Self Command Shared Object Symbol
+ 5.79% 5.34% ovn-controller ovn-controller [.] patch_run
With IDL indexed iteration:
Children Self Command Shared Object Symbol
+ 0.55% 0.04% ovn-controller ovn-controller [.] patch_run
Reported-at: https://bugzilla.redhat.com/1938950
Signed-off-by: Dumitru Ceara <[email protected]>
---
controller/ovn-controller.c | 5 +-
controller/patch.c | 114 ++++++++++++++++++++----------------
controller/patch.h | 3 +-
3 files changed, 70 insertions(+), 52 deletions(-)
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 5dd643f52..ea0b677bd 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -2489,6 +2489,9 @@ main(int argc, char *argv[])
struct ovsdb_idl_index *sbrec_port_binding_by_datapath
= ovsdb_idl_index_create1(ovnsb_idl_loop.idl,
&sbrec_port_binding_col_datapath);
+ struct ovsdb_idl_index *sbrec_port_binding_by_type
+ = ovsdb_idl_index_create1(ovnsb_idl_loop.idl,
+ &sbrec_port_binding_col_type);
struct ovsdb_idl_index *sbrec_datapath_binding_by_key
= ovsdb_idl_index_create1(ovnsb_idl_loop.idl,
&sbrec_datapath_binding_col_tunnel_key);
@@ -2912,10 +2915,10 @@ main(int argc, char *argv[])
runtime_data = engine_get_data(&en_runtime_data);
if (runtime_data) {
patch_run(ovs_idl_txn,
+ sbrec_port_binding_by_type,
ovsrec_bridge_table_get(ovs_idl_loop.idl),
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);
pinctrl_run(ovnsb_idl_txn,
sbrec_datapath_binding_by_key,
diff --git a/controller/patch.c b/controller/patch.c
index a2a7bcd79..e54b56354 100644
--- a/controller/patch.c
+++ b/controller/patch.c
@@ -187,26 +187,24 @@ add_ovs_bridge_mappings(const struct
ovsrec_open_vswitch_table *ovs_table,
}
}
-/* Obtains external-ids:ovn-bridge-mappings from OVSDB and adds patch ports for
- * the local bridge mappings. Removes any patch ports for bridge mappings that
- * already existed from 'existing_ports'. */
static void
-add_bridge_mappings(struct ovsdb_idl_txn *ovs_idl_txn,
- const struct ovsrec_bridge_table *bridge_table,
- const struct ovsrec_open_vswitch_table *ovs_table,
- const struct sbrec_port_binding_table *port_binding_table,
- const struct ovsrec_bridge *br_int,
- struct shash *existing_ports,
- const struct sbrec_chassis *chassis,
- const struct hmap *local_datapaths)
+add_bridge_mappings_by_type(struct ovsdb_idl_txn *ovs_idl_txn,
+ struct ovsdb_idl_index *sbrec_port_binding_by_type,
+ const struct ovsrec_bridge *br_int,
+ struct shash *existing_ports,
+ const struct sbrec_chassis *chassis,
+ struct shash *bridge_mappings,
+ const char *pb_type, const char *patch_port_id,
+ const struct hmap *local_datapaths,
+ bool log_missing_bridge)
{
- /* Get ovn-bridge-mappings. */
- struct shash bridge_mappings = SHASH_INITIALIZER(&bridge_mappings);
-
- add_ovs_bridge_mappings(ovs_table, bridge_table, &bridge_mappings);
+ struct sbrec_port_binding *target =
+ sbrec_port_binding_index_init_row(sbrec_port_binding_by_type);
+ sbrec_port_binding_index_set_type(target, pb_type);
const struct sbrec_port_binding *binding;
- SBREC_PORT_BINDING_TABLE_FOR_EACH (binding, port_binding_table) {
+ SBREC_PORT_BINDING_FOR_EACH_EQUAL (binding, target,
+ sbrec_port_binding_by_type) {
/* When ovn-monitor-all is true, there can be port-bindings
* on datapaths that are not related to this chassis. Ignore them. */
if (!get_local_datapath(local_datapaths,
@@ -214,21 +212,9 @@ add_bridge_mappings(struct ovsdb_idl_txn *ovs_idl_txn,
continue;
}
- bool is_localnet = false;
- const char *patch_port_id;
- if (!strcmp(binding->type, "localnet")) {
- is_localnet = true;
- patch_port_id = "ovn-localnet-port";
- } else if (!strcmp(binding->type, "l2gateway")) {
- if (!binding->chassis
- || strcmp(chassis->name, binding->chassis->name)) {
- /* This L2 gateway port is not bound to this chassis,
- * so we should not create any patch ports for it. */
- continue;
- }
- patch_port_id = "ovn-l2gateway-port";
- } else {
- /* not a localnet or L2 gateway port. */
+ /* If needed, check if the port is bound to this chassis. */
+ const struct sbrec_chassis *bchassis = binding->chassis;
+ if (chassis && (!bchassis || strcmp(chassis->name, bchassis->name))) {
continue;
}
@@ -240,25 +226,18 @@ add_bridge_mappings(struct ovsdb_idl_txn *ovs_idl_txn,
continue;
}
char *msg_key = xasprintf("%s;%s", binding->logical_port, network);
- struct ovsrec_bridge *br_ln = shash_find_data(&bridge_mappings,
network);
+ struct ovsrec_bridge *br_ln = shash_find_data(bridge_mappings,
network);
if (!br_ln) {
- if (!is_localnet) {
+ if (log_missing_bridge) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
VLOG_ERR_RL(&rl, "bridge not found for %s port '%s' "
- "with network name '%s'",
- binding->type, binding->logical_port, network);
- } else {
- /* Since having localnet ports that are not mapped on some
- * chassis is a supported configuration used to implement
- * multisegment switches with fabric L3 routing between
- * segments, log the following message once per run but don't
- * unnecessarily pollute the log file. */
- if (!sset_contains(&missed_bridges, msg_key)) {
- VLOG_INFO("bridge not found for localnet port '%s' with "
- "network name '%s'; skipping",
- binding->logical_port, network);
- sset_add(&missed_bridges, msg_key);
- }
+ "with network name '%s'",
+ binding->type, binding->logical_port, network);
+ } else if (!sset_contains(&missed_bridges, msg_key)) {
+ VLOG_INFO("bridge not found for %s port '%s' with "
+ "network name '%s'; skipping",
+ binding->type, binding->logical_port, network);
+ sset_add(&missed_bridges, msg_key);
}
free(msg_key);
continue;
@@ -275,16 +254,51 @@ add_bridge_mappings(struct ovsdb_idl_txn *ovs_idl_txn,
free(name1);
free(name2);
}
+ sbrec_port_binding_index_destroy_row(target);
+}
+
+/* Obtains external-ids:ovn-bridge-mappings from OVSDB and adds patch ports for
+ * the local bridge mappings. Removes any patch ports for bridge mappings that
+ * already existed from 'existing_ports'. */
+static void
+add_bridge_mappings(struct ovsdb_idl_txn *ovs_idl_txn,
+ struct ovsdb_idl_index *sbrec_port_binding_by_type,
+ const struct ovsrec_bridge_table *bridge_table,
+ const struct ovsrec_open_vswitch_table *ovs_table,
+ const struct ovsrec_bridge *br_int,
+ struct shash *existing_ports,
+ const struct sbrec_chassis *chassis,
+ const struct hmap *local_datapaths)
+{
+ /* Get ovn-bridge-mappings. */
+ struct shash bridge_mappings = SHASH_INITIALIZER(&bridge_mappings);
+
+ add_ovs_bridge_mappings(ovs_table, bridge_table, &bridge_mappings);
+ add_bridge_mappings_by_type(ovs_idl_txn, sbrec_port_binding_by_type,
+ br_int, existing_ports, chassis,
+ &bridge_mappings, "l2gateway",
+ "ovn-l2gateway-port", local_datapaths, true);
+
+ /* Since having localnet ports that are not mapped on some chassis is a
+ * supported configuration used to implement multisegment switches with
+ * fabric L3 routing between segments, log the following message once per
+ * run but don't unnecessarily pollute the log file; pass
+ * 'log_missing_bridge = false'.
+ */
+ add_bridge_mappings_by_type(ovs_idl_txn, sbrec_port_binding_by_type,
+ br_int, existing_ports, NULL,
+ &bridge_mappings, "localnet",
+ "ovn-localnet-port", local_datapaths, false);
shash_destroy(&bridge_mappings);
}
void
patch_run(struct ovsdb_idl_txn *ovs_idl_txn,
+ struct ovsdb_idl_index *sbrec_port_binding_by_type,
const struct ovsrec_bridge_table *bridge_table,
const struct ovsrec_open_vswitch_table *ovs_table,
const struct ovsrec_port_table *port_table,
- 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)
@@ -313,8 +327,8 @@ patch_run(struct ovsdb_idl_txn *ovs_idl_txn,
/* Create in the database any patch ports that should exist. Remove from
* 'existing_ports' any patch ports that do exist in the database and
* should be there. */
- add_bridge_mappings(ovs_idl_txn, bridge_table, ovs_table,
- port_binding_table, br_int, &existing_ports, chassis,
+ add_bridge_mappings(ovs_idl_txn, sbrec_port_binding_by_type, bridge_table,
+ ovs_table, br_int, &existing_ports, chassis,
local_datapaths);
/* Now 'existing_ports' only still contains patch ports that exist in the
diff --git a/controller/patch.h b/controller/patch.h
index e470d502c..208b8d3ee 100644
--- a/controller/patch.h
+++ b/controller/patch.h
@@ -24,6 +24,7 @@
struct hmap;
struct ovsdb_idl_txn;
+struct ovsdb_idl_index;
struct ovsrec_bridge;
struct ovsrec_bridge_table;
struct ovsrec_open_vswitch_table;
@@ -36,10 +37,10 @@ void add_ovs_bridge_mappings(const struct
ovsrec_open_vswitch_table *ovs_table,
const struct ovsrec_bridge_table *bridge_table,
struct shash *bridge_mappings);
void patch_run(struct ovsdb_idl_txn *ovs_idl_txn,
+ struct ovsdb_idl_index *sbrec_port_binding_by_type,
const struct ovsrec_bridge_table *,
const struct ovsrec_open_vswitch_table *,
const struct ovsrec_port_table *,
- const struct sbrec_port_binding_table *,
const struct ovsrec_bridge *br_int,
const struct sbrec_chassis *,
const struct hmap *local_datapaths);
--
2.27.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
