[ovs-dev] [PATCH v2] ovn: Delete stale MAC_Binding records

Amitabha Biswas azbiswas at gmail.com
Sat Aug 20 05:14:36 UTC 2016


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 <abiswas at us.ibm.com>

Thanks
Amitabha


> On Aug 16, 2016, at 12:27 PM, Chandra S Vejendla <csvejend at 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 <csvejend at 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 at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev




More information about the dev mailing list