[PATCH v2] net/ice: fix RSS hash function implementation

[Medvedkin, Vladimir]

Hi Anurag,

On 3/17/2026 10:32 AM, Anurag Mandal wrote:
> Hash function type is not meaningful for non-empty patterns as they
> are per VSI i.e. they are set globally only &  not per pattern.
> So, for non-empty patterns, user should not specify hash function.
> ICE PMD does not adhere to these rules.
>
> This patch fixes that by implementing the following:
> - interpret "default" hash function as "currently set up"
> - store the "current" hash function globally per VSI
> - for empty patterns, overwrite the setting
> - for non-empty patterns, disallow hash function that is not "default"
> or does not match currently selected global hash function type
>
> Tested the following:
> 1. non-empty pattern with default hash function
> 2. empty pattern with simple_xor/symmetric_toeplitz hash function
> 3. non-empty pattern with simple_xor/symmetric_toeplitz hash function
> 4. empty pattern with no/default hash function
>
> Bugzilla ID: 1518
> Fixes: 0b952714e9c1 ("net/ice: refactor PF hash flow")
> Cc: stable at dpdk.org
>
> Signed-off-by: Anurag Mandal <anurag.mandal at intel.com>
> ---
> V2: Addressed Vladimir Medvedkin's feedback
>   - interpret "default" hash function as "whatever is currently set up"
>   - store the "current" hash function globally somewhere (in VSI context)
>   - for empty patterns, overwrite the setting
>   - for non-empty patterns, disallow hash function that is not "default"
>     or does not match currently selected global hash function type
>
>   drivers/net/intel/ice/ice_ethdev.c |  3 +++
>   drivers/net/intel/ice/ice_ethdev.h |  1 +
>   drivers/net/intel/ice/ice_hash.c   | 20 ++++++++++++++++----
>   3 files changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/intel/ice/ice_ethdev.c b/drivers/net/intel/ice/ice_ethdev.c
> index 0d6b030536..0948a8cb23 100644
> --- a/drivers/net/intel/ice/ice_ethdev.c
> +++ b/drivers/net/intel/ice/ice_ethdev.c
> @@ -3741,6 +3741,7 @@ static int ice_init_rss(struct ice_pf *pf)
>   	vsi->rss_key_size = ICE_AQC_GET_SET_RSS_KEY_DATA_RSS_KEY_SIZE +
>   			    ICE_AQC_GET_SET_RSS_KEY_DATA_HASH_KEY_SIZE;
>   	vsi->rss_lut_size = pf->hash_lut_size;
> +	vsi->hash_function = rss_conf->algorithm;

ok, you set the hash function in this new field, but you don't change 
the HW state, hash function for this VSI is still symmetric Toeplitz

>   
>   	if (nb_q == 0) {
>   		PMD_DRV_LOG(WARNING,
> @@ -5700,6 +5701,8 @@ ice_rss_hash_update(struct rte_eth_dev *dev,
>   	if (rss_conf->rss_hf == 0)
>   		pf->rss_hf = 0;
>   
> +	vsi->hash_function = rss_conf->algorithm;
> +

Same here


>   	/* RSS hash configuration */
>   	ice_rss_hash_set(pf, rss_conf->rss_hf);
>   
> diff --git a/drivers/net/intel/ice/ice_ethdev.h b/drivers/net/intel/ice/ice_ethdev.h
> index 4b3718f715..d8a01fe7fa 100644
> --- a/drivers/net/intel/ice/ice_ethdev.h
> +++ b/drivers/net/intel/ice/ice_ethdev.h
> @@ -351,6 +351,7 @@ struct ice_vsi {
>   	bool offset_loaded;
>   	/* holds previous values so limitations can be enlarged to 64 bits */
>   	struct ice_vsi_get_stats_fields old_get_stats_fields;
> +	enum rte_eth_hash_function hash_function;
>   };
>   
>   enum proto_xtr_type {
> diff --git a/drivers/net/intel/ice/ice_hash.c b/drivers/net/intel/ice/ice_hash.c
> index 77829e607b..8e97a6fd42 100644
> --- a/drivers/net/intel/ice/ice_hash.c
> +++ b/drivers/net/intel/ice/ice_hash.c
> @@ -1084,6 +1084,10 @@ ice_any_invalid_rss_type(enum rte_eth_hash_function rss_func,
>   	/* check not allowed RSS type */
>   	rss_type &= ~VALID_RSS_ATTR;
>   
> +	/* For Empty patterns */
> +	if (!allow_rss_type)
> +		return false;
> +
I'm not sure I understood this part, could you please provide a more 
verbose comment here?
>   	return ((rss_type & allow_rss_type) != rss_type);
>   }
>   
> @@ -1091,7 +1095,8 @@ static int
>   ice_hash_parse_action(struct ice_pattern_match_item *pattern_match_item,
>   		const struct rte_flow_action actions[],
>   		uint64_t pattern_hint, struct ice_rss_meta *rss_meta,
> -		struct rte_flow_error *error)
> +		struct rte_flow_error *error,
> +		enum rte_eth_hash_function *vsi_hash_function)
>   {
>   	struct ice_rss_hash_cfg *cfg = pattern_match_item->meta;
>   	enum rte_flow_action_type action_type;
> @@ -1110,8 +1115,8 @@ ice_hash_parse_action(struct ice_pattern_match_item *pattern_match_item,
>   
>   			/* Check hash function and save it to rss_meta. */
>   			if (pattern_match_item->pattern_list !=
> -			    pattern_empty && rss->func ==
> -			    RTE_ETH_HASH_FUNCTION_SIMPLE_XOR) {
> +			    pattern_empty && rss->func &&
> +			    rss->func != *vsi_hash_function) {
>   				return rte_flow_error_set(error, ENOTSUP,
>   					RTE_FLOW_ERROR_TYPE_ACTION, action,
>   					"Not supported flow");
> @@ -1119,6 +1124,7 @@ ice_hash_parse_action(struct ice_pattern_match_item *pattern_match_item,
>   				   RTE_ETH_HASH_FUNCTION_SIMPLE_XOR){
>   				rss_meta->hash_function =
>   				RTE_ETH_HASH_FUNCTION_SIMPLE_XOR;
> +				*vsi_hash_function = rss_meta->hash_function;
>   				return 0;
>   			} else if (rss->func ==
>   				   RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ) {
> @@ -1161,7 +1167,12 @@ ice_hash_parse_action(struct ice_pattern_match_item *pattern_match_item,
>   					RTE_FLOW_ERROR_TYPE_ACTION,
>   					action, "RSS type not supported");
>   
> +			/* For Empty patterns*/
> +			if (cfg->hash_flds == ICE_HASH_INVALID)
> +				cfg->hash_flds = 1;
what does this "1" mean here? Hashing on Ethernet DST MAC address? Why?
>   			rss_meta->cfg = *cfg;
> +			if (rss->func)
> +				*vsi_hash_function = rss_meta->hash_function;
>   			ice_refine_hash_cfg(&rss_meta->cfg,
>   					    rss_type, pattern_hint);
>   			break;
> @@ -1193,6 +1204,7 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad,
>   	struct ice_pattern_match_item *pattern_match_item;
>   	struct ice_rss_meta *rss_meta_ptr;
>   	uint64_t phint = ICE_PHINT_NONE;
> +	struct ice_vsi *vsi = ad->pf.main_vsi;
>   
>   	if (priority >= 1)
>   		return -rte_errno;
> @@ -1230,7 +1242,7 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad,
>   
>   	/* Check rss action. */
>   	ret = ice_hash_parse_action(pattern_match_item, actions, phint,
> -				    rss_meta_ptr, error);
> +				    rss_meta_ptr, error, &vsi->hash_function);
Ok, your vsi->hash_function may be updated, how does the HW know about 
this?
>   
>   error:
>   	if (!ret && meta)

-- 
Regards,
Vladimir