> On Jul 29, 2016, at 3:54 PM, Ben Pfaff <b...@ovn.org> wrote: > > On Thu, Jul 28, 2016 at 05:56:08PM -0700, Jarno Rajahalme wrote: >> Change mf_are_prereqs_ok() take a flow_wildcards pointer, so that the >> wildcards can be set at the same time as the prerequisiteis are >> checked. This makes it easier to write more obviously correct code. >> >> Remove the functions mf_mask_field_and_prereqs() and >> mf_mask_field_and_prereqs__(), and make the callers first check the >> prerequisites, while supplying 'wc' to mf_are_prereqs_ok(), and if >> successful, mask the bits of the field that were read or set using >> mf_mask_field_masked(). >> >> Signed-off-by: Jarno Rajahalme <ja...@ovn.org> > > Some of these new functions are very similar! How about: >
Done. Should think when copy&pasting :-) > ... > I don't know what the comment about nw_frag means here, do you? > /* A flow may wildcard nw_frag. Do nothing if the packet does not have > * the fields. */ > or even the older one: > /* A flow may wildcard nw_frag. Do nothing if setting a transport > * header field on a packet that does not have them. */ > The original comment means that even if the packet is a TCP packet, it may not have the transport port fields, and we may not know this at flow setup time, as nw_frag is typically wildcarded. mf_are_prereqs_ok() checks both that the packet is of the correct type, and that ports are not accessed on a later fragment, and we have to use it also at flow translation time, rather than just when the flow is set up. I changed this comment to simply: "Check that the fields exist.". > Acked-by: Ben Pfaff <b...@ovn.org> Thanks for the review, Jarno _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
