On Tue, Apr 23, 2024 at 7:54 AM Xavier Simonart <[email protected]> wrote: > > When a port was claimed by two chassis, both chassis were fighting for the > port; > a claim was postponed if the port was claimed recently. > However, there were two issues: > - Not all the flows were properly removed when the remote chassis was claiming > the port. > - A port was not properly released if claim was postponed when the > port_binding > was deleted. > > There were also multiple cases causing ovn-controller to always wake-up > immediately as a port was not removed from postponed_port list. > - Changing port type to localport while port claim was postponed. > - Deleting parent of a container port while parent was postponed. > > Signed-off-by: Xavier Simonart <[email protected]>
Acked-by: Numan Siddique <[email protected]> Numan > --- > controller/binding.c | 29 +++++++++++++++++++++++++++-- > tests/ovn.at | 10 ++++++++++ > 2 files changed, 37 insertions(+), 2 deletions(-) > > diff --git a/controller/binding.c b/controller/binding.c > index 1499ceae1..0bef5dc42 100644 > --- a/controller/binding.c > +++ b/controller/binding.c > @@ -1270,6 +1270,18 @@ update_port_additional_encap_if_needed( > return true; > } > > +static bool > +is_requested_additional_chassis(const struct sbrec_port_binding *pb, > + const struct sbrec_chassis *chassis_rec) > +{ > + for (size_t i = 0; i < pb->n_requested_additional_chassis; i++) { > + if (pb->requested_additional_chassis[i] == chassis_rec) { > + return true; > + } > + } > + return false; > +} > + > bool > is_additional_chassis(const struct sbrec_port_binding *pb, > const struct sbrec_chassis *chassis_rec) > @@ -1587,6 +1599,15 @@ consider_vif_lport_(const struct sbrec_port_binding > *pb, > b_ctx_out->if_mgr); > } > } > + if (pb->chassis != b_ctx_in->chassis_rec > + && !is_requested_additional_chassis(pb, b_ctx_in->chassis_rec) > + && if_status_is_port_claimed(b_ctx_out->if_mgr, > + pb->logical_port)) { > + update_lport_tracking(pb, b_ctx_out->tracked_dp_bindings, false); > + if_status_mgr_remove_ovn_installed(b_ctx_out->if_mgr, > + b_lport->lbinding->iface->name, > + &b_lport->lbinding->iface->header_.uuid); > + } > > return true; > } > @@ -1787,7 +1808,8 @@ consider_localport(const struct sbrec_port_binding *pb, > struct shash *local_bindings = &b_ctx_out->lbinding_data->bindings; > struct local_binding *lbinding = local_binding_find(local_bindings, > pb->logical_port); > - > + /* Make sure there is no previous postponed port claim */ > + sset_find_and_delete(b_ctx_out->postponed_ports, pb->logical_port); > if (!lbinding) { > return true; > } > @@ -2754,11 +2776,14 @@ handle_deleted_vif_lport(const struct > sbrec_port_binding *pb, > binding_lport_delete(binding_lports, b_lport); > } > > - if (bound && lbinding && lport_type == LP_VIF) { > + if ((lbinding && lport_type == LP_VIF) && > + (bound || sset_find_and_delete(b_ctx_out->postponed_ports, > + pb->logical_port))) { > /* We need to release the container/virtual binding lports (if any) > if > * deleted 'pb' type is LP_VIF. */ > struct binding_lport *c_lport; > LIST_FOR_EACH (c_lport, list_node, &lbinding->binding_lports) { > + sset_find_and_delete(b_ctx_out->postponed_ports, c_lport->name); > remove_local_lports(c_lport->pb->logical_port, b_ctx_out); > if (!release_binding_lport(b_ctx_in->chassis_rec, c_lport, > !b_ctx_in->ovnsb_idl_txn, > diff --git a/tests/ovn.at b/tests/ovn.at > index b68678472..74c5bccc0 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -16413,6 +16413,10 @@ ovn_start > > ovn-nbctl ls-add ls0 > ovn-nbctl lsp-add ls0 lsp0 > +ovn-nbctl lsp-add ls0 lsp1 > +ovn-nbctl lsp-add ls0 lsp2 > +check ovn-nbctl lsp-add ls0 lsp-cont1 lsp0 1 > + > > net_add n1 > for i in 1 2; do > @@ -16426,6 +16430,8 @@ for i in 1 2; do > as hv$i > ovs-vsctl -- add-port br-int vif \ > -- set Interface vif external-ids:iface-id=lsp0 > + ovs-vsctl -- add-port br-int vif$i \ > + -- set Interface vif$i external-ids:iface-id=lsp$i > done > > # give controllers some time to fight for the port binding > @@ -16443,6 +16449,10 @@ max_claims=20 > AT_CHECK([test "${hv1_claims}" -le "${max_claims}"], [0], []) > AT_CHECK([test "${hv2_claims}" -le "${max_claims}"], [0], []) > > +check ovn-nbctl --wait=hv lsp-del lsp0 > +CHECK_FLOWS_AFTER_RECOMPUTE([hv1], [hv1]) > +CHECK_FLOWS_AFTER_RECOMPUTE([hv2], [hv2]) > + > OVN_CLEANUP([hv1],[hv2]) > > AT_CLEANUP > -- > 2.31.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
