From 5341d04613a359b949279ce3f54f1a5bd7731999 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Fri, 16 Mar 2012 13:12:54 -0700 Subject: [PATCH] ovs-vsctl: Allow "fake bridges" to be created for VLAN 0. A fake bridge for VLAN 0 is useful, because it provides a way to create access ports for VLAN 0. There is no good reason to prevent it. NIC-464. Reported-by: Rob Hoes Signed-off-by: Ben Pfaff --- tests/ovs-vsctl.at | 64 ++++++++++++++++++++++------------------ utilities/ovs-vsctl.8.in | 5 ++-- utilities/ovs-vsctl.c | 21 ++++++++----- 3 files changed, 52 insertions(+), 38 deletions(-) diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at index 9d742cc1..4b17b05c 100644 --- a/tests/ovs-vsctl.at +++ b/tests/ovs-vsctl.at @@ -401,8 +401,7 @@ OVS_VSCTL_CLEANUP AT_CLEANUP dnl ---------------------------------------------------------------------- -AT_BANNER([ovs-vsctl unit tests -- fake bridges]) - +dnl OVS_VSCTL_SETUP_SIMPLE_FAKE_CONF([VLAN]) m4_define([OVS_VSCTL_SETUP_SIMPLE_FAKE_CONF], [AT_CHECK( [RUN_OVS_VSCTL( @@ -410,36 +409,40 @@ m4_define([OVS_VSCTL_SETUP_SIMPLE_FAKE_CONF], [--may-exist add-br xenbr0], [add-port xenbr0 eth0], [--may-exist add-port xenbr0 eth0], - [add-br xapi1 xenbr0 9], - [--may-exist add-br xapi1 xenbr0 9], - [add-port xapi1 eth0.9])], + [add-br xapi1 xenbr0 $1], + [--may-exist add-br xapi1 xenbr0 $1], + [add-port xapi1 eth0.$1])], [0], [], [], [OVS_VSCTL_CLEANUP])]) -AT_SETUP([simple fake bridge]) +dnl OVS_VSCTL_FAKE_BRIDGE_TESTS([VLAN]) +m4_define([OVS_VSCTL_FAKE_BRIDGE_TESTS], [ +AT_BANNER([ovs-vsctl unit tests -- fake bridges (VLAN $1)]) + +AT_SETUP([simple fake bridge (VLAN $1)]) AT_KEYWORDS([ovs-vsctl fake-bridge]) OVS_VSCTL_SETUP -OVS_VSCTL_SETUP_SIMPLE_FAKE_CONF +OVS_VSCTL_SETUP_SIMPLE_FAKE_CONF([$1]) AT_CHECK([RUN_OVS_VSCTL([--may-exist add-br xapi1])], [1], [], - [ovs-vsctl: "--may-exist add-br xapi1" but xapi1 is a VLAN bridge for VLAN 9 + [ovs-vsctl: "--may-exist add-br xapi1" but xapi1 is a VLAN bridge for VLAN $1 ], [OVS_VSCTL_CLEANUP]) -AT_CHECK([RUN_OVS_VSCTL([--may-exist add-br xapi1 xxx 9])], [1], [], - [ovs-vsctl: "--may-exist add-br xapi1 xxx 9" but xapi1 has the wrong parent xenbr0 +AT_CHECK([RUN_OVS_VSCTL([--may-exist add-br xapi1 xxx $1])], [1], [], + [ovs-vsctl: "--may-exist add-br xapi1 xxx $1" but xapi1 has the wrong parent xenbr0 ], [OVS_VSCTL_CLEANUP]) AT_CHECK([RUN_OVS_VSCTL([--may-exist add-br xapi1 xenbr0 10])], [1], [], - [ovs-vsctl: "--may-exist add-br xapi1 xenbr0 10" but xapi1 is a VLAN bridge for the wrong VLAN 9 + [ovs-vsctl: "--may-exist add-br xapi1 xenbr0 10" but xapi1 is a VLAN bridge for the wrong VLAN $1 ], [OVS_VSCTL_CLEANUP]) -CHECK_BRIDGES([xapi1, xenbr0, 9], [xenbr0, xenbr0, 0]) +CHECK_BRIDGES([xapi1, xenbr0, $1], [xenbr0, xenbr0, 0]) CHECK_PORTS([xenbr0], [eth0]) CHECK_IFACES([xenbr0], [eth0]) -CHECK_PORTS([xapi1], [eth0.9]) -CHECK_IFACES([xapi1], [eth0.9]) +CHECK_PORTS([xapi1], [eth0.$1]) +CHECK_IFACES([xapi1], [eth0.$1]) OVS_VSCTL_CLEANUP AT_CLEANUP -AT_SETUP([simple fake bridge + del-br fake bridge]) +AT_SETUP([simple fake bridge + del-br fake bridge (VLAN $1)]) AT_KEYWORDS([ovs-vsctl fake-bridge]) OVS_VSCTL_SETUP -OVS_VSCTL_SETUP_SIMPLE_FAKE_CONF +OVS_VSCTL_SETUP_SIMPLE_FAKE_CONF([$1]) AT_CHECK([RUN_OVS_VSCTL([del-br xapi1])], [0], [], [], [OVS_VSCTL_CLEANUP]) CHECK_BRIDGES([xenbr0, xenbr0, 0]) CHECK_PORTS([xenbr0], [eth0]) @@ -447,19 +450,19 @@ CHECK_IFACES([xenbr0], [eth0]) OVS_VSCTL_CLEANUP AT_CLEANUP -AT_SETUP([simple fake bridge + del-br real bridge]) +AT_SETUP([simple fake bridge + del-br real bridge (VLAN $1)]) AT_KEYWORDS([ovs-vsctl fake-bridge]) OVS_VSCTL_SETUP -OVS_VSCTL_SETUP_SIMPLE_FAKE_CONF +OVS_VSCTL_SETUP_SIMPLE_FAKE_CONF([$1]) AT_CHECK([RUN_OVS_VSCTL([del-br xenbr0])], [0], [], [], [OVS_VSCTL_CLEANUP]) CHECK_BRIDGES OVS_VSCTL_CLEANUP AT_CLEANUP -AT_SETUP([simple fake bridge + external IDs]) +AT_SETUP([simple fake bridge + external IDs (VLAN $1)]) AT_KEYWORDS([ovs-vsctl fake-bridge]) OVS_VSCTL_SETUP -OVS_VSCTL_SETUP_SIMPLE_FAKE_CONF +OVS_VSCTL_SETUP_SIMPLE_FAKE_CONF([$1]) AT_CHECK([RUN_OVS_VSCTL_TOGETHER( [br-set-external-id xenbr0 key0 value0], [br-set-external-id xapi1 key1 value1], @@ -473,27 +476,32 @@ value0 key1=value1 value1 ], [], [OVS_VSCTL_CLEANUP]) -CHECK_BRIDGES([xapi1, xenbr0, 9], [xenbr0, xenbr0, 0]) +CHECK_BRIDGES([xapi1, xenbr0, $1], [xenbr0, xenbr0, 0]) CHECK_PORTS([xenbr0], [eth0]) CHECK_IFACES([xenbr0], [eth0]) -CHECK_PORTS([xapi1], [eth0.9]) -CHECK_IFACES([xapi1], [eth0.9]) +CHECK_PORTS([xapi1], [eth0.$1]) +CHECK_IFACES([xapi1], [eth0.$1]) OVS_VSCTL_CLEANUP AT_CLEANUP +]) # OVS_VSCTL_FAKE_BRIDGE_TESTS + +OVS_VSCTL_FAKE_BRIDGE_TESTS([9]) +OVS_VSCTL_FAKE_BRIDGE_TESTS([0]) +dnl OVS_VSCTL_SETUP_BOND_FAKE_CONF([VLAN]) m4_define([OVS_VSCTL_SETUP_BOND_FAKE_CONF], [AT_CHECK( [RUN_OVS_VSCTL( [add-br xapi1], [add-bond xapi1 bond0 eth0 eth1], - [add-br xapi2 xapi1 11], - [add-port xapi2 bond0.11])], + [add-br xapi2 xapi1 $1], + [add-port xapi2 bond0.$1])], [0], [], [], [OVS_VSCTL_CLEANUP])]) AT_SETUP([fake bridge on bond]) AT_KEYWORDS([ovs-vsctl fake-bridge]) OVS_VSCTL_SETUP -OVS_VSCTL_SETUP_BOND_FAKE_CONF +OVS_VSCTL_SETUP_BOND_FAKE_CONF([11]) CHECK_BRIDGES([xapi1, xapi1, 0], [xapi2, xapi1, 11]) CHECK_PORTS([xapi1], [bond0]) CHECK_IFACES([xapi1], [eth0], [eth1]) @@ -505,7 +513,7 @@ AT_CLEANUP AT_SETUP([fake bridge on bond + del-br fake bridge]) AT_KEYWORDS([ovs-vsctl fake-bridge]) OVS_VSCTL_SETUP -OVS_VSCTL_SETUP_BOND_FAKE_CONF +OVS_VSCTL_SETUP_BOND_FAKE_CONF([11]) AT_CHECK([RUN_OVS_VSCTL_ONELINE([del-br xapi2])], [0], [ ], [], [OVS_VSCTL_CLEANUP]) CHECK_BRIDGES([xapi1, xapi1, 0]) @@ -517,7 +525,7 @@ AT_CLEANUP AT_SETUP([fake bridge on bond + del-br real bridge]) AT_KEYWORDS([ovs-vsctl fake-bridge]) OVS_VSCTL_SETUP -OVS_VSCTL_SETUP_BOND_FAKE_CONF +OVS_VSCTL_SETUP_BOND_FAKE_CONF([11]) AT_CHECK([RUN_OVS_VSCTL([del-br xapi1])]) CHECK_BRIDGES OVS_VSCTL_CLEANUP diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in index 0d858a15..57f76d5f 100644 --- a/utilities/ovs-vsctl.8.in +++ b/utilities/ovs-vsctl.8.in @@ -69,7 +69,8 @@ When such a ``fake bridge'' is active, \fBovs\-vsctl\fR will treat it much like a bridge separate from its ``parent bridge,'' but the actual implementation in Open vSwitch uses only a single bridge, with ports on the fake bridge assigned the implicit VLAN of the fake bridge of which -they are members. +they are members. (A fake bridge for VLAN 0 receives packets that +have no 802.1Q tag or a tag with VLAN 0.) . .SH OPTIONS . @@ -179,7 +180,7 @@ nothing if \fIbridge\fR already exists as a real bridge. Creates a ``fake bridge'' named \fIbridge\fR within the existing Open vSwitch bridge \fIparent\fR, which must already exist and must not itself be a fake bridge. The new fake bridge will be on 802.1Q VLAN -\fIvlan\fR, which must be an integer between 1 and 4095. Initially +\fIvlan\fR, which must be an integer between 0 and 4095. Initially \fIbridge\fR will have no ports (other than \fIbridge\fR itself). .IP Without \fB\-\-may\-exist\fR, attempting to create a bridge that diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c index 560d747a..86889cc3 100644 --- a/utilities/ovs-vsctl.c +++ b/utilities/ovs-vsctl.c @@ -612,8 +612,13 @@ struct vsctl_bridge { struct ovsrec_controller **ctrl; char *fail_mode; size_t n_ctrl; - struct vsctl_bridge *parent; - int vlan; + + /* VLAN ("fake") bridge support. + * + * Use 'parent != NULL' to detect a fake bridge, because 'vlan' can be 0 + * in either case. */ + struct vsctl_bridge *parent; /* Real bridge, or NULL. */ + int vlan; /* VLAN VID (0...4095), or 0. */ }; struct vsctl_port { @@ -704,7 +709,7 @@ port_is_fake_bridge(const struct ovsrec_port *port_cfg) { return (port_cfg->fake_bridge && port_cfg->tag - && *port_cfg->tag >= 1 && *port_cfg->tag <= 4095); + && *port_cfg->tag >= 0 && *port_cfg->tag <= 4095); } static struct vsctl_bridge * @@ -841,7 +846,7 @@ get_info(struct vsctl_context *ctx, struct vsctl_info *info) port = xmalloc(sizeof *port); port->port_cfg = port_cfg; if (port_cfg->tag - && *port_cfg->tag >= 1 && *port_cfg->tag <= 4095) { + && *port_cfg->tag >= 0 && *port_cfg->tag <= 4095) { port->bridge = find_vlan_bridge(info, br, *port_cfg->tag); if (!port->bridge) { port->bridge = br; @@ -1329,8 +1334,8 @@ cmd_add_br(struct vsctl_context *ctx) } else if (ctx->argc == 4) { parent_name = ctx->argv[2]; vlan = atoi(ctx->argv[3]); - if (vlan < 1 || vlan > 4095) { - vsctl_fatal("%s: vlan must be between 1 and 4095", ctx->argv[0]); + if (vlan < 0 || vlan > 4095) { + vsctl_fatal("%s: vlan must be between 0 and 4095", ctx->argv[0]); } } else { vsctl_fatal("'%s' command takes exactly 1 or 3 arguments", @@ -1397,7 +1402,7 @@ cmd_add_br(struct vsctl_context *ctx) int64_t tag = vlan; parent = find_bridge(&info, parent_name, false); - if (parent && parent->vlan) { + if (parent && parent->parent) { vsctl_fatal("cannot create bridge with fake bridge as parent"); } if (!parent) { @@ -1750,7 +1755,7 @@ add_port(struct vsctl_context *ctx, ovsrec_port_set_bond_fake_iface(port, fake_iface); free(ifaces); - if (bridge->vlan) { + if (bridge->parent) { int64_t tag = bridge->vlan; ovsrec_port_set_tag(port, &tag, 1); } -- 2.30.2