Re: [PATCH iwl-net] ice: keep the RSS hash function on indirection-only updates

[Maciej Fijalkowski <maciej.fijalkowski@intel.com>]

On Thu, Oct 08, 2026 at 08:34:12AM -0700, Jakub Kicinski wrote:
> On Wed,  7 Oct 2026 22:03:11 +0200 Maciej Fijalkowski wrote:
> > The ethtool core passes RXH_XFRM_NO_CHANGE (0xff) in input_xfrm when a
> > request does not touch the input transformation, e.g. when it only
> > rewrites the RSS indirection table. ice_set_rxfh() tests input_xfrm only
> > for RXH_XFRM_SYM_XOR, which 0xff has set, so such an update switches the
> > VSI to symmetric Toeplitz hashing behind the user's back.
> > 
> > Once symmetric-xor is reported, the core requires all hashed flow types
> > to be symmetric for any following RSS set. The GTP flow types hash on
> > the TEID, so every later netlink RSS update fails with "hash field config
> > is not symmetric", including the one that would put the indirection
> > table back.
> > 
> > Keep the current hash function when input_xfrm is RXH_XFRM_NO_CHANGE.
> > ice_set_rss_hfunc() then returns early and leaves the VSI as it is.
> > 
> > This has been encountered while trying to run xskxceiver within drv-net
> > tests where one of the setup steps was queue pull out of RSS indirection
> > table.
> > 
> > Fixes: 352e9bf23813 ("ice: enable symmetric-xor RSS for Toeplitz hash function")
> > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> 
> Hm, not seeing it on the list now but I thought I submitted this 
> a while back:

Hi! This comes from your response where you asked Aleksandr to pull out
this fix out of -next changes and send to -net separately:

https://lore.kernel.org/netdev/20260626152636.1e48330f@kernel.org/

> 
> --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> @@ -3681,10 +3681,10 @@ 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;
> +       u8 hfunc;
>         int err;
>  
>         dev = ice_pf_to_dev(pf);
> @@ -3703,9 +3703,12 @@ 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)
> +       if (rxfh->input_xfrm == RXH_XFRM_NO_CHANGE)
> +               hfunc = vsi->rss_hfunc;
> +       else if (rxfh->input_xfrm & RXH_XFRM_SYM_XOR)
>                 hfunc = ICE_AQ_VSI_Q_OPT_RSS_HASH_SYM_TPLZ;
> +       else /* input_xfrm == 0; core rejects any other value */
> +               hfunc = ICE_AQ_VSI_Q_OPT_RSS_HASH_TPLZ;
>  
>         err = ice_set_rss_hfunc(vsi, hfunc);
>         if (err)
> 
> Could you double check if we need the trailing else as well? 
> Not 100% sure now if I added it just in case or it mattered :S

else part seems to be just a move from hfunc being initialized to
ICE_AQ_VSI_Q_OPT_RSS_HASH_TPLZ.