Looks good to me, thanks. Tested-by: Yifeng Sun <[email protected]>
Reviewed-by: Yifeng Sun <[email protected]> On Fri, Feb 2, 2018 at 1:51 PM, Ben Pfaff <[email protected]> wrote: > It was not too hard to build these commands using the database commands, > but a few people have asked for them over the years, so here they are. > > Signed-off-by: Ben Pfaff <[email protected]> > --- > NEWS | 1 + > tests/ovs-vsctl.at | 60 +++++++++++++++++++++++++++++++++++++ > utilities/ovs-vsctl.8.in | 70 ++++++++++++++++++++++++++++++------------- > utilities/ovs-vsctl.c | 77 ++++++++++++++++++++++++++++++ > ++++++++++++++++-- > 4 files changed, 185 insertions(+), 23 deletions(-) > > diff --git a/NEWS b/NEWS > index 3b6ff74ad368..cc6dd95026de 100644 > --- a/NEWS > +++ b/NEWS > @@ -9,6 +9,7 @@ Post-v2.9.0 > * ovs-ofctl now accepts and display table names in place of > numbers. By > default it always accepts names and in interactive use it displays > them; > use --names or --no-names to override. See ovs-ofctl(8) for > details. > + - ovs-vsctl: New commands "add-bond-iface" and "del-bond-iface". > > > v2.9.0 - xx xxx xxxx > diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at > index 415e14833249..e6c3f4596793 100644 > --- a/tests/ovs-vsctl.at > +++ b/tests/ovs-vsctl.at > @@ -313,6 +313,66 @@ CHECK_IFACES([a], [a1], [a2], [a3]) > OVS_VSCTL_CLEANUP > AT_CLEANUP > > +AT_SETUP([add-bond-iface and del-bond-iface]) > +AT_KEYWORDS([ovs-vsctl]) > +OVS_VSCTL_SETUP > + > +# Create 2-interface bond. > +AT_CHECK([RUN_OVS_VSCTL( > + [add-br a], > + [add-bond a bond0 a1 a2])]) > +CHECK_BRIDGES([a, a, 0]) > +CHECK_PORTS([a], [bond0]) > +CHECK_IFACES([a], [a1], [a2]) > + > +# Add interface a3 to bond. > +AT_CHECK([RUN_OVS_VSCTL([add-bond-iface bond0 a3])]) > +CHECK_BRIDGES([a, a, 0]) > +CHECK_PORTS([a], [bond0]) > +CHECK_IFACES([a], [a1], [a2], [a3]) > + > +# Delete interface a2 from bond. > +AT_CHECK([RUN_OVS_VSCTL([del-bond-iface bond0 a2])]) > +CHECK_BRIDGES([a, a, 0]) > +CHECK_PORTS([a], [bond0]) > +CHECK_IFACES([a], [a1], [a3]) > + > +# Add interface a2 to bond. > +AT_CHECK([RUN_OVS_VSCTL([add-bond-iface bond0 a2])]) > +CHECK_BRIDGES([a, a, 0]) > +CHECK_PORTS([a], [bond0]) > +CHECK_IFACES([a], [a1], [a2], [a3]) > + > +# Delete interface a2 from bond. > +AT_CHECK([RUN_OVS_VSCTL([del-bond-iface a2])]) > +CHECK_BRIDGES([a, a, 0]) > +CHECK_PORTS([a], [bond0]) > +CHECK_IFACES([a], [a1], [a3]) > + > +AT_CHECK([RUN_OVS_VSCTL([--if-exists del-bond-iface bond0 a4])]) > +AT_CHECK([RUN_OVS_VSCTL([del-bond-iface bond0 a4])], [1], [], > + [ovs-vsctl: no interface named a4 > +]) > +AT_CHECK([RUN_OVS_VSCTL([del-bond-iface a4])], [1], [], > + [ovs-vsctl: no interface named a4 > +]) > +AT_CHECK([RUN_OVS_VSCTL_TOGETHER([add-port a a4], [del-bond-iface bond0 > a4])], [1], [], > + [ovs-vsctl: port bond0 does not have an interface a4 > +]) > +AT_CHECK([RUN_OVS_VSCTL([--may-exist add-bond-iface bond0 a3])]) > +AT_CHECK([RUN_OVS_VSCTL_TOGETHER([add-bond a bond1 b1 b2 b3], > [--may-exist add-bond-iface bond1 a3])], [1], [], > + [ovs-vsctl: "--may-exist add-bond-iface bond1 a3" but a3 is actually > attached to port bond0 > +]) > +AT_CHECK([RUN_OVS_VSCTL([add-bond-iface bond0 a3])], [1], [], > + [ovs-vsctl: cannot create an interface named a3 because an interface > named a3 already exists on bridge a > +]) > +AT_CHECK([RUN_OVS_VSCTL_TOGETHER([del-bond-iface a1], [del-bond-iface > a3])], [1], [], > + [ovs-vsctl: cannot delete last interface from port bond0 > +]) > + > +OVS_VSCTL_CLEANUP > +AT_CLEANUP > + > AT_SETUP([add-br a b, add-port a a1, add-port b b1, del-port a a1]) > AT_KEYWORDS([ovs-vsctl]) > OVS_VSCTL_SETUP > diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in > index f539af5486e4..34f41e4e88e6 100644 > --- a/utilities/ovs-vsctl.8.in > +++ b/utilities/ovs-vsctl.8.in > @@ -274,26 +274,6 @@ Without \fB\-\-may\-exist\fR, attempting to create a > port that exists > is an error. With \fB\-\-may\-exist\fR, this command does nothing if > \fIport\fR already exists on \fIbridge\fR and is not a bonded port. > . > -.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. If the interfaces are DPDK enabled then > -the transaction will need to include operations to explicitly set the > -interface type to 'dpdk'. > -.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. > -.IP > -Without \fB\-\-may\-exist\fR, attempting to create a port that exists > -is an error. With \fB\-\-may\-exist\fR, this command does nothing if > -\fIport\fR already exists on \fIbridge\fR and bonds together exactly > -the specified interfaces. > -. > .IP "[\fB\-\-if\-exists\fR] \fBdel\-port \fR[\fIbridge\fR] \fIport\fR" > Deletes \fIport\fR. If \fIbridge\fR is omitted, \fIport\fR is removed > from whatever bridge contains it; if \fIbridge\fR is specified, it > @@ -318,6 +298,56 @@ no effect. > Prints the name of the bridge that contains \fIport\fR on standard > output. > . > +.SS "Bond Commands" > +. > +These commands work with ports that have more than one interface, > +which Open vSwitch calls ``bonds.'' > +. > +.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. If the interfaces are DPDK enabled then > +the transaction will need to include operations to explicitly set the > +interface type to 'dpdk'. > +.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. > +.IP > +Without \fB\-\-may\-exist\fR, attempting to create a port that exists > +is an error. With \fB\-\-may\-exist\fR, this command does nothing if > +\fIport\fR already exists on \fIbridge\fR and bonds together exactly > +the specified interfaces. > +. > +.IP "[\fB\-\-may\-exist\fR] \fBadd\-bond\-iface \fIbond iface\fR" > +Adds \fIiface\fR as a new bond interface to the existing port > +\fIbond\fR. If \fIbond\fR previously had only one port, this > +transforms it into a bond. > +.IP > +Without \fB\-\-may\-exist\fR, attempting to add an \fIiface\fR that is > +already part of \fIbond\fR is an error. With \fB\-\-may\-exist\fR, > +this command does nothing if \fIiface\fR is already part of > +\fIbond\fR. (It is still an error if \fIiface\fR is an interface of > +some other port or bond.) > +. > +.IP "[\fB\-\-if\-exists\fR] \fBdel\-bond\-iface\fR [\fIbond\fR] > \fIiface\fR" > +Removes \fIiface\fR from its port. If \fIbond\fR is omitted, > +\fIiface\fR is removed from whatever port contains it; if \fIbond\fR > +is specified, it must be the port that contains \fIbond\fR. > +.IP > +If removing \fIiface\fR causes its port to have only a single > +interface, then that port transforms from a bond into an ordinary > +port. It is an error if \fIiface\fR is the only interface in its > +port. > +.IP > +Without \fB\-\-if\-exists\fR, attempting to delete an interface that > +does not exist is an error. With \fB\-\-if\-exists\fR, attempting to > +delete an interface that does not exist has no effect. > +. > .SS "Interface Commands" > . > These commands examine the interfaces attached to an Open vSwitch > diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c > index 6e47ca361ac4..e22baeb28c36 100644 > --- a/utilities/ovs-vsctl.c > +++ b/utilities/ovs-vsctl.c > @@ -1636,6 +1636,71 @@ cmd_add_bond(struct ctl_context *ctx) > &ctx->argv[n_ifaces + 3], ctx->argc - 3 - n_ifaces); > } > > +static void > +cmd_add_bond_iface(struct ctl_context *ctx) > +{ > + vsctl_context_populate_cache(ctx); > + > + struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx); > + bool may_exist = shash_find(&ctx->options, "--may-exist") != NULL; > + struct vsctl_port *port = find_port(vsctl_ctx, ctx->argv[1], true); > + > + const char *iface_name = ctx->argv[2]; > + if (may_exist) { > + struct vsctl_iface *iface = find_iface(vsctl_ctx, iface_name, > false); > + if (iface) { > + if (iface->port == port) { > + return; > + } > + char *command = vsctl_context_to_string(ctx); > + ctl_fatal("\"%s\" but %s is actually attached to port %s", > + command, iface_name, iface->port->port_cfg->name); > + } > + } > + check_conflicts(vsctl_ctx, iface_name, > + xasprintf("cannot create an interface named %s", > + iface_name)); > + > + struct ovsrec_interface *iface = ovsrec_interface_insert(ctx->txn); > + ovsrec_interface_set_name(iface, iface_name); > + ovsrec_port_update_interfaces_addvalue(port->port_cfg, iface); > + post_db_reload_expect_iface(iface); > + add_iface_to_cache(vsctl_ctx, port, iface); > +} > + > +static void > +cmd_del_bond_iface(struct ctl_context *ctx) > +{ > + vsctl_context_populate_cache(ctx); > + > + struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx); > + const char *iface_name = ctx->argv[ctx->argc - 1]; > + bool must_exist = !shash_find(&ctx->options, "--if-exists"); > + struct vsctl_iface *iface = find_iface(vsctl_ctx, iface_name, > must_exist); > + if (!iface) { > + ovs_assert(!must_exist); > + return; > + } > + > + const char *port_name = ctx->argc > 2 ? ctx->argv[1] : NULL; > + if (port_name) { > + struct vsctl_port *port = find_port(vsctl_ctx, port_name, true); > + if (iface->port != port) { > + ctl_fatal("port %s does not have an interface %s", > + port_name, iface_name); > + } > + } > + > + if (ovs_list_is_short(&iface->port->ifaces)) { > + ctl_fatal("cannot delete last interface from port %s", > + iface->port->port_cfg->name); > + } > + > + ovsrec_port_update_interfaces_delvalue(iface->port->port_cfg, > + iface->iface_cfg); > + del_cached_iface(vsctl_ctx, iface); > +} > + > static void > cmd_del_port(struct ctl_context *ctx) > { > @@ -2749,13 +2814,19 @@ static const struct ctl_command_syntax > vsctl_commands[] = { > RO}, > {"add-port", 2, INT_MAX, "BRIDGE NEW-PORT [COLUMN[:KEY]=VALUE]...", > pre_get_info, cmd_add_port, NULL, "--may-exist", RW}, > - {"add-bond", 4, INT_MAX, > - "BRIDGE NEW-BOND-PORT SYSIFACE... [COLUMN[:KEY]=VALUE]...", > pre_get_info, > - cmd_add_bond, NULL, "--may-exist,--fake-iface", RW}, > {"del-port", 1, 2, "[BRIDGE] PORT|IFACE", pre_get_info, cmd_del_port, > NULL, > "--if-exists,--with-iface", RW}, > {"port-to-br", 1, 1, "PORT", pre_get_info, cmd_port_to_br, NULL, "", > RO}, > > + /* Bond commands. */ > + {"add-bond", 4, INT_MAX, > + "BRIDGE BOND IFACE... [COLUMN[:KEY]=VALUE]...", pre_get_info, > + cmd_add_bond, NULL, "--may-exist,--fake-iface", RW}, > + {"add-bond-iface", 2, 2, "BOND IFACE", pre_get_info, > cmd_add_bond_iface, > + NULL, "--may-exist", RW}, > + {"del-bond-iface", 1, 2, "[BOND] IFACE", pre_get_info, > cmd_del_bond_iface, > + NULL, "--if-exists", RW}, > + > /* Interface commands. */ > {"list-ifaces", 1, 1, "BRIDGE", pre_get_info, cmd_list_ifaces, NULL, > "", > RO}, > -- > 2.15.1 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
