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

> From: ZhiPeng Lu <luzhipeng(a)uniudc.com&gt; > > Signed-off-by: ZhiPeng Lu <luzhipeng(a)uniudc.com&gt; 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;