On Tue, Apr 23, 2024 at 7:54 AM Xavier Simonart <[email protected]> wrote:
>
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2023-May/405107.html
> Suggested-by: Priyankar Jain <[email protected]>
>
> Signed-off-by: Xavier Simonart <[email protected]>
> ---
>  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 <[email protected]>

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
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to