[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