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); >
