netdev - Re: [RFC PATCH net-next 1/3] net: ethtool: add symmetric Toeplitz RSS hash function

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230824111455.686e98b4@kernel.org>
Date: Thu, 24 Aug 2023 11:14:55 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Ahmed Zaki <ahmed.zaki@...el.com>
Cc: netdev@...r.kernel.org, jesse.brandeburg@...el.com,
 anthony.l.nguyen@...el.com, Willem de Bruijn
 <willemdebruijn.kernel@...il.com>
Subject: Re: [RFC PATCH net-next 1/3] net: ethtool: add symmetric Toeplitz
 RSS hash function

CC Willem

On Wed, 23 Aug 2023 10:48:29 -0600 Ahmed Zaki wrote:
> Symmetric RSS hash functions are beneficial in applications that monitor
> both Tx and Rx packets of the same flow (IDS, software firewalls, ..etc).
> Getting all traffic of the same flow on the same RX queue results in
> higher CPU cache efficiency.
> 
> Allow ethtool to support symmetric Toeplitz algorithm. A user can set the
> RSS function of the netdevice via:
>     # ethtool -X eth0 hfunc symmetric_toeplitz

Looks fairly reasonable, but there are two questions we need to answer:
 - what do we do if RXH config includes fields which are by definition
   not symmetric (l2 DA or in the future flow label)?
 - my initial thought was the same as Saeed's - that the fields are
   sorted, so how do we inform user about the exact implementation?

One way to fix both problems would be to, instead of changing the hash
function, change the RXH config. Add new "xor-ed" fields there.

Another would be to name the function "XORSYM_TOP" and make the core
check that it cannot be combined with uni-dir fields?

I like the first option more.

Either way, please make sure to add docs, and extend the toeplitz test
for this.

> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 62b61527bcc4..9a8e1fb7170d 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -60,10 +60,11 @@ enum {
>  	ETH_RSS_HASH_TOP_BIT, /* Configurable RSS hash function - Toeplitz */
>  	ETH_RSS_HASH_XOR_BIT, /* Configurable RSS hash function - Xor */
>  	ETH_RSS_HASH_CRC32_BIT, /* Configurable RSS hash function - Crc32 */
> +	ETH_RSS_HASH_SYM_TOP_BIT, /* Configurable RSS hash function - Symmetric Toeplitz */
>  
>  	/*
>  	 * Add your fresh new hash function bits above and remember to update
> -	 * rss_hash_func_strings[] in ethtool.c
> +	 * rss_hash_func_strings[] in ethtool/common.c
>  	 */
>  	ETH_RSS_HASH_FUNCS_COUNT
>  };
> @@ -108,6 +109,7 @@ enum ethtool_supported_ring_param {
>  #define __ETH_RSS_HASH(name)	__ETH_RSS_HASH_BIT(ETH_RSS_HASH_##name##_BIT)
>  
>  #define ETH_RSS_HASH_TOP	__ETH_RSS_HASH(TOP)
> +#define ETH_RSS_HASH_SYM_TOP	__ETH_RSS_HASH(SYM_TOP)
>  #define ETH_RSS_HASH_XOR	__ETH_RSS_HASH(XOR)
>  #define ETH_RSS_HASH_CRC32	__ETH_RSS_HASH(CRC32)
>  
> diff --git a/net/ethtool/common.c b/net/ethtool/common.c
> index f5598c5f50de..a0e0c6b2980e 100644
> --- a/net/ethtool/common.c
> +++ b/net/ethtool/common.c
> @@ -81,6 +81,7 @@ rss_hash_func_strings[ETH_RSS_HASH_FUNCS_COUNT][ETH_GSTRING_LEN] = {
>  	[ETH_RSS_HASH_TOP_BIT] =	"toeplitz",
>  	[ETH_RSS_HASH_XOR_BIT] =	"xor",
>  	[ETH_RSS_HASH_CRC32_BIT] =	"crc32",
> +	[ETH_RSS_HASH_SYM_TOP_BIT] =	"symmetric_toeplitz",
>  };
>  
>  const char