Re: [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.bura...@intel.com>
Sent: 09 March 2026 15:49
To: Mandal, Anurag <anurag.man...@intel.com>; dev@dpdk.org
Cc: Richardson, Bruce <bruce.richard...@intel.com>;
alex.chap...@arm.com; sta...@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: sta...@dpdk.org

Signed-off-by: Anurag Mandal <anurag.man...@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