> -----Original Message-----
> From: Simon Horman <[email protected]>
> Sent: Friday, February 13, 2026 4:02 PM
> To: Loktionov, Aleksandr <[email protected]>
> Cc: Simon Horman <[email protected]>; Nguyen, Anthony L
> <[email protected]>; [email protected];
> [email protected]
> 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);
> >
