We have seen our Cloud deployment and usage come to a halt after deleting a 
Logical Router, due the problem being addressed below. 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 (in our case OpenStack) from 
being committed to the Southbound DB (the northd keeps getting the violation 
for newer commits).

Another way to avoid the Referential Integrity Violation in this case is to 
make the datapath Column have a weak reference (default is strong) to the 
Datapath_Binding row and allow the column to be empty (min 0). We can probably 
add that on top of the current patch. I’m making that assumption that it won’t 
affect the conditional monitoring patch 
(791a7747427310f6a09c7b2f57a99d65338dfb45) which introduced the Data_Binding 
column strong reference.

@@ -1,7 +1,7 @@
 {
     "name": "OVN_Southbound",
     "version": "1.8.0",
-    "cksum": "59582657 7376",
+    "cksum": "362513609 7489",
     "tables": {
         "SB_Global": {
             "columns": {
@@ -127,7 +127,9 @@
                 "ip": {"type": "string"},
                 "mac": {"type": "string"},
                 "datapath": {"type": {"key": {"type": "uuid",
-                                              "refTable": 
"Datapath_Binding"}}}},
+                                              "refTable": "Datapath_Binding",
+                                              "refType": "weak"},
+                                      "min": 0}}},
             "indexes": [["logical_port", "ip"]],
             "isRoot": true},
         "DHCP_Options": {

Acked-By: Amitabha Biswas <abis...@us.ibm.com>

Thanks
Amitabha


> On Aug 16, 2016, at 12:27 PM, Chandra S Vejendla <csvej...@us.ibm.com> wrote:
> 
> Entries in MAC_Binding table are not deleted when the logical_ports
> referred to in MAC_Bindings are deleted. The patch fixes this by
> deleting the MAC_Binding entry when the logical_port is not found.
> 
> Signed-off-by: Chandra Sekhar Vejendla <csvej...@us.ibm.com>
> ---
> ovn/controller/lflow.c | 25 +++++++++++++++++--------
> tests/ovn.at           | 25 +++++++++++++++++++++++++
> 2 files changed, 42 insertions(+), 8 deletions(-)
> 
> diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
> index 341ca08..fbd36dc 100644
> --- a/ovn/controller/lflow.c
> +++ b/ovn/controller/lflow.c
> @@ -510,13 +510,11 @@ put_load(const uint8_t *data, size_t len,
> }
> 
> static void
> -consider_neighbor_flow(const struct lport_index *lports,
> +consider_neighbor_flow(const struct sbrec_port_binding *pb,
>                        const struct sbrec_mac_binding *b,
>                        struct ofpbuf *ofpacts_p,
>                        struct match *match_p)
> {
> -    const struct sbrec_port_binding *pb
> -        = lport_lookup_by_name(lports, b->logical_port);
>     if (!pb) {
>         return;
>     }
> @@ -571,10 +569,19 @@ add_neighbor_flows(struct controller_ctx *ctx,
>     match_init_catchall(&match);
>     ofpbuf_init(&ofpacts, 0);
> 
> -    const struct sbrec_mac_binding *b;
> +    const struct sbrec_port_binding *pb;
> +    const struct sbrec_mac_binding *b, *n;
>     if (full_neighbor_flow_processing) {
> -        SBREC_MAC_BINDING_FOR_EACH (b, ctx->ovnsb_idl) {
> -            consider_neighbor_flow(lports, b, &ofpacts, &match);
> +        SBREC_MAC_BINDING_FOR_EACH_SAFE (b, n, ctx->ovnsb_idl) {
> +            /* Remove mac binding record if the port binding for the logical
> +             * port in mac binding record is not found. */
> +            pb = lport_lookup_by_name(lports, b->logical_port);
> +            if (!pb && ctx->ovnsb_idl_txn) {
> +                ofctrl_remove_flows(&b->header_.uuid);
> +                sbrec_mac_binding_delete(b);
> +                continue;
> +            }
> +            consider_neighbor_flow(pb, b, &ofpacts, &match);
>         }
>         full_neighbor_flow_processing = false;
>     } else {
> @@ -585,11 +592,13 @@ add_neighbor_flows(struct controller_ctx *ctx,
>                 if (!sbrec_mac_binding_is_new(b)) {
>                     ofctrl_remove_flows(&b->header_.uuid);
>                 }
> -                consider_neighbor_flow(lports, b, &ofpacts, &match);
> +                pb = lport_lookup_by_name(lports, b->logical_port);
> +                if (pb) {
> +                    consider_neighbor_flow(pb, b, &ofpacts, &match);
> +                }
>             }
>         }
>     }
> -
>     ofpbuf_uninit(&ofpacts);
> }
> 
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 216bb07..dbb205c 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -4965,3 +4965,28 @@ 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 lp0
> +ovn-nbctl lsp-set-addresses lp0 "f0:00:00:00:00:01 192.168.0.1"
> +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="mac"
> +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], [0], [])
> +
> +OVN_CLEANUP([hv1])
> +
> +AT_CLEANUP
> -- 
> 2.6.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