@@ -236,6 +236,14 @@ lflow_ls_stateful_handler(struct engine_node *node, void *data)
return EN_UNHANDLED;
}
+ /* Switch datapaths created/updated in this run had their ls_stateful
+ * flows already handled by lflow_handle_northd_ls_changes(); pass northd's
+ * tracked switches so we don't reprocess them here. */
+ struct northd_data *northd_data = engine_get_input_data("northd", node);
+ const struct tracked_dps *trk_switches =
+ northd_has_lswitches_in_tracked_data(&northd_data->trk_data)
+ ? &northd_data->trk_data.trk_switches : NULL;
+
const struct engine_context *eng_ctx = engine_get_context();
struct lflow_data *lflow_data = data;
struct lflow_input lflow_input;
@@ -243,6 +251,7 @@ lflow_ls_stateful_handler(struct engine_node *node, void *data)
lflow_get_input_data(node, &lflow_input);
if (!lflow_handle_ls_stateful_changes(eng_ctx->ovnsb_idl_txn,
&ls_sful_data->trk_data,
+ trk_switches,
&lflow_input,
lflow_data->lflow_table)) {
return EN_UNHANDLED;
@@ -22620,6 +22620,7 @@ exit:
bool
lflow_handle_ls_stateful_changes(struct ovsdb_idl_txn *ovnsb_txn,
struct ls_stateful_tracked_data *trk_data,
+ const struct tracked_dps *trk_switches,
struct lflow_input *lflow_input,
struct lflow_table *lflows)
{
@@ -22633,6 +22634,15 @@ lflow_handle_ls_stateful_changes(struct ovsdb_idl_txn *ovnsb_txn,
ovs_assert(od->nbs && uuid_equals(&od->nbs->header_.uuid,
&ls_stateful_rec->nbs_uuid));
+ /* Newly created/updated switch datapaths already had their
+ * ls_stateful flows built and synced by
+ * lflow_handle_northd_ls_changes() (which processes both the by_ls
+ * and ls_stateful refs together to keep shared datapath groups
+ * stable). Skip them here to avoid rebuilding the same flows. */
+ if (trk_switches && hmapx_contains(&trk_switches->crupdated, od)) {
+ continue;
+ }
+
lflow_ref_unlink_lflows(ls_stateful_rec->lflow_ref, lflows);
/* Generate new lflows. */
@@ -22650,6 +22660,17 @@ lflow_handle_ls_stateful_changes(struct ovsdb_idl_txn *ovnsb_txn,
* 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;
+
+ /* Already synced by lflow_handle_northd_ls_changes() (see above). */
+ if (trk_switches) {
+ const struct ovn_datapath *od =
+ ovn_datapaths_find_by_index(lflow_input->ls_datapaths,
+ ls_stateful_rec->ls_index);
+ if (hmapx_contains(&trk_switches->crupdated, od)) {
+ continue;
+ }
+ }
+
/* Sync the new flows to SB. */
bool handled = lflow_ref_sync_lflows(
ls_stateful_rec->lflow_ref, lflows, ovnsb_txn,
@@ -22662,6 +22683,10 @@ lflow_handle_ls_stateful_changes(struct ovsdb_idl_txn *ovnsb_txn,
}
}
+ /* No skip is needed for deleted records: an ls_stateful record is deleted
+ * only along with its switch datapath, so lflow_handle_northd_ls_changes()
+ * has already unlinked and synced its lflow_ref, and syncing destroys the
+ * unlinked ref nodes. The resync below therefore walks an empty ref. */
HMAPX_FOR_EACH (hmapx_node, &trk_data->deleted) {
struct ls_stateful_record *ls_stateful_rec = hmapx_node->data;
@@ -1026,6 +1026,7 @@ bool lflow_handle_lr_stateful_changes(struct ovsdb_idl_txn *,
struct lflow_table *lflows);
bool lflow_handle_ls_stateful_changes(struct ovsdb_idl_txn *,
struct ls_stateful_tracked_data *,
+ const struct tracked_dps *trk_switches,
struct lflow_input *,
struct lflow_table *lflows);
bool northd_handle_sb_port_binding_changes(
@@ -16992,6 +16992,79 @@ OVN_CLEANUP_NORTHD
AT_CLEANUP
])
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([ls_stateful lflow handling for created switches])
+AT_KEYWORDS([incremental processing])
+
+ovn_start
+
+# A pre-existing switch with an ACL. It is not part of northd's tracked
+# switches, so its ls_stateful record is processed by
+# lflow_handle_ls_stateful_changes().
+check ovn-nbctl ls-add sw0
+check ovn-nbctl --wait=sb acl-add sw0 to-lport 1002 'ip4.src == 10.0.0.1' \
+ allow-related
+
+# Creating a switch together with an ACL: the switch is in northd's tracked
+# switches, so lflow_handle_northd_ls_changes() builds and syncs its
+# ls_stateful flows, and the ls_stateful handler skips the record instead of
+# building the very same flows a second time.
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+check ovn-nbctl --wait=sb ls-add sw1 \
+ -- acl-add sw1 to-lport 1002 'ip4.src == 10.0.0.2' allow-related
+check_engine_stats northd norecompute compute
+check_engine_stats ls_stateful norecompute compute
+check_engine_stats lflow norecompute compute
+AT_CHECK([ovn-sbctl dump-flows sw1 | grep ls_out_acl_eval | \
+ grep -c 'ip4.src == 10.0.0.2'], [0], [2
+])
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+
+# The skip is per switch. A single transaction that both creates a switch
+# with an ACL and adds an ACL to the pre-existing switch must skip only the
+# created one: sw0's record is still processed here.
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+check ovn-nbctl --wait=sb ls-add sw2 \
+ -- acl-add sw2 to-lport 1002 'ip4.src == 10.0.0.3' allow-related \
+ -- acl-add sw0 to-lport 1003 'ip4.src == 10.0.0.4' drop
+check_engine_stats ls_stateful norecompute compute
+check_engine_stats lflow norecompute compute
+AT_CHECK([ovn-sbctl dump-flows sw2 | grep ls_out_acl_eval | \
+ grep -c 'ip4.src == 10.0.0.3'], [0], [2
+])
+AT_CHECK([ovn-sbctl dump-flows sw0 | grep ls_out_acl_eval | \
+ grep -c 'ip4.src == 10.0.0.4'], [0], [2
+])
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+
+# An ACL change on a switch northd did not track goes through the ls_stateful
+# handler unchanged.
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+check ovn-nbctl --wait=sb acl-del sw0 to-lport 1003 'ip4.src == 10.0.0.4'
+check_engine_stats ls_stateful norecompute compute
+check_engine_stats lflow norecompute compute
+AT_CHECK([ovn-sbctl dump-flows sw0 | grep -c 'ip4.src == 10.0.0.4'], [1], [0
+])
+AT_CHECK([ovn-sbctl dump-flows sw0 | grep ls_out_acl_eval | \
+ grep -c 'ip4.src == 10.0.0.1'], [0], [2
+])
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+
+# Deleting a switch that has ACLs: its ls_stateful record is deleted along
+# with the datapath, whose lflow_ref lflow_handle_northd_ls_changes() has
+# already unlinked and synced, so the deleted-record path walks an empty ref.
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+check ovn-nbctl --wait=sb ls-del sw2
+check_engine_stats ls_stateful norecompute compute
+check_engine_stats lflow norecompute compute
+AT_CHECK([ovn-sbctl lflow-list | grep -c 'ip4.src == 10.0.0.3'], [1], [0
+])
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+
+OVN_CLEANUP_NORTHD
+AT_CLEANUP
+])
+
OVN_FOR_EACH_NORTHD_NO_HV([
AT_SETUP([Datapath count change with shared dp-group flow])
AT_KEYWORDS([incremental processing])
When a logical switch is created or updated, lflow_handle_northd_ls_changes() already builds and syncs that switch's ls_stateful flows together with its by_ls flows (so the shared datapath groups stay stable). The ls_stateful record for the same switch also shows up in the ls_stateful node's tracked "crupdated" set, so lflow_ls_stateful_handler() would rebuild and resync those exact flows a second time. Pass northd's tracked switches down to lflow_handle_ls_stateful_changes() and skip any crupdated ls_stateful record whose switch datapath was already handled by lflow_handle_northd_ls_changes(). Records for pre-existing switches (e.g. an ACL or port-group change) are untouched and still processed here. Deleted records need no such skip. An ls_stateful record is deleted only along with its switch datapath, so lflow_handle_northd_ls_changes() has already unlinked and synced its lflow_ref, and syncing destroys the ref nodes that were left unlinked. The resync done here therefore walks an already empty ref. Add a test covering the switches on both sides of the skip: a switch created together with an ACL (skipped here), an ACL change on a pre-existing switch (still processed here), a single transaction doing both at once, and the deletion of a switch that has ACLs. The transaction doing both at once is what pins the skip down to the individual switch: skipping every crupdated record instead drops the pre-existing switch's ACL flows and fails the test. Assisted-by: Claude Opus 4.8, Claude Opus 5, Claude Code Signed-off-by: Lucas Vargas Dias <lucas.vdias@magalu.cloud> --- northd/en-lflow.c | 9 ++++++ northd/northd.c | 25 ++++++++++++++++ northd/northd.h | 1 + tests/ovn-northd.at | 73 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 108 insertions(+)