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 <lrich...@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 --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);
--
2.13.0
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
