发件人:Laine Stump <laine@laine.org>
发送日期:2018-12-03 22:37:36
收件人:libvir-list@redhat.com
抄送人:luzhipeng@uniudc.com
主题:Re: [libvirt] [PATCH v3] openvswitch: Add new port VLAN mode "dot1q-tunnel">On 12/2/18 10:18 PM, luzhipeng@uniudc.com wrote:
>> From: ZhiPeng Lu <luzhipeng@uniudc.com>
>>
>> Signed-off-by: ZhiPeng Lu <luzhipeng@uniudc.com>
>
>
>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>&lt;tag&gt;</code> subelement: <code>nativeMode</code>
>> -      may be set to 'tagged' or 'untagged'. The <code>id</code>
>> -      attribute of the <code>&lt;tag&gt;</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>&lt;tag&gt;</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>&lt;tag&gt;</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>&lt;vlan&gt;</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; > >