[libvirt] [PATCH v5] openvswitch: Add new port VLAN mode "802.1ad"
Laine Stump
laine at laine.org
Fri Mar 8 16:21:37 UTC 2019
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 at 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 at 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></vlan></b>
>> ...
>> </interface>
>> + <interface type='bridge'>
>> + <b><vlan trunk='yes'></b>
>> + <b><tag id='42'/></b>
>> + <b><tag id='555' nativeMode='802.1ad'/></b>
>> + <b></vlan></b>
>> + ...
>> + </interface>
>> </devices>
>> ...</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><tag></code> subelement: <code>nativeMode</code>
>> - may be set to 'tagged' or 'untagged'. The <code>id</code>
>> - attribute of the <code><tag></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><tag></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><tag></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><tag></code> subelement: <code>nativeMode</code>
>> - may be set to 'tagged' or 'untagged'. The <code>id</code>
>> - attribute of the <code><tag></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><tag></code> subelement. The attribute may be
>> + set to a string from the above list. The <code>id</code> attribute
>> + of the <code><tag></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><vlan></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>
>>
More information about the libvir-list
mailing list