[ovs-dev] [PATCH v17 2/5] Convert binding_run to incremental processing.
Ryan Moats
rmoats at us.ibm.com
Sun May 22 21:36:19 UTC 2016
Ensure that the entire port binding table is processed
when chassis are added/removed or when get_local_iface_ids
finds new ports on the local vswitch.
Side effects:
- Persist local_datapaths and patch_datapaths across runs so
that changes to either can be used as a trigger to reset
incremental flow processing.
- Persist all_lports structure
- Revert commit 9baaabfff3c7df014e9acbd4c68189b568552ca9
as these changes are not desirable once local_datatpath
is persisted.
Signed-off-by: Ryan Moats <rmoats at us.ibm.com>
---
ovn/controller/binding.c | 177 ++++++++++++++++++++++++++++++++--------
ovn/controller/binding.h | 1 +
ovn/controller/encaps.c | 3 +
ovn/controller/ovn-controller.c | 28 ++-----
ovn/controller/ovn-controller.h | 3 +
ovn/controller/patch.c | 3 +-
ovn/controller/physical.c | 2 +-
7 files changed, 158 insertions(+), 59 deletions(-)
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index a0d8b96..4235d5c 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -27,6 +27,16 @@
VLOG_DEFINE_THIS_MODULE(binding);
+static struct sset all_lports = SSET_INITIALIZER(&all_lports);
+
+static bool process_full_binding = false;
+
+void
+reset_binding_processing(void)
+{
+ process_full_binding = true;
+}
+
void
binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
{
@@ -72,6 +82,10 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int, struct shash *lports)
continue;
}
shash_add(lports, iface_id, iface_rec);
+ if (!sset_find(&all_lports, iface_id)) {
+ sset_add(&all_lports, iface_id);
+ reset_binding_processing();
+ }
}
}
}
@@ -121,9 +135,59 @@ update_ct_zones(struct sset *lports, struct simap *ct_zones,
}
}
+/* Contains "struct local_datpath" nodes whose hash values are the
+ * row uuids of datapaths with at least one local port binding. */
+struct hmap local_datapaths_by_uuid =
+ HMAP_INITIALIZER(&local_datapaths_by_uuid);
+
+static struct local_datapath *
+local_datapath_lookup_by_uuid(struct hmap *hmap_p, const struct uuid *uuid)
+{
+ struct local_datapath *ld;
+ HMAP_FOR_EACH_WITH_HASH(ld, uuid_hmap_node, uuid_hash(uuid), hmap_p) {
+ if (uuid_equals(ld->uuid, uuid)) {
+ return ld;
+ }
+ }
+ return NULL;
+}
+
+static void
+remove_local_datapath(struct hmap *local_datapaths, struct local_datapath *ld)
+{
+ if (ld->logical_port) {
+ sset_find_and_delete(&all_lports, ld->logical_port);
+ free(ld->logical_port);
+ ld->logical_port = NULL;
+ }
+ hmap_remove(local_datapaths, &ld->hmap_node);
+ hmap_remove(&local_datapaths_by_uuid, &ld->uuid_hmap_node);
+ free(ld);
+}
+
+static void
+remove_local_datapath_by_binding(struct hmap *local_datapaths,
+ const struct sbrec_port_binding *binding_rec)
+{
+ const struct uuid *uuid = &binding_rec->header_.uuid;
+ struct local_datapath *ld = local_datapath_lookup_by_uuid(local_datapaths,
+ uuid);
+ if (ld) {
+ remove_local_datapath(local_datapaths, ld);
+ } else {
+ struct local_datapath *ld;
+ HMAP_FOR_EACH (ld, hmap_node, local_datapaths) {
+ if (ld->localnet_port == binding_rec) {
+ ld->localnet_port = NULL;
+ }
+ }
+ }
+}
+
static void
add_local_datapath(struct hmap *local_datapaths,
- const struct sbrec_port_binding *binding_rec)
+ const struct sbrec_port_binding *binding_rec,
+ const struct uuid *uuid)
{
if (get_local_datapath(local_datapaths,
binding_rec->datapath->tunnel_key)) {
@@ -131,8 +195,12 @@ add_local_datapath(struct hmap *local_datapaths,
}
struct local_datapath *ld = xzalloc(sizeof *ld);
+ ld->logical_port = xstrdup(binding_rec->logical_port);
+ ld->uuid = &binding_rec->header_.uuid;
hmap_insert(local_datapaths, &ld->hmap_node,
binding_rec->datapath->tunnel_key);
+ hmap_insert(&local_datapaths_by_uuid, &ld->uuid_hmap_node,
+ uuid_hash(uuid));
}
static void
@@ -146,36 +214,14 @@ update_qos(const struct ovsrec_interface *iface_rec,
ovsrec_interface_set_ingress_policing_burst(iface_rec, MAX(0, burst));
}
-void
-binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
- const char *chassis_id, struct simap *ct_zones,
- unsigned long *ct_zone_bitmap, struct hmap *local_datapaths)
+static void
+consider_local_datapath(struct controller_ctx *ctx, struct shash *lports,
+ const struct sbrec_chassis *chassis_rec,
+ const struct sbrec_port_binding *binding_rec,
+ struct hmap *local_datapaths)
{
- const struct sbrec_chassis *chassis_rec;
- const struct sbrec_port_binding *binding_rec;
-
- chassis_rec = get_chassis(ctx->ovnsb_idl, chassis_id);
-
- struct shash lports = SHASH_INITIALIZER(&lports);
- if (br_int) {
- get_local_iface_ids(br_int, &lports);
- } else {
- /* We have no integration bridge, therefore no local logical ports.
- * We'll remove our chassis from all port binding records below. */
- }
-
- struct sset all_lports = SSET_INITIALIZER(&all_lports);
- struct shash_node *node;
- SHASH_FOR_EACH (node, &lports) {
- sset_add(&all_lports, node->name);
- }
-
- /* Run through each binding record to see if it is resident on this
- * 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) {
const struct ovsrec_interface *iface_rec
- = shash_find_and_delete(&lports, binding_rec->logical_port);
+ = shash_find_and_delete(lports, binding_rec->logical_port);
if (iface_rec
|| (binding_rec->parent_port && binding_rec->parent_port[0] &&
sset_contains(&all_lports, binding_rec->parent_port))) {
@@ -183,12 +229,15 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
/* Add child logical port to the set of all local ports. */
sset_add(&all_lports, binding_rec->logical_port);
}
- add_local_datapath(local_datapaths, binding_rec);
+ add_local_datapath(local_datapaths, binding_rec,
+ &binding_rec->header_.uuid);
if (iface_rec && ctx->ovs_idl_txn) {
update_qos(iface_rec, binding_rec);
}
- if (ctx->ovnsb_idl_txn && chassis_rec
- && binding_rec->chassis != chassis_rec) {
+ if (binding_rec->chassis == chassis_rec) {
+ return;
+ }
+ if (ctx->ovnsb_idl_txn) {
if (binding_rec->chassis) {
VLOG_INFO("Changing chassis for lport %s from %s to %s.",
binding_rec->logical_port,
@@ -200,7 +249,7 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
}
sbrec_port_binding_set_chassis(binding_rec, chassis_rec);
}
- } else if (chassis_rec && binding_rec->chassis == chassis_rec) {
+ } else if (binding_rec->chassis == chassis_rec) {
if (ctx->ovnsb_idl_txn) {
VLOG_INFO("Releasing lport %s from this chassis.",
binding_rec->logical_port);
@@ -214,16 +263,72 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
* a patch port for each one. */
sset_add(&all_lports, binding_rec->logical_port);
}
+}
+
+void
+binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
+ const char *chassis_id, struct simap *ct_zones,
+ unsigned long *ct_zone_bitmap, struct hmap *local_datapaths)
+{
+ const struct sbrec_chassis *chassis_rec;
+ const struct sbrec_port_binding *binding_rec;
+
+ chassis_rec = get_chassis(ctx->ovnsb_idl, chassis_id);
+ if (!chassis_rec) {
+ return;
}
- SHASH_FOR_EACH (node, &lports) {
- VLOG_DBG("No port binding record for lport %s", node->name);
+ struct shash lports = SHASH_INITIALIZER(&lports);
+ if (br_int) {
+ get_local_iface_ids(br_int, &lports);
+ } else {
+ /* We have no integration bridge, therefore no local logical ports.
+ * We'll remove our chassis from all port binding records below. */
+ }
+
+ /* Run through each binding record to see if it is resident on this
+ * chassis and update the binding accordingly. This includes both
+ * directly connected logical ports and children of those ports. */
+ if (process_full_binding) {
+ struct hmap keep_local_datapath_by_uuid =
+ HMAP_INITIALIZER(&keep_local_datapath_by_uuid);
+ SBREC_PORT_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
+ consider_local_datapath(ctx, &lports, chassis_rec, binding_rec,
+ local_datapaths);
+ struct local_datapath *ld = xzalloc(sizeof *ld);
+ ld->uuid = &binding_rec->header_.uuid;
+ hmap_insert(&keep_local_datapath_by_uuid, &ld->uuid_hmap_node,
+ uuid_hash(ld->uuid));
+ }
+ struct local_datapath *old_ld, *next;
+ HMAP_FOR_EACH_SAFE (old_ld, next, hmap_node, local_datapaths) {
+ if (!local_datapath_lookup_by_uuid(&keep_local_datapath_by_uuid,
+ old_ld->uuid)) {
+ remove_local_datapath(local_datapaths, old_ld);
+ }
+ }
+ HMAP_FOR_EACH_POP(old_ld, hmap_node, &keep_local_datapath_by_uuid) {
+ ;
+ }
+ hmap_destroy(&keep_local_datapath_by_uuid);
+ process_full_binding = false;
+ } else {
+ SBREC_PORT_BINDING_FOR_EACH_TRACKED(binding_rec, ctx->ovnsb_idl) {
+ bool is_deleted = sbrec_port_binding_row_get_seqno(binding_rec,
+ OVSDB_IDL_CHANGE_DELETE) > 0;
+
+ if (is_deleted) {
+ remove_local_datapath_by_binding(local_datapaths, binding_rec);
+ continue;
+ }
+ consider_local_datapath(ctx, &lports, chassis_rec, binding_rec,
+ local_datapaths);
+ }
}
update_ct_zones(&all_lports, ct_zones, ct_zone_bitmap);
shash_destroy(&lports);
- sset_destroy(&all_lports);
}
/* Returns true if the database is all cleaned up, false if more work is
diff --git a/ovn/controller/binding.h b/ovn/controller/binding.h
index 6e19c10..df41070 100644
--- a/ovn/controller/binding.h
+++ b/ovn/controller/binding.h
@@ -26,6 +26,7 @@ struct ovsrec_bridge;
struct simap;
void binding_register_ovs_idl(struct ovsdb_idl *);
+void reset_binding_processing(void);
void binding_run(struct controller_ctx *, const struct ovsrec_bridge *br_int,
const char *chassis_id, struct simap *ct_zones,
unsigned long *ct_zone_bitmap, struct hmap *local_datapaths);
diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
index 6bd2f57..a582c45 100644
--- a/ovn/controller/encaps.c
+++ b/ovn/controller/encaps.c
@@ -15,6 +15,7 @@
#include <config.h>
#include "encaps.h"
+#include "binding.h"
#include "lflow.h"
#include "lib/hash.h"
@@ -232,6 +233,7 @@ tunnel_add(const struct sbrec_chassis *chassis_rec,
sset_add(&tc.port_names, port_name);
free(port_name);
free(ports);
+ reset_binding_processing();
process_full_encaps = true;
}
@@ -415,6 +417,7 @@ encaps_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
hmap_remove(&tc.tunnel_hmap_by_uuid,
&port_hash->uuid_node);
free(port_hash);
+ reset_binding_processing();
}
continue;
}
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 3b14518..da1d419 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -252,6 +252,11 @@ get_ovnsb_remote_probe_interval(struct ovsdb_idl *ovs_idl, int *value)
return false;
}
+/* Contains "struct local_datpath" nodes whose hash values are the
+ * tunnel_key of datapaths with at least one local port binding. */
+static struct hmap local_datapaths = HMAP_INITIALIZER(&local_datapaths);
+static struct hmap patched_datapaths = HMAP_INITIALIZER(&patched_datapaths);
+
int
main(int argc, char *argv[])
{
@@ -341,6 +346,8 @@ main(int argc, char *argv[])
free(ovnsb_remote);
ovnsb_remote = new_ovnsb_remote;
ovsdb_idl_set_remote(ovnsb_idl_loop.idl, ovnsb_remote, true);
+ hmap_clear(&local_datapaths);
+ hmap_clear(&patched_datapaths);
} else {
free(new_ovnsb_remote);
}
@@ -352,12 +359,6 @@ main(int argc, char *argv[])
.ovnsb_idl_txn = ovsdb_idl_loop_run(&ovnsb_idl_loop),
};
- /* Contains "struct local_datpath" nodes whose hash values are the
- * tunnel_key of datapaths with at least one local port binding. */
- struct hmap local_datapaths = HMAP_INITIALIZER(&local_datapaths);
-
- struct hmap patched_datapaths = HMAP_INITIALIZER(&patched_datapaths);
-
const struct ovsrec_bridge *br_int = get_br_int(&ctx);
const char *chassis_id = get_chassis_id(ctx.ovs_idl);
@@ -394,21 +395,6 @@ main(int argc, char *argv[])
lport_index_destroy(&lports);
}
- struct local_datapath *cur_node, *next_node;
- HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node, &local_datapaths) {
- hmap_remove(&local_datapaths, &cur_node->hmap_node);
- free(cur_node);
- }
- hmap_destroy(&local_datapaths);
-
- struct patched_datapath *pd_cur_node, *pd_next_node;
- HMAP_FOR_EACH_SAFE (pd_cur_node, pd_next_node, hmap_node,
- &patched_datapaths) {
- hmap_remove(&patched_datapaths, &pd_cur_node->hmap_node);
- free(pd_cur_node);
- }
- hmap_destroy(&patched_datapaths);
-
unixctl_server_run(unixctl);
unixctl_server_wait(unixctl);
diff --git a/ovn/controller/ovn-controller.h b/ovn/controller/ovn-controller.h
index 9af7959..4c8f3a2 100644
--- a/ovn/controller/ovn-controller.h
+++ b/ovn/controller/ovn-controller.h
@@ -38,6 +38,9 @@ struct controller_ctx {
* the localnet port */
struct local_datapath {
struct hmap_node hmap_node;
+ struct hmap_node uuid_hmap_node;
+ const struct uuid *uuid;
+ char *logical_port;
const struct sbrec_port_binding *localnet_port;
};
diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
index 4808146..e07e829 100644
--- a/ovn/controller/patch.c
+++ b/ovn/controller/patch.c
@@ -185,7 +185,8 @@ add_bridge_mappings(struct controller_ctx *ctx,
* to create patch ports for it. */
continue;
}
- if (ld->localnet_port) {
+ if (ld->localnet_port && strcmp(ld->localnet_port->logical_port,
+ binding->logical_port)) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
VLOG_WARN_RL(&rl, "localnet port '%s' already set for datapath "
"'%"PRId64"', skipping the new port '%s'.",
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 576c695..d1e8479 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -310,7 +310,7 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
if (!binding->chassis) {
continue;
}
- if (localnet_port) {
+ if (localnet_port && localnet_port->logical_port) {
ofport = u16_to_ofp(simap_get(&localvif_to_ofport,
localnet_port->logical_port));
if (!ofport) {
--
1.9.1
More information about the dev
mailing list