[ovs-dev] [RFC PATCH v2 2/2] Fix setting transport ports with frags.

Jarno Rajahalme jrajahalme at nicira.com
Mon Nov 10 21:42:31 UTC 2014


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

> On Wed, Nov 05, 2014 at 10:37:19AM -0800, Jarno Rajahalme wrote:
>> Packets with 'LATER' fragment do not have a transport header, so it is
>> not possible to either match on or set transport ports on such
>> packets.  Matching is prevented by augmenting mf_are_prereqs_ok() with
>> a nw_frag 'LATER' bit check.  Setting the transport headers on such
>> packets is prevented in three ways:
>> 
>> 1. Flows with an explicit match on nw_frag, where the LATER bit is 1:
>>   existing calls to the modified mf_are_prereqs_ok() prohibit using
>>   transport header fields (port numbers) in OXM/NXM actions
>>   (set_field, move).  SET_TP_* actions need a new check on the LATER
>>   bit.
>> 
>> 2. Flows that wildcard the nw_frag LATER bit: At flow translation
>>   time, add calls to mf_are_prereqs_ok() to make sure that we do not
>>   use transport ports in flows that do not have them.
>> 
>> 3. At action execution time, do not set transport ports, if the packet
>>   does not have a full transport header.  This ensures that we never
>>   call the packet_set functions, that require a valid transport
>>   header, with packets that do not have them.  For example, if the
>>   flow was created with a IPv6 first fragment that had the full TCP
>>   header, but the next packet's first fragment is missing them.
>> 
>> 3 alone would suffice for correct behavior, but 1 and 2 seem like a
>> right thing to do, anyway.
>> 
>> Currently, if we are setting port numbers, we will also match them,
>> due to us tracking the set fields with the same flow_wildcards as the
>> matched fields.  Hence, if the incoming port number was not zero, the
>> flow would not match any packets with missing or truncated transport
>> headers.  However, relying on no packets having zero port numbers
>> would not be very robust.  Also, we may separate the tracking of set
>> and matched fields in the future, which would allow some flows that
>> blindly set port numbers to not match on them at all.
>> 
>> For TCP in case 3 we use ofpbuf_get_tcp_payload() that requires the
>> whole (potentially variable size) TCP header to be present.  However,
>> when parsing a flow, we only require the fixed size portion of the TCP
>> header to be present, which would be enough to set the port numbers
>> and fix the TCP checksum.
>> 
>> Finally, we add tests testing the new behavior.
>> 
>> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
> 
> Acked-by: Ben Pfaff <blp at nicira.com>


Thanks for the review!

Pushed to master,

  Jarno


More information about the dev mailing list