[ovs-dev] [PATCH ovn branch-20.06 1/2] binding: Fix container port removal from local bindings.

[Numan Siddique]

On Tue, Jan 19, 2021 at 7:04 PM Dumitru Ceara <dceara at redhat.com> wrote:

> When the Port_Binding associated to a container port is removed make
> sure we also remove it from the parent's 'children' shash.  Container
> ports don't have any VIFs associated so it's safe to destroy the
> container port local binding when the SB.Port_Binding is deleted.
>
> Signed-off-by: Dumitru Ceara <dceara at redhat.com>
>
> (cherry-picked from master commit 68cf9fdceba80ce0c03e3ddb3e0a5531f248fa04)
>

Thanks. I pushed both the patches in this series to branch-20.06.

Numan


> ---
>  controller/binding.c |   18 ++++++++++++++++-
>  controller/binding.h |    1 +
>  tests/ovn.at         |   53
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 71 insertions(+), 1 deletion(-)
>
> diff --git a/controller/binding.c b/controller/binding.c
> index e632203..3512a1d 100644
> --- a/controller/binding.c
> +++ b/controller/binding.c
> @@ -688,6 +688,7 @@ local_binding_add_child(struct local_binding *lbinding,
>                          struct local_binding *child)
>  {
>      local_binding_add(&lbinding->children, child);
> +    child->parent = lbinding;
>  }
>
>  static struct local_binding *
> @@ -697,6 +698,13 @@ local_binding_find_child(struct local_binding
> *lbinding,
>      return local_binding_find(&lbinding->children, child_name);
>  }
>
> +static void
> +local_binding_delete_child(struct local_binding *lbinding,
> +                           struct local_binding *child)
> +{
> +    shash_find_and_delete(&lbinding->children, child->name);
> +}
> +
>  static bool
>  is_lport_vif(const struct sbrec_port_binding *pb)
>  {
> @@ -2062,6 +2070,14 @@ handle_deleted_vif_lport(const struct
> sbrec_port_binding *pb,
>       * when the interface change happens. */
>      if (is_lport_container(pb)) {
>          remove_local_lports(pb->logical_port, b_ctx_out);
> +
> +        /* If the container port is removed we should also remove it from
> +         * its parent's children set.
> +         */
> +        if (lbinding->parent) {
> +            local_binding_delete_child(lbinding->parent, lbinding);
> +        }
> +        local_binding_destroy(lbinding);
>      }
>
>      handle_deleted_lport(pb, b_ctx_in, b_ctx_out);
> @@ -2147,7 +2163,7 @@ binding_handle_port_binding_changes(struct
> binding_ctx_in *b_ctx_in,
>          enum en_lport_type lport_type = get_lport_type(pb);
>          if (lport_type == LP_VIF || lport_type == LP_VIRTUAL) {
>              handled = handle_deleted_vif_lport(pb, lport_type, b_ctx_in,
> -                                                b_ctx_out);
> +                                               b_ctx_out);
>          } else {
>              handle_deleted_lport(pb, b_ctx_in, b_ctx_out);
>          }
> diff --git a/controller/binding.h b/controller/binding.h
> index c974056..2885971 100644
> --- a/controller/binding.h
> +++ b/controller/binding.h
> @@ -100,6 +100,7 @@ struct local_binding {
>
>      /* shash of 'struct local_binding' representing children. */
>      struct shash children;
> +    struct local_binding *parent;
>  };
>
>  static inline struct local_binding *
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 12b5828..812df1d 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -20110,6 +20110,59 @@ OVS_WAIT_UNTIL([test 0 = $(ovn-sbctl show | grep
> Port_Binding -c)], [0])
>  OVN_CLEANUP([hv1])
>  AT_CLEANUP
>
> +AT_SETUP([ovn -- Container port Incremental Processing])
> +ovn_start
> +
> +net_add n1
> +sim_add hv1
> +as hv1
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.10
> +
> +as hv1
> +ovs-vsctl \
> +    -- add-port br-int vif1 \
> +    -- set Interface vif1 external_ids:iface-id=lsp1 \
> +    ofport-request=1
> +
> +check ovn-nbctl ls-add ls1 \
> +    -- ls-add ls2 \
> +    -- lsp-add ls1 lsp1 \
> +    -- lsp-add ls2 lsp-cont1 lsp1 1
> +check ovn-nbctl --wait=hv sync
> +
> +# Wait for ports to be bound.
> +wait_row_count Chassis 1 name=hv1
> +ch=$(fetch_column Chassis _uuid name=hv1)
> +wait_row_count Port_Binding 1 logical_port=lsp1 chassis=$ch
> +wait_row_count Port_Binding 1 logical_port=lsp-cont1 chassis=$ch
> +
> +AS_BOX([delete OVS VIF and OVN container port])
> +as hv1 ovn-appctl -t ovn-controller debug/pause
> +as hv1 ovs-vsctl del-port vif1
> +
> +check ovn-nbctl --wait=sb lsp-del lsp-cont1
> +as hv1 ovn-appctl -t ovn-controller debug/resume
> +
> +check ovn-nbctl --wait=hv sync
> +check_row_count Port_Binding 1 logical_port=lsp1 chassis="[[]]"
> +
> +AS_BOX([readd OVS VIF])
> +as hv1
> +ovs-vsctl \
> +    -- add-port br-int vif1 \
> +    -- set Interface vif1 external_ids:iface-id=lsp1 \
> +    ofport-request=1
> +wait_row_count Port_Binding 1 logical_port=lsp1 chassis=$ch
> +
> +AS_BOX([readd OVN container port])
> +check ovn-nbctl lsp-add ls2 lsp-cont1 lsp1 1
> +check ovn-nbctl --wait=hv sync
> +check_row_count Port_Binding 1 logical_port=lsp-cont1 chassis=$ch
> +
> +OVN_CLEANUP([hv1])
> +AT_CLEANUP
> +
>  AT_SETUP([ovn -- normalized lrp-add])
>  ovn_start
>
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>