[libvirt] [PATCH] network: Allow <vlan trunk='yes'/> without explicit <tag> subelements

Until now at least one <tag> subelement was required for a <vlan> element. An optional trunk='yes' attribute can be used to indicate trunking of the given tags. This is not how VLAN tagging works with ESX. One can either specify a single VLAN tag per virtual network or enable trunking for all possible VLAN tags. It is not possible to limit trunking to a subset of tags. This cannot be represented properly with the current network config format. This patch removes the requirement for at least on tag per <vlan> element and adapts all related code to deal with the new situation. --- docs/formatnetwork.html.in | 8 +++- docs/schemas/networkcommon.rng | 4 +- src/conf/domain_conf.c | 6 ++- src/conf/netdev_vlan_conf.c | 58 ++++++++++++++++++++--------------- src/network/bridge_driver.c | 28 +++++++++++++++++ src/util/virnetdevopenvswitch.c | 9 ++++- src/util/virnetdevvlan.c | 8 ++++- tests/networkxml2xmlin/esx-net.xml | 15 +++++++++ tests/networkxml2xmlout/esx-net.xml | 15 +++++++++ tests/networkxml2xmltest.c | 1 + 10 files changed, 118 insertions(+), 34 deletions(-) create mode 100644 tests/networkxml2xmlin/esx-net.xml create mode 100644 tests/networkxml2xmlout/esx-net.xml diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 49206dd..5d601e1 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -398,7 +398,7 @@ transparent to the guest, an optional <code>&lt;vlan&gt;</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 + 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> @@ -410,7 +410,11 @@ 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. + be added to the vlan element. <span class="since">Since 0.10.3</span> + <code>trunk='yes'</code> can be set while specifing no + <code>&lt;vlan&gt;</code> elements at all to indicate trunking of all + available vlan tags. This is supported by ESX only and is the only + trunking mode that ESX supports. </p> <p> <code>&lt;vlan&gt;</code> elements can also be specified in diff --git a/docs/schemas/networkcommon.rng b/docs/schemas/networkcommon.rng index c7749e7..70cdcbb 100644 --- a/docs/schemas/networkcommon.rng +++ b/docs/schemas/networkcommon.rng @@ -192,7 +192,7 @@ <value>yes</value> </attribute> </optional> - <oneOrMore> + <zeroOrMore> <element name="tag"> <attribute name="id"> <data type="unsignedInt"> @@ -201,7 +201,7 @@ </attribute> <empty/> </element> - </oneOrMore> + </zeroOrMore> </element> </define> </grammar> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 33e1e7f..65a201c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14897,9 +14897,11 @@ virDomainNetGetActualVlan(virDomainNetDefPtr iface) { if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK && iface->data.network.actual && - iface->data.network.actual->vlan.nTags > 0) + (iface->data.network.actual->vlan.nTags > 0 || + iface->data.network.actual->vlan.trunk)) return &iface->data.network.actual->vlan; - if (iface->vlan.nTags > 0) + if (iface->vlan.nTags > 0 || + iface->vlan.trunk) return &iface->vlan; return 0; } diff --git a/src/conf/netdev_vlan_conf.c b/src/conf/netdev_vlan_conf.c index 6a0511a..8f9aa04 100644 --- a/src/conf/netdev_vlan_conf.c +++ b/src/conf/netdev_vlan_conf.c @@ -42,33 +42,28 @@ virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlanPtr de if (nTags < 0) goto error; - if (nTags == 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing tag id - each <vlan> must have " - "at least one <tag id='n'/> subelement")); - goto error; - } - - if (VIR_ALLOC_N(def->tag, nTags) < 0) { - virReportOOMError(); - goto error; - } - - for (ii = 0; ii < nTags; ii++) { - unsigned long id; - - ctxt->node = tagNodes[ii]; - if (virXPathULong("string(./@id)", ctxt, &id) < 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing or invalid vlan tag id attribute")); + if (nTags > 0) { + if (VIR_ALLOC_N(def->tag, nTags) < 0) { + virReportOOMError(); goto error; } - if (id > 4095) { - virReportError(VIR_ERR_XML_ERROR, - _("vlan tag id %lu too large (maximum 4095)"), id); - goto error; + + for (ii = 0; ii < nTags; ii++) { + unsigned long id; + + ctxt->node = tagNodes[ii]; + if (virXPathULong("string(./@id)", ctxt, &id) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing or invalid vlan tag id attribute")); + goto error; + } + if (id > 4095) { + virReportError(VIR_ERR_XML_ERROR, + _("vlan tag id %lu too large (maximum 4095)"), id); + goto error; + } + def->tag[ii] = id; } - def->tag[ii] = id; } def->nTags = nTags; @@ -99,6 +94,14 @@ virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlanPtr de } } + /* only in trunk mode the tag subelements can be omitted */ + if (!def->trunk && nTags == 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid element - each <vlan> must have at least " + "one <tag id='n'/> subelement and/or \"trunk='yes'\"")); + goto error; + } + ret = 0; error: ctxt->node = save; @@ -113,8 +116,13 @@ virNetDevVlanFormat(virNetDevVlanPtr def, virBufferPtr buf) { int ii; - if (def->nTags == 0) + if (def->nTags == 0) { + if (def->trunk) { + virBufferAddLit(buf, "<vlan trunk='yes'/>\n"); + } + return 0; + } if (!def->tag) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index e1846ee..37ddb07 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2616,6 +2616,14 @@ networkValidate(virNetworkDefPtr def) int ii; bool vlanUsed, vlanAllowed; + if (def->vlan.nTags == 0 && def->vlan.trunk) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("<vlan> element specified for network %s is missing at " + "least one <tag> subelement required for trunk mode"), + def->name); + return -1; + } + /* 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 Open vSwitch bridge. @@ -2627,6 +2635,13 @@ networkValidate(virNetworkDefPtr def) vlanUsed = def->vlan.nTags > 0; for (ii = 0; ii < def->nPortGroups && !(vlanUsed && vlanAllowed); ii++) { + if (def->portGroups[ii].vlan.nTags == 0 && def->portGroups[ii].vlan.trunk) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("<vlan> element specified for portgroup %s of network %s " + "is missing at least one <tag> subelement required for trunk mode"), + def->portGroups[ii].name, def->name); + return -1; + } if (def->portGroups[ii].vlan.nTags > 0) vlanUsed = true; if (def->forwardType == VIR_NETWORK_FORWARD_BRIDGE && @@ -3352,6 +3367,8 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) /* it's handy to have this initialized if we skip directly to validate */ if (iface->vlan.nTags > 0) vlan = &iface->vlan; + else if (iface->vlan.trunk) + goto trunk_error; if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) goto validate; @@ -3627,10 +3644,16 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) /* copy appropriate vlan info to actualNet */ if (iface->vlan.nTags > 0) vlan = &iface->vlan; + else if (iface->vlan.trunk) + goto trunk_error; else if (portgroup && portgroup->vlan.nTags > 0) vlan = &portgroup->vlan; + else if (portgroup && portgroup->vlan.trunk) + goto trunk_error; else if (netdef && netdef->vlan.nTags > 0) vlan = &netdef->vlan; + else if (netdef && netdef->vlan.trunk) + goto trunk_error; if (virNetDevVlanCopy(&iface->data.network.actual->vlan, vlan) < 0) goto error; @@ -3699,6 +3722,11 @@ error: iface->data.network.actual = NULL; } goto cleanup; + +trunk_error: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("vlan trunk mode without a tag is not supported")); + goto cleanup; } /* networkNotifyActualDevice: diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index a6993b6..3b29dcc 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -80,14 +80,19 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, goto out_of_memory; } - if (virtVlan && virtVlan->nTags > 0) { + if (virtVlan && (virtVlan->nTags > 0 || virtVlan->trunk)) { + if (virtVlan->nTags == 0 && virtVlan->trunk) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("vlan trunk mode without a tag is not supported")); + goto cleanup; + } /* Trunk port first */ if (virtVlan->trunk) { virBufferAddLit(&buf, "trunk="); /* - * Trunk ports have at least one VLAN. Do the first one + * Trunk ports must have at least one VLAN. Do the first one * outside the "for" loop so we can put a "," at the * start of the for loop if there are more than one VLANs * on this trunk port. diff --git a/src/util/virnetdevvlan.c b/src/util/virnetdevvlan.c index 7a6ff9b..4cf95dc 100644 --- a/src/util/virnetdevvlan.c +++ b/src/util/virnetdevvlan.c @@ -31,6 +31,7 @@ void virNetDevVlanClear(virNetDevVlanPtr vlan) { + vlan->trunk = false; VIR_FREE(vlan->tag); vlan->nTags = 0; } @@ -79,9 +80,14 @@ virNetDevVlanEqual(const virNetDevVlanPtr a, const virNetDevVlanPtr b) int virNetDevVlanCopy(virNetDevVlanPtr dst, const virNetDevVlanPtr src) { - if (!src || src->nTags == 0) + if (!src) return 0; + if (src->nTags == 0) { + dst->trunk = src->trunk; + return 0; + } + if (VIR_ALLOC_N(dst->tag, src->nTags) < 0) { virReportOOMError(); return -1; diff --git a/tests/networkxml2xmlin/esx-net.xml b/tests/networkxml2xmlin/esx-net.xml new file mode 100644 index 0000000..546c634 --- /dev/null +++ b/tests/networkxml2xmlin/esx-net.xml @@ -0,0 +1,15 @@ +<network> + <name>vSwitch0</name> + <uuid>2f467347-8b4e-8655-e7d6-c4c3fb968009</uuid> + <forward dev='vmnic0' mode='bridge'> + <interface dev='vmnic0'/> + </forward> + <portgroup name='Network1'> + <vlan trunk='yes'/> + </portgroup> + <portgroup name='Network2'> + <vlan> + <tag id='536'/> + </vlan> + </portgroup> +</network> diff --git a/tests/networkxml2xmlout/esx-net.xml b/tests/networkxml2xmlout/esx-net.xml new file mode 100644 index 0000000..546c634 --- /dev/null +++ b/tests/networkxml2xmlout/esx-net.xml @@ -0,0 +1,15 @@ +<network> + <name>vSwitch0</name> + <uuid>2f467347-8b4e-8655-e7d6-c4c3fb968009</uuid> + <forward dev='vmnic0' mode='bridge'> + <interface dev='vmnic0'/> + </forward> + <portgroup name='Network1'> + <vlan trunk='yes'/> + </portgroup> + <portgroup name='Network2'> + <vlan> + <tag id='536'/> + </vlan> + </portgroup> +</network> diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c index e57d190..426c541 100644 --- a/tests/networkxml2xmltest.c +++ b/tests/networkxml2xmltest.c @@ -108,6 +108,7 @@ mymain(void) DO_TEST_FULL("passthrough-pf", VIR_NETWORK_XML_INACTIVE); DO_TEST("hostdev"); DO_TEST_FULL("hostdev-pf", VIR_NETWORK_XML_INACTIVE); + DO_TEST("esx-net"); return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 1.7.4.1