Skip to content

Commit

Permalink
ovn-controller: use idl index for multicast group table
Browse files Browse the repository at this point in the history 
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>
Signed-off-by: Ben Pfaff <blp@ovn.org>
  • Loading branch information
@hlrichardson @blp
hlrichardson authored and blp committed Aug 3, 2017
1 parent 0a8606e commit 5be10ec
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 86 deletions.
20 changes: 9 additions & 11 deletions ovn/controller/lflow.c
View file
Expand Up @@ -46,8 +46,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;
};

Expand All @@ -58,8 +58,8 @@ struct condition_aux {
const struct chassis_index *chassis_index;
};

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 chassis_index *chassis_index,
const struct sbrec_logical_flow *lflow,
const struct hmap *local_datapaths,
Expand All @@ -85,7 +85,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;
Expand Down Expand Up @@ -141,7 +141,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 chassis_index *chassis_index,
const struct hmap *local_datapaths,
struct group_table *group_table,
Expand Down Expand Up @@ -169,7 +168,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, chassis_index,
consider_logical_flow(ctx, lports, chassis_index,
lflow, local_datapaths,
group_table, chassis,
&dhcp_opts, &dhcpv6_opts, &conj_id_ofs,
Expand All @@ -181,8 +180,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 chassis_index *chassis_index,
const struct sbrec_logical_flow *lflow,
const struct hmap *local_datapaths,
Expand Down Expand Up @@ -248,7 +247,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 = {
Expand Down Expand Up @@ -416,15 +415,14 @@ void
lflow_run(struct controller_ctx *ctx,
const struct sbrec_chassis *chassis,
const struct lport_index *lports,
const struct mcgroup_index *mcgroups,
const struct chassis_index *chassis_index,
const struct hmap *local_datapaths,
struct group_table *group_table,
const struct shash *addr_sets,
struct hmap *flow_table,
struct sset *active_tunnels)
{
add_logical_flows(ctx, lports, mcgroups, chassis_index, local_datapaths,
add_logical_flows(ctx, lports, chassis_index, local_datapaths,
group_table, chassis, addr_sets, flow_table,
active_tunnels);
add_neighbor_flows(ctx, lports, flow_table);
Expand Down
2 changes: 0 additions & 2 deletions ovn/controller/lflow.h
View file
Expand Up @@ -40,7 +40,6 @@ struct controller_ctx;
struct group_table;
struct hmap;
struct lport_index;
struct mcgroup_index;
struct sbrec_chassis;
struct simap;
struct sset;
Expand All @@ -67,7 +66,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 chassis_index *,
const struct hmap *local_datapaths,
struct group_table *group_table,
Expand Down
85 changes: 33 additions & 52 deletions ovn/controller/lport.c
View file
Expand Up @@ -22,6 +22,8 @@
#include "ovn/lib/ovn-sb-idl.h"
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);

Expand Down Expand Up @@ -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);
}
20 changes: 3 additions & 17 deletions ovn/controller/lport.h
View file
Expand Up @@ -76,26 +76,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 */
23 changes: 19 additions & 4 deletions ovn/controller/ovn-controller.c
View file
Expand Up @@ -531,6 +531,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[])
{
Expand Down Expand Up @@ -575,6 +589,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);
Expand Down Expand Up @@ -633,12 +651,10 @@ main(int argc, char *argv[])

struct ldatapath_index ldatapaths;
struct lport_index lports;
struct mcgroup_index mcgroups;
struct chassis_index chassis_index;

ldatapath_index_init(&ldatapaths, ctx.ovnsb_idl);
lport_index_init(&lports, ctx.ovnsb_idl);
mcgroup_index_init(&mcgroups, ctx.ovnsb_idl);
chassis_index_init(&chassis_index, ctx.ovnsb_idl);

const struct sbrec_chassis *chassis = NULL;
Expand Down Expand Up @@ -668,7 +684,7 @@ 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,
lflow_run(&ctx, chassis, &lports,
&chassis_index, &local_datapaths, &group_table,
&addr_sets, &flow_table, &active_tunnels);

Expand Down Expand Up @@ -723,7 +739,6 @@ main(int argc, char *argv[])
free(pending_pkt.flow_s);
}

mcgroup_index_destroy(&mcgroups);
lport_index_destroy(&lports);
ldatapath_index_destroy(&ldatapaths);
chassis_index_destroy(&chassis_index);
Expand Down

0 comments on commit 5be10ec

Please sign in to comment.