This reverts commit 482af6847f9edbcc8834ab06a7d2d14c1dbe252c.
Reported-at:
ihttps://mail.openvswitch.org/pipermail/ovs-dev/2025-October/426727.html
Signed-off-by: Ales Musil <[email protected]>
---
northd/en-lflow.c | 2 +-
northd/lflow-mgr.c | 108 ++++++++++++++++++++------------------------
northd/lflow-mgr.h | 10 +---
northd/northd.c | 8 ++--
tests/ovn-northd.at | 32 -------------
5 files changed, 54 insertions(+), 106 deletions(-)
diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index 84507f9e4..e80fd9f9c 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -122,7 +122,7 @@ en_lflow_run(struct engine_node *node, void *data)
stopwatch_start(BUILD_LFLOWS_STOPWATCH_NAME, time_msec());
struct lflow_data *lflow_data = data;
- lflow_table_clear(lflow_data->lflow_table, false);
+ lflow_table_clear(lflow_data->lflow_table);
lflow_reset_northd_refs(&lflow_input);
lflow_ref_clear(lflow_input.igmp_lflow_ref);
diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c
index 5a989fe19..6a66a9718 100644
--- a/northd/lflow-mgr.c
+++ b/northd/lflow-mgr.c
@@ -26,7 +26,6 @@
#include "lflow-mgr.h"
#include "lib/ovn-parallel-hmap.h"
#include "lib/ovn-util.h"
-#include "lib/uuidset.h"
VLOG_DEFINE_THIS_MODULE(lflow_mgr);
@@ -38,8 +37,7 @@ static void ovn_lflow_init(struct ovn_lflow *, struct
ovn_datapath *od,
uint16_t priority, char *match,
char *actions, char *io_port,
char *ctrl_meter, char *stage_hint,
- const char *where, const char *flow_desc,
- struct uuid sbuuid);
+ const char *where, const char *flow_desc);
static struct ovn_lflow *ovn_lflow_find(const struct hmap *lflows,
enum ovn_stage stage,
uint16_t priority, const char *match,
@@ -149,13 +147,6 @@ static struct ovs_mutex
lflow_hash_locks[LFLOW_HASH_LOCK_MASK + 1];
*/
extern struct ovs_mutex fake_hash_mutex;
-
-enum ovn_lflow_state {
- LFLOW_STALE,
- LFLOW_TO_SYNC,
- LFLOW_SYNCED,
-};
-
/* Represents a logical ovn flow (lflow).
*
* A logical flow with match 'M' and actions 'A' - L(M, A) is created
@@ -190,7 +181,14 @@ struct ovn_lflow {
struct hmap dp_refcnts_map; /* Maintains the number of times this ovn_lflow
* is referenced by a given datapath.
* Contains 'struct dp_refcnt' in the map. */
- enum ovn_lflow_state sync_state;
+};
+
+/* Logical flow table. */
+struct lflow_table {
+ struct hmap entries; /* hmap of lflows. */
+ struct hmap ls_dp_groups; /* hmap of logical switch dp groups. */
+ struct hmap lr_dp_groups; /* hmap of logical router dp groups. */
+ ssize_t max_seen_lflow_size;
};
struct lflow_table *
@@ -212,14 +210,10 @@ lflow_table_init(struct lflow_table *lflow_table)
}
void
-lflow_table_clear(struct lflow_table *lflow_table, bool destroy_all)
+lflow_table_clear(struct lflow_table *lflow_table)
{
struct ovn_lflow *lflow;
HMAP_FOR_EACH_SAFE (lflow, hmap_node, &lflow_table->entries) {
- if (!destroy_all) {
- lflow->sync_state = LFLOW_STALE;
- continue;
- }
ovn_lflow_destroy(lflow_table, lflow);
}
@@ -230,7 +224,7 @@ lflow_table_clear(struct lflow_table *lflow_table, bool
destroy_all)
void
lflow_table_destroy(struct lflow_table *lflow_table)
{
- lflow_table_clear(lflow_table, true);
+ lflow_table_clear(lflow_table);
hmap_destroy(&lflow_table->entries);
ovn_dp_groups_destroy(&lflow_table->ls_dp_groups);
ovn_dp_groups_destroy(&lflow_table->lr_dp_groups);
@@ -263,42 +257,16 @@ lflow_table_sync_to_sb(struct lflow_table *lflow_table,
const struct sbrec_logical_flow_table *sb_flow_table,
const struct sbrec_logical_dp_group_table *dpgrp_table)
{
- struct uuidset sb_uuid_set = UUIDSET_INITIALIZER(&sb_uuid_set);
struct hmap lflows_temp = HMAP_INITIALIZER(&lflows_temp);
struct hmap *lflows = &lflow_table->entries;
struct ovn_lflow *lflow;
- const struct sbrec_logical_flow *sbflow;
fast_hmap_size_for(&lflows_temp,
lflow_table->max_seen_lflow_size);
- HMAP_FOR_EACH_SAFE (lflow, hmap_node, lflows) {
- if (lflow->sync_state == LFLOW_STALE) {
- ovn_lflow_destroy(lflow_table, lflow);
- continue;
- }
- sbflow = NULL;
- if (!uuid_is_zero(&lflow->sb_uuid)) {
- sbflow = sbrec_logical_flow_table_get_for_uuid(sb_flow_table,
- &lflow->sb_uuid);
- }
- sync_lflow_to_sb(lflow, ovnsb_txn, lflow_table, ls_datapaths,
- lr_datapaths, ovn_internal_version_changed,
- sbflow, dpgrp_table);
- uuidset_insert(&sb_uuid_set, &lflow->sb_uuid);
- hmap_remove(lflows, &lflow->hmap_node);
- hmap_insert(&lflows_temp, &lflow->hmap_node,
- hmap_node_hash(&lflow->hmap_node));
- }
/* Push changes to the Logical_Flow table to database. */
+ const struct sbrec_logical_flow *sbflow;
SBREC_LOGICAL_FLOW_TABLE_FOR_EACH_SAFE (sbflow, sb_flow_table) {
- struct uuidset_node *node = uuidset_find(&sb_uuid_set,
- &sbflow->header_.uuid);
- if (!node) {
- sbrec_logical_flow_delete(sbflow);
- continue;
- }
- uuidset_delete(&sb_uuid_set, node);
struct sbrec_logical_dp_group *dp_group = sbflow->logical_dp_group;
struct ovn_datapath *logical_datapath_od = NULL;
size_t i;
@@ -329,8 +297,38 @@ lflow_table_sync_to_sb(struct lflow_table *lflow_table,
sbrec_logical_flow_delete(sbflow);
continue;
}
+
+ enum ovn_pipeline pipeline
+ = !strcmp(sbflow->pipeline, "ingress") ? P_IN : P_OUT;
+
+ lflow = ovn_lflow_find(
+ lflows,
+ ovn_stage_build(ovn_datapath_get_type(logical_datapath_od),
+ pipeline, sbflow->table_id),
+ sbflow->priority, sbflow->match, sbflow->actions,
+ sbflow->controller_meter, sbflow->hash);
+ if (lflow) {
+ sync_lflow_to_sb(lflow, ovnsb_txn, lflow_table, ls_datapaths,
+ lr_datapaths, ovn_internal_version_changed,
+ sbflow, dpgrp_table);
+
+ hmap_remove(lflows, &lflow->hmap_node);
+ hmap_insert(&lflows_temp, &lflow->hmap_node,
+ hmap_node_hash(&lflow->hmap_node));
+ } else {
+ sbrec_logical_flow_delete(sbflow);
+ }
+ }
+
+ HMAP_FOR_EACH_SAFE (lflow, hmap_node, lflows) {
+ sync_lflow_to_sb(lflow, ovnsb_txn, lflow_table, ls_datapaths,
+ lr_datapaths, ovn_internal_version_changed,
+ NULL, dpgrp_table);
+
+ hmap_remove(lflows, &lflow->hmap_node);
+ hmap_insert(&lflows_temp, &lflow->hmap_node,
+ hmap_node_hash(&lflow->hmap_node));
}
- uuidset_destroy(&sb_uuid_set);
hmap_swap(lflows, &lflows_temp);
hmap_destroy(&lflows_temp);
}
@@ -849,7 +847,7 @@ ovn_lflow_init(struct ovn_lflow *lflow, struct ovn_datapath
*od,
size_t dp_bitmap_len, enum ovn_stage stage, uint16_t priority,
char *match, char *actions, char *io_port, char *ctrl_meter,
char *stage_hint, const char *where,
- const char *flow_desc, struct uuid sbuuid)
+ const char *flow_desc)
{
lflow->dpg_bitmap = bitmap_allocate(dp_bitmap_len);
lflow->od = od;
@@ -863,8 +861,7 @@ ovn_lflow_init(struct ovn_lflow *lflow, struct ovn_datapath
*od,
lflow->flow_desc = flow_desc;
lflow->dpg = NULL;
lflow->where = where;
- lflow->sb_uuid = sbuuid;
- lflow->sync_state = LFLOW_TO_SYNC;
+ lflow->sb_uuid = UUID_ZERO;
hmap_init(&lflow->dp_refcnts_map);
ovs_list_init(&lflow->referenced_by);
}
@@ -960,18 +957,13 @@ do_ovn_lflow_add(struct lflow_table *lflow_table, size_t
dp_bitmap_len,
{
struct ovn_lflow *old_lflow;
struct ovn_lflow *lflow;
- struct uuid sbuuid = UUID_ZERO;
ovs_assert(dp_bitmap_len);
old_lflow = ovn_lflow_find(&lflow_table->entries, stage,
priority, match, actions, ctrl_meter, hash);
if (old_lflow) {
- if (old_lflow->sync_state != LFLOW_STALE) {
- return old_lflow;
- }
- sbuuid = old_lflow->sb_uuid;
- ovn_lflow_destroy(lflow_table, old_lflow);
+ return old_lflow;
}
lflow = xzalloc(sizeof *lflow);
@@ -983,16 +975,14 @@ do_ovn_lflow_add(struct lflow_table *lflow_table, size_t
dp_bitmap_len,
io_port ? xstrdup(io_port) : NULL,
nullable_xstrdup(ctrl_meter),
ovn_lflow_hint(stage_hint), where,
- flow_desc, sbuuid);
+ flow_desc);
if (parallelization_state != STATE_USE_PARALLELIZATION) {
hmap_insert(&lflow_table->entries, &lflow->hmap_node, hash);
} else {
hmap_insert_fast(&lflow_table->entries, &lflow->hmap_node,
hash);
- if (uuid_is_zero(&lflow->sb_uuid)) {
- thread_lflow_counter++;
- }
+ thread_lflow_counter++;
}
return lflow;
@@ -1160,7 +1150,6 @@ sync_lflow_to_sb(struct ovn_lflow *lflow,
"referencing the dp group ["UUID_FMT"]",
UUID_ARGS(&sbflow->header_.uuid),
UUID_ARGS(&lflow->dpg->dpg_uuid));
- lflow->sync_state = LFLOW_STALE;
return false;
}
} else {
@@ -1180,7 +1169,6 @@ sync_lflow_to_sb(struct ovn_lflow *lflow,
ovn_dp_group_release(dp_groups, pre_sync_dpg);
}
- lflow->sync_state = LFLOW_SYNCED;
return true;
}
diff --git a/northd/lflow-mgr.h b/northd/lflow-mgr.h
index 91708c8a9..1521270d6 100644
--- a/northd/lflow-mgr.h
+++ b/northd/lflow-mgr.h
@@ -26,16 +26,10 @@ struct ovn_datapath;
struct ovsdb_idl_row;
/* lflow map which stores the logical flows. */
-struct lflow_table {
- struct hmap entries; /* hmap of lflows. */
- struct hmap ls_dp_groups; /* hmap of logical switch dp groups. */
- struct hmap lr_dp_groups; /* hmap of logical router dp groups. */
- ssize_t max_seen_lflow_size;
-};
-
+struct lflow_table;
struct lflow_table *lflow_table_alloc(void);
void lflow_table_init(struct lflow_table *);
-void lflow_table_clear(struct lflow_table *, bool);
+void lflow_table_clear(struct lflow_table *);
void lflow_table_destroy(struct lflow_table *);
void lflow_table_expand(struct lflow_table *);
void lflow_table_set_size(struct lflow_table *, size_t);
diff --git a/northd/northd.c b/northd/northd.c
index 8efbf4553..4a3463a8e 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -18850,9 +18850,9 @@ noop_callback(struct worker_pool *pool OVS_UNUSED,
static void
fix_flow_table_size(struct lflow_table *lflow_table,
struct lswitch_flow_build_info *lsiv,
- size_t n_lsiv, size_t start)
+ size_t n_lsiv)
{
- size_t total = start;
+ size_t total = 0;
for (size_t i = 0; i < n_lsiv; i++) {
total += lsiv[i].thread_lflow_counter;
}
@@ -18927,10 +18927,8 @@ build_lswitch_and_lrouter_flows(
}
/* Run thread pool. */
- size_t current_lflow_table_size = hmap_count(&lflows->entries);
run_pool_callback(build_lflows_pool, NULL, NULL, noop_callback);
- fix_flow_table_size(lflows, lsiv, build_lflows_pool->size,
- current_lflow_table_size);
+ fix_flow_table_size(lflows, lsiv, build_lflows_pool->size);
for (index = 0; index < build_lflows_pool->size; index++) {
ds_destroy(&lsiv[index].match);
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index cded6dcd7..b1aee0008 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -12585,38 +12585,6 @@ AT_CHECK([echo $dpgrp_dps | grep $sw3_uuid], [0],
[ignore])
AT_CHECK([echo $dpgrp_dps | grep $sw4_uuid], [0], [ignore])
AT_CHECK([echo $dpgrp_dps | grep $sw5_uuid], [0], [ignore])
-# Clear the SB:Logical_Flow.logical_dp_groups column of all the
-# logical flows and then modify the NB:Load_balancer. ovn-northd
-# should resync the logical flows.
-for l in $(ovn-sbctl --bare --columns _uuid list logical_flow)
-do
- check ovn-sbctl clear logical_flow $l logical_dp_group
-done
-
-check ovn-nbctl --wait=sb set load_balancer lb2
vips='{"10.0.0.10:80"="10.0.0.3:80,10.0.0.4:80"}'
-check_engine_stats lflow recompute compute
-CHECK_NO_CHANGE_AFTER_RECOMPUTE
-
-lb_lflow_uuid=$(fetch_column Logical_flow _uuid match='"ct.new && ip4.dst ==
10.0.0.10 && reg1[[16..23]] == 6 && reg1[[0..15]] == 80"')
-AT_CHECK([test "$lb_lflow_uuid" != ""])
-
-lb_lflow_dp=$(ovn-sbctl --bare --columns logical_datapath list logical_flow
$lb_lflow_uuid)
-AT_CHECK([test "$lb_lflow_dp" = ""])
-
-lb_lflow_dpgrp=$(ovn-sbctl --bare --columns logical_dp_group list logical_flow
$lb_lflow_uuid)
-AT_CHECK([test "$lb_lflow_dpgrp" != ""])
-
-dpgrp_dps=$(ovn-sbctl --bare --columns datapaths list logical_dp_group
$lb_lflow_dpgrp)
-
-echo "dpgrp dps - $dpgrp_dps"
-
-AT_CHECK([echo $dpgrp_dps | grep $sw0_uuid], [1], [ignore])
-AT_CHECK([echo $dpgrp_dps | grep $sw1_uuid], [1], [ignore])
-AT_CHECK([echo $dpgrp_dps | grep $sw2_uuid], [1], [ignore])
-AT_CHECK([echo $dpgrp_dps | grep $sw3_uuid], [0], [ignore])
-AT_CHECK([echo $dpgrp_dps | grep $sw4_uuid], [0], [ignore])
-AT_CHECK([echo $dpgrp_dps | grep $sw5_uuid], [0], [ignore])
-
AT_CLEANUP
])
--
2.51.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
