On 2/14/19 7:53 AM, 芦志朋 wrote:
This patch adds functionality to allow libvirt to configure the
'dot1q-tunnel'
modes(802.1ad double-tagged) on openvswitch networks.
no need for CRLF after 'dot1q-tunnel'....
also this will need to be fixed to match code and comments below.
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'/>
see my note below about interfaceid... this one is just short enough it
seems...
</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
Nice to also add a pointer to the history of where the previous patch
come from in the archive, such as:
v4-resend:
https://www.redhat.com/archives/libvir-list/2018-December/msg00244.html
v4:
https://www.redhat.com/archives/libvir-list/2018-December/msg00197.html
v3-resend:
https://www.redhat.com/archives/libvir-list/2018-December/msg00044.html
v3:
https://www.redhat.com/archives/libvir-list/2018-December/msg00001.html
NB: The only one to get at least some review it seems
v2:
https://www.redhat.com/archives/libvir-list/2018-November/msg01112.html
v1:
https://www.redhat.com/archives/libvir-list/2018-November/msg01027.html
Note that once your updated patch contains a link to the previous one,
then the next one would only need to contain one link. Allows future
reviewer to see history just in case it's not reviewed by the same
person who did the first review.
docs/formatdomain.html.in | 20
++++++++++++++------
docs/formatnetwork.html.in | 20
+++++++++++---------
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, 80 insertions(+), 16 deletions(-)
First off - it doesn't seem git send-email was used to generate the
patch and your client has done some less than nice things to the
generated patch. I'll just comment here inline, but hopefully your next
posting can use the proper git send-email.
I'm not the expert in this area, I would suspect Laine would know better
than I do... I CC'd him mainly to keep him in the loop in case he missed
this posting... I also note that the last time you posted this and it
got reviewed you used "dot1q-tunnel" Laine pointed out you should use
the official name; however, you just reposted with the common nickname.
Since "dot1q-tunnel" wasn't acceptable and you put asked about using
"802.1ad", then I think that's what you should have gone with in this
update.
I do see that src/util/virnetdevvportprofile.c has virNetDevVPort with a
couple of "802.1Q*" strings, so there is precedent... There's also
virInterfaceDefParseBondMode that converts "802.3ad"... Use those as
guides for your naming.
I also see your examples use "<parameters
profileid='8021ad-profile'/>"... The profileid is not explained, but
reading the networkcommon.rng file, I see it can be a string of up to 39
characters. Not quite sure how it's used, but perhaps we can also take
this opportunity to add a patch that would explain it prior to this
patch that's adding more consumers.
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 2ae5006..2a53d7c 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6085,8 +6085,14 @@ qemu-kvm -net nic,model=? /dev/null
<b><tag id='42'/></b>
<b><tag id='123'
nativeMode='untagged'/></b>
<b></vlan></b>
- ...
^^^^
This removal seems spurious
</interface>
+ <interface type='bridge'>
+ <b><vlan trunk='yes'></b>
+ <b><tag id='42'/></b>
+ <b><tag id='123'
nativeMode='dot1q-tunnel'/></b>
Rather than pure copy-n-paste, change 123 to some other number since 123
is associated with untagged AFAIUI
+ <b></vlan></b>
+ </interface>
+...
This one should be indented by a few more spaces to align with the
</interface> and it should be above that line as well since using ...
indicates there's more XML that would be present, but all the snippet is
doing it showing the important piece.
</devices>
...</pre>
@@ -6122,11 +6128,13 @@ 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>
+ to configure 'native-tagged' and 'native-untagged'
+ <span class="since">Since 1.1.0.</span> and
'dot1q-tunnel'
+ (802.1ad double-tagged) <span class="since">Since
5.0.0.</span>
+ VLAN modes 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' or
+ 'dot1q-tunnel'. The <code>id</code>
Reading the above is getting confusing, can we go with:
<p>
For network connections using Open vSwitch it is also possible
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.
</p>
NB: the formatnetwork.html.in is very similar to what you had here;
however, in that hunk you added "or QinQ" to the last sentence. That
probably should be done here. However, seeing QinQ is "odd" for this
less knowledgeable reader - perhaps expand to "Q-in-Q" or "Q-in-Q
tunneling" or perhaps "802.1ad double tagged" or something similar (I
*can* google ;-))
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
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index 509cca9..cfce4a1 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -705,16 +705,18 @@
</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
+ to configure 'native-tagged' and 'native-untagged'
+ <span class="since">Since 1.1.0.</span>
+ and 'dot1q-tunnel'(802.1ad double-tagged) VLAN modes.
+ <span class="since">Since 5.0.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' or 'dot1q-tunnel'.
+ 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.
+ traffic for that VLAN will be tagged or QinQ.
See above - this should change similarly I believe.
</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..11c48ff 100644
--- a/docs/schemas/networkcommon.rng
+++ b/docs/schemas/networkcommon.rng
@@ -223,6 +223,7 @@
<choice>
<value>tagged</value>
<value>untagged</value>
+ <value>dot1q-tunnel</value>
802.1ad
searches on 802 in doc/schemas/*.rng finds 802.3ad, 802.1Qbg, 802.1Qbh
</choice>
</attribute>
</optional>
diff --git a/src/conf/netdev_vlan_conf.c b/src/conf/netdev_vlan_conf.c
index 57d73ed..892d31c 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",
"dot1q-tunnel",
"802.1ad"
);
int
diff --git a/src/util/virnetdevopenvswitch.c
b/src/util/virnetdevopenvswitch.c
index 4fa3a57..dd2aeac 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_DOT1Q_TUNNEL:
see below for new name...
+ 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",
I have no idea if this is right, but have to assume it is...
"--", "--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..9d0a2bd 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_DOT1Q_TUNNEL,
This nomenclature would seemingly need to be changed too to be something
like VIR_NATIVE_VLAN_MODE_8021AD,
John
VIR_NATIVE_VLAN_MODE_LAST
} virNativeVlanMode;
diff --git a/tests/networkxml2xmlin/openvswitch-net.xml
b/tests/networkxml2xmlin/openvswitch-net.xml
index 2f6084d..0952859 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='dot1q-tunnel'/>
+ <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..0952859 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='dot1q-tunnel'/>
+ <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..e5794fc 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='dot1q-tunnel'/>
+ <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..3df0b96 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='dot1q-tunnel'/>
+ <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..38846aa 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='dot1q-tunnel'/>
+ <tag id='666'/>
+ </vlan>
+ <virtualport>
+ <parameters profileid='8021ad-profile'/>
+ </virtualport>
+ </portgroup>
</network>
--
1.8.3.1
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list