On Thu, Dec 19, 2013 at 01:01:39PM -0800, Jarno Rajahalme wrote:
>
> 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.
Thanks. That sounds good.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
