[ovs-dev] [PATCH v2 06/21] ovn-controller: Factor encapsulation code out of chassis code.
Ben Pfaff
blp at nicira.com
Tue Jul 28 15:44:24 UTC 2015
These two pieces of code have different requirements and hardly anything in
common, and I found it easier to understand and reasonably modify the
overall structure of the program when they were separated.
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
ovn/controller/automake.mk | 2 +
ovn/controller/chassis.c | 315 ++-------------------------------
ovn/controller/chassis.h | 6 +-
ovn/controller/{chassis.c => encaps.c} | 129 ++------------
ovn/controller/{chassis.h => encaps.h} | 12 +-
ovn/controller/ovn-controller.c | 12 +-
6 files changed, 49 insertions(+), 427 deletions(-)
copy ovn/controller/{chassis.c => encaps.c} (72%)
copy ovn/controller/{chassis.h => encaps.h} (79%)
diff --git a/ovn/controller/automake.mk b/ovn/controller/automake.mk
index 25b5f8b..9ed6bec 100644
--- a/ovn/controller/automake.mk
+++ b/ovn/controller/automake.mk
@@ -4,6 +4,8 @@ ovn_controller_ovn_controller_SOURCES = \
ovn/controller/binding.h \
ovn/controller/chassis.c \
ovn/controller/chassis.h \
+ ovn/controller/encaps.c \
+ ovn/controller/encaps.h \
ovn/controller/ofctrl.c \
ovn/controller/ofctrl.h \
ovn/controller/ovn-controller.c \
diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
index 4bd39e7..5f1c194 100644
--- a/ovn/controller/chassis.c
+++ b/ovn/controller/chassis.c
@@ -16,10 +16,6 @@
#include <config.h>
#include "chassis.h"
-#include "lib/hash.h"
-#include "lib/poll-loop.h"
-#include "lib/sset.h"
-#include "lib/util.h"
#include "lib/vswitch-idl.h"
#include "openvswitch/vlog.h"
#include "ovn/lib/ovn-sb-idl.h"
@@ -32,21 +28,15 @@ chassis_init(struct controller_ctx *ctx)
{
ovsdb_idl_add_table(ctx->ovs_idl, &ovsrec_table_open_vswitch);
ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_open_vswitch_col_external_ids);
- ovsdb_idl_add_table(ctx->ovs_idl, &ovsrec_table_bridge);
- ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_bridge_col_ports);
- ovsdb_idl_add_table(ctx->ovs_idl, &ovsrec_table_port);
- ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_port_col_name);
- ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_port_col_interfaces);
- ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_port_col_external_ids);
- ovsdb_idl_add_table(ctx->ovs_idl, &ovsrec_table_interface);
- ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_interface_col_name);
- ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_interface_col_type);
- ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_interface_col_options);
}
-static void
-register_chassis(struct controller_ctx *ctx)
+void
+chassis_run(struct controller_ctx *ctx)
{
+ if (!ctx->ovnsb_idl_txn) {
+ return;
+ }
+
const struct sbrec_chassis *chassis_rec;
const struct ovsrec_open_vswitch *cfg;
const char *encap_type, *encap_ip;
@@ -109,303 +99,22 @@ register_chassis(struct controller_ctx *ctx)
inited = true;
}
-/* Enough context to create a new tunnel, using tunnel_add(). */
-struct tunnel_ctx {
- /* Contains "struct port_hash_node"s. Used to figure out what
- * existing tunnels should be deleted: we index all of the OVN encap
- * rows into this data structure, then as existing rows are
- * generated we remove them. After generating all the rows, any
- * remaining in 'tunnel_hmap' must be deleted from the database. */
- struct hmap tunnel_hmap;
-
- /* Names of all ports in the bridge, to allow checking uniqueness when
- * adding a new tunnel. */
- struct sset port_names;
-
- struct ovsdb_idl_txn *ovs_txn;
- const struct ovsrec_bridge *br_int;
-};
-
-struct port_hash_node {
- struct hmap_node node;
- const struct ovsrec_port *port;
- const struct ovsrec_bridge *bridge;
-};
-
-static size_t
-port_hash(const char *chassis_id, const char *type, const char *ip)
-{
- size_t hash = hash_string(chassis_id, 0);
- hash = hash_string(type, hash);
- return hash_string(ip, hash);
-}
-
-static size_t
-port_hash_rec(const struct ovsrec_port *port)
-{
- const char *chassis_id, *ip;
- const struct ovsrec_interface *iface;
-
- chassis_id = smap_get(&port->external_ids, "ovn-chassis-id");
-
- if (!chassis_id || !port->n_interfaces) {
- /* This should not happen for an OVN-created port. */
- return 0;
- }
-
- iface = port->interfaces[0];
- ip = smap_get(&iface->options, "remote_ip");
-
- return port_hash(chassis_id, iface->type, ip);
-}
-
-static char *
-tunnel_create_name(struct tunnel_ctx *tc, const char *chassis_id)
-{
- int i;
-
- for (i = 0; i < UINT16_MAX; i++) {
- char *port_name;
- port_name = xasprintf("ovn-%.6s-%x", chassis_id, i);
-
- if (!sset_contains(&tc->port_names, port_name)) {
- return port_name;
- }
-
- free(port_name);
- }
-
- return NULL;
-}
-
-
-static void
-tunnel_add(struct tunnel_ctx *tc, const char *new_chassis_id,
- const struct sbrec_encap *encap)
-{
- struct port_hash_node *hash_node;
-
- /* Check whether such a row already exists in OVS. If so, remove it
- * from 'tc->tunnel_hmap' and we're done. */
- HMAP_FOR_EACH_WITH_HASH (hash_node, node,
- port_hash(new_chassis_id,
- encap->type, encap->ip),
- &tc->tunnel_hmap) {
- const struct ovsrec_port *port = hash_node->port;
- const char *chassis_id = smap_get(&port->external_ids,
- "ovn-chassis-id");
- const struct ovsrec_interface *iface;
- const char *ip;
-
- if (!chassis_id || !port->n_interfaces) {
- continue;
- }
-
- iface = port->interfaces[0];
- ip = smap_get(&iface->options, "remote_ip");
- if (!ip) {
- continue;
- }
-
- if (!strcmp(new_chassis_id, chassis_id)
- && !strcmp(encap->type, iface->type)
- && !strcmp(encap->ip, ip)) {
- hmap_remove(&tc->tunnel_hmap, &hash_node->node);
- free(hash_node);
- return;
- }
- }
-
- /* No such port, so add one. */
- struct smap external_ids = SMAP_INITIALIZER(&external_ids);
- struct smap options = SMAP_INITIALIZER(&options);
- struct ovsrec_port *port, **ports;
- struct ovsrec_interface *iface;
- char *port_name;
- size_t i;
-
- port_name = tunnel_create_name(tc, new_chassis_id);
- if (!port_name) {
- VLOG_WARN("Unable to allocate unique name for '%s' tunnel",
- new_chassis_id);
- return;
- }
-
- iface = ovsrec_interface_insert(tc->ovs_txn);
- ovsrec_interface_set_name(iface, port_name);
- ovsrec_interface_set_type(iface, encap->type);
- smap_add(&options, "remote_ip", encap->ip);
- smap_add(&options, "key", "flow");
- ovsrec_interface_set_options(iface, &options);
- smap_destroy(&options);
-
- port = ovsrec_port_insert(tc->ovs_txn);
- ovsrec_port_set_name(port, port_name);
- ovsrec_port_set_interfaces(port, &iface, 1);
- smap_add(&external_ids, "ovn-chassis-id", new_chassis_id);
- ovsrec_port_set_external_ids(port, &external_ids);
- smap_destroy(&external_ids);
-
- ports = xmalloc(sizeof *tc->br_int->ports * (tc->br_int->n_ports + 1));
- for (i = 0; i < tc->br_int->n_ports; i++) {
- ports[i] = tc->br_int->ports[i];
- }
- ports[tc->br_int->n_ports] = port;
- ovsrec_bridge_verify_ports(tc->br_int);
- ovsrec_bridge_set_ports(tc->br_int, ports, tc->br_int->n_ports + 1);
-
- sset_add(&tc->port_names, port_name);
- free(port_name);
- free(ports);
-}
-
-static void
-bridge_delete_port(const struct ovsrec_bridge *br,
- const struct ovsrec_port *port)
-{
- struct ovsrec_port **ports;
- size_t i, n;
-
- ports = xmalloc(sizeof *br->ports * br->n_ports);
- for (i = n = 0; i < br->n_ports; i++) {
- if (br->ports[i] != port) {
- ports[n++] = br->ports[i];
- }
- }
- ovsrec_bridge_verify_ports(br);
- ovsrec_bridge_set_ports(br, ports, n);
- free(ports);
-}
-
-static struct sbrec_encap *
-preferred_encap(const struct sbrec_chassis *chassis_rec)
-{
- size_t i;
-
- /* For hypervisors, we only support Geneve and STT encapsulations.
- * Sets are returned alphabetically, so "geneve" will be preferred
- * over "stt". */
- for (i = 0; i < chassis_rec->n_encaps; i++) {
- if (!strcmp(chassis_rec->encaps[i]->type, "geneve")
- || !strcmp(chassis_rec->encaps[i]->type, "stt")) {
- return chassis_rec->encaps[i];
- }
- }
-
- return NULL;
-}
-
-static void
-update_encaps(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int)
-{
- const struct sbrec_chassis *chassis_rec;
- const struct ovsrec_bridge *br;
-
- struct tunnel_ctx tc = {
- .tunnel_hmap = HMAP_INITIALIZER(&tc.tunnel_hmap),
- .port_names = SSET_INITIALIZER(&tc.port_names),
- .br_int = br_int
- };
-
- tc.ovs_txn = ctx->ovs_idl_txn;
- ovsdb_idl_txn_add_comment(tc.ovs_txn,
- "ovn-controller: modifying OVS tunnels '%s'",
- ctx->chassis_id);
-
- /* Collect all port names into tc.port_names.
- *
- * Collect all the OVN-created tunnels into tc.tunnel_hmap. */
- OVSREC_BRIDGE_FOR_EACH(br, ctx->ovs_idl) {
- size_t i;
-
- for (i = 0; i < br->n_ports; i++) {
- const struct ovsrec_port *port = br->ports[i];
-
- sset_add(&tc.port_names, port->name);
-
- if (smap_get(&port->external_ids, "ovn-chassis-id")) {
- struct port_hash_node *hash_node = xzalloc(sizeof *hash_node);
- hash_node->bridge = br;
- hash_node->port = port;
- hmap_insert(&tc.tunnel_hmap, &hash_node->node,
- port_hash_rec(port));
- }
- }
- }
-
- SBREC_CHASSIS_FOR_EACH(chassis_rec, ctx->ovnsb_idl) {
- if (strcmp(chassis_rec->name, ctx->chassis_id)) {
- /* Create tunnels to the other chassis. */
- const struct sbrec_encap *encap = preferred_encap(chassis_rec);
- if (!encap) {
- VLOG_INFO("No supported encaps for '%s'", chassis_rec->name);
- continue;
- }
- tunnel_add(&tc, chassis_rec->name, encap);
- }
- }
-
- /* Delete any existing OVN tunnels that were not still around. */
- struct port_hash_node *hash_node, *next_hash_node;
- HMAP_FOR_EACH_SAFE (hash_node, next_hash_node, node, &tc.tunnel_hmap) {
- hmap_remove(&tc.tunnel_hmap, &hash_node->node);
- bridge_delete_port(hash_node->bridge, hash_node->port);
- free(hash_node);
- }
- hmap_destroy(&tc.tunnel_hmap);
- sset_destroy(&tc.port_names);
-}
-
-void
-chassis_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int)
-{
- if (ctx->ovnsb_idl_txn) {
- register_chassis(ctx);
- }
-
- if (ctx->ovs_idl_txn) {
- update_encaps(ctx, br_int);
- }
-}
-
/* Returns true if the database is all cleaned up, false if more work is
* required. */
bool
-chassis_cleanup(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int)
+chassis_cleanup(struct controller_ctx *ctx)
{
- if (!ctx->ovnsb_idl_txn || !ctx->ovs_idl_txn) {
- return false;
- }
-
- bool any_changes = false;
-
/* Delete Chassis row. */
const struct sbrec_chassis *chassis_rec
= get_chassis_by_name(ctx->ovnsb_idl, ctx->chassis_id);
- if (chassis_rec) {
+ if (!chassis_rec) {
+ return true;
+ }
+ if (ctx->ovnsb_idl_txn) {
ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
"ovn-controller: unregistering chassis '%s'",
ctx->chassis_id);
sbrec_chassis_delete(chassis_rec);
- any_changes = true;
- }
-
- /* Delete all the OVS-created tunnels from the integration bridge. */
- ovsdb_idl_txn_add_comment(ctx->ovs_idl_txn,
- "ovn-controller: destroying tunnels");
- struct ovsrec_port **ports
- = xmalloc(sizeof *br_int->ports * br_int->n_ports);
- size_t n = 0;
- for (size_t i = 0; i < br_int->n_ports; i++) {
- if (!smap_get(&br_int->ports[i]->external_ids,
- "ovn-chassis-id")) {
- ports[n++] = br_int->ports[i];
- any_changes = true;
- }
}
- ovsrec_bridge_verify_ports(br_int);
- ovsrec_bridge_set_ports(br_int, ports, n);
- free(ports);
-
- return !any_changes;
+ return false;
}
diff --git a/ovn/controller/chassis.h b/ovn/controller/chassis.h
index bfacde1..18582ec 100644
--- a/ovn/controller/chassis.h
+++ b/ovn/controller/chassis.h
@@ -22,9 +22,7 @@ struct controller_ctx;
struct ovsrec_bridge;
void chassis_init(struct controller_ctx *);
-void chassis_run(struct controller_ctx *,
- const struct ovsrec_bridge *br_int);
-bool chassis_cleanup(struct controller_ctx *,
- const struct ovsrec_bridge *br_int);
+void chassis_run(struct controller_ctx *);
+bool chassis_cleanup(struct controller_ctx *);
#endif /* ovn/chassis.h */
diff --git a/ovn/controller/chassis.c b/ovn/controller/encaps.c
similarity index 72%
copy from ovn/controller/chassis.c
copy to ovn/controller/encaps.c
index 4bd39e7..529bf1e 100644
--- a/ovn/controller/chassis.c
+++ b/ovn/controller/encaps.c
@@ -14,10 +14,9 @@
*/
#include <config.h>
-#include "chassis.h"
+#include "encaps.h"
#include "lib/hash.h"
-#include "lib/poll-loop.h"
#include "lib/sset.h"
#include "lib/util.h"
#include "lib/vswitch-idl.h"
@@ -25,13 +24,11 @@
#include "ovn/lib/ovn-sb-idl.h"
#include "ovn-controller.h"
-VLOG_DEFINE_THIS_MODULE(chassis);
+VLOG_DEFINE_THIS_MODULE(encaps);
void
-chassis_init(struct controller_ctx *ctx)
+encaps_init(struct controller_ctx *ctx)
{
- ovsdb_idl_add_table(ctx->ovs_idl, &ovsrec_table_open_vswitch);
- ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_open_vswitch_col_external_ids);
ovsdb_idl_add_table(ctx->ovs_idl, &ovsrec_table_bridge);
ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_bridge_col_ports);
ovsdb_idl_add_table(ctx->ovs_idl, &ovsrec_table_port);
@@ -44,71 +41,6 @@ chassis_init(struct controller_ctx *ctx)
ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_interface_col_options);
}
-static void
-register_chassis(struct controller_ctx *ctx)
-{
- const struct sbrec_chassis *chassis_rec;
- const struct ovsrec_open_vswitch *cfg;
- const char *encap_type, *encap_ip;
- struct sbrec_encap *encap_rec;
- static bool inited = false;
-
- chassis_rec = get_chassis_by_name(ctx->ovnsb_idl, ctx->chassis_id);
-
- /* xxx Need to support more than one encap. Also need to support
- * xxx encap options. */
- cfg = ovsrec_open_vswitch_first(ctx->ovs_idl);
- if (!cfg) {
- VLOG_INFO("No Open_vSwitch row defined.");
- return;
- }
-
- encap_type = smap_get(&cfg->external_ids, "ovn-encap-type");
- encap_ip = smap_get(&cfg->external_ids, "ovn-encap-ip");
- if (!encap_type || !encap_ip) {
- VLOG_INFO("Need to specify an encap type and ip");
- return;
- }
-
- if (chassis_rec) {
- int i;
-
- for (i = 0; i < chassis_rec->n_encaps; i++) {
- if (!strcmp(chassis_rec->encaps[i]->type, encap_type)
- && !strcmp(chassis_rec->encaps[i]->ip, encap_ip)) {
- /* Nothing changed. */
- inited = true;
- return;
- } else if (!inited) {
- VLOG_WARN("Chassis config changing on startup, make sure "
- "multiple chassis are not configured : %s/%s->%s/%s",
- chassis_rec->encaps[i]->type,
- chassis_rec->encaps[i]->ip,
- encap_type, encap_ip);
- }
-
- }
- }
-
- ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
- "ovn-controller: registering chassis '%s'",
- ctx->chassis_id);
-
- if (!chassis_rec) {
- chassis_rec = sbrec_chassis_insert(ctx->ovnsb_idl_txn);
- sbrec_chassis_set_name(chassis_rec, ctx->chassis_id);
- }
-
- encap_rec = sbrec_encap_insert(ctx->ovnsb_idl_txn);
-
- sbrec_encap_set_type(encap_rec, encap_type);
- sbrec_encap_set_ip(encap_rec, encap_ip);
-
- sbrec_chassis_set_encaps(chassis_rec, &encap_rec, 1);
-
- inited = true;
-}
-
/* Enough context to create a new tunnel, using tunnel_add(). */
struct tunnel_ctx {
/* Contains "struct port_hash_node"s. Used to figure out what
@@ -295,9 +227,13 @@ preferred_encap(const struct sbrec_chassis *chassis_rec)
return NULL;
}
-static void
-update_encaps(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int)
+void
+encaps_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int)
{
+ if (!ctx->ovs_idl_txn) {
+ return;
+ }
+
const struct sbrec_chassis *chassis_rec;
const struct ovsrec_bridge *br;
@@ -356,55 +292,28 @@ update_encaps(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int)
sset_destroy(&tc.port_names);
}
-void
-chassis_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int)
-{
- if (ctx->ovnsb_idl_txn) {
- register_chassis(ctx);
- }
-
- if (ctx->ovs_idl_txn) {
- update_encaps(ctx, br_int);
- }
-}
-
/* Returns true if the database is all cleaned up, false if more work is
* required. */
bool
-chassis_cleanup(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int)
+encaps_cleanup(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int)
{
- if (!ctx->ovnsb_idl_txn || !ctx->ovs_idl_txn) {
- return false;
- }
-
- bool any_changes = false;
-
- /* Delete Chassis row. */
- const struct sbrec_chassis *chassis_rec
- = get_chassis_by_name(ctx->ovnsb_idl, ctx->chassis_id);
- if (chassis_rec) {
- ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
- "ovn-controller: unregistering chassis '%s'",
- ctx->chassis_id);
- sbrec_chassis_delete(chassis_rec);
- any_changes = true;
- }
-
/* Delete all the OVS-created tunnels from the integration bridge. */
- ovsdb_idl_txn_add_comment(ctx->ovs_idl_txn,
- "ovn-controller: destroying tunnels");
struct ovsrec_port **ports
= xmalloc(sizeof *br_int->ports * br_int->n_ports);
size_t n = 0;
for (size_t i = 0; i < br_int->n_ports; i++) {
- if (!smap_get(&br_int->ports[i]->external_ids,
- "ovn-chassis-id")) {
+ if (!smap_get(&br_int->ports[i]->external_ids, "ovn-chassis-id")) {
ports[n++] = br_int->ports[i];
- any_changes = true;
}
}
- ovsrec_bridge_verify_ports(br_int);
- ovsrec_bridge_set_ports(br_int, ports, n);
+
+ bool any_changes = n != br_int->n_ports;
+ if (any_changes && ctx->ovs_idl_txn) {
+ ovsdb_idl_txn_add_comment(ctx->ovs_idl_txn,
+ "ovn-controller: destroying tunnels");
+ ovsrec_bridge_verify_ports(br_int);
+ ovsrec_bridge_set_ports(br_int, ports, n);
+ }
free(ports);
return !any_changes;
diff --git a/ovn/controller/chassis.h b/ovn/controller/encaps.h
similarity index 79%
copy from ovn/controller/chassis.h
copy to ovn/controller/encaps.h
index bfacde1..0ec132d 100644
--- a/ovn/controller/chassis.h
+++ b/ovn/controller/encaps.h
@@ -13,18 +13,18 @@
* limitations under the License.
*/
-#ifndef OVN_CHASSIS_H
-#define OVN_CHASSIS_H 1
+#ifndef OVN_ENCAPS_H
+#define OVN_ENCAPS_H 1
#include <stdbool.h>
struct controller_ctx;
struct ovsrec_bridge;
-void chassis_init(struct controller_ctx *);
-void chassis_run(struct controller_ctx *,
+void encaps_init(struct controller_ctx *);
+void encaps_run(struct controller_ctx *,
const struct ovsrec_bridge *br_int);
-bool chassis_cleanup(struct controller_ctx *,
+bool encaps_cleanup(struct controller_ctx *,
const struct ovsrec_bridge *br_int);
-#endif /* ovn/chassis.h */
+#endif /* ovn/encaps.h */
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 0657140..f3fb3a4 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -42,6 +42,7 @@
#include "ofctrl.h"
#include "binding.h"
#include "chassis.h"
+#include "encaps.h"
#include "physical.h"
#include "pipeline.h"
@@ -255,6 +256,7 @@ main(int argc, char *argv[])
ovsdb_idl_add_column(ctx.ovs_idl, &ovsrec_open_vswitch_col_external_ids);
chassis_init(&ctx);
+ encaps_init(&ctx);
binding_init(&ctx);
physical_init(&ctx);
pipeline_init();
@@ -286,7 +288,8 @@ main(int argc, char *argv[])
goto exit;
}
- chassis_run(&ctx, br_int);
+ chassis_run(&ctx);
+ encaps_run(&ctx, br_int);
binding_run(&ctx, br_int);
struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
@@ -324,10 +327,11 @@ main(int argc, char *argv[])
goto exit;
}
- /* Run both the binding and chassis cleanup, even if one of them
- * returns false. We're done if both return true. */
+ /* Run all of the cleanup functions, even if one of them returns false.
+ * We're done if all of them return true. */
done = binding_cleanup(&ctx);
- done = chassis_cleanup(&ctx, br_int) && done;
+ done = chassis_cleanup(&ctx) && done;
+ done = encaps_cleanup(&ctx, br_int) && done;
if (done) {
poll_immediate_wake();
}
--
2.1.3
More information about the dev
mailing list