Re: [PATCH v2.34] datapath: Add basic MPLS support to kernel

On Thu, Jul 11, 2013 at 6:55 PM, Simon Horman [off-list ref] wrote:

On Thu, Jul 11, 2013 at 06:26:32PM -0700, Jesse Gross wrote:

quoted

On Wed, Jul 10, 2013 at 5:16 PM, Simon Horman [off-list ref] wrote:

quoted

On Mon, Jul 08, 2013 at 11:51:18PM -0700, Jesse Gross wrote:

quoted

On Tue, Jul 2, 2013 at 6:31 PM, Simon Horman [off-list ref] wrote:

quoted

On Tue, Jul 02, 2013 at 02:59:51PM -0700, Jesse Gross wrote:

quoted

On Mon, Jul 1, 2013 at 9:16 PM, Simon Horman [off-list ref] wrote:

quoted

On Mon, Jul 01, 2013 at 04:42:46PM -0700, Jesse Gross wrote:

quoted

On Wed, Jun 26, 2013 at 1:18 AM, Simon Horman [off-list ref] wrote:

quoted

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 <redacted>
Cc: Leo Alterman <redacted>
Cc: Isaku Yamahata <redacted>
Cc: Joe Stringer <redacted>
Signed-off-by: Simon Horman <horms@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.

quoted

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