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(a)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)?