[ovs-dev] [PATCH] lib/flow: Skip minimask value checks.

[Jarno Rajahalme]

On Dec 18, 2013, at 9:42 AM, Ben Pfaff <blp at nicira.com> wrote:

> On Fri, Dec 13, 2013 at 02:37:56PM -0800, Jarno Rajahalme wrote:
>> We allow zero 'values' in a miniflow for it to have the same map
>> as the corresponding minimask.  Minimasks themselves never have
>> zero data values, though.  Document this and optimize the code
>> accordingly.
>> 
>> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
> 
> Indentation looks odd to me--did you use tabs instead of spaces?
> 

Yes, accidentally, sorry.

> Acked-by: Ben Pfaff <blp at nicira.com>
> 
> Here is an alternate concept for minimatch_hash_range() that might be
> easier to understand.  I did not compile it or test it.
> 

This is clearer, thanks. Small twist at the end though:

> uint32_t
> minimatch_hash_range(const struct minimatch *match, uint8_t start, uint8_t end,
>                     uint32_t *basis)
> {
>    const uint32_t *p, *q;
>    uint32_t hash = *basis;
>    int n, i;
> 
>    n = count_1bits(miniflow_get_map_in_range(&match->mask.masks,
>                                              start, end, &p));
>    q = p + (match->flow.values - match->mask.masks.values);
>    for (i = 0; i < n; i++) {
>        hash = mhash_add(hash, p[i] & q[i]);
>    }
>    *basis = hash; /* Allow continuation from the unfinished value. */
>    return mhash_finish(hash, n * 4);

’n’ does not work, as the hash needs to be the same regardless in how many stages it is computed. flow_hash_in_minimask() uses the byte length of the values, so we must add the number of bytes hashed in previous stages. I’ve adjusted this accordingly, and also changed the miniflow_get_map_in_range to return the offset instead via the last parameter, so that we also get rid of the pointer arithmetic above.

v2 follows shortly,

  Jarno