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 <[email protected]> --- v8: Rebased, changes required. 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 | 23 +++++++++-- 5 files changed, 64 insertions(+), 86 deletions(-) diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c index f868e1fe5..373b70c3b 100644 --- a/ovn/controller/lflow.c +++ b/ovn/controller/lflow.c @@ -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; }; @@ -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, @@ -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; @@ -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, @@ -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, @@ -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, @@ -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 = { @@ -416,7 +415,6 @@ 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, @@ -424,7 +422,7 @@ lflow_run(struct controller_ctx *ctx, 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); diff --git a/ovn/controller/lflow.h b/ovn/controller/lflow.h index 484502f4f..1e7dead1d 100644 --- a/ovn/controller/lflow.h +++ b/ovn/controller/lflow.h @@ -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; @@ -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, diff --git a/ovn/controller/lport.c b/ovn/controller/lport.c index 28f5d7726..8d97078d1 100644 --- a/ovn/controller/lport.c +++ b/ovn/controller/lport.c @@ -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); @@ -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 9a5a5dad8..fbd3b8ec2 100644 --- a/ovn/controller/lport.h +++ b/ovn/controller/lport.h @@ -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 */ diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c index f1fc74155..b60cf3e75 100644 --- a/ovn/controller/ovn-controller.c +++ b/ovn/controller/ovn-controller.c @@ -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[]) { @@ -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); @@ -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; @@ -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); @@ -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); -- 2.13.3 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
