On Tue, Jun 24, 2014 at 04:24:37PM -0700, Jesse Gross wrote:

On Tue, Jun 24, 2014 at 4:56 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>

Applied, thanks for all your work. Time to break out the champagne :)

Thanks, a moment for many to savour.

That being said, I do have a couple of comments for follow up discussion:
 * I removed the addition of MPLS to key_attr_size. This is trying to
calculate the maximum key size and since MPLS will never show up in
conjunction with IPv6 (the current longest key) and isn't longer than
it, MPLS shouldn't increase the maximum size.

Thanks, sorry for messing that up.

 * I think there is a potential for MPLS packets to be incorrectly
offloaded on kernels 3.12-3.15 when encapsulated inside a tunnel. This
is because it won't hit either the OVS GSO code or your enhanced MPLS
feature check. I don't want to lose GSO for tunnels on those kernels
but maybe there is a way to avoid this potential problem without too
much work.

I'll take a look into this unless someone else wants to jump in.

 * Maybe you can refresh my memory - in the case of a push_mpls after
pop_vlan, why can't we do this check correctly for at least one level
of vlan? It seems like we could use the inner EtherType to tell
whether an additional vlan tag is present.

That seems likely. I'll do some analysis and get back to you.