On Fri, May 27, 2022 at 3:38 AM Ales Musil <amu...@redhat.com> wrote:
>
> When localport is removed from NB, and it is the last port
> remaining on the host, it is not part of local datapath
> anymore. Which can cause troubles when there is recompute
> happening in between the removal from NB and the removal
> of interface from host. The localport would stay in lport_ids
> set, so any new localport that happens to have the same unique
> lport key wouldn't be initiliazed properly thorugh I-P.
>
> Remove the localport port binding from local datapath
> if it was part of the that said datapath before.
>
> Reported-at: https://bugzilla.redhat.com/2076604
> Signed-off-by: Ales Musil <amu...@redhat.com>

Thanks for fixing these issues.  I applied both the patches to the
main branch and backported upto branch-21.12

Numan

> ---
> v2: Add a test case for the scenario and rebase on newer main
> v3: Fix flakiness of the new test
> ---
>  controller/binding.c    |  9 +++++++
>  tests/ovn-controller.at | 56 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 65 insertions(+)
>
> diff --git a/controller/binding.c b/controller/binding.c
> index 92baebd29..a900c9a50 100644
> --- a/controller/binding.c
> +++ b/controller/binding.c
> @@ -2131,6 +2131,15 @@ handle_deleted_lport(const struct sbrec_port_binding 
> *pb,
>          return;
>      }
>
> +    /*
> +     * Remove localport that was part of local datapath that is not
> +     * considered to be local anymore.
> +     */
> +    if (!ld && !strcmp(pb->type, "localport") &&
> +        sset_find(&b_ctx_out->related_lports->lport_names, 
> pb->logical_port)) {
> +        remove_related_lport(pb, b_ctx_out);
> +    }
> +
>      /* If the binding is not local, if 'pb' is a L3 gateway port, we should
>       * remove its peer, if that one is local.
>       */
> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> index 3ca59f7e0..71463187b 100644
> --- a/tests/ovn-controller.at
> +++ b/tests/ovn-controller.at
> @@ -2186,3 +2186,59 @@ AT_CHECK([grep "Parsing of ovn-chassis-mac-mappings 
> failed" hv1/ovn-controller.l
>
>  OVN_CLEANUP([hv1])
>  AT_CLEANUP
> +
> +AT_SETUP([ovn-controller - localport can be recreated])
> +
> +ovn_start
> +
> +net_add n1
> +sim_add hv1
> +as hv1
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.1
> +
> +port_binding_cookie() {
> +    name=$1
> +    ovn-sbctl --bare --columns _uuid find port_binding logical_port=$name |\
> +    cut -d '-' -f 1 | tr -d '\n' | sed 's/^0\{0,8\}//'
> +}
> +
> +create_localport() {
> +    AT_CHECK([ovn-nbctl lsp-add ls0 metadata])
> +    AT_CHECK([ovn-nbctl lsp-set-type metadata localport])
> +    AT_CHECK([ovn-nbctl lsp-set-addresses metadata "00:00:00:00:10:25 
> 192.168.10.25"])
> +}
> +
> +bind_ports() {
> +    AT_CHECK([ovs-vsctl add-port br-int vm0 -- set interface vm0 
> type=internal external_ids:iface-id=vm0])
> +    AT_CHECK([ovs-vsctl add-port br-int metadata -- set interface metadata 
> type=internal external_ids:iface-id=metadata])
> +}
> +
> +# Create one VIF and localport and bind it to chassis
> +AT_CHECK([ovn-nbctl ls-add ls0])
> +AT_CHECK([ovn-nbctl lsp-add ls0 vm0])
> +AT_CHECK([ovn-nbctl lsp-set-addresses vm0 "00:00:00:00:10:10 192.168.10.10"])
> +create_localport
> +bind_ports
> +
> +# Check that localport has all physical flows defined
> +OVS_WAIT_UNTIL([test 6 = $(as hv1 ovs-ofctl dump-flows br-int | grep -c 
> $(port_binding_cookie metadata))])
> +
> +# Remove ls0 from local datapaths
> +AT_CHECK([ovs-vsctl del-port br-int vm0])
> +AT_CHECK([ovn-appctl inc-engine/recompute])
> +
> +# Check that localports physical flows are removed
> +OVS_WAIT_UNTIL([test 0 = $(as hv1 ovs-ofctl dump-flows br-int | grep -c 
> $(port_binding_cookie metadata))])
> +
> +# The order is impotant, if the port is removed first, the bug wouldn't be 
> triggered
> +AT_CHECK([ovn-nbctl lsp-del metadata])
> +AT_CHECK([ovs-vsctl del-port br-int metadata])
> +create_localport
> +bind_ports
> +
> +# Check that localport has all physical flows re-defined
> +OVS_WAIT_UNTIL([test 6 = $(as hv1 ovs-ofctl dump-flows br-int | grep -c 
> $(port_binding_cookie metadata))])
> +
> +OVN_CLEANUP([hv1])
> +AT_CLEANUP
> --
> 2.35.3
>
> _______________________________________________
> 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

Reply via email to