Re: [PATCH v2 3/4] hash: reduce architecture special cases

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 26 Aug 2025 16:36:38 +0200
Mattias Rönnblom <hofors@lysator.liu.se> wrote:

> On 2025-08-22 20:19, Stephen Hemminger wrote:
> > Make comparison of sizes compatible across platforms.
> > Keep the special case code for 16 bytes for x86 and arm64 but
> > also add simple xor for others.
> > 
> > Need to keep rte_hash_k32_cmp_eq() exposed because ip_frag
> > code poaches it.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> >   lib/hash/rte_cmp_arm64.h   | 56 +------------------------
> >   lib/hash/rte_cmp_generic.h | 35 ++++++++++++++++
> >   lib/hash/rte_cmp_x86.h     | 60 ++------------------------
> >   lib/hash/rte_cuckoo_hash.c | 86 +++++++++++++++++++++++++++++++++-----
> >   4 files changed, 116 insertions(+), 121 deletions(-)
> >   create mode 100644 lib/hash/rte_cmp_generic.h
> > 
> > diff --git a/lib/hash/rte_cmp_arm64.h b/lib/hash/rte_cmp_arm64.h
> > index a3e85635eb..2b2a37ebd2 100644
> > --- a/lib/hash/rte_cmp_arm64.h
> > +++ b/lib/hash/rte_cmp_arm64.h
> > @@ -2,7 +2,7 @@
> >    * Copyright(c) 2015 Cavium, Inc
> >    */
> >   
> > -/* Functions to compare multiple of 16 byte keys (up to 128 bytes) */
> > +/* Functions to compare multiple of 16 byte keys */
> >   static inline int
> >   rte_hash_k16_cmp_eq(const void *key1, const void *key2,
> >   		    size_t key_len __rte_unused)
> > @@ -27,59 +27,7 @@ rte_hash_k16_cmp_eq(const void *key1, const void *key2,
> >   static inline int
> >   rte_hash_k32_cmp_eq(const void *key1, const void *key2, size_t key_len)
> >   {
> > -	return rte_hash_k16_cmp_eq(key1, key2, key_len) ||
> > +	return rte_hash_k16_cmp_eq(key1, key2, key_len) |
> >   		rte_hash_k16_cmp_eq((const char *) key1 + 16,
> >   				(const char *) key2 + 16, key_len);
> >   }
> > -
> > -static inline int
> > -rte_hash_k48_cmp_eq(const void *key1, const void *key2, size_t key_len)
> > -{
> > -	return rte_hash_k16_cmp_eq(key1, key2, key_len) ||
> > -		rte_hash_k16_cmp_eq((const char *) key1 + 16,
> > -				(const char *) key2 + 16, key_len) ||
> > -		rte_hash_k16_cmp_eq((const char *) key1 + 32,
> > -				(const char *) key2 + 32, key_len);
> > -}
> > -
> > -static inline int
> > -rte_hash_k64_cmp_eq(const void *key1, const void *key2, size_t key_len)
> > -{
> > -	return rte_hash_k32_cmp_eq(key1, key2, key_len) ||
> > -		rte_hash_k32_cmp_eq((const char *) key1 + 32,
> > -				(const char *) key2 + 32, key_len);
> > -}
> > -
> > -static inline int
> > -rte_hash_k80_cmp_eq(const void *key1, const void *key2, size_t key_len)
> > -{
> > -	return rte_hash_k64_cmp_eq(key1, key2, key_len) ||
> > -		rte_hash_k16_cmp_eq((const char *) key1 + 64,
> > -				(const char *) key2 + 64, key_len);
> > -}
> > -
> > -static inline int
> > -rte_hash_k96_cmp_eq(const void *key1, const void *key2, size_t key_len)
> > -{
> > -	return rte_hash_k64_cmp_eq(key1, key2, key_len) ||
> > -		rte_hash_k32_cmp_eq((const char *) key1 + 64,
> > -				(const char *) key2 + 64, key_len);
> > -}
> > -
> > -static inline int
> > -rte_hash_k112_cmp_eq(const void *key1, const void *key2, size_t key_len)
> > -{
> > -	return rte_hash_k64_cmp_eq(key1, key2, key_len) ||
> > -		rte_hash_k32_cmp_eq((const char *) key1 + 64,
> > -				(const char *) key2 + 64, key_len) ||
> > -		rte_hash_k16_cmp_eq((const char *) key1 + 96,
> > -				(const char *) key2 + 96, key_len);
> > -}
> > -
> > -static inline int
> > -rte_hash_k128_cmp_eq(const void *key1, const void *key2, size_t key_len)
> > -{
> > -	return rte_hash_k64_cmp_eq(key1, key2, key_len) ||
> > -		rte_hash_k64_cmp_eq((const char *) key1 + 64,
> > -				(const char *) key2 + 64, key_len);
> > -}
> > diff --git a/lib/hash/rte_cmp_generic.h b/lib/hash/rte_cmp_generic.h
> > new file mode 100644
> > index 0000000000..f846d562e3
> > --- /dev/null
> > +++ b/lib/hash/rte_cmp_generic.h
> > @@ -0,0 +1,35 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2025 Stephen Hemminger
> > + */
> > +
> > +#ifndef _RTE_CMP_GENERIC_H_
> > +#define _RTE_CMP_GENERIC_H_
> > +
> > +/* Function to compare 16 byte keys */
> > +static inline int
> > +rte_hash_k16_cmp_eq(const void *key1, const void *key2, size_t key_len __rte_unused)
> > +{
> > +#ifdef RTE_ARCH_64
> > +	const uint64_t *k1 = key1;
> > +	const unaligned_uint64_t *k2 = key2;
> > +
> > +	return ((k1[0] ^ k2[0]) | (k1[1] ^ k2[1])) != 0;  
> 
> Remove '!= 0'.
> 
> > +#else
> > +	const uint32_t *k1 = key1;
> > +	const unaligned_uint32_t *k2 = key2;
> > +
> > +	return (k1[0] ^ k2[0]) | (k1[1] ^ k2[1]) |
> > +	       (k1[2] ^ k2[2]) | (k1[3] ^ k2[3]);
> > +#endif
> > +}
> > +
> > +/* Function to compare 32 byte keys */
> > +static inline int
> > +rte_hash_k32_cmp_eq(const void *key1, const void *key2, size_t key_len)  
> 
> eq -> neq not worth the trouble. Correct? Some other patch set.

ipfrag is still poaching on these, so not changing