netdev - Re: [Intel-wired-lan] [PATCH iwl-next v3] ixgbe: preserve RSS indirection table across admin down/up

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: <20250902211640.85359-1-enjuk@amazon.com>
Date: Wed, 3 Sep 2025 06:16:39 +0900
From: Kohei Enju <enjuk@...zon.com>
To: <enjuk@...zon.com>
CC: <aleksandr.loktionov@...el.com>, <andrew+netdev@...n.ch>,
	<anthony.l.nguyen@...el.com>, <davem@...emloft.net>, <edumazet@...gle.com>,
	<intel-wired-lan@...ts.osuosl.org>, <kohei.enju@...il.com>,
	<kuba@...nel.org>, <netdev@...r.kernel.org>, <pabeni@...hat.com>,
	<przemyslaw.kitszel@...el.com>
Subject: Re: [Intel-wired-lan] [PATCH iwl-next v3] ixgbe: preserve RSS indirection table across admin down/up

On Wed, 3 Sep 2025 06:04:43 +0900, Kohei Enju wrote:

>On Tue, 2 Sep 2025 13:25:56 +0000, Loktionov, Aleksandr wrote:
>
>> [...]
>>> -
>>> -	for (i = 0, j = 0; i < reta_entries; i++, j++) {
>>> -		if (j == rss_i)
>>> -			j = 0;
>>> +	/* Update redirection table in memory on first init, queue
>>> count change,
>>> +	 * or reta entries change, otherwise preserve user
>>> configurations. Then
>>> +	 * always write to hardware.
>>> +	 */
>>> +	if (adapter->last_rss_indices != rss_i ||
>>> +	    adapter->last_reta_entries != reta_entries) {
>>> +		for (i = 0; i < reta_entries; i++)
>>> +			adapter->rss_indir_tbl[i] = i % rss_i;
>>Are you sure rss_i never ever can be a 0?
>>This is the only thing I'm worrying about.
>
>Oops, you're exactly right. Good catch!
>
>I see the original code assigns 0 to rss_indir_tbl[i] when rss_i is 0,
>like:
>  adapter->rss_indir_tbl[i] = 0;

Ahh, that's not true, my brain was not working... Sorry for messing up.
Anyway, in a situation where rss_i == 0, we should handle it somehow to
avoid zero-divisor.

>
>To handle this with keeping the behavior when rss_i == 0, I'm
>considering
>Option 1:
>  adapter->rss_indir_tbl[i] = rss_i ? i % rss_i : 0;
>
>Option 2:
>  if (rss_i)
>      for (i = 0; i < reta_entries; i++)
>          adapter->rss_indir_tbl[i] = i % rss_i;
>  else
>      memset(adapter->rss_indir_tbl, 0, reta_entries);
>
>Since this is not in the data path, the overhead of checking rss_i in
>each iteration might be acceptable. Therefore I'd like to adopt the
>option 1 for simplicity.
>
>Do you have any preference or other suggestions?