Patch (localnet / L2 gateway) port ofport changes force a full recompute
of the logical flow output through two independent engine edges, even
though logical flows never depend on patch ofports:
- non_vif_data bundles patch ofports together with tunnel data, and
en_lflow_output has no change handler for non_vif_data, so any patch
ofport change invalidates it too.
- Patch ports never carry an iface-id, so their ofport changes also hit
the "unhandled" branch of binding_handle_ovs_interface_changes(),
forcing a full recompute of runtime_data and, transitively, of
lflow_output again.
This is hit on the datapath every time a bridged / L2 logical port is
bound or moved between chassis, since the peer patch port is (re)created
and gets a new ofport. In deployments with many logical flows each
recompute can take several seconds, dominating the port's dataplane
downtime.
Fix both edges:
- Move patch ofports into their own engine node (en_patch_port_data)
that feeds only the physical flow output. en_non_vif_data keeps
feeding en_lflow_output, but now only changes on tunnel ofport
changes.
- Recognize patch-type OVS interfaces in
binding_handle_ovs_interface_changes() and skip them instead of
treating them as unhandled: their ofport is already tracked by
en_patch_port_data.
Tunnel (geneve/vxlan) interfaces are deliberately left alone in the
latter fix. runtime_data's 'active_tunnels' (BFD-derived reachability,
used for ECMP / gateway-chassis flow selection) has no incremental
tracking and is only recalculated on a full recompute; skipping tunnel
ofport changes there would silently stop refreshing it on BFD status
changes.
Recognizing patch ports in binding_handle_ovs_interface_changes() also
exposes a pre-existing gap: unlike binding_handle_port_binding_changes(),
it never re-scans a newly-local datapath's sibling ports (localnet /
external / vtep / multichassis) after calling add_local_datapath(). This
used to be masked by the very recompute this patch removes. Extract the
existing catch-up logic into catch_up_new_local_datapaths() and call it
from both incremental handlers.
Assisted-by: Claude Opus 4.8, Claude Code
Signed-off-by: Jun Gu <[email protected]>
---
controller/binding.c | 122 +++++++++++++++------
controller/local_data.c | 180 ++++++++++++++++++++-----------
controller/local_data.h | 22 ++--
controller/ovn-controller.c | 94 +++++++++++++---
tests/ovn-controller.at | 81 ++++++++++++++
tests/ovn-inc-proc-graph-dump.at | 5 +
6 files changed, 387 insertions(+), 117 deletions(-)
diff --git a/controller/binding.c b/controller/binding.c
index de51be823..a91193aa0 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -2706,6 +2706,30 @@ is_iface_vif(const struct ovsrec_interface *iface_rec)
return true;
}
+/* Patch (localnet / L2 gateway) ports are OVS interfaces that
+ * ovn-controller itself creates and manages. They never carry an iface-id
+ * and are of no interest to port binding processing: their ofport changes
+ * are already tracked by the patch_port_data engine node. Recognizing them
+ * here lets binding_handle_ovs_interface_changes() skip them instead of
+ * treating them as an unhandled change and forcing a full recompute of
+ * runtime_data (and, transitively, of lflow_output) on every patch port
+ * bind / migration.
+ *
+ * Tunnel (geneve/vxlan) interfaces are deliberately NOT included here even
+ * though their ofport changes are tracked by the non_vif_data engine node:
+ * runtime_data's 'active_tunnels' (BFD-derived tunnel reachability, used by
+ * ECMP/gateway-chassis logical flow selection) is only ever recalculated
+ * during a full recompute of runtime_data, and currently has no incremental
+ * tracking of its own (see the engine dependency table above
+ * en_runtime_data_run()). Treating tunnel interface changes as "handled"
+ * here would silently stop refreshing 'active_tunnels' on BFD status
+ * changes. */
+static bool
+is_iface_patch(const struct ovsrec_interface *iface_rec)
+{
+ return iface_rec->type && !strcmp(iface_rec->type, "patch");
+}
+
bool
is_iface_in_int_bridge(const struct ovsrec_interface *iface,
const struct ovsrec_bridge *br_int)
@@ -2786,6 +2810,58 @@ ovs_interface_change_need_handle(const struct
ovsrec_interface *iface_rec,
return false;
}
+/* A datapath can become local (added to 'b_ctx_out->local_datapaths' and
+ * marked TRACKED_RESOURCE_NEW in 'b_ctx_out->tracked_dp_bindings') from more
+ * than one incremental handler: e.g. a VIF getting claimed by
+ * binding_handle_ovs_interface_changes(), or a port_binding update handled
+ * by binding_handle_port_binding_changes(). 'tracked_dp_bindings' is reset
+ * on every engine iteration, but sibling ports of the newly-local datapath
+ * (localnet/external/vtep/multichassis) may already be present in the SB
+ * IDL and therefore never show up again as their own tracked change. So
+ * every incremental handler that can call add_local_datapath() for a
+ * previously-unknown datapath must run this catch-up scan afterwards,
+ * otherwise those sibling ports are silently never registered until the
+ * next full recompute. */
+static void
+catch_up_new_local_datapaths(struct binding_ctx_in *b_ctx_in,
+ struct binding_ctx_out *b_ctx_out)
+{
+ struct shash bridge_mappings = SHASH_INITIALIZER(&bridge_mappings);
+ add_ovs_bridge_mappings(b_ctx_in->ovs_table, b_ctx_in->bridge_table,
+ &bridge_mappings);
+
+ struct tracked_datapath *t_dp;
+ HMAP_FOR_EACH (t_dp, node, b_ctx_out->tracked_dp_bindings) {
+ if (t_dp->tracked_type != TRACKED_RESOURCE_NEW) {
+ continue;
+ }
+ struct sbrec_port_binding *target =
+ sbrec_port_binding_index_init_row(
+ b_ctx_in->sbrec_port_binding_by_datapath);
+ sbrec_port_binding_index_set_datapath(target, t_dp->dp);
+
+ const struct sbrec_port_binding *pb;
+ SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, target,
+ b_ctx_in->sbrec_port_binding_by_datapath) {
+ enum en_lport_type lport_type = get_lport_type(pb);
+ if (lport_type == LP_LOCALNET) {
+ consider_localnet_lport(pb, b_ctx_out);
+ update_ld_localnet_port(pb, &bridge_mappings,
+ b_ctx_out->local_datapaths);
+ } else if (lport_type == LP_EXTERNAL) {
+ update_ld_external_ports(pb, b_ctx_out->local_datapaths);
+ } else if (lport_type == LP_VTEP) {
+ update_ld_vtep_port(pb, b_ctx_out->local_datapaths);
+ } else if (pb->n_additional_chassis) {
+ update_ld_multichassis_ports(pb, b_ctx_out->local_datapaths);
+ }
+ }
+ sbrec_port_binding_index_destroy_row(target);
+ }
+
+ shash_destroy(&bridge_mappings);
+}
+
/* Returns true if the ovs interface changes were handled successfully,
* false otherwise.
*/
@@ -2824,6 +2900,10 @@ binding_handle_ovs_interface_changes(struct
binding_ctx_in *b_ctx_in,
const char *old_iface_id = smap_get(b_ctx_out->local_iface_ids,
iface_rec->name);
if (!iface_id && !old_iface_id && !is_iface_vif(iface_rec)) {
+ if (is_iface_patch(iface_rec)) {
+ /* Not a port binding concern, handled elsewhere. */
+ continue;
+ }
/* Right now we are not handling ovs_interface changes if the
* interface doesn't have iface-id or didn't have it
* previously. */
@@ -2904,6 +2984,12 @@ binding_handle_ovs_interface_changes(struct
binding_ctx_in *b_ctx_in,
}
}
+ if (handled) {
+ /* consider_iface_claim() above may have called add_local_datapath()
+ * for a datapath that was not local before. */
+ catch_up_new_local_datapaths(b_ctx_in, b_ctx_out);
+ }
+
return handled;
}
@@ -3471,41 +3557,7 @@ delete_done:
/* There may be new local datapaths added by the above handling, so go
* through each port_binding of newly added local datapaths to update
* related local_datapaths if needed. */
- struct shash bridge_mappings =
- SHASH_INITIALIZER(&bridge_mappings);
- add_ovs_bridge_mappings(b_ctx_in->ovs_table,
- b_ctx_in->bridge_table,
- &bridge_mappings);
- struct tracked_datapath *t_dp;
- HMAP_FOR_EACH (t_dp, node, b_ctx_out->tracked_dp_bindings) {
- if (t_dp->tracked_type != TRACKED_RESOURCE_NEW) {
- continue;
- }
- struct sbrec_port_binding *target =
- sbrec_port_binding_index_init_row(
- b_ctx_in->sbrec_port_binding_by_datapath);
- sbrec_port_binding_index_set_datapath(target, t_dp->dp);
-
- SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, target,
- b_ctx_in->sbrec_port_binding_by_datapath) {
- enum en_lport_type lport_type = get_lport_type(pb);
- if (lport_type == LP_LOCALNET) {
- consider_localnet_lport(pb, b_ctx_out);
- update_ld_localnet_port(pb, &bridge_mappings,
- b_ctx_out->local_datapaths);
- } else if (lport_type == LP_EXTERNAL) {
- update_ld_external_ports(pb, b_ctx_out->local_datapaths);
- } else if (lport_type == LP_VTEP) {
- update_ld_vtep_port(pb, b_ctx_out->local_datapaths);
- } else if (pb->n_additional_chassis) {
- update_ld_multichassis_ports(pb,
- b_ctx_out->local_datapaths);
- }
- }
- sbrec_port_binding_index_destroy_row(target);
- }
-
- shash_destroy(&bridge_mappings);
+ catch_up_new_local_datapaths(b_ctx_in, b_ctx_out);
}
return handled;
diff --git a/controller/local_data.c b/controller/local_data.c
index af6c75b40..451eb9d35 100644
--- a/controller/local_data.c
+++ b/controller/local_data.c
@@ -452,14 +452,57 @@ tracked_datapaths_destroy(struct hmap *tracked_datapaths)
hmap_destroy(tracked_datapaths);
}
-/* Iterates the br_int ports and build the simap of patch to ofports
- * and chassis tunnels. */
+/* Iterates the br_int ports and builds the simap of patch port to ofport. */
void
-local_nonvif_data_run(const struct ovsrec_bridge *br_int,
- const struct sbrec_chassis *chassis_rec,
- struct simap *patch_ofports,
- struct hmap *chassis_tunnels,
- struct flow_based_tunnel *flow_tunnels)
+local_patch_ports_run(const struct ovsrec_bridge *br_int,
+ struct simap *patch_ofports)
+{
+ for (size_t i = 0; i < br_int->n_ports; i++) {
+ const struct ovsrec_port *port_rec = br_int->ports[i];
+ if (!strcmp(port_rec->name, br_int->name)) {
+ continue;
+ }
+
+ const char *localnet = smap_get(&port_rec->external_ids,
+ "ovn-localnet-port");
+ const char *l2gateway = smap_get(&port_rec->external_ids,
+ "ovn-l2gateway-port");
+ if (!localnet && !l2gateway) {
+ continue;
+ }
+
+ for (size_t j = 0; j < port_rec->n_interfaces; j++) {
+ const struct ovsrec_interface *iface_rec = port_rec->interfaces[j];
+
+ /* Get OpenFlow port number. */
+ if (!iface_rec->n_ofport) {
+ continue;
+ }
+ int64_t ofport = iface_rec->ofport[0];
+ if (ofport < 1 || ofport > ofp_to_u16(OFPP_MAX)) {
+ continue;
+ }
+
+ if (strcmp(iface_rec->type, "patch")) {
+ continue;
+ }
+ if (localnet) {
+ simap_put(patch_ofports, localnet, ofport);
+ break;
+ } else if (l2gateway) {
+ /* L2 gateway patch ports can be handled just like VIFs. */
+ simap_put(patch_ofports, l2gateway, ofport);
+ break;
+ }
+ }
+ }
+}
+
+void
+local_tunnels_run(const struct ovsrec_bridge *br_int,
+ const struct sbrec_chassis *chassis_rec,
+ struct hmap *chassis_tunnels,
+ struct flow_based_tunnel *flow_tunnels)
{
for (int i = 0; i < br_int->n_ports; i++) {
const struct ovsrec_port *port_rec = br_int->ports[i];
@@ -477,10 +520,9 @@ local_nonvif_data_run(const struct ovsrec_bridge *br_int,
track_flow_based_tunnel(port_rec, chassis_rec, flow_tunnels);
- const char *localnet = smap_get(&port_rec->external_ids,
- "ovn-localnet-port");
- const char *l2gateway = smap_get(&port_rec->external_ids,
- "ovn-l2gateway-port");
+ if (!tunnel_id) {
+ continue;
+ }
for (int j = 0; j < port_rec->n_interfaces; j++) {
const struct ovsrec_interface *iface_rec = port_rec->interfaces[j];
@@ -494,67 +536,63 @@ local_nonvif_data_run(const struct ovsrec_bridge *br_int,
continue;
}
- bool is_patch = !strcmp(iface_rec->type, "patch");
- if (is_patch && localnet) {
- simap_put(patch_ofports, localnet, ofport);
- break;
- } else if (is_patch && l2gateway) {
- /* L2 gateway patch ports can be handled just like VIFs. */
- simap_put(patch_ofports, l2gateway, ofport);
- break;
- } else if (tunnel_id) {
- enum chassis_tunnel_type tunnel_type;
- if (!strcmp(iface_rec->type, "geneve")) {
- tunnel_type = GENEVE;
- } else if (!strcmp(iface_rec->type, "vxlan")) {
- tunnel_type = VXLAN;
- } else {
- continue;
- }
-
- /* We split the tunnel_id to get the chassis-id
- * and hash the tunnel list on the chassis-id. The
- * reason to use the chassis-id alone is because
- * there might be cases (multicast, gateway chassis)
- * where we need to tunnel to the chassis, but won't
- * have the encap-ip specifically.
- */
- char *hash_id = NULL;
- char *ip = NULL;
-
- if (!encaps_tunnel_id_parse(tunnel_id, &hash_id, &ip, NULL)) {
- continue;
- }
- struct chassis_tunnel *tun = xmalloc(sizeof *tun);
- hmap_insert(chassis_tunnels, &tun->hmap_node,
- hash_string(hash_id, 0));
- tun->chassis_id = xstrdup(tunnel_id);
- tun->ofport = u16_to_ofp(ofport);
- tun->type = tunnel_type;
- tun->is_ipv6 = ip ? addr_is_ipv6(ip) : false;
- tun->is_ramp_tunnel = is_ramp_tunnel(&iface_rec->other_config);
-
- free(hash_id);
- free(ip);
- break;
+ enum chassis_tunnel_type tunnel_type;
+ if (!strcmp(iface_rec->type, "geneve")) {
+ tunnel_type = GENEVE;
+ } else if (!strcmp(iface_rec->type, "vxlan")) {
+ tunnel_type = VXLAN;
+ } else {
+ continue;
+ }
+
+ /* We split the tunnel_id to get the chassis-id
+ * and hash the tunnel list on the chassis-id. The
+ * reason to use the chassis-id alone is because
+ * there might be cases (multicast, gateway chassis)
+ * where we need to tunnel to the chassis, but won't
+ * have the encap-ip specifically.
+ */
+ char *hash_id = NULL;
+ char *ip = NULL;
+
+ if (!encaps_tunnel_id_parse(tunnel_id, &hash_id, &ip, NULL)) {
+ continue;
}
+ struct chassis_tunnel *tun = xmalloc(sizeof *tun);
+ hmap_insert(chassis_tunnels, &tun->hmap_node,
+ hash_string(hash_id, 0));
+ tun->chassis_id = xstrdup(tunnel_id);
+ tun->ofport = u16_to_ofp(ofport);
+ tun->type = tunnel_type;
+ tun->is_ipv6 = ip ? addr_is_ipv6(ip) : false;
+ tun->is_ramp_tunnel = is_ramp_tunnel(&iface_rec->other_config);
+
+ free(hash_id);
+ free(ip);
+ break;
}
}
}
-bool
-local_nonvif_data_handle_ovs_iface_changes(
- const struct ovsrec_interface_table *iface_table)
+/* Returns false (i.e. a recompute is required) if any tracked OVS interface of
+ * one of the given 'types' had its ofport added, removed or changed. */
+static bool
+nonvif_iface_ofport_unchanged(const struct ovsrec_interface_table *iface_table,
+ const char *const *types, size_t n_types)
{
const struct ovsrec_interface *iface_rec;
OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, iface_table) {
- /* Check only patch ports or tunnels. */
- if (strcmp(iface_rec->type, "geneve") &&
- strcmp(iface_rec->type, "patch") &&
- strcmp(iface_rec->type, "vxlan")) {
+ bool type_match = false;
+ for (size_t i = 0; i < n_types; i++) {
+ if (!strcmp(iface_rec->type, types[i])) {
+ type_match = true;
+ break;
+ }
+ }
+ if (!type_match) {
continue;
}
- /* We are interested only in ofport changes for this handler. */
+ /* We are interested only in ofport changes for these handlers. */
if (ovsrec_interface_is_new(iface_rec) ||
ovsrec_interface_is_deleted(iface_rec) ||
ovsrec_interface_is_updated(iface_rec,
@@ -566,6 +604,24 @@ local_nonvif_data_handle_ovs_iface_changes(
return true;
}
+bool
+local_patch_ports_handle_ovs_iface_changes(
+ const struct ovsrec_interface_table *iface_table)
+{
+ static const char *const types[] = { "patch" };
+ return nonvif_iface_ofport_unchanged(iface_table, types,
+ ARRAY_SIZE(types));
+}
+
+bool
+local_tunnels_handle_ovs_iface_changes(
+ const struct ovsrec_interface_table *iface_table)
+{
+ static const char *const types[] = { "geneve", "vxlan" };
+ return nonvif_iface_ofport_unchanged(iface_table, types,
+ ARRAY_SIZE(types));
+}
+
bool
get_chassis_tunnel_ofport(const struct hmap *chassis_tunnels,
const char *chassis_name, ofp_port_t *ofport)
diff --git a/controller/local_data.h b/controller/local_data.h
index cbb8899eb..2e0fee0ca 100644
--- a/controller/local_data.h
+++ b/controller/local_data.h
@@ -159,13 +159,23 @@ struct flow_based_tunnel {
};
-void local_nonvif_data_run(const struct ovsrec_bridge *br_int,
- const struct sbrec_chassis *chassis,
- struct simap *patch_ofports,
- struct hmap *chassis_tunnels,
- struct flow_based_tunnel *flow_tunnels);
+/* Patch (localnet / L2 gateway) OVS ports. These are consumed only by the
+ * physical flow output, so they are tracked in their own engine node to avoid
+ * invalidating the logical flow output on every patch port change. */
+void local_patch_ports_run(const struct ovsrec_bridge *br_int,
+ struct simap *patch_ofports);
-bool local_nonvif_data_handle_ovs_iface_changes(
+bool local_patch_ports_handle_ovs_iface_changes(
+ const struct ovsrec_interface_table *);
+
+/* Tunnel OVS ports and the related chassis information. These are consumed by
+ * both the physical and the logical flow output. */
+void local_tunnels_run(const struct ovsrec_bridge *br_int,
+ const struct sbrec_chassis *chassis,
+ struct hmap *chassis_tunnels,
+ struct flow_based_tunnel *flow_tunnels);
+
+bool local_tunnels_handle_ovs_iface_changes(
const struct ovsrec_interface_table *);
struct chassis_tunnel *chassis_tunnel_find(const struct hmap *chassis_tunnels,
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 552e87b53..f4b60f0e8 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -3637,12 +3637,72 @@ en_dns_cache_cleanup(void *data OVS_UNUSED)
}
-/* Engine node which is used to handle the Non VIF data like
- * - OVS patch ports
- * - Tunnel ports and the related chassis information.
+/* Engine node which handles the OVS patch ports (localnet / L2 gateway).
+ *
+ * Kept separate from the tunnel data (en_non_vif_data) because patch ofports
+ * are consumed only by the physical flow output. Bundling them with the
+ * tunnel data would force a full recompute of the logical flow output (which
+ * has no handler for its non_vif_data input) on every patch port change, e.g.
+ * whenever a bridged/L2 logical port is bound or moved between chassis.
*/
-struct ed_type_non_vif_data {
+struct ed_type_patch_port_data {
struct simap patch_ofports; /* simap of patch ovs ports. */
+};
+
+static void *
+en_patch_port_data_init(struct engine_node *node OVS_UNUSED,
+ struct engine_arg *arg OVS_UNUSED)
+{
+ struct ed_type_patch_port_data *data = xzalloc(sizeof *data);
+ simap_init(&data->patch_ofports);
+ return data;
+}
+
+static void
+en_patch_port_data_cleanup(void *data OVS_UNUSED)
+{
+ struct ed_type_patch_port_data *ed_patch_port_data = data;
+ simap_destroy(&ed_patch_port_data->patch_ofports);
+}
+
+static enum engine_node_state
+en_patch_port_data_run(struct engine_node *node, void *data)
+{
+ struct ed_type_patch_port_data *ed_patch_port_data = data;
+ simap_destroy(&ed_patch_port_data->patch_ofports);
+ simap_init(&ed_patch_port_data->patch_ofports);
+
+ const struct ovsrec_open_vswitch_table *ovs_table =
+ EN_OVSDB_GET(engine_get_input("OVS_open_vswitch", node));
+ const struct ovsrec_bridge_table *bridge_table =
+ EN_OVSDB_GET(engine_get_input("OVS_bridge", node));
+
+ const struct ovsrec_bridge *br_int = get_br_int(bridge_table, ovs_table);
+ ovs_assert(br_int);
+
+ local_patch_ports_run(br_int, &ed_patch_port_data->patch_ofports);
+
+ return EN_UPDATED;
+}
+
+static enum engine_input_handler_result
+patch_port_data_ovs_iface_handler(struct engine_node *node,
+ void *data OVS_UNUSED)
+{
+ const struct ovsrec_interface_table *iface_table =
+ EN_OVSDB_GET(engine_get_input("OVS_interface", node));
+
+ if (local_patch_ports_handle_ovs_iface_changes(iface_table)) {
+ return EN_HANDLED_UNCHANGED;
+ } else {
+ return EN_UNHANDLED;
+ }
+}
+
+/* Engine node which handles the tunnel ports and the related chassis
+ * information. Consumed by both the physical and the logical flow output.
+ */
+struct ed_type_non_vif_data {
struct hmap chassis_tunnels; /* hmap of 'struct chassis_tunnel' from the
* tunnel OVS ports. */
struct flow_based_tunnel flow_tunnels[TUNNEL_TYPE_MAX];
@@ -3656,7 +3716,6 @@ en_non_vif_data_init(struct engine_node *node OVS_UNUSED,
struct engine_arg *arg OVS_UNUSED)
{
struct ed_type_non_vif_data *data = xzalloc(sizeof *data);
- simap_init(&data->patch_ofports);
hmap_init(&data->chassis_tunnels);
flow_based_tunnels_init(data->flow_tunnels);
data->use_flow_based_tunnels = false;
@@ -3667,7 +3726,6 @@ static void
en_non_vif_data_cleanup(void *data OVS_UNUSED)
{
struct ed_type_non_vif_data *ed_non_vif_data = data;
- simap_destroy(&ed_non_vif_data->patch_ofports);
chassis_tunnels_destroy(&ed_non_vif_data->chassis_tunnels);
flow_based_tunnels_destroy(ed_non_vif_data->flow_tunnels);
}
@@ -3676,11 +3734,9 @@ static enum engine_node_state
en_non_vif_data_run(struct engine_node *node, void *data)
{
struct ed_type_non_vif_data *ed_non_vif_data = data;
- simap_destroy(&ed_non_vif_data->patch_ofports);
chassis_tunnels_destroy(&ed_non_vif_data->chassis_tunnels);
flow_based_tunnels_destroy(ed_non_vif_data->flow_tunnels);
- simap_init(&ed_non_vif_data->patch_ofports);
hmap_init(&ed_non_vif_data->chassis_tunnels);
flow_based_tunnels_init(ed_non_vif_data->flow_tunnels);
@@ -3705,10 +3761,9 @@ en_non_vif_data_run(struct engine_node *node, void *data)
ed_non_vif_data->use_flow_based_tunnels =
is_flow_based_tunnels_enabled(ovs_table, chassis);
- local_nonvif_data_run(br_int, chassis,
- &ed_non_vif_data->patch_ofports,
- &ed_non_vif_data->chassis_tunnels,
- ed_non_vif_data->flow_tunnels);
+ local_tunnels_run(br_int, chassis,
+ &ed_non_vif_data->chassis_tunnels,
+ ed_non_vif_data->flow_tunnels);
return EN_UPDATED;
}
@@ -3719,7 +3774,7 @@ non_vif_data_ovs_iface_handler(struct engine_node *node,
void *data OVS_UNUSED)
const struct ovsrec_interface_table *iface_table =
EN_OVSDB_GET(engine_get_input("OVS_interface", node));
- if (local_nonvif_data_handle_ovs_iface_changes(iface_table)) {
+ if (local_tunnels_handle_ovs_iface_changes(iface_table)) {
return EN_HANDLED_UNCHANGED;
} else {
return EN_UNHANDLED;
@@ -4740,6 +4795,9 @@ static void init_physical_ctx(struct engine_node *node,
const struct ed_type_mff_ovn_geneve *ed_mff_ovn_geneve =
engine_get_input_data("mff_ovn_geneve", node);
+ struct ed_type_patch_port_data *patch_port_data =
+ engine_get_input_data("patch_port_data", node);
+
const struct ovsrec_interface_table *ovs_interface_table =
EN_OVSDB_GET(engine_get_input("if_status_mgr", node));
@@ -4791,7 +4849,7 @@ static void init_physical_ctx(struct engine_node *node,
p_ctx->ct_zones = ct_zones;
p_ctx->mff_ovn_geneve = ed_mff_ovn_geneve->mff_ovn_geneve;
p_ctx->local_bindings = &rt_data->lbinding_data.bindings;
- p_ctx->patch_ofports = &non_vif_data->patch_ofports;
+ p_ctx->patch_ofports = &patch_port_data->patch_ofports;
p_ctx->chassis_tunnels = &non_vif_data->chassis_tunnels;
p_ctx->flow_tunnels = non_vif_data->flow_tunnels;
p_ctx->use_flow_based_tunnels = non_vif_data->use_flow_based_tunnels;
@@ -6922,6 +6980,7 @@ static ENGINE_NODE(template_vars, CLEAR_TRACKED_DATA);
static ENGINE_NODE(ct_zones, CLEAR_TRACKED_DATA, IS_VALID);
static ENGINE_NODE(ovs_interface_shadow, CLEAR_TRACKED_DATA);
static ENGINE_NODE(runtime_data, CLEAR_TRACKED_DATA, SB_WRITE);
+static ENGINE_NODE(patch_port_data);
static ENGINE_NODE(non_vif_data);
static ENGINE_NODE(mff_ovn_geneve);
static ENGINE_NODE(ofctrl_is_connected);
@@ -7015,6 +7074,11 @@ inc_proc_ovn_controller_init(
engine_add_input(&en_port_groups, &en_runtime_data,
port_groups_runtime_data_handler);
+ engine_add_input(&en_patch_port_data, &en_ovs_open_vswitch, NULL);
+ engine_add_input(&en_patch_port_data, &en_ovs_bridge, NULL);
+ engine_add_input(&en_patch_port_data, &en_ovs_interface,
+ patch_port_data_ovs_iface_handler);
+
engine_add_input(&en_non_vif_data, &en_ovs_open_vswitch, NULL);
engine_add_input(&en_non_vif_data, &en_ovs_bridge, NULL);
engine_add_input(&en_non_vif_data, &en_sb_chassis, NULL);
@@ -7030,6 +7094,8 @@ inc_proc_ovn_controller_init(
/* Note: The order of inputs is important, all OVS interface changes must
* be handled before any ct_zone changes.
*/
+ engine_add_input(&en_pflow_output, &en_patch_port_data,
+ NULL);
engine_add_input(&en_pflow_output, &en_non_vif_data,
NULL);
engine_add_input(&en_pflow_output, &en_northd_options, NULL);
diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index e17ebea76..7459db3e9 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -1120,6 +1120,87 @@ OVN_CLEANUP([hv1])
AT_CLEANUP
])
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn-controller - patch and tunnel ofports tracked separately])
+AT_KEYWORDS([ovn-patch-port-data])
+
+ovn_start
+net_add n1
+
+sim_add hv1
+as hv1
+check ovs-vsctl add-br br-phys
+check ovs-vsctl add-br br-eth0
+check ovs-vsctl set open . external-ids:ovn-bridge-mappings=physnet1:br-eth0
+ovn_attach n1 br-phys 192.168.0.1
+
+sim_add hv2
+as hv2
+check ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.2
+
+check ovn-nbctl ls-add ls0 \
+ -- lsp-add ls0 ln0 \
+ -- lsp-set-type ln0 localnet \
+ -- lsp-set-addresses ln0 unknown \
+ -- lsp-set-options ln0 network_name=physnet1 \
+ -- lsp-add ls0 lsp0 \
+ -- lsp-set-addresses lsp0 "00:00:00:00:00:01 10.0.0.1"
+
+as hv1
+check ovs-vsctl add-port br-int vif0 \
+ -- set Interface vif0 external_ids:iface-id=lsp0
+
+wait_for_ports_up
+check ovn-nbctl --wait=hv sync
+
+# The localnet port of a locally bound datapath gets its OVS patch ports and
+# the remote chassis gets its tunnel port. Wait for both to settle before
+# looking at the engine statistics.
+OVS_WAIT_UNTIL([test 1 -le $(ovs-vsctl get Interface patch-br-int-to-ln0 \
+ ofport)])
+OVS_WAIT_UNTIL([test 1 -le $(ovs-vsctl get Interface ovn-hv2-0 ofport)])
+
+# A patch ofport change must be handled by en_patch_port_data only. The tunnel
+# data (en_non_vif_data), which is also consumed by the logical flow output,
+# must not be invalidated by it.
+check as hv1 ovn-appctl -t ovn-controller inc-engine/clear-stats
+check ovs-vsctl set Interface patch-br-int-to-ln0 ofport_request=4242
+OVS_WAIT_UNTIL([test 4242 = $(ovs-vsctl get Interface patch-br-int-to-ln0 \
+ ofport)])
+OVS_WAIT_UNTIL([test 1 -le $(ovs-ofctl dump-flows br-int | \
+ grep -c 'in_port=4242')])
+check_controller_engine_stats hv1 patch_port_data recompute nocompute
+check_controller_engine_stats hv1 non_vif_data norecompute compute
+check_controller_engine_stats hv1 pflow_output recompute nocompute
+check_controller_engine_stats hv1 runtime_data norecompute compute
+check_controller_engine_stats hv1 lflow_output norecompute nocompute
+
+# Conversely, a tunnel ofport change must be handled by en_non_vif_data only
+# and must leave en_patch_port_data alone. Tunnel interfaces are
+# deliberately NOT recognized by binding_handle_ovs_interface_changes() (see
+# is_iface_patch()), because runtime_data's 'active_tunnels' (BFD-derived
+# reachability, used by ECMP/gateway-chassis logical flow selection) has no
+# incremental tracking of its own and is only ever recalculated on a full
+# recompute of runtime_data. So a tunnel ofport change is still expected to
+# force a full recompute of both runtime_data and (transitively, since
+# en_lflow_output has no change handler for its en_non_vif_data input)
+# lflow_output.
+check as hv1 ovn-appctl -t ovn-controller inc-engine/clear-stats
+check ovs-vsctl set Interface ovn-hv2-0 ofport_request=4243
+OVS_WAIT_UNTIL([test 4243 = $(ovs-vsctl get Interface ovn-hv2-0 ofport)])
+OVS_WAIT_UNTIL([test 1 -le $(ovs-ofctl dump-flows br-int | \
+ grep -c 'in_port=4243')])
+check_controller_engine_stats hv1 non_vif_data recompute nocompute
+check_controller_engine_stats hv1 patch_port_data norecompute compute
+check_controller_engine_stats hv1 pflow_output recompute nocompute
+check_controller_engine_stats hv1 runtime_data recompute nocompute
+check_controller_engine_stats hv1 lflow_output recompute nocompute
+
+OVN_CLEANUP([hv1], [hv2])
+AT_CLEANUP
+])
+
OVN_FOR_EACH_NORTHD([
AT_SETUP([ovn-controller - localnet port change and chassisredirect bridged
redirect])
AT_KEYWORDS([ovn-localnet-cr-bridged])
diff --git a/tests/ovn-inc-proc-graph-dump.at b/tests/ovn-inc-proc-graph-dump.at
index 269766ded..b53ae3988 100644
--- a/tests/ovn-inc-proc-graph-dump.at
+++ b/tests/ovn-inc-proc-graph-dump.at
@@ -373,6 +373,10 @@ digraph "Incremental-Processing-Engine" {
lb_data -> lflow_output [[label="lflow_output_lb_data_handler"]];
SB_fdb -> lflow_output [[label="lflow_output_sb_fdb_handler"]];
SB_meter -> lflow_output [[label="lflow_output_sb_meter_handler"]];
+ patch_port_data [[style=filled, shape=box, fillcolor=white,
label="patch_port_data"]];
+ OVS_open_vswitch -> patch_port_data [[label=""]];
+ OVS_bridge -> patch_port_data [[label=""]];
+ OVS_interface -> patch_port_data
[[label="patch_port_data_ovs_iface_handler"]];
SB_sb_global [[style=filled, shape=box, fillcolor=white,
label="SB_sb_global"]];
northd_options [[style=filled, shape=box, fillcolor=white,
label="northd_options"]];
SB_sb_global -> northd_options
[[label="en_northd_options_sb_sb_global_handler"]];
@@ -419,6 +423,7 @@ digraph "Incremental-Processing-Engine" {
neighbor_exchange -> evpn_arp [[label=""]];
evpn_vtep_binding -> evpn_arp [[label="evpn_arp_vtep_binding_handler"]];
pflow_output [[style=filled, shape=box, fillcolor=white,
label="pflow_output"]];
+ patch_port_data -> pflow_output [[label=""]];
non_vif_data -> pflow_output [[label=""]];
northd_options -> pflow_output [[label=""]];
ct_zones -> pflow_output [[label="pflow_output_ct_zones_handler"]];
--
2.34.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
