On Fri, Aug 19, 2022 at 10:42 AM Han Zhou <hz...@ovn.org> wrote:
>
> On Thu, Aug 18, 2022 at 5:21 PM Han Zhou <hz...@ovn.org> 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>
> > ---
> > controller/binding.c | 37 +++++++++++++++++++++++++++++++++++++
> > 1 file changed, 37 insertions(+)
> >
> > diff --git a/controller/binding.c b/controller/binding.c
> > index efb747d52..9f04e6f35 100644
> > --- a/controller/binding.c
> > +++ b/controller/binding.c
> > @@ -2850,6 +2850,43 @@ delete_done:
> > sset_destroy(&postponed_ports);
> > cleanup_claimed_port_timestamps();
> >
> > + if (handled) {
> > + /* 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);
If you see the function add_local_datapath__() in
controller/local_data.c, we almost do
the same. i.e iterate through all the port bindings of the datapath.
Does it make sense to move the below code there ? If so, we could
avoid this loop.
Seems to me we can move the functions - update_ld_localnet_port(),
update_ld_external_ports() and
update_ld_multichassis_ports() to local_data.c
What do you think ?
Thanks
Numan
> > +
> > + 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) {
> > + update_ld_localnet_port(pb, &bridge_mappings,
> > + b_ctx_out->egress_ifaces,
> > + b_ctx_out->local_datapaths);
> > + } else if (lport_type == LP_EXTERNAL) {
> > + update_ld_external_ports(pb,
> b_ctx_out->local_datapaths);
> > + } else if (pb->n_additional_chassis) {
> > + 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,
> > --
> > 2.30.2
> >
>
> Oops, I missed destroying the index row. Please consider below incremental
> patch:
> ----- 8><
> ------------------------------------------------------------------------><8
> ----
> diff --git a/controller/binding.c b/controller/binding.c
> index 9f04e6f35..beb5c8e17 100644
> --- a/controller/binding.c
> +++ b/controller/binding.c
> @@ -2883,6 +2883,7 @@ delete_done:
>
> b_ctx_out->local_datapaths);
> }
> }
> + sbrec_port_binding_index_destroy_row(target);
> }
> shash_destroy(&bridge_mappings);
> }
> ------------------------------------------------------------------------------------------------
> Sorry for the mistake!
>
> Thanks,
> Han
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
