openvswitch: Add new port VLAN mode "802.1ad"

[libvirt] [PATCH v5] openvswitch: Add new port VLAN mode "802.1ad"

Posted by ZhiPeng LU 5 years, 2 months ago

This patch adds functionality to allow libvirt to configure the '802.1ad'
modes(802.1ad double-tagged) on openvswitch networks.
For example:
  <interface type='bridge'>
    <mac address='2c:da:41:1d:05:42'/>
    <source bridge='ovs0'/>
    <vlan>
      <tag id='41' nativeMode='dot1q-tunnel'/>
    </vlan>
    <virtualport type='openvswitch'>
      <parameters interfaceid='6401a152-0b99-40b5-92be-858810aa6d37'/>
    </virtualport>
    <model type='virtio'/>
    <driver name='vhost'/>
    <alias name='net0'/>
  </interface>

Signed-off-by: ZhiPeng Lu <luzhipeng@uniudc.com>
---
v1->v2:
  1. Fix "make syntax-check" failure
v2->v3:
  1. remove other_config when updating vlan
v3->v4:
  1. add commit message that has a brief description of the new
      feature
  2. add tests for 'dot1q-tunnel' vlan mode
v4->v5:
  1. modify some description and format

v4-resend:
  https://www.redhat.com/archives/libvir-list/2019-February/msg00988.html

 docs/formatdomain.html.in                          | 33 +++++++++++++++-------
 docs/formatnetwork.html.in                         | 26 ++++++++++-------
 docs/schemas/networkcommon.rng                     |  1 +
 src/conf/netdev_vlan_conf.c                        |  2 +-
 src/util/virnetdevopenvswitch.c                    |  7 +++++
 src/util/virnetdevvlan.h                           |  1 +
 tests/networkxml2xmlin/openvswitch-net.xml         |  9 ++++++
 tests/networkxml2xmlout/openvswitch-net.xml        |  9 ++++++
 .../openvswitch-net-modified.xml                   |  9 ++++++
 .../openvswitch-net-more-portgroups.xml            |  9 ++++++
 .../openvswitch-net-without-alice.xml              |  9 ++++++
 11 files changed, 94 insertions(+), 21 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index b848e53..253f329 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6097,6 +6097,13 @@ qemu-kvm -net nic,model=? /dev/null
     <b>&lt;/vlan&gt;</b>
     ...
   &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='555' nativeMode='802.1ad'/&gt;</b>
+    <b>&lt;/vlan&gt;</b>
+    ...
+  &lt;/interface&gt;
 &lt;/devices&gt;
 ...</pre>
 
@@ -6132,16 +6139,22 @@ qemu-kvm -net nic,model=? /dev/null
     </p>
     <p>
       For network connections using Open vSwitch it is also possible
-      to configure 'native-tagged' and 'native-untagged' VLAN modes
-      <span class="since">Since 1.1.0.</span> This is done with the
-      optional <code>nativeMode</code> attribute on
-      the <code>&lt;tag&gt;</code> subelement: <code>nativeMode</code>
-      may be set to 'tagged' or 'untagged'. The <code>id</code>
-      attribute of the <code>&lt;tag&gt;</code> subelement
-      containing <code>nativeMode</code> sets which VLAN is considered
-      to be the "native" VLAN for this interface, and
-      the <code>nativeMode</code> attribute determines whether or not
-      traffic for that VLAN will be tagged.
+      to configure the following VLAN modes:
+    </p>
+      <ul>
+        <li>'tagged' <span class="since">Since 1.1.0.</span></li>
+        <li>'untagged' <span class="since">Since 1.1.0.</span></li>
+        <li>'802.1ad' <span class="since">Since 5.1.0.</span></li>
+      </ul>
+    <p>
+      This is done with the optional <code>nativeMode</code> attribute
+      on the <code>&lt;tag&gt;</code> subelement. The attribute may be
+      set to a string from the above list. The <code>id</code> attribute
+      of the <code>&lt;tag&gt;</code> subelement containing
+      <code>nativeMode</code> sets which VLAN is considered to be the
+      "native" VLAN for this interface and the <code>nativeMode</code>
+      attribute determines whether or not traffic for that VLAN will be
+      tagged or 802.1ad double tagged.
     </p>
 
     <h5><a id="elementLink">Modifying virtual link state</a></h5>
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index 509cca9..eea86f4 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -705,16 +705,22 @@
     </p>
     <p>
       For network connections using Open vSwitch it is also possible
-      to configure 'native-tagged' and 'native-untagged' VLAN modes
-      <span class="since">Since 1.1.0.</span> This is done with the
-      optional <code>nativeMode</code> attribute on
-      the <code>&lt;tag&gt;</code> subelement: <code>nativeMode</code>
-      may be set to 'tagged' or 'untagged'. The <code>id</code>
-      attribute of the <code>&lt;tag&gt;</code> subelement
-      containing <code>nativeMode</code> sets which VLAN is considered
-      to be the "native" VLAN for this interface, and
-      the <code>nativeMode</code> attribute determines whether or not
-      traffic for that VLAN will be tagged.
+      to configure the following VLAN modes:
+    </p>
+      <ul>
+        <li>'tagged' <span class="since">Since 1.1.0.</span></li>
+        <li>'untagged' <span class="since">Since 1.1.0.</span></li>
+        <li>'802.1ad' <span class="since">Since 5.1.0.</span></li>
+      </ul>
+    <p>
+      This is done with the optional <code>nativeMode</code> attribute
+      on the <code>&lt;tag&gt;</code> subelement. The attribute may be
+      set to a string from the above list. The <code>id</code> attribute
+      of the <code>&lt;tag&gt;</code> subelement containing
+      <code>nativeMode</code> sets which VLAN is considered to be the
+      "native" VLAN for this interface and the <code>nativeMode</code>
+      attribute determines whether or not traffic for that VLAN will be
+      tagged or 802.1ad double tagged.
     </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 2699555..7262783 100644
--- a/docs/schemas/networkcommon.rng
+++ b/docs/schemas/networkcommon.rng
@@ -223,6 +223,7 @@
               <choice>
                 <value>tagged</value>
                 <value>untagged</value>
+                <value>802.1ad</value>
               </choice>
             </attribute>
           </optional>
diff --git a/src/conf/netdev_vlan_conf.c b/src/conf/netdev_vlan_conf.c
index 57d73ed..54e2b02 100644
--- a/src/conf/netdev_vlan_conf.c
+++ b/src/conf/netdev_vlan_conf.c
@@ -25,7 +25,7 @@
 #define VIR_FROM_THIS VIR_FROM_NONE
 
 VIR_ENUM_IMPL(virNativeVlanMode, VIR_NATIVE_VLAN_MODE_LAST,
-              "default", "tagged", "untagged",
+              "default", "tagged", "untagged", "802.1ad",
 );
 
 int
diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index 4fa3a57..d8268bc 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -85,6 +85,11 @@ virNetDevOpenvswitchConstructVlans(virCommandPtr cmd, virNetDevVlanPtr virtVlan)
         virCommandAddArg(cmd, "vlan_mode=native-untagged");
         virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag);
         break;
+    case VIR_NATIVE_VLAN_MODE_8021AD:
+        virCommandAddArg(cmd, "vlan_mode=dot1q-tunnel");
+        virCommandAddArg(cmd, "other_config:qinq-ethtype=802.1q");
+        virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag);
+        break;
     case VIR_NATIVE_VLAN_MODE_DEFAULT:
     default:
         break;
@@ -498,6 +503,8 @@ int virNetDevOpenvswitchUpdateVlan(const char *ifname,
                          "--", "--if-exists", "clear", "Port", ifname, "tag",
                          "--", "--if-exists", "clear", "Port", ifname, "trunk",
                          "--", "--if-exists", "clear", "Port", ifname, "vlan_mode",
+                         "--", "--if-exists", "remove", "Port", ifname,
+                         "other_config", "qinq-ethtype",
                          "--", "--if-exists", "set", "Port", ifname, NULL);
 
     if (virNetDevOpenvswitchConstructVlans(cmd, virtVlan) < 0)
diff --git a/src/util/virnetdevvlan.h b/src/util/virnetdevvlan.h
index 2a13759..54109c6 100644
--- a/src/util/virnetdevvlan.h
+++ b/src/util/virnetdevvlan.h
@@ -27,6 +27,7 @@ typedef enum {
     VIR_NATIVE_VLAN_MODE_DEFAULT = 0,
     VIR_NATIVE_VLAN_MODE_TAGGED,
     VIR_NATIVE_VLAN_MODE_UNTAGGED,
+    VIR_NATIVE_VLAN_MODE_8021AD,
 
     VIR_NATIVE_VLAN_MODE_LAST
 } virNativeVlanMode;
diff --git a/tests/networkxml2xmlin/openvswitch-net.xml b/tests/networkxml2xmlin/openvswitch-net.xml
index 2f6084d..5734d36 100644
--- a/tests/networkxml2xmlin/openvswitch-net.xml
+++ b/tests/networkxml2xmlin/openvswitch-net.xml
@@ -30,4 +30,13 @@
       <parameters profileid='native-profile'/>
     </virtualport>
   </portgroup>
+  <portgroup name='8021ad'>
+    <vlan trunk='yes'>
+      <tag id='555' nativeMode='802.1ad'/>
+      <tag id='666'/>
+    </vlan>
+    <virtualport>
+      <parameters profileid='8021ad-profile'/>
+    </virtualport>
+  </portgroup>
 </network>
diff --git a/tests/networkxml2xmlout/openvswitch-net.xml b/tests/networkxml2xmlout/openvswitch-net.xml
index 2f6084d..5734d36 100644
--- a/tests/networkxml2xmlout/openvswitch-net.xml
+++ b/tests/networkxml2xmlout/openvswitch-net.xml
@@ -30,4 +30,13 @@
       <parameters profileid='native-profile'/>
     </virtualport>
   </portgroup>
+  <portgroup name='8021ad'>
+    <vlan trunk='yes'>
+      <tag id='555' nativeMode='802.1ad'/>
+      <tag id='666'/>
+    </vlan>
+    <virtualport>
+      <parameters profileid='8021ad-profile'/>
+    </virtualport>
+  </portgroup>
 </network>
diff --git a/tests/networkxml2xmlupdateout/openvswitch-net-modified.xml b/tests/networkxml2xmlupdateout/openvswitch-net-modified.xml
index cc0c344..ed605bf 100644
--- a/tests/networkxml2xmlupdateout/openvswitch-net-modified.xml
+++ b/tests/networkxml2xmlupdateout/openvswitch-net-modified.xml
@@ -30,4 +30,13 @@
       <parameters profileid='native-profile'/>
     </virtualport>
   </portgroup>
+  <portgroup name='8021ad'>
+    <vlan trunk='yes'>
+      <tag id='555' nativeMode='802.1ad'/>
+      <tag id='666'/>
+    </vlan>
+    <virtualport>
+      <parameters profileid='8021ad-profile'/>
+    </virtualport>
+  </portgroup>
 </network>
diff --git a/tests/networkxml2xmlupdateout/openvswitch-net-more-portgroups.xml b/tests/networkxml2xmlupdateout/openvswitch-net-more-portgroups.xml
index 7c19ad9..0237e42 100644
--- a/tests/networkxml2xmlupdateout/openvswitch-net-more-portgroups.xml
+++ b/tests/networkxml2xmlupdateout/openvswitch-net-more-portgroups.xml
@@ -41,4 +41,13 @@
       <parameters profileid='native-profile'/>
     </virtualport>
   </portgroup>
+  <portgroup name='8021ad'>
+    <vlan trunk='yes'>
+      <tag id='555' nativeMode='802.1ad'/>
+      <tag id='666'/>
+    </vlan>
+    <virtualport>
+      <parameters profileid='8021ad-profile'/>
+    </virtualport>
+  </portgroup>
 </network>
diff --git a/tests/networkxml2xmlupdateout/openvswitch-net-without-alice.xml b/tests/networkxml2xmlupdateout/openvswitch-net-without-alice.xml
index 4104424..cb9d12d 100644
--- a/tests/networkxml2xmlupdateout/openvswitch-net-without-alice.xml
+++ b/tests/networkxml2xmlupdateout/openvswitch-net-without-alice.xml
@@ -20,4 +20,13 @@
       <parameters profileid='native-profile'/>
     </virtualport>
   </portgroup>
+  <portgroup name='8021ad'>
+    <vlan trunk='yes'>
+      <tag id='555' nativeMode='802.1ad'/>
+      <tag id='666'/>
+    </vlan>
+    <virtualport>
+      <parameters profileid='8021ad-profile'/>
+    </virtualport>
+  </portgroup>
 </network>
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v5] openvswitch: Add new port VLAN mode "802.1ad"

Posted by John Ferlan 5 years, 1 month ago


On 2/24/19 9:15 AM, ZhiPeng LU wrote:
> This patch adds functionality to allow libvirt to configure the '802.1ad'
> modes(802.1ad double-tagged) on openvswitch networks.
> For example:
>   <interface type='bridge'>
>     <mac address='2c:da:41:1d:05:42'/>
>     <source bridge='ovs0'/>
>     <vlan>
>       <tag id='41' nativeMode='dot1q-tunnel'/>
>     </vlan>
>     <virtualport type='openvswitch'>
>       <parameters interfaceid='6401a152-0b99-40b5-92be-858810aa6d37'/>
>     </virtualport>
>     <model type='virtio'/>
>     <driver name='vhost'/>
>     <alias name='net0'/>
>   </interface>
> 
> Signed-off-by: ZhiPeng Lu <luzhipeng@uniudc.com>
> ---
> v1->v2:
>   1. Fix "make syntax-check" failure
> v2->v3:
>   1. remove other_config when updating vlan
> v3->v4:
>   1. add commit message that has a brief description of the new
>       feature
>   2. add tests for 'dot1q-tunnel' vlan mode
> v4->v5:
>   1. modify some description and format
> 
> v4-resend:
>   https://www.redhat.com/archives/libvir-list/2019-February/msg00988.html
> 
>  docs/formatdomain.html.in                          | 33 +++++++++++++++-------
>  docs/formatnetwork.html.in                         | 26 ++++++++++-------
>  docs/schemas/networkcommon.rng                     |  1 +
>  src/conf/netdev_vlan_conf.c                        |  2 +-
>  src/util/virnetdevopenvswitch.c                    |  7 +++++
>  src/util/virnetdevvlan.h                           |  1 +
>  tests/networkxml2xmlin/openvswitch-net.xml         |  9 ++++++
>  tests/networkxml2xmlout/openvswitch-net.xml        |  9 ++++++
>  .../openvswitch-net-modified.xml                   |  9 ++++++
>  .../openvswitch-net-more-portgroups.xml            |  9 ++++++
>  .../openvswitch-net-without-alice.xml              |  9 ++++++
>  11 files changed, 94 insertions(+), 21 deletions(-)
> 

Apart from now needing to indicate support in 5.2.0 for the
format*.html.in files and the need for a docs/news.xml note this seems
fine to me and covers what Laine had originally reviewed. I can modify
those two before pushing.

Also, I've CC'd Laine in hopes he can also take a look for sanity's sake
to ensure I didn't misinterpret something he requested previously!

I'll also add a followup patch to update docs/news.xml with the
following text:

+      <change>
+        <summary>
+          Add support for "802.1ad" VLAN mode
+        </summary>
+        <description>
+          Add support for the 802.ad double-tagged modes on openvswitch
+          networks.
+        </description>
+      </change>

Please let me know if you believe that's enough wording or would like
any modifications...

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index b848e53..253f329 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -6097,6 +6097,13 @@ qemu-kvm -net nic,model=? /dev/null
>      <b>&lt;/vlan&gt;</b>
>      ...
>    &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='555' nativeMode='802.1ad'/&gt;</b>
> +    <b>&lt;/vlan&gt;</b>
> +    ...
> +  &lt;/interface&gt;
>  &lt;/devices&gt;
>  ...</pre>
>  
> @@ -6132,16 +6139,22 @@ qemu-kvm -net nic,model=? /dev/null
>      </p>
>      <p>
>        For network connections using Open vSwitch it is also possible
> -      to configure 'native-tagged' and 'native-untagged' VLAN modes
> -      <span class="since">Since 1.1.0.</span> This is done with the
> -      optional <code>nativeMode</code> attribute on
> -      the <code>&lt;tag&gt;</code> subelement: <code>nativeMode</code>
> -      may be set to 'tagged' or 'untagged'. The <code>id</code>
> -      attribute of the <code>&lt;tag&gt;</code> subelement
> -      containing <code>nativeMode</code> sets which VLAN is considered
> -      to be the "native" VLAN for this interface, and
> -      the <code>nativeMode</code> attribute determines whether or not
> -      traffic for that VLAN will be tagged.
> +      to configure the following VLAN modes:
> +    </p>
> +      <ul>
> +        <li>'tagged' <span class="since">Since 1.1.0.</span></li>
> +        <li>'untagged' <span class="since">Since 1.1.0.</span></li>
> +        <li>'802.1ad' <span class="since">Since 5.1.0.</span></li>
> +      </ul>
> +    <p>
> +      This is done with the optional <code>nativeMode</code> attribute
> +      on the <code>&lt;tag&gt;</code> subelement. The attribute may be
> +      set to a string from the above list. The <code>id</code> attribute
> +      of the <code>&lt;tag&gt;</code> subelement containing
> +      <code>nativeMode</code> sets which VLAN is considered to be the
> +      "native" VLAN for this interface and the <code>nativeMode</code>
> +      attribute determines whether or not traffic for that VLAN will be
> +      tagged or 802.1ad double tagged.
>      </p>
>  
>      <h5><a id="elementLink">Modifying virtual link state</a></h5>
> diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
> index 509cca9..eea86f4 100644
> --- a/docs/formatnetwork.html.in
> +++ b/docs/formatnetwork.html.in
> @@ -705,16 +705,22 @@
>      </p>
>      <p>
>        For network connections using Open vSwitch it is also possible
> -      to configure 'native-tagged' and 'native-untagged' VLAN modes
> -      <span class="since">Since 1.1.0.</span> This is done with the
> -      optional <code>nativeMode</code> attribute on
> -      the <code>&lt;tag&gt;</code> subelement: <code>nativeMode</code>
> -      may be set to 'tagged' or 'untagged'. The <code>id</code>
> -      attribute of the <code>&lt;tag&gt;</code> subelement
> -      containing <code>nativeMode</code> sets which VLAN is considered
> -      to be the "native" VLAN for this interface, and
> -      the <code>nativeMode</code> attribute determines whether or not
> -      traffic for that VLAN will be tagged.
> +      to configure the following VLAN modes:
> +    </p>
> +      <ul>
> +        <li>'tagged' <span class="since">Since 1.1.0.</span></li>
> +        <li>'untagged' <span class="since">Since 1.1.0.</span></li>
> +        <li>'802.1ad' <span class="since">Since 5.1.0.</span></li>
> +      </ul>
> +    <p>
> +      This is done with the optional <code>nativeMode</code> attribute
> +      on the <code>&lt;tag&gt;</code> subelement. The attribute may be
> +      set to a string from the above list. The <code>id</code> attribute
> +      of the <code>&lt;tag&gt;</code> subelement containing
> +      <code>nativeMode</code> sets which VLAN is considered to be the
> +      "native" VLAN for this interface and the <code>nativeMode</code>
> +      attribute determines whether or not traffic for that VLAN will be
> +      tagged or 802.1ad double tagged.
>      </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 2699555..7262783 100644
> --- a/docs/schemas/networkcommon.rng
> +++ b/docs/schemas/networkcommon.rng
> @@ -223,6 +223,7 @@
>                <choice>
>                  <value>tagged</value>
>                  <value>untagged</value>
> +                <value>802.1ad</value>
>                </choice>
>              </attribute>
>            </optional>
> diff --git a/src/conf/netdev_vlan_conf.c b/src/conf/netdev_vlan_conf.c
> index 57d73ed..54e2b02 100644
> --- a/src/conf/netdev_vlan_conf.c
> +++ b/src/conf/netdev_vlan_conf.c
> @@ -25,7 +25,7 @@
>  #define VIR_FROM_THIS VIR_FROM_NONE
>  
>  VIR_ENUM_IMPL(virNativeVlanMode, VIR_NATIVE_VLAN_MODE_LAST,
> -              "default", "tagged", "untagged",
> +              "default", "tagged", "untagged", "802.1ad",
>  );
>  
>  int
> diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
> index 4fa3a57..d8268bc 100644
> --- a/src/util/virnetdevopenvswitch.c
> +++ b/src/util/virnetdevopenvswitch.c
> @@ -85,6 +85,11 @@ virNetDevOpenvswitchConstructVlans(virCommandPtr cmd, virNetDevVlanPtr virtVlan)
>          virCommandAddArg(cmd, "vlan_mode=native-untagged");
>          virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag);
>          break;
> +    case VIR_NATIVE_VLAN_MODE_8021AD:
> +        virCommandAddArg(cmd, "vlan_mode=dot1q-tunnel");
> +        virCommandAddArg(cmd, "other_config:qinq-ethtype=802.1q");
> +        virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag);
> +        break;
>      case VIR_NATIVE_VLAN_MODE_DEFAULT:
>      default:
>          break;
> @@ -498,6 +503,8 @@ int virNetDevOpenvswitchUpdateVlan(const char *ifname,
>                           "--", "--if-exists", "clear", "Port", ifname, "tag",
>                           "--", "--if-exists", "clear", "Port", ifname, "trunk",
>                           "--", "--if-exists", "clear", "Port", ifname, "vlan_mode",
> +                         "--", "--if-exists", "remove", "Port", ifname,
> +                         "other_config", "qinq-ethtype",
>                           "--", "--if-exists", "set", "Port", ifname, NULL);
>  
>      if (virNetDevOpenvswitchConstructVlans(cmd, virtVlan) < 0)
> diff --git a/src/util/virnetdevvlan.h b/src/util/virnetdevvlan.h
> index 2a13759..54109c6 100644
> --- a/src/util/virnetdevvlan.h
> +++ b/src/util/virnetdevvlan.h
> @@ -27,6 +27,7 @@ typedef enum {
>      VIR_NATIVE_VLAN_MODE_DEFAULT = 0,
>      VIR_NATIVE_VLAN_MODE_TAGGED,
>      VIR_NATIVE_VLAN_MODE_UNTAGGED,
> +    VIR_NATIVE_VLAN_MODE_8021AD,
>  
>      VIR_NATIVE_VLAN_MODE_LAST
>  } virNativeVlanMode;
> diff --git a/tests/networkxml2xmlin/openvswitch-net.xml b/tests/networkxml2xmlin/openvswitch-net.xml
> index 2f6084d..5734d36 100644
> --- a/tests/networkxml2xmlin/openvswitch-net.xml
> +++ b/tests/networkxml2xmlin/openvswitch-net.xml
> @@ -30,4 +30,13 @@
>        <parameters profileid='native-profile'/>
>      </virtualport>
>    </portgroup>
> +  <portgroup name='8021ad'>
> +    <vlan trunk='yes'>
> +      <tag id='555' nativeMode='802.1ad'/>
> +      <tag id='666'/>
> +    </vlan>
> +    <virtualport>
> +      <parameters profileid='8021ad-profile'/>
> +    </virtualport>
> +  </portgroup>
>  </network>
> diff --git a/tests/networkxml2xmlout/openvswitch-net.xml b/tests/networkxml2xmlout/openvswitch-net.xml
> index 2f6084d..5734d36 100644
> --- a/tests/networkxml2xmlout/openvswitch-net.xml
> +++ b/tests/networkxml2xmlout/openvswitch-net.xml
> @@ -30,4 +30,13 @@
>        <parameters profileid='native-profile'/>
>      </virtualport>
>    </portgroup>
> +  <portgroup name='8021ad'>
> +    <vlan trunk='yes'>
> +      <tag id='555' nativeMode='802.1ad'/>
> +      <tag id='666'/>
> +    </vlan>
> +    <virtualport>
> +      <parameters profileid='8021ad-profile'/>
> +    </virtualport>
> +  </portgroup>
>  </network>
> diff --git a/tests/networkxml2xmlupdateout/openvswitch-net-modified.xml b/tests/networkxml2xmlupdateout/openvswitch-net-modified.xml
> index cc0c344..ed605bf 100644
> --- a/tests/networkxml2xmlupdateout/openvswitch-net-modified.xml
> +++ b/tests/networkxml2xmlupdateout/openvswitch-net-modified.xml
> @@ -30,4 +30,13 @@
>        <parameters profileid='native-profile'/>
>      </virtualport>
>    </portgroup>
> +  <portgroup name='8021ad'>
> +    <vlan trunk='yes'>
> +      <tag id='555' nativeMode='802.1ad'/>
> +      <tag id='666'/>
> +    </vlan>
> +    <virtualport>
> +      <parameters profileid='8021ad-profile'/>
> +    </virtualport>
> +  </portgroup>
>  </network>
> diff --git a/tests/networkxml2xmlupdateout/openvswitch-net-more-portgroups.xml b/tests/networkxml2xmlupdateout/openvswitch-net-more-portgroups.xml
> index 7c19ad9..0237e42 100644
> --- a/tests/networkxml2xmlupdateout/openvswitch-net-more-portgroups.xml
> +++ b/tests/networkxml2xmlupdateout/openvswitch-net-more-portgroups.xml
> @@ -41,4 +41,13 @@
>        <parameters profileid='native-profile'/>
>      </virtualport>
>    </portgroup>
> +  <portgroup name='8021ad'>
> +    <vlan trunk='yes'>
> +      <tag id='555' nativeMode='802.1ad'/>
> +      <tag id='666'/>
> +    </vlan>
> +    <virtualport>
> +      <parameters profileid='8021ad-profile'/>
> +    </virtualport>
> +  </portgroup>
>  </network>
> diff --git a/tests/networkxml2xmlupdateout/openvswitch-net-without-alice.xml b/tests/networkxml2xmlupdateout/openvswitch-net-without-alice.xml
> index 4104424..cb9d12d 100644
> --- a/tests/networkxml2xmlupdateout/openvswitch-net-without-alice.xml
> +++ b/tests/networkxml2xmlupdateout/openvswitch-net-without-alice.xml
> @@ -20,4 +20,13 @@
>        <parameters profileid='native-profile'/>
>      </virtualport>
>    </portgroup>
> +  <portgroup name='8021ad'>
> +    <vlan trunk='yes'>
> +      <tag id='555' nativeMode='802.1ad'/>
> +      <tag id='666'/>
> +    </vlan>
> +    <virtualport>
> +      <parameters profileid='8021ad-profile'/>
> +    </virtualport>
> +  </portgroup>
>  </network>
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v5] openvswitch: Add new port VLAN mode "802.1ad"

Posted by Laine Stump 5 years, 1 month ago

On 3/8/19 8:34 AM, John Ferlan wrote:
>
> On 2/24/19 9:15 AM, ZhiPeng LU wrote:
>> This patch adds functionality to allow libvirt to configure the '802.1ad'
>> modes(802.1ad double-tagged) on openvswitch networks.
>> For example:
>>    <interface type='bridge'>
>>      <mac address='2c:da:41:1d:05:42'/>
>>      <source bridge='ovs0'/>
>>      <vlan>
>>        <tag id='41' nativeMode='dot1q-tunnel'/>
>>      </vlan>
>>      <virtualport type='openvswitch'>
>>        <parameters interfaceid='6401a152-0b99-40b5-92be-858810aa6d37'/>
>>      </virtualport>
>>      <model type='virtio'/>
>>      <driver name='vhost'/>
>>      <alias name='net0'/>
>>    </interface>
>>
>> Signed-off-by: ZhiPeng Lu <luzhipeng@uniudc.com>
>> ---
>> v1->v2:
>>    1. Fix "make syntax-check" failure
>> v2->v3:
>>    1. remove other_config when updating vlan
>> v3->v4:
>>    1. add commit message that has a brief description of the new
>>        feature
>>    2. add tests for 'dot1q-tunnel' vlan mode
>> v4->v5:
>>    1. modify some description and format
>>
>> v4-resend:
>>    https://www.redhat.com/archives/libvir-list/2019-February/msg00988.html
>>
>>   docs/formatdomain.html.in                          | 33 +++++++++++++++-------
>>   docs/formatnetwork.html.in                         | 26 ++++++++++-------
>>   docs/schemas/networkcommon.rng                     |  1 +
>>   src/conf/netdev_vlan_conf.c                        |  2 +-
>>   src/util/virnetdevopenvswitch.c                    |  7 +++++
>>   src/util/virnetdevvlan.h                           |  1 +
>>   tests/networkxml2xmlin/openvswitch-net.xml         |  9 ++++++
>>   tests/networkxml2xmlout/openvswitch-net.xml        |  9 ++++++
>>   .../openvswitch-net-modified.xml                   |  9 ++++++
>>   .../openvswitch-net-more-portgroups.xml            |  9 ++++++
>>   .../openvswitch-net-without-alice.xml              |  9 ++++++
>>   11 files changed, 94 insertions(+), 21 deletions(-)
>>
> Apart from now needing to indicate support in 5.2.0 for the
> format*.html.in files and the need for a docs/news.xml note this seems
> fine to me and covers what Laine had originally reviewed. I can modify
> those two before pushing.
>
> Also, I've CC'd Laine in hopes he can also take a look for sanity's sake
> to ensure I didn't misinterpret something he requested previously!


Yeah, sorry I haven't responded to the last couple revisions of this 
patch. When I saw them I tagged them in red in my mail, but don't have 
an effective queuing mechanism and ended up getting lost in some other 
distraction and not getting back until the red message was scrolled way 
up out of sight :-/ Anyway, thanks to John for keeping track of it and 
reviewing it, and ZhiPeng Lu for being patient.



>
> I'll also add a followup patch to update docs/news.xml with the
> following text:
>
> +      <change>
> +        <summary>
> +          Add support for "802.1ad" VLAN mode


(You know, I expected someone to counter-propose use of a non-official 
term for this, since some places libvirt uses official names from the 
standards documents and other places it uses informal terms. Since there 
was no counter-proposal, I'm now unsure if that happened because 1) 
everyone agrees with using "802.1ad" (which is unambiguous but its 
function may be less obvious to a casual user), or 2) nobody even 
noticed :-P (but I still think using the official name is better, 
especially because it assures we won't end up with confusion if we later 
need to add some other sort of tunneled/nested tagging)


> +        </summary>
> +        <description>
> +          Add support for the 802.ad double-tagged modes on openvswitch


maybe "802.1ad double-tagged (tunneled) mode" ?? (I'm no expert with 
vlan terminology, but I think having "tunnel" in the description may 
help some people recognize the utility of the feature).


> +          networks.
> +        </description>
> +      </change>
>
> Please let me know if you believe that's enough wording or would like
> any modifications...
>
> Reviewed-by: John Ferlan <jferlan@redhat.com>
>
> John
>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index b848e53..253f329 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -6097,6 +6097,13 @@ qemu-kvm -net nic,model=? /dev/null
>>       <b>&lt;/vlan&gt;</b>
>>       ...
>>     &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='555' nativeMode='802.1ad'/&gt;</b>
>> +    <b>&lt;/vlan&gt;</b>
>> +    ...
>> +  &lt;/interface&gt;
>>   &lt;/devices&gt;
>>   ...</pre>
>>   
>> @@ -6132,16 +6139,22 @@ qemu-kvm -net nic,model=? /dev/null
>>       </p>
>>       <p>
>>         For network connections using Open vSwitch it is also possible
>> -      to configure 'native-tagged' and 'native-untagged' VLAN modes
>> -      <span class="since">Since 1.1.0.</span> This is done with the
>> -      optional <code>nativeMode</code> attribute on
>> -      the <code>&lt;tag&gt;</code> subelement: <code>nativeMode</code>
>> -      may be set to 'tagged' or 'untagged'. The <code>id</code>
>> -      attribute of the <code>&lt;tag&gt;</code> subelement
>> -      containing <code>nativeMode</code> sets which VLAN is considered
>> -      to be the "native" VLAN for this interface, and
>> -      the <code>nativeMode</code> attribute determines whether or not
>> -      traffic for that VLAN will be tagged.
>> +      to configure the following VLAN modes:
>> +    </p>
>> +      <ul>
>> +        <li>'tagged' <span class="since">Since 1.1.0.</span></li>
>> +        <li>'untagged' <span class="since">Since 1.1.0.</span></li>
>> +        <li>'802.1ad' <span class="since">Since 5.1.0.</span></li>
>> +      </ul>
>> +    <p>
>> +      This is done with the optional <code>nativeMode</code> attribute
>> +      on the <code>&lt;tag&gt;</code> subelement. The attribute may be
>> +      set to a string from the above list. The <code>id</code> attribute

   "set to a value" instead of "set to a string" ?? Not important but 
sounds better to my ear.


>> +      of the <code>&lt;tag&gt;</code> subelement containing
>> +      <code>nativeMode</code> sets which VLAN is considered to be the
>> +      "native" VLAN for this interface and the <code>nativeMode</code>
>> +      attribute determines whether or not traffic for that VLAN will be
>> +      tagged or 802.1ad double tagged.


Just to help me understand, can you explain what happens to packets in 
both directions when nativeMode="802.1ad" on a port? For example, for a 
packet coming *from* a guest and going onto the switch, is the packet 
tagged with the given id, which makes it singly tagged if it had no tag 
when coming from the guest, and double-tagged if it was previously 
tagged? And when a packet is going from the switch *to* the guest, is 
the configured tag removed? (and what if there is a packet that isn't 
tagged, or has a different tag?) Or do I have it backwards? Can you 
possibly make a simple text-picture showing what happens to packets 
(both those that were previously tagged and those that weren't) in each 
direction ?


(This isn't for adding to the documentation, but just for me to make 
sure I understand what's happening and that it's following the 
functional form of, for example, SRIOV interfaces' use of the <vlan> 
element).


Other than making sure that it fits functionally with other uses of 
<vlan>, I think it all looks fine.



>>       </p>
>>   
>>       <h5><a id="elementLink">Modifying virtual link state</a></h5>
>> diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
>> index 509cca9..eea86f4 100644
>> --- a/docs/formatnetwork.html.in
>> +++ b/docs/formatnetwork.html.in
>> @@ -705,16 +705,22 @@
>>       </p>
>>       <p>
>>         For network connections using Open vSwitch it is also possible
>> -      to configure 'native-tagged' and 'native-untagged' VLAN modes
>> -      <span class="since">Since 1.1.0.</span> This is done with the
>> -      optional <code>nativeMode</code> attribute on
>> -      the <code>&lt;tag&gt;</code> subelement: <code>nativeMode</code>
>> -      may be set to 'tagged' or 'untagged'. The <code>id</code>
>> -      attribute of the <code>&lt;tag&gt;</code> subelement
>> -      containing <code>nativeMode</code> sets which VLAN is considered
>> -      to be the "native" VLAN for this interface, and
>> -      the <code>nativeMode</code> attribute determines whether or not
>> -      traffic for that VLAN will be tagged.
>> +      to configure the following VLAN modes:
>> +    </p>
>> +      <ul>
>> +        <li>'tagged' <span class="since">Since 1.1.0.</span></li>
>> +        <li>'untagged' <span class="since">Since 1.1.0.</span></li>
>> +        <li>'802.1ad' <span class="since">Since 5.1.0.</span></li>
>> +      </ul>
>> +    <p>
>> +      This is done with the optional <code>nativeMode</code> attribute
>> +      on the <code>&lt;tag&gt;</code> subelement. The attribute may be
>> +      set to a string from the above list. The <code>id</code> attribute
>> +      of the <code>&lt;tag&gt;</code> subelement containing
>> +      <code>nativeMode</code> sets which VLAN is considered to be the
>> +      "native" VLAN for this interface and the <code>nativeMode</code>
>> +      attribute determines whether or not traffic for that VLAN will be
>> +      tagged or 802.1ad double tagged.
>>       </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 2699555..7262783 100644
>> --- a/docs/schemas/networkcommon.rng
>> +++ b/docs/schemas/networkcommon.rng
>> @@ -223,6 +223,7 @@
>>                 <choice>
>>                   <value>tagged</value>
>>                   <value>untagged</value>
>> +                <value>802.1ad</value>
>>                 </choice>
>>               </attribute>
>>             </optional>
>> diff --git a/src/conf/netdev_vlan_conf.c b/src/conf/netdev_vlan_conf.c
>> index 57d73ed..54e2b02 100644
>> --- a/src/conf/netdev_vlan_conf.c
>> +++ b/src/conf/netdev_vlan_conf.c
>> @@ -25,7 +25,7 @@
>>   #define VIR_FROM_THIS VIR_FROM_NONE
>>   
>>   VIR_ENUM_IMPL(virNativeVlanMode, VIR_NATIVE_VLAN_MODE_LAST,
>> -              "default", "tagged", "untagged",
>> +              "default", "tagged", "untagged", "802.1ad",
>>   );
>>   
>>   int
>> diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
>> index 4fa3a57..d8268bc 100644
>> --- a/src/util/virnetdevopenvswitch.c
>> +++ b/src/util/virnetdevopenvswitch.c
>> @@ -85,6 +85,11 @@ virNetDevOpenvswitchConstructVlans(virCommandPtr cmd, virNetDevVlanPtr virtVlan)
>>           virCommandAddArg(cmd, "vlan_mode=native-untagged");
>>           virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag);
>>           break;
>> +    case VIR_NATIVE_VLAN_MODE_8021AD:
>> +        virCommandAddArg(cmd, "vlan_mode=dot1q-tunnel");
>> +        virCommandAddArg(cmd, "other_config:qinq-ethtype=802.1q");
>> +        virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag);
>> +        break;
>>       case VIR_NATIVE_VLAN_MODE_DEFAULT:
>>       default:
>>           break;
>> @@ -498,6 +503,8 @@ int virNetDevOpenvswitchUpdateVlan(const char *ifname,
>>                            "--", "--if-exists", "clear", "Port", ifname, "tag",
>>                            "--", "--if-exists", "clear", "Port", ifname, "trunk",
>>                            "--", "--if-exists", "clear", "Port", ifname, "vlan_mode",
>> +                         "--", "--if-exists", "remove", "Port", ifname,
>> +                         "other_config", "qinq-ethtype",
>>                            "--", "--if-exists", "set", "Port", ifname, NULL);
>>   
>>       if (virNetDevOpenvswitchConstructVlans(cmd, virtVlan) < 0)
>> diff --git a/src/util/virnetdevvlan.h b/src/util/virnetdevvlan.h
>> index 2a13759..54109c6 100644
>> --- a/src/util/virnetdevvlan.h
>> +++ b/src/util/virnetdevvlan.h
>> @@ -27,6 +27,7 @@ typedef enum {
>>       VIR_NATIVE_VLAN_MODE_DEFAULT = 0,
>>       VIR_NATIVE_VLAN_MODE_TAGGED,
>>       VIR_NATIVE_VLAN_MODE_UNTAGGED,
>> +    VIR_NATIVE_VLAN_MODE_8021AD,
>>   
>>       VIR_NATIVE_VLAN_MODE_LAST
>>   } virNativeVlanMode;
>> diff --git a/tests/networkxml2xmlin/openvswitch-net.xml b/tests/networkxml2xmlin/openvswitch-net.xml
>> index 2f6084d..5734d36 100644
>> --- a/tests/networkxml2xmlin/openvswitch-net.xml
>> +++ b/tests/networkxml2xmlin/openvswitch-net.xml
>> @@ -30,4 +30,13 @@
>>         <parameters profileid='native-profile'/>
>>       </virtualport>
>>     </portgroup>
>> +  <portgroup name='8021ad'>
>> +    <vlan trunk='yes'>
>> +      <tag id='555' nativeMode='802.1ad'/>
>> +      <tag id='666'/>
>> +    </vlan>
>> +    <virtualport>
>> +      <parameters profileid='8021ad-profile'/>
>> +    </virtualport>
>> +  </portgroup>
>>   </network>
>> diff --git a/tests/networkxml2xmlout/openvswitch-net.xml b/tests/networkxml2xmlout/openvswitch-net.xml
>> index 2f6084d..5734d36 100644
>> --- a/tests/networkxml2xmlout/openvswitch-net.xml
>> +++ b/tests/networkxml2xmlout/openvswitch-net.xml
>> @@ -30,4 +30,13 @@
>>         <parameters profileid='native-profile'/>
>>       </virtualport>
>>     </portgroup>
>> +  <portgroup name='8021ad'>
>> +    <vlan trunk='yes'>
>> +      <tag id='555' nativeMode='802.1ad'/>
>> +      <tag id='666'/>
>> +    </vlan>
>> +    <virtualport>
>> +      <parameters profileid='8021ad-profile'/>
>> +    </virtualport>
>> +  </portgroup>
>>   </network>
>> diff --git a/tests/networkxml2xmlupdateout/openvswitch-net-modified.xml b/tests/networkxml2xmlupdateout/openvswitch-net-modified.xml
>> index cc0c344..ed605bf 100644
>> --- a/tests/networkxml2xmlupdateout/openvswitch-net-modified.xml
>> +++ b/tests/networkxml2xmlupdateout/openvswitch-net-modified.xml
>> @@ -30,4 +30,13 @@
>>         <parameters profileid='native-profile'/>
>>       </virtualport>
>>     </portgroup>
>> +  <portgroup name='8021ad'>
>> +    <vlan trunk='yes'>
>> +      <tag id='555' nativeMode='802.1ad'/>
>> +      <tag id='666'/>
>> +    </vlan>
>> +    <virtualport>
>> +      <parameters profileid='8021ad-profile'/>
>> +    </virtualport>
>> +  </portgroup>
>>   </network>
>> diff --git a/tests/networkxml2xmlupdateout/openvswitch-net-more-portgroups.xml b/tests/networkxml2xmlupdateout/openvswitch-net-more-portgroups.xml
>> index 7c19ad9..0237e42 100644
>> --- a/tests/networkxml2xmlupdateout/openvswitch-net-more-portgroups.xml
>> +++ b/tests/networkxml2xmlupdateout/openvswitch-net-more-portgroups.xml
>> @@ -41,4 +41,13 @@
>>         <parameters profileid='native-profile'/>
>>       </virtualport>
>>     </portgroup>
>> +  <portgroup name='8021ad'>
>> +    <vlan trunk='yes'>
>> +      <tag id='555' nativeMode='802.1ad'/>
>> +      <tag id='666'/>
>> +    </vlan>
>> +    <virtualport>
>> +      <parameters profileid='8021ad-profile'/>
>> +    </virtualport>
>> +  </portgroup>
>>   </network>
>> diff --git a/tests/networkxml2xmlupdateout/openvswitch-net-without-alice.xml b/tests/networkxml2xmlupdateout/openvswitch-net-without-alice.xml
>> index 4104424..cb9d12d 100644
>> --- a/tests/networkxml2xmlupdateout/openvswitch-net-without-alice.xml
>> +++ b/tests/networkxml2xmlupdateout/openvswitch-net-without-alice.xml
>> @@ -20,4 +20,13 @@
>>         <parameters profileid='native-profile'/>
>>       </virtualport>
>>     </portgroup>
>> +  <portgroup name='8021ad'>
>> +    <vlan trunk='yes'>
>> +      <tag id='555' nativeMode='802.1ad'/>
>> +      <tag id='666'/>
>> +    </vlan>
>> +    <virtualport>
>> +      <parameters profileid='8021ad-profile'/>
>> +    </virtualport>
>> +  </portgroup>
>>   </network>
>>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v5] openvswitch: Add new port VLAN mode "802.1ad"

Posted by Pavel Hrdina 5 years, 1 month ago

On Fri, Mar 08, 2019 at 11:21:37AM -0500, Laine Stump wrote:
> On 3/8/19 8:34 AM, John Ferlan wrote:
> > 
> > On 2/24/19 9:15 AM, ZhiPeng LU wrote:
> > > This patch adds functionality to allow libvirt to configure the '802.1ad'
> > > modes(802.1ad double-tagged) on openvswitch networks.
> > > For example:
> > >    <interface type='bridge'>
> > >      <mac address='2c:da:41:1d:05:42'/>
> > >      <source bridge='ovs0'/>
> > >      <vlan>
> > >        <tag id='41' nativeMode='dot1q-tunnel'/>
> > >      </vlan>
> > >      <virtualport type='openvswitch'>
> > >        <parameters interfaceid='6401a152-0b99-40b5-92be-858810aa6d37'/>
> > >      </virtualport>
> > >      <model type='virtio'/>
> > >      <driver name='vhost'/>
> > >      <alias name='net0'/>
> > >    </interface>
> > > 
> > > Signed-off-by: ZhiPeng Lu <luzhipeng@uniudc.com>
> > > ---
> > > v1->v2:
> > >    1. Fix "make syntax-check" failure
> > > v2->v3:
> > >    1. remove other_config when updating vlan
> > > v3->v4:
> > >    1. add commit message that has a brief description of the new
> > >        feature
> > >    2. add tests for 'dot1q-tunnel' vlan mode
> > > v4->v5:
> > >    1. modify some description and format
> > > 
> > > v4-resend:
> > >    https://www.redhat.com/archives/libvir-list/2019-February/msg00988.html
> > > 
> > >   docs/formatdomain.html.in                          | 33 +++++++++++++++-------
> > >   docs/formatnetwork.html.in                         | 26 ++++++++++-------
> > >   docs/schemas/networkcommon.rng                     |  1 +
> > >   src/conf/netdev_vlan_conf.c                        |  2 +-
> > >   src/util/virnetdevopenvswitch.c                    |  7 +++++
> > >   src/util/virnetdevvlan.h                           |  1 +
> > >   tests/networkxml2xmlin/openvswitch-net.xml         |  9 ++++++
> > >   tests/networkxml2xmlout/openvswitch-net.xml        |  9 ++++++
> > >   .../openvswitch-net-modified.xml                   |  9 ++++++
> > >   .../openvswitch-net-more-portgroups.xml            |  9 ++++++
> > >   .../openvswitch-net-without-alice.xml              |  9 ++++++
> > >   11 files changed, 94 insertions(+), 21 deletions(-)
> > > 
> > Apart from now needing to indicate support in 5.2.0 for the
> > format*.html.in files and the need for a docs/news.xml note this seems
> > fine to me and covers what Laine had originally reviewed. I can modify
> > those two before pushing.
> > 
> > Also, I've CC'd Laine in hopes he can also take a look for sanity's sake
> > to ensure I didn't misinterpret something he requested previously!
> 
> 
> Yeah, sorry I haven't responded to the last couple revisions of this patch.
> When I saw them I tagged them in red in my mail, but don't have an effective
> queuing mechanism and ended up getting lost in some other distraction and
> not getting back until the red message was scrolled way up out of sight :-/
> Anyway, thanks to John for keeping track of it and reviewing it, and ZhiPeng
> Lu for being patient.
> 
> 
> 
> > 
> > I'll also add a followup patch to update docs/news.xml with the
> > following text:
> > 
> > +      <change>
> > +        <summary>
> > +          Add support for "802.1ad" VLAN mode
> 
> 
> (You know, I expected someone to counter-propose use of a non-official term
> for this, since some places libvirt uses official names from the standards
> documents and other places it uses informal terms. Since there was no
> counter-proposal, I'm now unsure if that happened because 1) everyone agrees
> with using "802.1ad" (which is unambiguous but its function may be less
> obvious to a casual user), or 2) nobody even noticed :-P (but I still think
> using the official name is better, especially because it assures we won't
> end up with confusion if we later need to add some other sort of
> tunneled/nested tagging)

I would say 2) is the case :).  Is there a standard for the two existing
modes?  To not make it even more confusing how about we uses the
non-official term as the libvirt accepted value but in the docs we also
mention the specific standard for each mode to make it absolutely clear
what it refers to?

Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v5] openvswitch: Add new port VLAN mode "802.1ad"

Posted by Laine Stump 5 years, 1 month ago

On 3/12/19 7:48 AM, Pavel Hrdina wrote:
> On Fri, Mar 08, 2019 at 11:21:37AM -0500, Laine Stump wrote:
>> On 3/8/19 8:34 AM, John Ferlan wrote:
>>> On 2/24/19 9:15 AM, ZhiPeng LU wrote:
>>>> This patch adds functionality to allow libvirt to configure the '802.1ad'
>>>> modes(802.1ad double-tagged) on openvswitch networks.
>>>> For example:
>>>>     <interface type='bridge'>
>>>>       <mac address='2c:da:41:1d:05:42'/>
>>>>       <source bridge='ovs0'/>
>>>>       <vlan>
>>>>         <tag id='41' nativeMode='dot1q-tunnel'/>
>>>>       </vlan>
>>>>       <virtualport type='openvswitch'>
>>>>         <parameters interfaceid='6401a152-0b99-40b5-92be-858810aa6d37'/>
>>>>       </virtualport>
>>>>       <model type='virtio'/>
>>>>       <driver name='vhost'/>
>>>>       <alias name='net0'/>
>>>>     </interface>
>>>>
>>>> Signed-off-by: ZhiPeng Lu <luzhipeng@uniudc.com>
>>>> ---
>>>> v1->v2:
>>>>     1. Fix "make syntax-check" failure
>>>> v2->v3:
>>>>     1. remove other_config when updating vlan
>>>> v3->v4:
>>>>     1. add commit message that has a brief description of the new
>>>>         feature
>>>>     2. add tests for 'dot1q-tunnel' vlan mode
>>>> v4->v5:
>>>>     1. modify some description and format
>>>>
>>>> v4-resend:
>>>>     https://www.redhat.com/archives/libvir-list/2019-February/msg00988.html
>>>>
>>>>    docs/formatdomain.html.in                          | 33 +++++++++++++++-------
>>>>    docs/formatnetwork.html.in                         | 26 ++++++++++-------
>>>>    docs/schemas/networkcommon.rng                     |  1 +
>>>>    src/conf/netdev_vlan_conf.c                        |  2 +-
>>>>    src/util/virnetdevopenvswitch.c                    |  7 +++++
>>>>    src/util/virnetdevvlan.h                           |  1 +
>>>>    tests/networkxml2xmlin/openvswitch-net.xml         |  9 ++++++
>>>>    tests/networkxml2xmlout/openvswitch-net.xml        |  9 ++++++
>>>>    .../openvswitch-net-modified.xml                   |  9 ++++++
>>>>    .../openvswitch-net-more-portgroups.xml            |  9 ++++++
>>>>    .../openvswitch-net-without-alice.xml              |  9 ++++++
>>>>    11 files changed, 94 insertions(+), 21 deletions(-)
>>>>
>>> Apart from now needing to indicate support in 5.2.0 for the
>>> format*.html.in files and the need for a docs/news.xml note this seems
>>> fine to me and covers what Laine had originally reviewed. I can modify
>>> those two before pushing.
>>>
>>> Also, I've CC'd Laine in hopes he can also take a look for sanity's sake
>>> to ensure I didn't misinterpret something he requested previously!
>>
>> Yeah, sorry I haven't responded to the last couple revisions of this patch.
>> When I saw them I tagged them in red in my mail, but don't have an effective
>> queuing mechanism and ended up getting lost in some other distraction and
>> not getting back until the red message was scrolled way up out of sight :-/
>> Anyway, thanks to John for keeping track of it and reviewing it, and ZhiPeng
>> Lu for being patient.
>>
>>
>>
>>> I'll also add a followup patch to update docs/news.xml with the
>>> following text:
>>>
>>> +      <change>
>>> +        <summary>
>>> +          Add support for "802.1ad" VLAN mode
>>
>> (You know, I expected someone to counter-propose use of a non-official term
>> for this, since some places libvirt uses official names from the standards
>> documents and other places it uses informal terms. Since there was no
>> counter-proposal, I'm now unsure if that happened because 1) everyone agrees
>> with using "802.1ad" (which is unambiguous but its function may be less
>> obvious to a casual user), or 2) nobody even noticed :-P (but I still think
>> using the official name is better, especially because it assures we won't
>> end up with confusion if we later need to add some other sort of
>> tunneled/nested tagging)
> I would say 2) is the case :).  Is there a standard for the two existing
> modes?  To not make it even more confusing how about we uses the
> non-official term as the libvirt accepted value but in the docs we also
> mention the specific standard for each mode to make it absolutely clear
> what it refers to?


This question caused me to look at the "nativeMode" attribute more 
closely, and I now think that the nativeMode attribute may be the wrong 
place to configure 802.1ad tunneled mode.


It is true that all three of these modes are configured in openvswitch 
via the "vlan_mode" commandline option to ovs-vsctl, so they are 
mutually exclusive. However, I'm not sure that 802.1ad tunneling is only 
used for packets that are on the native vlan when the port is in trunk 
mode (which is what the nativeMode attribute is supposed to be for), so 
in libvirt's config maybe it shouldn't be set with an attribute called 
"nativeMode".


Fortunately, while looking all this up, I realized that I know the 
person who added support for 802.1ad "QinQ" tunneling to Open vSwitch 
(Hi Eric!), so I'm Cc'ing him to this message in hopes that his better 
knowledge of the subject matter can help to clear up my confusion.


So is it okay to shoehorn this setting into the nativeMode attribute of 
the tag id for the native VLAN? Or is this something that should apply 
to *all* traffic going through the port (and thus deserves a different 
config attribute, maybe a direct attribute of the <vlan> element)?


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v5] openvswitch: Add new port VLAN mode "802.1ad"

Posted by Eric Garver 5 years, 1 month ago

On Fri, Mar 15, 2019 at 01:46:37PM -0400, Laine Stump wrote:
> On 3/12/19 7:48 AM, Pavel Hrdina wrote:
> > On Fri, Mar 08, 2019 at 11:21:37AM -0500, Laine Stump wrote:
> > > On 3/8/19 8:34 AM, John Ferlan wrote:
> > > > On 2/24/19 9:15 AM, ZhiPeng LU wrote:
> > > > > This patch adds functionality to allow libvirt to configure the '802.1ad'
> > > > > modes(802.1ad double-tagged) on openvswitch networks.
> > > > > For example:
> > > > >     <interface type='bridge'>
> > > > >       <mac address='2c:da:41:1d:05:42'/>
> > > > >       <source bridge='ovs0'/>
> > > > >       <vlan>
> > > > >         <tag id='41' nativeMode='dot1q-tunnel'/>
> > > > >       </vlan>
> > > > >       <virtualport type='openvswitch'>
> > > > >         <parameters interfaceid='6401a152-0b99-40b5-92be-858810aa6d37'/>
> > > > >       </virtualport>
> > > > >       <model type='virtio'/>
> > > > >       <driver name='vhost'/>
> > > > >       <alias name='net0'/>
> > > > >     </interface>
> > > > > 
> > > > > Signed-off-by: ZhiPeng Lu <luzhipeng@uniudc.com>
> > > > > ---
> > > > > v1->v2:
> > > > >     1. Fix "make syntax-check" failure
> > > > > v2->v3:
> > > > >     1. remove other_config when updating vlan
> > > > > v3->v4:
> > > > >     1. add commit message that has a brief description of the new
> > > > >         feature
> > > > >     2. add tests for 'dot1q-tunnel' vlan mode
> > > > > v4->v5:
> > > > >     1. modify some description and format
> > > > > 
> > > > > v4-resend:
> > > > >     https://www.redhat.com/archives/libvir-list/2019-February/msg00988.html
> > > > > 
> > > > >    docs/formatdomain.html.in                          | 33 +++++++++++++++-------
> > > > >    docs/formatnetwork.html.in                         | 26 ++++++++++-------
> > > > >    docs/schemas/networkcommon.rng                     |  1 +
> > > > >    src/conf/netdev_vlan_conf.c                        |  2 +-
> > > > >    src/util/virnetdevopenvswitch.c                    |  7 +++++
> > > > >    src/util/virnetdevvlan.h                           |  1 +
> > > > >    tests/networkxml2xmlin/openvswitch-net.xml         |  9 ++++++
> > > > >    tests/networkxml2xmlout/openvswitch-net.xml        |  9 ++++++
> > > > >    .../openvswitch-net-modified.xml                   |  9 ++++++
> > > > >    .../openvswitch-net-more-portgroups.xml            |  9 ++++++
> > > > >    .../openvswitch-net-without-alice.xml              |  9 ++++++
> > > > >    11 files changed, 94 insertions(+), 21 deletions(-)
> > > > > 
> > > > Apart from now needing to indicate support in 5.2.0 for the
> > > > format*.html.in files and the need for a docs/news.xml note this seems
> > > > fine to me and covers what Laine had originally reviewed. I can modify
> > > > those two before pushing.
> > > > 
> > > > Also, I've CC'd Laine in hopes he can also take a look for sanity's sake
> > > > to ensure I didn't misinterpret something he requested previously!
> > > 
> > > Yeah, sorry I haven't responded to the last couple revisions of this patch.
> > > When I saw them I tagged them in red in my mail, but don't have an effective
> > > queuing mechanism and ended up getting lost in some other distraction and
> > > not getting back until the red message was scrolled way up out of sight :-/
> > > Anyway, thanks to John for keeping track of it and reviewing it, and ZhiPeng
> > > Lu for being patient.
> > > 
> > > 
> > > 
> > > > I'll also add a followup patch to update docs/news.xml with the
> > > > following text:
> > > > 
> > > > +      <change>
> > > > +        <summary>
> > > > +          Add support for "802.1ad" VLAN mode
> > > 
> > > (You know, I expected someone to counter-propose use of a non-official term
> > > for this, since some places libvirt uses official names from the standards
> > > documents and other places it uses informal terms. Since there was no
> > > counter-proposal, I'm now unsure if that happened because 1) everyone agrees
> > > with using "802.1ad" (which is unambiguous but its function may be less
> > > obvious to a casual user), or 2) nobody even noticed :-P (but I still think
> > > using the official name is better, especially because it assures we won't
> > > end up with confusion if we later need to add some other sort of
> > > tunneled/nested tagging)
> > I would say 2) is the case :).  Is there a standard for the two existing
> > modes?  To not make it even more confusing how about we uses the
> > non-official term as the libvirt accepted value but in the docs we also
> > mention the specific standard for each mode to make it absolutely clear
> > what it refers to?
> 
> 
> This question caused me to look at the "nativeMode" attribute more closely,
> and I now think that the nativeMode attribute may be the wrong place to
> configure 802.1ad tunneled mode.
> 
> 
> It is true that all three of these modes are configured in openvswitch via
> the "vlan_mode" commandline option to ovs-vsctl, so they are mutually
> exclusive. However, I'm not sure that 802.1ad tunneling is only used for
> packets that are on the native vlan when the port is in trunk mode (which is
> what the nativeMode attribute is supposed to be for), so in libvirt's config
> maybe it shouldn't be set with an attribute called "nativeMode".
> 
> 
> Fortunately, while looking all this up, I realized that I know the person
> who added support for 802.1ad "QinQ" tunneling to Open vSwitch (Hi Eric!),
> so I'm Cc'ing him to this message in hopes that his better knowledge of the
> subject matter can help to clear up my confusion.
> 
> 
> So is it okay to shoehorn this setting into the nativeMode attribute of the
> tag id for the native VLAN? Or is this something that should apply to *all*
> traffic going through the port (and thus deserves a different config
> attribute, maybe a direct attribute of the <vlan> element)?

I'm not familiar with the libvirt configuration or use cases, so please
take my input with a cup of salt.

I don't think "nativeMode" is appropriate here. If I understand it
correctly, nativeMode currently has values "tagged" and "untagged". This
implies it's used for trunk ports.

In OVS, "dot1q-tunnel" does _not_ behave like a trunk port. It more
closely resembles an access port. Frames that egress a dot1q-tunnel port
_always_ have their outer VLAN stripped.

Furthermore, dot1q-tunnel has other features that you may have interest
in supporting:

    - configurable outer TPID: 802.1ad (0x88a8) or 802.1q (0x8100, i.e.
      QinQ)
    - "tag" specifies the outer VLAN ID
        - maybe this is covered by the tag element inside the vlan
          element in the example above.
    - customer VLAN filtering (cvlans). Default accepts all C-VLANs.

Does libvirt have any tunnel-like configuration? If so that may be a
better fit.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list