Re: [libvirt] [PATCHv2] Configure native vlan modes on Open vSwitch ports

(I'm not sure what you did differently when you sent this mail, but somehow your mailer botched the "In-Reply-To:" header, which broke the threaded display in Thunderbird. No big deal, but I thought you might want to know.)

On 04/22/2013 12:51 PM, james robson wrote: >> Date: Thu, 18 Apr 2013 14:14:32 -0400 >> From: Laine Stump <laine(a)laine.org&gt; >> To: libvir-list(a)redhat.com >> Subject: Re: [libvirt] [PATCHv2] Configure native vlan modes on Open >> vSwitch ports >> Message-ID: <51703808.2000504(a)laine.org&gt; >> Content-Type: text/plain; charset=ISO-8859-1 >> >> On 04/18/2013 01:44 PM, james robson wrote: >>> Hello, >>> Has any one been able to review this yet? I realise that the 'Since >>> 1.0.3' in the doc page is now out of date, but is the code itself >>> acceptable? >> I was hoping that someone with more knowledge of Open vSwitch and/or >> vlan tagging/trunking/native mode would repond to the message (Kyle?) >> but there was silence instead... >> >> >>> >>> diff --git a/tests/networkxml2xmlin/openvswitch-net.xml >>> b/tests/networkxml2xmlin/openvswitch-net.xml >>> index a3d82b1..93c49d5 100644 >>> --- a/tests/networkxml2xmlin/openvswitch-net.xml >>> +++ b/tests/networkxml2xmlin/openvswitch-net.xml >>> @@ -21,4 +21,13 @@ >>> <parameters profileid='alice-profile'/> >>> </virtualport> >>> </portgroup> >>> + <portgroup name='tagged'> >>> + <vlan native_mode='tagged' native_tag='123'> >>> + <tag id='555'/> >>> + <tag id='444'/> >>> + </vlan> >>> + <virtualport> >>> + <parameters profileid='tagged-profile'/> >>> + </virtualport> >>> + </portgroup> >> As brought up again in a separate conversation today, we prefer to use >> camelCase rather than underscored in attribute and element names. So, if >> we were to use the layout you're proposing, the attributes should be >> called "nativeMode" and "nativeTag". >> >> However, I'm wondering if there might be a better way to structure it. >> What about this? >> >> >> <vlan trunk='yes'> >> <tag id='123' native='tagged|untagged'/> (or whatever values are >> appropriate) Sounds like this can be "native='yes'", since there is only the possiblity of a tag being the native tag, or *not* being the native tag. >> <tag id='555'/> >> <tag id='444'/> >> </vlan> >> >> Do I understand correctly that native mode is telling what to do with >> packets that come in untagged, and that (using your nomenclature >> "native_mode='yes' native_tag='123'" means "when an untagged packet come >> in from this interface, it should be tagged as 123 before forwarding"? > That is correct, setting the native vlan changes how an untagged packet > is handled when it enters the port. The difference between the 'tagged' > and 'untagged' modes is in how packets on the native vlan are processed > before exiting the port. > > >> And what happens when native_mode='yes' but there is no native_tag? > In that case you configuration is invalid, and will get an error. Okay, then doing the config the way I suggest would eliminate that possibility, so it has an upside :-) > > >> (that's what I was trying to describe with <tag id='123' >> native='untagged'/>, but I don't even know if that makes sense, because >> I don't know exactly what is the native vlan tag and what is done with >> it :-) > That arrangement would make sense, I chose the arrangement I did for two > main reasons. There can only be one native vlan on a port, making it an > attribute of the 'vlan' tag enforces this. Also, I wanted to keep the > validation and processing separate rather than add 'if native' branches > to the loops that operate on the vlan id list. > I can see the advantage of having a single setting to configure the > native vlan, rather than the two attributes I proposed. If the new > suggestion is preferred I can rework my patch to use that format. I think it's simpler, to do it that way, yes. > > >> Also, is it valid to have a native_mode/native_tag if trunk='no'? (right >> now trunk is automatically set to 'yes' if there is more than one vlan tag) > It isn't valid to have trunk='no' and the native settings. Therefore > "<vlan trunk='no' native_mode='tagged' native_tag='123'>" will get an > error if you try to enter it. > If no "trunk" attribute is set explicitly then it will be set to 'yes'. > This means "<vlan native_mode='tagged' native_tag='123'>" is equivalent > to "<vlan trunk='yes' native_mode='tagged' native_tag='123'>". Likewise, if you have more than one <id tag='x'/> element, trunk will automatically be set to yes. So in the end, if you don't foresee any problems with it, I think I do prefer this: <vlan trunk='yes'> <tag id='123' native='yes|no'/> (default is 'no', only one allowed to be yes) <tag id='555'/> <tag id='444'/> </vlan>

Note that we'll be going into freeze for 1.0.5 soon, so if you are able to rework the patch within the next couple days, it should go into libvirt-1.0.5 (which *might* end up in Fedora 19?)