Re: [ovs-dev] [PATCH] ovn: Delete stale MAC_Bindings that result in Referential Integrity Violation

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

> From: Chandra Sekhar Vejendla/San Jose/IBM@IBMUS
> To: dev@openvswitch.org
> Date: 08/24/2016 02: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>
> ---

I've run the test without and with the ovn-northd.c patch and verified
that without the patch it fails, and with the patch it succeeds.

I've followed this up by stacking the patch with tip of the tree
openstack master and verified that without the patch, trying to clean
up the default router made by stacking does not clean up the
Datapath_Binding because of a stale MAC_Binding entry, while with the
patch, everything cleans up correctly.

Lastly, I've run the stock rally create-and-delete-routers job and verified
that all Datapath_Binding entries for routers created by rally are cleaned
up correctly (i.e. there are no stale MAC_Binding entries), so...

Acked-by: Ryan Moats <rmo...@us.ibm.com>
Tested-by: Ryan Moats <rmo...@us.ibm.com>

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovn: Delete stale MAC_Bindings that result in Referential Integrity Violation

2016-08-24 Thread Hui Kang 


"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();
>  }
>
> +/* 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, , );
>  build_ipam(ctx, , );
>  build_lflows(ctx, , );
> +cleanup_mac_bindings(ctx, );

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,
_port_binding_col_options);
>  add_column_noalert(ovnsb_idl_loop.idl, _port_binding_col_mac);
>  ovsdb_idl_add_column(ovnsb_idl_loop.idl,
> _port_binding_col_chassis);
> +ovsdb_idl_add_table(ovnsb_idl_loop.idl, _table_mac_binding);
> +add_column_noalert(ovnsb_idl_loop.idl,
_mac_binding_col_datapath);
> +add_column_noalert(ovnsb_idl_loop.idl, _mac_binding_col_ip);
> +add_column_noalert(ovnsb_idl_loop.idl, _mac_binding_col_mac);
> +add_column_noalert(ovnsb_idl_loop.idl,
> +   _mac_binding_col_logical_port);
>  ovsdb_idl_add_table(ovnsb_idl_loop.idl, _table_dhcp_options);
>  add_column_noalert(ovnsb_idl_loop.idl,
_dhcp_options_col_code);
>  add_column_noalert(ovnsb_idl_loop.idl,
_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

[ovs-dev] [PATCH] ovn: Delete stale MAC_Bindings that result in Referential Integrity Violation

2016-08-24 Thread Chandra S Vejendla 
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 
---
 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();
 }
 
+/* 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, , );
 build_ipam(ctx, , );
 build_lflows(ctx, , );
+cleanup_mac_bindings(ctx, );
 
 sync_address_sets(ctx);
 
@@ -4347,6 +4361,12 @@ main(int argc, char *argv[])
 add_column_noalert(ovnsb_idl_loop.idl, _port_binding_col_options);
 add_column_noalert(ovnsb_idl_loop.idl, _port_binding_col_mac);
 ovsdb_idl_add_column(ovnsb_idl_loop.idl, _port_binding_col_chassis);
+ovsdb_idl_add_table(ovnsb_idl_loop.idl, _table_mac_binding);
+add_column_noalert(ovnsb_idl_loop.idl, _mac_binding_col_datapath);
+add_column_noalert(ovnsb_idl_loop.idl, _mac_binding_col_ip);
+add_column_noalert(ovnsb_idl_loop.idl, _mac_binding_col_mac);
+add_column_noalert(ovnsb_idl_loop.idl,
+   _mac_binding_col_logical_port);
 ovsdb_idl_add_table(ovnsb_idl_loop.idl, _table_dhcp_options);
 add_column_noalert(ovnsb_idl_loop.idl, _dhcp_options_col_code);
 add_column_noalert(ovnsb_idl_loop.idl, _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