On Mon, May 17, 2021 at 8:37 AM Lorenzo Bianconi < [email protected]> wrote: > > Properly update logical/openflow flows for localport removing the > interface from the ovs bridge. Openflows in table 65 are not recomputed > removing a localport from an ovs-bridge and the ovs bridge ends-up with > a stale configuration adding the interface back. Fix the issue taking > care of localport special case in physical_handle_ovs_iface_changes > routine. > > Signed-off-by: Lorenzo Bianconi <[email protected]> > --- > Changes since v1: > - fix test cases > - add some some comment in physical_handle_ovs_iface_changes > --- > controller/ovn-controller.c | 1 + > controller/physical.c | 12 +++++++++++- > tests/ovn.at | 20 ++++++++++++++++++++ > 3 files changed, 32 insertions(+), 1 deletion(-) > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index b08db728e..031b275c8 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -1846,6 +1846,7 @@ en_physical_flow_changes_run(struct engine_node *node, void *data) > { > struct ed_type_pfc_data *pfc_tdata = data; > pfc_tdata->recompute_physical_flows = true; > + pfc_tdata->ovs_ifaces_changed = true; > engine_set_node_state(node, EN_UPDATED); > } > > diff --git a/controller/physical.c b/controller/physical.c > index 96c959d18..dbef96720 100644 > --- a/controller/physical.c > +++ b/controller/physical.c > @@ -1874,7 +1874,17 @@ physical_handle_ovs_iface_changes(struct physical_ctx *p_ctx, > const struct sbrec_port_binding *lb_pb = > local_binding_get_primary_pb(p_ctx->local_bindings, iface_id); > if (!lb_pb) { > - continue; > + /* For regular VIFs (e.g. lsp) the upcoming port-binding update > + * will remove lfows related to the unclaimed ovs port. > + * Localport is a special case and it needs to be managed here > + * since the port is not binded and otherwise the related lfows > + * will not be cleared removing the ovs port. > + */ > + lb_pb = lport_lookup_by_name(p_ctx->sbrec_port_binding_by_name, > + iface_id); > + if (!lb_pb || strcmp(lb_pb->type, "localport")) { > + continue; > + } > } > > int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; > diff --git a/tests/ovn.at b/tests/ovn.at > index dcf3e0e09..363bd36fb 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -11814,6 +11814,26 @@ for i in 1 2; do > done > done > > +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int |awk '/table=65/{print substr($8, 16, length($8))}' |sort -n], [0], [dnl > +10 > +11 > +]) > + > +# remove the localport from br-int and re-create it > +as hv1 > +check ovs-vsctl del-port vif01 > +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int |awk '/table=65/{print substr($8, 16, length($8))}' |sort -n], [0], [dnl > +11 > +]) > + > +as hv1 > +check ovs-vsctl add-port br-int vif01 \ > + -- set Interface vif01 external-ids:iface-id=lp01 > +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int |awk '/table=65/{print substr($8, 16, length($8))}' |sort -n], [0], [dnl > +2 > +11 > +]) > + > OVN_CLEANUP([hv1],[hv2]) > > AT_CLEANUP > -- > 2.31.1 >
Thanks Lorenzo! I applied this to master branch and backported to 21.03. Please let me know if it needs to be backported to older branches. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
