On 6/9/25 13:35, Mark Michelson via dev wrote:
lflow_table_sync_to_sb() synced southbound logical flows based on two
criteria:
* Is the southbound logical flow's datapath or datapath group valid?
* Is there a matching flow in the lflow_table?
Upcoming commits will allow for multiple lflow_tables to be synced, so
the syncing based on datapaths only needs to happen once. Moving this
syncing to its own function allows us to avoid redundant processing of
logical flows we already know have valid datapath information.
Signed-off-by: Mark Michelson <mmich...@redhat.com>
---
northd/en-datapath-logical-router.c | 15 ++++++
northd/en-datapath-logical-router.h | 5 ++
northd/en-datapath-logical-switch.c | 15 ++++++
northd/en-datapath-logical-switch.h | 5 ++
northd/en-lflow.c | 73 +++++++++++++++++++++++++++++
northd/inc-proc-northd.c | 4 ++
northd/lflow-mgr.c | 56 ++++++++++------------
7 files changed, 143 insertions(+), 30 deletions(-)
diff --git a/northd/en-datapath-logical-router.c
b/northd/en-datapath-logical-router.c
index 1c70b32de..b8ec7073e 100644
--- a/northd/en-datapath-logical-router.c
+++ b/northd/en-datapath-logical-router.c
@@ -172,3 +172,18 @@ void en_datapath_synced_logical_router_cleanup(void *data)
struct ovn_synced_logical_router_map *router_map = data;
synced_logical_router_map_destroy(router_map);
}
+
+const struct ovn_synced_logical_router *
+ovn_synced_logical_router_find(const struct ovn_synced_logical_router_map *map,
+ const struct uuid *nb_uuid)
+{
+ uint32_t hash = uuid_hash(nb_uuid);
+ const struct ovn_synced_logical_router *router;
+ HMAP_FOR_EACH_WITH_HASH (router, hmap_node, hash, &map->synced_routers) {
+ if (uuid_equals(&router->nb->header_.uuid, nb_uuid)) {
+ return router;
+ }
+ }
+
+ return NULL;
+}
diff --git a/northd/en-datapath-logical-router.h
b/northd/en-datapath-logical-router.h
index 587de8393..eb0923f9c 100644
--- a/northd/en-datapath-logical-router.h
+++ b/northd/en-datapath-logical-router.h
@@ -46,4 +46,9 @@ enum engine_node_state en_datapath_synced_logical_router_run(
void en_datapath_synced_logical_router_cleanup(void *data);
+struct uuid;
+const struct ovn_synced_logical_router *
+ovn_synced_logical_router_find(const struct ovn_synced_logical_router_map *map,
+ const struct uuid *nb_uuid);
+
#endif /* EN_DATAPATH_LOGICAL_ROUTER_H */
diff --git a/northd/en-datapath-logical-switch.c
b/northd/en-datapath-logical-switch.c
index b667db4db..6dd2adf3c 100644
--- a/northd/en-datapath-logical-switch.c
+++ b/northd/en-datapath-logical-switch.c
@@ -170,3 +170,18 @@ void en_datapath_synced_logical_switch_cleanup(void *data)
struct ovn_synced_logical_switch_map *switch_map = data;
synced_logical_switch_map_destroy(switch_map);
}
+
+const struct ovn_synced_logical_switch *
+ovn_synced_logical_switch_find(const struct ovn_synced_logical_switch_map *map,
+ const struct uuid *nb_uuid)
+{
+ uint32_t hash = uuid_hash(nb_uuid);
+ const struct ovn_synced_logical_switch *ls;
+ HMAP_FOR_EACH_WITH_HASH (ls, hmap_node, hash, &map->synced_switches) {
+ if (uuid_equals(&ls->nb->header_.uuid, nb_uuid)) {
+ return ls;
+ }
+ }
+
+ return NULL;
+}
diff --git a/northd/en-datapath-logical-switch.h
b/northd/en-datapath-logical-switch.h
index 1190b7be8..3f00b9996 100644
--- a/northd/en-datapath-logical-switch.h
+++ b/northd/en-datapath-logical-switch.h
@@ -45,4 +45,9 @@ enum engine_node_state en_datapath_synced_logical_switch_run(
struct engine_node *, void *data);
void en_datapath_synced_logical_switch_cleanup(void *data);
+struct uuid;
+const struct ovn_synced_logical_switch *
+ovn_synced_logical_switch_find(const struct ovn_synced_logical_switch_map *map,
+ const struct uuid *nb_uuid);
+
#endif /* EN_DATAPATH_LOGICAL_SWITCH_H */
diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index 4992f7ea5..aa92755cc 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -29,6 +29,8 @@
#include "en-sampling-app.h"
#include "en-group-ecmp-route.h"
#include "lflow-mgr.h"
+#include "en-datapath-logical-switch.h"
+#include "en-datapath-logical-router.h"
#include "lib/inc-proc-eng.h"
#include "northd.h"
@@ -348,6 +350,71 @@ void en_lflow_clear_tracked_data(void *data_)
data->handled_incrementally = true;
}
+static bool
+datapath_is_valid(const struct sbrec_datapath_binding *dp,
+ const struct ovn_synced_logical_switch_map *synced_lses,
+ const struct ovn_synced_logical_router_map *synced_lrs)
+{
+ enum ovn_datapath_type dp_type = ovn_datapath_type_from_string(dp->type);
+ if (dp_type == DP_MAX) {
+ return false;
+ }
+ struct uuid nb_dp_key;
+ if (!smap_get_uuid(&dp->external_ids, "nb_uuid", &nb_dp_key)) {
+ return false;
+ }
This patch existed at one point when the datapath sync refactor was
using external-ids "nb_uuid" in the southbound Datapath_Binding. When I
rebased this series on a newer version of the datapath sync refactor, I
messed this part up, apparently. The four lines above should instead be:
struct uuid nb_dp_key;
if (!uuid_from_string(&nb_dp_key, dp->nb_uuid)) {
return false;
}
This is moot, however, since this part of the function is overwritten by
patch 12 anyway.
+ if (dp_type == DP_SWITCH) {
+ if (ovn_synced_logical_switch_find(synced_lses, &nb_dp_key)) {
+ return true;
+ } else {
+ return false;
+ }
+ } else if (dp_type == DP_ROUTER) {
+ if (ovn_synced_logical_router_find(synced_lrs, &nb_dp_key)) {
+ return true;
+ } else {
+ return false;
+ }
+ }
+
+ return false;
+}
+
+static void
+sb_lflows_sync_for_datapaths(
+ const struct ovn_synced_logical_switch_map *switches,
+ const struct ovn_synced_logical_router_map *routers,
+ struct sb_lflows *sb_lflows)
+{
+ struct sb_lflow *sb_lflow;
+ HMAP_FOR_EACH_SAFE (sb_lflow, hmap_node, &sb_lflows->valid) {
+ struct sbrec_datapath_binding *dp = sb_lflow->flow->logical_datapath;
+ if (dp) {
+ if (!datapath_is_valid(dp, switches, routers)) {
+ hmap_remove(&sb_lflows->valid, &sb_lflow->hmap_node);
+ hmap_insert(&sb_lflows->to_delete, &sb_lflow->hmap_node,
+ hmap_node_hash(&sb_lflow->hmap_node));
+ }
+ } else if (sb_lflow->flow->logical_dp_group) {
+ const struct sbrec_logical_dp_group *dp_group;
+ dp_group = sb_lflow->flow->logical_dp_group;
+ for (size_t i = 0; i < dp_group->n_datapaths; i++) {
+ dp = dp_group->datapaths[i];
+ if (datapath_is_valid(dp, switches, routers)) {
+ break;
+ }
+ dp = NULL;
+ }
+
+ }
+ if (!dp) {
+ hmap_remove(&sb_lflows->valid, &sb_lflow->hmap_node);
+ hmap_insert(&sb_lflows->to_delete, &sb_lflow->hmap_node,
+ hmap_node_hash(&sb_lflow->hmap_node));
+ }
+ }
+}
+
static void
lflow_sync_data_init(struct lflow_sync_data *lflow_sync,
const struct sbrec_logical_flow_table *sb_lflow_table)
@@ -391,6 +458,10 @@ en_lflow_sync_run(struct engine_node *node, void *data)
{
const struct sbrec_logical_flow_table *sb_lflow_table =
EN_OVSDB_GET(engine_get_input("SB_logical_flow", node));
+ const struct ovn_synced_logical_switch_map *synced_lses =
+ engine_get_input_data("datapath_synced_logical_switch", node);
+ const struct ovn_synced_logical_router_map *synced_lrs =
+ engine_get_input_data("datapath_synced_logical_router", node);
/* XXX The lflow table is currently not treated as const because it
* contains mutable logical datapath groups. A future commit will
* separate the dp groups from the lflow_table so that this can be
@@ -412,6 +483,8 @@ en_lflow_sync_run(struct engine_node *node, void *data)
stopwatch_start(LFLOWS_TO_SB_STOPWATCH_NAME, time_msec());
+ sb_lflows_sync_for_datapaths(synced_lses, synced_lrs,
+ &lflow_sync->sb_lflows);
lflow_table_sync_to_sb(lflow_data->lflow_table, eng_ctx->ovnsb_idl_txn,
&northd->ls_datapaths,
&northd->lr_datapaths,
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index 18b0ba9a5..3e6ff47b9 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -447,6 +447,10 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
engine_add_input(&en_lflow, &en_sb_acl_id, NULL);
engine_add_input(&en_lflow_sync, &en_sb_logical_flow, NULL);
+ engine_add_input(&en_lflow_sync, &en_datapath_synced_logical_router,
+ engine_noop_handler);
+ engine_add_input(&en_lflow_sync, &en_datapath_synced_logical_switch,
+ engine_noop_handler);
engine_add_input(&en_lflow_sync, &en_global_config,
node_global_config_handler);
engine_add_input(&en_lflow_sync, &en_lflow, lflow_sync_lflow_handler);
diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c
index 5973179b2..a4f061b1f 100644
--- a/northd/lflow-mgr.c
+++ b/northd/lflow-mgr.c
@@ -249,6 +249,26 @@ lflow_table_set_size(struct lflow_table *lflow_table,
size_t size)
lflow_table->entries.n = size;
}
+static enum ovn_datapath_type
+datapath_type_from_sb_lflow(const struct sbrec_logical_flow *sb_lflow)
+{
+ const char *dp_type_str = NULL;
+ if (sb_lflow->logical_datapath) {
+ dp_type_str = sb_lflow->logical_datapath->type;
+ } else if (sb_lflow->logical_dp_group) {
+ /* All datapaths in a dp_group are of the same type, so just use the
+ * first datapath in the group to determine the type.
+ */
+ dp_type_str = sb_lflow->logical_dp_group->datapaths[0]->type;
+ }
+
+ if (!dp_type_str) {
+ return DP_MAX;
+ }
+
+ return ovn_datapath_type_from_string(dp_type_str);
+}
+
void
lflow_table_sync_to_sb(struct lflow_table *lflow_table,
struct ovsdb_idl_txn *ovnsb_txn,
@@ -269,41 +289,17 @@ lflow_table_sync_to_sb(struct lflow_table *lflow_table,
struct sb_lflow *sb_lflow;
HMAP_FOR_EACH (sb_lflow, hmap_node, &sb_lflows->valid) {
const struct sbrec_logical_flow *sbflow = sb_lflow->flow;
- struct sbrec_logical_dp_group *dp_group = sbflow->logical_dp_group;
- struct ovn_datapath *logical_datapath_od = NULL;
- size_t i;
-
- /* Find one valid datapath to get the datapath type. */
- struct sbrec_datapath_binding *dp = sbflow->logical_datapath;
- if (dp) {
- logical_datapath_od = ovn_datapath_from_sbrec(
- &ls_datapaths->datapaths, &lr_datapaths->datapaths, dp);
- if (logical_datapath_od
- && ovn_datapath_is_stale(logical_datapath_od)) {
- logical_datapath_od = NULL;
- }
- }
- for (i = 0; dp_group && i < dp_group->n_datapaths; i++) {
- logical_datapath_od = ovn_datapath_from_sbrec(
- &ls_datapaths->datapaths, &lr_datapaths->datapaths,
- dp_group->datapaths[i]);
- if (logical_datapath_od
- && !ovn_datapath_is_stale(logical_datapath_od)) {
- break;
- }
- logical_datapath_od = NULL;
- }
-
- if (!logical_datapath_od) {
- /* This lflow has no valid logical datapaths. */
- continue;
- }
enum ovn_pipeline pipeline
= !strcmp(sbflow->pipeline, "ingress") ? P_IN : P_OUT;
enum ovn_datapath_type dp_type
- = ovn_datapath_get_type(logical_datapath_od);
+ = datapath_type_from_sb_lflow(sbflow);
+
+ /* Since we should have weeded out sb_lflows with invalid
+ * datapaths, we should only get valid datapath types here.
+ */
+ ovs_assert(dp_type < DP_MAX);
lflow = ovn_lflow_find(
lflows,
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
