[ovs-dev] [PATCH v8 7/7] ovn-controller: use idl indexes for logical datapath
Lance Richardson
lrichard at redhat.com
Thu Aug 3 18:20:32 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>
---
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
More information about the dev
mailing list