On Wed, Dec 17, 2014 at 10:30:41AM -0800, Jarno Rajahalme wrote:
> Add support for adding 64-bit words to hashes. This will be used by
> subsequent patches.
>
> Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
I noticed that hash_words64() takes a 64-bit basis (which is not new).
That seems odd because it returns a 32-bit hash value. Usually the
basis is used for chaining together the hashes of several values, so
usually I expect the basis to be the same size as the hash (both
either 32 or 64 bits, for example).
Assuming that a 64-bit basis does make sense, would you mind adding
parentheses here in hash_words64_inline():
> + hash = basis ^ basis >> 32;
so that it becomes:
hash = basis ^ (basis >> 32);
Also, again assuming that a 64-bit basis does makes sense, if the
provided basis is really a hash, then a plain XOR as above should be
OK, but if it's some other value provided by the caller, without
necessarily high or evenly distributed entropy, as we occasionally
use, then a better hash function than XOR might be useful.
> + for (i = 0; i < n_words; i++) {
> + hash = hash_add64(hash, p[i]);
> + }
> + return hash_finish(hash, n_words * 8);
> }
Other than that philosophical issue:
Acked-by: Ben Pfaff <b...@nicira.com>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
