Re: [libvirt] [PATCH v5] openvswitch: Add new port VLAN mode "802.1ad"

On 3/8/19 8:34 AM, John Ferlan wrote: > > On 2/24/19 9:15 AM, ZhiPeng LU wrote: > > This patch adds functionality to allow libvirt to configure the '802.1ad' > > modes(802.1ad double-tagged) on openvswitch networks. > > For example: > > <interface type='bridge'> > > <mac address='2c:da:41:1d:05:42'/> > > <source bridge='ovs0'/> > > <vlan> > > <tag id='41' nativeMode='dot1q-tunnel'/> > > </vlan> > > <virtualport type='openvswitch'> > > <parameters interfaceid='6401a152-0b99-40b5-92be-858810aa6d37'/> > > </virtualport> > > <model type='virtio'/> > > <driver name='vhost'/> > > <alias name='net0'/> > > </interface> > > > > Signed-off-by: ZhiPeng Lu <luzhipeng(a)uniudc.com&gt; > > --- > > v1->v2: > > 1. Fix "make syntax-check" failure > > v2->v3: > > 1. remove other_config when updating vlan > > v3->v4: > > 1. add commit message that has a brief description of the new > > feature > > 2. add tests for 'dot1q-tunnel' vlan mode > > v4->v5: > > 1. modify some description and format > > > > v4-resend: > > https://www.redhat.com/archives/libvir-list/2019-February/msg00988.html > > > > docs/formatdomain.html.in | 33 +++++++++++++++------- > > docs/formatnetwork.html.in | 26 ++++++++++------- > > docs/schemas/networkcommon.rng | 1 + > > src/conf/netdev_vlan_conf.c | 2 +- > > src/util/virnetdevopenvswitch.c | 7 +++++ > > src/util/virnetdevvlan.h | 1 + > > tests/networkxml2xmlin/openvswitch-net.xml | 9 ++++++ > > tests/networkxml2xmlout/openvswitch-net.xml | 9 ++++++ > > .../openvswitch-net-modified.xml | 9 ++++++ > > .../openvswitch-net-more-portgroups.xml | 9 ++++++ > > .../openvswitch-net-without-alice.xml | 9 ++++++ > > 11 files changed, 94 insertions(+), 21 deletions(-) > > > Apart from now needing to indicate support in 5.2.0 for the > format*.html.in files and the need for a docs/news.xml note this seems > fine to me and covers what Laine had originally reviewed. I can modify > those two before pushing. > > Also, I've CC'd Laine in hopes he can also take a look for sanity's sake > to ensure I didn't misinterpret something he requested previously! Yeah, sorry I haven't responded to the last couple revisions of this patch. When I saw them I tagged them in red in my mail, but don't have an effective queuing mechanism and ended up getting lost in some other distraction and not getting back until the red message was scrolled way up out of sight :-/ Anyway, thanks to John for keeping track of it and reviewing it, and ZhiPeng Lu for being patient. > > I'll also add a followup patch to update docs/news.xml with the > following text: > > + <change> > + <summary> > + Add support for "802.1ad" VLAN mode (You know, I expected someone to counter-propose use of a non-official term for this, since some places libvirt uses official names from the standards documents and other places it uses informal terms. Since there was no counter-proposal, I'm now unsure if that happened because 1) everyone agrees with using "802.1ad" (which is unambiguous but its function may be less obvious to a casual user), or 2) nobody even noticed :-P (but I still think using the official name is better, especially because it assures we won't end up with confusion if we later need to add some other sort of tunneled/nested tagging)