netdev - Re: [PATCH net-next v4 1/2] net: libwx: support multiple RSS for every pool

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: <20250915180133.2af67344@kernel.org>
Date: Mon, 15 Sep 2025 18:01:33 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Jiawen Wu <jiawenwu@...stnetic.com>
Cc: netdev@...r.kernel.org, Andrew Lunn <andrew+netdev@...n.ch>, "David S.
 Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Paolo
 Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, Alexander
 Lobakin <aleksander.lobakin@...el.com>, Mengyuan Lou
 <mengyuanlou@...-swift.com>
Subject: Re: [PATCH net-next v4 1/2] net: libwx: support multiple RSS for
 every pool

On Fri, 12 Sep 2025 14:23:56 +0800 Jiawen Wu wrote:
> Subject: [PATCH net-next v4 1/2] net: libwx: support multiple RSS for every pool

"support multiple RSS" needs an object. Multiple RSS keys? Multiple
contexts? Multiple tables?

> -static void wx_store_reta(struct wx *wx)
> +u32 wx_rss_indir_tbl_entries(struct wx *wx)
>  {
> +	if (test_bit(WX_FLAG_SRIOV_ENABLED, wx->flags))
> +		return 64;
> +	else
> +		return 128;
> +}

Is WX_FLAG_SRIOV_ENABLED set only when VFs are created?
What if the user set a table with 128 entries?
The RSS table can't shrink once intentionally set to a specific size.

> +void wx_store_reta(struct wx *wx)
> +{
> +	u32 reta_entries = wx_rss_indir_tbl_entries(wx);
>  	u8 *indir_tbl = wx->rss_indir_tbl;
>  	u32 reta = 0;
>  	u32 i;
> @@ -2007,36 +2016,55 @@ static void wx_store_reta(struct wx *wx)
>  	/* Fill out the redirection table as follows:
>  	 *  - 8 bit wide entries containing 4 bit RSS index
>  	 */
> -	for (i = 0; i < WX_MAX_RETA_ENTRIES; i++) {
> +	for (i = 0; i < reta_entries; i++) {
>  		reta |= indir_tbl[i] << (i & 0x3) * 8;
>  		if ((i & 3) == 3) {
> -			wr32(wx, WX_RDB_RSSTBL(i >> 2), reta);
> +			if (test_bit(WX_FLAG_SRIOV_ENABLED, wx->flags) &&
> +			    test_bit(WX_FLAG_MULTI_64_FUNC, wx->flags))
> +				wr32(wx, WX_RDB_VMRSSTBL(i >> 2, wx->num_vfs), reta);

Do we need to reprogram the RSS when number of VFs change, now?

> +			else
> +				wr32(wx, WX_RDB_RSSTBL(i >> 2), reta);
>  			reta = 0;
>  		}
>  	}
>  }
>  
> +void wx_store_rsskey(struct wx *wx)
> +{
> +	u32 random_key_size = WX_RSS_KEY_SIZE / 4;

They key is just initialized to a random value, it doesn't have to be
random. Just "key_size" is better.

> +	u32 i;
> +
> +	if (test_bit(WX_FLAG_SRIOV_ENABLED, wx->flags) &&
> +	    test_bit(WX_FLAG_MULTI_64_FUNC, wx->flags)) {
> +		for (i = 0; i < random_key_size; i++)
> +			wr32(wx, WX_RDB_VMRSSRK(i, wx->num_vfs),
> +			     *(wx->rss_key + i));

Prefer normal array indexing:

			     wx->rss_key[i]

> +	} else {
> +		for (i = 0; i < random_key_size; i++)
> +			wr32(wx, WX_RDB_RSSRK(i), wx->rss_key[i]);
> +	}
> +}

> -	u32 rss_field = 0;

completely unclear to me why moving rss_field to struct wx is part of
this patch. It looks unrelated / prep for the next patch. 
-- 
pw-bot: cr