On Dec 18, 2013, at 9:42 AM, Ben Pfaff <b...@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 <jrajaha...@nicira.com>
>
> Indentation looks odd to me--did you use tabs instead of spaces?
>
Yes, accidentally, sorry.
> Acked-by: Ben Pfaff <b...@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
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
