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

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Bili Dong [mailto:qobilidop@gmail.com]
> Sent: Wednesday, 15 February 2023 12.07
> 
> 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>
> ---

[...]

> +#define LEFT8b_MASK rte_cpu_to_be_32(0xff000000)
> +#define LEFT16b_MASK rte_cpu_to_be_32(0xffff0000)
> +
> +/**
> + * Calculate XOR32 hash on user-supplied byte array.
> + *
> + * @param data
> + *   Data to perform hash on.
> + * @param data_len
> + *   How many bytes to use to calculate hash value.
> + * @param init_val
> + *   Value to initialise hash generator.
> + * @return
> + *   32bit calculated hash value.
> + */
> +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;
> +		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;
> +}

I think that this function has swapped big endian and CPU endian everywhere. The result is the same, but the code would be much less confusing if using rte_cpu_32_to_be() when converting from CPU endian to big endian, and rte_be_to_cpu_32() when converting the other way.

I also suppose that the return type and the init_val parameter were meant to be rte_be32_t.

Also, please document that the byte array must be 32 bit aligned. Alternatively, implement support for unaligned data. You can find inspiration for handling of unaligned data in the __rte_raw_cksum() function:
https://elixir.bootlin.com/dpdk/v22.11.1/source/lib/net/rte_ip.h#L162