[PATCH iwl-next v2] ixgbe: preserve RSS indirection table across admin down/up

[PATCH iwl-next v2] ixgbe: preserve RSS indirection table across admin down/up

[Kohei Enju]

Currently, the RSS indirection table configured by user via ethtool is
reinitialized to default values during interface resets (e.g., admin
down/up, MTU change). As for RSS hash key, commit 3dfbfc7ebb95 ("ixgbe:
Check for RSS key before setting value") made it persistent across
interface resets.

Adopt the same approach used in igc and igb drivers which reinitializes
the RSS indirection table only when the queue count changes. Since the
number of RETA entries can also change in ixgbe, let's make user
configuration persistent as long as both queue count and the number of
RETA entries remain unchanged.

Tested on Intel Corporation 82599ES 10-Gigabit SFI/SFP+ Network
Connection.

Test:
Set custom indirection table and check the value after interface down/up

  # ethtool --set-rxfh-indir ens5 equal 2
  # ethtool --show-rxfh-indir ens5 | head -5

  RX flow hash indirection table for ens5 with 12 RX ring(s):
      0:      0     1     0     1     0     1     0     1
      8:      0     1     0     1     0     1     0     1
     16:      0     1     0     1     0     1     0     1
  # ip link set dev ens5 down && ip link set dev ens5 up

Without patch:
  # ethtool --show-rxfh-indir ens5 | head -5

  RX flow hash indirection table for ens5 with 12 RX ring(s):
      0:      0     1     2     3     4     5     6     7
      8:      8     9    10    11     0     1     2     3
     16:      4     5     6     7     8     9    10    11

With patch:
  # ethtool --show-rxfh-indir ens5 | head -5

  RX flow hash indirection table for ens5 with 12 RX ring(s):
      0:      0     1     0     1     0     1     0     1
      8:      0     1     0     1     0     1     0     1
     16:      0     1     0     1     0     1     0     1

Signed-off-by: Kohei Enju <enjuk@amazon.com>
---
Changes:
  v1->v2:
    - add check for reta_entries in addition to rss_i
    - update the commit message to reflect the additional check
  v1: https://lore.kernel.org/intel-wired-lan/20250824112037.32692-1-enjuk@amazon.com/
---
 drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  2 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 41 +++++++++++++------
 2 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 14d275270123..da3b23bdcce1 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -838,6 +838,8 @@ struct ixgbe_adapter {
  */
 #define IXGBE_MAX_RETA_ENTRIES 512
 	u8 rss_indir_tbl[IXGBE_MAX_RETA_ENTRIES];
+	u32 last_reta_entries;
+	u16 last_rss_i;
 
 #define IXGBE_RSS_KEY_SIZE     40  /* size of RSS Hash Key in bytes */
 	u32 *rss_key;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 27040076f068..05dfb06173d4 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -4323,14 +4323,21 @@ static void ixgbe_setup_reta(struct ixgbe_adapter *adapter)
 	/* Fill out hash function seeds */
 	ixgbe_store_key(adapter);
 
-	/* Fill out redirection table */
-	memset(adapter->rss_indir_tbl, 0, sizeof(adapter->rss_indir_tbl));
+	/* 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_i != rss_i ||
+	    adapter->last_reta_entries != reta_entries) {
+		for (i = 0, j = 0; i < reta_entries; i++, j++) {
+			if (j == rss_i)
+				j = 0;
 
-	for (i = 0, j = 0; i < reta_entries; i++, j++) {
-		if (j == rss_i)
-			j = 0;
+			adapter->rss_indir_tbl[i] = j;
+		}
 
-		adapter->rss_indir_tbl[i] = j;
+		adapter->last_rss_i = rss_i;
+		adapter->last_reta_entries = reta_entries;
 	}
 
 	ixgbe_store_reta(adapter);
@@ -4338,8 +4345,9 @@ static void ixgbe_setup_reta(struct ixgbe_adapter *adapter)
 
 static void ixgbe_setup_vfreta(struct ixgbe_adapter *adapter)
 {
-	struct ixgbe_hw *hw = &adapter->hw;
 	u16 rss_i = adapter->ring_feature[RING_F_RSS].indices;
+	struct ixgbe_hw *hw = &adapter->hw;
+	u32 reta_entries = 64;
 	int i, j;
 
 	/* Fill out hash function seeds */
@@ -4352,12 +4360,21 @@ static void ixgbe_setup_vfreta(struct ixgbe_adapter *adapter)
 					*(adapter->rss_key + i));
 	}
 
-	/* Fill out the redirection table */
-	for (i = 0, j = 0; i < 64; 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_i != rss_i ||
+	    adapter->last_reta_entries != reta_entries) {
+		for (i = 0, j = 0; i < reta_entries; i++, j++) {
+			if (j == rss_i)
+				j = 0;
+
+			adapter->rss_indir_tbl[i] = j;
+		}
 
-		adapter->rss_indir_tbl[i] = j;
+		adapter->last_rss_i = rss_i;
+		adapter->last_reta_entries = reta_entries;
 	}
 
 	ixgbe_store_vfreta(adapter);
-- 
2.51.0

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

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Kohei Enju
> Sent: Thursday, August 28, 2025 6:01 PM
> To: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; Andrew Lunn
> <andrew+netdev@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
> Paolo Abeni <pabeni@redhat.com>; kohei.enju@gmail.com; Kohei Enju
> <enjuk@amazon.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next v2] ixgbe: preserve RSS
> indirection table across admin down/up
> 
> Currently, the RSS indirection table configured by user via ethtool
> is
> reinitialized to default values during interface resets (e.g., admin
> down/up, MTU change). As for RSS hash key, commit 3dfbfc7ebb95
> ("ixgbe:
> Check for RSS key before setting value") made it persistent across
> interface resets.
> 
> Adopt the same approach used in igc and igb drivers which
> reinitializes
> the RSS indirection table only when the queue count changes. Since
> the
> number of RETA entries can also change in ixgbe, let's make user
> configuration persistent as long as both queue count and the number
> of
> RETA entries remain unchanged.
> 
> Tested on Intel Corporation 82599ES 10-Gigabit SFI/SFP+ Network
> Connection.
> 
> Test:
> Set custom indirection table and check the value after interface
> down/up
> 
>   # ethtool --set-rxfh-indir ens5 equal 2
>   # ethtool --show-rxfh-indir ens5 | head -5
> 
>   RX flow hash indirection table for ens5 with 12 RX ring(s):
>       0:      0     1     0     1     0     1     0     1
>       8:      0     1     0     1     0     1     0     1
>      16:      0     1     0     1     0     1     0     1
>   # ip link set dev ens5 down && ip link set dev ens5 up
> 
> Without patch:
>   # ethtool --show-rxfh-indir ens5 | head -5
> 
>   RX flow hash indirection table for ens5 with 12 RX ring(s):
>       0:      0     1     2     3     4     5     6     7
>       8:      8     9    10    11     0     1     2     3
>      16:      4     5     6     7     8     9    10    11
> 
> With patch:
>   # ethtool --show-rxfh-indir ens5 | head -5
> 
>   RX flow hash indirection table for ens5 with 12 RX ring(s):
>       0:      0     1     0     1     0     1     0     1
>       8:      0     1     0     1     0     1     0     1
>      16:      0     1     0     1     0     1     0     1
> 
> Signed-off-by: Kohei Enju <enjuk@amazon.com>
> ---
> Changes:
>   v1->v2:
>     - add check for reta_entries in addition to rss_i
>     - update the commit message to reflect the additional check
>   v1: https://lore.kernel.org/intel-wired-lan/20250824112037.32692-
> 1-enjuk@amazon.com/
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  2 +
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 41 +++++++++++++---
> ---
>  2 files changed, 31 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> index 14d275270123..da3b23bdcce1 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> @@ -838,6 +838,8 @@ struct ixgbe_adapter {
>   */
>  #define IXGBE_MAX_RETA_ENTRIES 512
>  	u8 rss_indir_tbl[IXGBE_MAX_RETA_ENTRIES];
> +	u32 last_reta_entries;
> +	u16 last_rss_i;
last_rss_i is cryptic; please, consider last_rss_indices (or similar)

> 
>  #define IXGBE_RSS_KEY_SIZE     40  /* size of RSS Hash Key in bytes
> */
>  	u32 *rss_key;
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 27040076f068..05dfb06173d4 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -4323,14 +4323,21 @@ static void ixgbe_setup_reta(struct
> ixgbe_adapter *adapter)
>  	/* Fill out hash function seeds */
>  	ixgbe_store_key(adapter);
> 
> -	/* Fill out redirection table */
> -	memset(adapter->rss_indir_tbl, 0, sizeof(adapter-
> >rss_indir_tbl));
> +	/* 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_i != rss_i ||
> +	    adapter->last_reta_entries != reta_entries) {
> +		for (i = 0, j = 0; i < reta_entries; i++, j++) {
You can avoid the top-of-loop equality test by using modulo, which reads easier, like:
for (i = 0, j = 0; i < reta_entries; i++, j++)
    adapter->rss_indir_tbl[i] = j % rss_i;

> +			if (j == rss_i)
> +				j = 0;
> 
> -	for (i = 0, j = 0; i < reta_entries; i++, j++) {
> -		if (j == rss_i)
> -			j = 0;
> +			adapter->rss_indir_tbl[i] = j;
> +		}
> 
> -		adapter->rss_indir_tbl[i] = j;
> +		adapter->last_rss_i = rss_i;
> +		adapter->last_reta_entries = reta_entries;
>  	}
> 
>  	ixgbe_store_reta(adapter);
> @@ -4338,8 +4345,9 @@ static void ixgbe_setup_reta(struct
> ixgbe_adapter *adapter)
> 
>  static void ixgbe_setup_vfreta(struct ixgbe_adapter *adapter)
>  {
> -	struct ixgbe_hw *hw = &adapter->hw;
>  	u16 rss_i = adapter->ring_feature[RING_F_RSS].indices;
> +	struct ixgbe_hw *hw = &adapter->hw;
> +	u32 reta_entries = 64;
Magic number. Can you #define IXGBE_VFRETA_ENTRIES 64 ?

>  	int i, j;
> 
>  	/* Fill out hash function seeds */
> @@ -4352,12 +4360,21 @@ static void ixgbe_setup_vfreta(struct
> ixgbe_adapter *adapter)
>  					*(adapter->rss_key + i));
>  	}
> 
> -	/* Fill out the redirection table */
> -	for (i = 0, j = 0; i < 64; 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_i != rss_i ||
> +	    adapter->last_reta_entries != reta_entries) {
> +		for (i = 0, j = 0; i < reta_entries; i++, j++) {
> +			if (j == rss_i)
> +				j = 0;
> +
> +			adapter->rss_indir_tbl[i] = j;
> +		}
> 
> -		adapter->rss_indir_tbl[i] = j;
> +		adapter->last_rss_i = rss_i;
> +		adapter->last_reta_entries = reta_entries;
>  	}
> 
>  	ixgbe_store_vfreta(adapter);
> --
> 2.51.0

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

[Kohei Enju]

On Thu, 28 Aug 2025 16:42:31 +0000, Loktionov, Aleksandr wrote:

> 
> 
> > -----Original Message-----
> > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> > Of Kohei Enju
> > Sent: Thursday, August 28, 2025 6:01 PM
> > To: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org
> > Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
> > Przemyslaw <przemyslaw.kitszel@intel.com>; Andrew Lunn
> > <andrew+netdev@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric
> > Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
> > Paolo Abeni <pabeni@redhat.com>; kohei.enju@gmail.com; Kohei Enju
> > <enjuk@amazon.com>
> > Subject: [Intel-wired-lan] [PATCH iwl-next v2] ixgbe: preserve RSS
> > indirection table across admin down/up
> > 
> > Currently, the RSS indirection table configured by user via ethtool
> > is
> > reinitialized to default values during interface resets (e.g., admin
> > down/up, MTU change). As for RSS hash key, commit 3dfbfc7ebb95
> > ("ixgbe:
> > Check for RSS key before setting value") made it persistent across
> > interface resets.
> > 
> > Adopt the same approach used in igc and igb drivers which
> > reinitializes
> > the RSS indirection table only when the queue count changes. Since
> > the
> > number of RETA entries can also change in ixgbe, let's make user
> > configuration persistent as long as both queue count and the number
> > of
> > RETA entries remain unchanged.
> > 
> > Tested on Intel Corporation 82599ES 10-Gigabit SFI/SFP+ Network
> > Connection.
> > 
> > Test:
> > Set custom indirection table and check the value after interface
> > down/up
> > 
> >   # ethtool --set-rxfh-indir ens5 equal 2
> >   # ethtool --show-rxfh-indir ens5 | head -5
> > 
> >   RX flow hash indirection table for ens5 with 12 RX ring(s):
> >       0:      0     1     0     1     0     1     0     1
> >       8:      0     1     0     1     0     1     0     1
> >      16:      0     1     0     1     0     1     0     1
> >   # ip link set dev ens5 down && ip link set dev ens5 up
> > 
> > Without patch:
> >   # ethtool --show-rxfh-indir ens5 | head -5
> > 
> >   RX flow hash indirection table for ens5 with 12 RX ring(s):
> >       0:      0     1     2     3     4     5     6     7
> >       8:      8     9    10    11     0     1     2     3
> >      16:      4     5     6     7     8     9    10    11
> > 
> > With patch:
> >   # ethtool --show-rxfh-indir ens5 | head -5
> > 
> >   RX flow hash indirection table for ens5 with 12 RX ring(s):
> >       0:      0     1     0     1     0     1     0     1
> >       8:      0     1     0     1     0     1     0     1
> >      16:      0     1     0     1     0     1     0     1
> > 
> > Signed-off-by: Kohei Enju <enjuk@amazon.com>
> > ---
> > Changes:
> >   v1->v2:
> >     - add check for reta_entries in addition to rss_i
> >     - update the commit message to reflect the additional check
> >   v1: https://lore.kernel.org/intel-wired-lan/20250824112037.32692-
> > 1-enjuk@amazon.com/
> > ---
> >  drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  2 +
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 41 +++++++++++++---
> > ---
> >  2 files changed, 31 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> > b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> > index 14d275270123..da3b23bdcce1 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> > @@ -838,6 +838,8 @@ struct ixgbe_adapter {
> >   */
> >  #define IXGBE_MAX_RETA_ENTRIES 512
> >  	u8 rss_indir_tbl[IXGBE_MAX_RETA_ENTRIES];
> > +	u32 last_reta_entries;
> > +	u16 last_rss_i;
> last_rss_i is cryptic; please, consider last_rss_indices (or similar)

Sure, I'll rename it to last_rss_indices for clarity.

> 
> > 
> >  #define IXGBE_RSS_KEY_SIZE     40  /* size of RSS Hash Key in bytes
> > */
> >  	u32 *rss_key;
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > index 27040076f068..05dfb06173d4 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > @@ -4323,14 +4323,21 @@ static void ixgbe_setup_reta(struct
> > ixgbe_adapter *adapter)
> >  	/* Fill out hash function seeds */
> >  	ixgbe_store_key(adapter);
> > 
> > -	/* Fill out redirection table */
> > -	memset(adapter->rss_indir_tbl, 0, sizeof(adapter-
> > >rss_indir_tbl));
> > +	/* 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_i != rss_i ||
> > +	    adapter->last_reta_entries != reta_entries) {
> > +		for (i = 0, j = 0; i < reta_entries; i++, j++) {
> You can avoid the top-of-loop equality test by using modulo, which reads easier, like:
> for (i = 0, j = 0; i < reta_entries; i++, j++)
>     adapter->rss_indir_tbl[i] = j % rss_i;

I got it. I'll use modulo and then j can be removed like:
    for (i = 0; i < reta_entries; i++)
           adapter->rss_indir_tbl[i] = i % rss_i;

> 
> > +			if (j == rss_i)
> > +				j = 0;
> > 
> > -	for (i = 0, j = 0; i < reta_entries; i++, j++) {
> > -		if (j == rss_i)
> > -			j = 0;
> > +			adapter->rss_indir_tbl[i] = j;
> > +		}
> > 
> > -		adapter->rss_indir_tbl[i] = j;
> > +		adapter->last_rss_i = rss_i;
> > +		adapter->last_reta_entries = reta_entries;
> >  	}
> > 
> >  	ixgbe_store_reta(adapter);
> > @@ -4338,8 +4345,9 @@ static void ixgbe_setup_reta(struct
> > ixgbe_adapter *adapter)
> > 
> >  static void ixgbe_setup_vfreta(struct ixgbe_adapter *adapter)
> >  {
> > -	struct ixgbe_hw *hw = &adapter->hw;
> >  	u16 rss_i = adapter->ring_feature[RING_F_RSS].indices;
> > +	struct ixgbe_hw *hw = &adapter->hw;
> > +	u32 reta_entries = 64;
> Magic number. Can you #define IXGBE_VFRETA_ENTRIES 64 ?

You're right about the magic number. 
I see it was introduced in commit 0f9b232b176d ("ixgbe: add support for 
X550 extended RSS support").

I'm considering using ixgbe_rss_indir_tbl_entries() instead of #define to 
avoid the magic number, since ixgbe_store_vfreta() already uses it.
This would ensure consistency between the two functions. Would that be 
acceptable, or would you prefer a #define?

> 
> >  	int i, j;
> > 
> >  	/* Fill out hash function seeds */
> > @@ -4352,12 +4360,21 @@ static void ixgbe_setup_vfreta(struct
> > ixgbe_adapter *adapter)
> >  					*(adapter->rss_key + i));
> >  	}
> > 
> > -	/* Fill out the redirection table */
> > -	for (i = 0, j = 0; i < 64; 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_i != rss_i ||
> > +	    adapter->last_reta_entries != reta_entries) {
> > +		for (i = 0, j = 0; i < reta_entries; i++, j++) {
> > +			if (j == rss_i)
> > +				j = 0;
> > +
> > +			adapter->rss_indir_tbl[i] = j;
> > +		}
> > 
> > -		adapter->rss_indir_tbl[i] = j;
> > +		adapter->last_rss_i = rss_i;
> > +		adapter->last_reta_entries = reta_entries;
> >  	}
> > 
> >  	ixgbe_store_vfreta(adapter);
> > --
> > 2.51.0

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

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Kohei Enju <enjuk@amazon.com>
> Sent: Thursday, August 28, 2025 7:41 PM
> To: Loktionov, Aleksandr <aleksandr.loktionov@intel.com>
> Cc: andrew+netdev@lunn.ch; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; davem@davemloft.net;
> edumazet@google.com; enjuk@amazon.com; intel-wired-
> lan@lists.osuosl.org; kohei.enju@gmail.com; kuba@kernel.org;
> netdev@vger.kernel.org; pabeni@redhat.com; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next v2] ixgbe: preserve
> RSS indirection table across admin down/up
> 
> On Thu, 28 Aug 2025 16:42:31 +0000, Loktionov, Aleksandr wrote:
> 
> >
> >
> > > -----Original Message-----
> > > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On
> Behalf
> > > Of Kohei Enju
> > > Sent: Thursday, August 28, 2025 6:01 PM
> > > To: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org
> > > Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
> > > Przemyslaw <przemyslaw.kitszel@intel.com>; Andrew Lunn
> > > <andrew+netdev@lunn.ch>; David S. Miller <davem@davemloft.net>;
> Eric
> > > Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
> > > Paolo Abeni <pabeni@redhat.com>; kohei.enju@gmail.com; Kohei
> Enju
> > > <enjuk@amazon.com>
> > > Subject: [Intel-wired-lan] [PATCH iwl-next v2] ixgbe: preserve
> RSS
> > > indirection table across admin down/up
> > >
> > > Currently, the RSS indirection table configured by user via
> ethtool
> > > is reinitialized to default values during interface resets
> (e.g.,
> > > admin down/up, MTU change). As for RSS hash key, commit
> 3dfbfc7ebb95
> > > ("ixgbe:
> > > Check for RSS key before setting value") made it persistent
> across
> > > interface resets.
> > >
> > > Adopt the same approach used in igc and igb drivers which
> > > reinitializes the RSS indirection table only when the queue
> count
> > > changes. Since the number of RETA entries can also change in
> ixgbe,
> > > let's make user configuration persistent as long as both queue
> count
> > > and the number of RETA entries remain unchanged.
> > >
> > > Tested on Intel Corporation 82599ES 10-Gigabit SFI/SFP+ Network
> > > Connection.
> > >
> > > Test:
> > > Set custom indirection table and check the value after interface
> > > down/up
> > >
> > >   # ethtool --set-rxfh-indir ens5 equal 2
> > >   # ethtool --show-rxfh-indir ens5 | head -5
> > >
> > >   RX flow hash indirection table for ens5 with 12 RX ring(s):
> > >       0:      0     1     0     1     0     1     0     1
> > >       8:      0     1     0     1     0     1     0     1
> > >      16:      0     1     0     1     0     1     0     1
> > >   # ip link set dev ens5 down && ip link set dev ens5 up
> > >
> > > Without patch:
> > >   # ethtool --show-rxfh-indir ens5 | head -5
> > >
> > >   RX flow hash indirection table for ens5 with 12 RX ring(s):
> > >       0:      0     1     2     3     4     5     6     7
> > >       8:      8     9    10    11     0     1     2     3
> > >      16:      4     5     6     7     8     9    10    11
> > >
> > > With patch:
> > >   # ethtool --show-rxfh-indir ens5 | head -5
> > >
> > >   RX flow hash indirection table for ens5 with 12 RX ring(s):
> > >       0:      0     1     0     1     0     1     0     1
> > >       8:      0     1     0     1     0     1     0     1
> > >      16:      0     1     0     1     0     1     0     1
> > >
> > > Signed-off-by: Kohei Enju <enjuk@amazon.com>
> > > ---
> > > Changes:
> > >   v1->v2:
> > >     - add check for reta_entries in addition to rss_i
> > >     - update the commit message to reflect the additional check
> > >   v1: https://lore.kernel.org/intel-wired-
> lan/20250824112037.32692-
> > > 1-enjuk@amazon.com/
> > > ---
> > >  drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  2 +
> > >  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 41
> +++++++++++++---
> > > ---
> > >  2 files changed, 31 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> > > b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> > > index 14d275270123..da3b23bdcce1 100644
> > > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> > > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> > > @@ -838,6 +838,8 @@ struct ixgbe_adapter {
> > >   */
> > >  #define IXGBE_MAX_RETA_ENTRIES 512
> > >  	u8 rss_indir_tbl[IXGBE_MAX_RETA_ENTRIES];
> > > +	u32 last_reta_entries;
> > > +	u16 last_rss_i;
> > last_rss_i is cryptic; please, consider last_rss_indices (or
> similar)
> 
> Sure, I'll rename it to last_rss_indices for clarity.
> 
> >
> > >
> > >  #define IXGBE_RSS_KEY_SIZE     40  /* size of RSS Hash Key in
> bytes
> > > */
> > >  	u32 *rss_key;
> > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > > b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > > index 27040076f068..05dfb06173d4 100644
> > > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > > @@ -4323,14 +4323,21 @@ static void ixgbe_setup_reta(struct
> > > ixgbe_adapter *adapter)
> > >  	/* Fill out hash function seeds */
> > >  	ixgbe_store_key(adapter);
> > >
> > > -	/* Fill out redirection table */
> > > -	memset(adapter->rss_indir_tbl, 0, sizeof(adapter-
> > > >rss_indir_tbl));
> > > +	/* 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_i != rss_i ||
> > > +	    adapter->last_reta_entries != reta_entries) {
> > > +		for (i = 0, j = 0; i < reta_entries; i++, j++) {
> > You can avoid the top-of-loop equality test by using modulo, which
> reads easier, like:
> > for (i = 0, j = 0; i < reta_entries; i++, j++)
> >     adapter->rss_indir_tbl[i] = j % rss_i;
> 
> I got it. I'll use modulo and then j can be removed like:
>     for (i = 0; i < reta_entries; i++)
>            adapter->rss_indir_tbl[i] = i % rss_i;
> 
> >
> > > +			if (j == rss_i)
> > > +				j = 0;
> > >
> > > -	for (i = 0, j = 0; i < reta_entries; i++, j++) {
> > > -		if (j == rss_i)
> > > -			j = 0;
> > > +			adapter->rss_indir_tbl[i] = j;
> > > +		}
> > >
> > > -		adapter->rss_indir_tbl[i] = j;
> > > +		adapter->last_rss_i = rss_i;
> > > +		adapter->last_reta_entries = reta_entries;
> > >  	}
> > >
> > >  	ixgbe_store_reta(adapter);
> > > @@ -4338,8 +4345,9 @@ static void ixgbe_setup_reta(struct
> > > ixgbe_adapter *adapter)
> > >
> > >  static void ixgbe_setup_vfreta(struct ixgbe_adapter *adapter)
> {
> > > -	struct ixgbe_hw *hw = &adapter->hw;
> > >  	u16 rss_i = adapter->ring_feature[RING_F_RSS].indices;
> > > +	struct ixgbe_hw *hw = &adapter->hw;
> > > +	u32 reta_entries = 64;
> > Magic number. Can you #define IXGBE_VFRETA_ENTRIES 64 ?
> 
> You're right about the magic number.
> I see it was introduced in commit 0f9b232b176d ("ixgbe: add support
> for
> X550 extended RSS support").
> 
> I'm considering using ixgbe_rss_indir_tbl_entries() instead of
> #define to avoid the magic number, since ixgbe_store_vfreta()
> already uses it.
> This would ensure consistency between the two functions. Would that
> be acceptable, or would you prefer a #define?
> 
Good catch!
ixgbe_rss_indir_tbl_entries() is proper solution ,because it's h/w and configuration dependent.

> >
> > >  	int i, j;
> > >
> > >  	/* Fill out hash function seeds */ @@ -4352,12 +4360,21 @@
> static
> > > void ixgbe_setup_vfreta(struct ixgbe_adapter *adapter)
> > >  					*(adapter->rss_key + i));
> > >  	}
> > >
> > > -	/* Fill out the redirection table */
> > > -	for (i = 0, j = 0; i < 64; 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_i != rss_i ||
> > > +	    adapter->last_reta_entries != reta_entries) {
> > > +		for (i = 0, j = 0; i < reta_entries; i++, j++) {
> > > +			if (j == rss_i)
> > > +				j = 0;
> > > +
> > > +			adapter->rss_indir_tbl[i] = j;
> > > +		}
> > >
> > > -		adapter->rss_indir_tbl[i] = j;
> > > +		adapter->last_rss_i = rss_i;
> > > +		adapter->last_reta_entries = reta_entries;
> > >  	}
> > >
> > >  	ixgbe_store_vfreta(adapter);
> > > --
> > > 2.51.0