[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]