[libvirt] [PATCHv4] Configure native vlan modes on Open vSwitch ports
This patch adds functionality to allow libvirt to configure the 'native-tagged' and 'native-untagged' modes on openvswitch networks. v2 changes: Fix problems reported by Eric Blake v3 changes: Re work patch to address review comments v4 changes: Use enum for native modes --- diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 9284534..ba32185 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3498,6 +3498,13 @@ qemu-kvm -net nic,model=? /dev/null <parameters interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/> </virtualport> </interface> + <interface type='bridge'> + <b><vlan trunk='yes'></b> + <b><tag id='42'/></b> + <b><tag id='123' nativeMode='untagged'/></b> + <b></vlan></b> + ... + </interface> <devices> ...</pre> @@ -3524,6 +3531,15 @@ qemu-kvm -net nic,model=? /dev/null vlan element. </p> + <p> + For network connections using openvswitch it is possible to + configure the 'native-tagged' and 'native-untagged' vlan modes + <span class="since">(Since 1.0.5).</span> This uses the optional + <code>nativeMode</code> attribute on the <code><tag></code> + element: <code>nativeMode</code> may be set to 'tagged' or + 'untagged'. The id atribute of the element sets the native vlan. + </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 a1198ce..29e12d9 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -446,6 +446,13 @@ <parameters interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/> </virtualport> </interface> + <interface type='bridge'> + <b><vlan trunk='yes'></b> + <b><tag id='42'/></b> + <b><tag id='123' nativeMode='untagged'/></b> + <b></vlan></b> + ... + </interface> <devices> ...</pre> @@ -469,6 +476,14 @@ be added to the vlan element. </p> <p> + For network connections using openvswitch it is possible to + configure the 'native-tagged' and 'native-untagged' vlan modes + <span class="since">(Since 1.0.6).</span> This uses the optional + <code>nativeMode</code> attribute on the <code><tag></code> + element: <code>nativeMode</code> may be set to 'tagged' or + 'untagged'. The id atribute of the element sets the native vlan. + </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 diff --git a/docs/schemas/networkcommon.rng b/docs/schemas/networkcommon.rng index 51ff759..e60f1fc 100644 --- a/docs/schemas/networkcommon.rng +++ b/docs/schemas/networkcommon.rng @@ -204,6 +204,14 @@ <param name="maxInclusive">4095</param> </data> </attribute> + <optional> + <attribute name="nativeMode"> + <choice> + <value>tagged</value> + <value>untagged</value> + </choice> + </attribute> + </optional> <empty/> </element> </oneOrMore> diff --git a/src/conf/netdev_vlan_conf.c b/src/conf/netdev_vlan_conf.c index 13ba8c6..2b4cd48 100644 --- a/src/conf/netdev_vlan_conf.c +++ b/src/conf/netdev_vlan_conf.c @@ -17,6 +17,7 @@ * * Authors: * Laine Stump <laine@redhat.com> + * James Robson <jrobson@websense.com> */ #include <config.h> @@ -27,12 +28,15 @@ #define VIR_FROM_THIS VIR_FROM_NONE +VIR_ENUM_IMPL(virNativeVlanMode, VIR_NATIVE_VLAN_MODE_LAST, "default", "tagged", "untagged") + int virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlanPtr def) { int ret = -1; xmlNodePtr save = ctxt->node; const char *trunk = NULL; + const char *nativeMode = NULL; xmlNodePtr *tagNodes = NULL; int nTags, ii; @@ -54,6 +58,8 @@ virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlanPtr de goto error; } + def->nativeMode = 0; + def->nativeTag = 0; for (ii = 0; ii < nTags; ii++) { unsigned long id; @@ -68,6 +74,19 @@ virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlanPtr de _("vlan tag id %lu too large (maximum 4095)"), id); goto error; } + if ((nativeMode = virXPathString("string(./@nativeMode)", ctxt)) != NULL) { + if (def->nativeMode != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("duplicate native vlan setting")); + goto error; + } + if ((def->nativeMode = virNativeVlanModeTypeFromString(nativeMode)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Attribute \"nativeMode='%s'\" is invalid"), nativeMode); + goto error; + } + def->nativeTag = id; + } def->tag[ii] = id; } @@ -89,6 +108,12 @@ virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlanPtr de "is required for more than one vlan tag"), trunk); goto error; } + if (def->nativeMode != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid configuration in <vlan> - \"trunk='no'\" is " + "not allowed with a native vlan id")); + goto error; + } /* allow (but discard) "trunk='no' if there is a single tag */ if (STRCASENEQ(trunk, "no")) { virReportError(VIR_ERR_XML_ERROR, @@ -125,7 +150,17 @@ virNetDevVlanFormat(virNetDevVlanPtr def, virBufferPtr buf) virBufferAsprintf(buf, "<vlan%s>\n", def->trunk ? " trunk='yes'" : ""); for (ii = 0; ii < def->nTags; ii++) { - virBufferAsprintf(buf, " <tag id='%u'/>\n", def->tag[ii]); + if (def->nativeMode != VIR_NATIVE_VLAN_MODE_DEFAULT && def->nativeTag == def->tag[ii]) { + /* check the nativeMode in case we get <tag id='0'/>*/ + const char *mode = virNativeVlanModeTypeToString(def->nativeMode); + if (!mode) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Bad vlaue for nativeMode")); + } + virBufferAsprintf(buf, " <tag id='%u' nativeMode='%s'/>\n", def->tag[ii], mode); + } else { + virBufferAsprintf(buf, " <tag id='%u'/>\n", def->tag[ii]); + } } virBufferAddLit(buf, "</vlan>\n"); return 0; diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 2aee445..47e6027 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -109,8 +109,22 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, virCommandAddArgList(cmd, "--timeout=5", "--", "--may-exist", "add-port", brname, ifname, NULL); - if (virBufferUse(&buf) != 0) + if (virBufferUse(&buf) != 0) { + switch (virtVlan->nativeMode) { + case VIR_NATIVE_VLAN_MODE_TAGGED: + virCommandAddArg(cmd, "vlan_mode=native-tagged"); + virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag); + break; + case VIR_NATIVE_VLAN_MODE_UNTAGGED: + virCommandAddArg(cmd, "vlan_mode=native-untagged"); + virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag); + break; + case VIR_NATIVE_VLAN_MODE_DEFAULT: + default: + break; + } virCommandAddArgList(cmd, virBufferCurrentContent(&buf), NULL); + } if (ovsport->profileID[0] == '\0') { virCommandAddArgList(cmd, diff --git a/src/util/virnetdevvlan.c b/src/util/virnetdevvlan.c index 2fe2017..eed32f7 100644 --- a/src/util/virnetdevvlan.c +++ b/src/util/virnetdevvlan.c @@ -33,6 +33,8 @@ virNetDevVlanClear(virNetDevVlanPtr vlan) { VIR_FREE(vlan->tag); vlan->nTags = 0; + vlan->nativeMode = 0; + vlan->nativeTag = 0; } void @@ -54,7 +56,9 @@ virNetDevVlanEqual(const virNetDevVlanPtr a, const virNetDevVlanPtr b) return false; if (a->trunk != b->trunk || - a->nTags != b->nTags) { + a->nTags != b->nTags || + a->nativeMode != b->nativeMode || + a->nativeTag != b->nativeTag) { return false; } @@ -89,6 +93,8 @@ virNetDevVlanCopy(virNetDevVlanPtr dst, const virNetDevVlanPtr src) dst->trunk = src->trunk; dst->nTags = src->nTags; + dst->nativeMode = src->nativeMode; + dst->nativeTag = src->nativeTag; memcpy(dst->tag, src->tag, src->nTags * sizeof(*src->tag)); return 0; } diff --git a/src/util/virnetdevvlan.h b/src/util/virnetdevvlan.h index c6b16ef..a084938 100644 --- a/src/util/virnetdevvlan.h +++ b/src/util/virnetdevvlan.h @@ -18,16 +18,28 @@ * Authors: * Laine Stump <laine@redhat.com> */ - +# include <virutil.h> #ifndef __VIR_NETDEV_VLAN_H__ # define __VIR_NETDEV_VLAN_H__ +typedef enum { + VIR_NATIVE_VLAN_MODE_DEFAULT = 0, + VIR_NATIVE_VLAN_MODE_TAGGED, + VIR_NATIVE_VLAN_MODE_UNTAGGED, + + VIR_NATIVE_VLAN_MODE_LAST +} virNativeVlanMode; + +VIR_ENUM_DECL(virNativeVlanMode) + 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 */ + int nativeMode; + unsigned int nativeTag; }; void virNetDevVlanClear(virNetDevVlanPtr vlan); diff --git a/tests/networkxml2xmlin/openvswitch-net.xml b/tests/networkxml2xmlin/openvswitch-net.xml index a3d82b1..2f6084d 100644 --- a/tests/networkxml2xmlin/openvswitch-net.xml +++ b/tests/networkxml2xmlin/openvswitch-net.xml @@ -21,4 +21,13 @@ <parameters profileid='alice-profile'/> </virtualport> </portgroup> + <portgroup name='native'> + <vlan trunk='yes'> + <tag id='123' nativeMode='tagged'/> + <tag id='444'/> + </vlan> + <virtualport> + <parameters profileid='native-profile'/> + </virtualport> + </portgroup> </network> diff --git a/tests/networkxml2xmlout/openvswitch-net.xml b/tests/networkxml2xmlout/openvswitch-net.xml index a3d82b1..2f6084d 100644 --- a/tests/networkxml2xmlout/openvswitch-net.xml +++ b/tests/networkxml2xmlout/openvswitch-net.xml @@ -21,4 +21,13 @@ <parameters profileid='alice-profile'/> </virtualport> </portgroup> + <portgroup name='native'> + <vlan trunk='yes'> + <tag id='123' nativeMode='tagged'/> + <tag id='444'/> + </vlan> + <virtualport> + <parameters profileid='native-profile'/> + </virtualport> + </portgroup> </network> Protected by Websense Hosted Email Security -- www.websense.com
Are there any comments on this iteration? On Thu, 2013-05-23 at 18:12 +0100, james robson wrote:
This patch adds functionality to allow libvirt to configure the 'native-tagged' and 'native-untagged' modes on openvswitch networks.
v2 changes: Fix problems reported by Eric Blake
v3 changes: Re work patch to address review comments
v4 changes: Use enum for native modes
---
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 9284534..ba32185 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3498,6 +3498,13 @@ qemu-kvm -net nic,model=? /dev/null <parameters interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/> </virtualport> </interface> + <interface type='bridge'> + <b><vlan trunk='yes'></b> + <b><tag id='42'/></b> + <b><tag id='123' nativeMode='untagged'/></b> + <b></vlan></b> + ... + </interface> <devices> ...</pre>
@@ -3524,6 +3531,15 @@ qemu-kvm -net nic,model=? /dev/null vlan element. </p>
+ <p> + For network connections using openvswitch it is possible to + configure the 'native-tagged' and 'native-untagged' vlan modes + <span class="since">(Since 1.0.5).</span> This uses the optional + <code>nativeMode</code> attribute on the <code><tag></code> + element: <code>nativeMode</code> may be set to 'tagged' or + 'untagged'. The id atribute of the element sets the native vlan. + </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 a1198ce..29e12d9 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -446,6 +446,13 @@ <parameters interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/> </virtualport> </interface> + <interface type='bridge'> + <b><vlan trunk='yes'></b> + <b><tag id='42'/></b> + <b><tag id='123' nativeMode='untagged'/></b> + <b></vlan></b> + ... + </interface> <devices> ...</pre>
@@ -469,6 +476,14 @@ be added to the vlan element. </p> <p> + For network connections using openvswitch it is possible to + configure the 'native-tagged' and 'native-untagged' vlan modes + <span class="since">(Since 1.0.6).</span> This uses the optional + <code>nativeMode</code> attribute on the <code><tag></code> + element: <code>nativeMode</code> may be set to 'tagged' or + 'untagged'. The id atribute of the element sets the native vlan. + </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 diff --git a/docs/schemas/networkcommon.rng b/docs/schemas/networkcommon.rng index 51ff759..e60f1fc 100644 --- a/docs/schemas/networkcommon.rng +++ b/docs/schemas/networkcommon.rng @@ -204,6 +204,14 @@ <param name="maxInclusive">4095</param> </data> </attribute> + <optional> + <attribute name="nativeMode"> + <choice> + <value>tagged</value> + <value>untagged</value> + </choice> + </attribute> + </optional> <empty/> </element> </oneOrMore> diff --git a/src/conf/netdev_vlan_conf.c b/src/conf/netdev_vlan_conf.c index 13ba8c6..2b4cd48 100644 --- a/src/conf/netdev_vlan_conf.c +++ b/src/conf/netdev_vlan_conf.c @@ -17,6 +17,7 @@ * * Authors: * Laine Stump <laine@redhat.com> + * James Robson <jrobson@websense.com> */
#include <config.h> @@ -27,12 +28,15 @@
#define VIR_FROM_THIS VIR_FROM_NONE
+VIR_ENUM_IMPL(virNativeVlanMode, VIR_NATIVE_VLAN_MODE_LAST, "default", "tagged", "untagged") + int virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlanPtr def) { int ret = -1; xmlNodePtr save = ctxt->node; const char *trunk = NULL; + const char *nativeMode = NULL; xmlNodePtr *tagNodes = NULL; int nTags, ii;
@@ -54,6 +58,8 @@ virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlanPtr de goto error; }
+ def->nativeMode = 0; + def->nativeTag = 0; for (ii = 0; ii < nTags; ii++) { unsigned long id;
@@ -68,6 +74,19 @@ virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlanPtr de _("vlan tag id %lu too large (maximum 4095)"), id); goto error; } + if ((nativeMode = virXPathString("string(./@nativeMode)", ctxt)) != NULL) { + if (def->nativeMode != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("duplicate native vlan setting")); + goto error; + } + if ((def->nativeMode = virNativeVlanModeTypeFromString(nativeMode)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Attribute \"nativeMode='%s'\" is invalid"), nativeMode); + goto error; + } + def->nativeTag = id; + } def->tag[ii] = id; }
@@ -89,6 +108,12 @@ virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlanPtr de "is required for more than one vlan tag"), trunk); goto error; } + if (def->nativeMode != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid configuration in <vlan> - \"trunk='no'\" is " + "not allowed with a native vlan id")); + goto error; + } /* allow (but discard) "trunk='no' if there is a single tag */ if (STRCASENEQ(trunk, "no")) { virReportError(VIR_ERR_XML_ERROR, @@ -125,7 +150,17 @@ virNetDevVlanFormat(virNetDevVlanPtr def, virBufferPtr buf)
virBufferAsprintf(buf, "<vlan%s>\n", def->trunk ? " trunk='yes'" : ""); for (ii = 0; ii < def->nTags; ii++) { - virBufferAsprintf(buf, " <tag id='%u'/>\n", def->tag[ii]); + if (def->nativeMode != VIR_NATIVE_VLAN_MODE_DEFAULT && def->nativeTag == def->tag[ii]) { + /* check the nativeMode in case we get <tag id='0'/>*/ + const char *mode = virNativeVlanModeTypeToString(def->nativeMode); + if (!mode) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Bad vlaue for nativeMode")); + } + virBufferAsprintf(buf, " <tag id='%u' nativeMode='%s'/>\n", def->tag[ii], mode); + } else { + virBufferAsprintf(buf, " <tag id='%u'/>\n", def->tag[ii]); + } } virBufferAddLit(buf, "</vlan>\n"); return 0; diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 2aee445..47e6027 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -109,8 +109,22 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, virCommandAddArgList(cmd, "--timeout=5", "--", "--may-exist", "add-port", brname, ifname, NULL);
- if (virBufferUse(&buf) != 0) + if (virBufferUse(&buf) != 0) { + switch (virtVlan->nativeMode) { + case VIR_NATIVE_VLAN_MODE_TAGGED: + virCommandAddArg(cmd, "vlan_mode=native-tagged"); + virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag); + break; + case VIR_NATIVE_VLAN_MODE_UNTAGGED: + virCommandAddArg(cmd, "vlan_mode=native-untagged"); + virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag); + break; + case VIR_NATIVE_VLAN_MODE_DEFAULT: + default: + break; + } virCommandAddArgList(cmd, virBufferCurrentContent(&buf), NULL); + }
if (ovsport->profileID[0] == '\0') { virCommandAddArgList(cmd, diff --git a/src/util/virnetdevvlan.c b/src/util/virnetdevvlan.c index 2fe2017..eed32f7 100644 --- a/src/util/virnetdevvlan.c +++ b/src/util/virnetdevvlan.c @@ -33,6 +33,8 @@ virNetDevVlanClear(virNetDevVlanPtr vlan) { VIR_FREE(vlan->tag); vlan->nTags = 0; + vlan->nativeMode = 0; + vlan->nativeTag = 0; }
void @@ -54,7 +56,9 @@ virNetDevVlanEqual(const virNetDevVlanPtr a, const virNetDevVlanPtr b) return false;
if (a->trunk != b->trunk || - a->nTags != b->nTags) { + a->nTags != b->nTags || + a->nativeMode != b->nativeMode || + a->nativeTag != b->nativeTag) { return false; }
@@ -89,6 +93,8 @@ virNetDevVlanCopy(virNetDevVlanPtr dst, const virNetDevVlanPtr src)
dst->trunk = src->trunk; dst->nTags = src->nTags; + dst->nativeMode = src->nativeMode; + dst->nativeTag = src->nativeTag; memcpy(dst->tag, src->tag, src->nTags * sizeof(*src->tag)); return 0; } diff --git a/src/util/virnetdevvlan.h b/src/util/virnetdevvlan.h index c6b16ef..a084938 100644 --- a/src/util/virnetdevvlan.h +++ b/src/util/virnetdevvlan.h @@ -18,16 +18,28 @@ * Authors: * Laine Stump <laine@redhat.com> */ - +# include <virutil.h> #ifndef __VIR_NETDEV_VLAN_H__ # define __VIR_NETDEV_VLAN_H__
+typedef enum { + VIR_NATIVE_VLAN_MODE_DEFAULT = 0, + VIR_NATIVE_VLAN_MODE_TAGGED, + VIR_NATIVE_VLAN_MODE_UNTAGGED, + + VIR_NATIVE_VLAN_MODE_LAST +} virNativeVlanMode; + +VIR_ENUM_DECL(virNativeVlanMode) + 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 */ + int nativeMode; + unsigned int nativeTag; };
void virNetDevVlanClear(virNetDevVlanPtr vlan); diff --git a/tests/networkxml2xmlin/openvswitch-net.xml b/tests/networkxml2xmlin/openvswitch-net.xml index a3d82b1..2f6084d 100644 --- a/tests/networkxml2xmlin/openvswitch-net.xml +++ b/tests/networkxml2xmlin/openvswitch-net.xml @@ -21,4 +21,13 @@ <parameters profileid='alice-profile'/> </virtualport> </portgroup> + <portgroup name='native'> + <vlan trunk='yes'> + <tag id='123' nativeMode='tagged'/> + <tag id='444'/> + </vlan> + <virtualport> + <parameters profileid='native-profile'/> + </virtualport> + </portgroup> </network> diff --git a/tests/networkxml2xmlout/openvswitch-net.xml b/tests/networkxml2xmlout/openvswitch-net.xml index a3d82b1..2f6084d 100644 --- a/tests/networkxml2xmlout/openvswitch-net.xml +++ b/tests/networkxml2xmlout/openvswitch-net.xml @@ -21,4 +21,13 @@ <parameters profileid='alice-profile'/> </virtualport> </portgroup> + <portgroup name='native'> + <vlan trunk='yes'> + <tag id='123' nativeMode='tagged'/> + <tag id='444'/> + </vlan> + <virtualport> + <parameters profileid='native-profile'/> + </virtualport> + </portgroup> </network>
Protected by Websense Hosted Email Security -- www.websense.com
On 06/17/2013 01:56 PM, james robson wrote:
Are there any comments on this iteration?
Sorry, it somehow got lost in my unread list mail. I'll do my best to review it before the end of the week.
On Thu, 2013-05-23 at 18:12 +0100, james robson wrote:
This patch adds functionality to allow libvirt to configure the 'native-tagged' and 'native-untagged' modes on openvswitch networks.
v2 changes: Fix problems reported by Eric Blake
v3 changes: Re work patch to address review comments
v4 changes: Use enum for native modes
---
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 9284534..ba32185 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3498,6 +3498,13 @@ qemu-kvm -net nic,model=? /dev/null <parameters interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/> </virtualport> </interface> + <interface type='bridge'> + <b><vlan trunk='yes'></b> + <b><tag id='42'/></b> + <b><tag id='123' nativeMode='untagged'/></b> + <b></vlan></b> + ... + </interface> <devices> ...</pre>
@@ -3524,6 +3531,15 @@ qemu-kvm -net nic,model=? /dev/null vlan element. </p>
+ <p> + For network connections using openvswitch it is possible to + configure the 'native-tagged' and 'native-untagged' vlan modes + <span class="since">(Since 1.0.5).</span> This uses the optional + <code>nativeMode</code> attribute on the <code><tag></code> + element: <code>nativeMode</code> may be set to 'tagged' or + 'untagged'. The id atribute of the element sets the native vlan. + </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 a1198ce..29e12d9 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -446,6 +446,13 @@ <parameters interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/> </virtualport> </interface> + <interface type='bridge'> + <b><vlan trunk='yes'></b> + <b><tag id='42'/></b> + <b><tag id='123' nativeMode='untagged'/></b> + <b></vlan></b> + ... + </interface> <devices> ...</pre>
@@ -469,6 +476,14 @@ be added to the vlan element. </p> <p> + For network connections using openvswitch it is possible to + configure the 'native-tagged' and 'native-untagged' vlan modes + <span class="since">(Since 1.0.6).</span> This uses the optional + <code>nativeMode</code> attribute on the <code><tag></code> + element: <code>nativeMode</code> may be set to 'tagged' or + 'untagged'. The id atribute of the element sets the native vlan. + </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 diff --git a/docs/schemas/networkcommon.rng b/docs/schemas/networkcommon.rng index 51ff759..e60f1fc 100644 --- a/docs/schemas/networkcommon.rng +++ b/docs/schemas/networkcommon.rng @@ -204,6 +204,14 @@ <param name="maxInclusive">4095</param> </data> </attribute> + <optional> + <attribute name="nativeMode"> + <choice> + <value>tagged</value> + <value>untagged</value> + </choice> + </attribute> + </optional> <empty/> </element> </oneOrMore> diff --git a/src/conf/netdev_vlan_conf.c b/src/conf/netdev_vlan_conf.c index 13ba8c6..2b4cd48 100644 --- a/src/conf/netdev_vlan_conf.c +++ b/src/conf/netdev_vlan_conf.c @@ -17,6 +17,7 @@ * * Authors: * Laine Stump <laine@redhat.com> + * James Robson <jrobson@websense.com> */
#include <config.h> @@ -27,12 +28,15 @@
#define VIR_FROM_THIS VIR_FROM_NONE
+VIR_ENUM_IMPL(virNativeVlanMode, VIR_NATIVE_VLAN_MODE_LAST, "default", "tagged", "untagged") + int virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlanPtr def) { int ret = -1; xmlNodePtr save = ctxt->node; const char *trunk = NULL; + const char *nativeMode = NULL; xmlNodePtr *tagNodes = NULL; int nTags, ii;
@@ -54,6 +58,8 @@ virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlanPtr de goto error; }
+ def->nativeMode = 0; + def->nativeTag = 0; for (ii = 0; ii < nTags; ii++) { unsigned long id;
@@ -68,6 +74,19 @@ virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlanPtr de _("vlan tag id %lu too large (maximum 4095)"), id); goto error; } + if ((nativeMode = virXPathString("string(./@nativeMode)", ctxt)) != NULL) { + if (def->nativeMode != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("duplicate native vlan setting")); + goto error; + } + if ((def->nativeMode = virNativeVlanModeTypeFromString(nativeMode)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Attribute \"nativeMode='%s'\" is invalid"), nativeMode); + goto error; + } + def->nativeTag = id; + } def->tag[ii] = id; }
@@ -89,6 +108,12 @@ virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlanPtr de "is required for more than one vlan tag"), trunk); goto error; } + if (def->nativeMode != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid configuration in <vlan> - \"trunk='no'\" is " + "not allowed with a native vlan id")); + goto error; + } /* allow (but discard) "trunk='no' if there is a single tag */ if (STRCASENEQ(trunk, "no")) { virReportError(VIR_ERR_XML_ERROR, @@ -125,7 +150,17 @@ virNetDevVlanFormat(virNetDevVlanPtr def, virBufferPtr buf)
virBufferAsprintf(buf, "<vlan%s>\n", def->trunk ? " trunk='yes'" : ""); for (ii = 0; ii < def->nTags; ii++) { - virBufferAsprintf(buf, " <tag id='%u'/>\n", def->tag[ii]); + if (def->nativeMode != VIR_NATIVE_VLAN_MODE_DEFAULT && def->nativeTag == def->tag[ii]) { + /* check the nativeMode in case we get <tag id='0'/>*/ + const char *mode = virNativeVlanModeTypeToString(def->nativeMode); + if (!mode) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Bad vlaue for nativeMode")); + } + virBufferAsprintf(buf, " <tag id='%u' nativeMode='%s'/>\n", def->tag[ii], mode); + } else { + virBufferAsprintf(buf, " <tag id='%u'/>\n", def->tag[ii]); + } } virBufferAddLit(buf, "</vlan>\n"); return 0; diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 2aee445..47e6027 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -109,8 +109,22 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, virCommandAddArgList(cmd, "--timeout=5", "--", "--may-exist", "add-port", brname, ifname, NULL);
- if (virBufferUse(&buf) != 0) + if (virBufferUse(&buf) != 0) { + switch (virtVlan->nativeMode) { + case VIR_NATIVE_VLAN_MODE_TAGGED: + virCommandAddArg(cmd, "vlan_mode=native-tagged"); + virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag); + break; + case VIR_NATIVE_VLAN_MODE_UNTAGGED: + virCommandAddArg(cmd, "vlan_mode=native-untagged"); + virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag); + break; + case VIR_NATIVE_VLAN_MODE_DEFAULT: + default: + break; + } virCommandAddArgList(cmd, virBufferCurrentContent(&buf), NULL); + }
if (ovsport->profileID[0] == '\0') { virCommandAddArgList(cmd, diff --git a/src/util/virnetdevvlan.c b/src/util/virnetdevvlan.c index 2fe2017..eed32f7 100644 --- a/src/util/virnetdevvlan.c +++ b/src/util/virnetdevvlan.c @@ -33,6 +33,8 @@ virNetDevVlanClear(virNetDevVlanPtr vlan) { VIR_FREE(vlan->tag); vlan->nTags = 0; + vlan->nativeMode = 0; + vlan->nativeTag = 0; }
void @@ -54,7 +56,9 @@ virNetDevVlanEqual(const virNetDevVlanPtr a, const virNetDevVlanPtr b) return false;
if (a->trunk != b->trunk || - a->nTags != b->nTags) { + a->nTags != b->nTags || + a->nativeMode != b->nativeMode || + a->nativeTag != b->nativeTag) { return false; }
@@ -89,6 +93,8 @@ virNetDevVlanCopy(virNetDevVlanPtr dst, const virNetDevVlanPtr src)
dst->trunk = src->trunk; dst->nTags = src->nTags; + dst->nativeMode = src->nativeMode; + dst->nativeTag = src->nativeTag; memcpy(dst->tag, src->tag, src->nTags * sizeof(*src->tag)); return 0; } diff --git a/src/util/virnetdevvlan.h b/src/util/virnetdevvlan.h index c6b16ef..a084938 100644 --- a/src/util/virnetdevvlan.h +++ b/src/util/virnetdevvlan.h @@ -18,16 +18,28 @@ * Authors: * Laine Stump <laine@redhat.com> */ - +# include <virutil.h> #ifndef __VIR_NETDEV_VLAN_H__ # define __VIR_NETDEV_VLAN_H__
+typedef enum { + VIR_NATIVE_VLAN_MODE_DEFAULT = 0, + VIR_NATIVE_VLAN_MODE_TAGGED, + VIR_NATIVE_VLAN_MODE_UNTAGGED, + + VIR_NATIVE_VLAN_MODE_LAST +} virNativeVlanMode; + +VIR_ENUM_DECL(virNativeVlanMode) + 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 */ + int nativeMode; + unsigned int nativeTag; };
void virNetDevVlanClear(virNetDevVlanPtr vlan); diff --git a/tests/networkxml2xmlin/openvswitch-net.xml b/tests/networkxml2xmlin/openvswitch-net.xml index a3d82b1..2f6084d 100644 --- a/tests/networkxml2xmlin/openvswitch-net.xml +++ b/tests/networkxml2xmlin/openvswitch-net.xml @@ -21,4 +21,13 @@ <parameters profileid='alice-profile'/> </virtualport> </portgroup> + <portgroup name='native'> + <vlan trunk='yes'> + <tag id='123' nativeMode='tagged'/> + <tag id='444'/> + </vlan> + <virtualport> + <parameters profileid='native-profile'/> + </virtualport> + </portgroup> </network> diff --git a/tests/networkxml2xmlout/openvswitch-net.xml b/tests/networkxml2xmlout/openvswitch-net.xml index a3d82b1..2f6084d 100644 --- a/tests/networkxml2xmlout/openvswitch-net.xml +++ b/tests/networkxml2xmlout/openvswitch-net.xml @@ -21,4 +21,13 @@ <parameters profileid='alice-profile'/> </virtualport> </portgroup> + <portgroup name='native'> + <vlan trunk='yes'> + <tag id='123' nativeMode='tagged'/> + <tag id='444'/> + </vlan> + <virtualport> + <parameters profileid='native-profile'/> + </virtualport> + </portgroup> </network>
Protected by Websense Hosted Email Security -- www.websense.com
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 06/17/2013 01:56 PM, james robson wrote: <...snip...>
diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 2aee445..47e6027 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -109,8 +109,22 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, virCommandAddArgList(cmd, "--timeout=5", "--", "--may-exist", "add-port", brname, ifname, NULL);
- if (virBufferUse(&buf) != 0) + if (virBufferUse(&buf) != 0) { + switch (virtVlan->nativeMode) { + case VIR_NATIVE_VLAN_MODE_TAGGED: + virCommandAddArg(cmd, "vlan_mode=native-tagged"); + virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag); + break; + case VIR_NATIVE_VLAN_MODE_UNTAGGED: + virCommandAddArg(cmd, "vlan_mode=native-untagged"); + virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag); + break; + case VIR_NATIVE_VLAN_MODE_DEFAULT: + default: + break; + } virCommandAddArgList(cmd, virBufferCurrentContent(&buf), NULL); + }
My overnight Coverity run has found the following problem with above code. The issue is that at line 84 there's an "if (virtVlan &&" condition; whereas, this reference at line 112 doesn't check virtVlan before referencing: 68 (1) Event cond_false: Condition "virAsprintf(&attachedmac_ex_id, "external-ids:attached-mac=\"%s\"", macaddrstr) < 0", taking false branch 69 if (virAsprintf(&attachedmac_ex_id, "external-ids:attached-mac=\"%s\"", 70 macaddrstr) < 0) 71 goto out_of_memory; (2) Event cond_false: Condition "virAsprintf(&ifaceid_ex_id, "external-ids:iface-id=\"%s\"", ifuuidstr) < 0", taking false branch 72 if (virAsprintf(&ifaceid_ex_id, "external-ids:iface-id=\"%s\"", 73 ifuuidstr) < 0) 74 goto out_of_memory; (3) Event cond_false: Condition "virAsprintf(&vmid_ex_id, "external-ids:vm-id=\"%s\"", vmuuidstr) < 0", taking false branch 75 if (virAsprintf(&vmid_ex_id, "external-ids:vm-id=\"%s\"", 76 vmuuidstr) < 0) 77 goto out_of_memory; (4) Event cond_true: Condition "ovsport->profileID[0] != 0", taking true branch 78 if (ovsport->profileID[0] != '\0') { (5) Event cond_false: Condition "virAsprintf(&profile_ex_id, "external-ids:port-profile=\"%s\"", ovsport->profileID) < 0", taking false branch 79 if (virAsprintf(&profile_ex_id, "external-ids:port-profile=\"%s\"", 80 ovsport->profileID) < 0) 81 goto out_of_memory; 82 } 83 (6) Event cond_false: Condition "virtVlan", taking false branch (8) Event var_compare_op: Comparing "virtVlan" to null implies that "virtVlan" might be null. Also see events: [var_deref_op] 84 if (virtVlan && virtVlan->nTags > 0) { 85 86 /* Trunk port first */ 87 if (virtVlan->trunk) { 88 virBufferAddLit(&buf, "trunk="); 89 90 /* 91 * Trunk ports have at least one VLAN. Do the first one 92 * outside the "for" loop so we can put a "," at the 93 * start of the for loop if there are more than one VLANs 94 * on this trunk port. 95 */ 96 virBufferAsprintf(&buf, "%d", virtVlan->tag[i]); 97 98 for (i = 1; i < virtVlan->nTags; i++) { 99 virBufferAddLit(&buf, ","); 100 virBufferAsprintf(&buf, "%d", virtVlan->tag[i]); 101 } 102 } else if (virtVlan->nTags) { 103 virBufferAsprintf(&buf, "tag=%d", virtVlan->tag[0]); 104 } (7) Event if_end: End of if statement 105 } 106 107 cmd = virCommandNew(OVSVSCTL); 108 109 virCommandAddArgList(cmd, "--timeout=5", "--", "--may-exist", "add-port", 110 brname, ifname, NULL); 111 (9) Event var_deref_op: Dereferencing null pointer "virtVlan". Also see events: [var_compare_op] 112 switch (virtVlan->nativeMode) { 113 case VIR_NATIVE_VLAN_MODE_TAGGED: 114 virCommandAddArg(cmd, "vlan_mode=native-tagged"); 115 virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag); 116 break; 117 case VIR_NATIVE_VLAN_MODE_UNTAGGED: 118 virCommandAddArg(cmd, "vlan_mode=native-untagged"); 119 virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag); 120 break; 121 case VIR_NATIVE_VLAN_MODE_DEFAULT: 122 default: 123 break; 124 } <...snip...>
Commit 861d40565 added code (my personal change to "clean up" the submitter's code, *not* the fault of the submitter) that dereferenced virtVlan without first checking for NULL. This patch fixes that and, as part of the fix, cleans up some unnecessary obtuseness. --- John - Does this eliminate the Coverity complaints? src/util/virnetdevopenvswitch.c | 48 ++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 5834e4e..75b196c 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -81,9 +81,27 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, goto out_of_memory; } + cmd = virCommandNew(OVSVSCTL); + + virCommandAddArgList(cmd, "--timeout=5", "--", "--may-exist", "add-port", + brname, ifname, NULL); + if (virtVlan && virtVlan->nTags > 0) { - /* Trunk port first */ + switch (virtVlan->nativeMode) { + case VIR_NATIVE_VLAN_MODE_TAGGED: + virCommandAddArg(cmd, "vlan_mode=native-tagged"); + virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag); + break; + case VIR_NATIVE_VLAN_MODE_UNTAGGED: + virCommandAddArg(cmd, "vlan_mode=native-untagged"); + virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag); + break; + case VIR_NATIVE_VLAN_MODE_DEFAULT: + default: + break; + } + if (virtVlan->trunk) { virBufferAddLit(&buf, "trunk="); @@ -99,33 +117,15 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, virBufferAddLit(&buf, ","); virBufferAsprintf(&buf, "%d", virtVlan->tag[i]); } + + if (virBufferError(&buf)) + goto out_of_memory; + virCommandAddArg(cmd, virBufferCurrentContent(&buf)); } else if (virtVlan->nTags) { - virBufferAsprintf(&buf, "tag=%d", virtVlan->tag[0]); + virCommandAddArgFormat(cmd, "tag=%d", virtVlan->tag[0]); } } - cmd = virCommandNew(OVSVSCTL); - - virCommandAddArgList(cmd, "--timeout=5", "--", "--may-exist", "add-port", - brname, ifname, NULL); - - switch (virtVlan->nativeMode) { - case VIR_NATIVE_VLAN_MODE_TAGGED: - virCommandAddArg(cmd, "vlan_mode=native-tagged"); - virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag); - break; - case VIR_NATIVE_VLAN_MODE_UNTAGGED: - virCommandAddArg(cmd, "vlan_mode=native-untagged"); - virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag); - break; - case VIR_NATIVE_VLAN_MODE_DEFAULT: - default: - break; - } - - if (virBufferUse(&buf) != 0) - virCommandAddArgList(cmd, virBufferCurrentContent(&buf), NULL); - if (ovsport->profileID[0] == '\0') { virCommandAddArgList(cmd, "--", "set", "Interface", ifname, attachedmac_ex_id, -- 1.7.11.7
On 06/25/2013 12:11 PM, Laine Stump wrote:
Commit 861d40565 added code (my personal change to "clean up" the submitter's code, *not* the fault of the submitter) that dereferenced virtVlan without first checking for NULL. This patch fixes that and, as part of the fix, cleans up some unnecessary obtuseness. --- John - Does this eliminate the Coverity complaints?
src/util/virnetdevopenvswitch.c | 48 ++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 24 deletions(-)
Yes - ACK John
On 06/25/2013 01:30 PM, John Ferlan wrote:
On 06/25/2013 12:11 PM, Laine Stump wrote:
Commit 861d40565 added code (my personal change to "clean up" the submitter's code, *not* the fault of the submitter) that dereferenced virtVlan without first checking for NULL. This patch fixes that and, as part of the fix, cleans up some unnecessary obtuseness. --- John - Does this eliminate the Coverity complaints?
src/util/virnetdevopenvswitch.c | 48 ++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 24 deletions(-)
Yes - ACK
Pushed. Thanks!
On 05/23/2013 01:12 PM, james robson wrote:
This patch adds functionality to allow libvirt to configure the 'native-tagged' and 'native-untagged' modes on openvswitch networks.
v2 changes: Fix problems reported by Eric Blake
v3 changes: Re work patch to address review comments
v4 changes: Use enum for native modes
The comments about differences between revisionis of a patch should be added with "git send-email --annotate" rather than included in the commit log message, since want what we push in the end to not include any of those. Normally people add them below the "---" line (see just below here). Not to worry though, since I'll be pushing the patch, I'll just edit the commit log before I push.
---
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 9284534..ba32185 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3498,6 +3498,13 @@ qemu-kvm -net nic,model=? /dev/null <parameters interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/> </virtualport> </interface> + <interface type='bridge'> + <b><vlan trunk='yes'></b> + <b><tag id='42'/></b> + <b><tag id='123' nativeMode='untagged'/></b> + <b></vlan></b> + ... + </interface> <devices> ...</pre>
@@ -3524,6 +3531,15 @@ qemu-kvm -net nic,model=? /dev/null vlan element. </p>
+ <p> + For network connections using openvswitch it is possible to + configure the 'native-tagged' and 'native-untagged' vlan modes + <span class="since">(Since 1.0.5).</span> This uses the optional
This is now 1.0.7 :-)
+ <code>nativeMode</code> attribute on the <code><tag></code> + element: <code>nativeMode</code> may be set to 'tagged' or + 'untagged'. The id atribute of the element sets the native vlan. + </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 a1198ce..29e12d9 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -446,6 +446,13 @@ <parameters interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/> </virtualport> </interface> + <interface type='bridge'> + <b><vlan trunk='yes'></b> + <b><tag id='42'/></b> + <b><tag id='123' nativeMode='untagged'/></b> + <b></vlan></b> + ... + </interface> <devices> ...</pre>
@@ -469,6 +476,14 @@ be added to the vlan element. </p> <p> + For network connections using openvswitch it is possible to + configure the 'native-tagged' and 'native-untagged' vlan modes + <span class="since">(Since 1.0.6).</span> This uses the optional
1.0.7
+ <code>nativeMode</code> attribute on the <code><tag></code> + element: <code>nativeMode</code> may be set to 'tagged' or + 'untagged'. The id atribute of the element sets the native vlan. + </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 diff --git a/docs/schemas/networkcommon.rng b/docs/schemas/networkcommon.rng index 51ff759..e60f1fc 100644 --- a/docs/schemas/networkcommon.rng +++ b/docs/schemas/networkcommon.rng @@ -204,6 +204,14 @@ <param name="maxInclusive">4095</param> </data> </attribute> + <optional> + <attribute name="nativeMode"> + <choice> + <value>tagged</value> + <value>untagged</value> + </choice> + </attribute> + </optional> <empty/> </element> </oneOrMore> diff --git a/src/conf/netdev_vlan_conf.c b/src/conf/netdev_vlan_conf.c index 13ba8c6..2b4cd48 100644 --- a/src/conf/netdev_vlan_conf.c +++ b/src/conf/netdev_vlan_conf.c @@ -17,6 +17,7 @@ * * Authors: * Laine Stump <laine@redhat.com> + * James Robson <jrobson@websense.com> */
#include <config.h> @@ -27,12 +28,15 @@
#define VIR_FROM_THIS VIR_FROM_NONE
+VIR_ENUM_IMPL(virNativeVlanMode, VIR_NATIVE_VLAN_MODE_LAST, "default", "tagged", "untagged") +
Yes, that's the stuff. This lack of enum was what I was hung up on last iteration, if I remember correctly.
int virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlanPtr def) { int ret = -1; xmlNodePtr save = ctxt->node; const char *trunk = NULL; + const char *nativeMode = NULL; xmlNodePtr *tagNodes = NULL; int nTags, ii;
@@ -54,6 +58,8 @@ virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlanPtr de goto error; }
+ def->nativeMode = 0; + def->nativeTag = 0; for (ii = 0; ii < nTags; ii++) { unsigned long id;
@@ -68,6 +74,19 @@ virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlanPtr de _("vlan tag id %lu too large (maximum 4095)"), id); goto error; } + if ((nativeMode = virXPathString("string(./@nativeMode)", ctxt)) != NULL) { + if (def->nativeMode != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("duplicate native vlan setting")); + goto error; + } + if ((def->nativeMode = virNativeVlanModeTypeFromString(nativeMode)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Attribute \"nativeMode='%s'\" is invalid"), nativeMode); + goto error; + } + def->nativeTag = id; + } def->tag[ii] = id; }
@@ -89,6 +108,12 @@ virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlanPtr de "is required for more than one vlan tag"), trunk); goto error; } + if (def->nativeMode != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid configuration in <vlan> - \"trunk='no'\" is " + "not allowed with a native vlan id")); + goto error; + } /* allow (but discard) "trunk='no' if there is a single tag */ if (STRCASENEQ(trunk, "no")) { virReportError(VIR_ERR_XML_ERROR, @@ -125,7 +150,17 @@ virNetDevVlanFormat(virNetDevVlanPtr def, virBufferPtr buf)
virBufferAsprintf(buf, "<vlan%s>\n", def->trunk ? " trunk='yes'" : ""); for (ii = 0; ii < def->nTags; ii++) { - virBufferAsprintf(buf, " <tag id='%u'/>\n", def->tag[ii]); + if (def->nativeMode != VIR_NATIVE_VLAN_MODE_DEFAULT && def->nativeTag == def->tag[ii]) { + /* check the nativeMode in case we get <tag id='0'/>*/ + const char *mode = virNativeVlanModeTypeToString(def->nativeMode); + if (!mode) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Bad vlaue for nativeMode")); + } + virBufferAsprintf(buf, " <tag id='%u' nativeMode='%s'/>\n", def->tag[ii], mode); + } else { + virBufferAsprintf(buf, " <tag id='%u'/>\n", def->tag[ii]); + } } virBufferAddLit(buf, "</vlan>\n"); return 0; diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 2aee445..47e6027 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -109,8 +109,22 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, virCommandAddArgList(cmd, "--timeout=5", "--", "--may-exist", "add-port", brname, ifname, NULL);
- if (virBufferUse(&buf) != 0) + if (virBufferUse(&buf) != 0) { + switch (virtVlan->nativeMode) { + case VIR_NATIVE_VLAN_MODE_TAGGED: + virCommandAddArg(cmd, "vlan_mode=native-tagged"); + virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag); + break; + case VIR_NATIVE_VLAN_MODE_UNTAGGED: + virCommandAddArg(cmd, "vlan_mode=native-untagged"); + virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag); + break; + case VIR_NATIVE_VLAN_MODE_DEFAULT: + default: + break; + }
I don't see a reason for putting this inside the "if virBufferUse(&buf)", since that will always be true as long as virtVlan->nTags is at least 1, and there would be no way virtVlan->nativMode to be set it nTags was 0. So I moved it out - if virtVlan->nativeMode == DEFAULT, then no args will be output.
virCommandAddArgList(cmd, virBufferCurrentContent(&buf), NULL); + }
if (ovsport->profileID[0] == '\0') { virCommandAddArgList(cmd, diff --git a/src/util/virnetdevvlan.c b/src/util/virnetdevvlan.c index 2fe2017..eed32f7 100644 --- a/src/util/virnetdevvlan.c +++ b/src/util/virnetdevvlan.c @@ -33,6 +33,8 @@ virNetDevVlanClear(virNetDevVlanPtr vlan) { VIR_FREE(vlan->tag); vlan->nTags = 0; + vlan->nativeMode = 0; + vlan->nativeTag = 0; }
void @@ -54,7 +56,9 @@ virNetDevVlanEqual(const virNetDevVlanPtr a, const virNetDevVlanPtr b) return false;
if (a->trunk != b->trunk || - a->nTags != b->nTags) { + a->nTags != b->nTags || + a->nativeMode != b->nativeMode || + a->nativeTag != b->nativeTag) { return false; }
@@ -89,6 +93,8 @@ virNetDevVlanCopy(virNetDevVlanPtr dst, const virNetDevVlanPtr src)
dst->trunk = src->trunk; dst->nTags = src->nTags; + dst->nativeMode = src->nativeMode; + dst->nativeTag = src->nativeTag; memcpy(dst->tag, src->tag, src->nTags * sizeof(*src->tag)); return 0; } diff --git a/src/util/virnetdevvlan.h b/src/util/virnetdevvlan.h index c6b16ef..a084938 100644 --- a/src/util/virnetdevvlan.h +++ b/src/util/virnetdevvlan.h @@ -18,16 +18,28 @@ * Authors: * Laine Stump <laine@redhat.com> */ - +# include <virutil.h>
You should put required #includes *after* the #ifndef __File_Identifier
#ifndef __VIR_NETDEV_VLAN_H__ # define __VIR_NETDEV_VLAN_H__
+typedef enum { + VIR_NATIVE_VLAN_MODE_DEFAULT = 0, + VIR_NATIVE_VLAN_MODE_TAGGED, + VIR_NATIVE_VLAN_MODE_UNTAGGED, + + VIR_NATIVE_VLAN_MODE_LAST +} virNativeVlanMode; + +VIR_ENUM_DECL(virNativeVlanMode) + 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 */ + int nativeMode; + unsigned int nativeTag; };
void virNetDevVlanClear(virNetDevVlanPtr vlan); diff --git a/tests/networkxml2xmlin/openvswitch-net.xml b/tests/networkxml2xmlin/openvswitch-net.xml index a3d82b1..2f6084d 100644git gggggdfdffdfdfdfdf --- a/tests/networkxml2xmlin/openvswitch-net.xml +++ b/tests/networkxml2xmlin/openvswitch-net.xml @@ -21,4 +21,13 @@ <parameters profileid='alice-profile'/> </virtualport> </portgroup> + <portgroup name='native'> + <vlan trunk='yes'> + <tag id='123' nativeMode='tagged'/> + <tag id='444'/> + </vlan> + <virtualport> + <parameters profileid='native-profile'/> + </virtualport> + </portgroup> </network> diff --git a/tests/networkxml2xmlout/openvswitch-net.xml b/tests/networkxml2xmlout/openvswitch-net.xml index a3d82b1..2f6084d 100644 --- a/tests/networkxml2xmlout/openvswitch-net.xml +++ b/tests/networkxml2xmlout/openvswitch-net.xml @@ -21,4 +21,13 @@ <parameters profileid='alice-profile'/> </virtualport> </portgroup> + <portgroup name='native'> + <vlan trunk='yes'> + <tag id='123' nativeMode='tagged'/> + <tag id='444'/> + </vlan> + <virtualport> + <parameters profileid='native-profile'/> + </virtualport> + </portgroup> </network>
ACK. I've made the small changes I indicated above and pushed. Since we're heading into freeze for v1.0.7 now, it would be a really good idea for you to pull the latest from git, built it, and make sure it still works as you intended - there's still about a week to fix any bugs that may have crept in. Thanks for your contribution!
participants (3)
-
james robson -
John Ferlan -
Laine Stump