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. build_network_function() and the
helpers it uses are moved ahead of build_ls_stateful_flows(), so no
forward declaration is needed; that code is not otherwise changed.
- 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-by: 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 | 255 ++++++++++++++++++++++++++++++++----------
northd/northd.h | 5 +
tests/ovn-northd.at | 4 +-
5 files changed, 232 insertions(+), 64 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 28c1632d6..7419eac32 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -6996,10 +6996,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
@@ -20023,39 +20024,6 @@ build_lr_stateful_flows(const struct
lr_stateful_record *lr_stateful_rec,
lr_stateful_rec->lflow_ref);
}
-static void
-build_ls_stateful_flows(const struct ls_stateful_record *ls_stateful_rec,
- const struct ovn_datapath *od,
- const struct ls_port_group_table *ls_pgs,
- const struct shash *meter_groups,
- const struct sampling_app_table *sampling_apps,
- const struct chassis_features *features,
- struct lflow_table *lflows,
- const struct sbrec_acl_id_table *sbrec_acl_id_table)
-{
- build_ls_stateful_rec_pre_acls(ls_stateful_rec, od, ls_pgs, lflows,
- ls_stateful_rec->lflow_ref);
- build_ls_stateful_rec_pre_lb(ls_stateful_rec, od, lflows,
- ls_stateful_rec->lflow_ref);
- build_acl_hints(ls_stateful_rec, od, lflows,
- ls_stateful_rec->lflow_ref);
- build_acls(ls_stateful_rec, od, lflows, ls_pgs, meter_groups,
- sampling_apps, features, ls_stateful_rec->lflow_ref,
- sbrec_acl_id_table);
-
- /* Build CT extraction flows - only needed if this datapath has load
- * balancers. */
- if (ls_stateful_rec->has_lb_vip) {
- ovn_lflow_add(lflows, od, S_SWITCH_IN_CT_EXTRACT, 100,
- "ct.new && ip",
- REG_CT_PROTO " = ct_proto(); "
- REG_CT_TP_DST " = ct_tp_dst(); next;",
- ls_stateful_rec->lflow_ref);
- }
-
- build_lb_hairpin(ls_stateful_rec, od, lflows, ls_stateful_rec->lflow_ref);
-}
-
/* For packets received on tunnel and egressing towards a network-function port
* commit the tunnel interface id in CT. This will be utilized when the packet
* comes out of the other network-function interface of the service VM. The
@@ -20829,6 +20797,45 @@ build_network_function(const struct ovn_datapath *od,
bitmap_free(nfg_egress_bitmap);
}
+static void
+build_ls_stateful_flows(const struct ls_stateful_record *ls_stateful_rec,
+ const struct ovn_datapath *od,
+ const struct ls_port_group_table *ls_pgs,
+ const struct shash *meter_groups,
+ const struct sampling_app_table *sampling_apps,
+ const struct chassis_features *features,
+ struct lflow_table *lflows,
+ const struct sbrec_acl_id_table *sbrec_acl_id_table)
+{
+ build_ls_stateful_rec_pre_acls(ls_stateful_rec, od, ls_pgs, lflows,
+ ls_stateful_rec->lflow_ref);
+ build_ls_stateful_rec_pre_lb(ls_stateful_rec, od, lflows,
+ ls_stateful_rec->lflow_ref);
+ build_acl_hints(ls_stateful_rec, od, lflows,
+ ls_stateful_rec->lflow_ref);
+ build_acls(ls_stateful_rec, od, lflows, ls_pgs, meter_groups,
+ sampling_apps, features, ls_stateful_rec->lflow_ref,
+ sbrec_acl_id_table);
+
+ /* Build CT extraction flows - only needed if this datapath has load
+ * balancers. */
+ if (ls_stateful_rec->has_lb_vip) {
+ ovn_lflow_add(lflows, od, S_SWITCH_IN_CT_EXTRACT, 100,
+ "ct.new && ip",
+ REG_CT_PROTO " = ct_proto(); "
+ REG_CT_TP_DST " = ct_tp_dst(); next;",
+ ls_stateful_rec->lflow_ref);
+ }
+
+ 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);
+}
+
struct lswitch_flow_build_info {
const struct ovn_datapaths *ls_datapaths;
const struct ovn_datapaths *lr_datapaths;
@@ -20875,33 +20882,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
@@ -21590,6 +21605,135 @@ 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 of the tracked switches are fully processed
+ * here. For a deleted switch the record is taken from 'ls_sful_trk' (it
+ * is already gone from the ls_stateful table) and is only unlinked and
+ * synced, which is what removes the flows that are no longer referenced.
+ * For a created or updated switch the record is looked up in
+ * lflow_input->ls_stateful_table and rebuilt. */
+
+ /* 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,
@@ -21809,8 +21953,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,
@@ -22094,9 +22236,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 c7b4e98d4..87f465769 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -16361,7 +16361,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.
@@ -16399,7 +16399,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
