The ovnsb_db_run() handles changes from SB DB and then updates NB/SB
accordingly. These changes are not directly related to the rest part of
the "northd" node, so split as a new node from it. This node doesn't
maintain any data, but simply output to NB/SB DBs. It is similar to the
sync-to-sb node but the input is from the other direction. This enables
handling SB changes in different (more efficient) ways from each node.
Signed-off-by: Han Zhou <hz...@ovn.org>
---
northd/automake.mk | 2 +
northd/en-northd.c | 12 ++++--
northd/en-sync-from-sb.c | 86 ++++++++++++++++++++++++++++++++++++++++
northd/en-sync-from-sb.h | 11 +++++
northd/inc-proc-northd.c | 9 +++++
northd/northd.c | 67 +++++++++++++------------------
northd/northd.h | 14 +++++--
7 files changed, 154 insertions(+), 47 deletions(-)
create mode 100644 northd/en-sync-from-sb.c
create mode 100644 northd/en-sync-from-sb.h
diff --git a/northd/automake.mk b/northd/automake.mk
index 3c3719a24936..f8e61a1683e8 100644
--- a/northd/automake.mk
+++ b/northd/automake.mk
@@ -16,6 +16,8 @@ northd_ovn_northd_SOURCES = \
northd/en-northd-output.h \
northd/en-sync-sb.c \
northd/en-sync-sb.h \
+ northd/en-sync-from-sb.c \
+ northd/en-sync-from-sb.h \
northd/inc-proc-northd.c \
northd/inc-proc-northd.h \
northd/ipam.c \
diff --git a/northd/en-northd.c b/northd/en-northd.c
index f2bf98f774b1..785a592516c0 100644
--- a/northd/en-northd.c
+++ b/northd/en-northd.c
@@ -18,6 +18,7 @@
#include <stdlib.h>
#include <stdio.h>
+#include "coverage.h"
#include "en-northd.h"
#include "lib/inc-proc-eng.h"
#include "lib/ovn-nb-idl.h"
@@ -25,11 +26,14 @@
* lib/ovn-parallel-hmap.h should be updated
* to include this dependency itself */
#include "lib/ovn-parallel-hmap.h"
+#include "stopwatch.h"
+#include "lib/stopwatch-names.h"
#include "northd.h"
#include "lib/util.h"
#include "openvswitch/vlog.h"
VLOG_DEFINE_THIS_MODULE(en_northd);
+COVERAGE_DEFINE(northd_run);
static void
northd_get_input_data(struct engine_node *node,
@@ -128,9 +132,11 @@ en_northd_run(struct engine_node *node, void *data)
northd_init(data);
northd_get_input_data(node, &input_data);
- northd_run(&input_data, data,
- eng_ctx->ovnnb_idl_txn,
- eng_ctx->ovnsb_idl_txn);
+ COVERAGE_INC(northd_run);
+ stopwatch_start(OVNNB_DB_RUN_STOPWATCH_NAME, time_msec());
+ ovnnb_db_run(&input_data, data, eng_ctx->ovnnb_idl_txn,
+ eng_ctx->ovnsb_idl_txn);
+ stopwatch_stop(OVNNB_DB_RUN_STOPWATCH_NAME, time_msec());
engine_set_node_state(node, EN_UPDATED);
}
diff --git a/northd/en-sync-from-sb.c b/northd/en-sync-from-sb.c
new file mode 100644
index 000000000000..55ece2d1627c
--- /dev/null
+++ b/northd/en-sync-from-sb.c
@@ -0,0 +1,86 @@
+/*
+ * 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 <getopt.h>
+#include <stdlib.h>
+#include <stdio.h>
+
+#include "openvswitch/util.h"
+
+#include "en-sync-from-sb.h"
+#include "include/ovn/expr.h"
+#include "lib/inc-proc-eng.h"
+#include "lib/lb.h"
+#include "lib/ovn-nb-idl.h"
+#include "lib/ovn-sb-idl.h"
+#include "lib/ovn-util.h"
+#include "stopwatch.h"
+#include "lib/stopwatch-names.h"
+#include "timeval.h"
+#include "northd.h"
+
+#include "openvswitch/vlog.h"
+
+VLOG_DEFINE_THIS_MODULE(en_sync_from_sb);
+
+void *
+en_sync_from_sb_init(struct engine_node *node OVS_UNUSED,
+ struct engine_arg *arg OVS_UNUSED)
+{
+ return NULL;
+}
+
+void
+en_sync_from_sb_run(struct engine_node *node, void *data OVS_UNUSED)
+{
+ const struct engine_context *eng_ctx = engine_get_context();
+ struct northd_data *nd = engine_get_input_data("northd", node);
+
+ const struct sbrec_port_binding_table *sb_pb_table =
+ EN_OVSDB_GET(engine_get_input("SB_port_binding", node));
+ const struct sbrec_ha_chassis_group_table *sb_ha_ch_grp_table =
+ EN_OVSDB_GET(engine_get_input("SB_ha_chassis_group", node));
+ struct ovsdb_idl_index *sb_ha_ch_grp_by_name =
+ engine_ovsdb_node_get_index(
+ engine_get_input("SB_ha_chassis_group", node),
+ "sbrec_ha_chassis_grp_by_name");
+ stopwatch_start(OVNSB_DB_RUN_STOPWATCH_NAME, time_msec());
+ ovnsb_db_run(eng_ctx->ovnnb_idl_txn, eng_ctx->ovnsb_idl_txn,
+ sb_pb_table, sb_ha_ch_grp_table, sb_ha_ch_grp_by_name,
+ &nd->ls_ports);
+ stopwatch_stop(OVNSB_DB_RUN_STOPWATCH_NAME, time_msec());
+}
+
+bool
+sync_from_sb_northd_handler(struct engine_node *node,
+ void *data OVS_UNUSED)
+{
+ struct northd_data *nd = engine_get_input_data("northd", node);
+ if (nd->change_tracked) {
+ /* There are only NB LSP related changes and the only field this node
+ * cares about is the "up" column, which is considered write-only to
+ * this node, so it is safe to ignore the change. (The real change
+ * matters to this node is always from the SB DB.) */
+ return true;
+ }
+ return false;
+}
+
+void
+en_sync_from_sb_cleanup(void *data OVS_UNUSED)
+{
+
+}
diff --git a/northd/en-sync-from-sb.h b/northd/en-sync-from-sb.h
new file mode 100644
index 000000000000..1f0cfd2cdbc2
--- /dev/null
+++ b/northd/en-sync-from-sb.h
@@ -0,0 +1,11 @@
+#ifndef EN_SYNC_FROM_SB_H
+#define EN_SYNC_FROM_SB_H 1
+
+#include "lib/inc-proc-eng.h"
+
+void *en_sync_from_sb_init(struct engine_node *, struct engine_arg *);
+void en_sync_from_sb_run(struct engine_node *, void *data);
+void en_sync_from_sb_cleanup(void *data);
+bool sync_from_sb_northd_handler(struct engine_node *, void *data OVS_UNUSED);
+
+#endif /* end of EN_SYNC_FROM_SB_H */
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index f6ceb8280624..0642c9514b7c 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -34,6 +34,7 @@
#include "en-lflow.h"
#include "en-northd-output.h"
#include "en-sync-sb.h"
+#include "en-sync-from-sb.h"
#include "unixctl.h"
#include "util.h"
@@ -129,6 +130,7 @@ enum sb_engine_node {
/* Define engine nodes for other nodes. They should be defined as static to
* avoid sparse errors. */
static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(northd, "northd");
+static ENGINE_NODE(sync_from_sb, "sync_from_sb");
static ENGINE_NODE(lflow, "lflow");
static ENGINE_NODE(mac_binding_aging, "mac_binding_aging");
static ENGINE_NODE(mac_binding_aging_waker, "mac_binding_aging_waker");
@@ -196,6 +198,13 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
* Right now this engine only syncs the SB Address_Set table.
*/
engine_add_input(&en_sync_to_sb, &en_sync_to_sb_addr_set, NULL);
+
+ engine_add_input(&en_sync_from_sb, &en_northd,
+ sync_from_sb_northd_handler);
+ engine_add_input(&en_sync_from_sb, &en_sb_port_binding, NULL);
+ engine_add_input(&en_sync_from_sb, &en_sb_ha_chassis_group, NULL);
+
+ engine_add_input(&en_northd_output, &en_sync_from_sb, NULL);
engine_add_input(&en_northd_output, &en_sync_to_sb,
northd_output_sync_to_sb_handler);
engine_add_input(&en_northd_output, &en_lflow,
diff --git a/northd/northd.c b/northd/northd.c
index 32c18ab3c932..f65b69848a09 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -60,8 +60,6 @@
VLOG_DEFINE_THIS_MODULE(northd);
-COVERAGE_DEFINE(northd_run);
-
static bool controller_event_en;
static bool lflow_hash_lock_initialized = false;
@@ -17116,7 +17114,7 @@ northd_destroy(struct northd_data *data)
sset_destroy(&data->svc_monitor_lsps);
}
-static void
+void
ovnnb_db_run(struct northd_input *input_data,
struct northd_data *data,
struct ovsdb_idl_txn *ovnnb_txn,
@@ -17331,16 +17329,16 @@ struct ha_chassis_group_node {
};
static void
-update_sb_ha_group_ref_chassis(struct northd_input *input_data,
- struct shash *ha_ref_chassis_map)
+update_sb_ha_group_ref_chassis(
+ const struct sbrec_ha_chassis_group_table *sb_ha_ch_grp_table,
+ struct shash *ha_ref_chassis_map)
{
struct hmap ha_ch_grps = HMAP_INITIALIZER(&ha_ch_grps);
struct ha_chassis_group_node *ha_ch_grp_node;
/* Initialize a set of all ha_chassis_groups in SB. */
const struct sbrec_ha_chassis_group *ha_ch_grp;
- SBREC_HA_CHASSIS_GROUP_TABLE_FOR_EACH (ha_ch_grp,
- input_data->sbrec_ha_chassis_group_table) {
+ SBREC_HA_CHASSIS_GROUP_TABLE_FOR_EACH (ha_ch_grp, sb_ha_ch_grp_table) {
ha_ch_grp_node = xzalloc(sizeof *ha_ch_grp_node);
ha_ch_grp_node->ha_ch_grp = ha_ch_grp;
hmap_insert(&ha_ch_grps, &ha_ch_grp_node->hmap_node,
@@ -17402,7 +17400,7 @@ update_sb_ha_group_ref_chassis(struct northd_input
*input_data,
* - 'ref_chassis' of hagrp1.
*/
static void
-build_ha_chassis_group_ref_chassis(struct northd_input *input_data,
+build_ha_chassis_group_ref_chassis(struct ovsdb_idl_index *ha_ch_grp_by_name,
const struct sbrec_port_binding *sb,
struct ovn_port *op,
struct shash *ha_ref_chassis_map)
@@ -17428,7 +17426,7 @@ build_ha_chassis_group_ref_chassis(struct northd_input
*input_data,
SSET_FOR_EACH (ha_group_name, &lr_group->ha_chassis_groups) {
const struct sbrec_ha_chassis_group *sb_ha_chassis_grp;
sb_ha_chassis_grp = ha_chassis_group_lookup_by_name(
- input_data->sbrec_ha_chassis_grp_by_name, ha_group_name);
+ ha_ch_grp_by_name, ha_group_name);
if (sb_ha_chassis_grp) {
struct ha_ref_chassis_info *ref_ch_info =
@@ -17443,17 +17441,18 @@ build_ha_chassis_group_ref_chassis(struct
northd_input *input_data,
* this column is not empty, it means we need to set the corresponding logical
* port as 'up' in the northbound DB. */
static void
-handle_port_binding_changes(struct northd_input *input_data,
- struct ovsdb_idl_txn *ovnsb_txn,
- struct hmap *ports,
- struct shash *ha_ref_chassis_map)
+handle_port_binding_changes(struct ovsdb_idl_txn *ovnsb_txn,
+ const struct sbrec_port_binding_table *sb_pb_table,
+ const struct sbrec_ha_chassis_group_table *sb_ha_ch_grp_table,
+ struct ovsdb_idl_index *sb_ha_ch_grp_by_name,
+ struct hmap *ls_ports,
+ struct shash *ha_ref_chassis_map)
{
const struct sbrec_port_binding *sb;
bool build_ha_chassis_ref = false;
if (ovnsb_txn) {
const struct sbrec_ha_chassis_group *ha_ch_grp;
- SBREC_HA_CHASSIS_GROUP_TABLE_FOR_EACH (ha_ch_grp,
- input_data->sbrec_ha_chassis_group_table) {
+ SBREC_HA_CHASSIS_GROUP_TABLE_FOR_EACH (ha_ch_grp, sb_ha_ch_grp_table) {
if (ha_ch_grp->n_ha_chassis > 1) {
struct ha_ref_chassis_info *ref_ch_info =
xzalloc(sizeof *ref_ch_info);
@@ -17464,9 +17463,8 @@ handle_port_binding_changes(struct northd_input
*input_data,
}
}
- SBREC_PORT_BINDING_TABLE_FOR_EACH (sb,
- input_data->sbrec_port_binding_table) {
- struct ovn_port *op = ovn_port_find(ports, sb->logical_port);
+ SBREC_PORT_BINDING_TABLE_FOR_EACH (sb, sb_pb_table) {
+ struct ovn_port *op = ovn_port_find(ls_ports, sb->logical_port);
if (!op || !op->nbsp) {
/* The logical port doesn't exist for this port binding. This can
@@ -17493,18 +17491,20 @@ handle_port_binding_changes(struct northd_input
*input_data,
if (build_ha_chassis_ref && ovnsb_txn && sb->chassis) {
/* Check and add the chassis which has claimed this 'sb'
* to the ha chassis group's ref_chassis if required. */
- build_ha_chassis_group_ref_chassis(input_data, sb, op,
+ build_ha_chassis_group_ref_chassis(sb_ha_ch_grp_by_name, sb, op,
ha_ref_chassis_map);
}
}
}
/* Handle a fairly small set of changes in the southbound database. */
-static void
-ovnsb_db_run(struct northd_input *input_data,
- struct ovsdb_idl_txn *ovnnb_txn,
+void
+ovnsb_db_run(struct ovsdb_idl_txn *ovnnb_txn,
struct ovsdb_idl_txn *ovnsb_txn,
- struct hmap *ports)
+ const struct sbrec_port_binding_table *sb_pb_table,
+ const struct sbrec_ha_chassis_group_table *sb_ha_ch_grp_table,
+ struct ovsdb_idl_index *sb_ha_ch_grp_by_name,
+ struct hmap *ls_ports)
{
if (!ovnnb_txn ||
!ovsdb_idl_has_ever_connected(ovsdb_idl_txn_get_idl(ovnsb_txn))) {
@@ -17512,29 +17512,16 @@ ovnsb_db_run(struct northd_input *input_data,
}
struct shash ha_ref_chassis_map = SHASH_INITIALIZER(&ha_ref_chassis_map);
- handle_port_binding_changes(input_data,
- ovnsb_txn, ports, &ha_ref_chassis_map);
+ handle_port_binding_changes(ovnsb_txn, sb_pb_table, sb_ha_ch_grp_table,
+ sb_ha_ch_grp_by_name, ls_ports,
+ &ha_ref_chassis_map);
if (ovnsb_txn) {
- update_sb_ha_group_ref_chassis(input_data,
+ update_sb_ha_group_ref_chassis(sb_ha_ch_grp_table,
&ha_ref_chassis_map);
}
shash_destroy(&ha_ref_chassis_map);
}
-void northd_run(struct northd_input *input_data,
- struct northd_data *data,
- struct ovsdb_idl_txn *ovnnb_txn,
- struct ovsdb_idl_txn *ovnsb_txn)
-{
- COVERAGE_INC(northd_run);
- stopwatch_start(OVNNB_DB_RUN_STOPWATCH_NAME, time_msec());
- ovnnb_db_run(input_data, data, ovnnb_txn, ovnsb_txn);
- stopwatch_stop(OVNNB_DB_RUN_STOPWATCH_NAME, time_msec());
- stopwatch_start(OVNSB_DB_RUN_STOPWATCH_NAME, time_msec());
- ovnsb_db_run(input_data, ovnnb_txn, ovnsb_txn, &data->ls_ports);
- stopwatch_stop(OVNSB_DB_RUN_STOPWATCH_NAME, time_msec());
-}
-
const char *
northd_get_svc_monitor_mac(void)
{
diff --git a/northd/northd.h b/northd/northd.h
index 8be504811921..c77b6b3685bc 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -315,10 +315,16 @@ struct ovn_datapath {
struct hmap ports;
};
-void northd_run(struct northd_input *input_data,
- struct northd_data *data,
- struct ovsdb_idl_txn *ovnnb_txn,
- struct ovsdb_idl_txn *ovnsb_txn);
+void ovnnb_db_run(struct northd_input *input_data,
+ struct northd_data *data,
+ struct ovsdb_idl_txn *ovnnb_txn,
+ struct ovsdb_idl_txn *ovnsb_txn);
+void ovnsb_db_run(struct ovsdb_idl_txn *ovnnb_txn,
+ struct ovsdb_idl_txn *ovnsb_txn,
+ const struct sbrec_port_binding_table *,
+ const struct sbrec_ha_chassis_group_table *,
+ struct ovsdb_idl_index *sb_ha_ch_grp_by_name,
+ struct hmap *ls_ports);
bool northd_handle_ls_changes(struct ovsdb_idl_txn *,
const struct northd_input *,
struct northd_data *);
--
2.30.2
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
