[Intel-wired-lan] [PATCH iwl-next 0/1] ice: Fix persistent failure in ice_get_rxfh

[Intel-wired-lan] [PATCH iwl-next 0/1] ice: Fix persistent failure in ice_get_rxfh

[Cody Haas]

Hey all,

This is a small bug fix for an issue I found when testing some e810 NICs
on kernel 6.16.5-200.fc42.x86_64. I originally reported the bug in
https://lore.kernel.org/intel-wired-lan/CAH7f-UKkJV8MLY7zCdgCrGE55whRhbGAXvgkDnwgiZ9gUZT7_w@mail.gmail.com/


User Impact of the Bug
----------------------
In the current in-tree ice driver, if a user tries to get the
indirection table using the SIOCETHTOOL command and the
ETHTOOL_GRXFHINDIR subcommand the subsequent function call with always
fail with -EINVAL. 

Cause of the Bug
----------------
When a user gets the indirection table using SIOCETHTOOL and ETHTOOL_GRXFHINDIR
ethtool_get_rxfh_indir is called. This function will end up calling
ice_get_rxfh which then calls ice_get_rss_key. The function
ice_get_rss_key expects its *seed parameter to never be null. This *seed
parameter is the key field in ethtool_rxfh_param. Neither ice_get_rxfh
nor ethtool_get_rxfh_indir set this value, so it will always be null.
This causes the *seed parameter for ice_get_rss_key to always be null,
ultimately causing ice_get_rss_key to always return -EINVAL.


Fix for the Bug
---------------
To fix this, I went ahead and implemented ice_get_rss which checks if
the rss_key is not null before calling ice_key_rss_key. This follows
suit with the i40e driver's implementation.

Cody Haas (1):
  ice: Fix persistent failure in ice_get_rxfh

 drivers/net/ethernet/intel/ice/ice.h         |  1 +
 drivers/net/ethernet/intel/ice/ice_ethtool.c |  6 +----
 drivers/net/ethernet/intel/ice/ice_main.c    | 28 ++++++++++++++++++++
 3 files changed, 30 insertions(+), 5 deletions(-)


base-commit: 67181985211850332c8ff942815c1961fd7058b9
-- 
2.50.0

[Intel-wired-lan] [PATCH iwl-next 1/1] ice: Fix persistent failure in ice_get_rxfh

[Cody Haas]

Several ioctl functions have the ability to call ice_get_rxfh, however
all of these iotctl functions do not provide all of the expected
information in ethtool_rxfh_param. For example,ethtool_get_rxfh_indir does
not provide an rss_key. Previously, caused ethtool_get_rxfh_indir to
always fail with an -EINVAL.

This change draws inspiration from i40e_get_rss to handle this
situation, by only calling the appropriate rss helpers when the
necessary information has been provided via ethtool_rxfh_param.

Signed-off-by: Cody Haas <chaas@riotgames.com>
Closes: https://lore.kernel.org/intel-wired-lan/CAH7f-UKkJV8MLY7zCdgCrGE55whRhbGAXvgkDnwgiZ9gUZT7_w@mail.gmail.com/ 

---
 drivers/net/ethernet/intel/ice/ice.h         |  1 +
 drivers/net/ethernet/intel/ice/ice_ethtool.c |  6 +----
 drivers/net/ethernet/intel/ice/ice_main.c    | 28 ++++++++++++++++++++
 3 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index c9104b13e1d2..87f4098324ed 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -953,6 +953,7 @@ void ice_map_xdp_rings(struct ice_vsi *vsi);
 int
 ice_xdp_xmit(struct net_device *dev, int n, struct xdp_frame **frames,
 	     u32 flags);
+int ice_get_rss(struct ice_vsi *vsi, u8 *seed, u8 *lut, u16 lut_size);
 int ice_set_rss_lut(struct ice_vsi *vsi, u8 *lut, u16 lut_size);
 int ice_get_rss_lut(struct ice_vsi *vsi, u8 *lut, u16 lut_size);
 int ice_set_rss_key(struct ice_vsi *vsi, u8 *seed);
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index b0805704834d..a5c139cc536d 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -3649,11 +3649,7 @@ ice_get_rxfh(struct net_device *netdev, struct ethtool_rxfh_param *rxfh)
 	if (!lut)
 		return -ENOMEM;
 
-	err = ice_get_rss_key(vsi, rxfh->key);
-	if (err)
-		goto out;
-
-	err = ice_get_rss_lut(vsi, lut, vsi->rss_table_size);
+	err = ice_get_rss(vsi, rxfh->key, lut, vsi->rss_table_size);
 	if (err)
 		goto out;
 
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index b084839eb811..7b409b9fca5c 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -8072,6 +8072,34 @@ int ice_get_rss_key(struct ice_vsi *vsi, u8 *seed)
 	return status;
 }
 
+/**
+ * ice_get_rss - Get RSS LUT and/or key
+ * @vsi: Pointer to VSI structure
+ * @seed: Buffer to store the key in
+ * @lut: Buffer to store the lookup table entries
+ * @lut_size: Size of buffer to store the lookup table entries
+ *
+ * Returns 0 on success, negative on failure
+ */
+int ice_get_rss(struct ice_vsi *vsi, u8 *seed, u8 *lut, u16 lut_size)
+{
+	int status = 0;
+
+	if (lut) {
+		status = ice_get_rss_lut(vsi, lut, lut_size);
+		if (status)
+			return status;
+	}
+
+	if (seed) {
+		status = ice_get_rss_key(vsi, seed);
+		if (status)
+			return status;
+	}
+
+	return status;
+}
+
 /**
  * ice_set_rss_hfunc - Set RSS HASH function
  * @vsi: Pointer to VSI structure
-- 
2.50.0

Re: [Intel-wired-lan] [PATCH iwl-next 1/1] ice: Fix persistent failure in ice_get_rxfh

[Przemek Kitszel]

On 12/3/25 19:49, Cody Haas wrote:
> Several ioctl functions have the ability to call ice_get_rxfh, however
> all of these iotctl functions do not provide all of the expected
> information in ethtool_rxfh_param. For example,ethtool_get_rxfh_indir does
> not provide an rss_key. Previously, caused ethtool_get_rxfh_indir to
> always fail with an -EINVAL.
> 
> This change draws inspiration from i40e_get_rss to handle this
> situation, by only calling the appropriate rss helpers when the
> necessary information has been provided via ethtool_rxfh_param.
> 
> Signed-off-by: Cody Haas <chaas@riotgames.com>
> Closes: https://lore.kernel.org/intel-wired-lan/CAH7f-UKkJV8MLY7zCdgCrGE55whRhbGAXvgkDnwgiZ9gUZT7_w@mail.gmail.com/

thank you again for the report, and thank you for the fix, it looks good
just some little nitpicks from me:

1. this is a bugfix, so you should add a Fixes: tag with the commit that
added the regression (I remember you have a "slow to rebuild" platform,
so just let me know how far you have reached with bisection/looking for
the root cause)
2. bugfixes should have [PATCH iwl-net] in the Subject
3. you should CC netdev mailing list on IWL submissions too:
	netdev@vger.kernel.org
4. minor thing in the code, see below

> 
> ---
>   drivers/net/ethernet/intel/ice/ice.h         |  1 +
>   drivers/net/ethernet/intel/ice/ice_ethtool.c |  6 +----
>   drivers/net/ethernet/intel/ice/ice_main.c    | 28 ++++++++++++++++++++
>   3 files changed, 30 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
> index c9104b13e1d2..87f4098324ed 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -953,6 +953,7 @@ void ice_map_xdp_rings(struct ice_vsi *vsi);
>   int
>   ice_xdp_xmit(struct net_device *dev, int n, struct xdp_frame **frames,
>   	     u32 flags);
> +int ice_get_rss(struct ice_vsi *vsi, u8 *seed, u8 *lut, u16 lut_size);
>   int ice_set_rss_lut(struct ice_vsi *vsi, u8 *lut, u16 lut_size);
>   int ice_get_rss_lut(struct ice_vsi *vsi, u8 *lut, u16 lut_size);
>   int ice_set_rss_key(struct ice_vsi *vsi, u8 *seed);
> diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> index b0805704834d..a5c139cc536d 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> @@ -3649,11 +3649,7 @@ ice_get_rxfh(struct net_device *netdev, struct ethtool_rxfh_param *rxfh)
>   	if (!lut)
>   		return -ENOMEM;
>   
> -	err = ice_get_rss_key(vsi, rxfh->key);
> -	if (err)
> -		goto out;
> -
> -	err = ice_get_rss_lut(vsi, lut, vsi->rss_table_size);
> +	err = ice_get_rss(vsi, rxfh->key, lut, vsi->rss_table_size);
>   	if (err)
>   		goto out;
>   
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index b084839eb811..7b409b9fca5c 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -8072,6 +8072,34 @@ int ice_get_rss_key(struct ice_vsi *vsi, u8 *seed)
>   	return status;
>   }
>   
> +/**
> + * ice_get_rss - Get RSS LUT and/or key
> + * @vsi: Pointer to VSI structure
> + * @seed: Buffer to store the key in
> + * @lut: Buffer to store the lookup table entries
> + * @lut_size: Size of buffer to store the lookup table entries
> + *
> + * Returns 0 on success, negative on failure
> + */
> +int ice_get_rss(struct ice_vsi *vsi, u8 *seed, u8 *lut, u16 lut_size)
> +{
> +	int status = 0;
> +
> +	if (lut) {
> +		status = ice_get_rss_lut(vsi, lut, lut_size);
> +		if (status)
> +			return status;
> +	}
> +
> +	if (seed) {
> +		status = ice_get_rss_key(vsi, seed);
> +		if (status)
> +			return status;
> +	}
> +
> +	return status;

nit: you could simply "return 0" here
then the status variable initialization during declaration could be
removed

yet another thing: for new code I would name such variable "err"


> +}
> +
>   /**
>    * ice_set_rss_hfunc - Set RSS HASH function
>    * @vsi: Pointer to VSI structure

Re: [Intel-wired-lan] [PATCH iwl-next 1/1] ice: Fix persistent failure in ice_get_rxfh

[Cody Haas]

On Fri, Dec 5, 2025 at 2:36 AM Przemek Kitszel
<przemyslaw.kitszel@intel.com> wrote:
> thank you again for the report, and thank you for the fix, it looks good
> just some little nitpicks from me:

Thank you for the response, I'll get working on the feedback right away.

> 1. this is a bugfix, so you should add a Fixes: tag with the commit that
> added the regression (I remember you have a "slow to rebuild" platform,
> so just let me know how far you have reached with bisection/looking for
> the root cause)

We recently moved the e810 NICs we have onto a server with a much larger CPU so
compilation time is no longer a concern. I believe the regression was
introduced in b66a972.
So I'm currently in the process of bisecting to verify my assumption.
Just running into an
issue with building the 5.12 kernel. I'll be reaching out to the
kernel newbies mailing list for
some advice on handling the compilation issues.

> 2. bugfixes should have [PATCH iwl-net] in the Subject
> 3. you should CC netdev mailing list on IWL submissions too:
>         netdev@vger.kernel.org

Acking these two pieces of feedback, I'll add them.

> nit: you could simply "return 0" here
> then the status variable initialization during declaration could be
> removed
>
> yet another thing: for new code I would name such variable "err"

Acking these two pieces of feedback, I'll update the code accordingly.

Thank you for your time

Cody

Re: [Intel-wired-lan] [PATCH iwl-next 1/1] ice: Fix persistent failure in ice_get_rxfh

[Dawid Osuchowski]

Hey Cody,

Thank you for finding the bug and proposing a fix. In addition to 
Przemek's review, below are some minor nitpicks from me, otherwise looks 
good to me!

On 2025-12-03 7:49 PM, Cody Haas wrote:
> Several ioctl functions have the ability to call ice_get_rxfh, however
> all of these iotctl functions do not provide all of the expected

typo s/iotctl/ioctl

> information in ethtool_rxfh_param. For example,ethtool_get_rxfh_indir does

punctuation: missing space after "For example,"

> not provide an rss_key. Previously, caused ethtool_get_rxfh_indir to
> always fail with an -EINVAL.
> 
> This change draws inspiration from i40e_get_rss to handle this
> situation, by only calling the appropriate rss helpers when the
> necessary information has been provided via ethtool_rxfh_param.
> 
> Signed-off-by: Cody Haas <chaas@riotgames.com>
> Closes: https://lore.kernel.org/intel-wired-lan/CAH7f-UKkJV8MLY7zCdgCrGE55whRhbGAXvgkDnwgiZ9gUZT7_w@mail.gmail.com/
> 
> ---
>   drivers/net/ethernet/intel/ice/ice.h         |  1 +
>   drivers/net/ethernet/intel/ice/ice_ethtool.c |  6 +----
>   drivers/net/ethernet/intel/ice/ice_main.c    | 28 ++++++++++++++++++++
>   3 files changed, 30 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
> index c9104b13e1d2..87f4098324ed 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -953,6 +953,7 @@ void ice_map_xdp_rings(struct ice_vsi *vsi);
>   int
>   ice_xdp_xmit(struct net_device *dev, int n, struct xdp_frame **frames,
>   	     u32 flags);
> +int ice_get_rss(struct ice_vsi *vsi, u8 *seed, u8 *lut, u16 lut_size);
>   int ice_set_rss_lut(struct ice_vsi *vsi, u8 *lut, u16 lut_size);
>   int ice_get_rss_lut(struct ice_vsi *vsi, u8 *lut, u16 lut_size);
>   int ice_set_rss_key(struct ice_vsi *vsi, u8 *seed);
> diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> index b0805704834d..a5c139cc536d 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> @@ -3649,11 +3649,7 @@ ice_get_rxfh(struct net_device *netdev, struct ethtool_rxfh_param *rxfh)
>   	if (!lut)
>   		return -ENOMEM;
>   
> -	err = ice_get_rss_key(vsi, rxfh->key);
> -	if (err)
> -		goto out;
> -
> -	err = ice_get_rss_lut(vsi, lut, vsi->rss_table_size);
> +	err = ice_get_rss(vsi, rxfh->key, lut, vsi->rss_table_size);
>   	if (err)
>   		goto out;
>   
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index b084839eb811..7b409b9fca5c 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -8072,6 +8072,34 @@ int ice_get_rss_key(struct ice_vsi *vsi, u8 *seed)
>   	return status;
>   }
>   
> +/**
> + * ice_get_rss - Get RSS LUT and/or key
> + * @vsi: Pointer to VSI structure
> + * @seed: Buffer to store the key in
> + * @lut: Buffer to store the lookup table entries
> + * @lut_size: Size of buffer to store the lookup table entries
> + *
> + * Returns 0 on success, negative on failure
> + */
> +int ice_get_rss(struct ice_vsi *vsi, u8 *seed, u8 *lut, u16 lut_size)
> +{
> +	int status = 0;
> +
> +	if (lut) {

nit: in function arguments "seed" appears first, maybe consider 
switching the order of the if statements so they appear in the same 
order as in function declaration? It would also match the i40e 
implementation then

Thanks,
Dawid

> +		status = ice_get_rss_lut(vsi, lut, lut_size);
> +		if (status)
> +			return status;
> +	}
> +
> +	if (seed) {
> +		status = ice_get_rss_key(vsi, seed);
> +		if (status)
> +			return status;
> +	}
> +
> +	return status;
> +}
> +
>   /**
>    * ice_set_rss_hfunc - Set RSS HASH function
>    * @vsi: Pointer to VSI structure

Re: [Intel-wired-lan] [PATCH iwl-next 1/1] ice: Fix persistent failure in ice_get_rxfh

[Cody Haas]

Thanks for the review, Dawid,

On Tue, Dec 9, 2025 at 12:51 AM Dawid Osuchowski
<dawid.osuchowski@linux.intel.com> wrote:
> typo s/iotctl/ioctl

Acked — I'll go ahead and fix this.

> punctuation: missing space after "For example,"

I'll fix this as well.

> nit: in function arguments "seed" appears first, maybe consider
> switching the order of the if statements so they appear in the same
> order as in function declaration? It would also match the i40e
> implementation then

Same with this one. Thanks for taking the time to review it.

Cody