[ovs-dev] [PATCH v2 16/26] meta-flow: Clean up masking with prerequisities checking.

[Jarno Rajahalme]

> On Jul 29, 2016, at 3:54 PM, Ben Pfaff <blp at 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 <jarno at 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 <blp at ovn.org>

Thanks for the review,

  Jarno