[ovs-dev] [murmurhash 3/4] hash: Change mhash_finish() data measurement from words to bytes.

Ethan Jackson ethan at nicira.com
Tue Jan 22 21:47:51 UTC 2013


It seems low risk to me.  I'd like it to be there in case we end up
backporting something important which relies on mhash_finish().

Ethan

On Tue, Jan 22, 2013 at 1:44 PM, Ben Pfaff <blp at nicira.com> wrote:
> Thanks.  I pushed these to master.
>
> Do you think I should apply "hash: Correct implementation of
> mhash_finish()." to older branches?
>
> On Tue, Jan 22, 2013 at 01:37:07PM -0800, Ethan Jackson wrote:
>> Acked-by: Ethan Jackson <ethan at nicira.com>
>>
>> Sorry it took a while to get around to this.  Looks good, thanks.
>>
>> Ethan
>>
>> On Fri, Dec 14, 2012 at 4:33 PM, Ben Pfaff <blp at nicira.com> wrote:
>> > murmurhash includes an xor with the number of bytes hashed in its finishing
>> > step.  Until now, we've only had murmurhash for full words, but an upcoming
>> > commit adds murmurhash for bytes, so the finishing function will need to
>> > take a count of bytes instead.
>> >
>> > Signed-off-by: Ben Pfaff <blp at nicira.com>
>> > ---
>> >  lib/flow.c |    4 ++--
>> >  lib/hash.c |    2 +-
>> >  lib/hash.h |    4 ++--
>> >  3 files changed, 5 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/lib/flow.c b/lib/flow.c
>> > index ce9e6c6..89488e5 100644
>> > --- a/lib/flow.c
>> > +++ b/lib/flow.c
>> > @@ -1125,7 +1125,7 @@ miniflow_hash_in_minimask(const struct miniflow *flow,
>> >          }
>> >      }
>> >
>> > -    return mhash_finish(hash, p - mask->masks.values);
>> > +    return mhash_finish(hash, (p - mask->masks.values) * 4);
>> >  }
>> >
>> >  /* Returns a hash value for the bits of 'flow' where there are 1-bits in
>> > @@ -1154,7 +1154,7 @@ flow_hash_in_minimask(const struct flow *flow, const struct minimask *mask,
>> >          }
>> >      }
>> >
>> > -    return mhash_finish(hash, p - mask->masks.values);
>> > +    return mhash_finish(hash, (p - mask->masks.values) * 4);
>> >  }
>> >
>> >  /* Initializes 'dst' as a copy of 'src'.  The caller must eventually free 'dst'
>> > diff --git a/lib/hash.c b/lib/hash.c
>> > index 41ad1bf..8cee5d0 100644
>> > --- a/lib/hash.c
>> > +++ b/lib/hash.c
>> > @@ -115,5 +115,5 @@ mhash_words(const uint32_t p[], size_t n_words, uint32_t basis)
>> >      for (i = 0; i < n_words; i++) {
>> >          hash = mhash_add(hash, p[i]);
>> >      }
>> > -    return mhash_finish(hash, n_words);
>> > +    return mhash_finish(hash, n_words * 4);
>> >  }
>> > diff --git a/lib/hash.h b/lib/hash.h
>> > index 96866c4..d33924f 100644
>> > --- a/lib/hash.h
>> > +++ b/lib/hash.h
>> > @@ -149,9 +149,9 @@ static inline uint32_t mhash_add(uint32_t hash, uint32_t data)
>> >      return hash * 5 + 0xe6546b64;
>> >  }
>> >
>> > -static inline uint32_t mhash_finish(uint32_t hash, size_t n)
>> > +static inline uint32_t mhash_finish(uint32_t hash, size_t n_bytes)
>> >  {
>> > -    hash ^= n * 4;
>> > +    hash ^= n_bytes;
>> >      hash ^= hash >> 16;
>> >      hash *= 0x85ebca6b;
>> >      hash ^= hash >> 13;
>> > --
>> > 1.7.2.5
>> >
>> > _______________________________________________
>> > dev mailing list
>> > dev at openvswitch.org
>> > http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list