netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net v2 0/2] ixgbe: Multiple RSS bugfixes
@ 2023-04-16 19:12 Joe Damato
  2023-04-16 19:12 ` [PATCH net v2 1/2] ixgbe: Allow flow hash to be set via ethtool Joe Damato
  2023-04-16 19:12 ` [PATCH net v2 2/2] ixgbe: Enable setting RSS table to default values Joe Damato
  0 siblings, 2 replies; 7+ messages in thread
From: Joe Damato @ 2023-04-16 19:12 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: netdev, jesse.brandeburg, anthony.l.nguyen, kuba,
	sridhar.samudrala, Joe Damato

Greetings:

Welcome to v2.

See the v2 change summary below; v2 contains only cosmetic changes to
commit messages. No other changes were made from v1.

This series fixes two bugs I stumbled on with ixgbe:

1. The flow hash cannot be set manually with ethool at all. Patch 1/2
addresses this by fixing what appears to be a small bug in set_rxfh in
ixgbe. See the commit message for more details.

2. Once the above patch is applied and the flow hash can be set,
resetting the flow hash to default fails if the number of queues is
greater than the number of queues supported by RSS. Other drivers (like
i40e) will reset the flowhash to use the maximum number of queues
supported by RSS even if the queue count configured is larger. In other
words: some queues will not have packets distributed to them by the RSS
hash if the queue count is too large. Patch 2/2 allows the user to reset
ixgbe to default and the flowhash is set correctly to either the
maximum number of queues supported by RSS or the configured queue count,
whichever is smaller.

I believe this is correct and it mimics the behavior of i40e;
`ethtool -X $iface default` should probably always succeed even if all the
queues cannot be utilized. See the commit message for more details and
examples.

I tested these on an ixgbe system I have access to and they appear to
work as intended, but I would appreciate a review by the experts on this
list :)

Thanks,
Joe

v2:
  - Include Reviewed-by tags from Sridhar Samudrala.
  - Fix title of patch 2/2:
      previously: ixgbe: Allow ixgbe to reset default flow hash
      now: ixgbe: Enable setting RSS table to default values
  - No functional changes.

Joe Damato (2):
  ixgbe: Allow flow hash to be set via ethtool
  ixgbe: Enable setting RSS table to default values

 .../net/ethernet/intel/ixgbe/ixgbe_ethtool.c  | 23 ++++++++++---------
 1 file changed, 12 insertions(+), 11 deletions(-)

-- 
2.25.1


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

* [PATCH net v2 1/2] ixgbe: Allow flow hash to be set via ethtool
  2023-04-16 19:12 [PATCH net v2 0/2] ixgbe: Multiple RSS bugfixes Joe Damato
@ 2023-04-16 19:12 ` Joe Damato
  2023-04-17  4:57   ` Joe Damato
  2023-04-16 19:12 ` [PATCH net v2 2/2] ixgbe: Enable setting RSS table to default values Joe Damato
  1 sibling, 1 reply; 7+ messages in thread
From: Joe Damato @ 2023-04-16 19:12 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: netdev, jesse.brandeburg, anthony.l.nguyen, kuba,
	sridhar.samudrala, Joe Damato

ixgbe currently returns `EINVAL` whenever the flowhash it set by ethtool
because the ethtool code in the kernel passes a non-zero value for hfunc
that ixgbe should allow.

When ethtool is called with `ETHTOOL_SRXFHINDIR`,
`ethtool_set_rxfh_indir` will call ixgbe's set_rxfh function
with `ETH_RSS_HASH_NO_CHANGE`. This value should be accepted.

When ethtool is called with `ETHTOOL_SRSSH`, `ethtool_set_rxfh` will
call ixgbe's set_rxfh function with `rxfh.hfunc`, which appears to be
hardcoded in ixgbe to always be `ETH_RSS_HASH_TOP`. This value should
also be accepted.

Before this patch:

$ sudo ethtool -L eth1 combined 10
$ sudo ethtool -X eth1 default
Cannot set RX flow hash configuration: Invalid argument

After this patch:

$ sudo ethtool -L eth1 combined 10
$ sudo ethtool -X eth1 default
$ sudo ethtool -x eth1
RX flow hash indirection table for eth1 with 10 RX ring(s):
    0:      0     1     2     3     4     5     6     7
    8:      8     9     0     1     2     3     4     5
   16:      6     7     8     9     0     1     2     3
   24:      4     5     6     7     8     9     0     1
   ...

Signed-off-by: Joe Damato <jdamato@fastly.com>
Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index 6cfc9dc16537..821dfd323fa9 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -3131,8 +3131,8 @@ static int ixgbe_set_rxfh(struct net_device *netdev, const u32 *indir,
 	int i;
 	u32 reta_entries = ixgbe_rss_indir_tbl_entries(adapter);
 
-	if (hfunc)
-		return -EINVAL;
+	if (hfunc != ETH_RSS_HASH_NO_CHANGE && hfunc != ETH_RSS_HASH_TOP)
+		return -EOPNOTSUPP;
 
 	/* Fill out the redirection table */
 	if (indir) {
-- 
2.25.1


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

* [PATCH net v2 2/2] ixgbe: Enable setting RSS table to default values
  2023-04-16 19:12 [PATCH net v2 0/2] ixgbe: Multiple RSS bugfixes Joe Damato
  2023-04-16 19:12 ` [PATCH net v2 1/2] ixgbe: Allow flow hash to be set via ethtool Joe Damato
@ 2023-04-16 19:12 ` Joe Damato
  2023-04-17  4:58   ` Joe Damato
  1 sibling, 1 reply; 7+ messages in thread
From: Joe Damato @ 2023-04-16 19:12 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: netdev, jesse.brandeburg, anthony.l.nguyen, kuba,
	sridhar.samudrala, Joe Damato

ethtool uses `ETHTOOL_GRXRINGS` to compute how many queues are supported
by RSS. The driver should return the smaller of either:
  - The maximum number of RSS queues the device supports, OR
  - The number of RX queues configured

Prior to this change, running `ethtool -X $iface default` fails if the
number of queues configured is larger than the number supported by RSS,
even though changing the queue count correctly resets the flowhash to
use all supported queues.

Other drivers (for example, i40e) will succeed but the flow hash will
reset to support the maximum number of queues supported by RSS, even if
that amount is smaller than the configured amount.

Prior to this change:

$ sudo ethtool -L eth1 combined 20
$ sudo ethtool -x eth1
RX flow hash indirection table for eth1 with 20 RX ring(s):
    0:      0     1     2     3     4     5     6     7
    8:      8     9    10    11    12    13    14    15
   16:      0     1     2     3     4     5     6     7
   24:      8     9    10    11    12    13    14    15
   32:      0     1     2     3     4     5     6     7
...

You can see that the flowhash was correctly set to use the maximum
number of queues supported by the driver (16).

However, asking the NIC to reset to "default" fails:

$ sudo ethtool -X eth1 default
Cannot set RX flow hash configuration: Invalid argument

After this change, the flowhash can be reset to default which will use
all of the available RSS queues (16) or the configured queue count,
whichever is smaller.

Starting with eth1 which has 10 queues and a flowhash distributing to
all 10 queues:

$ sudo ethtool -x eth1
RX flow hash indirection table for eth1 with 10 RX ring(s):
    0:      0     1     2     3     4     5     6     7
    8:      8     9     0     1     2     3     4     5
   16:      6     7     8     9     0     1     2     3
...

Increasing the queue count to 48 resets the flowhash to distribute to 16
queues, as it did before this patch:

$ sudo ethtool -L eth1 combined 48
$ sudo ethtool -x eth1
RX flow hash indirection table for eth1 with 16 RX ring(s):
    0:      0     1     2     3     4     5     6     7
    8:      8     9    10    11    12    13    14    15
   16:      0     1     2     3     4     5     6     7
...

Due to the other bugfix in this series, the flowhash can be set to use
queues 0-5:

$ sudo ethtool -X eth1 equal 5
$ sudo ethtool -x eth1
RX flow hash indirection table for eth1 with 16 RX ring(s):
    0:      0     1     2     3     4     0     1     2
    8:      3     4     0     1     2     3     4     0
   16:      1     2     3     4     0     1     2     3
...

Due to this bugfix, the flowhash can be reset to default and use 16
queues:

$ sudo ethtool -X eth1 default
$ sudo ethtool -x eth1
RX flow hash indirection table for eth1 with 16 RX ring(s):
    0:      0     1     2     3     4     5     6     7
    8:      8     9    10    11    12    13    14    15
   16:      0     1     2     3     4     5     6     7
...

Signed-off-by: Joe Damato <jdamato@fastly.com>
Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
---
 .../net/ethernet/intel/ixgbe/ixgbe_ethtool.c  | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index 821dfd323fa9..0bbad4a5cc2f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -2665,6 +2665,14 @@ static int ixgbe_get_rss_hash_opts(struct ixgbe_adapter *adapter,
 	return 0;
 }
 
+static int ixgbe_rss_indir_tbl_max(struct ixgbe_adapter *adapter)
+{
+	if (adapter->hw.mac.type < ixgbe_mac_X550)
+		return 16;
+	else
+		return 64;
+}
+
 static int ixgbe_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd,
 			   u32 *rule_locs)
 {
@@ -2673,7 +2681,8 @@ static int ixgbe_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd,
 
 	switch (cmd->cmd) {
 	case ETHTOOL_GRXRINGS:
-		cmd->data = adapter->num_rx_queues;
+		cmd->data = min_t(int, adapter->num_rx_queues,
+				  ixgbe_rss_indir_tbl_max(adapter));
 		ret = 0;
 		break;
 	case ETHTOOL_GRXCLSRLCNT:
@@ -3075,14 +3084,6 @@ static int ixgbe_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd)
 	return ret;
 }
 
-static int ixgbe_rss_indir_tbl_max(struct ixgbe_adapter *adapter)
-{
-	if (adapter->hw.mac.type < ixgbe_mac_X550)
-		return 16;
-	else
-		return 64;
-}
-
 static u32 ixgbe_get_rxfh_key_size(struct net_device *netdev)
 {
 	return IXGBE_RSS_KEY_SIZE;
-- 
2.25.1


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

* Re: [PATCH net v2 1/2] ixgbe: Allow flow hash to be set via ethtool
  2023-04-16 19:12 ` [PATCH net v2 1/2] ixgbe: Allow flow hash to be set via ethtool Joe Damato
@ 2023-04-17  4:57   ` Joe Damato
  2023-04-20 16:35     ` [Intel-wired-lan] " Pucha, HimasekharX Reddy
  0 siblings, 1 reply; 7+ messages in thread
From: Joe Damato @ 2023-04-17  4:57 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: netdev, jesse.brandeburg, anthony.l.nguyen, kuba, sridhar.samudrala

On Sun, Apr 16, 2023 at 07:12:22PM +0000, Joe Damato wrote:
> ixgbe currently returns `EINVAL` whenever the flowhash it set by ethtool
> because the ethtool code in the kernel passes a non-zero value for hfunc
> that ixgbe should allow.
> 
> When ethtool is called with `ETHTOOL_SRXFHINDIR`,
> `ethtool_set_rxfh_indir` will call ixgbe's set_rxfh function
> with `ETH_RSS_HASH_NO_CHANGE`. This value should be accepted.
> 
> When ethtool is called with `ETHTOOL_SRSSH`, `ethtool_set_rxfh` will
> call ixgbe's set_rxfh function with `rxfh.hfunc`, which appears to be
> hardcoded in ixgbe to always be `ETH_RSS_HASH_TOP`. This value should
> also be accepted.
> 
> Before this patch:
> 
> $ sudo ethtool -L eth1 combined 10
> $ sudo ethtool -X eth1 default
> Cannot set RX flow hash configuration: Invalid argument
> 
> After this patch:
> 
> $ sudo ethtool -L eth1 combined 10
> $ sudo ethtool -X eth1 default
> $ sudo ethtool -x eth1
> RX flow hash indirection table for eth1 with 10 RX ring(s):
>     0:      0     1     2     3     4     5     6     7
>     8:      8     9     0     1     2     3     4     5
>    16:      6     7     8     9     0     1     2     3
>    24:      4     5     6     7     8     9     0     1
>    ...
> 

Sorry for the noise, forgot the fixes tag.

Fixes: 1c7cf0784e4d ("ixgbe: support for ethtool set_rxfh")

> Signed-off-by: Joe Damato <jdamato@fastly.com>
> Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> index 6cfc9dc16537..821dfd323fa9 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> @@ -3131,8 +3131,8 @@ static int ixgbe_set_rxfh(struct net_device *netdev, const u32 *indir,
>  	int i;
>  	u32 reta_entries = ixgbe_rss_indir_tbl_entries(adapter);
>  
> -	if (hfunc)
> -		return -EINVAL;
> +	if (hfunc != ETH_RSS_HASH_NO_CHANGE && hfunc != ETH_RSS_HASH_TOP)
> +		return -EOPNOTSUPP;
>  
>  	/* Fill out the redirection table */
>  	if (indir) {
> -- 
> 2.25.1
> 

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

* Re: [PATCH net v2 2/2] ixgbe: Enable setting RSS table to default values
  2023-04-16 19:12 ` [PATCH net v2 2/2] ixgbe: Enable setting RSS table to default values Joe Damato
@ 2023-04-17  4:58   ` Joe Damato
  2023-04-20 16:39     ` [Intel-wired-lan] " Pucha, HimasekharX Reddy
  0 siblings, 1 reply; 7+ messages in thread
From: Joe Damato @ 2023-04-17  4:58 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: netdev, jesse.brandeburg, anthony.l.nguyen, kuba, sridhar.samudrala

On Sun, Apr 16, 2023 at 07:12:23PM +0000, Joe Damato wrote:
> ethtool uses `ETHTOOL_GRXRINGS` to compute how many queues are supported
> by RSS. The driver should return the smaller of either:
>   - The maximum number of RSS queues the device supports, OR
>   - The number of RX queues configured
> 
> Prior to this change, running `ethtool -X $iface default` fails if the
> number of queues configured is larger than the number supported by RSS,
> even though changing the queue count correctly resets the flowhash to
> use all supported queues.
> 
> Other drivers (for example, i40e) will succeed but the flow hash will
> reset to support the maximum number of queues supported by RSS, even if
> that amount is smaller than the configured amount.
> 
> Prior to this change:
> 
> $ sudo ethtool -L eth1 combined 20
> $ sudo ethtool -x eth1
> RX flow hash indirection table for eth1 with 20 RX ring(s):
>     0:      0     1     2     3     4     5     6     7
>     8:      8     9    10    11    12    13    14    15
>    16:      0     1     2     3     4     5     6     7
>    24:      8     9    10    11    12    13    14    15
>    32:      0     1     2     3     4     5     6     7
> ...
> 
> You can see that the flowhash was correctly set to use the maximum
> number of queues supported by the driver (16).
> 
> However, asking the NIC to reset to "default" fails:
> 
> $ sudo ethtool -X eth1 default
> Cannot set RX flow hash configuration: Invalid argument
> 
> After this change, the flowhash can be reset to default which will use
> all of the available RSS queues (16) or the configured queue count,
> whichever is smaller.
> 
> Starting with eth1 which has 10 queues and a flowhash distributing to
> all 10 queues:
> 
> $ sudo ethtool -x eth1
> RX flow hash indirection table for eth1 with 10 RX ring(s):
>     0:      0     1     2     3     4     5     6     7
>     8:      8     9     0     1     2     3     4     5
>    16:      6     7     8     9     0     1     2     3
> ...
> 
> Increasing the queue count to 48 resets the flowhash to distribute to 16
> queues, as it did before this patch:
> 
> $ sudo ethtool -L eth1 combined 48
> $ sudo ethtool -x eth1
> RX flow hash indirection table for eth1 with 16 RX ring(s):
>     0:      0     1     2     3     4     5     6     7
>     8:      8     9    10    11    12    13    14    15
>    16:      0     1     2     3     4     5     6     7
> ...
> 
> Due to the other bugfix in this series, the flowhash can be set to use
> queues 0-5:
> 
> $ sudo ethtool -X eth1 equal 5
> $ sudo ethtool -x eth1
> RX flow hash indirection table for eth1 with 16 RX ring(s):
>     0:      0     1     2     3     4     0     1     2
>     8:      3     4     0     1     2     3     4     0
>    16:      1     2     3     4     0     1     2     3
> ...
> 
> Due to this bugfix, the flowhash can be reset to default and use 16
> queues:
> 
> $ sudo ethtool -X eth1 default
> $ sudo ethtool -x eth1
> RX flow hash indirection table for eth1 with 16 RX ring(s):
>     0:      0     1     2     3     4     5     6     7
>     8:      8     9    10    11    12    13    14    15
>    16:      0     1     2     3     4     5     6     7
> ...

Fixes: 91cd94bfe4f0 ("ixgbe: add basic support for setting and getting nfc
controls")

> Signed-off-by: Joe Damato <jdamato@fastly.com>
> Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> ---
>  .../net/ethernet/intel/ixgbe/ixgbe_ethtool.c  | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> index 821dfd323fa9..0bbad4a5cc2f 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> @@ -2665,6 +2665,14 @@ static int ixgbe_get_rss_hash_opts(struct ixgbe_adapter *adapter,
>  	return 0;
>  }
>  
> +static int ixgbe_rss_indir_tbl_max(struct ixgbe_adapter *adapter)
> +{
> +	if (adapter->hw.mac.type < ixgbe_mac_X550)
> +		return 16;
> +	else
> +		return 64;
> +}
> +
>  static int ixgbe_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd,
>  			   u32 *rule_locs)
>  {
> @@ -2673,7 +2681,8 @@ static int ixgbe_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd,
>  
>  	switch (cmd->cmd) {
>  	case ETHTOOL_GRXRINGS:
> -		cmd->data = adapter->num_rx_queues;
> +		cmd->data = min_t(int, adapter->num_rx_queues,
> +				  ixgbe_rss_indir_tbl_max(adapter));
>  		ret = 0;
>  		break;
>  	case ETHTOOL_GRXCLSRLCNT:
> @@ -3075,14 +3084,6 @@ static int ixgbe_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd)
>  	return ret;
>  }
>  
> -static int ixgbe_rss_indir_tbl_max(struct ixgbe_adapter *adapter)
> -{
> -	if (adapter->hw.mac.type < ixgbe_mac_X550)
> -		return 16;
> -	else
> -		return 64;
> -}
> -
>  static u32 ixgbe_get_rxfh_key_size(struct net_device *netdev)
>  {
>  	return IXGBE_RSS_KEY_SIZE;
> -- 
> 2.25.1
> 

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

* RE: [Intel-wired-lan] [PATCH net v2 1/2] ixgbe: Allow flow hash to be set via ethtool
  2023-04-17  4:57   ` Joe Damato
@ 2023-04-20 16:35     ` Pucha, HimasekharX Reddy
  0 siblings, 0 replies; 7+ messages in thread
From: Pucha, HimasekharX Reddy @ 2023-04-20 16:35 UTC (permalink / raw)
  To: Joe Damato, intel-wired-lan
  Cc: netdev, Nguyen, Anthony L, Brandeburg, Jesse, kuba

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Joe Damato
> Sent: Monday, April 17, 2023 10:28 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Brandeburg, Jesse <jesse.brandeburg@intel.com>; kuba@kernel.org
> Subject: Re: [Intel-wired-lan] [PATCH net v2 1/2] ixgbe: Allow flow hash to be set via ethtool
>
> On Sun, Apr 16, 2023 at 07:12:22PM +0000, Joe Damato wrote:
> ixgbe currently returns `EINVAL` whenever the flowhash it set by 
> ethtool because the ethtool code in the kernel passes a non-zero value 
> for hfunc that ixgbe should allow.
> 
> When ethtool is called with `ETHTOOL_SRXFHINDIR`, 
> `ethtool_set_rxfh_indir` will call ixgbe's set_rxfh function with 
> `ETH_RSS_HASH_NO_CHANGE`. This value should be accepted.
> 
> When ethtool is called with `ETHTOOL_SRSSH`, `ethtool_set_rxfh` will 
> call ixgbe's set_rxfh function with `rxfh.hfunc`, which appears to be 
> hardcoded in ixgbe to always be `ETH_RSS_HASH_TOP`. This value should 
> also be accepted.
> 
> Before this patch:
> 
> $ sudo ethtool -L eth1 combined 10
> $ sudo ethtool -X eth1 default
> Cannot set RX flow hash configuration: Invalid argument
> 
> After this patch:
> 
> $ sudo ethtool -L eth1 combined 10
> $ sudo ethtool -X eth1 default
> $ sudo ethtool -x eth1
> RX flow hash indirection table for eth1 with 10 RX ring(s):
>     0:      0     1     2     3     4     5     6     7
>     8:      8     9     0     1     2     3     4     5
>    16:      6     7     8     9     0     1     2     3
>    24:      4     5     6     7     8     9     0     1
>    ...
> 
>
> Sorry for the noise, forgot the fixes tag.
> 
> Fixes: 1c7cf0784e4d ("ixgbe: support for ethtool set_rxfh")
>
> Signed-off-by: Joe Damato <jdamato@fastly.com>
> Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>

Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)


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

* RE: [Intel-wired-lan] [PATCH net v2 2/2] ixgbe: Enable setting RSS table to default values
  2023-04-17  4:58   ` Joe Damato
@ 2023-04-20 16:39     ` Pucha, HimasekharX Reddy
  0 siblings, 0 replies; 7+ messages in thread
From: Pucha, HimasekharX Reddy @ 2023-04-20 16:39 UTC (permalink / raw)
  To: Joe Damato, intel-wired-lan
  Cc: netdev, Nguyen, Anthony L, Brandeburg, Jesse, kuba

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Joe Damato
> Sent: Monday, April 17, 2023 10:29 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Brandeburg, Jesse <jesse.brandeburg@intel.com>; kuba@kernel.org
> Subject: Re: [Intel-wired-lan] [PATCH net v2 2/2] ixgbe: Enable setting RSS table to default values
> 
> On Sun, Apr 16, 2023 at 07:12:23PM +0000, Joe Damato wrote:
> ethtool uses `ETHTOOL_GRXRINGS` to compute how many queues are 
> supported by RSS. The driver should return the smaller of either:
>   - The maximum number of RSS queues the device supports, OR
>   - The number of RX queues configured
> 
> Prior to this change, running `ethtool -X $iface default` fails if the 
> number of queues configured is larger than the number supported by 
> RSS, even though changing the queue count correctly resets the 
> flowhash to use all supported queues.
> 
> Other drivers (for example, i40e) will succeed but the flow hash will 
> reset to support the maximum number of queues supported by RSS, even 
> if that amount is smaller than the configured amount.
> 
> Prior to this change:
> 
> $ sudo ethtool -L eth1 combined 20
> $ sudo ethtool -x eth1
> RX flow hash indirection table for eth1 with 20 RX ring(s):
>     0:      0     1     2     3     4     5     6     7
>     8:      8     9    10    11    12    13    14    15
>    16:      0     1     2     3     4     5     6     7
>    24:      8     9    10    11    12    13    14    15
>    32:      0     1     2     3     4     5     6     7
> ...
> 
> You can see that the flowhash was correctly set to use the maximum 
> number of queues supported by the driver (16).
> 
> However, asking the NIC to reset to "default" fails:
> 
> $ sudo ethtool -X eth1 default
> Cannot set RX flow hash configuration: Invalid argument
> 
> After this change, the flowhash can be reset to default which will use 
> all of the available RSS queues (16) or the configured queue count, 
> whichever is smaller.
> 
> Starting with eth1 which has 10 queues and a flowhash distributing to 
> all 10 queues:
> 
> $ sudo ethtool -x eth1
> RX flow hash indirection table for eth1 with 10 RX ring(s):
>     0:      0     1     2     3     4     5     6     7
>     8:      8     9     0     1     2     3     4     5
>    16:      6     7     8     9     0     1     2     3
> ...
> 
> Increasing the queue count to 48 resets the flowhash to distribute to 
> 16 queues, as it did before this patch:
> 
> $ sudo ethtool -L eth1 combined 48
> $ sudo ethtool -x eth1
> RX flow hash indirection table for eth1 with 16 RX ring(s):
>     0:      0     1     2     3     4     5     6     7
>     8:      8     9    10    11    12    13    14    15
>    16:      0     1     2     3     4     5     6     7
> ...
> 
> Due to the other bugfix in this series, the flowhash can be set to use 
> queues 0-5:
> 
> $ sudo ethtool -X eth1 equal 5
> $ sudo ethtool -x eth1
> RX flow hash indirection table for eth1 with 16 RX ring(s):
>     0:      0     1     2     3     4     0     1     2
>     8:      3     4     0     1     2     3     4     0
>    16:      1     2     3     4     0     1     2     3
> ...
> 
> Due to this bugfix, the flowhash can be reset to default and use 16
> queues:
> 
> $ sudo ethtool -X eth1 default
> $ sudo ethtool -x eth1
> RX flow hash indirection table for eth1 with 16 RX ring(s):
>     0:      0     1     2     3     4     5     6     7
>     8:      8     9    10    11    12    13    14    15
>    16:      0     1     2     3     4     5     6     7
> ...
>
> Fixes: 91cd94bfe4f0 ("ixgbe: add basic support for setting and getting nfc
controls")
>
> Signed-off-by: Joe Damato <jdamato@fastly.com>
> Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> ---
>  .../net/ethernet/intel/ixgbe/ixgbe_ethtool.c  | 19 
> ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 

Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)

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

end of thread, other threads:[~2023-04-20 16:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-16 19:12 [PATCH net v2 0/2] ixgbe: Multiple RSS bugfixes Joe Damato
2023-04-16 19:12 ` [PATCH net v2 1/2] ixgbe: Allow flow hash to be set via ethtool Joe Damato
2023-04-17  4:57   ` Joe Damato
2023-04-20 16:35     ` [Intel-wired-lan] " Pucha, HimasekharX Reddy
2023-04-16 19:12 ` [PATCH net v2 2/2] ixgbe: Enable setting RSS table to default values Joe Damato
2023-04-17  4:58   ` Joe Damato
2023-04-20 16:39     ` [Intel-wired-lan] " Pucha, HimasekharX Reddy

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).