On Nov 6, 2014, at 3:21 PM, Ben Pfaff <b...@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 <jrajaha...@nicira.com> > > Acked-by: Ben Pfaff <b...@nicira.com>
Thanks for the review! Pushed to master, Jarno _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
