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

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

 



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@xxxxxxxxxx>
---
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@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]

  Powered by Linux