There were two issues with the handler:
1) The handler was handling deleted ls_stateful, but in a wrong way.
It was trying to find ovn_datapath struct which was already
removed by northd.
2) The handler tried to sync the flows one by one which was causing
performance issues as the dp groups were created over and over
again.
None of the above was an issue because we would recompute the whole
lflow nodes in the both cases above. This was noticed with the
attempt to introduce incremental processing for LS addition.
To fix the issues above make sure we don't check the ovn_datapath for
deleted ls_stateful struct. Just resync the logical flows. Also make
a separate loop that will go over the ls_stateful updated structs
once again just to sync after all datapath groups are actually up to
date.
Finally make sure we propagate the updated correctly for newly
created ls_stateful structs.
Fixes: 00cda5a47bd4 ("northd: Add ls_stateful handler for lflow engine node.")
Fixes: 48e1736f89c6 ("northd: I-P for logical switch creation/deletion in
en_ls_stateful engine node.")
Co-authored-by: Lorenzo Bianconi <[email protected]>
Signed-off-by: Lorenzo Bianconi <[email protected]>
Signed-off-by: Ales Musil <[email protected]>
---
northd/en-ls-stateful.c | 6 ++++--
northd/northd.c | 21 +++++++++++++++------
2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/northd/en-ls-stateful.c b/northd/en-ls-stateful.c
index a9a685504..0dea24aee 100644
--- a/northd/en-ls-stateful.c
+++ b/northd/en-ls-stateful.c
@@ -161,8 +161,10 @@ ls_stateful_northd_handler(struct engine_node *node, void
*data_)
const struct ovn_datapath *od = hmapx_node->data;
if (!ls_stateful_table_find_(&data->table, od->nbs)) {
- ls_stateful_record_create(&data->table, od,
- input_data.ls_port_groups);
+ struct ls_stateful_record *ls_stateful_rec =
+ ls_stateful_record_create(&data->table, od,
+ input_data.ls_port_groups);
+ hmapx_add(&data->trk_data.crupdated, ls_stateful_rec);
}
}
diff --git a/northd/northd.c b/northd/northd.c
index 011f449ec..f5cc60e84 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -19694,7 +19694,13 @@ lflow_handle_ls_stateful_changes(struct ovsdb_idl_txn
*ovnsb_txn,
build_network_function(od, lflows,
lflow_input->ls_port_groups,
ls_stateful_rec->lflow_ref);
+ }
+ /* We need to make sure that all datapath groups are allocated before
+ * trying to sync logical flows. Otherwise, we would need to recompute
+ * those datapath groups within those flows over and over again. */
+ HMAPX_FOR_EACH (hmapx_node, &trk_data->crupdated) {
+ struct ls_stateful_record *ls_stateful_rec = hmapx_node->data;
/* Sync the new flows to SB. */
bool handled = lflow_ref_sync_lflows(
ls_stateful_rec->lflow_ref, lflows, ovnsb_txn,
@@ -19710,13 +19716,16 @@ lflow_handle_ls_stateful_changes(struct ovsdb_idl_txn
*ovnsb_txn,
HMAPX_FOR_EACH (hmapx_node, &trk_data->deleted) {
struct ls_stateful_record *ls_stateful_rec = hmapx_node->data;
- const struct ovn_datapath *od =
- ovn_datapaths_find_by_index(lflow_input->ls_datapaths,
- ls_stateful_rec->ls_index);
- ovs_assert(od->nbs && uuid_equals(&od->nbs->header_.uuid,
- &ls_stateful_rec->nbs_uuid));
- lflow_ref_unlink_lflows(ls_stateful_rec->lflow_ref);
+ if (!lflow_ref_resync_flows(
+ ls_stateful_rec->lflow_ref, lflows, ovnsb_txn,
+ lflow_input->ls_datapaths,
+ lflow_input->lr_datapaths,
+ lflow_input->ovn_internal_version_changed,
+ lflow_input->sbrec_logical_flow_table,
+ lflow_input->sbrec_logical_dp_group_table)) {
+ return false;
+ }
}
return true;
--
2.51.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
