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
unlinked via od->datapath_lflows, while created/updated switches are
(re)built with build_lswitch_and_lrouter_iterate_by_ls(); either way the
lflow_ref is marked dirty for the lflow sync node. 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.
The other lflow_ref of a switch, its ls_stateful record's, needs nothing
new: en_ls_stateful creates and drops the record along with the switch, and
lflow_handle_ls_stateful_changes() already builds and unlinks its flows.
Since the lflow sync node syncs every dirty lflow_ref together, the shared
datapath groups of the two refs need no coordination here either.
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. Add a test
for switches created and deleted with ACLs, on their own and together with
an ACL change on a pre-existing switch.
Assisted-by: Claude Opus 4.8, Claude Code
Signed-off-by: Lucas Vargas Dias <[email protected]>
---
northd/en-lflow.c | 9 +-
northd/en-multicast.c | 9 ++
northd/northd.c | 196 ++++++++++++++++++++++++++----------------
northd/northd.h | 4 +
tests/ovn-northd.at | 73 +++++++++++++++-
5 files changed, 212 insertions(+), 79 deletions(-)
diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index a7291c847..dc028dd4d 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -138,15 +138,16 @@ lflow_northd_handler(struct engine_node *node,
return EN_UNHANDLED;
}
- if (northd_has_lswitches_in_tracked_data(&northd_data->trk_data)) {
- return EN_UNHANDLED;
- }
-
struct lflow_data *lflow_data = data;
struct lflow_input lflow_input;
lflow_get_input_data(node, &lflow_input);
+ lflow_handle_northd_ls_changes(&northd_data->trk_data.trk_switches,
+ &lflow_input,
+ lflow_data->lflow_table,
+ &lflow_data->trk_data.dirty_lflow_refs);
+
lflow_handle_northd_lr_changes(&northd_data->trk_data.trk_routers,
&lflow_input,
lflow_data->lflow_table,
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 285563ea9..92619871b 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -7298,10 +7298,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
@@ -20605,54 +20606,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 && ip.is_frag",
- REG_CT_PROTO " = ct_proto(); "
- REG_CT_TP_DST " = ct_tp_dst(); next;",
- ls_stateful_rec->lflow_ref);
- ovn_lflow_add(lflows, od, S_SWITCH_IN_CT_EXTRACT, 100,
- "ct.new && ip && tcp && !ip.is_frag",
- REG_CT_PROTO " = ip.proto; "
- REG_CT_TP_DST " = tcp.dst; next;",
- ls_stateful_rec->lflow_ref);
- ovn_lflow_add(lflows, od, S_SWITCH_IN_CT_EXTRACT, 100,
- "ct.new && ip && udp && !ip.is_frag",
- REG_CT_PROTO " = ip.proto; "
- REG_CT_TP_DST " = udp.dst; next;",
- ls_stateful_rec->lflow_ref);
- ovn_lflow_add(lflows, od, S_SWITCH_IN_CT_EXTRACT, 100,
- "ct.new && ip && sctp && !ip.is_frag",
- REG_CT_PROTO " = ip.proto; "
- REG_CT_TP_DST " = sctp.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
@@ -21500,6 +21453,60 @@ 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 && ip.is_frag",
+ REG_CT_PROTO " = ct_proto(); "
+ REG_CT_TP_DST " = ct_tp_dst(); next;",
+ ls_stateful_rec->lflow_ref);
+ ovn_lflow_add(lflows, od, S_SWITCH_IN_CT_EXTRACT, 100,
+ "ct.new && ip && tcp && !ip.is_frag",
+ REG_CT_PROTO " = ip.proto; "
+ REG_CT_TP_DST " = tcp.dst; next;",
+ ls_stateful_rec->lflow_ref);
+ ovn_lflow_add(lflows, od, S_SWITCH_IN_CT_EXTRACT, 100,
+ "ct.new && ip && udp && !ip.is_frag",
+ REG_CT_PROTO " = ip.proto; "
+ REG_CT_TP_DST " = udp.dst; next;",
+ ls_stateful_rec->lflow_ref);
+ ovn_lflow_add(lflows, od, S_SWITCH_IN_CT_EXTRACT, 100,
+ "ct.new && ip && sctp && !ip.is_frag",
+ REG_CT_PROTO " = ip.proto; "
+ REG_CT_TP_DST " = sctp.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;
@@ -21546,33 +21553,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
@@ -22253,6 +22268,46 @@ lflow_reset_northd_refs(struct lflow_input
*lflow_input)
}
}
+void
+lflow_handle_northd_ls_changes(struct tracked_dps *tracked_lses,
+ struct lflow_input *lflow_input,
+ struct lflow_table *lflows,
+ struct hmapx *dirty_lflow_refs)
+{
+ 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,
+ };
+
+ /* Only the per-switch 'od->datapath_lflows' is handled here. The other
+ * per-switch lflow_ref, the ls_stateful record's, is created and dropped
+ * along with the switch by en_ls_stateful and handled by
+ * lflow_handle_ls_stateful_changes(). */
+ HMAPX_FOR_EACH (hmapx_node, &tracked_lses->deleted) {
+ struct ovn_datapath *od = hmapx_node->data;
+ lflow_ref_unlink_lflows(od->datapath_lflows, lflows);
+ hmapx_add(dirty_lflow_refs, od->datapath_lflows);
+ }
+ HMAPX_FOR_EACH (hmapx_node, &tracked_lses->crupdated) {
+ struct ovn_datapath *od = hmapx_node->data;
+
+ lflow_ref_unlink_lflows(od->datapath_lflows, lflows);
+ build_lswitch_and_lrouter_iterate_by_ls(od, &lsi);
+ hmapx_add(dirty_lflow_refs, od->datapath_lflows);
+ }
+
+ ds_destroy(&lsi.actions);
+ ds_destroy(&lsi.match);
+}
+
void
lflow_handle_northd_lr_changes(struct tracked_dps *tracked_lrs,
struct lflow_input *lflow_input,
@@ -22402,8 +22457,6 @@ lflow_handle_northd_port_changes(struct
tracked_ovn_ports *trk_lsps,
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);
hmapx_add(dirty_lflow_refs, ls_stateful_rec->lflow_ref);
}
@@ -22603,9 +22656,6 @@ lflow_handle_ls_stateful_changes(struct
ls_stateful_tracked_data *trk_data,
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);
hmapx_add(dirty_lflow_refs, ls_stateful_rec->lflow_ref);
}
diff --git a/northd/northd.h b/northd/northd.h
index a1b8ae414..46294c7d7 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -995,6 +995,10 @@ void build_route_data_flows_for_lrouter(
const struct group_ecmp_datapath *route_node,
const struct sset *bfd_ports);
+void lflow_handle_northd_ls_changes(struct tracked_dps *,
+ struct lflow_input *,
+ struct lflow_table *lflows,
+ struct hmapx *dirty_lflow_refs);
void lflow_handle_northd_lrp_changes(struct tracked_ovn_ports *,
struct lflow_input *,
struct lflow_table *lflows,
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index f90f0ee9d..d7bf45a04 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -17551,7 +17551,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.
@@ -17589,7 +17589,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.
@@ -17604,6 +17604,75 @@ OVN_CLEANUP_NORTHD
AT_CLEANUP
])
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([Logical switch incremental processing with ACLs])
+AT_KEYWORDS([incremental processing])
+
+ovn_start
+
+# A switch datapath's flows are split across two lflow_refs: its own
+# 'datapath_lflows', handled by lflow_handle_northd_ls_changes(), and its
+# ls_stateful record's, which holds the ACL flows and is handled by
+# lflow_handle_ls_stateful_changes() whether the switch is new or not.
+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 builds both.
+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
+
+# A single transaction that both creates a switch with an ACL and adds an ACL
+# to the pre-existing switch.
+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 only goes through the
+# ls_stateful handler.
+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 drops both: its ls_stateful record is
+# deleted along with the datapath.
+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])
--
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
