[PATCH] net/ice: fix rss simple_xor hash function

[Medvedkin, Vladimir]

Hi,

In the general case, hash function type is not meaningful for non-empty 
patterns, because hash function types are per VSI (meaning, they can 
only be globally set up, not per pattern). So, ideally, we do not want 
the user to specify a hash function when they also specify a non-empty 
pattern.

However, the ICE driver is not very well behaved in this matter, and 
does not enforce neither the "hash functions are only supposed to be 
globally defined" limitations, nor does it check if a user specified a 
different hash function from what they configured globally. Therefore, I 
think the proper fix for this issue should do the following:

-  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


On 3/9/2026 11:48 AM, Burakov, Anatoly wrote:
> On 3/9/2026 12:17 PM, Mandal, Anurag wrote:
>>
>>> -----Original Message-----
>>> From: Burakov, Anatoly <anatoly.burakov at intel.com>
>>> Sent: 09 March 2026 15:49
>>> To: Mandal, Anurag <anurag.mandal at intel.com>; dev at dpdk.org
>>> Cc: Richardson, Bruce <bruce.richardson at intel.com>;
>>> alex.chapman at arm.com; stable at dpdk.org
>>> Subject: Re: [PATCH] net/ice: fix rss simple_xor hash function
>>>
>>> On 3/9/2026 3:32 AM, Anurag Mandal wrote:
>>>> RSS Simple XOR hash function is supported by the NIC as per datasheet
>>>> & also in ICE Linux Ethernet kernel driver but the same is not enabled
>>>> in ICE PMD even though code support is already present.
>>>>
>>>> This patch fixes the issue by removing the simple_xor check & adding
>>>> proper error log for empty argument.
>>>>
>>>> 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>
>>>> ---
>>>
>>> Hi,
>>>
>>>>    drivers/net/intel/ice/ice_hash.c | 9 ++++-----
>>>>    1 file changed, 4 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/net/intel/ice/ice_hash.c
>>>> b/drivers/net/intel/ice/ice_hash.c
>>>> index 77829e607b..d57b5ee0e4 100644
>>>> --- a/drivers/net/intel/ice/ice_hash.c
>>>> +++ b/drivers/net/intel/ice/ice_hash.c
>>>> @@ -1109,12 +1109,11 @@ ice_hash_parse_action(struct
>>> ice_pattern_match_item *pattern_match_item,
>>>>                rss_type = rss->types;
>>>>
>>>>                /* 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) {
>>>> -                return rte_flow_error_set(error, ENOTSUP,
>>>> +            if (pattern_match_item->pattern_list ==
>>>> +                pattern_empty) {
>>>> +                return rte_flow_error_set(error, EINVAL,
>>>>                        RTE_FLOW_ERROR_TYPE_ACTION,
>>> action,
>>>> -                    "Not supported flow");
>>>> +                    "Invalid empty argument");
>>>
>>> I think this changes existing semantics a little too far.
>>>
>>> The original code disallowed simple XOR only for cases where pattern 
>>> wasn't
>>> empty (if it was, we allowed it - i.e. if it was a global RSS 
>>> configuration). The fix
>>> disallows empty patterns altogether, whereas what it should've done 
>>> instead
>>> is remove this check, and instead modify the second check to look 
>>> for empty
>>> patterns (because otherwise we would go through further checks down the
>>> line for non-empty patterns).
>>>
>>>>                } else if (rss->func ==
>>>>                       RTE_ETH_HASH_FUNCTION_SIMPLE_XOR){
>>>>                    rss_meta->hash_function =
>>>
>>>
>>> -- 
>>> Thanks,
>>> Anatoly
>>
>> Hi Anatoly,
>>
>> Thank you for your comments.
>> I understand the current change disallows all empty patterns 
>> altogether and that should be rectified.
>> My doubt  is should be allow both empty and non empty patterns to 
>> apply for simple_xor like symmetric Toeplitz ?
>
> That's what your patch fixes, is it not? It already allowed empty 
> patterns with simple xor (the second branch of the if condition).
>
>>
>> Thanks,
>> Anurag M
>>
>
>
-- 
Regards,
Vladimir