Re: [PATCH v6] lib/hash: add siphash

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

Hi Stephen,

Thanks for introducing this hash function.

I have just a few nits:

On 01/08/2024 16:31, Stephen Hemminger wrote:
> The existing hash functions in DPDK are not cryptographically
> secure and can be subject to carefully crafted packets causing
> DoS attack.
Currently in DPDK we have 3 hash functions, 2 of them can be used with 
our cuckoo hash table implementation:

1. CRC - Very weak, do not use with hash table if you don't fully 
control all keys to install into a hash table.

2. Toeplitz - keyed hash function, not used with hash tables, fastest if 
you have GFNI, level of diffusion fully depends on the hash key, weak 
against differential crypto analysis. Technically may be used with hash 
tables in number of usecases.

3. Jenkins hash (lookup3) - and here I can not say that it is not secure 
and it is subject to collisions. I'm not aware on any successful attacks 
on it, it has a great diffusion (see https://doi.org/10.1002/spe.2179). 
It is also keyed with the same size of the key as rte_hsiphash().

So I won't agree with this sentence.

>
> Add SipHash which is a fast and cryptographicly sound hash
> created by Jean-Philippe Aumasson and Daniel J. Bernstein.
> Siphash is widely used by Linux, FreeBSD, OpenBSD and other
> projects because it is fast and resistant to DoS attacks.
> This version is designed to be useful as alternative hash
> with cuckoo hash in DPDK.
>
> Implementation is based of the public domain and Creative
> Common license reference version as well as some optimizations
> from the GPL-2 or BSD-3 licensed version in Linux.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> v6 - rebase to 24.07
>
>   app/test/test_hash_functions.c |  75 +++++++++++++-
>   lib/hash/meson.build           |   2 +
>   lib/hash/rte_siphash.c         | 176 +++++++++++++++++++++++++++++++++
>   lib/hash/rte_siphash.h         |  87 ++++++++++++++++
>   lib/hash/version.map           |   4 +
>   5 files changed, 340 insertions(+), 4 deletions(-)
>   create mode 100644 lib/hash/rte_siphash.c
>   create mode 100644 lib/hash/rte_siphash.h
<snip>
> +	return (v0 ^ v1) ^ (v2 ^ v3);
do we need parentheses here? XOR is associative and commutative, the 
compiler must figure out by itself in which order it will be better to 
add them together

<snip>

Also please add release notes.

Apart from it - LGTM.

Acked-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>

-- 
Regards,
Vladimir