发件人：Laine Stump <laine@xxxxxxxxx>
发送日期：2018-12-03 22:37:36
收件人：libvir-list@xxxxxxxxxx
抄送人：luzhipeng@xxxxxxxxxx
主题：Re: [libvirt] [PATCH v3] openvswitch: Add new port VLAN mode "dot1q-tunnel">On 12/2/18 10:18 PM, luzhipeng@xxxxxxxxxx wrote:
>> From: ZhiPeng Lu <luzhipeng@xxxxxxxxxx>
>>
>> Signed-off-by: ZhiPeng Lu <luzhipeng@xxxxxxxxxx>
>
>
>Please include a commit message that has a brief description of the new
>feature, including an XML example (yes, this is in addition to the
>update to formatnetwork.html.in change). This makes it easier for
>someone searching the git log history for a feature to find the commit
>when it was introduced.
ok，thanks
>> ---
>> v1->v2:
>> 1. Fix "make syntax-check" failure
>> v2->v3:
>> 1. remove other_config when updating vlan
>>
>>
>> docs/formatnetwork.html.in | 17 +++++++++--------
>
>
>There is also a section about the <vlan> element in formatdomain.html.in
>that needs to be updated to show support for the new nativeMode setting.
>
>
>> docs/schemas/networkcommon.rng | 1 +
>> src/conf/netdev_vlan_conf.c | 2 +-
>> src/util/virnetdevopenvswitch.c | 7 +++++++
>> src/util/virnetdevvlan.h | 1 +
>> 5 files changed, 19 insertions(+), 9 deletions(-)
>>
>> diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
>> index 363a72b..3c1ae62 100644
>> --- a/docs/formatnetwork.html.in
>> +++ b/docs/formatnetwork.html.in
>> @@ -688,16 +688,17 @@
>> </p>
>> <p>
>> For network connections using Open vSwitch it is also possible
>> - to configure 'native-tagged' and 'native-untagged' VLAN modes
>> + to configure 'native-tagged' and 'native-untagged' and 'dot1q-tunnel'
>> + 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
>> + optional <code>nativeMode</code> attribute on the
>> + <code><tag></code> subelement: <code>nativeMode</code>
>> + may be set to 'tagged' or 'untagged' or 'dot1q-tunnel'.
>
>
>
>It would be preferable to use the official name (802.1Q) rather than a
>term that is just a commonly used nickname. That makes it easier for
>someone unfamiliar with the feature to learn what it is via an internet
>search.
802.1ad better？
>Also, you should add a '<space class="since">(since 5.0.0)' after the
>name of the new attribute so that it's clear that attribute was added
>later (it may also help to remove ambiguity if you add a "since" tag
>after each attribute name, otherwise someone might think the "since" tag
>at the end applies to all of the attribute names).
>
>
>
>> + 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.
>
>
>Here is another use of a nickname. Better to use the official name from
>the spec.
>
>
>> </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>
>> </choice>
>> </attribute>
>> </optional>
>> diff --git a/src/conf/netdev_vlan_conf.c b/src/conf/netdev_vlan_conf.c
>> index dff49c6..79710d9 100644
>> --- a/src/conf/netdev_vlan_conf.c
>> +++ b/src/conf/netdev_vlan_conf.c
>> @@ -29,7 +29,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")
>
>> int
>> virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlanPtr def)
>> diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
>> index 8fe06fd..9fec30b 100644
>> --- a/src/util/virnetdevopenvswitch.c
>> +++ b/src/util/virnetdevopenvswitch.c
>> @@ -91,6 +91,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:
>> + 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;
>> @@ -504,6 +509,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", NULL,
>> "--", "--if-exists", "set", "Port", ifname, NULL);
>>
>> if (virNetDevOpenvswitchConstructVlans(cmd, virtVlan) < 0)
>> diff --git a/src/util/virnetdevvlan.h b/src/util/virnetdevvlan.h
>> index be85f59..0667f9d 100644
>> --- a/src/util/virnetdevvlan.h
>> +++ b/src/util/virnetdevvlan.h
>> @@ -29,6 +29,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,
>>
>> VIR_NATIVE_VLAN_MODE_LAST
>> } virNativeVlanMode;
>
>
