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

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



On Mon, 2013-04-22 at 17:08 -0400, Laine Stump wrote:
> (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.)
I expect this was a side affect of getting the digest emails from the
list.

> 
> On 04/22/2013 12:51 PM, james robson wrote:
> >> Date: Thu, 18 Apr 2013 14:14:32 -0400
> >> From: Laine Stump <laine laine org>
> >> To: libvir-list redhat com
> >> Subject: Re: [libvirt] [PATCHv2] Configure native vlan modes on Open
> >> 	vSwitch ports
> >> Message-ID: <51703808 2000504 laine org>
> >> 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>

The only problem with that proposal is that there is no way to set the
mode to be 'tagged' or 'untagged'. What about the following:
  <vlan trunk='yes'>
    <tag id='123' nativeMode='tagged|untagged|none'/> (default is none)
    ...
 </vlan>

I think nativeMode would be more appropriate than simply 'native' for
choosing between tagged or untagged, because it more descriptive. 

> 
> 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?)

If nothing major goes wrong I should have this done before Friday.




 Protected by Websense Hosted Email Security -- www.websense.com 


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