[ovs-dev] [PATCH v15 3/6] Convert binding_run to incremental processing.

Ryan Moats rmoats at us.ibm.com
Thu Apr 14 13:27:07 UTC 2016


From: RYAN D. MOATS <rmoats at us.ibm.com>

Ensure that the entire port binding table is processed
when chassis are added/removed or when get_local_iface_ids
finds new ports on the local vswitch.

Side effects:
  - Persist local_datapaths and patch_datapaths across runs so
    that changes to either can be used as a trigger to reset
    incremental flow processing.
  - Persist all_lports structure

Signed-off-by: RYAN D. MOATS <rmoats at us.ibm.com>
---
 ovn/controller/binding.c        |  140 ++++++++++++++++++++++++++++----------
 ovn/controller/binding.h        |    1 +
 ovn/controller/encaps.c         |    4 +
 ovn/controller/ovn-controller.c |   26 ++------
 ovn/controller/ovn-controller.h |    2 +
 ovn/controller/patch.c          |    3 +-
 6 files changed, 117 insertions(+), 59 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 32fcb85..7f8c26c 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -27,6 +27,16 @@
 
 VLOG_DEFINE_THIS_MODULE(binding);
 
+struct sset all_lports = SSET_INITIALIZER(&all_lports);
+
+bool process_full_binding = false;
+
+void
+reset_binding_processing(void)
+{
+    process_full_binding = true;
+}
+
 void
 binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
 {
@@ -72,6 +82,10 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int, struct shash *lports)
                 continue;
             }
             shash_add(lports, iface_id, iface_rec);
+            if (!sset_find(&all_lports, iface_id)) {
+                sset_add(&all_lports, iface_id);
+                reset_binding_processing();
+            }
         }
     }
 }
@@ -121,9 +135,42 @@ update_ct_zones(struct sset *lports, struct simap *ct_zones,
     }
 }
 
+/* Contains "struct local_datpath" nodes whose hash values are the
+ * row uuids of datapaths with at least one local port binding. */
+struct hmap local_datapaths_by_uuid =
+    HMAP_INITIALIZER(&local_datapaths_by_uuid);
+
+static struct local_datapath *
+local_datapath_lookup_by_uuid(const struct uuid *uuid)
+{
+    struct hmap_node *ld_node = hmap_first_with_hash(&local_datapaths_by_uuid,
+                                                     uuid_hash(uuid));
+    if (ld_node) {
+        return CONTAINER_OF(ld_node, struct local_datapath, uuid_hmap_node);
+    }
+    return NULL;
+}
+
+static void
+remove_local_datapath(struct hmap *local_datapaths, const struct uuid *uuid)
+{
+    struct local_datapath *ld = local_datapath_lookup_by_uuid(uuid);
+    if (ld) {
+        if (ld->logical_port) {
+            sset_find_and_delete(&all_lports, ld->logical_port);
+            free(ld->logical_port);
+        }
+        hmap_remove(local_datapaths, &ld->hmap_node);
+        hmap_remove(&local_datapaths_by_uuid, &ld->uuid_hmap_node);
+        free(ld);
+        //reset_flow_processing();
+    }
+}
+
 static void
 add_local_datapath(struct hmap *local_datapaths,
-        const struct sbrec_port_binding *binding_rec)
+        const struct sbrec_port_binding *binding_rec,
+        const struct uuid *uuid)
 {
     if (get_local_datapath(local_datapaths,
                            binding_rec->datapath->tunnel_key)) {
@@ -131,8 +178,12 @@ add_local_datapath(struct hmap *local_datapaths,
     }
 
     struct local_datapath *ld = xzalloc(sizeof *ld);
+    ld->logical_port = xstrdup(binding_rec->logical_port);
     hmap_insert(local_datapaths, &ld->hmap_node,
                 binding_rec->datapath->tunnel_key);
+    hmap_insert(&local_datapaths_by_uuid, &ld->uuid_hmap_node,
+                uuid_hash(uuid));
+    //reset_flow_processing();
 }
 
 static void
@@ -146,39 +197,14 @@ update_qos(const struct ovsrec_interface *iface_rec,
     ovsrec_interface_set_ingress_policing_burst(iface_rec, MAX(0, burst));
 }
 
-void
-binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
-            const char *chassis_id, struct simap *ct_zones,
-            unsigned long *ct_zone_bitmap, struct hmap *local_datapaths)
+static void
+consider_local_datapath(struct controller_ctx *ctx, struct shash *lports,
+                        const struct sbrec_chassis *chassis_rec,
+                        const struct sbrec_port_binding *binding_rec,
+                        struct hmap *local_datapaths)
 {
-    const struct sbrec_chassis *chassis_rec;
-    const struct sbrec_port_binding *binding_rec;
-
-    chassis_rec = get_chassis(ctx->ovnsb_idl, chassis_id);
-    if (!chassis_rec) {
-        return;
-    }
-
-    struct shash lports = SHASH_INITIALIZER(&lports);
-    if (br_int) {
-        get_local_iface_ids(br_int, &lports);
-    } else {
-        /* We have no integration bridge, therefore no local logical ports.
-         * We'll remove our chassis from all port binding records below. */
-    }
-
-    struct sset all_lports = SSET_INITIALIZER(&all_lports);
-    struct shash_node *node;
-    SHASH_FOR_EACH (node, &lports) {
-        sset_add(&all_lports, node->name);
-    }
-
-    /* Run through each binding record to see if it is resident on this
-     * chassis and update the binding accordingly.  This includes both
-     * directly connected logical ports and children of those ports. */
-    SBREC_PORT_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
         const struct ovsrec_interface *iface_rec
-            = shash_find_and_delete(&lports, binding_rec->logical_port);
+            = shash_find_and_delete(lports, binding_rec->logical_port);
         if (iface_rec
             || (binding_rec->parent_port && binding_rec->parent_port[0] &&
                 sset_contains(&all_lports, binding_rec->parent_port))) {
@@ -186,12 +212,13 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
                 /* Add child logical port to the set of all local ports. */
                 sset_add(&all_lports, binding_rec->logical_port);
             }
-            add_local_datapath(local_datapaths, binding_rec);
+            add_local_datapath(local_datapaths, binding_rec,
+                               &binding_rec->header_.uuid);
             if (iface_rec && ctx->ovs_idl_txn) {
                 update_qos(iface_rec, binding_rec);
             }
             if (binding_rec->chassis == chassis_rec) {
-                continue;
+                return;
             }
             if (ctx->ovnsb_idl_txn) {
                 if (binding_rec->chassis) {
@@ -219,16 +246,55 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
              * a patch port for each one. */
             sset_add(&all_lports, binding_rec->logical_port);
         }
+}
+
+void
+binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
+            const char *chassis_id, struct simap *ct_zones,
+            unsigned long *ct_zone_bitmap, struct hmap *local_datapaths)
+{
+    const struct sbrec_chassis *chassis_rec;
+    const struct sbrec_port_binding *binding_rec;
+
+    chassis_rec = get_chassis(ctx->ovnsb_idl, chassis_id);
+    if (!chassis_rec) {
+        return;
+    }
+
+    struct shash lports = SHASH_INITIALIZER(&lports);
+    if (br_int) {
+        get_local_iface_ids(br_int, &lports);
+    } else {
+        /* We have no integration bridge, therefore no local logical ports.
+         * We'll remove our chassis from all port binding records below. */
     }
 
-    SHASH_FOR_EACH (node, &lports) {
-        VLOG_DBG("No port binding record for lport %s", node->name);
+    /* Run through each binding record to see if it is resident on this
+     * chassis and update the binding accordingly.  This includes both
+     * directly connected logical ports and children of those ports. */
+    if (process_full_binding) {
+        SBREC_PORT_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
+            consider_local_datapath(ctx, &lports, chassis_rec, binding_rec,
+                                    local_datapaths);
+        }
+        process_full_binding = false;
+    } else {
+        SBREC_PORT_BINDING_FOR_EACH_TRACKED(binding_rec, ctx->ovnsb_idl) {
+            bool is_deleted = sbrec_port_binding_row_get_seqno(binding_rec,
+                OVSDB_IDL_CHANGE_DELETE) > 0;
+
+            if (is_deleted) {
+                remove_local_datapath(local_datapaths, &binding_rec->header_.uuid);
+                continue;
+            }
+            consider_local_datapath(ctx, &lports, chassis_rec, binding_rec,
+                                    local_datapaths);
+        }
     }
 
     update_ct_zones(&all_lports, ct_zones, ct_zone_bitmap);
 
     shash_destroy(&lports);
-    sset_destroy(&all_lports);
 }
 
 /* Returns true if the database is all cleaned up, false if more work is
diff --git a/ovn/controller/binding.h b/ovn/controller/binding.h
index 6e19c10..df41070 100644
--- a/ovn/controller/binding.h
+++ b/ovn/controller/binding.h
@@ -26,6 +26,7 @@ struct ovsrec_bridge;
 struct simap;
 
 void binding_register_ovs_idl(struct ovsdb_idl *);
+void reset_binding_processing(void);
 void binding_run(struct controller_ctx *, const struct ovsrec_bridge *br_int,
                  const char *chassis_id, struct simap *ct_zones,
                  unsigned long *ct_zone_bitmap, struct hmap *local_datapaths);
diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
index 0d64e6a..271ddef 100644
--- a/ovn/controller/encaps.c
+++ b/ovn/controller/encaps.c
@@ -15,6 +15,7 @@
 
 #include <config.h>
 #include "encaps.h"
+#include "binding.h"
 #include "lflow.h"
 
 #include "lib/hash.h"
@@ -217,6 +218,7 @@ tunnel_add(const struct sbrec_chassis *chassis_rec,
     free(port_name);
     free(ports);
     // reset_flow_processing();
+    reset_binding_processing();
     process_full_encaps = true;
 }
 
@@ -339,6 +341,8 @@ encaps_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
                     hmap_remove(&tc.tunnel_hmap_by_uuid,
                                 &port_hash->uuid_node);
                     free(port_hash);
+                    //reset_flow_processing();
+                    reset_binding_processing();
                 }
                 continue;
             }
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 16731a4..10fd774 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -243,6 +243,11 @@ get_ovnsb_remote_probe_interval(struct ovsdb_idl *ovs_idl, int *value)
     return false;
 }
 
+/* Contains "struct local_datpath" nodes whose hash values are the
+ * tunnel_key of datapaths with at least one local port binding. */
+struct hmap local_datapaths = HMAP_INITIALIZER(&local_datapaths);
+struct hmap patched_datapaths = HMAP_INITIALIZER(&patched_datapaths);
+
 int
 main(int argc, char *argv[])
 {
@@ -332,12 +337,6 @@ main(int argc, char *argv[])
             .ovnsb_idl_txn = ovsdb_idl_loop_run(&ovnsb_idl_loop),
         };
 
-        /* Contains "struct local_datpath" nodes whose hash values are the
-         * tunnel_key of datapaths with at least one local port binding. */
-        struct hmap local_datapaths = HMAP_INITIALIZER(&local_datapaths);
-
-        struct hmap patched_datapaths = HMAP_INITIALIZER(&patched_datapaths);
-
         const struct ovsrec_bridge *br_int = get_br_int(&ctx);
         const char *chassis_id = get_chassis_id(ctx.ovs_idl);
 
@@ -374,21 +373,6 @@ main(int argc, char *argv[])
             lport_index_destroy(&lports);
         }
 
-        struct local_datapath *cur_node, *next_node;
-        HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node, &local_datapaths) {
-            hmap_remove(&local_datapaths, &cur_node->hmap_node);
-            free(cur_node);
-        }
-        hmap_destroy(&local_datapaths);
-
-        struct patched_datapath *pd_cur_node, *pd_next_node;
-        HMAP_FOR_EACH_SAFE (pd_cur_node, pd_next_node, hmap_node,
-                &patched_datapaths) {
-            hmap_remove(&patched_datapaths, &pd_cur_node->hmap_node);
-            free(pd_cur_node);
-        }
-        hmap_destroy(&patched_datapaths);
-
         unixctl_server_run(unixctl);
 
         unixctl_server_wait(unixctl);
diff --git a/ovn/controller/ovn-controller.h b/ovn/controller/ovn-controller.h
index 9af7959..f0d7025 100644
--- a/ovn/controller/ovn-controller.h
+++ b/ovn/controller/ovn-controller.h
@@ -38,6 +38,8 @@ struct controller_ctx {
  * the localnet port */
 struct local_datapath {
     struct hmap_node hmap_node;
+    struct hmap_node uuid_hmap_node;
+    char *logical_port;
     const struct sbrec_port_binding *localnet_port;
 };
 
diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
index 4808146..e07e829 100644
--- a/ovn/controller/patch.c
+++ b/ovn/controller/patch.c
@@ -185,7 +185,8 @@ add_bridge_mappings(struct controller_ctx *ctx,
                  * to create patch ports for it. */
                 continue;
             }
-            if (ld->localnet_port) {
+            if (ld->localnet_port && strcmp(ld->localnet_port->logical_port,
+                                            binding->logical_port)) {
                 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
                 VLOG_WARN_RL(&rl, "localnet port '%s' already set for datapath "
                              "'%"PRId64"', skipping the new port '%s'.",
-- 
1.7.1




More information about the dev mailing list