Until now, creating or deleting a logical switch forced a full recompute
of the en_lflow engine node. Handle it incrementally instead, mirroring
the existing logical router datapath handling.
Add lflow_handle_northd_ls_changes(), invoked from lflow_northd_handler()
for the tracked switch changes: deleted switches have their datapath
flows resynced away via od->datapath_lflows, while created/updated
switches are (re)built with build_lswitch_and_lrouter_iterate_by_ls() and
synced. To make this possible the datapath-wide switch flows built there
are now anchored to od->datapath_lflows instead of being untracked (NULL
lflow_ref), so they can be generated and torn down incrementally.
Two of those flows have a different owner and are handled accordingly:
- The network function flows are moved into build_ls_stateful_flows()
so they are owned by the per-switch ls_stateful lflow_ref (which is
already processed incrementally for switch add/delete). The now
redundant build_network_function() calls in the ls_stateful and port
change handlers are removed.
- The multicast flood flow is owned by the multicast_igmp node's
lflow_ref (built in build_igmp_lflows()). multicast_igmp_northd_handler()
now recomputes that (cheap) node when switches are created/deleted so
the flood flows are added/removed for them.
Update the "Logical switch incremental processing" test: adding and
deleting a logical switch no longer recomputes the lflow node.
Assisted-bt: Claude Opus 4.8, Claude Code
Signed-off-by: Lucas Vargas Dias <[email protected]>
---
northd/en-lflow.c | 23 ++++-
northd/en-multicast.c | 9 ++
northd/northd.c | 191 ++++++++++++++++++++++++++++++++++++------
northd/northd.h | 5 ++
tests/ovn-northd.at | 4 +-
5 files changed, 201 insertions(+), 31 deletions(-)
diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index 99df5f08f..9a517ae48 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -145,16 +145,31 @@ lflow_northd_handler(struct engine_node *node,
return EN_UNHANDLED;
}
- if (northd_has_lswitches_in_tracked_data(&northd_data->trk_data)) {
- return EN_UNHANDLED;
- }
-
const struct engine_context *eng_ctx = engine_get_context();
struct lflow_data *lflow_data = data;
struct lflow_input lflow_input;
lflow_get_input_data(node, &lflow_input);
+ /* The switch datapath handler also (re)builds the per-switch ls_stateful
+ * flows of created/deleted switches, coordinated with their
+ * datapath_lflows so that shared datapath groups are updated in place
+ * rather than churned.
+ * It therefore needs the ls_stateful tracked data. */
+ struct ed_type_ls_stateful *ls_stateful_data =
+ engine_get_input_data("ls_stateful", node);
+ struct ls_stateful_tracked_data *ls_sful_trk =
+ ls_stateful_has_tracked_data(&ls_stateful_data->trk_data)
+ ? &ls_stateful_data->trk_data : NULL;
+
+ if (!lflow_handle_northd_ls_changes(eng_ctx->ovnsb_idl_txn,
+ &northd_data->trk_data.trk_switches,
+ ls_sful_trk,
+ &lflow_input,
+ lflow_data->lflow_table)) {
+ return EN_UNHANDLED;
+ }
+
if (!lflow_handle_northd_lr_changes(eng_ctx->ovnsb_idl_txn,
&northd_data->trk_data.trk_routers,
&lflow_input,
diff --git a/northd/en-multicast.c b/northd/en-multicast.c
index bfe3c4d92..c8167b479 100644
--- a/northd/en-multicast.c
+++ b/northd/en-multicast.c
@@ -153,6 +153,15 @@ multicast_igmp_northd_handler(struct engine_node *node,
void *data OVS_UNUSED)
return EN_UNHANDLED;
}
+ /* A created/deleted logical switch owns per-datapath multicast flood
+ * flows (build_mcast_flood_lswitch() via build_igmp_lflows()). The lflow
+ * node now processes switch datapaths incrementally, so it no longer
+ * forces a full recompute; recompute this (cheap) node so its lflow_ref
+ * picks up (or drops) the switch's flood flows. */
+ if (northd_has_lswitches_in_tracked_data(&northd_data->trk_data)) {
+ return EN_UNHANDLED;
+ }
+
struct tracked_ovn_ports *trk_lsps = &northd_data->trk_data.trk_lsps;
if (hmapx_count(&trk_lsps->created) ||
hmapx_count(&trk_lsps->updated) ||
diff --git a/northd/northd.c b/northd/northd.c
index eca5d6f0e..9ecc072f3 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -6962,10 +6962,11 @@ enum mirror_filter {
static void
build_mirror_default_lflow(struct ovn_datapath *od,
- struct lflow_table *lflows)
+ struct lflow_table *lflows,
+ struct lflow_ref *lflow_ref)
{
- ovn_lflow_add(lflows, od, S_SWITCH_IN_MIRROR, 0, "1", "next;", NULL);
- ovn_lflow_add(lflows, od, S_SWITCH_OUT_MIRROR, 0, "1", "next;", NULL);
+ ovn_lflow_add(lflows, od, S_SWITCH_IN_MIRROR, 0, "1", "next;", lflow_ref);
+ ovn_lflow_add(lflows, od, S_SWITCH_OUT_MIRROR, 0, "1", "next;", lflow_ref);
}
static void
@@ -19912,6 +19913,11 @@ build_lr_stateful_flows(const struct
lr_stateful_record *lr_stateful_rec,
lr_stateful_rec->lflow_ref);
}
+static void build_network_function(const struct ovn_datapath *od,
+ struct lflow_table *lflows,
+ const struct ls_port_group_table *ls_pgs,
+ struct lflow_ref *lflow_ref);
+
static void
build_ls_stateful_flows(const struct ls_stateful_record *ls_stateful_rec,
const struct ovn_datapath *od,
@@ -19943,6 +19949,12 @@ build_ls_stateful_flows(const struct
ls_stateful_record *ls_stateful_rec,
}
build_lb_hairpin(ls_stateful_rec, od, lflows, ls_stateful_rec->lflow_ref);
+
+ /* Network function flows are datapath-wide but owned by the per-switch
+ * ls_stateful lflow_ref, so that they are (re)generated and torn down
+ * together with the rest of the switch's stateful flows during
+ * incremental processing. */
+ build_network_function(od, lflows, ls_pgs, ls_stateful_rec->lflow_ref);
}
/* For packets received on tunnel and egressing towards a network-function port
@@ -20764,33 +20776,41 @@ build_lswitch_and_lrouter_iterate_by_ls(struct
ovn_datapath *od,
struct lswitch_flow_build_info *lsi)
{
ovs_assert(od->nbs);
- build_mirror_default_lflow(od, lsi->lflows);
+ build_mirror_default_lflow(od, lsi->lflows, od->datapath_lflows);
build_lswitch_lflows_pre_acl_and_acl(od, lsi->lflows,
- lsi->meter_groups, NULL);
- build_network_function(od, lsi->lflows, lsi->ls_port_groups, NULL);
- build_fwd_group_lflows(od, lsi->lflows, NULL);
- build_lswitch_lflows_admission_control(od, lsi->lflows, NULL);
- build_lswitch_learn_fdb_od(od, lsi->lflows, NULL);
+ lsi->meter_groups,
+ od->datapath_lflows);
+ build_fwd_group_lflows(od, lsi->lflows, od->datapath_lflows);
+ build_lswitch_lflows_admission_control(od, lsi->lflows,
+ od->datapath_lflows);
+ build_lswitch_learn_fdb_od(od, lsi->lflows, od->datapath_lflows);
build_lswitch_arp_nd_evpn_responder(od, lsi->lflows, lsi->meter_groups,
- NULL);
- build_lswitch_arp_nd_responder_default(od, lsi->lflows, NULL);
+ od->datapath_lflows);
+ build_lswitch_arp_nd_responder_default(od, lsi->lflows,
+ od->datapath_lflows);
build_lswitch_dns_lookup_and_response(od, lsi->lflows, lsi->meter_groups,
- NULL);
- build_lswitch_dhcp_and_dns_defaults(od, lsi->lflows, NULL);
+ od->datapath_lflows);
+ build_lswitch_dhcp_and_dns_defaults(od, lsi->lflows, od->datapath_lflows);
build_lswitch_destination_lookup_bmcast(od, lsi->lflows, &lsi->actions,
- lsi->meter_groups, NULL);
- build_lswitch_output_port_sec_od(od, lsi->lflows, NULL);
+ lsi->meter_groups,
+ od->datapath_lflows);
+ build_lswitch_output_port_sec_od(od, lsi->lflows, od->datapath_lflows);
/* CT extraction flows are built with stateful flows, but default rule is
* always needed */
ovn_lflow_add(lsi->lflows, od, S_SWITCH_IN_CT_EXTRACT, 0, "1", "next;",
- NULL);
- build_lswitch_lb_affinity_default_flows(od, lsi->lflows, NULL);
+ od->datapath_lflows);
+ build_lswitch_lb_affinity_default_flows(od, lsi->lflows,
+ od->datapath_lflows);
if (od->has_evpn_vni) {
- build_lswitch_lflows_evpn_l2_unknown(od, lsi->lflows, NULL);
+ build_lswitch_lflows_evpn_l2_unknown(od, lsi->lflows,
+ od->datapath_lflows);
} else {
- build_lswitch_lflows_l2_unknown(od, lsi->lflows, NULL);
+ build_lswitch_lflows_l2_unknown(od, lsi->lflows, od->datapath_lflows);
}
- build_mcast_flood_lswitch(od, lsi->lflows, &lsi->actions, NULL);
+ /* build_network_function() flows are owned by the per-switch ls_stateful
+ * lflow_ref (built in build_ls_stateful_flows()). The multicast flood
+ * flow (build_mcast_flood_lswitch()) is owned by the multicast_igmp
+ * node's lflow_ref (built in build_igmp_lflows()). */
}
/* Helper function to combine all lflow generation which is iterated by
@@ -21480,6 +21500,132 @@ lflow_reset_northd_refs(struct lflow_input
*lflow_input)
}
}
+bool
+lflow_handle_northd_ls_changes(struct ovsdb_idl_txn *ovnsb_txn,
+ struct tracked_dps *tracked_lses,
+ struct ls_stateful_tracked_data *ls_sful_trk,
+ struct lflow_input *lflow_input,
+ struct lflow_table *lflows)
+{
+ bool handled = true;
+ struct hmapx_node *hmapx_node;
+
+ struct lswitch_flow_build_info lsi = {
+ .ls_datapaths = lflow_input->ls_datapaths,
+ .ls_ports = lflow_input->ls_ports,
+ .ls_port_groups = lflow_input->ls_port_groups,
+ .meter_groups = lflow_input->meter_groups,
+ .features = lflow_input->features,
+ .lflows = lflows,
+ .match = DS_EMPTY_INITIALIZER,
+ .actions = DS_EMPTY_INITIALIZER,
+ };
+
+ /* A switch datapath's logical flows are split across two lflow_refs: the
+ * per-switch 'od->datapath_lflows' (built here, by_ls) and the per-switch
+ * ls_stateful lflow_ref (built by build_ls_stateful_flows()). When a
+ * switch datapath is added or removed, any datapath group shared by these
+ * flows gains or loses that datapath. To let ovn_dp_group_create() update
+ * the group's SB row in place (instead of deleting and re-creating it,
+ * which would churn the SB) the old group must be fully released before it
+ * is re-synced. That only happens if _all_ of the switch's flows -- from
+ * both refs -- are unlinked/rebuilt before _any_ of them is synced. So we
+ * do all the unlinking and building first, then sync.
+ *
+ * The ls_stateful records are also (re)processed by
+ * lflow_ls_stateful_handler(); doing it here as well is redundant but
+ * harmless (the group is already stable, so the second pass reuses it). */
+
+ /* Phase 1: unlink (and, for created/updated switches, rebuild). */
+ HMAPX_FOR_EACH (hmapx_node, &tracked_lses->deleted) {
+ struct ovn_datapath *od = hmapx_node->data;
+ lflow_ref_unlink_lflows(od->datapath_lflows);
+ }
+ if (ls_sful_trk) {
+ HMAPX_FOR_EACH (hmapx_node, &ls_sful_trk->deleted) {
+ struct ls_stateful_record *ls_stateful_rec = hmapx_node->data;
+ lflow_ref_unlink_lflows(ls_stateful_rec->lflow_ref);
+ }
+ }
+ HMAPX_FOR_EACH (hmapx_node, &tracked_lses->crupdated) {
+ struct ovn_datapath *od = hmapx_node->data;
+
+ lflow_ref_unlink_lflows(od->datapath_lflows);
+ build_lswitch_and_lrouter_iterate_by_ls(od, &lsi);
+
+ const struct ls_stateful_record *ls_stateful_rec =
+ ls_stateful_table_find(lflow_input->ls_stateful_table, od->nbs);
+ if (ls_stateful_rec) {
+ lflow_ref_unlink_lflows(ls_stateful_rec->lflow_ref);
+ build_ls_stateful_flows(ls_stateful_rec, od,
+ lflow_input->ls_port_groups,
+ lflow_input->meter_groups,
+ lflow_input->sampling_apps,
+ lflow_input->features, lflows,
+ lflow_input->sbrec_acl_id_table);
+ }
+ }
+
+ /* Phase 2: sync. All datapath groups are now allocated, so this won't
+ * recompute the same groups over and over again. */
+ HMAPX_FOR_EACH (hmapx_node, &tracked_lses->deleted) {
+ struct ovn_datapath *od = hmapx_node->data;
+ handled = lflow_ref_sync_lflows(
+ od->datapath_lflows, lflows, ovnsb_txn, lflow_input->dps,
+ lflow_input->ovn_internal_version_changed,
+ lflow_input->sbrec_logical_flow_table,
+ lflow_input->sbrec_logical_dp_group_table);
+ if (!handled) {
+ goto out;
+ }
+ }
+ if (ls_sful_trk) {
+ HMAPX_FOR_EACH (hmapx_node, &ls_sful_trk->deleted) {
+ struct ls_stateful_record *ls_stateful_rec = hmapx_node->data;
+ handled = lflow_ref_sync_lflows(
+ ls_stateful_rec->lflow_ref, lflows, ovnsb_txn,
+ lflow_input->dps,
+ lflow_input->ovn_internal_version_changed,
+ lflow_input->sbrec_logical_flow_table,
+ lflow_input->sbrec_logical_dp_group_table);
+ if (!handled) {
+ goto out;
+ }
+ }
+ }
+ HMAPX_FOR_EACH (hmapx_node, &tracked_lses->crupdated) {
+ struct ovn_datapath *od = hmapx_node->data;
+
+ handled = lflow_ref_sync_lflows(
+ od->datapath_lflows, lflows, ovnsb_txn, lflow_input->dps,
+ lflow_input->ovn_internal_version_changed,
+ lflow_input->sbrec_logical_flow_table,
+ lflow_input->sbrec_logical_dp_group_table);
+ if (!handled) {
+ goto out;
+ }
+
+ const struct ls_stateful_record *ls_stateful_rec =
+ ls_stateful_table_find(lflow_input->ls_stateful_table, od->nbs);
+ if (ls_stateful_rec) {
+ handled = lflow_ref_sync_lflows(
+ ls_stateful_rec->lflow_ref, lflows, ovnsb_txn,
+ lflow_input->dps,
+ lflow_input->ovn_internal_version_changed,
+ lflow_input->sbrec_logical_flow_table,
+ lflow_input->sbrec_logical_dp_group_table);
+ if (!handled) {
+ goto out;
+ }
+ }
+ }
+
+out:
+ ds_destroy(&lsi.actions);
+ ds_destroy(&lsi.match);
+ return handled;
+}
+
bool
lflow_handle_northd_lr_changes(struct ovsdb_idl_txn *ovnsb_txn,
struct tracked_dps *tracked_lrs,
@@ -21699,8 +21845,6 @@ lflow_handle_northd_port_changes(struct ovsdb_idl_txn
*ovnsb_txn,
lflow_input->features,
lflows,
lflow_input->sbrec_acl_id_table);
- build_network_function(od, lflows, lflow_input->ls_port_groups,
- ls_stateful_rec->lflow_ref);
handled = lflow_ref_sync_lflows(
ls_stateful_rec->lflow_ref, lflows, ovnsb_txn,
lflow_input->dps,
@@ -22005,9 +22149,6 @@ lflow_handle_ls_stateful_changes(struct ovsdb_idl_txn
*ovnsb_txn,
lflow_input->features,
lflows,
lflow_input->sbrec_acl_id_table);
- 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
diff --git a/northd/northd.h b/northd/northd.h
index 61546bdd2..217f3c6fb 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -997,6 +997,11 @@ void build_route_data_flows_for_lrouter(
const struct group_ecmp_datapath *route_node,
const struct sset *bfd_ports);
+bool lflow_handle_northd_ls_changes(struct ovsdb_idl_txn *ovnsb_txn,
+ struct tracked_dps *,
+ struct ls_stateful_tracked_data *,
+ struct lflow_input *,
+ struct lflow_table *lflows);
bool lflow_handle_northd_lr_changes(struct ovsdb_idl_txn *ovnsh_txn,
struct tracked_dps *,
struct lflow_input *,
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 9a0ed2f01..eaaec17a2 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -16186,7 +16186,7 @@ check as northd ovn-appctl -t ovn-northd
inc-engine/clear-stats
check ovn-nbctl --wait=sb ls-add sw0
check_engine_stats northd norecompute compute
check_engine_stats ls_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
# For the below engine nodes, en_northd is input. So check
# their engine status.
@@ -16224,7 +16224,7 @@ check as northd ovn-appctl -t ovn-northd
inc-engine/clear-stats
check ovn-nbctl --wait=sb ls-del sw0
check_engine_stats northd norecompute compute
check_engine_stats ls_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
# For the below engine nodes, en_northd is input. So check
# their engine status.
--
2.43.0
--
_'Esta mensagem é direcionada apenas para os endereços constantes no
cabeçalho inicial. Se você não está listado nos endereços constantes no
cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa
mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas estão
imediatamente anuladas e proibidas'._
* **'Apesar do Magazine Luiza tomar
todas as precauções razoáveis para assegurar que nenhum vírus esteja
presente nesse e-mail, a empresa não poderá aceitar a responsabilidade por
quaisquer perdas ou danos causados por esse e-mail ou por seus anexos'.*
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
