[ovs-dev] [CCC 1/3] vswitchd: Remove default controller config from Open_vSwitch table
Justin Pettit
jpettit at nicira.com
Fri Jul 16 07:10:23 UTC 2010
An OpenFlow controller is normally associated with a bridge. It was
possible to define a default controller in the Open_vSwitch table that
would be used if one was not associated with a bridge. This was seldom
used and mostly just caused confusion. This commit removes that
support, so an OpenFlow controller must always be associated with a
bridge.
---
utilities/ovs-vsctl.8.in | 21 ++---
utilities/ovs-vsctl.c | 179 ++++++++++++--------------------------------
vswitchd/bridge.c | 37 +++------
vswitchd/vswitch.ovsschema | 4 -
vswitchd/vswitch.xml | 13 +---
5 files changed, 71 insertions(+), 183 deletions(-)
diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
index 50c9482..56a3baa 100644
--- a/utilities/ovs-vsctl.8.in
+++ b/utilities/ovs-vsctl.8.in
@@ -316,16 +316,15 @@ explicitly configured. Otherwise, \fIbridge\fR must name a bridge,
and the settings apply only to that bridge. (Omitting \fIbridge\fR
entirely usually has the same effect as specifying \fBdefault\fR.)
.
-.IP "\fBget\-controller\fR [\fIbridge\fR]"
+.IP "\fBget\-controller\fR \fIbridge\fR"
Prints the configured controller target.
.
-.IP "\fBdel\-controller\fR [\fIbridge\fR]"
+.IP "\fBdel\-controller\fR \fIbridge\fR"
Deletes the configured controller target.
.
-.IP "\fBset\-controller\fR [\fIbridge\fR] \fItarget\fR\&..."
-Sets the configured controller target or targets. If more than one
-\fItarget\fR is specified, then \fIbridge\fR may not be omitted. Each
-\fItarget\fR may use any of the following forms:
+.IP "\fBset\-controller\fR \fIbridge\fR \fItarget\fR\&..."
+Sets the configured controller target or targets. Each \fItarget\fR may
+use any of the following forms:
.
.RS
.so lib/vconn-active.man
@@ -352,13 +351,13 @@ it discontinues its standalone behavior.
If this option is set to \fBsecure\fR, \fBovs\-vswitchd\fR will not
set up flows on its own when the controller connection fails.
.
-.IP "\fBget\-fail\-mode\fR [\fIbridge\fR]"
+.IP "\fBget\-fail\-mode\fR \fIbridge\fR"
Prints the configured failure mode.
.
-.IP "\fBdel\-fail\-mode\fR [\fIbridge\fR]"
+.IP "\fBdel\-fail\-mode\fR \fIbridge\fR"
Deletes the configured failure mode.
.
-.IP "\fBset\-fail\-mode\fR [\fIbridge\fR] \fBstandalone\fR|\fBsecure\fR"
+.IP "\fBset\-fail\-mode\fR \fIbridge\fR \fBstandalone\fR|\fBsecure\fR"
Sets the configured failure mode.
.
.SS "SSL Configuration"
@@ -450,9 +449,7 @@ A port mirroring configuration attached to a bridge. Records may be
identified by mirror name.
.IP "\fBController\fR"
Configuration for an OpenFlow controller. A controller attached to a
-particular bridge may be identified by the bridge's name. The default
-controller controller for an Open vSwitch may be identified by
-specifying \fB.\fR as the record name.
+particular bridge may be identified by the bridge's name.
.IP "\fBNetFlow\fR"
A NetFlow configuration attached to a bridge. Records may be
identified by bridge name.
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index d9d3b63..ff430f2 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -441,12 +441,12 @@ Interface commands (a bond consists of multiple interfaces):\n\
iface-to-br IFACE print name of bridge that contains IFACE\n\
\n\
Controller commands:\n\
- get-controller [BRIDGE] print the controller for BRIDGE\n\
- del-controller [BRIDGE] delete the controller for BRIDGE\n\
- set-controller [BRIDGE] TARGET set the controller for BRIDGE to TARGET\n\
- get-fail-mode [BRIDGE] print the fail-mode for BRIDGE\n\
- del-fail-mode [BRIDGE] delete the fail-mode for BRIDGE\n\
- set-fail-mode [BRIDGE] MODE set the fail-mode for BRIDGE to MODE\n\
+ get-controller BRIDGE print the controller for BRIDGE\n\
+ del-controller BRIDGE delete the controller for BRIDGE\n\
+ set-controller BRIDGE TARGET set the controller for BRIDGE to TARGET\n\
+ get-fail-mode BRIDGE print the fail-mode for BRIDGE\n\
+ del-fail-mode BRIDGE delete the fail-mode for BRIDGE\n\
+ set-fail-mode BRIDGE MODE set the fail-mode for BRIDGE to MODE\n\
\n\
SSL commands:\n\
get-ssl print the SSL configuration\n\
@@ -532,8 +532,6 @@ struct vsctl_info {
struct shash bridges;
struct shash ports;
struct shash ifaces;
- struct ovsrec_controller **ctrl;
- size_t n_ctrl;
};
static char *
@@ -631,9 +629,6 @@ get_info(const struct ovsrec_open_vswitch *ovs, struct vsctl_info *info)
shash_init(&info->ports);
shash_init(&info->ifaces);
- info->ctrl = ovs->controller;
- info->n_ctrl = ovs->n_controller;
-
shash_init(&bridges);
shash_init(&ports);
for (i = 0; i < ovs->n_bridges; i++) {
@@ -886,7 +881,6 @@ cmd_emer_reset(struct vsctl_context *ctx)
/* Reset the Open_vSwitch table. */
ovsrec_open_vswitch_set_managers(ctx->ovs, NULL, 0);
- ovsrec_open_vswitch_set_controller(ctx->ovs, NULL, 0);
ovsrec_open_vswitch_set_ssl(ctx->ovs, NULL);
OVSREC_BRIDGE_FOR_EACH (br, idl) {
@@ -1542,20 +1536,21 @@ cmd_iface_to_br(struct vsctl_context *ctx)
free_info(&info);
}
-/* Print targets of the 'n_controllers' in 'controllers' on the output for
- * 'ctx'. */
static void
-print_controllers(struct vsctl_context *ctx,
- struct ovsrec_controller **controllers,
- size_t n_controllers)
+cmd_get_controller(struct vsctl_context *ctx)
{
- /* Print the targets in sorted order for reproducibility. */
+ struct vsctl_info info;
+ struct vsctl_bridge *br;
struct svec targets;
size_t i;
+ get_info(ctx->ovs, &info);
+ br = find_bridge(&info, ctx->argv[1], true);
+
+ /* Print the targets in sorted order for reproducibility. */
svec_init(&targets);
- for (i = 0; i < n_controllers; i++) {
- svec_add(&targets, controllers[i]->target);
+ for (i = 0; i < br->n_ctrl; i++) {
+ svec_add(&targets, br->ctrl[i]->target);
}
svec_sort(&targets);
@@ -1563,25 +1558,6 @@ print_controllers(struct vsctl_context *ctx,
ds_put_format(&ctx->output, "%s\n", targets.names[i]);
}
svec_destroy(&targets);
-}
-
-static void
-cmd_get_controller(struct vsctl_context *ctx)
-{
- struct vsctl_info info;
-
- get_info(ctx->ovs, &info);
-
- if (ctx->argc == 1 || !strcmp(ctx->argv[1], "default")) {
- print_controllers(ctx, info.ctrl, info.n_ctrl);
- } else {
- struct vsctl_bridge *br = find_bridge(&info, ctx->argv[1], true);
- if (br->n_ctrl) {
- print_controllers(ctx, br->ctrl, br->n_ctrl);
- } else {
- print_controllers(ctx, info.ctrl, info.n_ctrl);
- }
- }
free_info(&info);
}
@@ -1601,20 +1577,14 @@ static void
cmd_del_controller(struct vsctl_context *ctx)
{
struct vsctl_info info;
+ struct vsctl_bridge *br;
get_info(ctx->ovs, &info);
+ br = find_real_bridge(&info, ctx->argv[1], true);
- if (ctx->argc == 1 || !strcmp(ctx->argv[1], "default")) {
- if (info.n_ctrl) {
- delete_controllers(info.ctrl, info.n_ctrl);
- ovsrec_open_vswitch_set_controller(ctx->ovs, NULL, 0);
- }
- } else {
- struct vsctl_bridge *br = find_real_bridge(&info, ctx->argv[1], true);
- if (br->ctrl) {
- delete_controllers(br->ctrl, br->n_ctrl);
- ovsrec_bridge_set_controller(br->br_cfg, NULL, 0);
- }
+ if (br->ctrl) {
+ delete_controllers(br->ctrl, br->n_ctrl);
+ ovsrec_bridge_set_controller(br->br_cfg, NULL, 0);
}
free_info(&info);
@@ -1636,43 +1606,22 @@ insert_controllers(struct ovsdb_idl_txn *txn, char *targets[], size_t n)
}
static void
-set_default_controllers(struct vsctl_context *ctx, char *targets[], size_t n)
-{
- struct ovsrec_controller **controllers;
-
- delete_controllers(ctx->ovs->controller, ctx->ovs->n_controller);
-
- controllers = insert_controllers(ctx->txn, targets, n);
- ovsrec_open_vswitch_set_controller(ctx->ovs, controllers, n);
- free(controllers);
-}
-
-static void
cmd_set_controller(struct vsctl_context *ctx)
{
struct vsctl_info info;
+ struct vsctl_bridge *br;
+ struct ovsrec_controller **controllers;
+ size_t n;
get_info(ctx->ovs, &info);
+ br = find_real_bridge(&info, ctx->argv[1], true);
- if (ctx->argc == 2) {
- /* Set one controller in the "Open_vSwitch" table. */
- set_default_controllers(ctx, &ctx->argv[1], 1);
- } else if (!strcmp(ctx->argv[1], "default")) {
- /* Set one or more controllers in the "Open_vSwitch" table. */
- set_default_controllers(ctx, &ctx->argv[2], ctx->argc - 2);
- } else {
- /* Set one or more controllers for a particular bridge. */
- struct vsctl_bridge *br = find_real_bridge(&info, ctx->argv[1], true);
- struct ovsrec_controller **controllers;
- size_t n;
-
- delete_controllers(br->ctrl, br->n_ctrl);
+ delete_controllers(br->ctrl, br->n_ctrl);
- n = ctx->argc - 2;
- controllers = insert_controllers(ctx->txn, &ctx->argv[2], n);
- ovsrec_bridge_set_controller(br->br_cfg, controllers, n);
- free(controllers);
- }
+ n = ctx->argc - 2;
+ controllers = insert_controllers(ctx->txn, &ctx->argv[2], n);
+ ovsrec_bridge_set_controller(br->br_cfg, controllers, n);
+ free(controllers);
free_info(&info);
}
@@ -1702,23 +1651,13 @@ static void
cmd_get_fail_mode(struct vsctl_context *ctx)
{
struct vsctl_info info;
+ struct vsctl_bridge *br;
const char *fail_mode = NULL;
get_info(ctx->ovs, &info);
+ br = find_bridge(&info, ctx->argv[1], true);
- if (ctx->argc == 1 || !strcmp(ctx->argv[1], "default")) {
- /* Return the fail-mode from the "Open_vSwitch" table */
- fail_mode = get_fail_mode(info.ctrl, info.n_ctrl);
- } else {
- /* Return the fail-mode for a particular bridge. */
- struct vsctl_bridge *br = find_bridge(&info, ctx->argv[1], true);
-
- /* If no controller is defined for the requested bridge, fallback to
- * the "Open_vSwitch" table's controller. */
- fail_mode = (br->n_ctrl
- ? get_fail_mode(br->ctrl, br->n_ctrl)
- : get_fail_mode(info.ctrl, info.n_ctrl));
- }
+ fail_mode = get_fail_mode(br->ctrl, br->n_ctrl);
if (fail_mode && strlen(fail_mode)) {
ds_put_format(&ctx->output, "%s\n", fail_mode);
@@ -1742,16 +1681,12 @@ static void
cmd_del_fail_mode(struct vsctl_context *ctx)
{
struct vsctl_info info;
+ struct vsctl_bridge *br;
get_info(ctx->ovs, &info);
+ br = find_real_bridge(&info, ctx->argv[1], true);
- if (ctx->argc == 1 || !strcmp(ctx->argv[1], "default")) {
- set_fail_mode(info.ctrl, info.n_ctrl, NULL);
- } else {
- struct vsctl_bridge *br = find_real_bridge(&info, ctx->argv[1], true);
-
- set_fail_mode(br->ctrl, br->n_ctrl, NULL);
- }
+ set_fail_mode(br->ctrl, br->n_ctrl, NULL);
free_info(&info);
}
@@ -1760,37 +1695,20 @@ static void
cmd_set_fail_mode(struct vsctl_context *ctx)
{
struct vsctl_info info;
- const char *bridge;
- const char *fail_mode;
+ struct vsctl_bridge *br;
+ const char *fail_mode = ctx->argv[2];
get_info(ctx->ovs, &info);
-
- if (ctx->argc == 2) {
- bridge = "default";
- fail_mode = ctx->argv[1];
- } else {
- bridge = ctx->argv[1];
- fail_mode = ctx->argv[2];
- }
+ br = find_real_bridge(&info, ctx->argv[1], true);
if (strcmp(fail_mode, "standalone") && strcmp(fail_mode, "secure")) {
vsctl_fatal("fail-mode must be \"standalone\" or \"secure\"");
}
- if (!strcmp(bridge, "default")) {
- /* Set the fail-mode in the "Open_vSwitch" table. */
- if (!info.ctrl) {
- vsctl_fatal("no controller declared");
- }
- set_fail_mode(info.ctrl, info.n_ctrl, fail_mode);
- } else {
- struct vsctl_bridge *br = find_real_bridge(&info, bridge, true);
-
- if (!br->ctrl) {
- vsctl_fatal("no controller declared for %s", br->name);
- }
- set_fail_mode(br->ctrl, br->n_ctrl, fail_mode);
+ if (!br->ctrl) {
+ vsctl_fatal("no controller declared for %s", br->name);
}
+ set_fail_mode(br->ctrl, br->n_ctrl, fail_mode);
free_info(&info);
}
@@ -1861,10 +1779,7 @@ static const struct vsctl_table_class tables[] = {
{&ovsrec_table_controller,
{{&ovsrec_table_bridge,
&ovsrec_bridge_col_name,
- &ovsrec_bridge_col_controller},
- {&ovsrec_table_open_vswitch,
- NULL,
- &ovsrec_open_vswitch_col_controller}}},
+ &ovsrec_bridge_col_controller}}},
{&ovsrec_table_interface,
{{&ovsrec_table_interface, &ovsrec_interface_col_name, NULL},
@@ -2988,12 +2903,12 @@ static const struct vsctl_command_syntax all_commands[] = {
{"iface-to-br", 1, 1, cmd_iface_to_br, NULL, ""},
/* Controller commands. */
- {"get-controller", 0, 1, cmd_get_controller, NULL, ""},
- {"del-controller", 0, 1, cmd_del_controller, NULL, ""},
+ {"get-controller", 1, 1, cmd_get_controller, NULL, ""},
+ {"del-controller", 1, 1, cmd_del_controller, NULL, ""},
{"set-controller", 1, INT_MAX, cmd_set_controller, NULL, ""},
- {"get-fail-mode", 0, 1, cmd_get_fail_mode, NULL, ""},
- {"del-fail-mode", 0, 1, cmd_del_fail_mode, NULL, ""},
- {"set-fail-mode", 1, 2, cmd_set_fail_mode, NULL, ""},
+ {"get-fail-mode", 1, 1, cmd_get_fail_mode, NULL, ""},
+ {"del-fail-mode", 1, 1, cmd_del_fail_mode, NULL, ""},
+ {"set-fail-mode", 2, 2, cmd_set_fail_mode, NULL, ""},
/* SSL commands. */
{"get-ssl", 0, 0, cmd_get_ssl, NULL, ""},
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index e4d9cb7..d930b1d 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -199,13 +199,10 @@ static struct bridge *bridge_lookup(const char *name);
static unixctl_cb_func bridge_unixctl_dump_flows;
static unixctl_cb_func bridge_unixctl_reconnect;
static int bridge_run_one(struct bridge *);
-static size_t bridge_get_controllers(const struct ovsrec_open_vswitch *ovs_cfg,
- const struct bridge *br,
+static size_t bridge_get_controllers(const struct bridge *br,
struct ovsrec_controller ***controllersp);
-static void bridge_reconfigure_one(const struct ovsrec_open_vswitch *,
- struct bridge *);
-static void bridge_reconfigure_remotes(const struct ovsrec_open_vswitch *,
- struct bridge *,
+static void bridge_reconfigure_one(struct bridge *);
+static void bridge_reconfigure_remotes(struct bridge *,
const struct sockaddr_in *managers,
size_t n_managers);
static void bridge_get_all_ifaces(const struct bridge *, struct shash *ifaces);
@@ -606,7 +603,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
/* Reconfigure all bridges. */
LIST_FOR_EACH (br, struct bridge, node, &all_bridges) {
- bridge_reconfigure_one(ovs_cfg, br);
+ bridge_reconfigure_one(br);
}
/* Add and delete ports on all datapaths.
@@ -799,7 +796,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
oso.agent_device = sflow_cfg->agent;
oso.control_ip = NULL;
- n_controllers = bridge_get_controllers(ovs_cfg, br, &controllers);
+ n_controllers = bridge_get_controllers(br, &controllers);
for (i = 0; i < n_controllers; i++) {
if (controllers[i]->local_ip) {
oso.control_ip = controllers[i]->local_ip;
@@ -822,7 +819,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
* yet; when a controller is configured, resetting the datapath ID will
* immediately disconnect from the controller, so it's better to set
* the datapath ID before the controller. */
- bridge_reconfigure_remotes(ovs_cfg, br, managers, n_managers);
+ bridge_reconfigure_remotes(br, managers, n_managers);
}
LIST_FOR_EACH (br, struct bridge, node, &all_bridges) {
for (i = 0; i < br->n_ports; i++) {
@@ -1399,20 +1396,14 @@ bridge_run_one(struct bridge *br)
}
static size_t
-bridge_get_controllers(const struct ovsrec_open_vswitch *ovs_cfg,
- const struct bridge *br,
+bridge_get_controllers(const struct bridge *br,
struct ovsrec_controller ***controllersp)
{
struct ovsrec_controller **controllers;
size_t n_controllers;
- if (br->cfg->n_controller) {
- controllers = br->cfg->controller;
- n_controllers = br->cfg->n_controller;
- } else {
- controllers = ovs_cfg->controller;
- n_controllers = ovs_cfg->n_controller;
- }
+ controllers = br->cfg->controller;
+ n_controllers = br->cfg->n_controller;
if (n_controllers == 1 && !strcmp(controllers[0]->target, "none")) {
controllers = NULL;
@@ -1426,8 +1417,7 @@ bridge_get_controllers(const struct ovsrec_open_vswitch *ovs_cfg,
}
static void
-bridge_reconfigure_one(const struct ovsrec_open_vswitch *ovs_cfg,
- struct bridge *br)
+bridge_reconfigure_one(struct bridge *br)
{
struct shash old_ports, new_ports;
struct svec listeners, old_listeners;
@@ -1455,7 +1445,7 @@ bridge_reconfigure_one(const struct ovsrec_open_vswitch *ovs_cfg,
* user didn't specify one.
*
* XXX perhaps we should synthesize a port ourselves in this case. */
- if (bridge_get_controllers(ovs_cfg, br, NULL)) {
+ if (bridge_get_controllers(br, NULL)) {
char local_name[IF_NAMESIZE];
int error;
@@ -1533,8 +1523,7 @@ bridge_reconfigure_one(const struct ovsrec_open_vswitch *ovs_cfg,
}
static void
-bridge_reconfigure_remotes(const struct ovsrec_open_vswitch *ovs_cfg,
- struct bridge *br,
+bridge_reconfigure_remotes(struct bridge *br,
const struct sockaddr_in *managers,
size_t n_managers)
{
@@ -1543,7 +1532,7 @@ bridge_reconfigure_remotes(const struct ovsrec_open_vswitch *ovs_cfg,
ofproto_set_extra_in_band_remotes(br->ofproto, managers, n_managers);
- n_controllers = bridge_get_controllers(ovs_cfg, br, &controllers);
+ n_controllers = bridge_get_controllers(br, &controllers);
if (ofproto_has_controller(br->ofproto) != (n_controllers != 0)) {
ofproto_flush_flows(br->ofproto);
}
diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index b38463b..417ce96 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -6,10 +6,6 @@
"type": {"key": {"type": "uuid",
"refTable": "Bridge"},
"min": 0, "max": "unlimited"}},
- "controller": {
- "type": {"key": {"type": "uuid",
- "refTable": "Controller"},
- "min": 0, "max": "unlimited"}},
"managers": {
"type": {"key": "string", "min": 0, "max": "unlimited"}},
"ssl": {
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index b93a8db..da3652b 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -15,12 +15,6 @@
Set of bridges managed by the daemon.
</column>
- <column name="controller">
- Default OpenFlow <ref table="Controller"/> set used by bridges. May be
- overridden on a per-bridge basis by the <ref table="Bridge"
- column="controller"/> column in <ref table="Bridge"/>.
- </column>
-
<column name="managers">
Remote database clients to which the Open vSwitch's database server
should connect or to which it should listen.
@@ -132,11 +126,8 @@
<group title="OpenFlow Configuration">
<column name="controller">
- OpenFlow controller set. If unset, defaults to the set of
- controllers specified by <ref column="controller"
- table="Open_vSwitch"/> in the <ref table="Open_vSwitch"/>
- table. If the default is also unset, then no OpenFlow
- controllers will be used.
+ OpenFlow controller set. If unset, then no OpenFlow controllers
+ will be used.
</column>
<column name="datapath_id">
--
1.7.1
More information about the dev
mailing list