For now it's still a node recompute for every port group change. It
doesn't trigger northd recompute anymore though.
A follow up commit will add incremental processing of NB.Port_Group
changes.
Signed-off-by: Dumitru Ceara <dce...@redhat.com>
---
lib/stopwatch-names.h | 1 +
northd/en-lflow.c | 4 ++-
northd/en-northd.c | 4 ---
northd/en-port-group.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++
northd/en-port-group.h | 21 +++++++++++++++++
northd/inc-proc-northd.c | 15 ++++++++++--
northd/northd.c | 9 -------
northd/northd.h | 3 --
tests/ovn-northd.at | 8 ++-----
9 files changed, 96 insertions(+), 25 deletions(-)
diff --git a/lib/stopwatch-names.h b/lib/stopwatch-names.h
index de6fca4ccc..08cb0159a7 100644
--- a/lib/stopwatch-names.h
+++ b/lib/stopwatch-names.h
@@ -30,5 +30,6 @@
#define LFLOWS_IGMP_STOPWATCH_NAME "lflows_igmp"
#define LFLOWS_DP_GROUPS_STOPWATCH_NAME "lflows_dp_groups"
#define LFLOWS_TO_SB_STOPWATCH_NAME "lflows_to_sb"
+#define PORT_GROUP_RUN_STOPWATCH_NAME "port_group_run"
#endif
diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index 7187cf959f..7f6a7872b2 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -35,6 +35,8 @@ lflow_get_input_data(struct engine_node *node,
struct lflow_input *lflow_input)
{
struct northd_data *northd_data = engine_get_input_data("northd", node);
+ struct port_group_data *pg_data =
+ engine_get_input_data("port_group", node);
lflow_input->nbrec_bfd_table =
EN_OVSDB_GET(engine_get_input("NB_bfd", node));
lflow_input->sbrec_bfd_table =
@@ -55,7 +57,7 @@ lflow_get_input_data(struct engine_node *node,
lflow_input->lr_datapaths = &northd_data->lr_datapaths;
lflow_input->ls_ports = &northd_data->ls_ports;
lflow_input->lr_ports = &northd_data->lr_ports;
- lflow_input->ls_port_groups = &northd_data->ls_port_groups;
+ lflow_input->ls_port_groups = &pg_data->ls_port_groups;
lflow_input->meter_groups = &northd_data->meter_groups;
lflow_input->lbs = &northd_data->lbs;
lflow_input->features = &northd_data->features;
diff --git a/northd/en-northd.c b/northd/en-northd.c
index 044fa70190..6fb0597144 100644
--- a/northd/en-northd.c
+++ b/northd/en-northd.c
@@ -74,8 +74,6 @@ northd_get_input_data(struct engine_node *node,
EN_OVSDB_GET(engine_get_input("NB_load_balancer", node));
input_data->nbrec_load_balancer_group_table =
EN_OVSDB_GET(engine_get_input("NB_load_balancer_group", node));
- input_data->nbrec_port_group_table =
- EN_OVSDB_GET(engine_get_input("NB_port_group", node));
input_data->nbrec_meter_table =
EN_OVSDB_GET(engine_get_input("NB_meter", node));
input_data->nbrec_acl_table =
@@ -105,8 +103,6 @@ northd_get_input_data(struct engine_node *node,
EN_OVSDB_GET(engine_get_input("SB_load_balancer", node));
input_data->sbrec_service_monitor_table =
EN_OVSDB_GET(engine_get_input("SB_service_monitor", node));
- input_data->sbrec_port_group_table =
- EN_OVSDB_GET(engine_get_input("SB_port_group", node));
input_data->sbrec_meter_table =
EN_OVSDB_GET(engine_get_input("SB_meter", node));
input_data->sbrec_dns_table =
diff --git a/northd/en-port-group.c b/northd/en-port-group.c
index b83926c351..2c36410246 100644
--- a/northd/en-port-group.c
+++ b/northd/en-port-group.c
@@ -17,8 +17,10 @@
#include <config.h>
#include "openvswitch/vlog.h"
+#include "stopwatch.h"
#include "en-port-group.h"
+#include "lib/stopwatch-names.h"
#include "northd.h"
VLOG_DEFINE_THIS_MODULE(en_port_group);
@@ -235,3 +237,57 @@ ls_port_group_record_destroy(struct ls_port_group *ls_pg,
}
}
+/* Incremental processing implementation. */
+static struct port_group_input
+port_group_get_input_data(struct engine_node *node)
+{
+ struct northd_data *northd_data = engine_get_input_data("northd", node);
+
+ return (struct port_group_input) {
+ .nbrec_port_group_table =
+ EN_OVSDB_GET(engine_get_input("NB_port_group", node)),
+ .sbrec_port_group_table =
+ EN_OVSDB_GET(engine_get_input("SB_port_group", node)),
+ .ls_ports = &northd_data->ls_ports,
+ };
+}
+
+void *
+en_port_group_init(struct engine_node *node OVS_UNUSED,
+ struct engine_arg *arg OVS_UNUSED)
+{
+ struct port_group_data *pg_data = xmalloc(sizeof *pg_data);
+
+ ls_port_group_table_init(&pg_data->ls_port_groups);
+ return pg_data;
+}
+
+void
+en_port_group_cleanup(void *data_)
+{
+ struct port_group_data *data = data_;
+
+ ls_port_group_table_destroy(&data->ls_port_groups);
+}
+
+void
+en_port_group_run(struct engine_node *node, void *data_)
+{
+ const struct engine_context *eng_ctx = engine_get_context();
+ struct port_group_input input_data = port_group_get_input_data(node);
+ struct port_group_data *data = data_;
+
+ stopwatch_start(PORT_GROUP_RUN_STOPWATCH_NAME, time_msec());
+
+ ls_port_group_table_clear(&data->ls_port_groups);
+ ls_port_group_table_build(&data->ls_port_groups,
+ input_data.nbrec_port_group_table,
+ input_data.ls_ports);
+
+ ls_port_group_table_sync(&data->ls_port_groups,
+ input_data.sbrec_port_group_table,
+ eng_ctx->ovnsb_idl_txn);
+
+ stopwatch_stop(PORT_GROUP_RUN_STOPWATCH_NAME, time_msec());
+ engine_set_node_state(node, EN_UPDATED);
+}
diff --git a/northd/en-port-group.h b/northd/en-port-group.h
index 2c8e01f51f..5cbf6c6c4a 100644
--- a/northd/en-port-group.h
+++ b/northd/en-port-group.h
@@ -60,4 +60,25 @@ void ls_port_group_table_build(struct ls_port_group_table
*ls_port_groups,
void ls_port_group_table_sync(const struct ls_port_group_table *ls_port_groups,
const struct sbrec_port_group_table *,
struct ovsdb_idl_txn *ovnsb_txn);
+
+/* Incremental processing implementation. */
+struct port_group_input {
+ /* Northbound table references. */
+ const struct nbrec_port_group_table *nbrec_port_group_table;
+
+ /* Southbound table references. */
+ const struct sbrec_port_group_table *sbrec_port_group_table;
+
+ /* northd node references. */
+ const struct hmap *ls_ports;
+};
+
+struct port_group_data {
+ struct ls_port_group_table ls_port_groups;
+};
+
+void *en_port_group_init(struct engine_node *, struct engine_arg *);
+void en_port_group_cleanup(void *data);
+void en_port_group_run(struct engine_node *, void *data);
+
#endif /* EN_PORT_GROUP_H */
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index d328deb222..6d5f9e8d16 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -137,6 +137,7 @@ static ENGINE_NODE(mac_binding_aging_waker,
"mac_binding_aging_waker");
static ENGINE_NODE(northd_output, "northd_output");
static ENGINE_NODE(sync_to_sb, "sync_to_sb");
static ENGINE_NODE(sync_to_sb_addr_set, "sync_to_sb_addr_set");
+static ENGINE_NODE(port_group, "port_group");
static ENGINE_NODE(fdb_aging, "fdb_aging");
static ENGINE_NODE(fdb_aging_waker, "fdb_aging_waker");
@@ -145,7 +146,6 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
{
/* Define relationships between nodes where first argument is dependent
* on the second argument */
- engine_add_input(&en_northd, &en_nb_port_group, NULL);
engine_add_input(&en_northd, &en_nb_load_balancer, NULL);
engine_add_input(&en_northd, &en_nb_load_balancer_group, NULL);
engine_add_input(&en_northd, &en_nb_acl, NULL);
@@ -157,7 +157,6 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
engine_add_input(&en_northd, &en_sb_sb_global, NULL);
engine_add_input(&en_northd, &en_sb_chassis, NULL);
- engine_add_input(&en_northd, &en_sb_port_group, NULL);
engine_add_input(&en_northd, &en_sb_mirror, NULL);
engine_add_input(&en_northd, &en_sb_meter, NULL);
engine_add_input(&en_northd, &en_sb_datapath_binding, NULL);
@@ -194,6 +193,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
engine_add_input(&en_lflow, &en_sb_multicast_group, NULL);
engine_add_input(&en_lflow, &en_sb_igmp_group, NULL);
engine_add_input(&en_lflow, &en_northd, lflow_northd_handler);
+ engine_add_input(&en_lflow, &en_port_group, NULL);
engine_add_input(&en_sync_to_sb_addr_set, &en_nb_address_set,
sync_to_sb_addr_set_nb_address_set_handler);
@@ -202,11 +202,20 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
engine_add_input(&en_sync_to_sb_addr_set, &en_northd, NULL);
engine_add_input(&en_sync_to_sb_addr_set, &en_sb_address_set, NULL);
+ engine_add_input(&en_port_group, &en_nb_port_group, NULL);
+ engine_add_input(&en_port_group, &en_sb_port_group, NULL);
+ /* No need for an explicit handler for northd changes. Port changes
+ * that affect port_groups trigger updates to the NB.Port_Group
+ * table too (because of the explicit dependency in the schema). */
+ engine_add_input(&en_port_group, &en_northd, engine_noop_handler);
+
/* en_sync_to_sb engine node syncs the SB database tables from
* the NB database tables.
- * Right now this engine only syncs the SB Address_Set table.
+ * Right now this engine syncs the SB Address_Set and Port_Group
+ * tables.
*/
engine_add_input(&en_sync_to_sb, &en_sync_to_sb_addr_set, NULL);
+ engine_add_input(&en_sync_to_sb, &en_port_group, NULL);
engine_add_input(&en_sync_from_sb, &en_northd,
sync_from_sb_northd_handler);
diff --git a/northd/northd.c b/northd/northd.c
index 04da75fa96..b695153805 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -17238,7 +17238,6 @@ northd_init(struct northd_data *data)
ovn_datapaths_init(&data->lr_datapaths);
hmap_init(&data->ls_ports);
hmap_init(&data->lr_ports);
- ls_port_group_table_init(&data->ls_port_groups);
shash_init(&data->meter_groups);
hmap_init(&data->lbs);
hmap_init(&data->lb_groups);
@@ -17270,8 +17269,6 @@ northd_destroy(struct northd_data *data)
}
hmap_destroy(&data->lb_groups);
- ls_port_group_table_destroy(&data->ls_port_groups);
-
struct shash_node *node;
SHASH_FOR_EACH_SAFE (node, &data->meter_groups) {
shash_delete(&data->meter_groups, node);
@@ -17410,9 +17407,6 @@ ovnnb_db_run(struct northd_input *input_data,
ods_size(&data->ls_datapaths),
ods_size(&data->lr_datapaths));
build_ipam(&data->ls_datapaths.datapaths, &data->ls_ports);
- ls_port_group_table_build(&data->ls_port_groups,
- input_data->nbrec_port_group_table,
- &data->ls_ports);
build_lrouter_groups(&data->lr_ports, &data->lr_list);
build_ip_mcast(ovnsb_txn, input_data->sbrec_ip_multicast_table,
input_data->sbrec_ip_mcast_by_dp,
@@ -17430,9 +17424,6 @@ ovnnb_db_run(struct northd_input *input_data,
sync_lbs(ovnsb_txn, input_data->sbrec_load_balancer_table,
&data->ls_datapaths, &data->lbs);
- ls_port_group_table_sync(&data->ls_port_groups,
- input_data->sbrec_port_group_table,
- ovnsb_txn);
sync_meters(ovnsb_txn, input_data->nbrec_meter_table,
input_data->nbrec_acl_table, input_data->sbrec_meter_table,
&data->meter_groups);
diff --git a/northd/northd.h b/northd/northd.h
index da93a7c6a5..ba28ec63af 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -31,7 +31,6 @@ struct northd_input {
const struct nbrec_load_balancer_table *nbrec_load_balancer_table;
const struct nbrec_load_balancer_group_table
*nbrec_load_balancer_group_table;
- const struct nbrec_port_group_table *nbrec_port_group_table;
const struct nbrec_meter_table *nbrec_meter_table;
const struct nbrec_acl_table *nbrec_acl_table;
const struct nbrec_static_mac_binding_table
@@ -50,7 +49,6 @@ struct northd_input {
const struct sbrec_fdb_table *sbrec_fdb_table;
const struct sbrec_load_balancer_table *sbrec_load_balancer_table;
const struct sbrec_service_monitor_table *sbrec_service_monitor_table;
- const struct sbrec_port_group_table *sbrec_port_group_table;
const struct sbrec_meter_table *sbrec_meter_table;
const struct sbrec_dns_table *sbrec_dns_table;
const struct sbrec_ip_multicast_table *sbrec_ip_multicast_table;
@@ -109,7 +107,6 @@ struct northd_data {
struct ovn_datapaths lr_datapaths;
struct hmap ls_ports;
struct hmap lr_ports;
- struct ls_port_group_table ls_port_groups;
struct shash meter_groups;
struct hmap lbs;
struct hmap lb_groups;
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index d5be3be75b..1a12513d7a 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -8923,11 +8923,9 @@ check as northd ovn-appctl -t NORTHD_TYPE
inc-engine/clear-stats
check ovn-nbctl add port_group pg1 ports ${p1_uuid}
wait_column '20.0.0.4' Address_Set addresses name=pg1_ip4
-# There should be recompute of the sync_to_sb_addr_set engine node since
northd engine changes.
-# There will be another recompute when the update message is received from the
sb ovsdb-server.
-# Once we add I-P for Port_Groups, there should be no recompute here.
-recompute_stat=$(as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats
sync_to_sb_addr_set recompute)
-AT_CHECK([test $recompute_stat -ge 1])
+# There should be no recompute of the sync_to_sb_addr_set engine node.
+AT_CHECK([as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats
sync_to_sb_addr_set recompute], [0], [0
+])
# No change, no recompute
check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
