Re: [PATCH v3] openvswitch: Add new port VLAN mode "dot1q-tunnel"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.



> ---
> 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.


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;


Attachment: pEpkey.asc
Description: application/pgp-keys

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux