Re: [PATCHv4] Configure native vlan modes on Open vSwitch ports

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

 

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>&lt;vlan trunk='yes'&gt;</b>
>> +        <b>&lt;tag id='42'/&gt;</b>
>> +        <b>&lt;tag id='123' nativeMode='untagged'/&gt;</b>
>> +      <b>&lt;/vlan&gt;</b>
>> +      ...
>> +    &lt;/interface&gt;
>>    &lt;devices&gt;
>>    ...</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>&lt;tag&gt;</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 @@
>>          &lt;parameters interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/&gt;
>>        &lt;/virtualport&gt;
>>      &lt;/interface&gt;
>> +    &lt;interface type='bridge'&gt;
>> +      <b>&lt;vlan trunk='yes'&gt;</b>
>> +        <b>&lt;tag id='42'/&gt;</b>
>> +        <b>&lt;tag id='123' nativeMode='untagged'/&gt;</b>
>> +      <b>&lt;/vlan&gt;</b>
>> +      ...
>> +    &lt;/interface&gt;
>>    &lt;devices&gt;
>>    ...</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>&lt;tag&gt;</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>&lt;vlan&gt;</code> elements can also be specified in
>>        a <code>&lt;portgroup&gt;</code> element, as well as directly in
>>        a domain's <code>&lt;interface&gt;</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@xxxxxxxxxx>
>> + *     James Robson <jrobson@xxxxxxxxxxxx>
>>   */
>>  
>>  #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@xxxxxxxxxx>
>>   */
>> -
>> +# 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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
>

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]