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 <hz...@ovn.org>
> ---
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?
> + 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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
