"dev" <dev-boun...@openvswitch.org> wrote on 08/24/2016 03:10:29 AM:

> From: Chandra Sekhar Vejendla/San Jose/IBM@IBMUS
> To: dev@openvswitch.org
> Date: 08/24/2016 03:11 AM
> Subject: [ovs-dev] [PATCH] ovn: Delete stale MAC_Bindings that
> result in Referential Integrity Violation
> Sent by: "dev" <dev-boun...@openvswitch.org>
>
> The MAC_Bindings have a strong reference to the Datapath_Binding. However
the
> MAC_Bindings are never deleted anywhere, and when the Datapath
(associated
> with a MAC_Binding) is deleted, the ovsdb-server returns Referential
> Integrity Violation. This prevents newer operations initiated from the
CMS
> from being committed to the Southbound DB.
>
> The patch fixes this  by deleting the MAC_Binding entry when the
> logical_port referred in the mac_binding entry is deleted.
>
> Signed-off-by: Chandra Sekhar Vejendla <csvej...@us.ibm.com>
> ---
>  ovn/northd/ovn-northd.c | 20 ++++++++++++++++++++
>  tests/ovn.at            | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 51 insertions(+)
>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 0874a9c..2e81390 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -3921,6 +3921,19 @@ build_lflows(struct northd_context *ctx,
> struct hmap *datapaths,
>      hmap_destroy(&mcgroups);
>  }
>
> +/* Remove mac_binding entries that refer to logical_ports which are
> + * deleted. */
> +static void
> +cleanup_mac_bindings(struct northd_context *ctx, struct hmap *ports)
> +{
> +    const struct sbrec_mac_binding *b, *n;
> +    SBREC_MAC_BINDING_FOR_EACH_SAFE (b, n, ctx->ovnsb_idl) {
> +        if (!ovn_port_find(ports, b->logical_port)) {
> +            sbrec_mac_binding_delete(b);
> +        }
> +    }
> +}
> +
>  /* OVN_Northbound and OVN_Southbound have an identical Address_Set
table.
>   * We always update OVN_Southbound to match the current data in
>   * OVN_Northbound, so that the address sets used in Logical_Flows in
> @@ -3969,6 +3982,7 @@ ovnnb_db_run(struct northd_context *ctx,
> struct ovsdb_idl_loop *sb_loop)
>      build_ports(ctx, &datapaths, &ports);
>      build_ipam(ctx, &datapaths, &ports);
>      build_lflows(ctx, &datapaths, &ports);
> +    cleanup_mac_bindings(ctx, &ports);

Hi, Chandra,
Should the function be called inside build_ports() for the sake the
cleanness?
In addition, all port processing in that function.

Thanks

- Hui

>
>      sync_address_sets(ctx);
>
> @@ -4347,6 +4361,12 @@ main(int argc, char *argv[])
>      add_column_noalert(ovnsb_idl_loop.idl,
&sbrec_port_binding_col_options);
>      add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_mac);
>      ovsdb_idl_add_column(ovnsb_idl_loop.idl,
> &sbrec_port_binding_col_chassis);
> +    ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_mac_binding);
> +    add_column_noalert(ovnsb_idl_loop.idl,
&sbrec_mac_binding_col_datapath);
> +    add_column_noalert(ovnsb_idl_loop.idl, &sbrec_mac_binding_col_ip);
> +    add_column_noalert(ovnsb_idl_loop.idl, &sbrec_mac_binding_col_mac);
> +    add_column_noalert(ovnsb_idl_loop.idl,
> +                       &sbrec_mac_binding_col_logical_port);
>      ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_dhcp_options);
>      add_column_noalert(ovnsb_idl_loop.idl,
&sbrec_dhcp_options_col_code);
>      add_column_noalert(ovnsb_idl_loop.idl,
&sbrec_dhcp_options_col_type);
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 216bb07..4804f35 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -4965,3 +4965,34 @@ cat packets
>  OVN_CLEANUP([hv1])
>
>  AT_CLEANUP
> +
> +AT_SETUP([ovn -- delete mac bindings])
> +AT_KEYWORDS([ovn])
> +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
> +# Create logical switch ls0
> +ovn-nbctl ls-add ls0
> +# Create port lp0 in ls0
> +ovn-nbctl lsp-add ls0 lp1
> +ovn-nbctl lsp-set-addresses lp0 "f0:00:00:00:00:01 192.168.0.1"
> +ovn-nbctl lsp-set-addresses lp1 "f0:00:00:00:00:02 192.168.0.2"
> +dp_uuid=`ovn-sbctl find datapath | grep uuid | cut -f2 -d ":" | cut
> -f2 -d " "`
> +ovn-sbctl create MAC_Binding ip=10.0.0.1 datapath=$dp_uuid
> logical_port=lp0 mac="mac1"
> +ovn-sbctl create MAC_Binding ip=10.0.0.1 datapath=$dp_uuid
> logical_port=lp1 mac="mac2"
> +ovn-sbctl find MAC_Binding
> +#Delete port lp0
> +ovn-nbctl lsp-del lp0
> +ovn-sbctl find MAC_Binding
> +AT_CHECK([ovn-sbctl find MAC_Binding logical_port=lp0], [0], [])
> +#Delete ls0
> +ovn-nbctl ls-del ls0
> +ovn-sbctl find MAC_Binding
> +AT_CHECK([ovn-sbctl find MAC_Binding], [0], [])
> +
> +OVN_CLEANUP([hv1])
> +
> +AT_CLEANUP
> --
> 1.9.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to