[libvirt] [PATCH 0/4] support <vlan> element for interfaces and networks
The purpose of these patches is to define a data type that can properly describe an interface's vlan configuration (including multiple vlans for trunking), provide XML parser and formatter to translate to and from the new data type, add the new <vlan> element to both domain <interface> and to <network> in appropriate places, and to provide the new vlan data to the hypervisor driver. Once these 4 patches are applied, all that will be left to support vlan configuration for any network connection that supports it, is to request the vlan data for the interface with virDomainNetGetActualVlan(netdef). If that function returns NULL, there is no vlan config for this interface; if it's non-zero, it is a pointer to virNetDevVlan, and you can grab the data from there. Since I currently know of only two types of network connection we will be able to easily support this for (sr-iov VFs using PCI passthrough, and openvswitch), any type other than those will refuse to start if a vlan has been requested (see PATCH 4/4). This covers validation for the qemu and lxc drivers (since they use common code), but I'm not sure about xen, esx, or any other driver - they will likely require extra code to either implement transparent vlan configuration, or log an error if it's requested (my guess is that, in spite of our best efforts, *many* people will misunderstand the limitations of this functionality, and try to put all sorts of interfaces on vlans; it will be much friendlier to inform them of the problem rather than simply ignore the vlan config. == This patchset was developed on top of the 9 part patchset here: https://www.redhat.com/archives/libvir-list/2012-August/msg00293.html and the 5 part set here: https://www.redhat.com/archives/libvir-list/2012-August/msg00293.html It doesn't depend on any of the functionality, but not having those patchset will probably lead to some merge conflicts.
<portgroup> allows a <bandwidth> element, but the schema didn't have this. Since this makes for multiple elements in portgroup, they must be interleaved. <interface type='bridge'> needs to allow <virtualport> elements for openvswitch, but the schema didn't allow this. --- docs/schemas/domaincommon.rng | 3 +++ docs/schemas/network.rng | 11 ++++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index c85d763..238e57e 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1409,6 +1409,9 @@ <empty/> </element> </optional> + <optional> + <ref name="virtualPortProfile"/> + </optional> <ref name="interface-options"/> </interleave> </group> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 2ae879e..30c5a31 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -123,9 +123,14 @@ </choice> </attribute> </optional> - <optional> - <ref name="virtualPortProfile"/> - </optional> + <interleave> + <optional> + <ref name="virtualPortProfile"/> + </optional> + <optional> + <ref name="bandwidth"/> + </optional> + </interleave> </element> </zeroOrMore> -- 1.7.11.2
To allow for the possibility of vlan "trunks", which have more than one vlan tag associated with them, we need a vlan struct. Since it will be used by multiple files in src/util, src/conf, src/network, and src/qemu, it must be defined in src/util. Unfortunately there isn't currently a common file for simple netdev data definitions, so I created a new file. --- src/Makefile.am | 1 + src/libvirt_private.syms | 6 ++++ src/util/virnetdevvlan.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virnetdevvlan.h | 38 +++++++++++++++++++++ 4 files changed, 133 insertions(+) create mode 100644 src/util/virnetdevvlan.c create mode 100644 src/util/virnetdevvlan.h diff --git a/src/Makefile.am b/src/Makefile.am index 79b4e59..ff16d6f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -104,6 +104,7 @@ UTIL_SOURCES = \ util/virnetdevopenvswitch.h util/virnetdevopenvswitch.c \ util/virnetdevtap.h util/virnetdevtap.c \ util/virnetdevveth.h util/virnetdevveth.c \ + util/virnetdevvlan.h util/virnetdevvlan.c \ util/virnetdevvportprofile.h util/virnetdevvportprofile.c \ util/virnetlink.c util/virnetlink.h \ util/virrandom.h util/virrandom.c \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 89fb1f4..d20f9e3 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1425,6 +1425,12 @@ virNetDevVethCreate; virNetDevVethDelete; +# virnetdevvlan.h +virNetDevVlanClear; +virNetDevVlanCopy; +virNetDevVlanEqual; +virNetDevVlanFree; + # virnetdevvportprofile.h virNetDevVPortProfileAssociate; virNetDevVPortProfileCheckComplete; diff --git a/src/util/virnetdevvlan.c b/src/util/virnetdevvlan.c new file mode 100644 index 0000000..d66902f --- /dev/null +++ b/src/util/virnetdevvlan.c @@ -0,0 +1,88 @@ +/* + * Copyright (C) 2009-2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>. + * + * Authors: + * Laine Stump <laine@redhat.com> + */ + +#include <config.h> + +#include "internal.h" +#include "virterror_internal.h" +#include "virnetdevvlan.h" +#include "memory.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +void +virNetDevVlanClear(virNetDevVlanPtr vlan) +{ + VIR_FREE(vlan->tag); + vlan->nTags = 0; +} + +void +virNetDevVlanFree(virNetDevVlanPtr vlan) +{ + if (vlan) + virNetDevVlanClear(vlan); + VIR_FREE(vlan); +} + +int +virNetDevVlanEqual(const virNetDevVlanPtr a, const virNetDevVlanPtr b) +{ + int ii; + + if (!(a || b)) + return true; + if (!a || !b) + return false; + + if (a->trunk != b->trunk || + a->nTags != b->nTags) { + return false; + } + + for (ii = 0; ii < a->nTags; ii++) { + if (a->tag[ii] != b->tag[ii]) + return false; + } + return true; +} + +/* + * virNetDevVlanCopy - copy from src into (already existing) dst. + * If src is NULL, dst will have nTags set to 0. + * dst is assumed to be empty on entry. + */ +int +virNetDevVlanCopy(virNetDevVlanPtr dst, const virNetDevVlanPtr src) +{ + if (!src || src->nTags == 0) + return 0; + + if (VIR_ALLOC_N(dst->tag, src->nTags) < 0) { + virReportOOMError(); + return -1; + } + + dst->trunk = src->trunk; + dst->nTags = src->nTags; + memmove(dst->tag, src->tag, src->nTags * sizeof(*src->tag)); + return 0; +} diff --git a/src/util/virnetdevvlan.h b/src/util/virnetdevvlan.h new file mode 100644 index 0000000..7a046f9 --- /dev/null +++ b/src/util/virnetdevvlan.h @@ -0,0 +1,38 @@ +/* + * Copyright (C) 2009-2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>. + * + * Authors: + * Laine Stump <laine@redhat.com> + */ + +#ifndef __VIR_NETDEV_VLAN_H__ +# define __VIR_NETDEV_VLAN_H__ + +typedef struct _virNetDevVlan virNetDevVlan; +typedef virNetDevVlan *virNetDevVlanPtr; +struct _virNetDevVlan { + bool trunk; /* true if this is a trunk */ + int nTags; /* number of tags in array */ + unsigned int *tag; /* pointer to array of tags */ +}; + +void virNetDevVlanClear(virNetDevVlanPtr vlan); +void virNetDevVlanFree(virNetDevVlanPtr vlan); +int virNetDevVlanEqual(const virNetDevVlanPtr a, const virNetDevVlanPtr b); +int virNetDevVlanCopy(virNetDevVlanPtr dst, const virNetDevVlanPtr src); + +#endif /* __VIR_NETDEV_VLAN_H__ */ -- 1.7.11.2
The following config elements now support multiple <vlan> subelements: within a domain: <interface>, and the <actual> subelement of <interface> within a network: the toplevel, as well as any <portgroup> If multiple vlan elements are given, it is assumed that vlan trunking is being requested. If trunking is required with only a single tag, the attribute "trunk='yes'" should be added. Some examples: <interface type='hostdev'/> <vlan tag='42'/> <mac address='52:54:00:12:34:56'/> ... </interface> <network> <name>vlan-net</name> <vlan tag='30' trunk='yes'/> <virtualport type='openvswitch'/> </network> <interface type='network'/> <source network='vlan-net'/> ... </interface> <network> <name>trunk-vlan</name> <vlan tag='42'/> <vlan tag='43'/> ... </network> <network> <name>multi</name> ... <portgroup name='production'/> <vlan tag='42'/> </portgroup> <portgroup name='test'/> <vlan tag='666'/> </portgroup> </network> <interface type='network'/> <source network='multi' portgroup='test'/> ... </interface> IMPORTANT NOTE: As of this patch there is no backend support for the vlan element for *any* network device type. When support is added in later patches, it will only be for those select network types that support setting up a vlan on the host side, without the guest's involvement. (For example, it will be possible to configure a vlan for a guest connected to an openvswitch bridge, but it won't be possible to do that for one that is connected to a standard Linux host bridge.) --- In a couple of places I noticed mis-indented code in the context of this patch, so I fixed it. docs/formatdomain.html.in | 35 +++++++ docs/formatnetwork.html.in | 48 +++++++++ docs/schemas/domaincommon.rng | 3 + docs/schemas/network.rng | 6 ++ docs/schemas/networkcommon.rng | 15 +++ po/POTFILES.in | 1 + src/Makefile.am | 3 +- src/conf/domain_conf.c | 29 +++++- src/conf/domain_conf.h | 4 + src/conf/netdev_vlan_conf.c | 111 +++++++++++++++++++++ src/conf/netdev_vlan_conf.h | 33 ++++++ src/conf/network_conf.c | 14 ++- src/conf/network_conf.h | 3 + src/libvirt_private.syms | 6 ++ tests/networkxml2xmlin/8021Qbh-net.xml | 1 + tests/networkxml2xmlin/openvswitch-net.xml | 4 + tests/networkxml2xmlout/8021Qbh-net.xml | 1 + tests/networkxml2xmlout/openvswitch-net.xml | 4 + .../qemuxml2argvdata/qemuxml2argv-net-hostdev.xml | 1 + .../qemuxml2argv-net-openvswitch.xml | 36 +++++++ .../qemuxml2argv-net-virtio-network-portgroup.xml | 1 + tests/qemuxml2xmltest.c | 1 + 22 files changed, 355 insertions(+), 5 deletions(-) create mode 100644 src/conf/netdev_vlan_conf.c create mode 100644 src/conf/netdev_vlan_conf.h create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-openvswitch.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f3b3fa8..618710c 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2855,6 +2855,41 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">Since 0.9.4</span> </p> + <h5><a name="elementVlanTag">Setting VLAN tag (on supported network types only)</a></h5> + +<pre> + ... + <devices> + <interface type='bridge'> + <b><vlan tag='42'/></b> + <source bridge='ovsbr0'/> + <virtualport type='openvswitch'> + <parameters interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/> + </virtualport> + </interface> + <devices> + ...</pre> + + <p> + If (and only if) the network connection used by the guest + supports vlan tagging transparent to the guest, an + optional <code><vlan></code> element can specify one or + more vlan tags to apply to the guest's network + traffic <span class="since">Since 0.10.0</span>. (openvswitch + and type='hostdev' SR-IOV interfaces do support transparent vlan + tagging of guest traffic; everything else, including standard + linux bridges and libvirt's own virtual networks, <b>do not</b> + support it. 802.1Qbh (vn-link) and 802.1Qbg (VEPA) switches + provide their own way (outside of libvirt) to tag guest traffic + onto specific vlans.) As expected, the <code>tag</code> + attribute specifies which vlan tag to use. If an interface has + more than one <code><vlan></code> element defined, it is + assumed that the user wants to do VLAN trunking using all the + specified tags. In the case that vlan trunking with a single tag + is desired, the optional attribute <code>trunk='yes'</code> can + be added to the vlan element. + </p> + <h5><a name="elementLink">Modifying virtual link state</a></h5> <pre> ... diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 0ba4d2d..e5fcaa5 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -290,6 +290,54 @@ <span class="since">Since 0.9.4</span> </p> + <h5><a name="elementVlanTag">Setting VLAN tag (on supported network types only)</a></h5> + +<pre> + ... + <devices> + <interface type='bridge'> + <b><vlan tag='42'/></b> + <b><vlan tag='47'/></b> + <source bridge='ovsbr0'/> + <virtualport type='openvswitch'> + <parameters interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/> + </virtualport> + </interface> + <devices> + ...</pre> + + <p> + If (and only if) the network type supports vlan tagging + transparent to the guest, an optional <code><vlan></code> + element can specify one or more vlan tags to apply to the + traffic of all guests using this + network <span class="since">Since 0.10.0</span>. (openvswitch + and type='hostdev' SR-IOV networks do support transparent vlan + tagging of guest traffic; everything else, including standard + linux bridges and libvirt's own virtual networks, <b>do not</b> + support it. 802.1Qbh (vn-link) and 802.1Qbg (VEPA) switches + provide their own way (outside of libvirt) to tag guest traffic + onto specific vlans.) As expected, the <code>tag</code> + attribute specifies which vlan tag to use. If a network has more + than one <code><vlan></code> element defined, it is + assumed that the user wants to do VLAN trunking using all the + specified tags. In the case that vlan trunking with a single tag + is desired, the optional attribute <code>trunk='yes'</code> can + be added to the vlan element. + </p> + <p> + <code><vlan></code> elements can also be specified in + a <code><portgroup></code> element, as well as directly in + a domain's <code><interface></code> element. In the case + that a vlan tag is specified in multiple locations, the setting + in <code><interface></code> takes precedence, followed by + the setting in the <code><portgroup></code> selected by + the interface config. The <code><vlan></code> + in <code><network></code> will be selected only if none is + given in <code><portgroup></code> + or <code><interface></code>. + </p> + <h5><a name="elementsPortgroup">Portgroups</a></h5> <pre> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 238e57e..7bd53d7 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1712,6 +1712,9 @@ <optional> <ref name="bandwidth"/> </optional> + <zeroOrMore> + <ref name="vlan"/> + </zeroOrMore> </interleave> </define> <!-- diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 30c5a31..0f59b3c 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -130,6 +130,9 @@ <optional> <ref name="bandwidth"/> </optional> + <zeroOrMore> + <ref name="vlan"/> + </zeroOrMore> </interleave> </element> </zeroOrMore> @@ -177,6 +180,9 @@ <optional> <ref name="bandwidth"/> </optional> + <zeroOrMore> + <ref name="vlan"/> + </zeroOrMore> <optional> <element name="link"> <attribute name="state"> diff --git a/docs/schemas/networkcommon.rng b/docs/schemas/networkcommon.rng index f2c3330..10644b1 100644 --- a/docs/schemas/networkcommon.rng +++ b/docs/schemas/networkcommon.rng @@ -184,4 +184,19 @@ <param name="pattern">(ipv4)|(ipv6)</param> </data> </define> + + <define name="vlan"> + <element name="vlan"> + <attribute name="tag"> + <data type="unsignedInt"> + <param name="maxInclusive">4095</param> + </data> + </attribute> + <optional> + <attribute name="trunk"> + <value>yes</value> + </attribute> + </optional> + </element> + </define> </grammar> diff --git a/po/POTFILES.in b/po/POTFILES.in index e617952..6424726 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -11,6 +11,7 @@ src/conf/domain_conf.c src/conf/domain_event.c src/conf/interface_conf.c src/conf/netdev_bandwidth_conf.c +src/conf/netdev_vlan_conf.c src/conf/netdev_vport_profile_conf.c src/conf/network_conf.c src/conf/node_device_conf.c diff --git a/src/Makefile.am b/src/Makefile.am index ff16d6f..5748fd2 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -143,7 +143,8 @@ LOCK_DRIVER_SANLOCK_SOURCES = \ NETDEV_CONF_SOURCES = \ conf/netdev_bandwidth_conf.h conf/netdev_bandwidth_conf.c \ - conf/netdev_vport_profile_conf.h conf/netdev_vport_profile_conf.c + conf/netdev_vport_profile_conf.h conf/netdev_vport_profile_conf.c \ + conf/netdev_vlan_conf.h conf/netdev_vlan_conf.c # XML configuration format handling sources # Domain driver generic impl APIs diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0cda374..55679d2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -51,6 +51,7 @@ #include "secret_conf.h" #include "netdev_vport_profile_conf.h" #include "netdev_bandwidth_conf.h" +#include "netdev_vlan_conf.h" #include "virdomainlist.h" #define VIR_FROM_THIS VIR_FROM_DOMAIN @@ -1031,7 +1032,7 @@ virDomainActualNetDefFree(virDomainActualNetDefPtr def) VIR_FREE(def->virtPortProfile); virNetDevBandwidthFree(def->bandwidth); - + virNetDevVlanClear(&def->vlan); VIR_FREE(def); } @@ -1092,6 +1093,7 @@ void virDomainNetDefFree(virDomainNetDefPtr def) virNWFilterHashTableFree(def->filterparams); virNetDevBandwidthFree(def->bandwidth); + virNetDevVlanClear(&def->vlan); VIR_FREE(def); } @@ -4463,6 +4465,9 @@ virDomainActualNetDefParseXML(xmlNodePtr node, !(actual->bandwidth = virNetDevBandwidthParse(bandwidth_node))) goto error; + if (virNetDevVlanParse(ctxt, &actual->vlan) < 0) + goto error; + *def = actual; actual = NULL; ret = 0; @@ -4933,6 +4938,9 @@ virDomainNetDefParseXML(virCapsPtr caps, goto error; } + if (virNetDevVlanParse(ctxt, &def->vlan) < 0) + goto error; + cleanup: ctxt->node = oldnode; VIR_FREE(macaddr); @@ -11618,8 +11626,10 @@ virDomainActualNetDefFormat(virBufferPtr buf, return -1; } + if (virNetDevVlanFormat(&def->vlan, buf) < 0) + return -1; if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0) - return -1; + return -1; if (virNetDevBandwidthFormat(def->bandwidth, buf) < 0) return -1; @@ -11720,8 +11730,10 @@ virDomainNetDefFormat(virBufferPtr buf, break; } + if (virNetDevVlanFormat(&def->vlan, buf) < 0) + return -1; if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0) - return -1; + return -1; virBufferEscapeString(buf, "<script path='%s'/>\n", def->script); if (def->ifname && @@ -15078,6 +15090,17 @@ virDomainNetGetActualBandwidth(virDomainNetDefPtr iface) return iface->bandwidth; } +virNetDevVlanPtr +virDomainNetGetActualVlan(virDomainNetDefPtr iface) +{ + if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK && + iface->data.network.actual && + iface->data.network.actual->vlan.nTags > 0) + return &iface->data.network.actual->vlan; + if (iface->vlan.nTags > 0) + return &iface->vlan; + return 0; +} /* Return listens[ii] from the appropriate union for the graphics * type, or NULL if this is an unsuitable type, or the index is out of diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f36f7d6..eea81d5 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -43,6 +43,7 @@ # include "virnetdevvportprofile.h" # include "virnetdevopenvswitch.h" # include "virnetdevbandwidth.h" +# include "virnetdevvlan.h" # include "virobject.h" /* forward declarations of all device types, required by @@ -781,6 +782,7 @@ struct _virDomainActualNetDef { } data; virNetDevVPortProfilePtr virtPortProfile; virNetDevBandwidthPtr bandwidth; + virNetDevVlan vlan; }; /* Stores the virtual network interface configuration */ @@ -845,6 +847,7 @@ struct _virDomainNetDef { char *filter; virNWFilterHashTablePtr filterparams; virNetDevBandwidthPtr bandwidth; + virNetDevVlan vlan; int linkstate; }; @@ -2039,6 +2042,7 @@ virNetDevVPortProfilePtr virDomainNetGetActualVirtPortProfile(virDomainNetDefPtr iface); virNetDevBandwidthPtr virDomainNetGetActualBandwidth(virDomainNetDefPtr iface); +virNetDevVlanPtr virDomainNetGetActualVlan(virDomainNetDefPtr iface); int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller); diff --git a/src/conf/netdev_vlan_conf.c b/src/conf/netdev_vlan_conf.c new file mode 100644 index 0000000..3ff50b7 --- /dev/null +++ b/src/conf/netdev_vlan_conf.c @@ -0,0 +1,111 @@ +/* + * Copyright (C) 2009-2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>. + * + * Authors: + * Laine Stump <laine@redhat.com> + */ + +#include <config.h> + +#include "netdev_vlan_conf.h" +#include "virterror_internal.h" +#include "memory.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +int +virNetDevVlanParse(xmlXPathContextPtr ctxt, virNetDevVlanPtr def) +{ + int ret = -1; + xmlNodePtr save = ctxt->node; + xmlNodePtr *vlanNodes = NULL; + int nVlans = virXPathNodeSet("./vlan", ctxt, &vlanNodes); + + if (nVlans < 0) + goto error; + + if (nVlans > 0) { + int ii; + + if (VIR_ALLOC_N(def->tag, nVlans) < 0) { + virReportOOMError(); + goto error; + } + + if (nVlans > 1) + def->trunk = true; + + for (ii = 0; ii < nVlans; ii++) { + unsigned long tag; + char *trunk = NULL; + + ctxt->node = vlanNodes[ii]; + if (virXPathULong("string(./@tag)", ctxt, &tag) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing or invalid vlan tag")); + goto error; + } + if (tag > 4095) { + virReportError(VIR_ERR_XML_ERROR, + _("vlan tag %lu too large (maximum 4095)"), tag); + goto error; + } + def->tag[ii] = tag; + if ((trunk = virXPathString("string(./@trunk)", ctxt)) != NULL) { + def->trunk = STRCASEEQ(trunk, "yes"); + if (nVlans > 1 && !def->trunk) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid \"trunk='%s'\" in <vlan> - " + "trunk='yes' is required for more " + "than one vlan tag"), trunk); + goto error; + } + } + } + } + + def->nTags = nVlans; + ret = 0; +error: + ctxt->node = save; + VIR_FREE(vlanNodes); + if (ret < 0) + virNetDevVlanClear(def); + return ret; +} + +int +virNetDevVlanFormat(virNetDevVlanPtr def, virBufferPtr buf) +{ + int ii; + + if (def->nTags == 0) + return 0; + + if (!def->tag) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing vlan tag data")); + return -1; + } + + for (ii = 0; ii < def->nTags; ii++) { + virBufferAsprintf(buf, "<vlan tag='%u'%s/>\n", + def->tag[ii], + (def->nTags == 1 && def->trunk) ? " trunk='yes'" : ""); + } + return 0; +} diff --git a/src/conf/netdev_vlan_conf.h b/src/conf/netdev_vlan_conf.h new file mode 100644 index 0000000..b96f887 --- /dev/null +++ b/src/conf/netdev_vlan_conf.h @@ -0,0 +1,33 @@ +/* + * Copyright (C) 2009-2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>. + * + * Authors: + * Laine Stump <laine@redhat.com> + */ + +#ifndef __VIR_NETDEV_VLAN_CONF_H__ +# define __VIR_NETDEV_VLAN_CONF_H__ + +# include "internal.h" +# include "virnetdevvlan.h" +# include "buf.h" +# include "xml.h" + +int virNetDevVlanParse(xmlXPathContextPtr ctxt, virNetDevVlanPtr def); +int virNetDevVlanFormat(virNetDevVlanPtr def, virBufferPtr buf); + +#endif /* __VIR_NETDEV_VPORT_PROFILE_CONF_H__ */ diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 45f8e69..6545857 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -36,6 +36,7 @@ #include "network_conf.h" #include "netdev_vport_profile_conf.h" #include "netdev_bandwidth_conf.h" +#include "netdev_vlan_conf.h" #include "memory.h" #include "xml.h" #include "uuid.h" @@ -88,6 +89,7 @@ virPortGroupDefClear(virPortGroupDefPtr def) VIR_FREE(def->name); VIR_FREE(def->virtPortProfile); virNetDevBandwidthFree(def->bandwidth); + virNetDevVlanClear(&def->vlan); def->bandwidth = NULL; } @@ -187,7 +189,7 @@ void virNetworkDefFree(virNetworkDefPtr def) VIR_FREE(def->virtPortProfile); virNetDevBandwidthFree(def->bandwidth); - + virNetDevVlanClear(&def->vlan); VIR_FREE(def); } @@ -915,6 +917,9 @@ virNetworkPortGroupParseXML(virPortGroupDefPtr def, goto error; } + if (virNetDevVlanParse(ctxt, &def->vlan) < 0) + goto error; + result = 0; error: if (result < 0) { @@ -982,6 +987,9 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) (def->bandwidth = virNetDevBandwidthParse(bandwidthNode)) == NULL) goto error; + if (virNetDevVlanParse(ctxt, &def->vlan) < 0) + goto error; + /* Parse bridge information */ def->bridge = virXPathString("string(./bridge[1]/@name)", ctxt); stp = virXPathString("string(./bridge[1]/@stp)", ctxt); @@ -1448,6 +1456,8 @@ virPortGroupDefFormat(virBufferPtr buf, } virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 4); + if (virNetDevVlanFormat(&def->vlan, buf) < 0) + return -1; if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0) return -1; virNetDevBandwidthFormat(def->bandwidth, buf); @@ -1542,6 +1552,8 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags) goto error; virBufferAdjustIndent(&buf, 2); + if (virNetDevVlanFormat(&def->vlan, &buf) < 0) + goto error; if (virNetDevBandwidthFormat(def->bandwidth, &buf) < 0) goto error; virBufferAdjustIndent(&buf, -2); diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 32fc765..a029f70 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -35,6 +35,7 @@ # include "virsocketaddr.h" # include "virnetdevbandwidth.h" # include "virnetdevvportprofile.h" +# include "virnetdevvlan.h" # include "virmacaddr.h" enum virNetworkForwardType { @@ -148,6 +149,7 @@ struct _virPortGroupDef { bool isDefault; virNetDevVPortProfilePtr virtPortProfile; virNetDevBandwidthPtr bandwidth; + virNetDevVlan vlan; }; typedef struct _virNetworkDef virNetworkDef; @@ -185,6 +187,7 @@ struct _virNetworkDef { size_t nPortGroups; virPortGroupDefPtr portGroups; virNetDevBandwidthPtr bandwidth; + virNetDevVlan vlan; }; typedef struct _virNetworkObj virNetworkObj; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d20f9e3..689377e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -406,6 +406,7 @@ virDomainNetGetActualDirectMode; virDomainNetGetActualHostdev; virDomainNetGetActualType; virDomainNetGetActualVirtPortProfile; +virDomainNetGetActualVlan; virDomainNetIndexByMac; virDomainNetInsert; virDomainNetRemove; @@ -787,6 +788,11 @@ virNetDevBandwidthFormat; virNetDevBandwidthParse; +#netdev_vlan_conf.h +virNetDevVlanFormat; +virNetDevVlanParse; + + # netdev_vportprofile_conf.h virNetDevVPortProfileFormat; virNetDevVPortProfileParse; diff --git a/tests/networkxml2xmlin/8021Qbh-net.xml b/tests/networkxml2xmlin/8021Qbh-net.xml index 2d779dc..02d7278 100644 --- a/tests/networkxml2xmlin/8021Qbh-net.xml +++ b/tests/networkxml2xmlin/8021Qbh-net.xml @@ -8,6 +8,7 @@ <interface dev="eth4"/> <interface dev="eth5"/> </forward> + <vlan tag='549'/> <virtualport type="802.1Qbh"> <parameters profileid="spongebob24"/> </virtualport> diff --git a/tests/networkxml2xmlin/openvswitch-net.xml b/tests/networkxml2xmlin/openvswitch-net.xml index 8aa1897..46d0f01 100644 --- a/tests/networkxml2xmlin/openvswitch-net.xml +++ b/tests/networkxml2xmlin/openvswitch-net.xml @@ -4,11 +4,15 @@ <forward mode='bridge'/> <virtualport type='openvswitch'/> <portgroup name='bob' default='yes'> + <vlan tag='666' trunk='yes'/> <virtualport> <parameters profileid='bob-profile'/> </virtualport> </portgroup> <portgroup name='alice'> + <vlan tag='777'/> + <vlan tag='888'/> + <vlan tag='999'/> <virtualport> <parameters profileid='alice-profile'/> </virtualport> diff --git a/tests/networkxml2xmlout/8021Qbh-net.xml b/tests/networkxml2xmlout/8021Qbh-net.xml index d4d5b4b..9385240 100644 --- a/tests/networkxml2xmlout/8021Qbh-net.xml +++ b/tests/networkxml2xmlout/8021Qbh-net.xml @@ -8,6 +8,7 @@ <interface dev='eth4'/> <interface dev='eth5'/> </forward> + <vlan tag='549'/> <virtualport type='802.1Qbh'> <parameters profileid='spongebob24'/> </virtualport> diff --git a/tests/networkxml2xmlout/openvswitch-net.xml b/tests/networkxml2xmlout/openvswitch-net.xml index 8aa1897..46d0f01 100644 --- a/tests/networkxml2xmlout/openvswitch-net.xml +++ b/tests/networkxml2xmlout/openvswitch-net.xml @@ -4,11 +4,15 @@ <forward mode='bridge'/> <virtualport type='openvswitch'/> <portgroup name='bob' default='yes'> + <vlan tag='666' trunk='yes'/> <virtualport> <parameters profileid='bob-profile'/> </virtualport> </portgroup> <portgroup name='alice'> + <vlan tag='777'/> + <vlan tag='888'/> + <vlan tag='999'/> <virtualport> <parameters profileid='alice-profile'/> </virtualport> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml index 51b09e9..43384b3 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml @@ -26,6 +26,7 @@ <source> <address type='pci' domain='0x0002' bus='0x03' slot='0x07' function='0x1'/> </source> + <vlan tag='42'/> <virtualport type='802.1Qbg'> <parameters managerid='11' typeid='1193047' typeidversion='2' instanceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/> </virtualport> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-openvswitch.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-openvswitch.xml new file mode 100644 index 0000000..a2753eb --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-openvswitch.xml @@ -0,0 +1,36 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <interface type='network'> + <mac address='00:11:22:33:44:55'/> + <source network='ovs-net'/> + <vlan tag='42'/> + <vlan tag='48'/> + <vlan tag='456'/> + <virtualport type='openvswitch'> + <parameters interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f' profileid='bob'/> + </virtualport> + </interface> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-network-portgroup.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-network-portgroup.xml index 6d379a0..829be52 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-network-portgroup.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-network-portgroup.xml @@ -24,6 +24,7 @@ <interface type='network'> <mac address='00:11:22:33:44:55'/> <source network='rednet' portgroup='bob'/> + <vlan tag='4095'/> <virtualport type='802.1Qbg'> <parameters managerid='11' typeid='1193047' typeidversion='2' instanceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/> </virtualport> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index dcdba4f..da298b7 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -178,6 +178,7 @@ mymain(void) DO_TEST("net-eth-ifname"); DO_TEST("net-virtio-network-portgroup"); DO_TEST("net-hostdev"); + DO_TEST("net-openvswitch"); DO_TEST("sound"); DO_TEST("sound-device"); DO_TEST("net-bandwidth"); -- 1.7.11.2
On Mon, Aug 13, 2012 at 12:54:14AM -0400, Laine Stump wrote:
The following config elements now support multiple <vlan> subelements:
within a domain: <interface>, and the <actual> subelement of <interface> within a network: the toplevel, as well as any <portgroup>
If multiple vlan elements are given, it is assumed that vlan trunking is being requested. If trunking is required with only a single tag, the attribute "trunk='yes'" should be added.
I'm not sure I like the fact that there was 2 different configs which can both enable vlan trunking. I can understand why you don't want to repeat 'trunk=yes' on all <vlan> elements, but I feel that if trunking is enabled, there should be something visible in the XML to that effect. Also I'm concerned about what we do if we need to maintain further attributes related to vlans in the future. I think it'd be more future proof to use <vlan trunk='yes|no'> <tag id='42'/> ...more <tag>... </vlan> even if it is slightly more verbose in the single tag case.
Some examples:
<interface type='hostdev'/> <vlan tag='42'/> <mac address='52:54:00:12:34:56'/> ... </interface>
<network> <name>vlan-net</name> <vlan tag='30' trunk='yes'/> <virtualport type='openvswitch'/> </network>
<interface type='network'/> <source network='vlan-net'/> ... </interface>
<network> <name>trunk-vlan</name> <vlan tag='42'/> <vlan tag='43'/> ... </network>
<network> <name>multi</name> ... <portgroup name='production'/> <vlan tag='42'/> </portgroup> <portgroup name='test'/> <vlan tag='666'/> </portgroup> </network>
<interface type='network'/> <source network='multi' portgroup='test'/> ... </interface>
Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
The network driver now looks for the vlan element in network and portgroup objects, and logs an error at network define time if a vlan is requested for a network type that doesn't support it. (Currently vlan configuration is only supported for openvswitch networks, and networks used to do hostdev assignment of SR-IOV VFs.) At runtime, the three potential sources of vlan information are examined in this order: interface, chosen portgroup, network, and the first that is non-empty is used. Another check for valid network type is made at this time, since the interface may have requested a vlan (a legal thing to have in the interface config, since it's not known until runtime if the chosen network will actually support it). Since we must also check for domains requesting vlans for unsupported connection types even if they are type='network', and since networkAllocateActualDevice() is being called in exactly the correct places, and has all of the necessary information to check, I slightly modified the logic of that function so that interfaces that aren't type='network' don't just return immediately. Instead, they also perform all the same validation for supported features. Because of this, it's not necessary to make this identical check in the other three places that would normally require it: 1) qemu domain startup, 2) qemu device hotplug, 3) lxc domain startup. This can be seen as a first step in consolidating network-related functionality into the network driver, rather than having copies of the same code spread around in multiple places; this will make it easier to split the network parts off into a separate deamon, as we've discussed recently. --- src/network/bridge_driver.c | 113 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 103 insertions(+), 10 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index d5275f4..3ae4bae 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2246,6 +2246,42 @@ cleanup: } +static int +networkValidate(virNetworkDefPtr def) +{ + int ii; + bool vlanUsed, vlanAllowed; + + /* The only type of networks that currently support transparent + * vlan configuration are those using hostdev sr-iov devices from + * a pool, and those using an openvswitch bridge. + */ + + vlanAllowed = (def->forwardType == VIR_NETWORK_FORWARD_BRIDGE && + def->virtPortProfile && + def->virtPortProfile->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH); + + vlanUsed = def->vlan.nTags > 0; + for (ii = 0; ii < def->nPortGroups && !(vlanUsed && vlanAllowed); ii++) { + if (def->portGroups[ii].vlan.nTags > 0) + vlanUsed = true; + if (def->forwardType == VIR_NETWORK_FORWARD_BRIDGE && + def->portGroups[ii].virtPortProfile && + (def->portGroups[ii].virtPortProfile->virtPortType + == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH)) { + vlanAllowed = true; + } + } + if (vlanUsed && !vlanAllowed) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("<vlan> element specified for network %s, " + "whose type doesn't support vlan configuration"), + def->name); + return -1; + } + return 0; +} + static virNetworkPtr networkCreate(virConnectPtr conn, const char *xml) { struct network_driver *driver = conn->networkPrivateData; virNetworkDefPtr def; @@ -2273,6 +2309,9 @@ static virNetworkPtr networkCreate(virConnectPtr conn, const char *xml) { virNetworkSetBridgeMacAddr(def); } + if (networkValidate(def) < 0) + goto cleanup; + if (!(network = virNetworkAssignDef(&driver->networks, def))) goto cleanup; @@ -2346,6 +2385,9 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) { } } + if (networkValidate(def) < 0) + goto cleanup; + if (!(network = virNetworkAssignDef(&driver->networks, def))) goto cleanup; @@ -2744,18 +2786,24 @@ int networkAllocateActualDevice(virDomainNetDefPtr iface) { struct network_driver *driver = driverState; - virNetworkObjPtr network; - virNetworkDefPtr netdef; - virPortGroupDefPtr portgroup; - virNetDevVPortProfilePtr virtport = NULL; + enum virDomainNetType actualType = iface->type; + virNetworkObjPtr network = NULL; + virNetworkDefPtr netdef = NULL; + virPortGroupDefPtr portgroup = NULL; + virNetDevVPortProfilePtr virtport = iface->virtPortProfile; + virNetDevVlanPtr vlan = NULL; virNetworkForwardIfDefPtr dev = NULL; unsigned int num_virt_fns = 0; char **vfname = NULL; int ii; int ret = -1; + /* it's handy to have this initialized if we skip directly to validate */ + if (iface->vlan.nTags > 0) + vlan = &iface->vlan; + if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) - return 0; + goto validate; virDomainActualNetDefFree(iface->data.network.actual); iface->data.network.actual = NULL; @@ -2815,7 +2863,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) goto error; } - iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_BRIDGE; + iface->data.network.actual->type = actualType = VIR_DOMAIN_NET_TYPE_BRIDGE; iface->data.network.actual->data.bridge.brname = strdup(netdef->bridge); if (!iface->data.network.actual->data.bridge.brname) { virReportOOMError(); @@ -2861,7 +2909,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) } /* Set type=direct and appropriate <source mode='xxx'/> */ - iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_DIRECT; + iface->data.network.actual->type = actualType = VIR_DOMAIN_NET_TYPE_DIRECT; switch (netdef->forwardType) { case VIR_NETWORK_FORWARD_BRIDGE: iface->data.network.actual->data.direct.mode = VIR_NETDEV_MACVLAN_MODE_BRIDGE; @@ -2999,6 +3047,49 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) if (virNetDevVPortProfileCheckComplete(virtport) < 0) goto error; + /* copy appropriate vlan info to actualNet */ + if (iface->vlan.nTags > 0) + vlan = &iface->vlan; + else if (portgroup && portgroup->vlan.nTags > 0) + vlan = &portgroup->vlan; + else if (netdef && netdef->vlan.nTags > 0) + vlan = &netdef->vlan; + + if (virNetDevVlanCopy(&iface->data.network.actual->vlan, vlan) < 0) + goto error; + +validate: + /* make sure that everything now specified for the device is + * actually supported on this type of network. NB: network, + * netdev, and iface->data.network.actual may all be NULL. + */ + + if (vlan) { + /* vlan configuration via libvirt is only supported for + * PCI Passthrough SR-IOV devices and openvswitch bridges. + * otherwise log an error and fail + */ + if (!(actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV || + (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE && + virtport && virtport->virtPortType + == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH))) { + if (netdef) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("an interface connecting to network '%s' " + "is requesting a vlan tag, but that is not " + "supported for this type of network"), + netdef->name); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("an interface of type '%s' " + "is requesting a vlan tag, but that is not " + "supported for this type of connection"), + virDomainNetTypeToString(iface->type)); + } + goto error; + } + } + if (dev) { /* we are now assured of success, so mark the allocation */ dev->connections++; @@ -3006,9 +3097,11 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) dev->dev, dev->connections); } - netdef->connections++; - VIR_DEBUG("Using network %s, %d connections", - netdef->name, netdef->connections); + if (netdef) { + netdef->connections++; + VIR_DEBUG("Using network %s, %d connections", + netdef->name, netdef->connections); + } ret = 0; error: for (ii = 0; ii < num_virt_fns; ii++) -- 1.7.11.2
I've modified the XML as suggested by danpb, rebased, and reposted the entire series: http://www.redhat.com/archives/libvir-list/2012-August/msg00907.html Consider this set obsolete.
participants (2)
-
Daniel P. Berrange -
Laine Stump