发件人:Laine Stump <laine(a)laine.org>
发送日期:2018-12-03 22:37:36
收件人:libvir-list(a)redhat.com
抄送人:luzhipeng(a)uniudc.com
主题:Re: [libvirt] [PATCH v3] openvswitch: Add new port VLAN mode
"dot1q-tunnel">On 12/2/18 10:18 PM, luzhipeng(a)uniudc.com wrote:
> From: ZhiPeng Lu <luzhipeng(a)uniudc.com>
>
> Signed-off-by: ZhiPeng Lu <luzhipeng(a)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><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;