On Thu, Jul 11, 2013 at 06:26:32PM -0700, Jesse Gross wrote: > On Wed, Jul 10, 2013 at 5:16 PM, Simon Horman <ho...@verge.net.au> wrote: > > On Mon, Jul 08, 2013 at 11:51:18PM -0700, Jesse Gross wrote: > >> On Tue, Jul 2, 2013 at 6:31 PM, Simon Horman <ho...@verge.net.au> wrote: > >> > On Tue, Jul 02, 2013 at 02:59:51PM -0700, Jesse Gross wrote: > >> >> On Mon, Jul 1, 2013 at 9:16 PM, Simon Horman <ho...@verge.net.au> wrote: > >> >> > On Mon, Jul 01, 2013 at 04:42:46PM -0700, Jesse Gross wrote: > >> >> >> On Wed, Jun 26, 2013 at 1:18 AM, Simon Horman <ho...@verge.net.au> > >> >> >> wrote: > >> >> >> > Allow datapath to recognize and extract MPLS labels into flow keys > >> >> >> > and execute actions which push, pop, and set labels on packets. > >> >> >> > > >> >> >> > Based heavily on work by Leo Alterman, Ravi K, Isaku Yamahata and > >> >> >> > Joe Stringer. > >> >> >> > > >> >> >> > Cc: Ravi K <rke...@gmail.com> > >> >> >> > Cc: Leo Alterman <lalter...@nicira.com> > >> >> >> > Cc: Isaku Yamahata <yamah...@valinux.co.jp> > >> >> >> > Cc: Joe Stringer <j...@wand.net.nz> > >> >> >> > Signed-off-by: Simon Horman <ho...@verge.net.au> > >> >> >> > >> >> >> Simon, have you thought any more about the header ordering issues? I > >> >> >> don't think we've reached a conclusion at this point. > >> >> > > >> >> > I believe that you then raised the issue of QinQ, which somehow > >> >> > I failed to respond to, I apologise for that. > >> >> > > >> >> > In particular, my understanding is that you are concerned the code > >> >> > will > >> >> > miss-calculate the end of L2 headers in the presence of multiple VLAN > >> >> > tags. > >> >> > Thus resulting in an MPLS push action inserting an MPLS LSE after the > >> >> > first > >> >> > rather than the last VLAN tag. And that would likely change if QinQ > >> >> > support > >> >> > was added to Open vSwtich. > >> >> > > >> >> > I wonder if a good way forwards is to enhance the calculation > >> >> > of the end of L2 headers (mac_len) and the beginning of L3 headers > >> >> > (network_header) in ovs_flow_extract() such that it takes into > >> >> > account the presence of more than one VLAN tag. > >> >> > >> >> The problem is that this requires being able to calculate the length > >> >> of all possible headers that we might want to support in the future. > >> >> In the case of QinQ, this would mean the various EtherTypes. You could > >> >> also imagine other things like MAC-in-MAC that are farther afield from > >> >> what we currently support. > >> > > >> > That is true. > >> > > >> > I think that the key problem is it that it is hard for us > >> > to correctly determine where the end of the L2 header is, > >> > or more specifically where the MPLS tag should go, for all cases. > >> > Particularly cases which are yet to be defined. > >> > > >> > Having spoken with Joe about this we see two main options: > >> > > >> > 1. The status quo as of this patch. Which is that MPLS actions > >> > may be invalid for some cases. > >> > > >> > While it should be be possible to solve individual cases, > >> > this doesn't solve the general case. > >> > > >> > 2. Only allow MPLS actions on ether types where the implementation > >> > is known to work. > >> > > >> > This could act as a white list of sorts. It could start with > >> > the obvious candidates: IPv4, IPv6, ARP, 802.1Q,... > >> > And support for more protocols could be added in the future. > >> > > >> > This would seem to reflect the somewhat special nature of MPLS. > >> > >> I think what is really necessary at the kernel level is some > >> flexibility about where to put the newly inserted MPLS header. Then > >> you could basically chose either of the two options above or export > >> the flexibility through OpenFlow (which by my reading of the spec is > >> already supposed to be allowed). Furthermore, you could do different > >> things in different situations as OpenFlow evolves, which really has > >> to be done at the userspace level since only it has the full set of > >> knowledge about the protocol. > > > > I wonder if this can be achieved by adding a list of features to > > the MPLS push action, for example as a possibly zero-length array of > > integers which encode feature bits. > > > > At the time that MPLS support is added to the datapath we could define that > > all the bits are zero and the behaviour of the code at that time is the > > expected behaviour. > > > > Suppose that a new feature is added in the future. I will use the example > > of support for 802.1ad (the standardised variant of Q-in-Q). > > > > The logic in the datapath to determine the end of the L2 header and thus > > the top of the MPLS LSE stack could be guarded by a new feature bit, > > the ad-bit. > > > > If an MPLS push action, supplied by userspace, has the ad-bit set then the > > new logic is used and the MPLS LSE is inserted accordingly. > > Conversely, if the MPLS push action does not have the bit set then the > > old logic is used and the MPLS LSE is inserted as if the datapath > > didn't understand 802.1ad. > > > > In this way it would be possible for userspace to select the desired > > behaviour. An old user-space would use the old behaviour. A new userspace > > may choose the old or the new behaviour. > > > > And it would be possible for the datapath to reject facets with MPLS > > push actions with feature bits or combinations of feature bits that > > are not supported. > > Hmm, I think that this may become fairly complicated over time as you > have a number of different types. > > Going back to the OpenFlow spec: > > "Newly pushed tags should always be inserted as the outermost tag in > the outermost valid location for > that tag. When a new VLAN tag is pushed, it should be the outermost > tag inserted, immediately after > the Ethernet header and before other tags. Likewise, when a new MPLS > tag is pushed, it should be the > outermost tag inserted, immediately after the Ethernet header and > before other tags. > > When multiple push actions are added to the action set of the packet, > they apply to the packet in the > order defined by the action set rules, first MPLS, then PBB, than VLAN > (se 5.10). When multiple push > actions are included in an action list, they apply to the packet in > the list order (see 5.11)." > > This seems about as flexible as anything that I can think of at the > moment and fairly straightforward: basically we wouldn't need to skip > over vlan tags at the beginning because we would just push tags in > front of them. If userspace wants them in the opposite order then it > can pop off the tags and put them back but I suspect that this is > actually quite uncommon.
Thanks. I agree that your proposed scheme should cover all the bases. In the case of OpenFlow 1.2 I think the spec is fairly clear that the MPLS label stack should follow any VLAN tags. So I now plan to add logic to pop off the VLAN tags and put them on, as you suggest above. > I know you mentioned before that the valid location for the MPLS label > is after the vlan tags but there are several ways to use MPLS and I > think the last line of the first paragraph is fairly clear. With that in mind I plan to have userspace use the order that you suggest, just adding the MPLS as the outer-most tag, in the case of OpenFlow 1.3. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
