[ovs-dev] [PATCH] ovs-vsctl: Allow setting arbitrary database columns in add-port, add-bond.

Justin Pettit jpettit at nicira.com
Mon Apr 5 23:27:42 UTC 2010


Seems reasonable.  We should probably prepare a document at some point describing how to do common configuration without referencing the database.  However, I think exposing it at some level through ovs-vsctl makes sense, too.  We should probably look to IOS for an example of an interface done right...

--Justin


On Apr 5, 2010, at 12:58 PM, Ben Pfaff wrote:

> ---
> tests/ovs-vsctl.at       |   10 +++++++---
> utilities/ovs-vsctl.8.in |   13 +++++++++++--
> utilities/ovs-vsctl.c    |   33 +++++++++++++++++++++++++++++----
> 3 files changed, 47 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
> index 2ea9e40..c634703 100644
> --- a/tests/ovs-vsctl.at
> +++ b/tests/ovs-vsctl.at
> @@ -262,9 +262,11 @@ OVS_VSCTL_SETUP
> AT_CHECK([RUN_OVS_VSCTL(
>   [add-br a], 
>   [add-br b], 
> -  [add-port a a1],
> +  [add-port a a1 tag=9],
> +  [get port a1 tag],
>   [--may-exist add-port b b1],
> -  [del-port a a1])], [0], [], [], [OVS_VSCTL_CLEANUP])
> +  [del-port a a1])], [0], [9
> +], [], [OVS_VSCTL_CLEANUP])
> AT_CHECK([RUN_OVS_VSCTL([--may-exist add-port b b1])], [0], [], [],
>   [OVS_VSCTL_CLEANUP])
> AT_CHECK([RUN_OVS_VSCTL([--may-exist add-port a b1])], [1], [], 
> @@ -284,9 +286,11 @@ AT_KEYWORDS([ovs-vsctl])
> OVS_VSCTL_SETUP
> AT_CHECK([RUN_OVS_VSCTL_TOGETHER(
>   [add-br a], 
> -  [add-bond a bond0 a1 a2 a3],
> +  [add-bond a bond0 a1 a2 a3 tag=9],
> +  [get Port bond0 tag],
>   [del-port bond0])], [0], [
> 
> +9
> 
> ], [], [OVS_VSCTL_CLEANUP])
> CHECK_BRIDGES([a, a, 0])
> diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
> index ee1eadf..031615c 100644
> --- a/utilities/ovs-vsctl.8.in
> +++ b/utilities/ovs-vsctl.8.in
> @@ -225,19 +225,28 @@ commands treat a bonded port as a single entity.
> Lists all of the ports within \fIbridge\fR on standard output, one per
> line.  The local port \fIbridge\fR is not included in the list.
> .
> -.IP "[\fB\-\-may\-exist\fR] \fBadd\-port \fIbridge port\fR"
> +.IP "[\fB\-\-may\-exist\fR] \fBadd\-port \fIbridge port \fR[\fIcolumn\fR[\fB:\fIkey\fR]\fR=\fIvalue\fR]\&...\fR"
> Creates on \fIbridge\fR a new port named \fIport\fR from the network
> device of the same name.
> .IP
> +Optional arguments set values of column in the Port record created by
> +the command.  For example, \fBtag=9\fR would make the port an access
> +port for VLAN 9.  The syntax is the same as that for the \fBset\fR
> +command (see \fBDatabase Commands\fR below).
> +.IP
> Without \fB\-\-may\-exist\fR, attempting to create a port that exists
> is an error.  With \fB\-\-may\-exist\fR, \fIport\fR may already exist
> (but it must be on \fIbridge\fR and not be a bonded port).
> .
> -.IP "[\fB\-\-fake\-iface\fR] \fBadd\-bond \fIbridge port iface\fR\&..."
> +.IP "[\fB\-\-fake\-iface\fR] \fBadd\-bond \fIbridge port iface\fR\&... [\fIcolumn\fR[\fB:\fIkey\fR]\fR=\fIvalue\fR]\&...\fR"
> Creates on \fIbridge\fR a new port named \fIport\fR that bonds
> together the network devices given as each \fIiface\fR.  At least two
> interfaces must be named.
> .IP
> +Optional arguments set values of column in the Port record created by
> +the command.  The syntax is the same as that for the \fBset\fR command
> +(see \fBDatabase Commands\fR below).
> +.IP
> With \fB\-\-fake\-iface\fR, a fake interface with the name \fIport\fR is
> created.  This should only be used for compatibility with legacy
> software that requires it.
> diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
> index 0bf87c2..d39d610 100644
> --- a/utilities/ovs-vsctl.c
> +++ b/utilities/ovs-vsctl.c
> @@ -108,6 +108,11 @@ static void do_vsctl(const char *args,
>                      struct vsctl_command *, size_t n_commands,
>                      struct ovsdb_idl *);
> 
> +static const struct vsctl_table_class *get_table(const char *table_name);
> +static void set_column(const struct vsctl_table_class *,
> +                       const struct ovsdb_idl_row *, const char *arg);
> +
> +
> int
> main(int argc, char *argv[])
> {
> @@ -1173,7 +1178,8 @@ static void
> add_port(struct vsctl_context *ctx,
>          const char *br_name, const char *port_name,
>          bool may_exist, bool fake_iface,
> -         char *iface_names[], int n_ifaces)
> +         char *iface_names[], int n_ifaces,
> +         char *settings[], int n_settings)
> {
>     struct vsctl_info info;
>     struct vsctl_bridge *bridge;
> @@ -1248,6 +1254,10 @@ add_port(struct vsctl_context *ctx,
>         ovsrec_port_set_tag(port, &tag, 1);
>     }
> 
> +    for (i = 0; i < n_settings; i++) {
> +        set_column(get_table("Port"), &port->header_, settings[i]);
> +    }
> +
>     bridge_insert_port((bridge->parent ? bridge->parent->br_cfg
>                         : bridge->br_cfg), port);
> 
> @@ -1260,7 +1270,7 @@ cmd_add_port(struct vsctl_context *ctx)
>     bool may_exist = shash_find(&ctx->options, "--may-exist") != 0;
> 
>     add_port(ctx, ctx->argv[1], ctx->argv[2], may_exist, false,
> -             &ctx->argv[2], 1);
> +             &ctx->argv[2], 1, &ctx->argv[3], ctx->argc - 3);
> }
> 
> static void
> @@ -1268,9 +1278,24 @@ cmd_add_bond(struct vsctl_context *ctx)
> {
>     bool may_exist = shash_find(&ctx->options, "--may-exist") != 0;
>     bool fake_iface = shash_find(&ctx->options, "--fake-iface");
> +    int n_ifaces;
> +    int i;
> +
> +    n_ifaces = ctx->argc - 3;
> +    for (i = 3; i < ctx->argc; i++) {
> +        if (strchr(ctx->argv[i], '=')) {
> +            n_ifaces = i - 3;
> +            break;
> +        }
> +    }
> +    if (n_ifaces < 2) {
> +        vsctl_fatal("add-bond requires at least 2 interfaces, but only "
> +                    "%d were specified", n_ifaces);
> +    }
> 
>     add_port(ctx, ctx->argv[1], ctx->argv[2], may_exist, fake_iface,
> -             &ctx->argv[3], ctx->argc - 3);
> +             &ctx->argv[3], n_ifaces,
> +             &ctx->argv[n_ifaces + 3], ctx->argc - 3 - n_ifaces);
> }
> 
> static void
> @@ -2493,7 +2518,7 @@ static const struct vsctl_command_syntax all_commands[] = {
> 
>     /* Port commands. */
>     {"list-ports", 1, 1, cmd_list_ports, NULL, ""},
> -    {"add-port", 2, 2, cmd_add_port, NULL, "--may-exist"},
> +    {"add-port", 2, INT_MAX, cmd_add_port, NULL, "--may-exist"},
>     {"add-bond", 4, INT_MAX, cmd_add_bond, NULL, "--may-exist,--fake-iface"},
>     {"del-port", 1, 2, cmd_del_port, NULL, "--if-exists,--with-iface"},
>     {"port-to-br", 1, 1, cmd_port_to_br, NULL, ""},
> -- 
> 1.6.6.1
> 
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev_openvswitch.org





More information about the dev mailing list