public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH iwl-next v1] ice: implement symmetric RSS hash configuration
@ 2026-02-13  3:44 Aleksandr Loktionov
  2026-02-13 15:01 ` [iwl-next,v1] " Simon Horman
  0 siblings, 1 reply; 3+ messages in thread
From: Aleksandr Loktionov @ 2026-02-13  3:44 UTC (permalink / raw)
  To: intel-wired-lan, anthony.l.nguyen, aleksandr.loktionov; +Cc: netdev

The driver advertises symmetric RSS support via supported_input_xfrm
but ice_set_rxfh() ignored the input_xfrm parameter, making it
impossible to configure symmetric hashing.

Fix ice_set_rxfh() to check rxfh->input_xfrm and call
ice_set_rss_hfunc() with ICE_AQ_VSI_Q_OPT_RSS_HASH_SYM_TPLZ when
RXH_XFRM_SYM_XOR is requested.

Modify ice_set_rss_vsi_ctx() to use vsi->rss_hfunc instead of
hardcoding ICE_AQ_VSI_Q_OPT_RSS_HASH_TPLZ, and export it for
use by ice_set_rxfh().

The kernel requires get_rxfh_fields() to report symmetric hash
configurations when symmetric transforms are supported. Update
ice_get_rxfh_fields() to return symmetric field configuration
(src+dst IP and ports) for all flow types.

Tested with tools/testing/selftests/drivers/net/hw/rss_input_xfrm.py

Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_ethtool.c | 31 +++++++++++++++-----
 drivers/net/ethernet/intel/ice/ice_lib.c     |  6 ++--
 drivers/net/ethernet/intel/ice/ice_lib.h     |  1 +
 3 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index c6bc29c..6dc37f9 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -3037,8 +3037,10 @@ ice_get_rxfh_fields(struct net_device *netdev, struct ethtool_rxfh_fields *nfc)
 
 	hash_flds = ice_get_rss_cfg(&pf->hw, vsi->idx, hdrs, &symm);
 	if (hash_flds == ICE_HASH_INVALID) {
-		dev_dbg(dev, "No hash fields found for the given header type, vsi num = %d\n",
+		/* Provide default symmetric hash fields when no config exists */
+		dev_dbg(dev, "No RSS config for this flow, using symmetric defaults, vsi num = %d\n",
 			vsi->vsi_num);
+		nfc->data = RXH_IP_SRC | RXH_IP_DST | RXH_L4_B_0_1 | RXH_L4_B_2_3;
 		return 0;
 	}
 
@@ -3067,6 +3069,7 @@ ice_get_rxfh_fields(struct net_device *netdev, struct ethtool_rxfh_fields *nfc)
 	    hash_flds & ICE_FLOW_HASH_FLD_GTPU_DWN_TEID)
 		nfc->data |= (u64)RXH_GTP_TEID;
 
+	nfc->data = RXH_IP_SRC | RXH_IP_DST | RXH_L4_B_0_1 | RXH_L4_B_2_3;
 	return 0;
 }
 
@@ -3667,7 +3670,6 @@ ice_set_rxfh(struct net_device *netdev, struct ethtool_rxfh_param *rxfh,
 	     struct netlink_ext_ack *extack)
 {
 	struct ice_netdev_priv *np = netdev_priv(netdev);
-	u8 hfunc = ICE_AQ_VSI_Q_OPT_RSS_HASH_TPLZ;
 	struct ice_vsi *vsi = np->vsi;
 	struct ice_pf *pf = vsi->back;
 	struct device *dev;
@@ -3689,13 +3691,26 @@ ice_set_rxfh(struct net_device *netdev, struct ethtool_rxfh_param *rxfh,
 		return -EOPNOTSUPP;
 	}
 
-	/* Update the VSI's hash function */
-	if (rxfh->input_xfrm & RXH_XFRM_SYM_XOR)
-		hfunc = ICE_AQ_VSI_Q_OPT_RSS_HASH_SYM_TPLZ;
+	/* Handle RSS symmetric hash transformation */
+	if (rxfh->input_xfrm != RXH_XFRM_NO_CHANGE) {
+		u8 new_hfunc;
 
-	err = ice_set_rss_hfunc(vsi, hfunc);
-	if (err)
-		return err;
+		if (rxfh->input_xfrm & RXH_XFRM_SYM_XOR)
+			new_hfunc = ICE_AQ_VSI_Q_OPT_RSS_HASH_SYM_TPLZ;
+		else
+			new_hfunc = ICE_AQ_VSI_Q_OPT_RSS_HASH_TPLZ;
+
+		if (new_hfunc != vsi->rss_hfunc) {
+			err = ice_set_rss_hfunc(vsi, new_hfunc);
+			if (err) {
+				netdev_err(netdev, "Failed to set RSS hash function\n");
+				return err;
+			}
+			netdev_info(netdev, "RSS hash function: %sToeplitz\n",
+				    new_hfunc == ICE_AQ_VSI_Q_OPT_RSS_HASH_SYM_TPLZ ?
+				    "Symmetric " : "");
+		}
+	}
 
 	if (rxfh->key) {
 		if (!vsi->rss_hkey_user) {
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index d921269..31f2757 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -1155,7 +1155,7 @@ static void ice_set_fd_vsi_ctx(struct ice_vsi_ctx *ctxt, struct ice_vsi *vsi)
  * @ctxt: the VSI context being set
  * @vsi: the VSI being configured
  */
-static void ice_set_rss_vsi_ctx(struct ice_vsi_ctx *ctxt, struct ice_vsi *vsi)
+void ice_set_rss_vsi_ctx(struct ice_vsi_ctx *ctxt, struct ice_vsi *vsi)
 {
 	u8 lut_type, hash_type;
 	struct device *dev;
@@ -1181,7 +1181,9 @@ static void ice_set_rss_vsi_ctx(struct ice_vsi_ctx *ctxt, struct ice_vsi *vsi)
 		return;
 	}
 
-	hash_type = ICE_AQ_VSI_Q_OPT_RSS_HASH_TPLZ;
+        if (!vsi->rss_hfunc)
+		vsi->rss_hfunc = ICE_AQ_VSI_Q_OPT_RSS_HASH_TPLZ;
+	hash_type = vsi->rss_hfunc;
 	vsi->rss_hfunc = hash_type;
 
 	ctxt->info.q_opt_rss =
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.h b/drivers/net/ethernet/intel/ice/ice_lib.h
index 49454d98..29ba335 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.h
+++ b/drivers/net/ethernet/intel/ice/ice_lib.h
@@ -46,6 +46,7 @@ void ice_vsi_delete(struct ice_vsi *vsi);
 int ice_vsi_cfg_tc(struct ice_vsi *vsi, u8 ena_tc);
 
 int ice_vsi_cfg_rss_lut_key(struct ice_vsi *vsi);
+void ice_set_rss_vsi_ctx(struct ice_vsi_ctx *ctxt, struct ice_vsi *vsi);
 
 void ice_vsi_cfg_netdev_tc(struct ice_vsi *vsi, u8 ena_tc);
 
-- 
2.52.0


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

* Re: [iwl-next,v1] ice: implement symmetric RSS hash configuration
  2026-02-13  3:44 [PATCH iwl-next v1] ice: implement symmetric RSS hash configuration Aleksandr Loktionov
@ 2026-02-13 15:01 ` Simon Horman
  2026-02-13 16:03   ` Loktionov, Aleksandr
  0 siblings, 1 reply; 3+ messages in thread
From: Simon Horman @ 2026-02-13 15:01 UTC (permalink / raw)
  To: aleksandr.loktionov
  Cc: Simon Horman, anthony.l.nguyen, intel-wired-lan, netdev

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
ice: implement symmetric RSS hash configuration

The driver advertises symmetric RSS support via supported_input_xfrm
but ice_set_rxfh() ignored the input_xfrm parameter, making it
impossible to configure symmetric hashing. This patch fixes
ice_set_rxfh() to check the input_xfrm parameter and configure
symmetric hashing appropriately, modifies ice_set_rss_vsi_ctx() to use
vsi->rss_hfunc instead of hardcoding values, and updates
ice_get_rxfh_fields() to report symmetric field configurations.

> diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> index c6bc29cfb8e6..6dc37f96b57e 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> @@ -3037,8 +3037,10 @@ ice_get_rxfh_fields(struct net_device *netdev, struct ethtool_rxfh_fields *nfc)
>
>  	hash_flds = ice_get_rss_cfg(&pf->hw, vsi->idx, hdrs, &symm);
>  	if (hash_flds == ICE_HASH_INVALID) {
> -		dev_dbg(dev, "No hash fields found for the given header type, vsi num = %d\n",
> +		/* Provide default symmetric hash fields when no config exists */
> +		dev_dbg(dev, "No RSS config for this flow, using symmetric defaults, vsi num = %d\n",
>  			vsi->vsi_num);
> +		nfc->data = RXH_IP_SRC | RXH_IP_DST | RXH_L4_B_0_1 | RXH_L4_B_2_3;
>  		return 0;
>  	}
>
> @@ -3067,6 +3069,7 @@ ice_get_rxfh_fields(struct net_device *netdev, struct ethtool_rxfh_fields *nfc)
>  	    hash_flds & ICE_FLOW_HASH_FLD_GTPU_DWN_TEID)
>  		nfc->data |= (u64)RXH_GTP_TEID;
>
> +	nfc->data = RXH_IP_SRC | RXH_IP_DST | RXH_L4_B_0_1 | RXH_L4_B_2_3;
                  ^^^^^^^

Does this unconditional assignment at the end of ice_get_rxfh_fields()
overwrite the actual hardware configuration that was just computed?

The hash fields are carefully decoded from the hardware configuration
returned by ice_get_rss_cfg() and built up in nfc->data through the series
of bitwise OR operations. Then this line uses assignment (=) rather than
bitwise OR (|=), replacing all those computed values with a fixed set of
hash fields.

This would make the entire per-field computation from the hardware state
become dead code. The function would always report the same fixed hash
fields (RXH_IP_SRC | RXH_IP_DST | RXH_L4_B_0_1 | RXH_L4_B_2_3) regardless
of what is actually configured in hardware. GTP TEID hash fields would
never be reported even when configured.

Was this intended to be an additional |= operation, or should this line
be removed entirely?

> @@ -3667,7 +3670,6 @@ ice_set_rxfh(struct net_device *netdev, struct ethtool_rxfh_param *rxfh,
>  	     struct netlink_ext_ack *extack)
>  {
>  	struct ice_netdev_priv *np = netdev_priv(netdev);
> -	u8 hfunc = ICE_AQ_VSI_Q_OPT_RSS_HASH_TPLZ;
>  	struct ice_vsi *vsi = np->vsi;
>  	struct ice_pf *pf = vsi->back;
>  	struct device *dev;

[ ... ]

> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
> index d921269e1fe7..31f2757cf7fa 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -1155,7 +1155,7 @@ static void ice_set_fd_vsi_ctx(struct ice_vsi_ctx *ctxt, struct ice_vsi *vsi)
>   * @ctxt: the VSI context being set
>   * @vsi: the VSI being configured
>   */
> -static void ice_set_rss_vsi_ctx(struct ice_vsi_ctx *ctxt, struct ice_vsi *vsi)
> +void ice_set_rss_vsi_ctx(struct ice_vsi_ctx *ctxt, struct ice_vsi *vsi)
>  {
>  	u8 lut_type, hash_type;
>  	struct device *dev;
> @@ -1181,7 +1181,9 @@ static void ice_set_rss_vsi_ctx(struct ice_vsi_ctx *ctxt, struct ice_vsi *vsi)
>  		return;
>  	}
>
> -	hash_type = ICE_AQ_VSI_Q_OPT_RSS_HASH_TPLZ;
> +        if (!vsi->rss_hfunc)
> +		vsi->rss_hfunc = ICE_AQ_VSI_Q_OPT_RSS_HASH_TPLZ;
> +	hash_type = vsi->rss_hfunc;
>  	vsi->rss_hfunc = hash_type;
>
>  	ctxt->info.q_opt_rss =

[ ... ]

> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.h b/drivers/net/ethernet/intel/ice/ice_lib.h
> index 49454d98dcfe..29ba335dcc54 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.h
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.h
> @@ -46,6 +46,7 @@ void ice_vsi_delete(struct ice_vsi *vsi);
>  int ice_vsi_cfg_tc(struct ice_vsi *vsi, u8 ena_tc);
>
>  int ice_vsi_cfg_rss_lut_key(struct ice_vsi *vsi);
> +void ice_set_rss_vsi_ctx(struct ice_vsi_ctx *ctxt, struct ice_vsi *vsi);
>
>  void ice_vsi_cfg_netdev_tc(struct ice_vsi *vsi, u8 ena_tc);
>

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

* RE: [iwl-next,v1] ice: implement symmetric RSS hash configuration
  2026-02-13 15:01 ` [iwl-next,v1] " Simon Horman
@ 2026-02-13 16:03   ` Loktionov, Aleksandr
  0 siblings, 0 replies; 3+ messages in thread
From: Loktionov, Aleksandr @ 2026-02-13 16:03 UTC (permalink / raw)
  To: Simon Horman
  Cc: Nguyen, Anthony L, intel-wired-lan@lists.osuosl.org,
	netdev@vger.kernel.org



> -----Original Message-----
> From: Simon Horman <horms@kernel.org>
> Sent: Friday, February 13, 2026 4:02 PM
> To: Loktionov, Aleksandr <aleksandr.loktionov@intel.com>
> Cc: Simon Horman <horms@kernel.org>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; intel-wired-lan@lists.osuosl.org;
> netdev@vger.kernel.org
> Subject: Re: [iwl-next,v1] ice: implement symmetric RSS hash
> configuration
> 
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> 
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-
> local.html
> ---
> ice: implement symmetric RSS hash configuration
> 
> The driver advertises symmetric RSS support via supported_input_xfrm
> but ice_set_rxfh() ignored the input_xfrm parameter, making it
> impossible to configure symmetric hashing. This patch fixes
> ice_set_rxfh() to check the input_xfrm parameter and configure
> symmetric hashing appropriately, modifies ice_set_rss_vsi_ctx() to use
> vsi->rss_hfunc instead of hardcoding values, and updates
> ice_get_rxfh_fields() to report symmetric field configurations.
> 
> > diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> > b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> > index c6bc29cfb8e6..6dc37f96b57e 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> > @@ -3037,8 +3037,10 @@ ice_get_rxfh_fields(struct net_device
> *netdev,
> > struct ethtool_rxfh_fields *nfc)
> >
> >  	hash_flds = ice_get_rss_cfg(&pf->hw, vsi->idx, hdrs, &symm);
> >  	if (hash_flds == ICE_HASH_INVALID) {
> > -		dev_dbg(dev, "No hash fields found for the given header
> type, vsi num = %d\n",
> > +		/* Provide default symmetric hash fields when no config
> exists */
> > +		dev_dbg(dev, "No RSS config for this flow, using
> symmetric
> > +defaults, vsi num = %d\n",
> >  			vsi->vsi_num);
> > +		nfc->data = RXH_IP_SRC | RXH_IP_DST | RXH_L4_B_0_1 |
> RXH_L4_B_2_3;
> >  		return 0;
> >  	}
> >
> > @@ -3067,6 +3069,7 @@ ice_get_rxfh_fields(struct net_device *netdev,
> struct ethtool_rxfh_fields *nfc)
> >  	    hash_flds & ICE_FLOW_HASH_FLD_GTPU_DWN_TEID)
> >  		nfc->data |= (u64)RXH_GTP_TEID;
> >
> > +	nfc->data = RXH_IP_SRC | RXH_IP_DST | RXH_L4_B_0_1 |
> RXH_L4_B_2_3;
>                   ^^^^^^^
> 
> Does this unconditional assignment at the end of ice_get_rxfh_fields()
> overwrite the actual hardware configuration that was just computed?
> 
> The hash fields are carefully decoded from the hardware configuration
> returned by ice_get_rss_cfg() and built up in nfc->data through the
> series of bitwise OR operations. Then this line uses assignment (=)
> rather than bitwise OR (|=), replacing all those computed values with
> a fixed set of hash fields.
> 
> This would make the entire per-field computation from the hardware
> state become dead code. The function would always report the same
> fixed hash fields (RXH_IP_SRC | RXH_IP_DST | RXH_L4_B_0_1 |
> RXH_L4_B_2_3) regardless of what is actually configured in hardware.
> GTP TEID hash fields would never be reported even when configured.
> 
> Was this intended to be an additional |= operation, or should this
> line be removed entirely?
> 
I've found the issue today running GTPU tests.
Will fix in in v3 on Monday.

With the best regards
Alex

> > @@ -3667,7 +3670,6 @@ ice_set_rxfh(struct net_device *netdev, struct
> ethtool_rxfh_param *rxfh,
> >  	     struct netlink_ext_ack *extack)  {
> >  	struct ice_netdev_priv *np = netdev_priv(netdev);
> > -	u8 hfunc = ICE_AQ_VSI_Q_OPT_RSS_HASH_TPLZ;
> >  	struct ice_vsi *vsi = np->vsi;
> >  	struct ice_pf *pf = vsi->back;
> >  	struct device *dev;
> 
> [ ... ]
> 
> > diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c
> > b/drivers/net/ethernet/intel/ice/ice_lib.c
> > index d921269e1fe7..31f2757cf7fa 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> > @@ -1155,7 +1155,7 @@ static void ice_set_fd_vsi_ctx(struct
> ice_vsi_ctx *ctxt, struct ice_vsi *vsi)
> >   * @ctxt: the VSI context being set
> >   * @vsi: the VSI being configured
> >   */
> > -static void ice_set_rss_vsi_ctx(struct ice_vsi_ctx *ctxt, struct
> > ice_vsi *vsi)
> > +void ice_set_rss_vsi_ctx(struct ice_vsi_ctx *ctxt, struct ice_vsi
> > +*vsi)
> >  {
> >  	u8 lut_type, hash_type;
> >  	struct device *dev;
> > @@ -1181,7 +1181,9 @@ static void ice_set_rss_vsi_ctx(struct
> ice_vsi_ctx *ctxt, struct ice_vsi *vsi)
> >  		return;
> >  	}
> >
> > -	hash_type = ICE_AQ_VSI_Q_OPT_RSS_HASH_TPLZ;
> > +        if (!vsi->rss_hfunc)
> > +		vsi->rss_hfunc = ICE_AQ_VSI_Q_OPT_RSS_HASH_TPLZ;
> > +	hash_type = vsi->rss_hfunc;
> >  	vsi->rss_hfunc = hash_type;
> >
> >  	ctxt->info.q_opt_rss =
> 
> [ ... ]
> 
> > diff --git a/drivers/net/ethernet/intel/ice/ice_lib.h
> > b/drivers/net/ethernet/intel/ice/ice_lib.h
> > index 49454d98dcfe..29ba335dcc54 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_lib.h
> > +++ b/drivers/net/ethernet/intel/ice/ice_lib.h
> > @@ -46,6 +46,7 @@ void ice_vsi_delete(struct ice_vsi *vsi);  int
> > ice_vsi_cfg_tc(struct ice_vsi *vsi, u8 ena_tc);
> >
> >  int ice_vsi_cfg_rss_lut_key(struct ice_vsi *vsi);
> > +void ice_set_rss_vsi_ctx(struct ice_vsi_ctx *ctxt, struct ice_vsi
> > +*vsi);
> >
> >  void ice_vsi_cfg_netdev_tc(struct ice_vsi *vsi, u8 ena_tc);
> >

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

end of thread, other threads:[~2026-02-13 16:03 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-13  3:44 [PATCH iwl-next v1] ice: implement symmetric RSS hash configuration Aleksandr Loktionov
2026-02-13 15:01 ` [iwl-next,v1] " Simon Horman
2026-02-13 16:03   ` Loktionov, Aleksandr

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox