On Wed, Aug 17, 2022 at 8:27 AM Dumitru Ceara <[email protected]> wrote: > > On 7/25/22 23:34, Han Zhou wrote: > > When handling port_binding changes, it is possible that new > > local_datapaths are added, and the fields of the local_datapath, such as > > localnet_port, external_ports, etc. need to be updated for the newly > > added local_datapaths. > > > > This problem doesn't happen in most cases because the changes that > > trigger the new local_datapath addition usually trigger recomputes, > > but it may not always be the case. If recompute is not triggered, > > local_datapaths are not properly updated and will result in missing OVS > > flows. This is more likely to happen when we delay patch interface > > deletion in the future, or if we implement patch interface I-P. > > > > Signed-off-by: Han Zhou <[email protected]> > > --- > > Hi Han, > > > controller/binding.c | 31 ++++++++++++++++++++----------- > > 1 file changed, 20 insertions(+), 11 deletions(-) > > > > diff --git a/controller/binding.c b/controller/binding.c > > index ba803ae3e..52661c07a 100644 > > --- a/controller/binding.c > > +++ b/controller/binding.c > > @@ -2663,7 +2663,6 @@ delete_done: > > case LP_VIF: > > case LP_CONTAINER: > > case LP_VIRTUAL: > > - update_ld_multichassis_ports(pb, b_ctx_out->local_datapaths); > > handled = handle_updated_vif_lport(pb, lport_type, b_ctx_in, > > b_ctx_out, qos_map_ptr); > > break; > > @@ -2721,21 +2720,11 @@ delete_done: > > > > case LP_EXTERNAL: > > handled = consider_external_lport(pb, b_ctx_in, b_ctx_out); > > - update_ld_external_ports(pb, b_ctx_out->local_datapaths); > > break; > > > > case LP_LOCALNET: { > > consider_localnet_lport(pb, b_ctx_in, b_ctx_out, qos_map_ptr); > > > > - struct shash bridge_mappings = > > - SHASH_INITIALIZER(&bridge_mappings); > > - add_ovs_bridge_mappings(b_ctx_in->ovs_table, > > - b_ctx_in->bridge_table, > > - &bridge_mappings); > > - update_ld_localnet_port(pb, &bridge_mappings, > > - b_ctx_out->egress_ifaces, > > - b_ctx_out->local_datapaths); > > - shash_destroy(&bridge_mappings); > > break; > > } > > > > @@ -2749,6 +2738,26 @@ delete_done: > > } > > } > > > > + if (handled) { > > + /* There may be new local_datapaths added by the above handling, so go > > + * through each port_binding 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); > > + SBREC_PORT_BINDING_TABLE_FOR_EACH (pb, > > + b_ctx_in->port_binding_table) { > > This seems quite expensive. In a large scale deployment there can be > tens of thousands of ports. Can we instead just walk the port bindings > corresponding to datapaths that were added locally?
Thanks Dumitru. I fixed this in V3: https://patchwork.ozlabs.org/project/ovn/list/?series=314426 Han > > > + update_ld_localnet_port(pb, &bridge_mappings, > > + b_ctx_out->egress_ifaces, > > + b_ctx_out->local_datapaths); > > + update_ld_external_ports(pb, b_ctx_out->local_datapaths); > > + update_ld_multichassis_ports(pb, b_ctx_out->local_datapaths); > > + } > > + shash_destroy(&bridge_mappings); > > + } > > + > > if (handled && qos_map_ptr && set_noop_qos(b_ctx_in->ovs_idl_txn, > > b_ctx_in->port_table, > > b_ctx_in->qos_table, > > Thanks, > Dumitru > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
