On Tue, May 23, 2023 at 10:25:01PM +0000, Eric Biggers wrote: > On Tue, May 23, 2023 at 06:13:08PM -0400, corwin wrote: > > On 5/23/23 6:06 PM, Eric Biggers wrote: > > > On Tue, May 23, 2023 at 05:45:02PM -0400, J. corwin Coburn wrote: > > > > MurmurHash3 is a fast, non-cryptographic, 128-bit hash. It was originally > > > > written by Austin Appleby and placed in the public domain. This version has > > > > been modified to produce the same result on both big endian and little > > > > endian processors, making it suitable for use in portable persistent data. > > > > > > > > Signed-off-by: J. corwin Coburn <corwin@xxxxxxxxxx> > > > > --- > > > > drivers/md/dm-vdo/murmurhash3.c | 175 ++++++++++++++++++++++++++++++++ > > > > drivers/md/dm-vdo/murmurhash3.h | 15 +++ > > > > 2 files changed, 190 insertions(+) > > > > create mode 100644 drivers/md/dm-vdo/murmurhash3.c > > > > create mode 100644 drivers/md/dm-vdo/murmurhash3.h > > > > > > Do we really need yet another hash algorithm? > > > > > > xxHash is another very fast non-cryptographic hash algorithm that is already > > > available in the kernel (lib/xxhash.c). > > > > > > - Eric > > > > The main reason why vdo uses Murmur3 and not xxHash is that vdo has been in > > deployment since 2013, and, if I am reading correctly, xxHash did not have a > > 128 bit variant until 2019. > > Why do you need a 128-bit non-cryptographic hash algorithm? What problem are > you trying to solve? To elaborate a bit: the reason this seems strange to me is that when people say they need a 128-bit (or larger) non-cryptographic hash function, usually they are either mistaken and 64-bit would suffice, or they actually need a cryptographic hash function. In this case, the hash function seems to be used for data deduplication. This is unusual, since data deduplication normally requires a cryptographic hash algorithm. I see that this is touched on by the following paragraph in vdo-design.rst (though it incorrectly calls the hash an "identifier for the block"): Each block of data is hashed to produce a 16-byte block name which serves as an identifier for the block. An index record consists of this block name paired with the presumed location of that data on the underlying storage. However, it is not possible to guarantee that the index is accurate. Most often, this occurs because it is too costly to update the index when a block is over-written or discarded. Doing so would require either storing the block name along with the blocks, which is difficult to do efficiently in block-based storage, or reading and rehashing each block before overwriting it. Inaccuracy can also result from a hash collision where two different blocks have the same name. In practice, this is extremely unlikely, but because vdo does not use a cryptographic hash, a malicious workload can be constructed. Because of these inaccuracies, vdo treats the locations in the index as hints, and reads each indicated block to verify that it is indeed a duplicate before sharing the existing block with a new one. So, dm-vdo handles hash collisions by reading back and verifying that the data matches before allowing deduplication. That solves the security problem. However, it does seem strange, and it's more complex than the usual solution of just using a cryptographic hash. Note that cryptographic hashing is very fast on modern CPUs with modern algorithms. So, some more details about the rationale for designing the data deduplication in this (IMO unusual) way should be included. - Eric