On Thu, Jul 11, 2013 at 6:55 PM, Simon Horman <ho...@verge.net.au> wrote: > 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.
Great, sounds good to me. X-CudaMail-Whitelist-To: dev@openvswitch.org _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
