[ovs-dev] [PATCH 3/3] datapath: Add basic MPLS support to kernel

[Simon Horman]

On Mon, Mar 11, 2013 at 10:00:54AM -0700, Jesse Gross wrote:
> On Mon, Mar 11, 2013 at 6:02 AM, Rajahalme, Jarno (NSN - FI/Espoo)
> <jarno.rajahalme at nsn.com> wrote:
> >
> > On Mar 9, 2013, at 16:54 , ext Simon Horman wrote:
> >
> >> It is not obvious to me how the behaviour relating to get_priority() as
> >> described above can sensibly be reconciled with a desire to preserve the
> >> order of actions. Or even reconciled with a looser constraint that actions
> >> that need l3+ data occur before push_mpls actions.
> >
> > Maybe do_xlate_actions() should take care of translating the skb_priority to nw_tos, and make sure that it happens before any L2.5 actions are translated. This would make any later set actions on skb_priority ineffective, though. Also, it seems that on the patch port output the nw_tos is not set, so that might require some further thought. Maybe it is of no functional use for the network transport, but still the peer might actually depend (via a match) on the nw_tos being set the same way as any other port might have it?
> 
> I agree that simply moving the setting of nw_tos earlier in the
> process is the best way to handle this.  A similar problem occurs if
> an MPLS (or any other non-IP) packet is received on the wire -
> changing the priority will have no effect on the marking.  Since
> pushing MPLS labels and setting the priority are both done by the
> controller, this behavior should at least be fairly transparent and
> not surprising.

Although it seems like an invasive change I can see how it may be possible
for ovs-vwitchd to clone the packet and emit multiple execute operations, one
per output port.

However, I'm not sure I see how an appropriate put operation can be emitted
to the datapath using this scheme as, recirculation aside, there can only
be one facet per match.

I'm also a little dubious about the complexity of this change vs the
objective of not adding an element to the skb callback.

> As far the as marking when using patch ports, that just seems like a
> bug to me.  Ethan?