From: Justin Pettit Date: Thu, 15 Jul 2010 21:53:36 +0000 (-0700) Subject: vswitchd: Remove default controller config from Open_vSwitch table X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1a048029a0902439c3f17c49a7f71e4d09fecf70;p=openvswitch vswitchd: Remove default controller config from Open_vSwitch table 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. --- diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at index 061a4f48..a8dc8546 100644 --- a/tests/ovs-vsctl.at +++ b/tests/ovs-vsctl.at @@ -360,76 +360,21 @@ OVS_VSCTL_SETUP AT_CHECK([RUN_OVS_VSCTL_TOGETHER( [add-br br0], - [set-controller tcp:1.2.3.4], - [get-controller], - [get-controller default], [get-controller br0], - [set-controller br0 tcp:4.5.6.7], - [get-controller], - [get-controller default], - [get-controller br0], - - [del-controller], - [get-controller], - [get-controller default], - [get-controller br0], - - [set-controller default tcp:8.9.10.11], - [get-controller], - [get-controller default], - [get-controller br0], - - [del-controller default], - [get-controller], - [get-controller default], [get-controller br0], [del-controller br0], - [get-controller], - [get-controller default], - [get-controller br0], - - [set-controller default tcp:1.2.3.4 tcp:4.5.6.7], - [get-controller], - [get-controller default], [get-controller br0], [set-controller br0 tcp:8.9.10.11 tcp:5.4.3.2], - [get-controller], - [get-controller default], [get-controller br0])], [0], [ -tcp:1.2.3.4 -tcp:1.2.3.4 -tcp:1.2.3.4 - -tcp:1.2.3.4 -tcp:1.2.3.4 -tcp:4.5.6.7 - - - -tcp:4.5.6.7 -tcp:8.9.10.11 -tcp:8.9.10.11 tcp:4.5.6.7 -tcp:4.5.6.7 - - - - - -tcp:1.2.3.4\ntcp:4.5.6.7 -tcp:1.2.3.4\ntcp:4.5.6.7 -tcp:1.2.3.4\ntcp:4.5.6.7 - -tcp:1.2.3.4\ntcp:4.5.6.7 -tcp:1.2.3.4\ntcp:4.5.6.7 tcp:5.4.3.2\ntcp:8.9.10.11 ], [], [OVS_VSCTL_CLEANUP]) OVS_VSCTL_CLEANUP diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in index 3137a847..ea8a8aad 100644 --- a/utilities/ovs-vsctl.8.in +++ b/utilities/ovs-vsctl.8.in @@ -310,22 +310,15 @@ output. locally, or it can be configured to connect a given bridge to one or more external OpenFlow controllers, such as NOX. . -For each of these commands, a \fIbridge\fR of \fBdefault\fR applies -the configuration as the default for any bridge that has not been -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 +345,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 +443,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 76e896a0..88ddd996 100644 --- a/utilities/ovs-vsctl.c +++ b/utilities/ovs-vsctl.c @@ -440,12 +440,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\ @@ -531,8 +531,6 @@ struct vsctl_info { struct shash bridges; struct shash ports; struct shash ifaces; - struct ovsrec_controller **ctrl; - size_t n_ctrl; }; static char * @@ -630,9 +628,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++) { @@ -885,7 +880,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) { @@ -1541,20 +1535,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); @@ -1562,25 +1557,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); } @@ -1600,20 +1576,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); @@ -1634,44 +1604,23 @@ insert_controllers(struct ovsdb_idl_txn *txn, char *targets[], size_t n) return controllers; } -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); } @@ -1701,23 +1650,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); @@ -1741,16 +1680,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); } @@ -1759,37 +1694,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); } @@ -1860,10 +1778,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}, @@ -2966,12 +2881,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 be0e8758..fc03a122 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 b38463b0..417ce96a 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 b93a8db2..da3652b9 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -15,12 +15,6 @@ Set of bridges managed by the daemon. - - Default OpenFlow set used by bridges. May be - overridden on a per-bridge basis by the column in . - - Remote database clients to which the Open vSwitch's database server should connect or to which it should listen. @@ -132,11 +126,8 @@ - OpenFlow controller set. If unset, defaults to the set of - controllers specified by in the - 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.