diff mbox

[ovs-dev,v7,5/7] ovn-controller: use idl index for multicast group table

Message ID 20170714191549.29899-6-lrichard@redhat.com
State Not Applicable
Headers show

Commit Message

Lance Richardson July 14, 2017, 7:15 p.m. UTC
Use IDL index for multicast group table lookups, avoiding the overhead
of creating/destroying an index hmap for each iteration of the
ovn-controller main loop.

Signed-off-by: Lance Richardson <lrichard@redhat.com>
---
 v7: New patch.

 ovn/controller/lflow.c          | 20 +++++-----
 ovn/controller/lflow.h          |  2 -
 ovn/controller/lport.c          | 85 ++++++++++++++++-------------------------
 ovn/controller/lport.h          | 20 ++--------
 ovn/controller/ovn-controller.c | 26 ++++++++++---
 5 files changed, 65 insertions(+), 88 deletions(-)
diff mbox

Patch

diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index b1b4b2372..56a7c02c2 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -45,8 +45,8 @@  lflow_init(void)
 }
 
 struct lookup_port_aux {
+    struct ovsdb_idl *ovnsb_idl;
     const struct lport_index *lports;
-    const struct mcgroup_index *mcgroups;
     const struct sbrec_datapath_binding *dp;
 };
 
@@ -55,8 +55,8 @@  struct condition_aux {
     const struct sbrec_chassis *chassis;
 };
 
-static void consider_logical_flow(const struct lport_index *lports,
-                                  const struct mcgroup_index *mcgroups,
+static void consider_logical_flow(struct controller_ctx *ctx,
+                                  const struct lport_index *lports,
                                   const struct sbrec_logical_flow *lflow,
                                   const struct hmap *local_datapaths,
                                   struct group_table *group_table,
@@ -80,7 +80,7 @@  lookup_port_cb(const void *aux_, const char *port_name, unsigned int *portp)
     }
 
     const struct sbrec_multicast_group *mg
-        = mcgroup_lookup_by_dp_name(aux->mcgroups, aux->dp, port_name);
+        = mcgroup_lookup_by_dp_name(aux->ovnsb_idl, aux->dp, port_name);
     if (mg) {
         *portp = mg->tunnel_key;
         return true;
@@ -118,7 +118,6 @@  is_gateway_router(const struct sbrec_datapath_binding *ldp,
 /* Adds the logical flows from the Logical_Flow table to flow tables. */
 static void
 add_logical_flows(struct controller_ctx *ctx, const struct lport_index *lports,
-                  const struct mcgroup_index *mcgroups,
                   const struct hmap *local_datapaths,
                   struct group_table *group_table,
                   const struct sbrec_chassis *chassis,
@@ -144,7 +143,7 @@  add_logical_flows(struct controller_ctx *ctx, const struct lport_index *lports,
     }
 
     SBREC_LOGICAL_FLOW_FOR_EACH (lflow, ctx->ovnsb_idl) {
-        consider_logical_flow(lports, mcgroups, lflow, local_datapaths,
+        consider_logical_flow(ctx, lports, lflow, local_datapaths,
                               group_table, chassis,
                               &dhcp_opts, &dhcpv6_opts, &conj_id_ofs,
                               addr_sets, flow_table);
@@ -155,8 +154,8 @@  add_logical_flows(struct controller_ctx *ctx, const struct lport_index *lports,
 }
 
 static void
-consider_logical_flow(const struct lport_index *lports,
-                      const struct mcgroup_index *mcgroups,
+consider_logical_flow(struct controller_ctx *ctx,
+                      const struct lport_index *lports,
                       const struct sbrec_logical_flow *lflow,
                       const struct hmap *local_datapaths,
                       struct group_table *group_table,
@@ -220,7 +219,7 @@  consider_logical_flow(const struct lport_index *lports,
     struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
     struct lookup_port_aux aux = {
         .lports = lports,
-        .mcgroups = mcgroups,
+        .ovnsb_idl = ctx->ovnsb_idl,
         .dp = lflow->logical_datapath
     };
     struct ovnact_encode_params ep = {
@@ -387,13 +386,12 @@  void
 lflow_run(struct controller_ctx *ctx,
           const struct sbrec_chassis *chassis,
           const struct lport_index *lports,
-          const struct mcgroup_index *mcgroups,
           const struct hmap *local_datapaths,
           struct group_table *group_table,
           const struct shash *addr_sets,
           struct hmap *flow_table)
 {
-    add_logical_flows(ctx, lports, mcgroups, local_datapaths, group_table,
+    add_logical_flows(ctx, lports, local_datapaths, group_table,
                       chassis, addr_sets, flow_table);
     add_neighbor_flows(ctx, lports, flow_table);
 }
diff --git a/ovn/controller/lflow.h b/ovn/controller/lflow.h
index a23cde099..60d86cdc8 100644
--- a/ovn/controller/lflow.h
+++ b/ovn/controller/lflow.h
@@ -39,7 +39,6 @@  struct controller_ctx;
 struct group_table;
 struct hmap;
 struct lport_index;
-struct mcgroup_index;
 struct sbrec_chassis;
 struct simap;
 struct uuid;
@@ -65,7 +64,6 @@  void lflow_init(void);
 void lflow_run(struct controller_ctx *,
                const struct sbrec_chassis *chassis,
                const struct lport_index *,
-               const struct mcgroup_index *,
                const struct hmap *local_datapaths,
                struct group_table *group_table,
                const struct shash *addr_sets,
diff --git a/ovn/controller/lport.c b/ovn/controller/lport.c
index 906fda222..7636d911c 100644
--- a/ovn/controller/lport.c
+++ b/ovn/controller/lport.c
@@ -22,6 +22,8 @@ 
 
 VLOG_DEFINE_THIS_MODULE(lport);
 
+static struct ovsdb_idl_index_cursor mc_grp_by_dp_name_cursor;
+
 static struct ldatapath *ldatapath_lookup_by_key__(
     const struct ldatapath_index *, uint32_t dp_key);
 
@@ -177,63 +179,42 @@  lport_lookup_by_key(const struct lport_index *lports,
     return NULL;
 }
 
-struct mcgroup {
-    struct hmap_node dp_name_node; /* Index by (logical datapath, name). */
-    const struct sbrec_multicast_group *mg;
-};
-
-void
-mcgroup_index_init(struct mcgroup_index *mcgroups, struct ovsdb_idl *ovnsb_idl)
-{
-    hmap_init(&mcgroups->by_dp_name);
-
-    const struct sbrec_multicast_group *mg;
-    SBREC_MULTICAST_GROUP_FOR_EACH (mg, ovnsb_idl) {
-        const struct uuid *dp_uuid = &mg->datapath->header_.uuid;
-        if (mcgroup_lookup_by_dp_name(mcgroups, mg->datapath, mg->name)) {
-            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
-            VLOG_WARN_RL(&rl, "datapath "UUID_FMT" contains duplicate "
-                         "multicast group '%s'", UUID_ARGS(dp_uuid), mg->name);
-            continue;
-        }
-
-        struct mcgroup *m = xmalloc(sizeof *m);
-        hmap_insert(&mcgroups->by_dp_name, &m->dp_name_node,
-                    hash_string(mg->name, uuid_hash(dp_uuid)));
-        m->mg = mg;
-    }
-}
-
-void
-mcgroup_index_destroy(struct mcgroup_index *mcgroups)
+/* Finds and returns the logical multicast group with the given 'name' and
+ * datapath binding, or NULL if no such logical multicast group exists. */
+const struct sbrec_multicast_group *
+mcgroup_lookup_by_dp_name(struct ovsdb_idl *idl,
+                           const struct sbrec_datapath_binding *dp,
+                           const char *name)
 {
-    if (!mcgroups) {
-        return;
+    struct sbrec_multicast_group *mcval;
+    const struct sbrec_multicast_group *mc, *retval = NULL;
+
+    /* Build key for an indexed lookup. */
+    mcval = sbrec_multicast_group_index_init_row(idl,
+                                                 &sbrec_table_multicast_group);
+    sbrec_multicast_group_index_set_name(mcval, name);
+    sbrec_multicast_group_index_set_datapath(mcval, dp);
+
+    /* Find an entry with matching logical multicast group name and datapath.
+     * Since this column pair is declared to be an index in the OVN_Southbound
+     * schema, the first match (if any) will be the only match. */
+    SBREC_MULTICAST_GROUP_FOR_EACH_EQUAL (mc, &mc_grp_by_dp_name_cursor,
+                                          mcval) {
+        retval = mc;
+        break;
     }
 
-    struct mcgroup *mcgroup, *next;
-    HMAP_FOR_EACH_SAFE (mcgroup, next, dp_name_node, &mcgroups->by_dp_name) {
-        hmap_remove(&mcgroups->by_dp_name, &mcgroup->dp_name_node);
-        free(mcgroup);
-    }
+    sbrec_multicast_group_index_destroy_row(mcval);
 
-    hmap_destroy(&mcgroups->by_dp_name);
+    return retval;
 }
 
-const struct sbrec_multicast_group *
-mcgroup_lookup_by_dp_name(const struct mcgroup_index *mcgroups,
-                          const struct sbrec_datapath_binding *dp,
-                          const char *name)
+void
+lport_init(struct ovsdb_idl *idl)
 {
-    const struct uuid *dp_uuid = &dp->header_.uuid;
-    const struct mcgroup *mcgroup;
-    HMAP_FOR_EACH_WITH_HASH (mcgroup, dp_name_node,
-                             hash_string(name, uuid_hash(dp_uuid)),
-                             &mcgroups->by_dp_name) {
-        if (uuid_equals(&mcgroup->mg->datapath->header_.uuid, dp_uuid)
-            && !strcmp(mcgroup->mg->name, name)) {
-            return mcgroup->mg;
-        }
-    }
-    return NULL;
+    /* Create a cursor for searching multicast group table by datapath
+     * and group name. */
+    ovsdb_idl_initialize_cursor(idl, &sbrec_table_multicast_group,
+                                "multicast-group-by-dp-name",
+                                &mc_grp_by_dp_name_cursor);
 }
diff --git a/ovn/controller/lport.h b/ovn/controller/lport.h
index fe0e430ac..4ee19a536 100644
--- a/ovn/controller/lport.h
+++ b/ovn/controller/lport.h
@@ -71,26 +71,12 @@  const struct sbrec_port_binding *lport_lookup_by_name(
 const struct sbrec_port_binding *lport_lookup_by_key(
     const struct lport_index *, uint32_t dp_key, uint16_t port_key);
 
-/* Multicast group index
- * =====================
- *
- * This is separate from the logical port index because of namespace issues:
- * logical port names are globally unique, but multicast group names are only
- * unique within the scope of a logical datapath.
- *
- * Multicast groups could be indexed by number also, but so far the clients do
- * not need this index. */
-
-struct mcgroup_index {
-    struct hmap by_dp_name;
-};
-
-void mcgroup_index_init(struct mcgroup_index *, struct ovsdb_idl *);
-void mcgroup_index_destroy(struct mcgroup_index *);
 
 const struct sbrec_multicast_group *mcgroup_lookup_by_dp_name(
-    const struct mcgroup_index *,
+    struct ovsdb_idl *idl,
     const struct sbrec_datapath_binding *,
     const char *name);
 
+void lport_init(struct ovsdb_idl *idl);
+
 #endif /* ovn/lport.h */
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 45a670b5d..d01227d53 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -525,6 +525,20 @@  ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
     physical_register_ovs_idl(ovs_idl);
 }
 
+static void
+create_ovnsb_indexes(struct ovsdb_idl *ovnsb_idl)
+{
+    struct ovsdb_idl_index *index;
+
+    /* Index multicast group table by name and datapath. */
+    index = ovsdb_idl_create_index(ovnsb_idl, &sbrec_table_multicast_group,
+                                   "multicast-group-by-dp-name");
+    ovsdb_idl_index_add_column(index, &sbrec_multicast_group_col_name,
+                               OVSDB_INDEX_ASC, NULL);
+    ovsdb_idl_index_add_column(index, &sbrec_multicast_group_col_datapath,
+                               OVSDB_INDEX_ASC, NULL);
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -569,6 +583,10 @@  main(int argc, char *argv[])
     char *ovnsb_remote = get_ovnsb_remote(ovs_idl_loop.idl);
     struct ovsdb_idl_loop ovnsb_idl_loop = OVSDB_IDL_LOOP_INITIALIZER(
         ovsdb_idl_create(ovnsb_remote, &sbrec_idl_class, true, true));
+
+    create_ovnsb_indexes(ovnsb_idl_loop.idl);
+    lport_init(ovnsb_idl_loop.idl);
+
     ovsdb_idl_omit_alert(ovnsb_idl_loop.idl, &sbrec_chassis_col_nb_cfg);
     update_sb_monitors(ovnsb_idl_loop.idl, NULL, NULL, NULL);
     ovsdb_idl_get_initial_snapshot(ovnsb_idl_loop.idl);
@@ -626,10 +644,8 @@  main(int argc, char *argv[])
 
         struct ldatapath_index ldatapaths;
         struct lport_index lports;
-        struct mcgroup_index mcgroups;
         ldatapath_index_init(&ldatapaths, ctx.ovnsb_idl);
         lport_index_init(&lports, ctx.ovnsb_idl);
-        mcgroup_index_init(&mcgroups, ctx.ovnsb_idl);
 
         const struct sbrec_chassis *chassis = NULL;
         if (chassis_id) {
@@ -656,9 +672,8 @@  main(int argc, char *argv[])
                     commit_ct_zones(br_int, &pending_ct_zones);
 
                     struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
-                    lflow_run(&ctx, chassis, &lports, &mcgroups,
-                              &local_datapaths, &group_table,
-                              &addr_sets, &flow_table);
+                    lflow_run(&ctx, chassis, &lports, &local_datapaths,
+                              &group_table, &addr_sets, &flow_table);
 
                     physical_run(&ctx, mff_ovn_geneve,
                                  br_int, chassis, &ct_zones, &lports,
@@ -706,7 +721,6 @@  main(int argc, char *argv[])
             free(pending_pkt.flow_s);
         }
 
-        mcgroup_index_destroy(&mcgroups);
         lport_index_destroy(&lports);
         ldatapath_index_destroy(&ldatapaths);