Re: [PATCH v3] hash: add XOR32 hash function

["Medvedkin, Vladimir" <vladimir.medvedkin@intel.com>]

Hi Bill,

On 15/02/2023 11:06, Bili Dong wrote:
> An XOR32 hash is needed in the Software Switch (SWX) Pipeline for its
> use case in P4. We implement it in this patch so it could be easily
> registered in the pipeline later.
>
> Signed-off-by: Bili Dong <qobilidop@gmail.com>
> ---
> +static inline uint32_t
> +rte_hash_xor(const void *data, uint32_t data_len, uint32_t init_val)
> +{
> +	uint32_t i;
> +	uintptr_t pd = (uintptr_t) data;
> +	init_val = rte_cpu_to_be_32(init_val);
> +
> +	for (i = 0; i < data_len / 4; i++) {
> +		init_val ^= *(const uint32_t *)pd;
> +		pd += 4;
> +	}
> +
> +	if (data_len & 0x2) {
> +		init_val ^= *(const uint32_t *)pd & LEFT16b_MASK;

Here you are reading 2 bytes after the data buffer, which can sometimes 
lead to segfault. I think it would be better just to:

init_val ^= *(const uint16_t *)pd << 16;

The same with the section bellow

> +		pd += 2;
> +	}
> +
> +	if (data_len & 0x1)
> +		init_val ^= *(const uint32_t *)pd & LEFT8b_MASK;
> +
> +	init_val = rte_be_to_cpu_32(init_val);
> +	return init_val;
> +}
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* _RTE_HASH_XOR_H_ */

-- 
Regards,
Vladimir