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

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



Hello,
Has any one been able to review this yet? I realise that the 'Since
1.0.3' in the doc page is now out of date, but is the code itself
acceptable?


On Mon, 2013-02-18 at 11:01 +0000, 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
> 
> ---
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index ffcc33e..a5054cc 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -3209,6 +3209,12 @@ qemu-kvm -net nic,model=? /dev/null
>          <parameters
> interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/>
>        </virtualport>
>      </interface>
> +    <interface type='bridge'>
> +      <b>&lt;vlan trunk='yes' native_mode='tagged'
> native_tag='123'&gt;</b>
> +        <b>&lt;tag id='42'/&gt;</b>
> +      <b>&lt;/vlan&gt;</b>
> +    ...
> +  &lt;/interface&gt;
>    &lt;devices&gt;
>    ...</pre>
>  
> @@ -3234,7 +3240,17 @@ qemu-kvm -net nic,model=? /dev/null
>        attribute <code>trunk='yes'</code> can be added to the toplevel
>        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.3).</span> This uses the optional
> +      <code>native_mode</code> and <code>native_tag</code> attributes
> +      on the <code>&lt;vlan&gt;</code> element:
> <code>native_mode</code>
> +      may be set to 'tagged' or 'untagged', <code>native_tag</code>
> +      sets the id of the native vlan. Setting a native vlan implies
> +      this is a trunk port, so <code>trunk='yes'</code> will be added
> if not
> +      explicitly set.
> +    </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 7b42529..68c562b 100644
> --- a/docs/formatnetwork.html.in
> +++ b/docs/formatnetwork.html.in
> @@ -396,6 +396,12 @@
>          &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' native_mode='tagged'
> native_tag='123'&gt;</b>
> +        <b>&lt;tag id='42'/&gt;</b>
> +      <b>&lt;/vlan&gt;</b>
> +      ...
> +    &lt;/interface&gt;
>    &lt;devices&gt;
>    ...</pre>
>  
> diff --git a/docs/schemas/networkcommon.rng
> b/docs/schemas/networkcommon.rng
> index 51ff759..4696f43 100644
> --- a/docs/schemas/networkcommon.rng
> +++ b/docs/schemas/networkcommon.rng
> @@ -197,6 +197,21 @@
>            <value>yes</value>
>          </attribute>
>        </optional>
> +      <optional>
> +        <attribute name="native_mode">
> +          <choice>
> +            <value>tagged</value>
> +            <value>untagged</value>
> +          </choice>
> +        </attribute>
> +      </optional>
> +      <optional>
> +        <attribute name="native_tag">
> +          <data type="unsignedInt">
> +            <param name="maxInclusive">4095</param>
> +          </data>
> +        </attribute>
> +      </optional>
>        <oneOrMore>
>          <element name="tag">
>            <attribute name="id">
> diff --git a/src/conf/netdev_vlan_conf.c b/src/conf/netdev_vlan_conf.c
> index 13ba8c6..618eb4c 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>
> @@ -33,6 +34,8 @@ virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr
> ctxt, virNetDevVlanPtr de
>      int ret = -1;
>      xmlNodePtr save = ctxt->node;
>      const char *trunk = NULL;
> +    const char *nativeMode;
> +    unsigned int nativeTag;
>      xmlNodePtr *tagNodes = NULL;
>      int nTags, ii;
>  
> @@ -73,16 +76,44 @@ virNetDevVlanParse(xmlNodePtr node,
> xmlXPathContextPtr ctxt, virNetDevVlanPtr de
>  
>      def->nTags = nTags;
>  
> -    /* now that we know how many tags there are, look for an explicit
> -     * trunk setting.
> -     */
> -    if (nTags > 1)
> -        def->trunk = true;
> +    def->nativeMode = 0;
> +    def->nativeTag = 0;
>  
>      ctxt->node = node;
> +    if ((nativeMode = virXPathString("string(./@native_mode)", ctxt)) !
> = NULL) {
> +        if (STRCASENEQ(nativeMode, "tagged") && STRCASENEQ(nativeMode,
> "untagged")) {
> +            virReportError(VIR_ERR_XML_ERROR,
> +                           _("invalid \"native_mode='%s'\" in <vlan> -
> "
> +                             "native_mode must be 'tagged' or
> 'untagged'"), nativeMode);
> +            goto error;
> +        }
> +        if (virXPathUInt("string(./@native_tag)", ctxt, &nativeTag) <
> 0) {
> +            virReportError(VIR_ERR_XML_ERROR, "%s",
> +                           _("missing or invalid native_tag
> attribute"));
> +            goto error;
> +        }
> +        if (nativeTag > 4095) {
> +            virReportError(VIR_ERR_XML_ERROR,
> +                           _("native_tag %u too large (maximum 4095)"),
> nativeTag);
> +            goto error;
> +        }
> +        def->nativeMode = STRCASEEQ(nativeMode, "tagged") ? 1 : 2;
> +        def->nativeTag = nativeTag;
> +    }
> +
> +    /* def->trunk will be set to true if:
> +     * "trunk='yes'" is set in xml
> +     * a native-* vlan mode has been set
> +     * >1 tag has been set */
>      if ((trunk = virXPathString("string(./@trunk)", ctxt)) != NULL) {
>          def->trunk = STRCASEEQ(trunk, "yes");
>          if (!def->trunk) {
> +            if (def->nativeMode > 0) {
> +                virReportError(VIR_ERR_XML_ERROR, "%s",
> +                               _("invalid configuration in <vlan> -
> \"trunk='no'\" is "
> +                                 "not allowed with native_mode"));
> +                goto error;
> +            }
>              if (nTags > 1) {
>                  virReportError(VIR_ERR_XML_ERROR,
>                                 _("invalid \"trunk='%s'\" in <vlan> -
> trunk='yes' "
> @@ -97,6 +128,8 @@ virNetDevVlanParse(xmlNodePtr node,
> xmlXPathContextPtr ctxt, virNetDevVlanPtr de
>                  goto error;
>              }
>          }
> +    } else if (nTags > 1 || def->nativeMode > 0) {
> +        def->trunk = true;
>      }
>  
>      ret = 0;
> @@ -122,8 +155,11 @@ virNetDevVlanFormat(virNetDevVlanPtr def,
> virBufferPtr buf)
>                         _("missing vlan tag data"));
>          return -1;
>      }
> -
> -    virBufferAsprintf(buf, "<vlan%s>\n", def->trunk ? " trunk='yes'" :
> "");
> +    if (def->nativeMode == 0) {
> +        virBufferAsprintf(buf, "<vlan%s>\n", def->trunk ? "
> trunk='yes'" : "");
> +    } else {
> +        virBufferAsprintf(buf, "<vlan trunk='yes' native_mode='%s'
> native_tag='%u'>\n", def->nativeMode == 1 ? "tagged" : "untagged",
> def->nativeTag);
> +    }
>      for (ii = 0; ii < def->nTags; ii++) {
>          virBufferAsprintf(buf, "  <tag id='%u'/>\n", def->tag[ii]);
>      }
> diff --git a/src/util/virnetdevopenvswitch.c
> b/src/util/virnetdevopenvswitch.c
> index 4fe077a..87122b0 100644
> --- a/src/util/virnetdevopenvswitch.c
> +++ b/src/util/virnetdevopenvswitch.c
> @@ -19,6 +19,7 @@
>   *     Dan Wendlandt <dan nicira com>
>   *     Kyle Mestery <kmestery cisco com>
>   *     Ansis Atteka <aatteka nicira com>
> + *     James Robson <jrobson websense com>
>   */
>  
>  #include <config.h>
> @@ -108,9 +109,13 @@ 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) {
> +        if (virtVlan->nativeMode > 0) {
> +            virCommandAddArgFormat(cmd, "vlan_mode=%s",
> virtVlan->nativeMode == 1 ? "native-tagged" : "native-untagged");
> +            virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag);
> +            }
>          virCommandAddArgList(cmd, virBufferCurrentContent(&buf), NULL);
> -
> +    }
>      if (ovsport->profileID[0] == '\0') {
>          virCommandAddArgList(cmd,
>                          "--", "set", "Interface", ifname,
> attachedmac_ex_id,
> diff --git a/src/util/virnetdevvlan.c b/src/util/virnetdevvlan.c
> index 2fe2017..298673d 100644
> --- a/src/util/virnetdevvlan.c
> +++ b/src/util/virnetdevvlan.c
> @@ -17,6 +17,7 @@
>   *
>   * Authors:
>   *      Laine Stump <laine redhat com>
> + *      James Robson <jrobson websense com>
>   */
>  
>  #include <config.h>
> @@ -33,6 +34,8 @@ virNetDevVlanClear(virNetDevVlanPtr vlan)
>  {
>      VIR_FREE(vlan->tag);
>      vlan->nTags = 0;
> +    vlan->nativeMode = 0;
> +    vlan->nativeTag = -1;
>  }
>  
>  void
> @@ -54,7 +57,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 +94,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..f0f78f0 100644
> --- a/src/util/virnetdevvlan.h
> +++ b/src/util/virnetdevvlan.h
> @@ -17,6 +17,7 @@
>   *
>   * Authors:
>   *      Laine Stump <laine redhat com>
> + *      James Robson <jrobson websense com>
>   */
>  
>  #ifndef __VIR_NETDEV_VLAN_H__
> @@ -25,6 +26,8 @@
>  typedef struct _virNetDevVlan virNetDevVlan;
>  typedef virNetDevVlan *virNetDevVlanPtr;
>  struct _virNetDevVlan {
> +    short int nativeMode; /* 0=off, 1=tagged, 2=untagged */
> +    unsigned int nativeTag;
>      bool trunk;        /* true if this is a trunk */
>      int nTags;          /* number of tags in array */
>      unsigned int *tag; /* pointer to array of tags */
> diff --git a/tests/networkxml2xmlin/openvswitch-net.xml
> b/tests/networkxml2xmlin/openvswitch-net.xml
> index a3d82b1..93c49d5 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='tagged'>
> +    <vlan native_mode='tagged' native_tag='123'>
> +      <tag id='555'/>
> +      <tag id='444'/>
> +    </vlan>
> +    <virtualport>
> +      <parameters profileid='tagged-profile'/>
> +    </virtualport>
> +  </portgroup>
>  </network>
> diff --git a/tests/networkxml2xmlout/openvswitch-net.xml
> b/tests/networkxml2xmlout/openvswitch-net.xml
> index a3d82b1..ab3d797 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='tagged'>
> +    <vlan trunk='yes' native_mode='tagged' native_tag='123'>
> +      <tag id='555'/>
> +      <tag id='444'/>
> +    </vlan>
> +    <virtualport>
> +      <parameters profileid='tagged-profile'/>
> +    </virtualport>
> +  </portgroup>
>  </network>
> 




 Protected by Websense Hosted Email Security -- www.websense.com 


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