On Thu, May 30, 2024 at 11:31 AM Numan Siddique <num...@ovn.org> wrote:
>
> On Tue, Apr 23, 2024 at 7:54 AM Xavier Simonart <xsimo...@redhat.com> wrote:
> >
> > Reported-at:
> > https://mail.openvswitch.org/pipermail/ovs-dev/2023-May/405107.html
> > Suggested-by: Priyankar Jain <priyankar.j...@nutanix.com>
> >
> > Signed-off-by: Xavier Simonart <xsimo...@redhat.com>
> > ---
> > controller/binding.c | 12 +++++++++-
> > tests/ovn.at | 57 ++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 68 insertions(+), 1 deletion(-)
> >
> > diff --git a/controller/binding.c b/controller/binding.c
> > index 0bef5dc42..c37066cbe 100644
> > --- a/controller/binding.c
> > +++ b/controller/binding.c
> > @@ -1317,6 +1317,14 @@ lport_maybe_postpone(const char *port_name, long
> > long int now,
> > return true;
> > }
> >
> > +static bool
> > +is_postponed_port(const char *port_name)
> > +{
> > + struct claimed_port *cp =
> > + (struct claimed_port *) sset_find(&_postponed_ports, port_name);
> > + return !!cp;
> > +}
>
> This is wrong. '_postponed_ports' is an sset and not shash and
> sset_find returns 'struct sset_node'.
>
> You can use sset_contains i.,e
> --------------------
> static bool
> is_postponed_port(const char *port_name)
> {
> return sset_contains(&_postponed_ports, port_name);
> }
> --------------------
>
>
> With the above addressed:
>
> Acked-by: Numan Siddique <num...@ovn.org>
I applied this patch series to main with the above changes.
Numan
>
> Numan
>
>
> > +
> > /* Returns false if lport is not claimed due to 'sb_readonly'.
> > * Returns true otherwise.
> > */
> > @@ -1491,7 +1499,8 @@ is_binding_lport_this_chassis(struct binding_lport
> > *b_lport,
> > {
> > return (b_lport && b_lport->pb && chassis &&
> > (b_lport->pb->chassis == chassis
> > - || is_additional_chassis(b_lport->pb, chassis)));
> > + || is_additional_chassis(b_lport->pb, chassis)
> > + || is_postponed_port(b_lport->pb->logical_port)));
> > }
> >
> > /* Returns 'true' if the 'lbinding' has binding lports of type
> > LP_CONTAINER,
> > @@ -1593,6 +1602,7 @@ consider_vif_lport_(const struct sbrec_port_binding
> > *pb,
> > }
> >
> > if (!lbinding_set || !can_bind) {
> > + remove_related_lport(pb, b_ctx_out);
> > return release_lport(pb, b_ctx_in->chassis_rec,
> > !b_ctx_in->ovnsb_idl_txn,
> > b_ctx_out->tracked_dp_bindings,
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index 74c5bccc0..b0aba2207 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -37838,3 +37838,60 @@ OVN_CLEANUP([hv1])
> >
> > AT_CLEANUP
> > ])
> > +
> > +OVN_FOR_EACH_NORTHD([
> > +AT_SETUP([Deleting vif while controller fight for port claim])
> > +ovn_start
> > +
> > +ovn-nbctl ls-add ls0
> > +ovn-nbctl lsp-add ls0 lsp0
> > +ovn-nbctl lsp-add ls0 lsp1
> > +
> > +net_add n1
> > +for i in 1 2; do
> > + sim_add hv$i
> > + as hv$i
> > + ovs-vsctl add-br br-phys
> > + ovn_attach n1 br-phys 192.168.0.$i
> > +done
> > +
> > +check ovn-nbctl --wait=hv sync
> > +hv1_uuid=$(ovn-sbctl --bare --columns _uuid list chassis hv1)
> > +hv2_uuid=$(ovn-sbctl --bare --columns _uuid list chassis hv2)
> > +
> > +as hv1 ovs-vsctl -- add-port br-int vif1 -- set Interface vif1
> > external-ids:iface-id=lsp1
> > +as hv1 ovs-vsctl -- add-port br-int vif -- set Interface vif
> > external-ids:iface-id=lsp0
> > +wait_for_ports_up
> > +
> > +# Delete vif => store flows w/ only vif1, and no vif
> > +as hv1 ovs-vsctl -- del-port br-int vif
> > +check ovn-nbctl --wait=hv sync
> > +DUMP_FLOWS([hv1], [oflows1])
> > +sleep_controller hv1
> > +as hv2 ovs-vsctl -- add-port br-int vif -- set Interface vif
> > external-ids:iface-id=lsp0
> > +as hv1 ovs-vsctl -- add-port br-int vif -- set Interface vif
> > external-ids:iface-id=lsp0
> > +
> > +OVS_WAIT_UNTIL([
> > + chassis=$(ovn-sbctl --bare --columns chassis list port_binding lsp0)
> > + test "$chassis" = $hv2_uuid
> > +])
> > +
> > +sleep_sb
> > +wake_up_controller hv1
> > +sleep_controller hv2
> > +
> > +as hv1 ovs-vsctl -- del-port br-int vif
> > +wake_up_sb
> > +wake_up_controller hv2
> > +
> > +as hv2 ovs-vsctl -- del-port br-int vif
> > +check ovn-nbctl --wait=hv sync
> > +
> > +DUMP_FLOWS([hv1], [oflows2])
> > +
> > +check diff oflows1 oflows2
> > +OVN_CLEANUP([hv1],[hv2])
> > +
> > +AT_CLEANUP
> > +])
> > +
> > --
> > 2.31.1
> >
> > _______________________________________________
> > 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
