netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH iwl-next v1] ixgbe: preserve RSS indirection table across admin down/up
@ 2025-08-24 11:20 Kohei Enju
  2025-08-24 18:00 ` [Intel-wired-lan] " Paul Menzel
  2025-08-25  9:48 ` Przemek Kitszel
  0 siblings, 2 replies; 5+ messages in thread
From: Kohei Enju @ 2025-08-24 11:20 UTC (permalink / raw)
  To: netdev, intel-wired-lan
  Cc: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, kohei.enju, 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.

By adopting the same approach used in igc and igb drivers which
reinitializes the RSS indirection table only when the queue count
changes, let's make user configuration persistent as long as queue count
remains 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>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 37 +++++++++++++------
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 14d275270123..d8b088c90b05 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -838,6 +838,7 @@ struct ixgbe_adapter {
  */
 #define IXGBE_MAX_RETA_ENTRIES 512
 	u8 rss_indir_tbl[IXGBE_MAX_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 80e6a2ef1350..dc5a8373b0c3 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -4318,14 +4318,22 @@ 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 or queue count
+	 * change, otherwise preserve user configurations. Then always
+	 * write to hardware.
+	 */
+	if (adapter->last_rss_i != rss_i) {
+		memset(adapter->rss_indir_tbl, 0,
+		       sizeof(adapter->rss_indir_tbl));
+
+		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;
 	}
 
 	ixgbe_store_reta(adapter);
@@ -4347,12 +4355,19 @@ 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 or queue count
+	 * change, otherwise preserve user configurations. Then always
+	 * write to hardware.
+	 */
+	if (adapter->last_rss_i != rss_i) {
+		for (i = 0, j = 0; i < 64; 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;
 	}
 
 	ixgbe_store_vfreta(adapter);
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Intel-wired-lan] [PATCH iwl-next v1] ixgbe: preserve RSS indirection table across admin down/up
  2025-08-24 11:20 [PATCH iwl-next v1] ixgbe: preserve RSS indirection table across admin down/up Kohei Enju
@ 2025-08-24 18:00 ` Paul Menzel
  2025-08-28 16:08   ` Kohei Enju
  2025-08-25  9:48 ` Przemek Kitszel
  1 sibling, 1 reply; 5+ messages in thread
From: Paul Menzel @ 2025-08-24 18:00 UTC (permalink / raw)
  To: Kohei Enju
  Cc: netdev, intel-wired-lan, Tony Nguyen, Przemek Kitszel,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, kohei.enju

Dear Kohei,


Thank you for your patch.

Am 24.08.25 um 13:20 schrieb 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.
> 
> By adopting the same approach used in igc and igb drivers which
> reinitializes the RSS indirection table only when the queue count
> changes, let's make user configuration persistent as long as queue count
> remains 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>
> ---
>   drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  1 +
>   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 37 +++++++++++++------
>   2 files changed, 27 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> index 14d275270123..d8b088c90b05 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> @@ -838,6 +838,7 @@ struct ixgbe_adapter {
>    */
>   #define IXGBE_MAX_RETA_ENTRIES 512
>   	u8 rss_indir_tbl[IXGBE_MAX_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 80e6a2ef1350..dc5a8373b0c3 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -4318,14 +4318,22 @@ 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 or queue count
> +	 * change, otherwise preserve user configurations. Then always
> +	 * write to hardware.
> +	 */
> +	if (adapter->last_rss_i != rss_i) {
> +		memset(adapter->rss_indir_tbl, 0,
> +		       sizeof(adapter->rss_indir_tbl));
> +
> +		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;
>   	}
>   
>   	ixgbe_store_reta(adapter);
> @@ -4347,12 +4355,19 @@ 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 or queue count
> +	 * change, otherwise preserve user configurations. Then always
> +	 * write to hardware.
> +	 */
> +	if (adapter->last_rss_i != rss_i) {
> +		for (i = 0, j = 0; i < 64; 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;
>   	}
>   
>   	ixgbe_store_vfreta(adapter);

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH iwl-next v1] ixgbe: preserve RSS indirection table across admin down/up
  2025-08-24 11:20 [PATCH iwl-next v1] ixgbe: preserve RSS indirection table across admin down/up Kohei Enju
  2025-08-24 18:00 ` [Intel-wired-lan] " Paul Menzel
@ 2025-08-25  9:48 ` Przemek Kitszel
  2025-08-25 11:36   ` [Intel-wired-lan] " Kohei Enju
  1 sibling, 1 reply; 5+ messages in thread
From: Przemek Kitszel @ 2025-08-25  9:48 UTC (permalink / raw)
  To: Kohei Enju
  Cc: Tony Nguyen, intel-wired-lan, netdev, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	kohei.enju

On 8/24/25 13:20, Kohei Enju wrote:
> 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.
> 
> By adopting the same approach used in igc and igb drivers which
> reinitializes the RSS indirection table only when the queue count
> changes, let's make user configuration persistent as long as queue count
> remains 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>
> ---
>   drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  1 +
>   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 37 +++++++++++++------
>   2 files changed, 27 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> index 14d275270123..d8b088c90b05 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> @@ -838,6 +838,7 @@ struct ixgbe_adapter {
>    */
>   #define IXGBE_MAX_RETA_ENTRIES 512
>   	u8 rss_indir_tbl[IXGBE_MAX_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 80e6a2ef1350..dc5a8373b0c3 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -4318,14 +4318,22 @@ 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 or queue count
> +	 * change, otherwise preserve user configurations. Then always
> +	 * write to hardware.
> +	 */
> +	if (adapter->last_rss_i != rss_i) {
> +		memset(adapter->rss_indir_tbl, 0,
> +		       sizeof(adapter->rss_indir_tbl));

Thank you for the patch,
I see no point in the memset() above, especially given 0 as a valid
entry in the table.

> +
> +		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;
>   	}
>   
>   	ixgbe_store_reta(adapter);
> @@ -4347,12 +4355,19 @@ 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 or queue count
> +	 * change, otherwise preserve user configurations. Then always
> +	 * write to hardware.
> +	 */
> +	if (adapter->last_rss_i != rss_i) {
> +		for (i = 0, j = 0; i < 64; 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;
>   	}
>   
>   	ixgbe_store_vfreta(adapter);


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Intel-wired-lan] [PATCH iwl-next v1] ixgbe: preserve RSS indirection table across admin down/up
  2025-08-25  9:48 ` Przemek Kitszel
@ 2025-08-25 11:36   ` Kohei Enju
  0 siblings, 0 replies; 5+ messages in thread
From: Kohei Enju @ 2025-08-25 11:36 UTC (permalink / raw)
  To: przemyslaw.kitszel
  Cc: andrew+netdev, anthony.l.nguyen, davem, edumazet, enjuk,
	intel-wired-lan, kohei.enju, kuba, netdev, pabeni, pmenzel

On Mon, 25 Aug 2025 11:48:21 +0200, Przemek Kitszel wrote:

>On 8/24/25 13:20, Kohei Enju wrote:
>> 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.
>> 
>> By adopting the same approach used in igc and igb drivers which
>> reinitializes the RSS indirection table only when the queue count
>> changes, let's make user configuration persistent as long as queue count
>> remains 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>
>> ---
>>   drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  1 +
>>   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 37 +++++++++++++------
>>   2 files changed, 27 insertions(+), 11 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>> index 14d275270123..d8b088c90b05 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>> @@ -838,6 +838,7 @@ struct ixgbe_adapter {
>>    */
>>   #define IXGBE_MAX_RETA_ENTRIES 512
>>   	u8 rss_indir_tbl[IXGBE_MAX_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 80e6a2ef1350..dc5a8373b0c3 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> @@ -4318,14 +4318,22 @@ 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 or queue count
>> +	 * change, otherwise preserve user configurations. Then always
>> +	 * write to hardware.
>> +	 */
>> +	if (adapter->last_rss_i != rss_i) {
>> +		memset(adapter->rss_indir_tbl, 0,
>> +		       sizeof(adapter->rss_indir_tbl));
>
>Thank you for the patch,
>I see no point in the memset() above, especially given 0 as a valid
>entry in the table.

Indeed, you're right. I'll remove that. 
Thank you for reviewing.

BTW, I noticed that this patch would unintentionally skip
reinitialization when rss_i remains unchanged but reta_entries changes.
I apologize for overlooking this in my initial patch.

For example, on some devices reta_entries changes according to its
SR-IOV status (IXGBE_FLAG_SRIOV_ENABLED). By setting queue count in
advance, we may be able to create a situation where rss_i remains
unchanged but reta_entries changes. In this case, this patch would cause
an unintentional skip of reinitialization.

Unlike igb/igc, ixgbe's reta_entries may change, so IIUC I need to check
reta_entries as well in ixgbe_setup_reta() and ixgbe_setup_vfreta()
like:

        /* 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->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;
>>   	}
>>   
>>   	ixgbe_store_reta(adapter);
>> @@ -4347,12 +4355,19 @@ 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 or queue count
>> +	 * change, otherwise preserve user configurations. Then always
>> +	 * write to hardware.
>> +	 */
>> +	if (adapter->last_rss_i != rss_i) {
>> +		for (i = 0, j = 0; i < 64; 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;
>>   	}
>>   
>>   	ixgbe_store_vfreta(adapter);
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Intel-wired-lan] [PATCH iwl-next v1] ixgbe: preserve RSS indirection table across admin down/up
  2025-08-24 18:00 ` [Intel-wired-lan] " Paul Menzel
@ 2025-08-28 16:08   ` Kohei Enju
  0 siblings, 0 replies; 5+ messages in thread
From: Kohei Enju @ 2025-08-28 16:08 UTC (permalink / raw)
  To: pmenzel
  Cc: andrew+netdev, anthony.l.nguyen, davem, edumazet, enjuk,
	intel-wired-lan, kohei.enju, kuba, netdev, pabeni,
	przemyslaw.kitszel

On Sun, 24 Aug 2025 20:00:32 +0200, Paul Menzel wrote:

> Dear Kohei,
> 
> 
> Thank you for your patch.
> 
> Am 24.08.25 um 13:20 schrieb 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.
> > 
> > By adopting the same approach used in igc and igb drivers which
> > reinitializes the RSS indirection table only when the queue count
> > changes, let's make user configuration persistent as long as queue count
> > remains 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>
> > ---
> >   drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  1 +
> >   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 37 +++++++++++++------
> >   2 files changed, 27 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> > index 14d275270123..d8b088c90b05 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> > @@ -838,6 +838,7 @@ struct ixgbe_adapter {
> >    */
> >   #define IXGBE_MAX_RETA_ENTRIES 512
> >   	u8 rss_indir_tbl[IXGBE_MAX_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 80e6a2ef1350..dc5a8373b0c3 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > @@ -4318,14 +4318,22 @@ 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 or queue count
> > +	 * change, otherwise preserve user configurations. Then always
> > +	 * write to hardware.
> > +	 */
> > +	if (adapter->last_rss_i != rss_i) {
> > +		memset(adapter->rss_indir_tbl, 0,
> > +		       sizeof(adapter->rss_indir_tbl));
> > +
> > +		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;
> >   	}
> >   
> >   	ixgbe_store_reta(adapter);
> > @@ -4347,12 +4355,19 @@ 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 or queue count
> > +	 * change, otherwise preserve user configurations. Then always
> > +	 * write to hardware.
> > +	 */
> > +	if (adapter->last_rss_i != rss_i) {
> > +		for (i = 0, j = 0; i < 64; 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;
> >   	}
> >   
> >   	ixgbe_store_vfreta(adapter);
> 
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>

Thank you for reviewing v1, Paul.

I've posted v2 with additional logic to check reta_entries changes.
Since v2 includes a functional change, I haven't carried forward your 
Reviewed-by: tag.

> 
> Kind regards,
> 
> Paul

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-08-28 16:08 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-24 11:20 [PATCH iwl-next v1] ixgbe: preserve RSS indirection table across admin down/up Kohei Enju
2025-08-24 18:00 ` [Intel-wired-lan] " Paul Menzel
2025-08-28 16:08   ` Kohei Enju
2025-08-25  9:48 ` Przemek Kitszel
2025-08-25 11:36   ` [Intel-wired-lan] " Kohei Enju

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).