Use IDL index to iterate over all logical ports in a given logical datapath, avoiding the overhead of creating/destroying an indexing data structure in each iteration of the ovn-controller main loop.
Signed-off-by: Lance Richardson <lrich...@redhat.com> --- v8: Rebased, changes required. v7: New patch. ovn/controller/bfd.c | 28 +++++++++++++----- ovn/controller/binding.c | 45 ++++++++++++++++------------- ovn/controller/binding.h | 3 +- ovn/controller/lport.c | 63 +---------------------------------------- ovn/controller/lport.h | 22 ++------------ ovn/controller/ovn-controller.c | 11 ++++--- ovn/controller/ovn-controller.h | 2 +- ovn/controller/pinctrl.c | 15 ++++++++-- 8 files changed, 71 insertions(+), 118 deletions(-) diff --git a/ovn/controller/bfd.c b/ovn/controller/bfd.c index 03cf377e1..d1448b13f 100644 --- a/ovn/controller/bfd.c +++ b/ovn/controller/bfd.c @@ -101,7 +101,8 @@ bfd_calculate_active_tunnels(const struct ovsrec_bridge *br_int, } static void -bfd_calculate_chassis(const struct sbrec_chassis *our_chassis, +bfd_calculate_chassis(struct controller_ctx *ctx, + const struct sbrec_chassis *our_chassis, struct hmap *local_datapaths, const struct chassis_index *chassis_index, struct sset *bfd_chassis) @@ -112,14 +113,25 @@ bfd_calculate_chassis(const struct sbrec_chassis *our_chassis, * 2) Chassis hosting peer datapaths (with ports) connected * to a router datapath when our chassis is hosting a router * with a chassis redirect port. */ + + const struct sbrec_port_binding *pb; + struct ovsdb_idl_index_cursor cursor; + struct sbrec_port_binding *lpval; struct local_datapath *dp; + + ovsdb_idl_initialize_cursor(ctx->ovnsb_idl, + &sbrec_table_port_binding, + "lport-by-datapath", &cursor); + lpval = sbrec_port_binding_index_init_row(ctx->ovnsb_idl, + &sbrec_table_port_binding); + HMAP_FOR_EACH (dp, hmap_node, local_datapaths) { const char *is_router = smap_get(&dp->datapath->external_ids, "logical-router"); bool our_chassis_is_gw_for_dp = false; if (is_router) { - for (size_t j = 0; j < dp->ldatapath->n_lports; j++) { - const struct sbrec_port_binding *pb = dp->ldatapath->lports[j]; + sbrec_port_binding_index_set_datapath(lpval, dp->datapath); + SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, &cursor, lpval) { if (!strcmp(pb->type, "chassisredirect")) { struct ovs_list *gateway_chassis = NULL; gateway_chassis = @@ -144,12 +156,13 @@ bfd_calculate_chassis(const struct sbrec_chassis *our_chassis, } if (our_chassis_is_gw_for_dp) { for (size_t i = 0; i < dp->n_peer_dps; i++) { - const struct ldatapath *pdp = dp->peer_dps[i]; + const struct sbrec_datapath_binding *pdp = dp->peer_dps[i]; if (!pdp) { continue; } - for (size_t j = 0; j < pdp->n_lports; j++) { - const struct sbrec_port_binding *pb = pdp->lports[j]; + + sbrec_port_binding_index_set_datapath(lpval, pdp); + SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, &cursor, lpval) { if (pb->chassis) { /* Gateway node has to enable bfd to all nodes hosting * connected network ports */ @@ -162,6 +175,7 @@ bfd_calculate_chassis(const struct sbrec_chassis *our_chassis, } } } + sbrec_port_binding_index_destroy_row(lpval); } void @@ -174,7 +188,7 @@ bfd_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, return; } struct sset bfd_chassis = SSET_INITIALIZER(&bfd_chassis); - bfd_calculate_chassis(chassis_rec, local_datapaths, chassis_index, + bfd_calculate_chassis(ctx, chassis_rec, local_datapaths, chassis_index, &bfd_chassis); /* Identify tunnels ports(connected to remote chassis id) to enable bfd */ struct sset tunnels = SSET_INITIALIZER(&tunnels); diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c index 28e08721d..32309e9e3 100644 --- a/ovn/controller/binding.c +++ b/ovn/controller/binding.c @@ -111,12 +111,14 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int, static void add_local_datapath__(struct controller_ctx *ctx, - const struct ldatapath_index *ldatapaths, const struct sbrec_datapath_binding *datapath, bool has_local_l3gateway, int depth, struct hmap *local_datapaths) { uint32_t dp_key = datapath->tunnel_key; + const struct sbrec_port_binding *pb; + struct ovsdb_idl_index_cursor cursor; + struct sbrec_port_binding *lpval; struct local_datapath *ld = get_local_datapath(local_datapaths, dp_key); if (ld) { @@ -129,8 +131,6 @@ add_local_datapath__(struct controller_ctx *ctx, ld = xzalloc(sizeof *ld); hmap_insert(local_datapaths, &ld->hmap_node, dp_key); ld->datapath = datapath; - ld->ldatapath = ldatapath_lookup_by_key(ldatapaths, dp_key); - ovs_assert(ld->ldatapath); ld->localnet_port = NULL; ld->has_local_l3gateway = has_local_l3gateway; @@ -141,35 +141,42 @@ add_local_datapath__(struct controller_ctx *ctx, } /* Recursively add logical datapaths to which this one patches. */ - for (size_t i = 0; i < ld->ldatapath->n_lports; i++) { - const struct sbrec_port_binding *pb = ld->ldatapath->lports[i]; + lpval = sbrec_port_binding_index_init_row(ctx->ovnsb_idl, + &sbrec_table_port_binding); + sbrec_port_binding_index_set_datapath(lpval, datapath); + ovsdb_idl_initialize_cursor(ctx->ovnsb_idl, &sbrec_table_port_binding, + "lport-by-datapath", &cursor); + + SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, &cursor, lpval) { if (!strcmp(pb->type, "patch")) { const char *peer_name = smap_get(&pb->options, "peer"); if (peer_name) { - const struct sbrec_port_binding *peer = lport_lookup_by_name( - ctx->ovnsb_idl, peer_name); + const struct sbrec_port_binding *peer; + + peer = lport_lookup_by_name( ctx->ovnsb_idl, peer_name); + if (peer && peer->datapath) { - add_local_datapath__(ctx, ldatapaths, peer->datapath, + add_local_datapath__(ctx, peer->datapath, false, depth + 1, local_datapaths); ld->n_peer_dps++; ld->peer_dps = xrealloc( ld->peer_dps, ld->n_peer_dps * sizeof *ld->peer_dps); - ld->peer_dps[ld->n_peer_dps - 1] = ldatapath_lookup_by_key( - ldatapaths, peer->datapath->tunnel_key); + ld->peer_dps[ld->n_peer_dps - 1] = datapath_lookup_by_key( + ctx->ovnsb_idl, peer->datapath->tunnel_key); } } } } + sbrec_port_binding_index_destroy_row(lpval); } static void add_local_datapath(struct controller_ctx *ctx, - const struct ldatapath_index *ldatapaths, const struct sbrec_datapath_binding *datapath, bool has_local_l3gateway, struct hmap *local_datapaths) { - add_local_datapath__(ctx, ldatapaths, datapath, has_local_l3gateway, 0, + add_local_datapath__(ctx, datapath, has_local_l3gateway, 0, local_datapaths); } @@ -365,7 +372,6 @@ setup_qos(const char *egress_iface, struct hmap *queue_map) static void consider_local_datapath(struct controller_ctx *ctx, - const struct ldatapath_index *ldatapaths, const struct chassis_index *chassis_index, struct sset *active_tunnels, const struct sbrec_chassis *chassis_rec, @@ -387,13 +393,13 @@ consider_local_datapath(struct controller_ctx *ctx, /* Add child logical port to the set of all local ports. */ sset_add(local_lports, binding_rec->logical_port); } - add_local_datapath(ctx, ldatapaths, binding_rec->datapath, + add_local_datapath(ctx, binding_rec->datapath, false, local_datapaths); if (iface_rec && qos_map && ctx->ovs_idl_txn) { get_qos_params(binding_rec, qos_map); } /* This port is in our chassis unless it is a localport. */ - if (strcmp(binding_rec->type, "localport")) { + if (strcmp(binding_rec->type, "localport")) { our_chassis = true; } } else if (!strcmp(binding_rec->type, "l2gateway")) { @@ -402,7 +408,7 @@ consider_local_datapath(struct controller_ctx *ctx, our_chassis = chassis_id && !strcmp(chassis_id, chassis_rec->name); if (our_chassis) { sset_add(local_lports, binding_rec->logical_port); - add_local_datapath(ctx, ldatapaths, binding_rec->datapath, + add_local_datapath(ctx, binding_rec->datapath, false, local_datapaths); } } else if (!strcmp(binding_rec->type, "chassisredirect")) { @@ -414,7 +420,7 @@ consider_local_datapath(struct controller_ctx *ctx, our_chassis = gateway_chassis_is_active( gateway_chassis, chassis_rec, active_tunnels); - add_local_datapath(ctx, ldatapaths, binding_rec->datapath, + add_local_datapath(ctx, binding_rec->datapath, false, local_datapaths); } gateway_chassis_destroy(gateway_chassis); @@ -423,7 +429,7 @@ consider_local_datapath(struct controller_ctx *ctx, "l3gateway-chassis"); our_chassis = chassis_id && !strcmp(chassis_id, chassis_rec->name); if (our_chassis) { - add_local_datapath(ctx, ldatapaths, binding_rec->datapath, + add_local_datapath(ctx, binding_rec->datapath, true, local_datapaths); } } else if (!strcmp(binding_rec->type, "localnet")) { @@ -486,7 +492,6 @@ consider_localnet_port(const struct sbrec_port_binding *binding_rec, void binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, const struct sbrec_chassis *chassis_rec, - const struct ldatapath_index *ldatapaths, const struct chassis_index *chassis_index, struct sset *active_tunnels, struct hmap *local_datapaths, struct sset *local_lports) @@ -510,7 +515,7 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, * chassis and update the binding accordingly. This includes both * directly connected logical ports and children of those ports. */ SBREC_PORT_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) { - consider_local_datapath(ctx, ldatapaths, chassis_index, + consider_local_datapath(ctx, chassis_index, active_tunnels, chassis_rec, binding_rec, sset_is_empty(&egress_ifaces) ? NULL : &qos_map, local_datapaths, &lport_to_iface, diff --git a/ovn/controller/binding.h b/ovn/controller/binding.h index cead062eb..c78f8d932 100644 --- a/ovn/controller/binding.h +++ b/ovn/controller/binding.h @@ -22,7 +22,6 @@ struct controller_ctx; struct chassis_index; struct hmap; -struct ldatapath_index; struct ovsdb_idl; struct ovsrec_bridge; struct sbrec_chassis; @@ -30,7 +29,7 @@ struct sset; void binding_register_ovs_idl(struct ovsdb_idl *); void binding_run(struct controller_ctx *, const struct ovsrec_bridge *br_int, - const struct sbrec_chassis *, const struct ldatapath_index *, + const struct sbrec_chassis *, const struct chassis_index *, struct sset *active_tunnels, struct hmap *local_datapaths, struct sset *all_lports); diff --git a/ovn/controller/lport.c b/ovn/controller/lport.c index 711cf4bd1..e5305d0b0 100644 --- a/ovn/controller/lport.c +++ b/ovn/controller/lport.c @@ -27,67 +27,6 @@ static struct ovsdb_idl_index_cursor lport_by_key_cursor; static struct ovsdb_idl_index_cursor dpath_by_key_cursor; 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); - -void -ldatapath_index_init(struct ldatapath_index *ldatapaths, - struct ovsdb_idl *ovnsb_idl) -{ - hmap_init(&ldatapaths->by_key); - - const struct sbrec_port_binding *pb; - SBREC_PORT_BINDING_FOR_EACH (pb, ovnsb_idl) { - if (!pb->datapath) { - continue; - } - uint32_t dp_key = pb->datapath->tunnel_key; - struct ldatapath *ld = ldatapath_lookup_by_key__(ldatapaths, dp_key); - if (!ld) { - ld = xzalloc(sizeof *ld); - hmap_insert(&ldatapaths->by_key, &ld->by_key_node, dp_key); - ld->db = pb->datapath; - } - - if (ld->n_lports >= ld->allocated_lports) { - ld->lports = x2nrealloc(ld->lports, &ld->allocated_lports, - sizeof *ld->lports); - } - ld->lports[ld->n_lports++] = pb; - } -} - -void -ldatapath_index_destroy(struct ldatapath_index *ldatapaths) -{ - if (!ldatapaths) { - return; - } - - struct ldatapath *ld, *ld_next; - HMAP_FOR_EACH_SAFE (ld, ld_next, by_key_node, &ldatapaths->by_key) { - hmap_remove(&ldatapaths->by_key, &ld->by_key_node); - free(ld->lports); - free(ld); - } - hmap_destroy(&ldatapaths->by_key); -} - -static struct ldatapath *ldatapath_lookup_by_key__( - const struct ldatapath_index *ldatapaths, uint32_t dp_key) -{ - struct ldatapath *ld; - HMAP_FOR_EACH_WITH_HASH (ld, by_key_node, dp_key, &ldatapaths->by_key) { - return ld; - } - return NULL; -} - -const struct ldatapath *ldatapath_lookup_by_key( - const struct ldatapath_index *ldatapaths, uint32_t dp_key) -{ - return ldatapath_lookup_by_key__(ldatapaths, dp_key); -} /* Finds and returns the port binding record with the given 'name', @@ -117,7 +56,7 @@ lport_lookup_by_name(struct ovsdb_idl *idl, const char *name) /* Finds and returns the datapath binding record with tunnel_key equal to the * given 'dp_key', or NULL if no such datapath binding exists. */ -static const struct sbrec_datapath_binding * +const struct sbrec_datapath_binding * datapath_lookup_by_key(struct ovsdb_idl *idl, uint64_t dp_key) { struct sbrec_datapath_binding *dbval; diff --git a/ovn/controller/lport.h b/ovn/controller/lport.h index 822496cec..38c7344dc 100644 --- a/ovn/controller/lport.h +++ b/ovn/controller/lport.h @@ -35,27 +35,9 @@ struct sbrec_port_binding; * instead we define our own indexes. */ -/* Logical datapath index - * ====================== - */ - -struct ldatapath { - struct hmap_node by_key_node; /* Index by tunnel key. */ - const struct sbrec_datapath_binding *db; - const struct sbrec_port_binding **lports; - size_t n_lports, allocated_lports; -}; - -struct ldatapath_index { - struct hmap by_key; -}; -void ldatapath_index_init(struct ldatapath_index *, struct ovsdb_idl *); -void ldatapath_index_destroy(struct ldatapath_index *); - -const struct ldatapath *ldatapath_lookup_by_key( - const struct ldatapath_index *, uint32_t dp_key); - +const struct sbrec_datapath_binding *datapath_lookup_by_key(struct ovsdb_idl *, + uint64_t dp_key); const struct sbrec_port_binding *lport_lookup_by_name( struct ovsdb_idl *, const char *name); diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c index b6abb62bc..e2c965290 100644 --- a/ovn/controller/ovn-controller.c +++ b/ovn/controller/ovn-controller.c @@ -558,6 +558,12 @@ create_ovnsb_indexes(struct ovsdb_idl *ovnsb_idl) ovsdb_idl_index_add_column(index, &sbrec_port_binding_col_datapath, OVSDB_INDEX_ASC, NULL); + /* Index logical port table by datapath. */ + index = ovsdb_idl_create_index(ovnsb_idl, &sbrec_table_port_binding, + "lport-by-datapath"); + ovsdb_idl_index_add_column(index, &sbrec_port_binding_col_datapath, + OVSDB_INDEX_ASC, NULL); + /* Index datapath binding table by tunnel key. */ index = ovsdb_idl_create_index(ovnsb_idl, &sbrec_table_datapath_binding, "dpath-by-key"); @@ -669,10 +675,8 @@ main(int argc, char *argv[]) const struct ovsrec_bridge *br_int = get_br_int(&ctx); const char *chassis_id = get_chassis_id(ctx.ovs_idl); - struct ldatapath_index ldatapaths; struct chassis_index chassis_index; - ldatapath_index_init(&ldatapaths, ctx.ovnsb_idl); chassis_index_init(&chassis_index, ctx.ovnsb_idl); const struct sbrec_chassis *chassis = NULL; @@ -680,7 +684,7 @@ main(int argc, char *argv[]) chassis = chassis_run(&ctx, chassis_id, br_int); encaps_run(&ctx, br_int, chassis_id); bfd_calculate_active_tunnels(br_int, &active_tunnels); - binding_run(&ctx, br_int, chassis, &ldatapaths, + binding_run(&ctx, br_int, chassis, &chassis_index, &active_tunnels, &local_datapaths, &local_lports); } @@ -757,7 +761,6 @@ main(int argc, char *argv[]) free(pending_pkt.flow_s); } - ldatapath_index_destroy(&ldatapaths); chassis_index_destroy(&chassis_index); sset_destroy(&local_lports); diff --git a/ovn/controller/ovn-controller.h b/ovn/controller/ovn-controller.h index cce382fe4..6617b0c16 100644 --- a/ovn/controller/ovn-controller.h +++ b/ovn/controller/ovn-controller.h @@ -66,7 +66,7 @@ struct local_datapath { /* True if this datapath contains an l3gateway port located on this * hypervisor. */ bool has_local_l3gateway; - const struct ldatapath **peer_dps; + const struct sbrec_datapath_binding **peer_dps; size_t n_peer_dps; }; diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c index eb5ac004c..469a35586 100644 --- a/ovn/controller/pinctrl.c +++ b/ovn/controller/pinctrl.c @@ -1476,7 +1476,15 @@ get_localnet_vifs_l3gwports(struct controller_ctx *ctx, } const struct local_datapath *ld; + struct ovsdb_idl_index_cursor cursor; + struct sbrec_port_binding *lpval; + lpval = sbrec_port_binding_index_init_row(ctx->ovnsb_idl, + &sbrec_table_port_binding); + ovsdb_idl_initialize_cursor(ctx->ovnsb_idl, &sbrec_table_port_binding, + "lport-by-datapath", &cursor); HMAP_FOR_EACH (ld, hmap_node, local_datapaths) { + const struct sbrec_port_binding *pb; + if (!ld->localnet_port) { continue; } @@ -1485,14 +1493,17 @@ get_localnet_vifs_l3gwports(struct controller_ctx *ctx, * that connect to gateway routers (if local), and consider port * bindings of type "patch" since they might connect to * distributed gateway ports with NAT addresses. */ - for (size_t i = 0; i < ld->ldatapath->n_lports; i++) { - const struct sbrec_port_binding *pb = ld->ldatapath->lports[i]; + + sbrec_port_binding_index_set_datapath(lpval, ld->datapath); + + SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, &cursor, lpval) { if ((ld->has_local_l3gateway && !strcmp(pb->type, "l3gateway")) || !strcmp(pb->type, "patch")) { sset_add(local_l3gw_ports, pb->logical_port); } } } + sbrec_port_binding_index_destroy_row(lpval); } static bool -- 2.13.3 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev