[ovs-dev] [RFC 3/3] ovn-controller: use idl indexes for logical datapath
Lance Richardson
lrichard at redhat.com
Mon Jun 26 15:54:44 UTC 2017
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 <lrichard at redhat.com>
---
ovn/controller/binding.c | 42 +++++++++++++++-------------
ovn/controller/binding.h | 3 +-
ovn/controller/lport.c | 61 -----------------------------------------
ovn/controller/lport.h | 21 --------------
ovn/controller/ovn-controller.c | 13 +++++----
ovn/controller/pinctrl.c | 15 ++++++++--
6 files changed, 44 insertions(+), 111 deletions(-)
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index f76e692..e03ab89 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -107,12 +107,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) {
@@ -125,8 +127,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;
@@ -136,30 +136,36 @@ add_local_datapath__(struct controller_ctx *ctx,
return;
}
+ 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);
+
/* 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];
+ 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,
- false, depth + 1, local_datapaths);
+ add_local_datapath__(ctx, peer->datapath, false,
+ depth + 1, local_datapaths);
}
}
}
}
+ 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);
}
@@ -355,7 +361,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 sbrec_chassis *chassis_rec,
const struct sbrec_port_binding *binding_rec,
struct hmap *qos_map,
@@ -374,13 +379,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")) {
@@ -389,7 +394,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")) {
@@ -397,7 +402,7 @@ consider_local_datapath(struct controller_ctx *ctx,
"redirect-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,
false, local_datapaths);
}
} else if (!strcmp(binding_rec->type, "l3gateway")) {
@@ -405,7 +410,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")) {
@@ -444,7 +449,6 @@ consider_local_datapath(struct controller_ctx *ctx,
void
binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
const struct sbrec_chassis *chassis_rec,
- const struct ldatapath_index *ldatapaths,
struct hmap *local_datapaths,
struct sset *local_lports)
{
@@ -467,7 +471,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,
+ consider_local_datapath(ctx,
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 26ca5ab..7ff4095 100644
--- a/ovn/controller/binding.h
+++ b/ovn/controller/binding.h
@@ -21,7 +21,6 @@
struct controller_ctx;
struct hmap;
-struct ldatapath_index;
struct ovsdb_idl;
struct ovsrec_bridge;
struct sbrec_chassis;
@@ -29,7 +28,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 *,
struct hmap *local_datapaths,
struct sset *all_lports);
bool binding_cleanup(struct controller_ctx *, const struct sbrec_chassis *);
diff --git a/ovn/controller/lport.c b/ovn/controller/lport.c
index aed6389..4b216dd 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',
diff --git a/ovn/controller/lport.h b/ovn/controller/lport.h
index 60532c8..59fe94c 100644
--- a/ovn/controller/lport.h
+++ b/ovn/controller/lport.h
@@ -30,27 +30,6 @@ struct sbrec_datapath_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_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 4455083..497991a 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -552,6 +552,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");
@@ -662,14 +668,11 @@ 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;
- ldatapath_index_init(&ldatapaths, ctx.ovnsb_idl);
-
const struct sbrec_chassis *chassis = NULL;
if (chassis_id) {
chassis = chassis_run(&ctx, chassis_id, br_int);
encaps_run(&ctx, br_int, chassis_id);
- binding_run(&ctx, br_int, chassis, &ldatapaths,
+ binding_run(&ctx, br_int, chassis,
&local_datapaths, &local_lports);
}
@@ -739,8 +742,6 @@ main(int argc, char *argv[])
free(pending_pkt.flow_s);
}
- ldatapath_index_destroy(&ldatapaths);
-
sset_destroy(&local_lports);
struct local_datapath *cur_node, *next_node;
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index e1fb188..ac0e2c7 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -1465,7 +1465,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;
}
@@ -1474,14 +1482,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.9.4
More information about the dev
mailing list