ovs-vsctl has the concept of a VLAN (or "fake") bridge, which is a sort of a sub-bridge that receives only packets on a particular VLAN. There is no way to distinguish two VLAN bridges with the same parent on the same VLAN, but until now ovs-vsctl did not prevent creating duplicates or report them. This commit fixes the problem.
Reported-by: rwxybh Reported-at: https://github.com/openvswitch/ovs/issues/21 Signed-off-by: Ben Pfaff <b...@nicira.com> --- tests/ovs-vsctl.at | 3 +++ utilities/ovs-vsctl.8.in | 3 ++- utilities/ovs-vsctl.c | 19 ++++++++++++++++++- 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at index efe7817..f6e6994 100644 --- a/tests/ovs-vsctl.at +++ b/tests/ovs-vsctl.at @@ -483,6 +483,9 @@ AT_CHECK([RUN_OVS_VSCTL([--may-exist add-br xapi1 xxx $1])], [1], [], 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 $1 ], [OVS_VSCTL_CLEANUP]) +AT_CHECK([RUN_OVS_VSCTL([--may-exist add-br dup xenbr0 $1])], [1], [], + [ovs-vsctl: bridge xenbr0 already has a child VLAN bridge xapi1 on VLAN $1 +], [OVS_VSCTL_CLEANUP]) CHECK_BRIDGES([xapi1, xenbr0, $1], [xenbr0, xenbr0, 0]) CHECK_PORTS([xenbr0], [eth0]) CHECK_IFACES([xenbr0], [eth0]) diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in index 8cf13ae..4a580d7 100644 --- a/utilities/ovs-vsctl.8.in +++ b/utilities/ovs-vsctl.8.in @@ -192,7 +192,8 @@ 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 0 and 4095. Initially +\fIvlan\fR, which must be an integer between 0 and 4095. The parent +bridge must not already have a fake bridge for \fIvlan\fR. 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 8ac6d10..e33d79d 100644 --- a/utilities/ovs-vsctl.c +++ b/utilities/ovs-vsctl.c @@ -814,6 +814,9 @@ struct vsctl_iface { struct vsctl_port *port; }; +static struct vsctl_bridge *find_vlan_bridge(struct vsctl_bridge *parent, + int vlan); + static char * vsctl_context_to_string(const struct vsctl_context *ctx) { @@ -870,7 +873,15 @@ add_bridge_to_cache(struct vsctl_context *ctx, br->vlan = vlan; hmap_init(&br->children); if (parent) { - hmap_insert(&parent->children, &br->children_node, hash_int(vlan, 0)); + struct vsctl_bridge *conflict = find_vlan_bridge(parent, vlan); + if (conflict) { + VLOG_WARN("%s: bridge has multiple VLAN bridges (%s and %s) " + "for VLAN %d, but only one is allowed", + parent->name, name, conflict->name, vlan); + } else { + hmap_insert(&parent->children, &br->children_node, + hash_int(vlan, 0)); + } } shash_add(&ctx->bridges, br->name, br); return br; @@ -1660,6 +1671,7 @@ cmd_add_br(struct vsctl_context *ctx) ovs_insert_bridge(ctx->ovs, br); } else { + struct vsctl_bridge *conflict; struct vsctl_bridge *parent; struct ovsrec_port *port; struct ovsrec_bridge *br; @@ -1672,6 +1684,11 @@ cmd_add_br(struct vsctl_context *ctx) if (!parent) { vsctl_fatal("parent bridge %s does not exist", parent_name); } + conflict = find_vlan_bridge(parent, vlan); + if (conflict) { + vsctl_fatal("bridge %s already has a child VLAN bridge %s " + "on VLAN %d", parent_name, conflict->name, vlan); + } br = parent->br_cfg; iface = ovsrec_interface_insert(ctx->txn); -- 2.1.1 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
