Based on what I see in the patches and tests, it appears the concerns from v1 and v2 have been addressed in v3. I had a look and I can't see anything wrong with this. Thanks!
Acked-by: Mark Michelson <[email protected]> On Tue, Aug 4, 2026 at 6:51 AM Jun Gu <[email protected]> wrote: > > Patch (localnet / L2 gateway) port ofport changes force a full recompute > of the logical flow output, 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. > > - The patch ports that ovn-controller creates itself carry no > external_ids:iface-id, so their ofport changes hit the "unhandled" > branch of binding_handle_ovs_interface_changes(), recomputing > runtime_data and, transitively, lflow_output again. > > This is hit 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 the patch ofports into their own engine node > (en_patch_port_data) that feeds only the physical flow output, and skip > patch-type OVS interfaces that have no iface-id now and had none when > they were last seen in binding_handle_ovs_interface_changes(). A patch > port that a CMS backs a logical port with does carry an iface-id and is > still handled like any other VIF; a test covers that case. > > Tunnel interfaces are deliberately left alone in the latter fix, as > runtime_data's 'active_tunnels' has no incremental tracking and is only > recalculated on a full recompute. > > Skipping patch ports also exposes a pre-existing gap: unlike > binding_handle_port_binding_changes(), the OVS interface handler never > re-scans a newly-local datapath's sibling ports, which used to be masked > by the recompute this patch removes. Extract the existing catch-up logic > into catch_up_new_local_datapaths() and call it from both handlers. > > Assisted-by: Claude Opus 4.8, Claude Code > Signed-off-by: Jun Gu <[email protected]> > --- > controller/binding.c | 100 +++++++++++------ > controller/local_data.c | 180 ++++++++++++++++++++----------- > controller/local_data.h | 19 ++-- > controller/ovn-controller.c | 90 +++++++++++++--- > tests/ovn-controller.at | 127 ++++++++++++++++++++++ > tests/ovn-inc-proc-graph-dump.at | 5 + > 6 files changed, 403 insertions(+), 118 deletions(-) > > diff --git a/controller/binding.c b/controller/binding.c > index de51be823..46f87c43b 100644 > --- a/controller/binding.c > +++ b/controller/binding.c > @@ -2706,6 +2706,13 @@ is_iface_vif(const struct ovsrec_interface *iface_rec) > return true; > } > > +/* Returns true if 'iface_rec' is an OVS patch port. */ > +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 +2793,51 @@ ovs_interface_change_need_handle(const struct > ovsrec_interface *iface_rec, > return false; > } > > +/* Goes through each port_binding of the newly added local datapaths to > update > + * related local_datapaths if needed. Must be called by every incremental > + * handler that can add a local datapath, because the sibling ports > + * (localnet/external/vtep/multichassis) of such a datapath may already be in > + * the IDL and never show up again as their own tracked change. */ > +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 +2876,12 @@ 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)) { > + /* An OVS patch port without an iface-id, now or before: > + * nothing to claim or release here. Its ofport is tracked > + * by the patch_port_data engine node. */ > + 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 +2962,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 +3535,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..c66b2bf63 100644 > --- a/controller/local_data.h > +++ b/controller/local_data.h > @@ -159,13 +159,20 @@ 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. */ > +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. */ > +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..239deb6a1 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -3637,12 +3637,66 @@ 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. > - */ > -struct ed_type_non_vif_data { > +/* Engine node which handles the OVS patch ports (localnet / L2 gateway). > + * Kept separate from en_non_vif_data because patch ofports are consumed only > + * by the physical flow output. */ > +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. */ > +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 +3710,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 +3720,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 +3728,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 +3755,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 +3768,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 +4789,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 +4843,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 +6974,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 +7068,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 +7088,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..241d702f9 100644 > --- a/tests/ovn-controller.at > +++ b/tests/ovn-controller.at > @@ -1120,6 +1120,133 @@ 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 > + > +# Wait for the localnet patch port and the tunnel port to settle. > +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 not invalidate en_non_vif_data (and hence > +# en_lflow_output). > +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 leave en_patch_port_data alone. It > +# still recomputes runtime_data (and hence lflow_output), because tunnel > +# interfaces are not skipped by binding_handle_ovs_interface_changes(). > +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 - VIF backed by an OVS patch port]) > +AT_KEYWORDS([ovn-patch-port-data]) > + > +ovn_start > +net_add n1 > + > +sim_add hv1 > +as hv1 > +check ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.1 > + > +check ovn-nbctl ls-add ls0 \ > + -- lsp-add ls0 lsp0 \ > + -- lsp-set-addresses lsp0 "00:00:00:00:00:01 10.0.0.1" > + > +# An OVS patch port created by the CMS carries an iface-id, so it must be > +# claimed, followed and released just like any other VIF. > +check ovs-vsctl add-br br-cms > +check ovs-vsctl add-port br-int lsp0-int \ > + -- set Interface lsp0-int type=patch options:peer=lsp0-cms \ > + external_ids:iface-id=lsp0 \ > + -- add-port br-cms lsp0-cms \ > + -- set Interface lsp0-cms type=patch options:peer=lsp0-int > + > +wait_for_ports_up lsp0 > +check ovn-nbctl --wait=hv sync > + > +# Physical flows for a VIF get their ofport from the local binding, so their > +# presence proves the interface was not skipped. > +OVS_WAIT_UNTIL([test 1 -le $(ovs-vsctl get Interface lsp0-int ofport)]) > +ofport=$(ovs-vsctl get Interface lsp0-int ofport) > +OVS_WAIT_UNTIL([test 1 -le $(ovs-ofctl dump-flows br-int | \ > + grep -c "in_port=$ofport")]) > + > +# An ofport change of such an interface must be followed as well. > +check ovs-vsctl set Interface lsp0-int ofport_request=4242 > +OVS_WAIT_UNTIL([test 4242 = $(ovs-vsctl get Interface lsp0-int ofport)]) > +OVS_WAIT_UNTIL([test 1 -le $(ovs-ofctl dump-flows br-int | \ > + grep -c 'in_port=4242')]) > + > +# Clearing the iface-id must release the port binding. > +check ovs-vsctl remove Interface lsp0-int external_ids iface-id > +wait_column "" Port_Binding chassis logical_port=lsp0 > +wait_row_count nb:Logical_Switch_Port 1 up=false name=lsp0 > + > +# Setting it back must claim it again. > +check ovs-vsctl set Interface lsp0-int external_ids:iface-id=lsp0 > +wait_for_ports_up lsp0 > +OVS_WAIT_UNTIL([test 1 -le $(ovs-ofctl dump-flows br-int | \ > + grep -c 'in_port=4242')]) > + > +OVN_CLEANUP([hv1]) > +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 > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
