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 <[email protected]>
Closes: 
https://lore.kernel.org/intel-wired-lan/cah7f-ukkjv8mly7zcdgcrge55whrhbgaxvgkdnwgiz9guzt...@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

Reply via email to