[ovs-dev] [PATCH 3/3] Fix setting transport ports with frags. Packets with 'LATER' fragment do not have a transport header, so it is not possible to set them. Setting the transport headers on such packets is prevented in two ways:

[Jarno Rajahalme]

On Nov 3, 2014, at 2:03 PM, Ben Pfaff <blp at nicira.com> wrote:

> On Wed, Oct 29, 2014 at 03:12:02PM -0700, Jarno Rajahalme wrote:
>> 1. Flows with an explicit match on nw_frag, where the LATER bit is 1:
>>   Prohibit setting transport header fields (port numbers) with
>>   set_field or move, or using such a field as a source in a move.
>> 
>> 2. Flows that wildcard the nw_frag LATER bit: At flow translation
>>   time, detect the fact that the packet/flow has no transport header,
>>   and (silently) do nothing when translating a set_field, set_tp_src,
>>   set_tp_dst, or reg_move action that reads or writes on transport
>>   headers.  nw_frag is exact matched, so non-LATER packets deal with
>>   the transport ports as before.
>> 
>> 2. alone would suffice for correct behavior, but 1. seems like a right
>> thing to do, anyway.
>> 
>> Finally, we add tests testing the new behavior.
>> 
>> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
> 
> The subject line is much too long.  Break it after the first sentence?
> 

Oops. Will do.

> I think that this new constraint is really just a refinement of the
> prerequisites for MFP_TCP, MFP_UDP, and MFP_SCTP, so I would expect
> mf_are_prereqs_ok() to change instead of mf_check__().
> 

OK.

> In IPv6, even the first fragment is not guaranteed to carry the
> transport port.  Have you had any thoughts about how to handle IPv6?

Not yet :-) Looking at the documentation of OFPC_FRAG_NX_MATCH:

     *  - OFPC_FRAG_NX_MATCH (a Nicira extension): Similar to OFPC_FRAG_NORMAL,
     *    except that TCP and UDP port numbers should be included in fragments
     *    with offset 0.

It seems that the only way to implement this mode with IPv6 is to drop fragments of TCP/UDP/SCTP packet that have 0 offset, but do not contain the transport header port numbers. I’m not aware of legitimate traffic that would be harmed by this.

Thoughts?

  Jarno