[ovs-dev] [PATCH] [RFC] flow: Do not clear L3+ fields of flow in flow_push_mpls()

[Jarno Rajahalme]

On Mar 20, 2014, at 10:05 AM, Ben Pfaff <blp at nicira.com> wrote:

> On Fri, Mar 14, 2014 at 04:19:52PM +0900, Simon Horman wrote:
>> When creating a flow in the datapath as the result of an upcall
>> the match itself is the match supplied in the upcall while
>> the mask of the match, if supplied, is generated based on the
>> flow and mask composed during action translation.
>> 
>> In the case of, for example a UDP packet, the match will include
>> of L2, L3 and L4 fields. However, if the flow is cleared in
>> flow_push_mpls() then the mask that is synthesised from it will
>> not include L3 and L4 fields. This seems incorrect and the kernel
>> datapath complains about this mismatch.
>> 
>> Signed-off-by: Simon Horman <horms at verge.net.au>
> 
> The goal of clearing the fields is to ensure that later flow tables
> can't match on fields that aren't visible anymore.  That's important for
> accurate OpenFlow implementation, so I'd rather not change it.  On the
> other hand, I see the point you're making, but I don't immediately
> understand why it happens that way.  After all, I can change fields with
> OpenFlow actions and the datapath flows work out OK, why doesn't this
> work out OK too?  Do you understand the reason?

As the flow’s dl_type is changed to an MPLS type, later non-MPLS rules will not match on the modified flow. AFAIK, you can match on L3/L4 fields only by also matching on the corresponding dl_type as a prerequisite, no?

If this holds, I’d rather not clear the fields so we can properly do a set IPv4 action followed by an MPLS push action. Currently the the MPLS action clears the flow values at the translation time set in the preceding action, so that at the commit time the values intended for set IPv4 action are lost.

  Jarno