Hi Some tests are failing after applying this patch. - 686. ovn-performance.at:227: ovn-controller incremental processing This seems to fail as the test is expecting some lflow_run which your patch prevents. So, a simple test update is probably needed. - 262. system-ovn.at:20706: dynamic-routing - BGP learned routes This seems more serious. Note that it fails in all variations (system-test, system-test userspace, system-test-userspace-dpdk). It looks like your patch prevents route-exchange from adding route table watchers; the watchers were likely added previously through a recompute. Both fail in v2 & v3.
Thanks Xavier On Tue, Aug 4, 2026 at 9:56 PM Mark Michelson via dev < [email protected]> wrote: > 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 > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
