[ovs-dev] [patch_v1] ovn: Add datapaths of interest filtering (RFC).

Darrell Ball dlu998 at gmail.com
Fri Jul 29 08:46:09 UTC 2016


This patch adds datapaths of interest support where only datapaths of
local interest are monitored by the ovn-controller ovsdb client.  The
idea is to do a flood fill in ovn-controller of datapath associations
calculated by northd. A new column is added to the SB database
datapath_binding table - related_datapaths to facilitate this.  This
allows monitoring to adapt quickly with a single new monitor setting for
all datapaths of interest locally.

Presently, the code does not cleanup logical port monitor contexts
as some refactoring may need to be done given several recent
changes in this area.

Signed-off-by: Darrell Ball <dlu998 at gmail.com>
---
 ovn/controller/binding.c        |  21 +++++---
 ovn/controller/ovn-controller.c | 116 ++++++++++++++++++++++++++++++++++++++--
 ovn/controller/ovn-controller.h |   8 +++
 ovn/controller/patch.c          |  19 +++++--
 ovn/northd/ovn-northd.c         |  74 +++++++++++++++++++++++++
 ovn/ovn-sb.ovsschema            |  11 ++--
 ovn/ovn-sb.xml                  |   9 ++++
 7 files changed, 239 insertions(+), 19 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 41165bc..84f4740 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -66,7 +66,8 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
 
 static bool
 get_local_iface_ids(const struct ovsrec_bridge *br_int,
-                    struct shash *lport_to_iface)
+                    struct shash *lport_to_iface,
+                    struct controller_ctx *ctx)
 {
     int i;
     bool changed = false;
@@ -92,6 +93,7 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int,
                 continue;
             }
             shash_add(lport_to_iface, iface_id, iface_rec);
+            sbrec_port_binding_add_clause_logical_port(ctx->ovnsb_idl, OVSDB_F_EQ, iface_id);
             if (!sset_find_and_delete(&old_local_ids, iface_id)) {
                 sset_add(&local_ids, iface_id);
                 changed = true;
@@ -128,20 +130,23 @@ local_datapath_lookup_by_uuid(struct hmap *hmap_p, const struct uuid *uuid)
 }
 
 static void
-remove_local_datapath(struct hmap *local_datapaths, struct local_datapath *ld)
+remove_local_datapath(struct hmap *local_datapaths, struct local_datapath *ld,
+                      struct controller_ctx *ctx)
 {
     if (ld->logical_port) {
         free(ld->logical_port);
         ld->logical_port = NULL;
     }
     hmap_remove(local_datapaths, &ld->hmap_node);
+    remove_datapath_of_interest(ctx, ld->dp);
     free(ld);
     lflow_reset_processing();
 }
 
 static void
 add_local_datapath(struct hmap *local_datapaths,
-        const struct sbrec_port_binding *binding_rec)
+        const struct sbrec_port_binding *binding_rec,
+		struct controller_ctx *ctx)
 {
     if (get_local_datapath(local_datapaths,
                            binding_rec->datapath->tunnel_key)) {
@@ -150,9 +155,11 @@ add_local_datapath(struct hmap *local_datapaths,
 
     struct local_datapath *ld = xzalloc(sizeof *ld);
     ld->logical_port = xstrdup(binding_rec->logical_port);
+    ld->dp = binding_rec->datapath;
     memcpy(&ld->uuid, &binding_rec->header_.uuid, sizeof ld->uuid);
     hmap_insert(local_datapaths, &ld->hmap_node,
                 binding_rec->datapath->tunnel_key);
+    do_datapath_flood_fill(ctx, binding_rec->datapath);
     lport_index_reset();
     mcgroup_index_reset();
     lflow_reset_processing();
@@ -182,7 +189,7 @@ consider_local_datapath(struct controller_ctx *ctx,
     if (iface_rec
         || (binding_rec->parent_port && binding_rec->parent_port[0] &&
             sset_contains(&local_ids, binding_rec->parent_port))) {
-        add_local_datapath(local_datapaths, binding_rec);
+        add_local_datapath(local_datapaths, binding_rec, ctx);
         if (iface_rec && ctx->ovs_idl_txn) {
             update_qos(iface_rec, binding_rec);
         }
@@ -221,7 +228,7 @@ consider_local_datapath(struct controller_ctx *ctx,
             VLOG_INFO("Claiming l2gateway port %s for this chassis.",
                       binding_rec->logical_port);
             sbrec_port_binding_set_chassis(binding_rec, chassis_rec);
-            add_local_datapath(local_datapaths, binding_rec);
+            add_local_datapath(local_datapaths, binding_rec, ctx);
         }
     } else if (chassis_rec && binding_rec->chassis == chassis_rec
                && strcmp(binding_rec->type, "l3gateway")) {
@@ -247,7 +254,7 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
     }
 
     if (br_int) {
-        if (ctx->ovnsb_idl_txn && get_local_iface_ids(br_int, &lport_to_iface)) {
+        if (ctx->ovnsb_idl_txn && get_local_iface_ids(br_int, &lport_to_iface, ctx)) {
             process_full_binding = true;
         }
     } else {
@@ -274,7 +281,7 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
         HMAP_FOR_EACH_SAFE (old_ld, next, hmap_node, local_datapaths) {
             if (!local_datapath_lookup_by_uuid(&keep_local_datapath_by_uuid,
                                                &old_ld->uuid)) {
-                remove_local_datapath(local_datapaths, old_ld);
+                remove_local_datapath(local_datapaths, old_ld, ctx);
             }
         }
         hmap_destroy(&keep_local_datapath_by_uuid);
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 5c74186..d043e24 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -69,6 +69,117 @@ OVS_NO_RETURN static void usage(void);
 
 static char *ovs_remote;
 
+struct dp_of_interest_node {
+    struct hmap_node hmap_node;
+    const struct sbrec_datapath_binding *sb_db;
+};
+
+/* Contains "struct local_datapath" nodes whose hash values are the
+ * tunnel_key of datapaths with at least one local port binding. */
+static struct hmap local_datapaths = HMAP_INITIALIZER(&local_datapaths);
+static struct hmap patched_datapaths = HMAP_INITIALIZER(&patched_datapaths);
+
+static struct hmap dps_of_interest = HMAP_INITIALIZER(&dps_of_interest);
+
+static bool
+add_datapath_of_interest(struct controller_ctx *ctx,
+                         const struct sbrec_datapath_binding *dp)
+{
+
+    struct dp_of_interest_node *node;
+    HMAP_FOR_EACH_WITH_HASH (node, hmap_node, uuid_hash(&dp->header_.uuid),
+                             &dps_of_interest) {
+        if (uuid_equals(&node->sb_db->header_.uuid, &dp->header_.uuid)) {
+            return false;
+        }
+    }
+
+    if (!get_local_datapath(&local_datapaths, dp->tunnel_key) &&
+        !get_patched_datapath(&patched_datapaths, dp->tunnel_key)){
+
+        return false;
+    }
+    sbrec_port_binding_add_clause_datapath(ctx->ovnsb_idl, OVSDB_F_EQ, dp);
+    sbrec_logical_flow_add_clause_logical_datapath(ctx->ovnsb_idl, OVSDB_F_EQ, dp);
+    sbrec_mac_binding_add_clause_datapath(ctx->ovnsb_idl, OVSDB_F_EQ, dp);
+    sbrec_multicast_group_add_clause_datapath(ctx->ovnsb_idl, OVSDB_F_EQ, dp);
+
+    node = xzalloc(sizeof *node);
+    hmap_insert(&dps_of_interest, &node->hmap_node,
+                uuid_hash(&dp->header_.uuid));
+    node->sb_db = dp;
+
+    return true;
+}
+
+void
+remove_datapath_of_interest(struct controller_ctx *ctx,
+                            const struct sbrec_datapath_binding *dp)
+{
+    struct dp_of_interest_node *node;
+    HMAP_FOR_EACH_WITH_HASH (node, hmap_node, uuid_hash(&dp->header_.uuid),
+                             &dps_of_interest) {
+        if (uuid_equals(&node->sb_db->header_.uuid, &dp->header_.uuid)) {
+            if (!get_local_datapath(&local_datapaths, dp->tunnel_key) &&
+                !get_patched_datapath(&patched_datapaths, dp->tunnel_key)){
+
+                sbrec_port_binding_remove_clause_datapath(ctx->ovnsb_idl,
+                                                          OVSDB_F_EQ, dp);
+                sbrec_logical_flow_remove_clause_logical_datapath(ctx->ovnsb_idl,
+                                                                  OVSDB_F_EQ, dp);
+                sbrec_mac_binding_remove_clause_datapath(ctx->ovnsb_idl,
+                                                         OVSDB_F_EQ, dp);
+                sbrec_multicast_group_remove_clause_datapath(ctx->ovnsb_idl,
+                                                             OVSDB_F_EQ, dp);
+
+                hmap_remove(&dps_of_interest, &node->hmap_node);
+                free(node);
+                return;
+            }
+        }
+    }
+}
+
+void
+do_datapath_flood_fill(struct controller_ctx *ctx,
+                       const struct sbrec_datapath_binding *dp_start)
+{
+	struct interest_dps_list_elem {
+		struct ovs_list list_node;
+		const struct sbrec_datapath_binding * dp;
+	};
+
+	struct ovs_list interest_datapaths;
+	ovs_list_init(&interest_datapaths);
+
+	struct interest_dps_list_elem *dp_list_elem =
+        xzalloc(sizeof *dp_list_elem);
+	dp_list_elem->dp = dp_start;
+	ovs_list_push_back(&interest_datapaths, &dp_list_elem->list_node);
+
+	while (!ovs_list_is_empty(&interest_datapaths)) {
+
+		struct ovs_list *list_node_ptr =
+            ovs_list_pop_front(&interest_datapaths);
+		dp_list_elem = CONTAINER_OF(list_node_ptr,
+            struct interest_dps_list_elem, list_node);
+
+		size_t num_related_datapaths = dp_list_elem->dp->n_related_datapaths;
+		struct sbrec_datapath_binding **related_datapaths =
+            dp_list_elem->dp->related_datapaths;
+		if (!add_datapath_of_interest(ctx, dp_list_elem->dp)) {
+			free(dp_list_elem);
+			continue;
+		}
+		free(dp_list_elem);
+		for (int i = 0; i < num_related_datapaths; i++) {
+			dp_list_elem = xzalloc(sizeof *dp_list_elem);
+			dp_list_elem->dp = related_datapaths[i];
+			ovs_list_push_back(&interest_datapaths, &dp_list_elem->list_node);
+		}
+	}
+}
+
 struct local_datapath *
 get_local_datapath(const struct hmap *local_datapaths, uint32_t tunnel_key)
 {
@@ -306,11 +417,6 @@ get_nb_cfg(struct ovsdb_idl *idl)
     return sb ? sb->nb_cfg : 0;
 }
 
-/* Contains "struct local_datapath" nodes whose hash values are the
- * tunnel_key of datapaths with at least one local port binding. */
-static struct hmap local_datapaths = HMAP_INITIALIZER(&local_datapaths);
-static struct hmap patched_datapaths = HMAP_INITIALIZER(&patched_datapaths);
-
 static struct lport_index lports;
 static struct mcgroup_index mcgroups;
 
diff --git a/ovn/controller/ovn-controller.h b/ovn/controller/ovn-controller.h
index 470b1f5..2fe4f0c 100644
--- a/ovn/controller/ovn-controller.h
+++ b/ovn/controller/ovn-controller.h
@@ -42,6 +42,7 @@ struct local_datapath {
     struct uuid uuid;
     char *logical_port;
     const struct sbrec_port_binding *localnet_port;
+    const struct sbrec_datapath_binding *dp;
 };
 
 struct local_datapath *get_local_datapath(const struct hmap *,
@@ -52,6 +53,7 @@ struct local_datapath *get_local_datapath(const struct hmap *,
 struct patched_datapath {
     struct hmap_node hmap_node;
     char *key;  /* Holds the uuid of the corresponding datapath. */
+    const struct sbrec_datapath_binding *dp;
     bool local; /* 'True' if the datapath is for gateway router. */
     bool stale; /* 'True' if the datapath is not referenced by any patch
                  * port. */
@@ -66,6 +68,12 @@ const struct ovsrec_bridge *get_bridge(struct ovsdb_idl *,
 const struct sbrec_chassis *get_chassis(struct ovsdb_idl *,
                                         const char *chassis_id);
 
+void do_datapath_flood_fill(struct controller_ctx *,
+                            const struct sbrec_datapath_binding *);
+
+void remove_datapath_of_interest(struct controller_ctx *ctx,
+                                 const struct sbrec_datapath_binding *);
+
 /* Must be a bit-field ordered from most-preferred (higher number) to
  * least-preferred (lower number). */
 enum chassis_tunnel_type {
diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
index 012e6ba..23f6cc5 100644
--- a/ovn/controller/patch.c
+++ b/ovn/controller/patch.c
@@ -258,7 +258,9 @@ add_bridge_mappings(struct controller_ctx *ctx,
 
 static void
 add_patched_datapath(struct hmap *patched_datapaths,
-                     const struct sbrec_port_binding *binding_rec, bool local)
+                     const struct sbrec_port_binding *binding_rec,
+                     bool local,
+					 struct controller_ctx *ctx)
 {
     struct patched_datapath *pd = get_patched_datapath(patched_datapaths,
                                        binding_rec->datapath->tunnel_key);
@@ -273,9 +275,11 @@ add_patched_datapath(struct hmap *patched_datapaths,
     pd->local = local;
     pd->key = xasprintf(UUID_FMT,
                         UUID_ARGS(&binding_rec->datapath->header_.uuid));
+    pd->dp = binding_rec->datapath;
     /* stale is set to false. */
     hmap_insert(patched_datapaths, &pd->hmap_node,
                 binding_rec->datapath->tunnel_key);
+    do_datapath_flood_fill(ctx, binding_rec->datapath);
 }
 
 static void
@@ -292,7 +296,8 @@ add_logical_patch_ports_preprocess(struct hmap *patched_datapaths)
 /* This function should cleanup stale patched datapaths and any memory
  * allocated for fields within a stale patched datapath. */
 static void
-add_logical_patch_ports_postprocess(struct hmap *patched_datapaths)
+add_logical_patch_ports_postprocess(struct hmap *patched_datapaths,
+                                    struct controller_ctx *ctx)
 {
     /* Clean up stale patched datapaths. */
     struct patched_datapath *pd_cur_node, *pd_next_node;
@@ -300,6 +305,7 @@ add_logical_patch_ports_postprocess(struct hmap *patched_datapaths)
                         patched_datapaths) {
         if (pd_cur_node->stale == true) {
             hmap_remove(patched_datapaths, &pd_cur_node->hmap_node);
+            remove_datapath_of_interest(ctx, pd_cur_node->dp);
             free(pd_cur_node->key);
             free(pd_cur_node);
         }
@@ -368,15 +374,20 @@ add_logical_patch_ports(struct controller_ctx *ctx,
                               existing_ports);
             free(dst_name);
             free(src_name);
-            add_patched_datapath(patched_datapaths, binding, local_port);
+            add_patched_datapath(patched_datapaths, binding, local_port, ctx);
             if (local_port) {
                 if (binding->chassis != chassis_rec && ctx->ovnsb_idl_txn) {
                     sbrec_port_binding_set_chassis(binding, chassis_rec);
                 }
             }
+            sbrec_port_binding_add_clause_logical_port(ctx->ovnsb_idl,
+                                                       OVSDB_F_EQ, local);
+            sbrec_port_binding_add_clause_logical_port(ctx->ovnsb_idl,
+                                                       OVSDB_F_EQ, peer);
+
         }
     }
-    add_logical_patch_ports_postprocess(patched_datapaths);
+    add_logical_patch_ports_postprocess(patched_datapaths, ctx);
 }
 
 void
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 4a25fad..5ce7901 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -227,6 +227,43 @@ struct tnlid_node {
     uint32_t tnlid;
 };
 
+struct related_datapath_node {
+    struct hmap_node hmap_node;
+    const struct sbrec_datapath_binding *sb_db;
+};
+
+static void
+add_related_datapath(struct hmap *related_datapaths,
+                     const struct sbrec_datapath_binding *sb,
+                     size_t *n_related_datapaths)
+{
+    struct related_datapath_node *node;
+    HMAP_FOR_EACH_WITH_HASH (node, hmap_node, uuid_hash(&sb->header_.uuid),
+                             related_datapaths) {
+        if (uuid_equals(&sb->header_.uuid, &node->sb_db->header_.uuid)) {
+            return;
+        }
+    }
+
+    node = xzalloc(sizeof *node);
+    hmap_insert(related_datapaths, &node->hmap_node,
+                uuid_hash(&sb->header_.uuid));
+    node->sb_db = sb;
+    (*n_related_datapaths)++;
+}
+
+static void
+destroy_related_datapaths(struct hmap *related_datapaths)
+{
+    struct related_datapath_node *node;
+    HMAP_FOR_EACH_POP (node, hmap_node, related_datapaths) {
+        free(node);
+    }
+    hmap_destroy(related_datapaths);
+}
+
+/////////////////////////////////////////////////////////////////////
+
 static void
 destroy_tnlids(struct hmap *tnlids)
 {
@@ -292,6 +329,8 @@ struct ovn_datapath {
     size_t n_router_ports;
 
     struct hmap port_tnlids;
+    struct hmap related_datapaths;
+    size_t n_related_datapaths;
     uint32_t port_key_hint;
 
     bool has_unknown;
@@ -342,6 +381,7 @@ ovn_datapath_create(struct hmap *datapaths, const struct uuid *key,
     od->nbr = nbr;
     hmap_init(&od->port_tnlids);
     hmap_init(&od->ipam);
+    hmap_init(&od->related_datapaths);
     od->port_key_hint = 0;
     hmap_insert(datapaths, &od->key_node, uuid_hash(&od->key));
     return od;
@@ -357,6 +397,7 @@ ovn_datapath_destroy(struct hmap *datapaths, struct ovn_datapath *od)
         hmap_remove(datapaths, &od->key_node);
         destroy_tnlids(&od->port_tnlids);
         destroy_ipam(&od->ipam);
+        destroy_related_datapaths(&od->related_datapaths);
         free(od->router_ports);
         free(od);
     }
@@ -528,6 +569,25 @@ build_datapaths(struct northd_context *ctx, struct hmap *datapaths)
             sbrec_datapath_binding_set_external_ids(od->sb, &id);
 
             sbrec_datapath_binding_set_tunnel_key(od->sb, tunnel_key);
+
+            struct sbrec_datapath_binding **sb_related_datapaths
+                = xmalloc(sizeof(*sb_related_datapaths) * od->n_related_datapaths);
+            int rdi = 0;
+            struct related_datapath_node *related_datapath;
+            HMAP_FOR_EACH (related_datapath, hmap_node, &od->related_datapaths) {
+                if (rdi >= od->n_related_datapaths) {
+                    static struct vlog_rate_limit rl
+                        = VLOG_RATE_LIMIT_INIT(5, 1);
+                    VLOG_ERR_RL(&rl, "related datapaths accounting error"
+                                UUID_FMT, UUID_ARGS(&od->key));
+                    break;
+                }
+                sb_related_datapaths[rdi] = related_datapath->sb_db;
+                rdi++;
+            }
+            sbrec_datapath_binding_set_related_datapaths(od->sb,
+                sb_related_datapaths, od->n_related_datapaths);
+            free(sb_related_datapaths);
         }
         destroy_tnlids(&dp_tnlids);
     }
@@ -1147,6 +1207,12 @@ ovn_port_update_sbrec(const struct ovn_port *op)
             sbrec_port_binding_set_type(op->sb, "patch");
         }
 
+        if (op->peer && op->peer->od && op->peer->od->sb) {
+            add_related_datapath(&op->od->related_datapaths,
+                                 op->peer->od->sb,
+                                 &op->od->n_related_datapaths);
+        }
+
         const char *peer = op->peer ? op->peer->key : "<error>";
         struct smap new;
         smap_init(&new);
@@ -1178,6 +1244,12 @@ ovn_port_update_sbrec(const struct ovn_port *op)
                 sbrec_port_binding_set_type(op->sb, "patch");
             }
 
+            if (op->peer && op->peer->od && op->peer->od->sb) {
+                add_related_datapath(&op->od->related_datapaths,
+                                     op->peer->od->sb,
+                                     &op->od->n_related_datapaths);
+            }
+
             const char *router_port = smap_get(&op->nbsp->options,
                                                "router-port");
             if (!router_port) {
@@ -3910,6 +3982,8 @@ main(int argc, char *argv[])
     add_column_noalert(ovnsb_idl_loop.idl,
                        &sbrec_datapath_binding_col_tunnel_key);
     add_column_noalert(ovnsb_idl_loop.idl,
+                       &sbrec_datapath_binding_col_related_datapaths);
+    add_column_noalert(ovnsb_idl_loop.idl,
                        &sbrec_datapath_binding_col_external_ids);
 
     ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_port_binding);
diff --git a/ovn/ovn-sb.ovsschema b/ovn/ovn-sb.ovsschema
index b1737f5..35a3a63 100644
--- a/ovn/ovn-sb.ovsschema
+++ b/ovn/ovn-sb.ovsschema
@@ -1,7 +1,7 @@
 {
     "name": "OVN_Southbound",
-    "version": "1.7.0",
-    "cksum": "3677179333 6917",
+    "version": "1.8.0",
+    "cksum": "945864814 7260",
     "tables": {
         "SB_Global": {
             "columns": {
@@ -88,7 +88,12 @@
                                       "maxInteger": 16777215}}},
                 "external_ids": {
                     "type": {"key": "string", "value": "string",
-                             "min": 0, "max": "unlimited"}}},
+                             "min": 0, "max": "unlimited"}},
+                "related_datapaths": {"type": {"key": {"type": "uuid",
+                                               "refTable": "Datapath_Binding",
+                                               "refType": "weak"},
+                                               "min": 0,
+                                               "max": "unlimited"}}},
             "indexes": [["tunnel_key"]],
             "isRoot": true},
         "Port_Binding": {
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index c5f236e..d48d64f 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -1399,6 +1399,15 @@ tcp.flags = RST;
       constructed for each supported encapsulation.
     </column>
 
+    <column name="related_datapaths">
+      The related_datapaths column is used to keep track of which datapaths
+      are connected to each other.  This is leveraged in ovn-controller to 
+      do a flood fill from each local logical switch to determine the full
+      set of datapaths needed on a given ovn-controller.   This set of 
+      datapaths can be used to determine which logical ports and logical
+      flows an ovn-controller should monitor.
+    </column>
+
     <group title="OVN_Northbound Relationship">
       <p>
         Each row in <ref table="Datapath_Binding"/> is associated with some
-- 
1.9.1




More information about the dev mailing list