[PATCH net-next v4 0/6] Support symmetric RSS (Toeplitz) hash

[PATCH net-next v4 0/6] Support symmetric RSS (Toeplitz) hash

[Ahmed Zaki]

Patch 1 adds the support at the Kernel level, allowing the user to set a
symmetric RSS hash for any flow type via:

    # ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n symmetric-xor

Support for the new "symmetric-xor" flag will be later sent to the
"ethtool" user-space tool.

Patch 2 fixes a long standing bug with the register values. The bug has
been benign for now since only (asymmetric) Toeplitz hash (Zero) has been
used.

Patches 3 and 4 lay some groundwork refactoring. While the first is
mainly cosmetic, the second is needed since there is no more room in the
previous 64-bit RSS profile ID for the symmetric attribute introduced in 
the next patch.

Finally, patches 5 and 6 add the symmetric Toeplitz support for the ice 
(E800 PFs) and the iAVF drivers.

---
v4: add a comment to "#define RXH_SYMMETRIC_XOR" (in uapi/linux/ethtool.h)
v3: rename "symmetric" to "symmetric-xor" and drop "Fixes" tag in patch 2.
v2: fixed a "Reviewed by" to "Reviewed-by", also need to cc maintainers.

Ahmed Zaki (4):
  net: ethtool: allow symmetric-xor RSS hash for any flow type
  ice: fix ICE_AQ_VSI_Q_OPT_RSS_* register values
  ice: refactor the FD and RSS flow ID generation
  iavf: enable symmetric RSS Toeplitz hash

Jeff Guo (1):
  ice: enable symmetric RSS Toeplitz hash for any flow type

Qi Zhang (1):
  ice: refactor RSS configuration

 Documentation/networking/scaling.rst          |   6 +
 .../net/ethernet/intel/iavf/iavf_adv_rss.c    |   8 +-
 .../net/ethernet/intel/iavf/iavf_adv_rss.h    |   3 +-
 .../net/ethernet/intel/iavf/iavf_ethtool.c    |  22 +-
 drivers/net/ethernet/intel/ice/ice.h          |   2 +
 .../net/ethernet/intel/ice/ice_adminq_cmd.h   |   8 +-
 drivers/net/ethernet/intel/ice/ice_common.h   |   1 +
 drivers/net/ethernet/intel/ice/ice_ethtool.c  |  14 +-
 .../net/ethernet/intel/ice/ice_ethtool_fdir.c |  35 +-
 .../net/ethernet/intel/ice/ice_flex_pipe.c    |  43 +-
 .../net/ethernet/intel/ice/ice_flex_pipe.h    |   4 +-
 .../net/ethernet/intel/ice/ice_flex_type.h    |   7 +
 drivers/net/ethernet/intel/ice/ice_flow.c     | 439 +++++++++++++-----
 drivers/net/ethernet/intel/ice/ice_flow.h     |  46 +-
 .../net/ethernet/intel/ice/ice_hw_autogen.h   |   4 +
 drivers/net/ethernet/intel/ice/ice_lib.c      | 117 ++---
 drivers/net/ethernet/intel/ice/ice_main.c     |  49 +-
 drivers/net/ethernet/intel/ice/ice_type.h     |   1 +
 drivers/net/ethernet/intel/ice/ice_virtchnl.c |  55 ++-
 .../ethernet/intel/ice/ice_virtchnl_fdir.c    |  35 +-
 include/linux/avf/virtchnl.h                  |  16 +-
 include/uapi/linux/ethtool.h                  |  21 +-
 net/ethtool/ioctl.c                           |  11 +
 23 files changed, 641 insertions(+), 306 deletions(-)

-- 
2.34.1

[PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Ahmed Zaki]

Symmetric RSS hash functions are beneficial in applications that monitor
both Tx and Rx packets of the same flow (IDS, software firewalls, ..etc).
Getting all traffic of the same flow on the same RX queue results in
higher CPU cache efficiency.

A NIC that supports "symmetric-xor" can achieve this RSS hash symmetry
by XORing the source and destination fields and pass the values to the
RSS hash algorithm.

Only fields that has counterparts in the other direction can be
accepted; IP src/dst and L4 src/dst ports.

The user may request RSS hash symmetry for a specific flow type, via:

    # ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n symmetric-xor

or turn symmetry off (asymmetric) by:

    # ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n

Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com>
---
 Documentation/networking/scaling.rst |  6 ++++++
 include/uapi/linux/ethtool.h         | 21 +++++++++++++--------
 net/ethtool/ioctl.c                  | 11 +++++++++++
 3 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/Documentation/networking/scaling.rst b/Documentation/networking/scaling.rst
index 92c9fb46d6a2..64f3d7566407 100644
--- a/Documentation/networking/scaling.rst
+++ b/Documentation/networking/scaling.rst
@@ -44,6 +44,12 @@ by masking out the low order seven bits of the computed hash for the
 packet (usually a Toeplitz hash), taking this number as a key into the
 indirection table and reading the corresponding value.
 
+Some NICs support symmetric RSS hashing where, if the IP (source address,
+destination address) and TCP/UDP (source port, destination port) tuples
+are swapped, the computed hash is the same. This is beneficial in some
+applications that monitor TCP/IP flows (IDS, firewalls, ...etc) and need
+both directions of the flow to land on the same Rx queue (and CPU).
+
 Some advanced NICs allow steering packets to queues based on
 programmable filters. For example, webserver bound TCP port 80 packets
 can be directed to their own receive queue. Such “n-tuple” filters can
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index f7fba0dc87e5..4e8d38fb55ce 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -2018,14 +2018,19 @@ static inline int ethtool_validate_duplex(__u8 duplex)
 #define	FLOW_RSS	0x20000000
 
 /* L3-L4 network traffic flow hash options */
-#define	RXH_L2DA	(1 << 1)
-#define	RXH_VLAN	(1 << 2)
-#define	RXH_L3_PROTO	(1 << 3)
-#define	RXH_IP_SRC	(1 << 4)
-#define	RXH_IP_DST	(1 << 5)
-#define	RXH_L4_B_0_1	(1 << 6) /* src port in case of TCP/UDP/SCTP */
-#define	RXH_L4_B_2_3	(1 << 7) /* dst port in case of TCP/UDP/SCTP */
-#define	RXH_DISCARD	(1 << 31)
+#define	RXH_L2DA		(1 << 1)
+#define	RXH_VLAN		(1 << 2)
+#define	RXH_L3_PROTO		(1 << 3)
+#define	RXH_IP_SRC		(1 << 4)
+#define	RXH_IP_DST		(1 << 5)
+#define	RXH_L4_B_0_1		(1 << 6) /* src port in case of TCP/UDP/SCTP */
+#define	RXH_L4_B_2_3		(1 << 7) /* dst port in case of TCP/UDP/SCTP */
+/* XOR the corresponding source and destination fields of each specified
+ * protocol. Both copies of the XOR'ed fields are fed into the RSS and RXHASH
+ * calculation.
+ */
+#define	RXH_SYMMETRIC_XOR	(1 << 30)
+#define	RXH_DISCARD		(1 << 31)
 
 #define	RX_CLS_FLOW_DISC	0xffffffffffffffffULL
 #define RX_CLS_FLOW_WAKE	0xfffffffffffffffeULL
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 0b0ce4f81c01..b1bd0d4b48e8 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -980,6 +980,17 @@ static noinline_for_stack int ethtool_set_rxnfc(struct net_device *dev,
 	if (rc)
 		return rc;
 
+	/* If a symmetric hash is requested, then:
+	 * 1 - no other fields besides IP src/dst and/or L4 src/dst
+	 * 2 - If src is set, dst must also be set
+	 */
+	if ((info.data & RXH_SYMMETRIC_XOR) &&
+	    ((info.data & ~(RXH_SYMMETRIC_XOR | RXH_IP_SRC | RXH_IP_DST |
+	      RXH_L4_B_0_1 | RXH_L4_B_2_3)) ||
+	     (!!(info.data & RXH_IP_SRC) ^ !!(info.data & RXH_IP_DST)) ||
+	     (!!(info.data & RXH_L4_B_0_1) ^ !!(info.data & RXH_L4_B_2_3))))
+		return -EINVAL;
+
 	rc = dev->ethtool_ops->set_rxnfc(dev, &info);
 	if (rc)
 		return rc;
-- 
2.34.1

Re: [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Alexander H Duyck]

On Mon, 2023-10-16 at 09:49 -0600, Ahmed Zaki wrote:
> Symmetric RSS hash functions are beneficial in applications that monitor
> both Tx and Rx packets of the same flow (IDS, software firewalls, ..etc).
> Getting all traffic of the same flow on the same RX queue results in
> higher CPU cache efficiency.
> 
> A NIC that supports "symmetric-xor" can achieve this RSS hash symmetry
> by XORing the source and destination fields and pass the values to the
> RSS hash algorithm.
> 
> Only fields that has counterparts in the other direction can be
> accepted; IP src/dst and L4 src/dst ports.
> 
> The user may request RSS hash symmetry for a specific flow type, via:
> 
>     # ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n symmetric-xor
> 
> or turn symmetry off (asymmetric) by:
> 
>     # ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n
> 
> Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
> Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com>
> ---
>  Documentation/networking/scaling.rst |  6 ++++++
>  include/uapi/linux/ethtool.h         | 21 +++++++++++++--------
>  net/ethtool/ioctl.c                  | 11 +++++++++++
>  3 files changed, 30 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/networking/scaling.rst b/Documentation/networking/scaling.rst
> index 92c9fb46d6a2..64f3d7566407 100644
> --- a/Documentation/networking/scaling.rst
> +++ b/Documentation/networking/scaling.rst
> @@ -44,6 +44,12 @@ by masking out the low order seven bits of the computed hash for the
>  packet (usually a Toeplitz hash), taking this number as a key into the
>  indirection table and reading the corresponding value.
>  
> +Some NICs support symmetric RSS hashing where, if the IP (source address,
> +destination address) and TCP/UDP (source port, destination port) tuples
> +are swapped, the computed hash is the same. This is beneficial in some
> +applications that monitor TCP/IP flows (IDS, firewalls, ...etc) and need
> +both directions of the flow to land on the same Rx queue (and CPU).
> +
>  Some advanced NICs allow steering packets to queues based on
>  programmable filters. For example, webserver bound TCP port 80 packets
>  can be directed to their own receive queue. Such “n-tuple” filters can
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index f7fba0dc87e5..4e8d38fb55ce 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -2018,14 +2018,19 @@ static inline int ethtool_validate_duplex(__u8 duplex)
>  #define	FLOW_RSS	0x20000000
>  
>  /* L3-L4 network traffic flow hash options */
> -#define	RXH_L2DA	(1 << 1)
> -#define	RXH_VLAN	(1 << 2)
> -#define	RXH_L3_PROTO	(1 << 3)
> -#define	RXH_IP_SRC	(1 << 4)
> -#define	RXH_IP_DST	(1 << 5)
> -#define	RXH_L4_B_0_1	(1 << 6) /* src port in case of TCP/UDP/SCTP */
> -#define	RXH_L4_B_2_3	(1 << 7) /* dst port in case of TCP/UDP/SCTP */
> -#define	RXH_DISCARD	(1 << 31)
> +#define	RXH_L2DA		(1 << 1)
> +#define	RXH_VLAN		(1 << 2)
> +#define	RXH_L3_PROTO		(1 << 3)
> +#define	RXH_IP_SRC		(1 << 4)
> +#define	RXH_IP_DST		(1 << 5)
> +#define	RXH_L4_B_0_1		(1 << 6) /* src port in case of TCP/UDP/SCTP */
> +#define	RXH_L4_B_2_3		(1 << 7) /* dst port in case of TCP/UDP/SCTP */
> +/* XOR the corresponding source and destination fields of each specified
> + * protocol. Both copies of the XOR'ed fields are fed into the RSS and RXHASH
> + * calculation.
> + */
> +#define	RXH_SYMMETRIC_XOR	(1 << 30)
> +#define	RXH_DISCARD		(1 << 31)

I guess this has already been discussed but I am not a fan of long
names for defines. I would prefer to see this just be something like
RXH_SYMMETRIC or something like that. The XOR is just an implementation
detail. I have seen the same thing accomplished by just reordering the
fields by min/max approaches.

>  
>  #define	RX_CLS_FLOW_DISC	0xffffffffffffffffULL
>  #define RX_CLS_FLOW_WAKE	0xfffffffffffffffeULL
> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
> index 0b0ce4f81c01..b1bd0d4b48e8 100644
> --- a/net/ethtool/ioctl.c
> +++ b/net/ethtool/ioctl.c
> @@ -980,6 +980,17 @@ static noinline_for_stack int ethtool_set_rxnfc(struct net_device *dev,
>  	if (rc)
>  		return rc;
>  
> +	/* If a symmetric hash is requested, then:
> +	 * 1 - no other fields besides IP src/dst and/or L4 src/dst
> +	 * 2 - If src is set, dst must also be set
> +	 */
> +	if ((info.data & RXH_SYMMETRIC_XOR) &&
> +	    ((info.data & ~(RXH_SYMMETRIC_XOR | RXH_IP_SRC | RXH_IP_DST |
> +	      RXH_L4_B_0_1 | RXH_L4_B_2_3)) ||
> +	     (!!(info.data & RXH_IP_SRC) ^ !!(info.data & RXH_IP_DST)) ||
> +	     (!!(info.data & RXH_L4_B_0_1) ^ !!(info.data & RXH_L4_B_2_3))))
> +		return -EINVAL;
> +
>  	rc = dev->ethtool_ops->set_rxnfc(dev, &info);
>  	if (rc)
>  		return rc;

You are pushing implementation from your device into the interface
design here. You should probably push these requirements down into the
driver rather than making it a part of the generic implementation.

It would be nice to see input from other NIC vendors on this as I
suspect they probably have similar functionality available to them.

Re: [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Ahmed Zaki]

On 2023-10-16 14:17, Alexander H Duyck wrote:
> On Mon, 2023-10-16 at 09:49 -0600, Ahmed Zaki wrote:
>> Symmetric RSS hash functions are beneficial in applications that monitor
>> both Tx and Rx packets of the same flow (IDS, software firewalls, ..etc).
>> Getting all traffic of the same flow on the same RX queue results in
>> higher CPU cache efficiency.
>>
>> A NIC that supports "symmetric-xor" can achieve this RSS hash symmetry
>> by XORing the source and destination fields and pass the values to the
>> RSS hash algorithm.
>>
>> Only fields that has counterparts in the other direction can be
>> accepted; IP src/dst and L4 src/dst ports.
>>
>> The user may request RSS hash symmetry for a specific flow type, via:
>>
>>      # ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n symmetric-xor
>>
>> or turn symmetry off (asymmetric) by:
>>
>>      # ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n
>>
>> Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
>> Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com>
>> ---
>>   Documentation/networking/scaling.rst |  6 ++++++
>>   include/uapi/linux/ethtool.h         | 21 +++++++++++++--------
>>   net/ethtool/ioctl.c                  | 11 +++++++++++
>>   3 files changed, 30 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/networking/scaling.rst b/Documentation/networking/scaling.rst
>> index 92c9fb46d6a2..64f3d7566407 100644
>> --- a/Documentation/networking/scaling.rst
>> +++ b/Documentation/networking/scaling.rst
>> @@ -44,6 +44,12 @@ by masking out the low order seven bits of the computed hash for the
>>   packet (usually a Toeplitz hash), taking this number as a key into the
>>   indirection table and reading the corresponding value.
>>   
>> +Some NICs support symmetric RSS hashing where, if the IP (source address,
>> +destination address) and TCP/UDP (source port, destination port) tuples
>> +are swapped, the computed hash is the same. This is beneficial in some
>> +applications that monitor TCP/IP flows (IDS, firewalls, ...etc) and need
>> +both directions of the flow to land on the same Rx queue (and CPU).
>> +
>>   Some advanced NICs allow steering packets to queues based on
>>   programmable filters. For example, webserver bound TCP port 80 packets
>>   can be directed to their own receive queue. Such “n-tuple” filters can
>> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
>> index f7fba0dc87e5..4e8d38fb55ce 100644
>> --- a/include/uapi/linux/ethtool.h
>> +++ b/include/uapi/linux/ethtool.h
>> @@ -2018,14 +2018,19 @@ static inline int ethtool_validate_duplex(__u8 duplex)
>>   #define	FLOW_RSS	0x20000000
>>   
>>   /* L3-L4 network traffic flow hash options */
>> -#define	RXH_L2DA	(1 << 1)
>> -#define	RXH_VLAN	(1 << 2)
>> -#define	RXH_L3_PROTO	(1 << 3)
>> -#define	RXH_IP_SRC	(1 << 4)
>> -#define	RXH_IP_DST	(1 << 5)
>> -#define	RXH_L4_B_0_1	(1 << 6) /* src port in case of TCP/UDP/SCTP */
>> -#define	RXH_L4_B_2_3	(1 << 7) /* dst port in case of TCP/UDP/SCTP */
>> -#define	RXH_DISCARD	(1 << 31)
>> +#define	RXH_L2DA		(1 << 1)
>> +#define	RXH_VLAN		(1 << 2)
>> +#define	RXH_L3_PROTO		(1 << 3)
>> +#define	RXH_IP_SRC		(1 << 4)
>> +#define	RXH_IP_DST		(1 << 5)
>> +#define	RXH_L4_B_0_1		(1 << 6) /* src port in case of TCP/UDP/SCTP */
>> +#define	RXH_L4_B_2_3		(1 << 7) /* dst port in case of TCP/UDP/SCTP */
>> +/* XOR the corresponding source and destination fields of each specified
>> + * protocol. Both copies of the XOR'ed fields are fed into the RSS and RXHASH
>> + * calculation.
>> + */
>> +#define	RXH_SYMMETRIC_XOR	(1 << 30)
>> +#define	RXH_DISCARD		(1 << 31)
> 
> I guess this has already been discussed but I am not a fan of long
> names for defines. I would prefer to see this just be something like
> RXH_SYMMETRIC or something like that. The XOR is just an implementation
> detail. I have seen the same thing accomplished by just reordering the
> fields by min/max approaches.

Correct. We discussed this and the consensus was that the user needs to 
have complete control on which implementation/algorithm is used to 
provide this symmetry, because each will yield different hash and may be 
different performance.


> 
>>   
>>   #define	RX_CLS_FLOW_DISC	0xffffffffffffffffULL
>>   #define RX_CLS_FLOW_WAKE	0xfffffffffffffffeULL
>> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
>> index 0b0ce4f81c01..b1bd0d4b48e8 100644
>> --- a/net/ethtool/ioctl.c
>> +++ b/net/ethtool/ioctl.c
>> @@ -980,6 +980,17 @@ static noinline_for_stack int ethtool_set_rxnfc(struct net_device *dev,
>>   	if (rc)
>>   		return rc;
>>   
>> +	/* If a symmetric hash is requested, then:
>> +	 * 1 - no other fields besides IP src/dst and/or L4 src/dst
>> +	 * 2 - If src is set, dst must also be set
>> +	 */
>> +	if ((info.data & RXH_SYMMETRIC_XOR) &&
>> +	    ((info.data & ~(RXH_SYMMETRIC_XOR | RXH_IP_SRC | RXH_IP_DST |
>> +	      RXH_L4_B_0_1 | RXH_L4_B_2_3)) ||
>> +	     (!!(info.data & RXH_IP_SRC) ^ !!(info.data & RXH_IP_DST)) ||
>> +	     (!!(info.data & RXH_L4_B_0_1) ^ !!(info.data & RXH_L4_B_2_3))))
>> +		return -EINVAL;
>> +
>>   	rc = dev->ethtool_ops->set_rxnfc(dev, &info);
>>   	if (rc)
>>   		return rc;
> 
> You are pushing implementation from your device into the interface
> design here. You should probably push these requirements down into the
> driver rather than making it a part of the generic implementation.

This is the most basic check and should be applied in any symmetric RSS 
implementation. Nothing specific to the XOR method. It can also be 
extended to include other "RXH_SYMMETRIC_XXX" in the future.

Re: [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Alexander Duyck]

On Mon, Oct 16, 2023 at 2:09 PM Ahmed Zaki <ahmed.zaki@intel.com> wrote:
>
>
>
> On 2023-10-16 14:17, Alexander H Duyck wrote:
> > On Mon, 2023-10-16 at 09:49 -0600, Ahmed Zaki wrote:
> >> Symmetric RSS hash functions are beneficial in applications that monitor
> >> both Tx and Rx packets of the same flow (IDS, software firewalls, ..etc).
> >> Getting all traffic of the same flow on the same RX queue results in
> >> higher CPU cache efficiency.
> >>
> >> A NIC that supports "symmetric-xor" can achieve this RSS hash symmetry
> >> by XORing the source and destination fields and pass the values to the
> >> RSS hash algorithm.
> >>
> >> Only fields that has counterparts in the other direction can be
> >> accepted; IP src/dst and L4 src/dst ports.
> >>
> >> The user may request RSS hash symmetry for a specific flow type, via:
> >>
> >>      # ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n symmetric-xor
> >>
> >> or turn symmetry off (asymmetric) by:
> >>
> >>      # ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n
> >>
> >> Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
> >> Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com>
> >> ---
> >>   Documentation/networking/scaling.rst |  6 ++++++
> >>   include/uapi/linux/ethtool.h         | 21 +++++++++++++--------
> >>   net/ethtool/ioctl.c                  | 11 +++++++++++
> >>   3 files changed, 30 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/Documentation/networking/scaling.rst b/Documentation/networking/scaling.rst
> >> index 92c9fb46d6a2..64f3d7566407 100644
> >> --- a/Documentation/networking/scaling.rst
> >> +++ b/Documentation/networking/scaling.rst
> >> @@ -44,6 +44,12 @@ by masking out the low order seven bits of the computed hash for the
> >>   packet (usually a Toeplitz hash), taking this number as a key into the
> >>   indirection table and reading the corresponding value.
> >>
> >> +Some NICs support symmetric RSS hashing where, if the IP (source address,
> >> +destination address) and TCP/UDP (source port, destination port) tuples
> >> +are swapped, the computed hash is the same. This is beneficial in some
> >> +applications that monitor TCP/IP flows (IDS, firewalls, ...etc) and need
> >> +both directions of the flow to land on the same Rx queue (and CPU).
> >> +
> >>   Some advanced NICs allow steering packets to queues based on
> >>   programmable filters. For example, webserver bound TCP port 80 packets
> >>   can be directed to their own receive queue. Such “n-tuple” filters can
> >> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> >> index f7fba0dc87e5..4e8d38fb55ce 100644
> >> --- a/include/uapi/linux/ethtool.h
> >> +++ b/include/uapi/linux/ethtool.h
> >> @@ -2018,14 +2018,19 @@ static inline int ethtool_validate_duplex(__u8 duplex)
> >>   #define    FLOW_RSS        0x20000000
> >>
> >>   /* L3-L4 network traffic flow hash options */
> >> -#define     RXH_L2DA        (1 << 1)
> >> -#define     RXH_VLAN        (1 << 2)
> >> -#define     RXH_L3_PROTO    (1 << 3)
> >> -#define     RXH_IP_SRC      (1 << 4)
> >> -#define     RXH_IP_DST      (1 << 5)
> >> -#define     RXH_L4_B_0_1    (1 << 6) /* src port in case of TCP/UDP/SCTP */
> >> -#define     RXH_L4_B_2_3    (1 << 7) /* dst port in case of TCP/UDP/SCTP */
> >> -#define     RXH_DISCARD     (1 << 31)
> >> +#define     RXH_L2DA                (1 << 1)
> >> +#define     RXH_VLAN                (1 << 2)
> >> +#define     RXH_L3_PROTO            (1 << 3)
> >> +#define     RXH_IP_SRC              (1 << 4)
> >> +#define     RXH_IP_DST              (1 << 5)
> >> +#define     RXH_L4_B_0_1            (1 << 6) /* src port in case of TCP/UDP/SCTP */
> >> +#define     RXH_L4_B_2_3            (1 << 7) /* dst port in case of TCP/UDP/SCTP */
> >> +/* XOR the corresponding source and destination fields of each specified
> >> + * protocol. Both copies of the XOR'ed fields are fed into the RSS and RXHASH
> >> + * calculation.
> >> + */
> >> +#define     RXH_SYMMETRIC_XOR       (1 << 30)
> >> +#define     RXH_DISCARD             (1 << 31)
> >
> > I guess this has already been discussed but I am not a fan of long
> > names for defines. I would prefer to see this just be something like
> > RXH_SYMMETRIC or something like that. The XOR is just an implementation
> > detail. I have seen the same thing accomplished by just reordering the
> > fields by min/max approaches.
>
> Correct. We discussed this and the consensus was that the user needs to
> have complete control on which implementation/algorithm is used to
> provide this symmetry, because each will yield different hash and may be
> different performance.

I agree about the user having control over the algorithm, but this
interface isn't about selecting the algorithm. It is just about
setting up the inputs. Selecting the algorithm is handled via the
set/get_rxfh interface hfunc variable. If this is just a different
hash function it really belongs there rather than being made a part of
the input string.

> >
> >>
> >>   #define    RX_CLS_FLOW_DISC        0xffffffffffffffffULL
> >>   #define RX_CLS_FLOW_WAKE   0xfffffffffffffffeULL
> >> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
> >> index 0b0ce4f81c01..b1bd0d4b48e8 100644
> >> --- a/net/ethtool/ioctl.c
> >> +++ b/net/ethtool/ioctl.c
> >> @@ -980,6 +980,17 @@ static noinline_for_stack int ethtool_set_rxnfc(struct net_device *dev,
> >>      if (rc)
> >>              return rc;
> >>
> >> +    /* If a symmetric hash is requested, then:
> >> +     * 1 - no other fields besides IP src/dst and/or L4 src/dst
> >> +     * 2 - If src is set, dst must also be set
> >> +     */
> >> +    if ((info.data & RXH_SYMMETRIC_XOR) &&
> >> +        ((info.data & ~(RXH_SYMMETRIC_XOR | RXH_IP_SRC | RXH_IP_DST |
> >> +          RXH_L4_B_0_1 | RXH_L4_B_2_3)) ||
> >> +         (!!(info.data & RXH_IP_SRC) ^ !!(info.data & RXH_IP_DST)) ||
> >> +         (!!(info.data & RXH_L4_B_0_1) ^ !!(info.data & RXH_L4_B_2_3))))
> >> +            return -EINVAL;
> >> +
> >>      rc = dev->ethtool_ops->set_rxnfc(dev, &info);
> >>      if (rc)
> >>              return rc;
> >
> > You are pushing implementation from your device into the interface
> > design here. You should probably push these requirements down into the
> > driver rather than making it a part of the generic implementation.
>
> This is the most basic check and should be applied in any symmetric RSS
> implementation. Nothing specific to the XOR method. It can also be
> extended to include other "RXH_SYMMETRIC_XXX" in the future.

You are partially correct. Your item 2 is accurate, however you are
excluding other fields in your item 1. Fields such as L2DA wouldn't be
symmetric, but VLAN and L3_PROTO would be. That is the implementation
specific detail I was referring to.

Re: [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Ahmed Zaki]

On 2023-10-16 16:15, Alexander Duyck wrote:
> On Mon, Oct 16, 2023 at 2:09 PM Ahmed Zaki <ahmed.zaki@intel.com> wrote:
>>
>>
>>
>> On 2023-10-16 14:17, Alexander H Duyck wrote:
>>> On Mon, 2023-10-16 at 09:49 -0600, Ahmed Zaki wrote:
>>>> Symmetric RSS hash functions are beneficial in applications that monitor
>>>> both Tx and Rx packets of the same flow (IDS, software firewalls, ..etc).
>>>> Getting all traffic of the same flow on the same RX queue results in
>>>> higher CPU cache efficiency.
>>>>
>>>> A NIC that supports "symmetric-xor" can achieve this RSS hash symmetry
>>>> by XORing the source and destination fields and pass the values to the
>>>> RSS hash algorithm.
>>>>
>>>> Only fields that has counterparts in the other direction can be
>>>> accepted; IP src/dst and L4 src/dst ports.
>>>>
>>>> The user may request RSS hash symmetry for a specific flow type, via:
>>>>
>>>>       # ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n symmetric-xor
>>>>
>>>> or turn symmetry off (asymmetric) by:
>>>>
>>>>       # ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n
>>>>
>>>> Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
>>>> Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com>
>>>> ---
>>>>    Documentation/networking/scaling.rst |  6 ++++++
>>>>    include/uapi/linux/ethtool.h         | 21 +++++++++++++--------
>>>>    net/ethtool/ioctl.c                  | 11 +++++++++++
>>>>    3 files changed, 30 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/Documentation/networking/scaling.rst b/Documentation/networking/scaling.rst
>>>> index 92c9fb46d6a2..64f3d7566407 100644
>>>> --- a/Documentation/networking/scaling.rst
>>>> +++ b/Documentation/networking/scaling.rst
>>>> @@ -44,6 +44,12 @@ by masking out the low order seven bits of the computed hash for the
>>>>    packet (usually a Toeplitz hash), taking this number as a key into the
>>>>    indirection table and reading the corresponding value.
>>>>
>>>> +Some NICs support symmetric RSS hashing where, if the IP (source address,
>>>> +destination address) and TCP/UDP (source port, destination port) tuples
>>>> +are swapped, the computed hash is the same. This is beneficial in some
>>>> +applications that monitor TCP/IP flows (IDS, firewalls, ...etc) and need
>>>> +both directions of the flow to land on the same Rx queue (and CPU).
>>>> +
>>>>    Some advanced NICs allow steering packets to queues based on
>>>>    programmable filters. For example, webserver bound TCP port 80 packets
>>>>    can be directed to their own receive queue. Such “n-tuple” filters can
>>>> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
>>>> index f7fba0dc87e5..4e8d38fb55ce 100644
>>>> --- a/include/uapi/linux/ethtool.h
>>>> +++ b/include/uapi/linux/ethtool.h
>>>> @@ -2018,14 +2018,19 @@ static inline int ethtool_validate_duplex(__u8 duplex)
>>>>    #define    FLOW_RSS        0x20000000
>>>>
>>>>    /* L3-L4 network traffic flow hash options */
>>>> -#define     RXH_L2DA        (1 << 1)
>>>> -#define     RXH_VLAN        (1 << 2)
>>>> -#define     RXH_L3_PROTO    (1 << 3)
>>>> -#define     RXH_IP_SRC      (1 << 4)
>>>> -#define     RXH_IP_DST      (1 << 5)
>>>> -#define     RXH_L4_B_0_1    (1 << 6) /* src port in case of TCP/UDP/SCTP */
>>>> -#define     RXH_L4_B_2_3    (1 << 7) /* dst port in case of TCP/UDP/SCTP */
>>>> -#define     RXH_DISCARD     (1 << 31)
>>>> +#define     RXH_L2DA                (1 << 1)
>>>> +#define     RXH_VLAN                (1 << 2)
>>>> +#define     RXH_L3_PROTO            (1 << 3)
>>>> +#define     RXH_IP_SRC              (1 << 4)
>>>> +#define     RXH_IP_DST              (1 << 5)
>>>> +#define     RXH_L4_B_0_1            (1 << 6) /* src port in case of TCP/UDP/SCTP */
>>>> +#define     RXH_L4_B_2_3            (1 << 7) /* dst port in case of TCP/UDP/SCTP */
>>>> +/* XOR the corresponding source and destination fields of each specified
>>>> + * protocol. Both copies of the XOR'ed fields are fed into the RSS and RXHASH
>>>> + * calculation.
>>>> + */
>>>> +#define     RXH_SYMMETRIC_XOR       (1 << 30)
>>>> +#define     RXH_DISCARD             (1 << 31)
>>>
>>> I guess this has already been discussed but I am not a fan of long
>>> names for defines. I would prefer to see this just be something like
>>> RXH_SYMMETRIC or something like that. The XOR is just an implementation
>>> detail. I have seen the same thing accomplished by just reordering the
>>> fields by min/max approaches.
>>
>> Correct. We discussed this and the consensus was that the user needs to
>> have complete control on which implementation/algorithm is used to
>> provide this symmetry, because each will yield different hash and may be
>> different performance.
> 
> I agree about the user having control over the algorithm, but this
> interface isn't about selecting the algorithm. It is just about
> setting up the inputs. Selecting the algorithm is handled via the
> set/get_rxfh interface hfunc variable. If this is just a different
> hash function it really belongs there rather than being made a part of
> the input string.

My bad. It is the same RSS algorithm (Toeplitz in our case). Still the 
user needs to be able to manipulate the inputs. The point is, a generic 
define like "RXH_SYMETRIC" was rejected (that was actually v1).


> 
>>>
>>>>
>>>>    #define    RX_CLS_FLOW_DISC        0xffffffffffffffffULL
>>>>    #define RX_CLS_FLOW_WAKE   0xfffffffffffffffeULL
>>>> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
>>>> index 0b0ce4f81c01..b1bd0d4b48e8 100644
>>>> --- a/net/ethtool/ioctl.c
>>>> +++ b/net/ethtool/ioctl.c
>>>> @@ -980,6 +980,17 @@ static noinline_for_stack int ethtool_set_rxnfc(struct net_device *dev,
>>>>       if (rc)
>>>>               return rc;
>>>>
>>>> +    /* If a symmetric hash is requested, then:
>>>> +     * 1 - no other fields besides IP src/dst and/or L4 src/dst
>>>> +     * 2 - If src is set, dst must also be set
>>>> +     */
>>>> +    if ((info.data & RXH_SYMMETRIC_XOR) &&
>>>> +        ((info.data & ~(RXH_SYMMETRIC_XOR | RXH_IP_SRC | RXH_IP_DST |
>>>> +          RXH_L4_B_0_1 | RXH_L4_B_2_3)) ||
>>>> +         (!!(info.data & RXH_IP_SRC) ^ !!(info.data & RXH_IP_DST)) ||
>>>> +         (!!(info.data & RXH_L4_B_0_1) ^ !!(info.data & RXH_L4_B_2_3))))
>>>> +            return -EINVAL;
>>>> +
>>>>       rc = dev->ethtool_ops->set_rxnfc(dev, &info);
>>>>       if (rc)
>>>>               return rc;
>>>
>>> You are pushing implementation from your device into the interface
>>> design here. You should probably push these requirements down into the
>>> driver rather than making it a part of the generic implementation.
>>
>> This is the most basic check and should be applied in any symmetric RSS
>> implementation. Nothing specific to the XOR method. It can also be
>> extended to include other "RXH_SYMMETRIC_XXX" in the future.
> 
> You are partially correct. Your item 2 is accurate, however you are
> excluding other fields in your item 1. Fields such as L2DA wouldn't be
> symmetric, but VLAN and L3_PROTO would be. That is the implementation
> specific detail I was referring to.

hmm.. not sure how VLAN tag would be used in this case. But moving this 
into ice_ethtool is trivial. We can start there and unify when/if other 
vendors push similar functionalities.

How does that sound?

Re: [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Alexander Duyck]

On Mon, Oct 16, 2023 at 3:44 PM Ahmed Zaki <ahmed.zaki@intel.com> wrote:
>
>
>
> On 2023-10-16 16:15, Alexander Duyck wrote:
> > On Mon, Oct 16, 2023 at 2:09 PM Ahmed Zaki <ahmed.zaki@intel.com> wrote:
> >>
> >>
> >>
> >> On 2023-10-16 14:17, Alexander H Duyck wrote:
> >>> On Mon, 2023-10-16 at 09:49 -0600, Ahmed Zaki wrote:
> >>>> Symmetric RSS hash functions are beneficial in applications that monitor
> >>>> both Tx and Rx packets of the same flow (IDS, software firewalls, ..etc).
> >>>> Getting all traffic of the same flow on the same RX queue results in
> >>>> higher CPU cache efficiency.
> >>>>
> >>>> A NIC that supports "symmetric-xor" can achieve this RSS hash symmetry
> >>>> by XORing the source and destination fields and pass the values to the
> >>>> RSS hash algorithm.
> >>>>
> >>>> Only fields that has counterparts in the other direction can be
> >>>> accepted; IP src/dst and L4 src/dst ports.
> >>>>
> >>>> The user may request RSS hash symmetry for a specific flow type, via:
> >>>>
> >>>>       # ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n symmetric-xor
> >>>>
> >>>> or turn symmetry off (asymmetric) by:
> >>>>
> >>>>       # ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n
> >>>>
> >>>> Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
> >>>> Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com>
> >>>> ---
> >>>>    Documentation/networking/scaling.rst |  6 ++++++
> >>>>    include/uapi/linux/ethtool.h         | 21 +++++++++++++--------
> >>>>    net/ethtool/ioctl.c                  | 11 +++++++++++
> >>>>    3 files changed, 30 insertions(+), 8 deletions(-)
> >>>>
> >>>> diff --git a/Documentation/networking/scaling.rst b/Documentation/networking/scaling.rst
> >>>> index 92c9fb46d6a2..64f3d7566407 100644
> >>>> --- a/Documentation/networking/scaling.rst
> >>>> +++ b/Documentation/networking/scaling.rst
> >>>> @@ -44,6 +44,12 @@ by masking out the low order seven bits of the computed hash for the
> >>>>    packet (usually a Toeplitz hash), taking this number as a key into the
> >>>>    indirection table and reading the corresponding value.
> >>>>
> >>>> +Some NICs support symmetric RSS hashing where, if the IP (source address,
> >>>> +destination address) and TCP/UDP (source port, destination port) tuples
> >>>> +are swapped, the computed hash is the same. This is beneficial in some
> >>>> +applications that monitor TCP/IP flows (IDS, firewalls, ...etc) and need
> >>>> +both directions of the flow to land on the same Rx queue (and CPU).
> >>>> +
> >>>>    Some advanced NICs allow steering packets to queues based on
> >>>>    programmable filters. For example, webserver bound TCP port 80 packets
> >>>>    can be directed to their own receive queue. Such “n-tuple” filters can
> >>>> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> >>>> index f7fba0dc87e5..4e8d38fb55ce 100644
> >>>> --- a/include/uapi/linux/ethtool.h
> >>>> +++ b/include/uapi/linux/ethtool.h
> >>>> @@ -2018,14 +2018,19 @@ static inline int ethtool_validate_duplex(__u8 duplex)
> >>>>    #define    FLOW_RSS        0x20000000
> >>>>
> >>>>    /* L3-L4 network traffic flow hash options */
> >>>> -#define     RXH_L2DA        (1 << 1)
> >>>> -#define     RXH_VLAN        (1 << 2)
> >>>> -#define     RXH_L3_PROTO    (1 << 3)
> >>>> -#define     RXH_IP_SRC      (1 << 4)
> >>>> -#define     RXH_IP_DST      (1 << 5)
> >>>> -#define     RXH_L4_B_0_1    (1 << 6) /* src port in case of TCP/UDP/SCTP */
> >>>> -#define     RXH_L4_B_2_3    (1 << 7) /* dst port in case of TCP/UDP/SCTP */
> >>>> -#define     RXH_DISCARD     (1 << 31)
> >>>> +#define     RXH_L2DA                (1 << 1)
> >>>> +#define     RXH_VLAN                (1 << 2)
> >>>> +#define     RXH_L3_PROTO            (1 << 3)
> >>>> +#define     RXH_IP_SRC              (1 << 4)
> >>>> +#define     RXH_IP_DST              (1 << 5)
> >>>> +#define     RXH_L4_B_0_1            (1 << 6) /* src port in case of TCP/UDP/SCTP */
> >>>> +#define     RXH_L4_B_2_3            (1 << 7) /* dst port in case of TCP/UDP/SCTP */
> >>>> +/* XOR the corresponding source and destination fields of each specified
> >>>> + * protocol. Both copies of the XOR'ed fields are fed into the RSS and RXHASH
> >>>> + * calculation.
> >>>> + */
> >>>> +#define     RXH_SYMMETRIC_XOR       (1 << 30)
> >>>> +#define     RXH_DISCARD             (1 << 31)
> >>>
> >>> I guess this has already been discussed but I am not a fan of long
> >>> names for defines. I would prefer to see this just be something like
> >>> RXH_SYMMETRIC or something like that. The XOR is just an implementation
> >>> detail. I have seen the same thing accomplished by just reordering the
> >>> fields by min/max approaches.
> >>
> >> Correct. We discussed this and the consensus was that the user needs to
> >> have complete control on which implementation/algorithm is used to
> >> provide this symmetry, because each will yield different hash and may be
> >> different performance.
> >
> > I agree about the user having control over the algorithm, but this
> > interface isn't about selecting the algorithm. It is just about
> > setting up the inputs. Selecting the algorithm is handled via the
> > set/get_rxfh interface hfunc variable. If this is just a different
> > hash function it really belongs there rather than being made a part of
> > the input string.
>
> My bad. It is the same RSS algorithm (Toeplitz in our case). Still the
> user needs to be able to manipulate the inputs. The point is, a generic
> define like "RXH_SYMETRIC" was rejected (that was actually v1).

No it is not. That is the point and you made it quite clear. The hash
you are using is a variant of Toeplitz, but it is also hybridized with
XOR. Why would it make sense to add it as an input mask bit when what
you are doing is using a different algorithm.

It would make more sense to just add it as a variant hash function of
toeplitz. If you did it right you could probably make the formatting
pretty, something like:
RSS hash function:
    toeplitz: on
        symmetric xor: on
    xor: off
    crc32: off

It doesn't make sense to place it in the input flags and will just
cause quick congestion as things get added there. This is an algorithm
change so it makes more sense to place it there.

> >
> >>>
> >>>>
> >>>>    #define    RX_CLS_FLOW_DISC        0xffffffffffffffffULL
> >>>>    #define RX_CLS_FLOW_WAKE   0xfffffffffffffffeULL
> >>>> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
> >>>> index 0b0ce4f81c01..b1bd0d4b48e8 100644
> >>>> --- a/net/ethtool/ioctl.c
> >>>> +++ b/net/ethtool/ioctl.c
> >>>> @@ -980,6 +980,17 @@ static noinline_for_stack int ethtool_set_rxnfc(struct net_device *dev,
> >>>>       if (rc)
> >>>>               return rc;
> >>>>
> >>>> +    /* If a symmetric hash is requested, then:
> >>>> +     * 1 - no other fields besides IP src/dst and/or L4 src/dst
> >>>> +     * 2 - If src is set, dst must also be set
> >>>> +     */
> >>>> +    if ((info.data & RXH_SYMMETRIC_XOR) &&
> >>>> +        ((info.data & ~(RXH_SYMMETRIC_XOR | RXH_IP_SRC | RXH_IP_DST |
> >>>> +          RXH_L4_B_0_1 | RXH_L4_B_2_3)) ||
> >>>> +         (!!(info.data & RXH_IP_SRC) ^ !!(info.data & RXH_IP_DST)) ||
> >>>> +         (!!(info.data & RXH_L4_B_0_1) ^ !!(info.data & RXH_L4_B_2_3))))
> >>>> +            return -EINVAL;
> >>>> +
> >>>>       rc = dev->ethtool_ops->set_rxnfc(dev, &info);
> >>>>       if (rc)
> >>>>               return rc;
> >>>
> >>> You are pushing implementation from your device into the interface
> >>> design here. You should probably push these requirements down into the
> >>> driver rather than making it a part of the generic implementation.
> >>
> >> This is the most basic check and should be applied in any symmetric RSS
> >> implementation. Nothing specific to the XOR method. It can also be
> >> extended to include other "RXH_SYMMETRIC_XXX" in the future.
> >
> > You are partially correct. Your item 2 is accurate, however you are
> > excluding other fields in your item 1. Fields such as L2DA wouldn't be
> > symmetric, but VLAN and L3_PROTO would be. That is the implementation
> > specific detail I was referring to.
>
> hmm.. not sure how VLAN tag would be used in this case. But moving this
> into ice_ethtool is trivial. We can start there and unify when/if other
> vendors push similar functionalities.
>
> How does that sound?

I still think this might be something best handled in your set_rxnfc
function rather than generically here. I would be okay with adding a
helper though since this seems like something that could probably be
handled via an inline. As it stands with the suggestion to move this
out as a separate hash type it would make more sense to not have this
as a part of the ethtool_set_rxnfc itself.

Re: [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Jakub Kicinski]

On Mon, 16 Oct 2023 15:55:21 -0700 Alexander Duyck wrote:
> It would make more sense to just add it as a variant hash function of
> toeplitz. If you did it right you could probably make the formatting
> pretty, something like:
> RSS hash function:
>     toeplitz: on
>         symmetric xor: on
>     xor: off
>     crc32: off
> 
> It doesn't make sense to place it in the input flags and will just
> cause quick congestion as things get added there. This is an algorithm
> change so it makes more sense to place it there.

Algo is also a bit confusing, it's more like key pre-processing?
There's nothing toeplitz about xoring input fields. Works as well
for CRC32.. or XOR.

We can use one of the reserved fields of struct ethtool_rxfh to carry
this extension. I think I asked for this at some point, but there's
only so much repeated feedback one can send in a day :(

Re: [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Ahmed Zaki]

On 2023-10-16 17:30, Jakub Kicinski wrote:
> On Mon, 16 Oct 2023 15:55:21 -0700 Alexander Duyck wrote:
>> It would make more sense to just add it as a variant hash function of
>> toeplitz. If you did it right you could probably make the formatting
>> pretty, something like:
>> RSS hash function:
>>      toeplitz: on
>>          symmetric xor: on
>>      xor: off
>>      crc32: off
>>
>> It doesn't make sense to place it in the input flags and will just
>> cause quick congestion as things get added there. This is an algorithm
>> change so it makes more sense to place it there.
> 
> Algo is also a bit confusing, it's more like key pre-processing?
> There's nothing toeplitz about xoring input fields. Works as well
> for CRC32.. or XOR.
> 
> We can use one of the reserved fields of struct ethtool_rxfh to carry
> this extension. I think I asked for this at some point, but there's
> only so much repeated feedback one can send in a day :(

Sorry you felt that. I took you comment [1]:

"Using hashing algo for configuring fields feels like a dirty hack".

To mean that the we should not use the hfunc API ("ethtool_rxfh"). This 
is why in the new series I chose to configure the RSS fields. This also 
provides the user with more control and better granularity on which 
flow-types to be symmetric, and which protocols (L3 and/or L4) to use. I 
have no idea how to do any of these via hfunc/ethtool_rxfh API so it 
seemed a better approach.

I see you marked the series as "Changes Requested". I will send a new 
version tomorrow and move the sanity checks inside ice_ethtool.


[1]: https://lore.kernel.org/netdev/20230824174336.6fb801d5@kernel.org/

Re: [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Alexander Duyck]

On Mon, Oct 16, 2023 at 5:08 PM Ahmed Zaki <ahmed.zaki@intel.com> wrote:
>
>
>
> On 2023-10-16 17:30, Jakub Kicinski wrote:
> > On Mon, 16 Oct 2023 15:55:21 -0700 Alexander Duyck wrote:
> >> It would make more sense to just add it as a variant hash function of
> >> toeplitz. If you did it right you could probably make the formatting
> >> pretty, something like:
> >> RSS hash function:
> >>      toeplitz: on
> >>          symmetric xor: on
> >>      xor: off
> >>      crc32: off
> >>
> >> It doesn't make sense to place it in the input flags and will just
> >> cause quick congestion as things get added there. This is an algorithm
> >> change so it makes more sense to place it there.
> >
> > Algo is also a bit confusing, it's more like key pre-processing?
> > There's nothing toeplitz about xoring input fields. Works as well
> > for CRC32.. or XOR.
> >
> > We can use one of the reserved fields of struct ethtool_rxfh to carry
> > this extension. I think I asked for this at some point, but there's
> > only so much repeated feedback one can send in a day :(
>
> Sorry you felt that. I took you comment [1]:
>
> "Using hashing algo for configuring fields feels like a dirty hack".
>
> To mean that the we should not use the hfunc API ("ethtool_rxfh"). This
> is why in the new series I chose to configure the RSS fields. This also
> provides the user with more control and better granularity on which
> flow-types to be symmetric, and which protocols (L3 and/or L4) to use. I
> have no idea how to do any of these via hfunc/ethtool_rxfh API so it
> seemed a better approach.
>
> I see you marked the series as "Changes Requested". I will send a new
> version tomorrow and move the sanity checks inside ice_ethtool.
>
>
> [1]: https://lore.kernel.org/netdev/20230824174336.6fb801d5@kernel.org/

So one question I would have is what happens if you were to ignore the
extra configuration that prevents people from disabling either source
or destination from the input? Does it actually have to be hard
restricted or do you end up with the hardware generating non-symmetric
hashes because it isn't doing the XOR with both source and destination
fields?

My thought would be to possibly just look at reducing your messaging
to a warning from the driver if the inputs are not symmetric, but you
have your symmetric xor hash function enabled.

Re: [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Ahmed Zaki]

On 2023-10-17 12:42, Alexander Duyck wrote:
> On Mon, Oct 16, 2023 at 5:08 PM Ahmed Zaki <ahmed.zaki@intel.com> wrote:
>>
>>
>>
>> On 2023-10-16 17:30, Jakub Kicinski wrote:
>>> On Mon, 16 Oct 2023 15:55:21 -0700 Alexander Duyck wrote:
>>>> It would make more sense to just add it as a variant hash function of
>>>> toeplitz. If you did it right you could probably make the formatting
>>>> pretty, something like:
>>>> RSS hash function:
>>>>       toeplitz: on
>>>>           symmetric xor: on
>>>>       xor: off
>>>>       crc32: off
>>>>
>>>> It doesn't make sense to place it in the input flags and will just
>>>> cause quick congestion as things get added there. This is an algorithm
>>>> change so it makes more sense to place it there.
>>>
>>> Algo is also a bit confusing, it's more like key pre-processing?
>>> There's nothing toeplitz about xoring input fields. Works as well
>>> for CRC32.. or XOR.
>>>
>>> We can use one of the reserved fields of struct ethtool_rxfh to carry
>>> this extension. I think I asked for this at some point, but there's
>>> only so much repeated feedback one can send in a day :(
>>
>> Sorry you felt that. I took you comment [1]:
>>
>> "Using hashing algo for configuring fields feels like a dirty hack".
>>
>> To mean that the we should not use the hfunc API ("ethtool_rxfh"). This
>> is why in the new series I chose to configure the RSS fields. This also
>> provides the user with more control and better granularity on which
>> flow-types to be symmetric, and which protocols (L3 and/or L4) to use. I
>> have no idea how to do any of these via hfunc/ethtool_rxfh API so it
>> seemed a better approach.
>>
>> I see you marked the series as "Changes Requested". I will send a new
>> version tomorrow and move the sanity checks inside ice_ethtool.
>>
>>
>> [1]: https://lore.kernel.org/netdev/20230824174336.6fb801d5@kernel.org/
> 
> So one question I would have is what happens if you were to ignore the
> extra configuration that prevents people from disabling either source
> or destination from the input? Does it actually have to be hard
> restricted or do you end up with the hardware generating non-symmetric
> hashes because it isn't doing the XOR with both source and destination
> fields?

Do you mean allow the user to use any RSS fields as input? What do we 
gain by that?

The hardware's TOEPLITZ and SYM_TOEPLITZ functions are the same except 
for the XORing step. What gets XOR'd needs to be programmed (Patch 5: 
ice_rss_config_xor()) and we are programming the hardware to XOR the src 
and dst fields to get this hash symmetry. If any fields that are not 
swapped in the other flow direction or if (for example) only src is 
used, the hardware will generate non-symmetric hash.


> 
> My thought would be to possibly just look at reducing your messaging
> to a warning from the driver if the inputs are not symmetric, but you
> have your symmetric xor hash function enabled.

With the restrictions (to be moved into ice_ethtool), the user is unable 
to use non-symmetric inputs.

Re: [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Alexander Duyck]

On Tue, Oct 17, 2023 at 12:15 PM Ahmed Zaki <ahmed.zaki@intel.com> wrote:
>
>
>
> On 2023-10-17 12:42, Alexander Duyck wrote:
> > On Mon, Oct 16, 2023 at 5:08 PM Ahmed Zaki <ahmed.zaki@intel.com> wrote:
> >>
> >>
> >>
> >> On 2023-10-16 17:30, Jakub Kicinski wrote:
> >>> On Mon, 16 Oct 2023 15:55:21 -0700 Alexander Duyck wrote:
> >>>> It would make more sense to just add it as a variant hash function of
> >>>> toeplitz. If you did it right you could probably make the formatting
> >>>> pretty, something like:
> >>>> RSS hash function:
> >>>>       toeplitz: on
> >>>>           symmetric xor: on
> >>>>       xor: off
> >>>>       crc32: off
> >>>>
> >>>> It doesn't make sense to place it in the input flags and will just
> >>>> cause quick congestion as things get added there. This is an algorithm
> >>>> change so it makes more sense to place it there.
> >>>
> >>> Algo is also a bit confusing, it's more like key pre-processing?
> >>> There's nothing toeplitz about xoring input fields. Works as well
> >>> for CRC32.. or XOR.
> >>>
> >>> We can use one of the reserved fields of struct ethtool_rxfh to carry
> >>> this extension. I think I asked for this at some point, but there's
> >>> only so much repeated feedback one can send in a day :(
> >>
> >> Sorry you felt that. I took you comment [1]:
> >>
> >> "Using hashing algo for configuring fields feels like a dirty hack".
> >>
> >> To mean that the we should not use the hfunc API ("ethtool_rxfh"). This
> >> is why in the new series I chose to configure the RSS fields. This also
> >> provides the user with more control and better granularity on which
> >> flow-types to be symmetric, and which protocols (L3 and/or L4) to use. I
> >> have no idea how to do any of these via hfunc/ethtool_rxfh API so it
> >> seemed a better approach.
> >>
> >> I see you marked the series as "Changes Requested". I will send a new
> >> version tomorrow and move the sanity checks inside ice_ethtool.
> >>
> >>
> >> [1]: https://lore.kernel.org/netdev/20230824174336.6fb801d5@kernel.org/
> >
> > So one question I would have is what happens if you were to ignore the
> > extra configuration that prevents people from disabling either source
> > or destination from the input? Does it actually have to be hard
> > restricted or do you end up with the hardware generating non-symmetric
> > hashes because it isn't doing the XOR with both source and destination
> > fields?
>
> Do you mean allow the user to use any RSS fields as input? What do we
> gain by that?
>
> The hardware's TOEPLITZ and SYM_TOEPLITZ functions are the same except
> for the XORing step. What gets XOR'd needs to be programmed (Patch 5:
> ice_rss_config_xor()) and we are programming the hardware to XOR the src
> and dst fields to get this hash symmetry. If any fields that are not
> swapped in the other flow direction or if (for example) only src is
> used, the hardware will generate non-symmetric hash.

The point I am getting at is to determine if the
toeplitz-symmetric-xor is actually changes to the inputs or a change
to the algorithm. Based on your description here it is essentially a
subset of toeplitz, and all of the same inputs would apply. All you
have essentially done is collapsed the key. Rather than symmetric
toeplitz this could almost be considered simplified toeplitz.

One side effect of XORing the source and destination data is that you
can essentially collapse the key. You could XOR together the 5 DWORDs
(159 bits) associated with the source and destination IP portion of
the key, and then do the same with the 3 WORDs (47 bits) associated
with the source and destination port. Then you would only have to
process the XORed inputs. As a result you are going to lose a fair bit
of entropy since it effectively cuts the input length and key length
in half. The same could essentially be done by doing a bit of key
manipulation, the simplest approach being using a 16b repeating key
value, and the more nuanced requiring paying attention to IP and port
boundaries in terms of repetition. I would say because of the extra
processing steps it is a different hfunc versus just being a different
set of inputs.

> >
> > My thought would be to possibly just look at reducing your messaging
> > to a warning from the driver if the inputs are not symmetric, but you
> > have your symmetric xor hash function enabled.
>
> With the restrictions (to be moved into ice_ethtool), the user is unable
> to use non-symmetric inputs.

I think a warning would make more sense than an outright restriction.
You could warn on both the enabling if the mask is already unbalanced,
or you could warn if the mask is set to be unbalanced after enabling
your hashing.

Re: [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Jakub Kicinski]

On Tue, 17 Oct 2023 13:03:39 -0700 Alexander Duyck wrote:
> > > My thought would be to possibly just look at reducing your messaging
> > > to a warning from the driver if the inputs are not symmetric, but you
> > > have your symmetric xor hash function enabled.  
> >
> > With the restrictions (to be moved into ice_ethtool), the user is unable
> > to use non-symmetric inputs.  
> 
> I think a warning would make more sense than an outright restriction.
> You could warn on both the enabling if the mask is already unbalanced,
> or you could warn if the mask is set to be unbalanced after enabling
> your hashing.

Either it's a valid configuration or we should error out in the core.
Keep in mind that we can always _loosen_ the restriction, like you
asked for VLAN ID, but we can never _tighten_ it without breaking uAPI.
So error.

Re: [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Alexander Duyck]

On Tue, Oct 17, 2023 at 1:20 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue, 17 Oct 2023 13:03:39 -0700 Alexander Duyck wrote:
> > > > My thought would be to possibly just look at reducing your messaging
> > > > to a warning from the driver if the inputs are not symmetric, but you
> > > > have your symmetric xor hash function enabled.
> > >
> > > With the restrictions (to be moved into ice_ethtool), the user is unable
> > > to use non-symmetric inputs.
> >
> > I think a warning would make more sense than an outright restriction.
> > You could warn on both the enabling if the mask is already unbalanced,
> > or you could warn if the mask is set to be unbalanced after enabling
> > your hashing.
>
> Either it's a valid configuration or we should error out in the core.
> Keep in mind that we can always _loosen_ the restriction, like you
> asked for VLAN ID, but we can never _tighten_ it without breaking uAPI.
> So error.

I would say it is a valid configuration then. If the user opts to
shoot themselves in the foot then so be it. It doesn't actually break
anything and is just there to make sure the hashing conforms to the
marketing use case.

Re: [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Alexander Duyck]

On Mon, Oct 16, 2023 at 4:31 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 16 Oct 2023 15:55:21 -0700 Alexander Duyck wrote:
> > It would make more sense to just add it as a variant hash function of
> > toeplitz. If you did it right you could probably make the formatting
> > pretty, something like:
> > RSS hash function:
> >     toeplitz: on
> >         symmetric xor: on
> >     xor: off
> >     crc32: off
> >
> > It doesn't make sense to place it in the input flags and will just
> > cause quick congestion as things get added there. This is an algorithm
> > change so it makes more sense to place it there.
>
> Algo is also a bit confusing, it's more like key pre-processing?
> There's nothing toeplitz about xoring input fields. Works as well
> for CRC32.. or XOR.

I agree that the change to the algorithm doesn't necessarily have
anything to do with toeplitz, however it is still a change to the
algorithm by performing the extra XOR on the inputs prior to
processing. That is why I figured it might make sense to just add a
new hfunc value that would mean toeplitz w/ symmetric XOR.

> We can use one of the reserved fields of struct ethtool_rxfh to carry
> this extension. I think I asked for this at some point, but there's
> only so much repeated feedback one can send in a day :(

Why add an extra reserved field when this is just a variant on a hash
function? I view it as not being dissimilar to how we handle TSO or
tx-checksumming. It would make sense to me to just set something like
toeplitz-symmetric-xor to on in order to turn this on.

Re: [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Jakub Kicinski]

On Tue, 17 Oct 2023 11:37:52 -0700 Alexander Duyck wrote:
> > Algo is also a bit confusing, it's more like key pre-processing?
> > There's nothing toeplitz about xoring input fields. Works as well
> > for CRC32.. or XOR.  
> 
> I agree that the change to the algorithm doesn't necessarily have
> anything to do with toeplitz, however it is still a change to the
> algorithm by performing the extra XOR on the inputs prior to
> processing. That is why I figured it might make sense to just add a
> new hfunc value that would mean toeplitz w/ symmetric XOR.

XOR is just one form of achieving symmetric hashing, sorting is another.

> > We can use one of the reserved fields of struct ethtool_rxfh to carry
> > this extension. I think I asked for this at some point, but there's
> > only so much repeated feedback one can send in a day :(  
> 
> Why add an extra reserved field when this is just a variant on a hash
> function? I view it as not being dissimilar to how we handle TSO or
> tx-checksumming. It would make sense to me to just set something like
> toeplitz-symmetric-xor to on in order to turn this on.

It's entirely orthogonal. {sym-XOR, sym-sort} x {toep, crc, xor} -
all combinations can work.

Forget the "is it algo or not algo" question, just purely from data
normalization perspective, in terms of the API, if combinations make
sense they should be controllable independently.

https://en.wikipedia.org/wiki/First_normal_form

Re: [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Alexander Duyck]

On Tue, Oct 17, 2023 at 1:17 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue, 17 Oct 2023 11:37:52 -0700 Alexander Duyck wrote:
> > > Algo is also a bit confusing, it's more like key pre-processing?
> > > There's nothing toeplitz about xoring input fields. Works as well
> > > for CRC32.. or XOR.
> >
> > I agree that the change to the algorithm doesn't necessarily have
> > anything to do with toeplitz, however it is still a change to the
> > algorithm by performing the extra XOR on the inputs prior to
> > processing. That is why I figured it might make sense to just add a
> > new hfunc value that would mean toeplitz w/ symmetric XOR.
>
> XOR is just one form of achieving symmetric hashing, sorting is another.

Right, but there are huge algorithmic differences between the two.
With sorting you don't lose any entropy, whereas with XOR you do. For
example one side effect of XOR is that for every two hosts on the same
IP subnet the IP subnets will cancel out. As such with the same key
192.168.0.1->192.168.0.2 will hash out essentially the same as
fc::1->fc::2.

> > > We can use one of the reserved fields of struct ethtool_rxfh to carry
> > > this extension. I think I asked for this at some point, but there's
> > > only so much repeated feedback one can send in a day :(
> >
> > Why add an extra reserved field when this is just a variant on a hash
> > function? I view it as not being dissimilar to how we handle TSO or
> > tx-checksumming. It would make sense to me to just set something like
> > toeplitz-symmetric-xor to on in order to turn this on.
>
> It's entirely orthogonal. {sym-XOR, sym-sort} x {toep, crc, xor} -
> all combinations can work.
>
> Forget the "is it algo or not algo" question, just purely from data
> normalization perspective, in terms of the API, if combinations make
> sense they should be controllable independently.
>
> https://en.wikipedia.org/wiki/First_normal_form

I am thinking of this from a software engineering perspective. This
symmetric-xor aka simplified-toeplitz is actually much cheaper to
implement in software than the original. As such I would want it to be
considered a separate algorithm as I could make use of something like
that when having to implement RSS in QEMU for instance. Based on
earlier comments it doesn't change the inputs, it just changes how I
have to handle the data and the key. It starts reducing things down to
something like the Intel implementation of Flow Director in terms of
how the key gets generated and hashed.

As far as sorting that is a different can of worms, but I would be
more open to that being an input specific thing as all it would affect
is the ordering of the fields, it doesn't impact how I would have to
handle the key or hash the inputs.

Re: [Intel-wired-lan] [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Ahmed Zaki]

On 2023-10-17 14:41, Alexander Duyck wrote:
> On Tue, Oct 17, 2023 at 1:17 PM Jakub Kicinski <kuba@kernel.org> wrote:
>>
>> On Tue, 17 Oct 2023 11:37:52 -0700 Alexander Duyck wrote:
>>>> Algo is also a bit confusing, it's more like key pre-processing?
>>>> There's nothing toeplitz about xoring input fields. Works as well
>>>> for CRC32.. or XOR.
>>>
>>> I agree that the change to the algorithm doesn't necessarily have
>>> anything to do with toeplitz, however it is still a change to the
>>> algorithm by performing the extra XOR on the inputs prior to
>>> processing. That is why I figured it might make sense to just add a
>>> new hfunc value that would mean toeplitz w/ symmetric XOR.
>>
>> XOR is just one form of achieving symmetric hashing, sorting is another.
> 
> Right, but there are huge algorithmic differences between the two.
> With sorting you don't lose any entropy, whereas with XOR you do. For
> example one side effect of XOR is that for every two hosts on the same
> IP subnet the IP subnets will cancel out. As such with the same key
> 192.168.0.1->192.168.0.2 will hash out essentially the same as
> fc::1->fc::2.

I agree of course that we lose entropy by XORing, but don't we also lose 
entropy, for example, if we hash only the L4 dst_port vs (ip_src, 
ip_dst, l4_src, l4_dst,..etc)? we still say we are using the same alg.


>>>> We can use one of the reserved fields of struct ethtool_rxfh to carry
>>>> this extension. I think I asked for this at some point, but there's
>>>> only so much repeated feedback one can send in a day :(
>>>
>>> Why add an extra reserved field when this is just a variant on a hash
>>> function? I view it as not being dissimilar to how we handle TSO or
>>> tx-checksumming. It would make sense to me to just set something like
>>> toeplitz-symmetric-xor to on in order to turn this on.
>>
>> It's entirely orthogonal. {sym-XOR, sym-sort} x {toep, crc, xor} -
>> all combinations can work.
>>
>> Forget the "is it algo or not algo" question, just purely from data
>> normalization perspective, in terms of the API, if combinations make
>> sense they should be controllable independently.
>>
>> https://en.wikipedia.org/wiki/First_normal_form
> 
> I am thinking of this from a software engineering perspective. This
> symmetric-xor aka simplified-toeplitz is actually much cheaper to
> implement in software than the original. As such I would want it to be
> considered a separate algorithm as I could make use of something like
> that when having to implement RSS in QEMU for instance. Based on
> earlier comments it doesn't change the inputs, it just changes how I
> have to handle the data and the key. It starts reducing things down to
> something like the Intel implementation of Flow Director in terms of
> how the key gets generated and hashed.

The key is independent of all of this discussion. It is set by the user 
and whatever that key is, the hardware (after properly configuring what 
fields are XOR'd) will generate the symmetric hash from the input data. 
The "alg" does not handle or manipulate the key.

Re: [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Jakub Kicinski]

On Tue, 17 Oct 2023 13:41:18 -0700 Alexander Duyck wrote:
> I am thinking of this from a software engineering perspective. This
> symmetric-xor aka simplified-toeplitz is actually much cheaper to
> implement in software than the original. As such I would want it to be
> considered a separate algorithm as I could make use of something like
> that when having to implement RSS in QEMU for instance.

That's exactly why XOR and CRC32 _algorithms_ already exist.
CPUs have instructions to do them word at a time. 

	ETH_RSS_HASH_TOP_BIT, /* Configurable RSS hash function -
	Toeplitz */
	ETH_RSS_HASH_XOR_BIT, /* Configurable RSS hash function - Xor */
	ETH_RSS_HASH_CRC32_BIT, /* Configurable RSS hash function - Crc32 */

If efficient SW implementation is important why do some weird
bastardized para-toeplitz and not crc32? Hashes fairly well
from what I recall with the older NFPs. x86 has an instruction
for it, IIRC it was part of SSE but on normal registers.

> Based on earlier comments it doesn't change the inputs, it just
> changes how I have to handle the data and the key. It starts reducing
> things down to something like the Intel implementation of Flow
> Director in terms of how the key gets generated and hashed.

About Flow Director I know only that it is bad :)

Re: [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Alexander Duyck]

On Tue, Oct 17, 2023 at 5:34 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue, 17 Oct 2023 13:41:18 -0700 Alexander Duyck wrote:
> > I am thinking of this from a software engineering perspective. This
> > symmetric-xor aka simplified-toeplitz is actually much cheaper to
> > implement in software than the original. As such I would want it to be
> > considered a separate algorithm as I could make use of something like
> > that when having to implement RSS in QEMU for instance.
>
> That's exactly why XOR and CRC32 _algorithms_ already exist.
> CPUs have instructions to do them word at a time.
>
>         ETH_RSS_HASH_TOP_BIT, /* Configurable RSS hash function -
>         Toeplitz */
>         ETH_RSS_HASH_XOR_BIT, /* Configurable RSS hash function - Xor */
>         ETH_RSS_HASH_CRC32_BIT, /* Configurable RSS hash function - Crc32 */
>
> If efficient SW implementation is important why do some weird
> bastardized para-toeplitz and not crc32? Hashes fairly well
> from what I recall with the older NFPs. x86 has an instruction
> for it, IIRC it was part of SSE but on normal registers.

If we want to not support that I would be fine with that too. In my
view this is about as secure as using the 16b repeating key.

> > Based on earlier comments it doesn't change the inputs, it just
> > changes how I have to handle the data and the key. It starts reducing
> > things down to something like the Intel implementation of Flow
> > Director in terms of how the key gets generated and hashed.
>
> About Flow Director I know only that it is bad :)

Yeah, and that is my concern w/ the symmetric XOR is that it isn't
good. It opens up the toeplitz hash to exploitation. You can target
the same bucket by just making sure that source IP and port XOR with
destination IP and port to the same value. That can be done by adding
the same amount to each side. So there are 2^144 easily predictable
possible combinations that will end up in the same hash bucket. Seems
like it might be something that could be exploitable. That is why I
want it marked out as a separate algo since it is essentially
destroying entropy before we even get to the Toeplitz portion of the
hash. As such it isn't a hash I would want to use for anything that is
meant to spread workload since it is so easily exploitable.

Re: [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Jakub Kicinski]

On Wed, 18 Oct 2023 11:12:13 -0700 Alexander Duyck wrote:
> > > Based on earlier comments it doesn't change the inputs, it just
> > > changes how I have to handle the data and the key. It starts reducing
> > > things down to something like the Intel implementation of Flow
> > > Director in terms of how the key gets generated and hashed.  
> >
> > About Flow Director I know only that it is bad :)  
> 
> Yeah, and that is my concern w/ the symmetric XOR is that it isn't
> good. It opens up the toeplitz hash to exploitation. You can target
> the same bucket by just making sure that source IP and port XOR with
> destination IP and port to the same value. That can be done by adding
> the same amount to each side. So there are 2^144 easily predictable
> possible combinations that will end up in the same hash bucket. Seems
> like it might be something that could be exploitable. That is why I
> want it marked out as a separate algo since it is essentially
> destroying entropy before we even get to the Toeplitz portion of the
> hash. As such it isn't a hash I would want to use for anything that is
> meant to spread workload since it is so easily exploitable.

I see your point.

Which is not to say that I know what to do about it. crc or any
future secure algo will get destroyed all the same. It's the input
entropy that gets destroyed, independently of the algo.

We already support xor, and it doesn't come with a warning saying
it's insecure so we kind of assume user knows what they are doing.

I think the API we pick for configuring sym-xor should be the same as
sym-sort. And the "makes algo insecure" argument won't apply to sort.

IMO fat warning in the documentation and ethtool man saying that this
makes the algo (any / all) vulnerable to attack would be enough.
Willem?

Re: [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Ahmed Zaki]

On 2023-10-18 17:50, Jakub Kicinski wrote:
> On Wed, 18 Oct 2023 11:12:13 -0700 Alexander Duyck wrote:
>>>> Based on earlier comments it doesn't change the inputs, it just
>>>> changes how I have to handle the data and the key. It starts reducing
>>>> things down to something like the Intel implementation of Flow
>>>> Director in terms of how the key gets generated and hashed.
>>>
>>> About Flow Director I know only that it is bad :)
>>
>> Yeah, and that is my concern w/ the symmetric XOR is that it isn't
>> good. It opens up the toeplitz hash to exploitation. You can target
>> the same bucket by just making sure that source IP and port XOR with
>> destination IP and port to the same value. That can be done by adding
>> the same amount to each side. So there are 2^144 easily predictable
>> possible combinations that will end up in the same hash bucket. Seems
>> like it might be something that could be exploitable. That is why I
>> want it marked out as a separate algo since it is essentially
>> destroying entropy before we even get to the Toeplitz portion of the
>> hash. As such it isn't a hash I would want to use for anything that is
>> meant to spread workload since it is so easily exploitable.
> 
> I see your point.
> 
> Which is not to say that I know what to do about it. crc or any
> future secure algo will get destroyed all the same. It's the input
> entropy that gets destroyed, independently of the algo.
> 
> We already support xor, and it doesn't come with a warning saying
> it's insecure so we kind of assume user knows what they are doing.
> 
> I think the API we pick for configuring sym-xor should be the same as
> sym-sort. And the "makes algo insecure" argument won't apply to sort.
> 
> IMO fat warning in the documentation and ethtool man saying that this
> makes the algo (any / all) vulnerable to attack would be enough.
> Willem?

Please advise on the next step. Should I send a new version with the Doc 
warning, or will you use v5?

Thanks.

Re: [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Jakub Kicinski]

On Fri, 20 Oct 2023 15:24:41 -0600 Ahmed Zaki wrote:
> > IMO fat warning in the documentation and ethtool man saying that this
> > makes the algo (any / all) vulnerable to attack would be enough.
> > Willem?  
> 
> Please advise on the next step. Should I send a new version with the Doc 
> warning, or will you use v5?

Not just the doc changes:

| We can use one of the reserved fields of struct ethtool_rxfh to carry
| this extension. I think I asked for this at some point, but there's
| only so much repeated feedback one can send in a day :(

https://lore.kernel.org/all/20231016163059.23799429@kernel.org/

You can take care of that, post v6 and see what Alex and Willem say.

Re: [Intel-wired-lan] [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Ahmed Zaki]

On 2023-10-20 16:33, Jakub Kicinski wrote:
> On Fri, 20 Oct 2023 15:24:41 -0600 Ahmed Zaki wrote:
>>> IMO fat warning in the documentation and ethtool man saying that this
>>> makes the algo (any / all) vulnerable to attack would be enough.
>>> Willem?
>>
>> Please advise on the next step. Should I send a new version with the Doc
>> warning, or will you use v5?
> 
> Not just the doc changes:
> 
> | We can use one of the reserved fields of struct ethtool_rxfh to carry
> | this extension. I think I asked for this at some point, but there's
> | only so much repeated feedback one can send in a day :(
> 
> https://lore.kernel.org/all/20231016163059.23799429@kernel.org/

I replied to that here:

https://lore.kernel.org/all/afb4a06f-cfba-47ba-adb3-09bea7cb5f00@intel.com/

I am kind of confused now so please bear with me. ethtool either sends 
"ethtool_rxfh" or "ethtool_rxnfc". AFAIK "ethtool_rxfh" is the interface 
for "ethtool -X" which is used to set the RSS algorithm. But we kind of 
agreed to go with "ethtool -U|-N" for symmetric-xor, and that uses 
"ethtool_rxnfc" (as implemented in this series).

Do you mean use "ethtool_rxfh" instead of "ethtool_rxnfc"? how would 
that work on the ethtool user interface?

Finally, a note on Alex's comment:
 >It doesn't make sense to place it in the input flags and will just
 > cause quick congestion as things get added there. This is an algorithm
 > change so it makes more sense to place it there.

the "ethtool_rxnfc->data" is 64 bits and we are only using 8 bits so far.

Thank you.

Re: [Intel-wired-lan] [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Jakub Kicinski]

On Fri, 20 Oct 2023 17:14:11 -0600 Ahmed Zaki wrote:
> I replied to that here:
> 
> https://lore.kernel.org/all/afb4a06f-cfba-47ba-adb3-09bea7cb5f00@intel.com/
> 
> I am kind of confused now so please bear with me. ethtool either sends 
> "ethtool_rxfh" or "ethtool_rxnfc". AFAIK "ethtool_rxfh" is the interface 
> for "ethtool -X" which is used to set the RSS algorithm. But we kind of 
> agreed to go with "ethtool -U|-N" for symmetric-xor, and that uses 
> "ethtool_rxnfc" (as implemented in this series).

I have no strong preference. Sounds like Alex prefers to keep it closer
to algo, which is "ethtool_rxfh".

> Do you mean use "ethtool_rxfh" instead of "ethtool_rxnfc"? how would 
> that work on the ethtool user interface?

I don't know what you're asking of us. If you find the code to confusing
maybe someone at Intel can help you :|

Re: [Intel-wired-lan] [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Ahmed Zaki]

On 2023-10-20 17:49, Jakub Kicinski wrote:
> On Fri, 20 Oct 2023 17:14:11 -0600 Ahmed Zaki wrote:
>> I replied to that here:
>>
>> https://lore.kernel.org/all/afb4a06f-cfba-47ba-adb3-09bea7cb5f00@intel.com/
>>
>> I am kind of confused now so please bear with me. ethtool either sends
>> "ethtool_rxfh" or "ethtool_rxnfc". AFAIK "ethtool_rxfh" is the interface
>> for "ethtool -X" which is used to set the RSS algorithm. But we kind of
>> agreed to go with "ethtool -U|-N" for symmetric-xor, and that uses
>> "ethtool_rxnfc" (as implemented in this series).
> 
> I have no strong preference. Sounds like Alex prefers to keep it closer
> to algo, which is "ethtool_rxfh".
> 
>> Do you mean use "ethtool_rxfh" instead of "ethtool_rxnfc"? how would
>> that work on the ethtool user interface?
> 
> I don't know what you're asking of us. If you find the code to confusing
> maybe someone at Intel can help you :|

The code is straightforward. I am confused by the requirements: don't 
add a new algorithm but use "ethtool_rxfh".

I'll see if I can get more help, may be I am missing something.

Re: [Intel-wired-lan] [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Gal Pressman]

On 21/10/2023 3:00, Ahmed Zaki wrote:
> 
> 
> On 2023-10-20 17:49, Jakub Kicinski wrote:
>> On Fri, 20 Oct 2023 17:14:11 -0600 Ahmed Zaki wrote:
>>> I replied to that here:
>>>
>>> https://lore.kernel.org/all/afb4a06f-cfba-47ba-adb3-09bea7cb5f00@intel.com/
>>>
>>> I am kind of confused now so please bear with me. ethtool either sends
>>> "ethtool_rxfh" or "ethtool_rxnfc". AFAIK "ethtool_rxfh" is the interface
>>> for "ethtool -X" which is used to set the RSS algorithm. But we kind of
>>> agreed to go with "ethtool -U|-N" for symmetric-xor, and that uses
>>> "ethtool_rxnfc" (as implemented in this series).
>>
>> I have no strong preference. Sounds like Alex prefers to keep it closer
>> to algo, which is "ethtool_rxfh".
>>
>>> Do you mean use "ethtool_rxfh" instead of "ethtool_rxnfc"? how would
>>> that work on the ethtool user interface?
>>
>> I don't know what you're asking of us. If you find the code to confusing
>> maybe someone at Intel can help you :|
> 
> The code is straightforward. I am confused by the requirements: don't
> add a new algorithm but use "ethtool_rxfh".
> 
> I'll see if I can get more help, may be I am missing something.
> 

What was the decision here?
Is this going to be exposed through ethtool -N or -X?

Re: [Intel-wired-lan] [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Ahmed Zaki]

On 2023-10-29 06:25, Gal Pressman wrote:
> On 21/10/2023 3:00, Ahmed Zaki wrote:
>>
>>
>> On 2023-10-20 17:49, Jakub Kicinski wrote:
>>> On Fri, 20 Oct 2023 17:14:11 -0600 Ahmed Zaki wrote:
>>>> I replied to that here:
>>>>
>>>> https://lore.kernel.org/all/afb4a06f-cfba-47ba-adb3-09bea7cb5f00@intel.com/
>>>>
>>>> I am kind of confused now so please bear with me. ethtool either sends
>>>> "ethtool_rxfh" or "ethtool_rxnfc". AFAIK "ethtool_rxfh" is the interface
>>>> for "ethtool -X" which is used to set the RSS algorithm. But we kind of
>>>> agreed to go with "ethtool -U|-N" for symmetric-xor, and that uses
>>>> "ethtool_rxnfc" (as implemented in this series).
>>>
>>> I have no strong preference. Sounds like Alex prefers to keep it closer
>>> to algo, which is "ethtool_rxfh".
>>>
>>>> Do you mean use "ethtool_rxfh" instead of "ethtool_rxnfc"? how would
>>>> that work on the ethtool user interface?
>>>
>>> I don't know what you're asking of us. If you find the code to confusing
>>> maybe someone at Intel can help you :|
>>
>> The code is straightforward. I am confused by the requirements: don't
>> add a new algorithm but use "ethtool_rxfh".
>>
>> I'll see if I can get more help, may be I am missing something.
>>
> 
> What was the decision here?
> Is this going to be exposed through ethtool -N or -X?

I am working on a new version that uses "ethtool_rxfh" to set the 
symmetric-xor. The user will set per-device via:

ethtool -X eth0 hfunc toeplitz symmetric-xor

then specify the per-flow type RSS fields as usual:

ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n

The downside is that all flow-types will have to be either symmetric or 
asymmetric.

I should be able to send this early in the next cycle.

Re: [Intel-wired-lan] [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Gal Pressman]

On 29/10/2023 14:42, Ahmed Zaki wrote:
> 
> 
> On 2023-10-29 06:25, Gal Pressman wrote:
>> On 21/10/2023 3:00, Ahmed Zaki wrote:
>>>
>>>
>>> On 2023-10-20 17:49, Jakub Kicinski wrote:
>>>> On Fri, 20 Oct 2023 17:14:11 -0600 Ahmed Zaki wrote:
>>>>> I replied to that here:
>>>>>
>>>>> https://lore.kernel.org/all/afb4a06f-cfba-47ba-adb3-09bea7cb5f00@intel.com/
>>>>>
>>>>> I am kind of confused now so please bear with me. ethtool either sends
>>>>> "ethtool_rxfh" or "ethtool_rxnfc". AFAIK "ethtool_rxfh" is the
>>>>> interface
>>>>> for "ethtool -X" which is used to set the RSS algorithm. But we
>>>>> kind of
>>>>> agreed to go with "ethtool -U|-N" for symmetric-xor, and that uses
>>>>> "ethtool_rxnfc" (as implemented in this series).
>>>>
>>>> I have no strong preference. Sounds like Alex prefers to keep it closer
>>>> to algo, which is "ethtool_rxfh".
>>>>
>>>>> Do you mean use "ethtool_rxfh" instead of "ethtool_rxnfc"? how would
>>>>> that work on the ethtool user interface?
>>>>
>>>> I don't know what you're asking of us. If you find the code to
>>>> confusing
>>>> maybe someone at Intel can help you :|
>>>
>>> The code is straightforward. I am confused by the requirements: don't
>>> add a new algorithm but use "ethtool_rxfh".
>>>
>>> I'll see if I can get more help, may be I am missing something.
>>>
>>
>> What was the decision here?
>> Is this going to be exposed through ethtool -N or -X?
> 
> I am working on a new version that uses "ethtool_rxfh" to set the
> symmetric-xor. The user will set per-device via:
> 
> ethtool -X eth0 hfunc toeplitz symmetric-xor
> 
> then specify the per-flow type RSS fields as usual:
> 
> ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n
> 
> The downside is that all flow-types will have to be either symmetric or
> asymmetric.

Why are we making the interface less flexible than it can be with -N?

Re: [Intel-wired-lan] [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Ahmed Zaki]

On 2023-10-29 06:48, Gal Pressman wrote:
> On 29/10/2023 14:42, Ahmed Zaki wrote:
>>
>>
>> On 2023-10-29 06:25, Gal Pressman wrote:
>>> On 21/10/2023 3:00, Ahmed Zaki wrote:
>>>>
>>>>
>>>> On 2023-10-20 17:49, Jakub Kicinski wrote:
>>>>> On Fri, 20 Oct 2023 17:14:11 -0600 Ahmed Zaki wrote:
>>>>>> I replied to that here:
>>>>>>
>>>>>> https://lore.kernel.org/all/afb4a06f-cfba-47ba-adb3-09bea7cb5f00@intel.com/
>>>>>>
>>>>>> I am kind of confused now so please bear with me. ethtool either sends
>>>>>> "ethtool_rxfh" or "ethtool_rxnfc". AFAIK "ethtool_rxfh" is the
>>>>>> interface
>>>>>> for "ethtool -X" which is used to set the RSS algorithm. But we
>>>>>> kind of
>>>>>> agreed to go with "ethtool -U|-N" for symmetric-xor, and that uses
>>>>>> "ethtool_rxnfc" (as implemented in this series).
>>>>>
>>>>> I have no strong preference. Sounds like Alex prefers to keep it closer
>>>>> to algo, which is "ethtool_rxfh".
>>>>>
>>>>>> Do you mean use "ethtool_rxfh" instead of "ethtool_rxnfc"? how would
>>>>>> that work on the ethtool user interface?
>>>>>
>>>>> I don't know what you're asking of us. If you find the code to
>>>>> confusing
>>>>> maybe someone at Intel can help you :|
>>>>
>>>> The code is straightforward. I am confused by the requirements: don't
>>>> add a new algorithm but use "ethtool_rxfh".
>>>>
>>>> I'll see if I can get more help, may be I am missing something.
>>>>
>>>
>>> What was the decision here?
>>> Is this going to be exposed through ethtool -N or -X?
>>
>> I am working on a new version that uses "ethtool_rxfh" to set the
>> symmetric-xor. The user will set per-device via:
>>
>> ethtool -X eth0 hfunc toeplitz symmetric-xor
>>
>> then specify the per-flow type RSS fields as usual:
>>
>> ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n
>>
>> The downside is that all flow-types will have to be either symmetric or
>> asymmetric.
> 
> Why are we making the interface less flexible than it can be with -N?

Alexander Duyck prefers to implement the "symmetric-xor" interface as an 
algorithm or extension (please refer to previous messages), but ethtool 
does not provide flowtype/RSS fields setting via "-X". The above was the 
best solution that we (at Intel) could think of.


Another solution would be to add a similar flowtype interface to "-X":

ethtool -X eth0 hfunc toeplitz [symmetric-xor rx-flow-hash <flow_type>]

which will allow the user to set "symmetric-xor" per flow-type. IMHO 
such approach is confusing; consider if the user sets:

ethtool -X eth0 ALG-1 symmetric-xor rx-flow-hash tcp4

and then:

ethtool -X eth0 ALG-2

should we switch tcp4 to ALG-2? Also, just the idea of replicating 
"rx-flow-hash" did not sound good overall to me.


Anyway, we thought that, if we are using "-X", then limiting all 
flow-types to whatever is set with "-X" is cleaner and works best with 
the current ethtool design. Any other suggestions are welcome of course.

Re: [Intel-wired-lan] [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Gal Pressman]

On 29/10/2023 18:59, Ahmed Zaki wrote:
> 
> 
> On 2023-10-29 06:48, Gal Pressman wrote:
>> On 29/10/2023 14:42, Ahmed Zaki wrote:
>>>
>>>
>>> On 2023-10-29 06:25, Gal Pressman wrote:
>>>> On 21/10/2023 3:00, Ahmed Zaki wrote:
>>>>>
>>>>>
>>>>> On 2023-10-20 17:49, Jakub Kicinski wrote:
>>>>>> On Fri, 20 Oct 2023 17:14:11 -0600 Ahmed Zaki wrote:
>>>>>>> I replied to that here:
>>>>>>>
>>>>>>> https://lore.kernel.org/all/afb4a06f-cfba-47ba-adb3-09bea7cb5f00@intel.com/
>>>>>>>
>>>>>>> I am kind of confused now so please bear with me. ethtool either
>>>>>>> sends
>>>>>>> "ethtool_rxfh" or "ethtool_rxnfc". AFAIK "ethtool_rxfh" is the
>>>>>>> interface
>>>>>>> for "ethtool -X" which is used to set the RSS algorithm. But we
>>>>>>> kind of
>>>>>>> agreed to go with "ethtool -U|-N" for symmetric-xor, and that uses
>>>>>>> "ethtool_rxnfc" (as implemented in this series).
>>>>>>
>>>>>> I have no strong preference. Sounds like Alex prefers to keep it
>>>>>> closer
>>>>>> to algo, which is "ethtool_rxfh".
>>>>>>
>>>>>>> Do you mean use "ethtool_rxfh" instead of "ethtool_rxnfc"? how would
>>>>>>> that work on the ethtool user interface?
>>>>>>
>>>>>> I don't know what you're asking of us. If you find the code to
>>>>>> confusing
>>>>>> maybe someone at Intel can help you :|
>>>>>
>>>>> The code is straightforward. I am confused by the requirements: don't
>>>>> add a new algorithm but use "ethtool_rxfh".
>>>>>
>>>>> I'll see if I can get more help, may be I am missing something.
>>>>>
>>>>
>>>> What was the decision here?
>>>> Is this going to be exposed through ethtool -N or -X?
>>>
>>> I am working on a new version that uses "ethtool_rxfh" to set the
>>> symmetric-xor. The user will set per-device via:
>>>
>>> ethtool -X eth0 hfunc toeplitz symmetric-xor
>>>
>>> then specify the per-flow type RSS fields as usual:
>>>
>>> ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n
>>>
>>> The downside is that all flow-types will have to be either symmetric or
>>> asymmetric.
>>
>> Why are we making the interface less flexible than it can be with -N?
> 
> Alexander Duyck prefers to implement the "symmetric-xor" interface as an
> algorithm or extension (please refer to previous messages), but ethtool
> does not provide flowtype/RSS fields setting via "-X". The above was the
> best solution that we (at Intel) could think of.

OK, it's a weird we're deliberately limiting our interface, given
there's already hardware that supports controlling symmetric hashing per
flow type.

I saw you mentioned the way ice hardware implements symmetric-xor
somewhere, it definitely needs to be added somewhere in our
documentation to prevent confusion.
mlx5 hardware also does symmetric hashing with xor, but not exactly as
you described, we need the algorithm to be clear.

Re: [Intel-wired-lan] [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Ahmed Zaki]

On 2023-10-31 06:00, Gal Pressman wrote:
> On 29/10/2023 18:59, Ahmed Zaki wrote:
>>
>>
>> On 2023-10-29 06:48, Gal Pressman wrote:
>>> On 29/10/2023 14:42, Ahmed Zaki wrote:
>>>>
>>>>
>>>> On 2023-10-29 06:25, Gal Pressman wrote:
>>>>> On 21/10/2023 3:00, Ahmed Zaki wrote:
>>>>>>
>>>>>>
>>>>>> On 2023-10-20 17:49, Jakub Kicinski wrote:
>>>>>>> On Fri, 20 Oct 2023 17:14:11 -0600 Ahmed Zaki wrote:
>>>>>>>> I replied to that here:
>>>>>>>>
>>>>>>>> https://lore.kernel.org/all/afb4a06f-cfba-47ba-adb3-09bea7cb5f00@intel.com/
>>>>>>>>
>>>>>>>> I am kind of confused now so please bear with me. ethtool either
>>>>>>>> sends
>>>>>>>> "ethtool_rxfh" or "ethtool_rxnfc". AFAIK "ethtool_rxfh" is the
>>>>>>>> interface
>>>>>>>> for "ethtool -X" which is used to set the RSS algorithm. But we
>>>>>>>> kind of
>>>>>>>> agreed to go with "ethtool -U|-N" for symmetric-xor, and that uses
>>>>>>>> "ethtool_rxnfc" (as implemented in this series).
>>>>>>>
>>>>>>> I have no strong preference. Sounds like Alex prefers to keep it
>>>>>>> closer
>>>>>>> to algo, which is "ethtool_rxfh".
>>>>>>>
>>>>>>>> Do you mean use "ethtool_rxfh" instead of "ethtool_rxnfc"? how would
>>>>>>>> that work on the ethtool user interface?
>>>>>>>
>>>>>>> I don't know what you're asking of us. If you find the code to
>>>>>>> confusing
>>>>>>> maybe someone at Intel can help you :|
>>>>>>
>>>>>> The code is straightforward. I am confused by the requirements: don't
>>>>>> add a new algorithm but use "ethtool_rxfh".
>>>>>>
>>>>>> I'll see if I can get more help, may be I am missing something.
>>>>>>
>>>>>
>>>>> What was the decision here?
>>>>> Is this going to be exposed through ethtool -N or -X?
>>>>
>>>> I am working on a new version that uses "ethtool_rxfh" to set the
>>>> symmetric-xor. The user will set per-device via:
>>>>
>>>> ethtool -X eth0 hfunc toeplitz symmetric-xor
>>>>
>>>> then specify the per-flow type RSS fields as usual:
>>>>
>>>> ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n
>>>>
>>>> The downside is that all flow-types will have to be either symmetric or
>>>> asymmetric.
>>>
>>> Why are we making the interface less flexible than it can be with -N?
>>
>> Alexander Duyck prefers to implement the "symmetric-xor" interface as an
>> algorithm or extension (please refer to previous messages), but ethtool
>> does not provide flowtype/RSS fields setting via "-X". The above was the
>> best solution that we (at Intel) could think of.
> 
> OK, it's a weird we're deliberately limiting our interface, given
> there's already hardware that supports controlling symmetric hashing per
> flow type.
> 
> I saw you mentioned the way ice hardware implements symmetric-xor
> somewhere, it definitely needs to be added somewhere in our
> documentation to prevent confusion.
> mlx5 hardware also does symmetric hashing with xor, but not exactly as
> you described, we need the algorithm to be clear.

Sure. I will add more ice-specific doc in:
Documentation/networking/device_drivers/ethernet/intel/ice.rst

Re: [Intel-wired-lan] [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Gal Pressman]

On 31/10/2023 16:40, Ahmed Zaki wrote:
> 
> 
> On 2023-10-31 06:00, Gal Pressman wrote:
>> On 29/10/2023 18:59, Ahmed Zaki wrote:
>>>
>>>
>>> On 2023-10-29 06:48, Gal Pressman wrote:
>>>> On 29/10/2023 14:42, Ahmed Zaki wrote:
>>>>>
>>>>>
>>>>> On 2023-10-29 06:25, Gal Pressman wrote:
>>>>>> On 21/10/2023 3:00, Ahmed Zaki wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 2023-10-20 17:49, Jakub Kicinski wrote:
>>>>>>>> On Fri, 20 Oct 2023 17:14:11 -0600 Ahmed Zaki wrote:
>>>>>>>>> I replied to that here:
>>>>>>>>>
>>>>>>>>> https://lore.kernel.org/all/afb4a06f-cfba-47ba-adb3-09bea7cb5f00@intel.com/
>>>>>>>>>
>>>>>>>>> I am kind of confused now so please bear with me. ethtool either
>>>>>>>>> sends
>>>>>>>>> "ethtool_rxfh" or "ethtool_rxnfc". AFAIK "ethtool_rxfh" is the
>>>>>>>>> interface
>>>>>>>>> for "ethtool -X" which is used to set the RSS algorithm. But we
>>>>>>>>> kind of
>>>>>>>>> agreed to go with "ethtool -U|-N" for symmetric-xor, and that uses
>>>>>>>>> "ethtool_rxnfc" (as implemented in this series).
>>>>>>>>
>>>>>>>> I have no strong preference. Sounds like Alex prefers to keep it
>>>>>>>> closer
>>>>>>>> to algo, which is "ethtool_rxfh".
>>>>>>>>
>>>>>>>>> Do you mean use "ethtool_rxfh" instead of "ethtool_rxnfc"? how
>>>>>>>>> would
>>>>>>>>> that work on the ethtool user interface?
>>>>>>>>
>>>>>>>> I don't know what you're asking of us. If you find the code to
>>>>>>>> confusing
>>>>>>>> maybe someone at Intel can help you :|
>>>>>>>
>>>>>>> The code is straightforward. I am confused by the requirements:
>>>>>>> don't
>>>>>>> add a new algorithm but use "ethtool_rxfh".
>>>>>>>
>>>>>>> I'll see if I can get more help, may be I am missing something.
>>>>>>>
>>>>>>
>>>>>> What was the decision here?
>>>>>> Is this going to be exposed through ethtool -N or -X?
>>>>>
>>>>> I am working on a new version that uses "ethtool_rxfh" to set the
>>>>> symmetric-xor. The user will set per-device via:
>>>>>
>>>>> ethtool -X eth0 hfunc toeplitz symmetric-xor
>>>>>
>>>>> then specify the per-flow type RSS fields as usual:
>>>>>
>>>>> ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n
>>>>>
>>>>> The downside is that all flow-types will have to be either
>>>>> symmetric or
>>>>> asymmetric.
>>>>
>>>> Why are we making the interface less flexible than it can be with -N?
>>>
>>> Alexander Duyck prefers to implement the "symmetric-xor" interface as an
>>> algorithm or extension (please refer to previous messages), but ethtool
>>> does not provide flowtype/RSS fields setting via "-X". The above was the
>>> best solution that we (at Intel) could think of.
>>
>> OK, it's a weird we're deliberately limiting our interface, given
>> there's already hardware that supports controlling symmetric hashing per
>> flow type.
>>
>> I saw you mentioned the way ice hardware implements symmetric-xor
>> somewhere, it definitely needs to be added somewhere in our
>> documentation to prevent confusion.
>> mlx5 hardware also does symmetric hashing with xor, but not exactly as
>> you described, we need the algorithm to be clear.
> 
> Sure. I will add more ice-specific doc in:
> Documentation/networking/device_drivers/ethernet/intel/ice.rst

I was thinking of somewhere more generic, where ethtool users (not
necessarily ice users) can refer to.

Perhaps Documentation/networking/ethtool-netlink.rst? Or ethtool man page?

Re: [Intel-wired-lan] [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Ahmed Zaki]

On 2023-10-31 08:45, Gal Pressman wrote:
> On 31/10/2023 16:40, Ahmed Zaki wrote:
>>
>>
>> On 2023-10-31 06:00, Gal Pressman wrote:
>>> On 29/10/2023 18:59, Ahmed Zaki wrote:
>>>>
>>>>
>>>> On 2023-10-29 06:48, Gal Pressman wrote:
>>>>> On 29/10/2023 14:42, Ahmed Zaki wrote:
>>>>>>
>>>>>>
>>>>>> On 2023-10-29 06:25, Gal Pressman wrote:
>>>>>>> On 21/10/2023 3:00, Ahmed Zaki wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2023-10-20 17:49, Jakub Kicinski wrote:
>>>>>>>>> On Fri, 20 Oct 2023 17:14:11 -0600 Ahmed Zaki wrote:
>>>>>>>>>> I replied to that here:
>>>>>>>>>>
>>>>>>>>>> https://lore.kernel.org/all/afb4a06f-cfba-47ba-adb3-09bea7cb5f00@intel.com/
>>>>>>>>>>
>>>>>>>>>> I am kind of confused now so please bear with me. ethtool either
>>>>>>>>>> sends
>>>>>>>>>> "ethtool_rxfh" or "ethtool_rxnfc". AFAIK "ethtool_rxfh" is the
>>>>>>>>>> interface
>>>>>>>>>> for "ethtool -X" which is used to set the RSS algorithm. But we
>>>>>>>>>> kind of
>>>>>>>>>> agreed to go with "ethtool -U|-N" for symmetric-xor, and that uses
>>>>>>>>>> "ethtool_rxnfc" (as implemented in this series).
>>>>>>>>>
>>>>>>>>> I have no strong preference. Sounds like Alex prefers to keep it
>>>>>>>>> closer
>>>>>>>>> to algo, which is "ethtool_rxfh".
>>>>>>>>>
>>>>>>>>>> Do you mean use "ethtool_rxfh" instead of "ethtool_rxnfc"? how
>>>>>>>>>> would
>>>>>>>>>> that work on the ethtool user interface?
>>>>>>>>>
>>>>>>>>> I don't know what you're asking of us. If you find the code to
>>>>>>>>> confusing
>>>>>>>>> maybe someone at Intel can help you :|
>>>>>>>>
>>>>>>>> The code is straightforward. I am confused by the requirements:
>>>>>>>> don't
>>>>>>>> add a new algorithm but use "ethtool_rxfh".
>>>>>>>>
>>>>>>>> I'll see if I can get more help, may be I am missing something.
>>>>>>>>
>>>>>>>
>>>>>>> What was the decision here?
>>>>>>> Is this going to be exposed through ethtool -N or -X?
>>>>>>
>>>>>> I am working on a new version that uses "ethtool_rxfh" to set the
>>>>>> symmetric-xor. The user will set per-device via:
>>>>>>
>>>>>> ethtool -X eth0 hfunc toeplitz symmetric-xor
>>>>>>
>>>>>> then specify the per-flow type RSS fields as usual:
>>>>>>
>>>>>> ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n
>>>>>>
>>>>>> The downside is that all flow-types will have to be either
>>>>>> symmetric or
>>>>>> asymmetric.
>>>>>
>>>>> Why are we making the interface less flexible than it can be with -N?
>>>>
>>>> Alexander Duyck prefers to implement the "symmetric-xor" interface as an
>>>> algorithm or extension (please refer to previous messages), but ethtool
>>>> does not provide flowtype/RSS fields setting via "-X". The above was the
>>>> best solution that we (at Intel) could think of.
>>>
>>> OK, it's a weird we're deliberately limiting our interface, given
>>> there's already hardware that supports controlling symmetric hashing per
>>> flow type.
>>>
>>> I saw you mentioned the way ice hardware implements symmetric-xor
>>> somewhere, it definitely needs to be added somewhere in our
>>> documentation to prevent confusion.
>>> mlx5 hardware also does symmetric hashing with xor, but not exactly as
>>> you described, we need the algorithm to be clear.
>>
>> Sure. I will add more ice-specific doc in:
>> Documentation/networking/device_drivers/ethernet/intel/ice.rst
> 
> I was thinking of somewhere more generic, where ethtool users (not
> necessarily ice users) can refer to.
> 
> Perhaps Documentation/networking/ethtool-netlink.rst? Or ethtool man page?

Do you mean add vendor-specific implementation details to common docs? 
Not sure if I have seen this before. Any examples?

Or, we can add a note in ethtool doc that each vendor's implementation 
is different and "Refer to your vendor's specifications for more info".

Re: [Intel-wired-lan] [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Jakub Kicinski]

On Tue, 31 Oct 2023 09:14:58 -0600 Ahmed Zaki wrote:
> Do you mean add vendor-specific implementation details to common docs? 
> Not sure if I have seen this before. Any examples?
> 
> Or, we can add a note in ethtool doc that each vendor's implementation 
> is different and "Refer to your vendor's specifications for more info".

Gal, can you shed any more detail on who your implementation differs?
It will help the discussion, and also - I'm not sure how you can do
xor differently..

Re: [Intel-wired-lan] [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Gal Pressman]

On 31/10/2023 17:20, Jakub Kicinski wrote:
> On Tue, 31 Oct 2023 09:14:58 -0600 Ahmed Zaki wrote:
>> Do you mean add vendor-specific implementation details to common docs? 
>> Not sure if I have seen this before. Any examples?
>>
>> Or, we can add a note in ethtool doc that each vendor's implementation 
>> is different and "Refer to your vendor's specifications for more info".
> 
> Gal, can you shed any more detail on who your implementation differs?
> It will help the discussion, and also - I'm not sure how you can do
> xor differently..

Sure, IIUC, ice's implementation does a:
(SRC_IP ^ DST_IP, SRC_IP ^ DST_IP, SRC_PORT ^ DST_PORT, SRC_PORT ^ DST_PORT)

Our implementation isn't exactly xor, it is:
(SRC_IP | DST_IP, SRC_IP ^ DST_IP, SRC_PORT | DST_PORT, SRC_PORT ^ DST_PORT)

The way I see it, the xor implementation should be clearly documented,
so no one uses the same flag with a different implementation by mistake.

Re: [Intel-wired-lan] [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Jakub Kicinski]

On Tue, 31 Oct 2023 18:13:20 +0200 Gal Pressman wrote:
> Sure, IIUC, ice's implementation does a:
> (SRC_IP ^ DST_IP, SRC_IP ^ DST_IP, SRC_PORT ^ DST_PORT, SRC_PORT ^ DST_PORT)
> 
> Our implementation isn't exactly xor, it is:
> (SRC_IP | DST_IP, SRC_IP ^ DST_IP, SRC_PORT | DST_PORT, SRC_PORT ^ DST_PORT)
> 
> The way I see it, the xor implementation should be clearly documented,
> so no one uses the same flag with a different implementation by mistake.

Got it, thanks!

Re: [Intel-wired-lan] [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Gal Pressman]

On 31/10/2023 17:14, Ahmed Zaki wrote:
> 
> 
> On 2023-10-31 08:45, Gal Pressman wrote:
>> On 31/10/2023 16:40, Ahmed Zaki wrote:
>>>
>>>
>>> On 2023-10-31 06:00, Gal Pressman wrote:
>>>> On 29/10/2023 18:59, Ahmed Zaki wrote:
>>>>>
>>>>>
>>>>> On 2023-10-29 06:48, Gal Pressman wrote:
>>>>>> On 29/10/2023 14:42, Ahmed Zaki wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 2023-10-29 06:25, Gal Pressman wrote:
>>>>>>>> On 21/10/2023 3:00, Ahmed Zaki wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 2023-10-20 17:49, Jakub Kicinski wrote:
>>>>>>>>>> On Fri, 20 Oct 2023 17:14:11 -0600 Ahmed Zaki wrote:
>>>>>>>>>>> I replied to that here:
>>>>>>>>>>>
>>>>>>>>>>> https://lore.kernel.org/all/afb4a06f-cfba-47ba-adb3-09bea7cb5f00@intel.com/
>>>>>>>>>>>
>>>>>>>>>>> I am kind of confused now so please bear with me. ethtool either
>>>>>>>>>>> sends
>>>>>>>>>>> "ethtool_rxfh" or "ethtool_rxnfc". AFAIK "ethtool_rxfh" is the
>>>>>>>>>>> interface
>>>>>>>>>>> for "ethtool -X" which is used to set the RSS algorithm. But we
>>>>>>>>>>> kind of
>>>>>>>>>>> agreed to go with "ethtool -U|-N" for symmetric-xor, and that
>>>>>>>>>>> uses
>>>>>>>>>>> "ethtool_rxnfc" (as implemented in this series).
>>>>>>>>>>
>>>>>>>>>> I have no strong preference. Sounds like Alex prefers to keep it
>>>>>>>>>> closer
>>>>>>>>>> to algo, which is "ethtool_rxfh".
>>>>>>>>>>
>>>>>>>>>>> Do you mean use "ethtool_rxfh" instead of "ethtool_rxnfc"? how
>>>>>>>>>>> would
>>>>>>>>>>> that work on the ethtool user interface?
>>>>>>>>>>
>>>>>>>>>> I don't know what you're asking of us. If you find the code to
>>>>>>>>>> confusing
>>>>>>>>>> maybe someone at Intel can help you :|
>>>>>>>>>
>>>>>>>>> The code is straightforward. I am confused by the requirements:
>>>>>>>>> don't
>>>>>>>>> add a new algorithm but use "ethtool_rxfh".
>>>>>>>>>
>>>>>>>>> I'll see if I can get more help, may be I am missing something.
>>>>>>>>>
>>>>>>>>
>>>>>>>> What was the decision here?
>>>>>>>> Is this going to be exposed through ethtool -N or -X?
>>>>>>>
>>>>>>> I am working on a new version that uses "ethtool_rxfh" to set the
>>>>>>> symmetric-xor. The user will set per-device via:
>>>>>>>
>>>>>>> ethtool -X eth0 hfunc toeplitz symmetric-xor
>>>>>>>
>>>>>>> then specify the per-flow type RSS fields as usual:
>>>>>>>
>>>>>>> ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n
>>>>>>>
>>>>>>> The downside is that all flow-types will have to be either
>>>>>>> symmetric or
>>>>>>> asymmetric.
>>>>>>
>>>>>> Why are we making the interface less flexible than it can be with -N?
>>>>>
>>>>> Alexander Duyck prefers to implement the "symmetric-xor" interface
>>>>> as an
>>>>> algorithm or extension (please refer to previous messages), but
>>>>> ethtool
>>>>> does not provide flowtype/RSS fields setting via "-X". The above
>>>>> was the
>>>>> best solution that we (at Intel) could think of.
>>>>
>>>> OK, it's a weird we're deliberately limiting our interface, given
>>>> there's already hardware that supports controlling symmetric hashing
>>>> per
>>>> flow type.
>>>>
>>>> I saw you mentioned the way ice hardware implements symmetric-xor
>>>> somewhere, it definitely needs to be added somewhere in our
>>>> documentation to prevent confusion.
>>>> mlx5 hardware also does symmetric hashing with xor, but not exactly as
>>>> you described, we need the algorithm to be clear.
>>>
>>> Sure. I will add more ice-specific doc in:
>>> Documentation/networking/device_drivers/ethernet/intel/ice.rst
>>
>> I was thinking of somewhere more generic, where ethtool users (not
>> necessarily ice users) can refer to.
>>
>> Perhaps Documentation/networking/ethtool-netlink.rst? Or ethtool man
>> page?
> 
> Do you mean add vendor-specific implementation details to common docs?
> Not sure if I have seen this before. Any examples?
> 
> Or, we can add a note in ethtool doc that each vendor's implementation
> is different and "Refer to your vendor's specifications for more info".

It's a generic ethtool flag, its documentation shouldn't be vendor specific.
The documentation should reflect the exact details about the algorithm,
then other vendors can either use it, or add a new symmetric flag and
document it separately.

Re: [Intel-wired-lan] [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Alexander Duyck]

On Tue, Oct 31, 2023 at 5:01 AM Gal Pressman <gal@nvidia.com> wrote:
>
> On 29/10/2023 18:59, Ahmed Zaki wrote:
> >
> >
> > On 2023-10-29 06:48, Gal Pressman wrote:
> >> On 29/10/2023 14:42, Ahmed Zaki wrote:
> >>>
> >>>
> >>> On 2023-10-29 06:25, Gal Pressman wrote:
> >>>> On 21/10/2023 3:00, Ahmed Zaki wrote:
> >>>>>
> >>>>>
> >>>>> On 2023-10-20 17:49, Jakub Kicinski wrote:
> >>>>>> On Fri, 20 Oct 2023 17:14:11 -0600 Ahmed Zaki wrote:
> >>>>>>> I replied to that here:
> >>>>>>>
> >>>>>>> https://lore.kernel.org/all/afb4a06f-cfba-47ba-adb3-09bea7cb5f00@intel.com/
> >>>>>>>
> >>>>>>> I am kind of confused now so please bear with me. ethtool either
> >>>>>>> sends
> >>>>>>> "ethtool_rxfh" or "ethtool_rxnfc". AFAIK "ethtool_rxfh" is the
> >>>>>>> interface
> >>>>>>> for "ethtool -X" which is used to set the RSS algorithm. But we
> >>>>>>> kind of
> >>>>>>> agreed to go with "ethtool -U|-N" for symmetric-xor, and that uses
> >>>>>>> "ethtool_rxnfc" (as implemented in this series).
> >>>>>>
> >>>>>> I have no strong preference. Sounds like Alex prefers to keep it
> >>>>>> closer
> >>>>>> to algo, which is "ethtool_rxfh".
> >>>>>>
> >>>>>>> Do you mean use "ethtool_rxfh" instead of "ethtool_rxnfc"? how would
> >>>>>>> that work on the ethtool user interface?
> >>>>>>
> >>>>>> I don't know what you're asking of us. If you find the code to
> >>>>>> confusing
> >>>>>> maybe someone at Intel can help you :|
> >>>>>
> >>>>> The code is straightforward. I am confused by the requirements: don't
> >>>>> add a new algorithm but use "ethtool_rxfh".
> >>>>>
> >>>>> I'll see if I can get more help, may be I am missing something.
> >>>>>
> >>>>
> >>>> What was the decision here?
> >>>> Is this going to be exposed through ethtool -N or -X?
> >>>
> >>> I am working on a new version that uses "ethtool_rxfh" to set the
> >>> symmetric-xor. The user will set per-device via:
> >>>
> >>> ethtool -X eth0 hfunc toeplitz symmetric-xor
> >>>
> >>> then specify the per-flow type RSS fields as usual:
> >>>
> >>> ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n
> >>>
> >>> The downside is that all flow-types will have to be either symmetric or
> >>> asymmetric.
> >>
> >> Why are we making the interface less flexible than it can be with -N?
> >
> > Alexander Duyck prefers to implement the "symmetric-xor" interface as an
> > algorithm or extension (please refer to previous messages), but ethtool
> > does not provide flowtype/RSS fields setting via "-X". The above was the
> > best solution that we (at Intel) could think of.
>
> OK, it's a weird we're deliberately limiting our interface, given
> there's already hardware that supports controlling symmetric hashing per
> flow type.
>
> I saw you mentioned the way ice hardware implements symmetric-xor
> somewhere, it definitely needs to be added somewhere in our
> documentation to prevent confusion.
> mlx5 hardware also does symmetric hashing with xor, but not exactly as
> you described, we need the algorithm to be clear.

It is precisely because of the way the symmetric-xor implements it
that I suggested that they change the algo type instead of the input
fields.

Instead of doing something such as rearranging the inputs, what they
do is start XORing them together and then using those values for both
the source and destination ports. It would be one thing if they
swapped them, but instead they destroy the entropy provided by XORing
the values together and then doubling them up in the source and
destination fields. The result is the hash value becomes predictable
in that once you know the target you just have to offset the source
and destination port/IP by the same amount so that they hash out to
the same values, and as a result it would make DDoS attacks based on
the RSS hash much easier.

Where I draw the line in this is if we start losing entropy without
explicitly removing the value then it is part of the algo, whereas if
it is something such as placement or us explicitly saying we don't
want certain fields in there then it would be part of the input.
Adding fields to the input should increase or at least maintain the
entropy is my point of view.

Re: [Intel-wired-lan] [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

[Gal Pressman]

On 31/10/2023 16:59, Alexander Duyck wrote:
> On Tue, Oct 31, 2023 at 5:01 AM Gal Pressman <gal@nvidia.com> wrote:
>>
>> On 29/10/2023 18:59, Ahmed Zaki wrote:
>>>
>>>
>>> On 2023-10-29 06:48, Gal Pressman wrote:
>>>> On 29/10/2023 14:42, Ahmed Zaki wrote:
>>>>>
>>>>>
>>>>> On 2023-10-29 06:25, Gal Pressman wrote:
>>>>>> On 21/10/2023 3:00, Ahmed Zaki wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 2023-10-20 17:49, Jakub Kicinski wrote:
>>>>>>>> On Fri, 20 Oct 2023 17:14:11 -0600 Ahmed Zaki wrote:
>>>>>>>>> I replied to that here:
>>>>>>>>>
>>>>>>>>> https://lore.kernel.org/all/afb4a06f-cfba-47ba-adb3-09bea7cb5f00@intel.com/
>>>>>>>>>
>>>>>>>>> I am kind of confused now so please bear with me. ethtool either
>>>>>>>>> sends
>>>>>>>>> "ethtool_rxfh" or "ethtool_rxnfc". AFAIK "ethtool_rxfh" is the
>>>>>>>>> interface
>>>>>>>>> for "ethtool -X" which is used to set the RSS algorithm. But we
>>>>>>>>> kind of
>>>>>>>>> agreed to go with "ethtool -U|-N" for symmetric-xor, and that uses
>>>>>>>>> "ethtool_rxnfc" (as implemented in this series).
>>>>>>>>
>>>>>>>> I have no strong preference. Sounds like Alex prefers to keep it
>>>>>>>> closer
>>>>>>>> to algo, which is "ethtool_rxfh".
>>>>>>>>
>>>>>>>>> Do you mean use "ethtool_rxfh" instead of "ethtool_rxnfc"? how would
>>>>>>>>> that work on the ethtool user interface?
>>>>>>>>
>>>>>>>> I don't know what you're asking of us. If you find the code to
>>>>>>>> confusing
>>>>>>>> maybe someone at Intel can help you :|
>>>>>>>
>>>>>>> The code is straightforward. I am confused by the requirements: don't
>>>>>>> add a new algorithm but use "ethtool_rxfh".
>>>>>>>
>>>>>>> I'll see if I can get more help, may be I am missing something.
>>>>>>>
>>>>>>
>>>>>> What was the decision here?
>>>>>> Is this going to be exposed through ethtool -N or -X?
>>>>>
>>>>> I am working on a new version that uses "ethtool_rxfh" to set the
>>>>> symmetric-xor. The user will set per-device via:
>>>>>
>>>>> ethtool -X eth0 hfunc toeplitz symmetric-xor
>>>>>
>>>>> then specify the per-flow type RSS fields as usual:
>>>>>
>>>>> ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n
>>>>>
>>>>> The downside is that all flow-types will have to be either symmetric or
>>>>> asymmetric.
>>>>
>>>> Why are we making the interface less flexible than it can be with -N?
>>>
>>> Alexander Duyck prefers to implement the "symmetric-xor" interface as an
>>> algorithm or extension (please refer to previous messages), but ethtool
>>> does not provide flowtype/RSS fields setting via "-X". The above was the
>>> best solution that we (at Intel) could think of.
>>
>> OK, it's a weird we're deliberately limiting our interface, given
>> there's already hardware that supports controlling symmetric hashing per
>> flow type.
>>
>> I saw you mentioned the way ice hardware implements symmetric-xor
>> somewhere, it definitely needs to be added somewhere in our
>> documentation to prevent confusion.
>> mlx5 hardware also does symmetric hashing with xor, but not exactly as
>> you described, we need the algorithm to be clear.
> 
> It is precisely because of the way the symmetric-xor implements it
> that I suggested that they change the algo type instead of the input
> fields.
> 
> Instead of doing something such as rearranging the inputs, what they
> do is start XORing them together and then using those values for both
> the source and destination ports. It would be one thing if they
> swapped them, but instead they destroy the entropy provided by XORing
> the values together and then doubling them up in the source and
> destination fields. The result is the hash value becomes predictable
> in that once you know the target you just have to offset the source
> and destination port/IP by the same amount so that they hash out to
> the same values, and as a result it would make DDoS attacks based on
> the RSS hash much easier.
> 
> Where I draw the line in this is if we start losing entropy without
> explicitly removing the value then it is part of the algo, whereas if
> it is something such as placement or us explicitly saying we don't
> want certain fields in there then it would be part of the input.
> Adding fields to the input should increase or at least maintain the
> entropy is my point of view.

Thanks for the detailed summary, that was helpful.

Though, if a vendor chooses to implement symmetric by sorting, we will
still have it as part of the algorithm, not input.

My main concern was about losing the ability to control symmetric per
flow-type, but I guess we can resolve that if the need arises.

[PATCH net-next v4 2/6] ice: fix ICE_AQ_VSI_Q_OPT_RSS_* register values

[Ahmed Zaki]

Fix the values of the ICE_AQ_VSI_Q_OPT_RSS_* registers. Shifting is
already done when the values are used, no need to double shift. Bug was
not discovered earlier since only ICE_AQ_VSI_Q_OPT_RSS_TPLZ (Zero) is
currently used.

Also, rename ICE_AQ_VSI_Q_OPT_RSS_XXX to ICE_AQ_VSI_Q_OPT_RSS_HASH_XXX
for consistency.

Co-developed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_adminq_cmd.h |  8 ++++----
 drivers/net/ethernet/intel/ice/ice_lib.c        |  4 ++--
 drivers/net/ethernet/intel/ice/ice_virtchnl.c   | 12 +++++-------
 3 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
index 51281b58ad72..56ec1897f4d8 100644
--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
@@ -491,10 +491,10 @@ struct ice_aqc_vsi_props {
 #define ICE_AQ_VSI_Q_OPT_RSS_GBL_LUT_M		(0xF << ICE_AQ_VSI_Q_OPT_RSS_GBL_LUT_S)
 #define ICE_AQ_VSI_Q_OPT_RSS_HASH_S		6
 #define ICE_AQ_VSI_Q_OPT_RSS_HASH_M		(0x3 << ICE_AQ_VSI_Q_OPT_RSS_HASH_S)
-#define ICE_AQ_VSI_Q_OPT_RSS_TPLZ		(0x0 << ICE_AQ_VSI_Q_OPT_RSS_HASH_S)
-#define ICE_AQ_VSI_Q_OPT_RSS_SYM_TPLZ		(0x1 << ICE_AQ_VSI_Q_OPT_RSS_HASH_S)
-#define ICE_AQ_VSI_Q_OPT_RSS_XOR		(0x2 << ICE_AQ_VSI_Q_OPT_RSS_HASH_S)
-#define ICE_AQ_VSI_Q_OPT_RSS_JHASH		(0x3 << ICE_AQ_VSI_Q_OPT_RSS_HASH_S)
+#define ICE_AQ_VSI_Q_OPT_RSS_HASH_TPLZ		0x0U
+#define ICE_AQ_VSI_Q_OPT_RSS_HASH_SYM_TPLZ	0x1U
+#define ICE_AQ_VSI_Q_OPT_RSS_HASH_XOR		0x2U
+#define ICE_AQ_VSI_Q_OPT_RSS_HASH_JHASH		0x3U
 	u8 q_opt_tc;
 #define ICE_AQ_VSI_Q_OPT_TC_OVR_S		0
 #define ICE_AQ_VSI_Q_OPT_TC_OVR_M		(0x1F << ICE_AQ_VSI_Q_OPT_TC_OVR_S)
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 1f45f0c3963d..64b6d9f7a46e 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -1186,12 +1186,12 @@ static void ice_set_rss_vsi_ctx(struct ice_vsi_ctx *ctxt, struct ice_vsi *vsi)
 	case ICE_VSI_PF:
 		/* PF VSI will inherit RSS instance of PF */
 		lut_type = ICE_AQ_VSI_Q_OPT_RSS_LUT_PF;
-		hash_type = ICE_AQ_VSI_Q_OPT_RSS_TPLZ;
+		hash_type = ICE_AQ_VSI_Q_OPT_RSS_HASH_TPLZ;
 		break;
 	case ICE_VSI_VF:
 		/* VF VSI will gets a small RSS table which is a VSI LUT type */
 		lut_type = ICE_AQ_VSI_Q_OPT_RSS_LUT_VSI;
-		hash_type = ICE_AQ_VSI_Q_OPT_RSS_TPLZ;
+		hash_type = ICE_AQ_VSI_Q_OPT_RSS_HASH_TPLZ;
 		break;
 	default:
 		dev_dbg(dev, "Unsupported VSI type %s\n",
diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.c b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
index 01e88b6e43a1..55bf7891981f 100644
--- a/drivers/net/ethernet/intel/ice/ice_virtchnl.c
+++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
@@ -823,8 +823,8 @@ static int ice_vc_handle_rss_cfg(struct ice_vf *vf, u8 *msg, bool add)
 		int status;
 
 		lut_type = ICE_AQ_VSI_Q_OPT_RSS_LUT_VSI;
-		hash_type = add ? ICE_AQ_VSI_Q_OPT_RSS_XOR :
-				ICE_AQ_VSI_Q_OPT_RSS_TPLZ;
+		hash_type = add ? ICE_AQ_VSI_Q_OPT_RSS_HASH_XOR :
+				ICE_AQ_VSI_Q_OPT_RSS_HASH_TPLZ;
 
 		ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
 		if (!ctx) {
@@ -832,11 +832,9 @@ static int ice_vc_handle_rss_cfg(struct ice_vf *vf, u8 *msg, bool add)
 			goto error_param;
 		}
 
-		ctx->info.q_opt_rss = ((lut_type <<
-					ICE_AQ_VSI_Q_OPT_RSS_LUT_S) &
-				       ICE_AQ_VSI_Q_OPT_RSS_LUT_M) |
-				       (hash_type &
-					ICE_AQ_VSI_Q_OPT_RSS_HASH_M);
+		ctx->info.q_opt_rss =
+			FIELD_PREP(ICE_AQ_VSI_Q_OPT_RSS_LUT_M, lut_type) |
+			FIELD_PREP(ICE_AQ_VSI_Q_OPT_RSS_HASH_M, hash_type);
 
 		/* Preserve existing queueing option setting */
 		ctx->info.q_opt_rss |= (vsi->info.q_opt_rss &
-- 
2.34.1

[PATCH net-next v4 3/6] ice: refactor RSS configuration

[Ahmed Zaki]

From: Qi Zhang <qi.z.zhang@intel.com>

Refactor the driver to use a communication data structure for RSS
config. To do so we introduce the new ice_rss_hash_cfg struct, and then
pass it as an argument to several functions.

Also introduce enum ice_rss_cfg_hdr_type to specify a more granular and
flexible RSS configuration:

ICE_RSS_OUTER_HEADERS - take outer layer as RSS input set
ICE_RSS_INNER_HEADERS - take inner layer as RSS input set
ICE_RSS_INNER_HEADERS_W_OUTER_IPV4 - take inner layer as RSS input set for
                                     packet with outer IPV4
ICE_RSS_INNER_HEADERS_W_OUTER_IPV6 - take inner layer as RSS input set for
                                     packet with outer IPV6
ICE_RSS_ANY_HEADERS - try with outer first then inner (same as the
		      behaviour without this change)

Finally, move the virtchnl_rss_algorithm enum to be with the other RSS
related structures in the virtchnl.h file.

There should be no functional change due to this patch.

Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
Co-developed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Co-developed-by: Ahmed Zaki <ahmed.zaki@intel.com>
Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_ethtool.c  |   6 +-
 drivers/net/ethernet/intel/ice/ice_flow.c     | 232 +++++++++++-------
 drivers/net/ethernet/intel/ice/ice_flow.h     |  33 ++-
 drivers/net/ethernet/intel/ice/ice_lib.c      | 100 +++-----
 drivers/net/ethernet/intel/ice/ice_virtchnl.c |  36 +--
 include/linux/avf/virtchnl.h                  |  16 +-
 6 files changed, 246 insertions(+), 177 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index d3cb08e66dcb..d30d1b2a819b 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -2508,6 +2508,7 @@ static int
 ice_set_rss_hash_opt(struct ice_vsi *vsi, struct ethtool_rxnfc *nfc)
 {
 	struct ice_pf *pf = vsi->back;
+	struct ice_rss_hash_cfg cfg;
 	struct device *dev;
 	u64 hashed_flds;
 	int status;
@@ -2534,7 +2535,10 @@ ice_set_rss_hash_opt(struct ice_vsi *vsi, struct ethtool_rxnfc *nfc)
 		return -EINVAL;
 	}
 
-	status = ice_add_rss_cfg(&pf->hw, vsi->idx, hashed_flds, hdrs);
+	cfg.hash_flds = hashed_flds;
+	cfg.addl_hdrs = hdrs;
+	cfg.hdr_type = ICE_RSS_ANY_HEADERS;
+	status = ice_add_rss_cfg(&pf->hw, vsi->idx, &cfg);
 	if (status) {
 		dev_dbg(dev, "ice_add_rss_cfg failed, vsi num = %d, error = %d\n",
 			vsi->vsi_num, status);
diff --git a/drivers/net/ethernet/intel/ice/ice_flow.c b/drivers/net/ethernet/intel/ice/ice_flow.c
index 85cca572c22a..ff76eaabfb69 100644
--- a/drivers/net/ethernet/intel/ice/ice_flow.c
+++ b/drivers/net/ethernet/intel/ice/ice_flow.c
@@ -1858,37 +1858,49 @@ int ice_flow_rem_vsi_prof(struct ice_hw *hw, u16 vsi_handle, u64 prof_id)
 /**
  * ice_flow_set_rss_seg_info - setup packet segments for RSS
  * @segs: pointer to the flow field segment(s)
- * @hash_fields: fields to be hashed on for the segment(s)
- * @flow_hdr: protocol header fields within a packet segment
+ * @seg_cnt: segment count
+ * @cfg: configure parameters
  *
  * Helper function to extract fields from hash bitmap and use flow
  * header value to set flow field segment for further use in flow
  * profile entry or removal.
  */
 static int
-ice_flow_set_rss_seg_info(struct ice_flow_seg_info *segs, u64 hash_fields,
-			  u32 flow_hdr)
+ice_flow_set_rss_seg_info(struct ice_flow_seg_info *segs, u8 seg_cnt,
+			  const struct ice_rss_hash_cfg *cfg)
 {
+	struct ice_flow_seg_info *seg;
 	u64 val;
-	u8 i;
+	u16 i;
+
+	/* set inner most segment */
+	seg = &segs[seg_cnt - 1];
 
-	for_each_set_bit(i, (unsigned long *)&hash_fields,
-			 ICE_FLOW_FIELD_IDX_MAX)
-		ice_flow_set_fld(segs, (enum ice_flow_field)i,
+	for_each_set_bit(i, (const unsigned long *)&cfg->hash_flds,
+			 (u16)ICE_FLOW_FIELD_IDX_MAX)
+		ice_flow_set_fld(seg, (enum ice_flow_field)i,
 				 ICE_FLOW_FLD_OFF_INVAL, ICE_FLOW_FLD_OFF_INVAL,
 				 ICE_FLOW_FLD_OFF_INVAL, false);
 
-	ICE_FLOW_SET_HDRS(segs, flow_hdr);
+	ICE_FLOW_SET_HDRS(seg, cfg->addl_hdrs);
 
-	if (segs->hdrs & ~ICE_FLOW_RSS_SEG_HDR_VAL_MASKS &
+	/* set outer most header */
+	if (cfg->hdr_type == ICE_RSS_INNER_HEADERS_W_OUTER_IPV4)
+		segs[ICE_RSS_OUTER_HEADERS].hdrs |= ICE_FLOW_SEG_HDR_IPV4 |
+						    ICE_FLOW_SEG_HDR_IPV_OTHER;
+	else if (cfg->hdr_type == ICE_RSS_INNER_HEADERS_W_OUTER_IPV6)
+		segs[ICE_RSS_OUTER_HEADERS].hdrs |= ICE_FLOW_SEG_HDR_IPV6 |
+						    ICE_FLOW_SEG_HDR_IPV_OTHER;
+
+	if (seg->hdrs & ~ICE_FLOW_RSS_SEG_HDR_VAL_MASKS &
 	    ~ICE_FLOW_RSS_HDRS_INNER_MASK & ~ICE_FLOW_SEG_HDR_IPV_OTHER)
 		return -EINVAL;
 
-	val = (u64)(segs->hdrs & ICE_FLOW_RSS_SEG_HDR_L3_MASKS);
+	val = (u64)(seg->hdrs & ICE_FLOW_RSS_SEG_HDR_L3_MASKS);
 	if (val && !is_power_of_2(val))
 		return -EIO;
 
-	val = (u64)(segs->hdrs & ICE_FLOW_RSS_SEG_HDR_L4_MASKS);
+	val = (u64)(seg->hdrs & ICE_FLOW_RSS_SEG_HDR_L4_MASKS);
 	if (val && !is_power_of_2(val))
 		return -EIO;
 
@@ -1958,6 +1970,39 @@ int ice_rem_vsi_rss_cfg(struct ice_hw *hw, u16 vsi_handle)
 	return status;
 }
 
+/**
+ * ice_get_rss_hdr_type - get a RSS profile's header type
+ * @prof: RSS flow profile
+ */
+static enum ice_rss_cfg_hdr_type
+ice_get_rss_hdr_type(struct ice_flow_prof *prof)
+{
+	if (prof->segs_cnt == ICE_FLOW_SEG_SINGLE) {
+		return ICE_RSS_OUTER_HEADERS;
+	} else if (prof->segs_cnt == ICE_FLOW_SEG_MAX) {
+		const struct ice_flow_seg_info *s;
+
+		s = &prof->segs[ICE_RSS_OUTER_HEADERS];
+		if (s->hdrs == ICE_FLOW_SEG_HDR_NONE)
+			return ICE_RSS_INNER_HEADERS;
+		if (s->hdrs & ICE_FLOW_SEG_HDR_IPV4)
+			return ICE_RSS_INNER_HEADERS_W_OUTER_IPV4;
+		if (s->hdrs & ICE_FLOW_SEG_HDR_IPV6)
+			return ICE_RSS_INNER_HEADERS_W_OUTER_IPV6;
+	}
+
+	return ICE_RSS_ANY_HEADERS;
+}
+
+static bool
+ice_rss_match_prof(struct ice_rss_cfg *r, struct ice_flow_prof *prof,
+		   enum ice_rss_cfg_hdr_type hdr_type)
+{
+	return (r->hash.hdr_type == hdr_type &&
+		r->hash.hash_flds == prof->segs[prof->segs_cnt - 1].match &&
+		r->hash.addl_hdrs == prof->segs[prof->segs_cnt - 1].hdrs);
+}
+
 /**
  * ice_rem_rss_list - remove RSS configuration from list
  * @hw: pointer to the hardware structure
@@ -1969,15 +2014,16 @@ int ice_rem_vsi_rss_cfg(struct ice_hw *hw, u16 vsi_handle)
 static void
 ice_rem_rss_list(struct ice_hw *hw, u16 vsi_handle, struct ice_flow_prof *prof)
 {
+	enum ice_rss_cfg_hdr_type hdr_type;
 	struct ice_rss_cfg *r, *tmp;
 
 	/* Search for RSS hash fields associated to the VSI that match the
 	 * hash configurations associated to the flow profile. If found
 	 * remove from the RSS entry list of the VSI context and delete entry.
 	 */
+	hdr_type = ice_get_rss_hdr_type(prof);
 	list_for_each_entry_safe(r, tmp, &hw->rss_list_head, l_entry)
-		if (r->hashed_flds == prof->segs[prof->segs_cnt - 1].match &&
-		    r->packet_hdr == prof->segs[prof->segs_cnt - 1].hdrs) {
+		if (ice_rss_match_prof(r, prof, hdr_type)) {
 			clear_bit(vsi_handle, r->vsis);
 			if (bitmap_empty(r->vsis, ICE_MAX_VSI)) {
 				list_del(&r->l_entry);
@@ -1998,11 +2044,12 @@ ice_rem_rss_list(struct ice_hw *hw, u16 vsi_handle, struct ice_flow_prof *prof)
 static int
 ice_add_rss_list(struct ice_hw *hw, u16 vsi_handle, struct ice_flow_prof *prof)
 {
+	enum ice_rss_cfg_hdr_type hdr_type;
 	struct ice_rss_cfg *r, *rss_cfg;
 
+	hdr_type = ice_get_rss_hdr_type(prof);
 	list_for_each_entry(r, &hw->rss_list_head, l_entry)
-		if (r->hashed_flds == prof->segs[prof->segs_cnt - 1].match &&
-		    r->packet_hdr == prof->segs[prof->segs_cnt - 1].hdrs) {
+		if (ice_rss_match_prof(r, prof, hdr_type)) {
 			set_bit(vsi_handle, r->vsis);
 			return 0;
 		}
@@ -2012,8 +2059,9 @@ ice_add_rss_list(struct ice_hw *hw, u16 vsi_handle, struct ice_flow_prof *prof)
 	if (!rss_cfg)
 		return -ENOMEM;
 
-	rss_cfg->hashed_flds = prof->segs[prof->segs_cnt - 1].match;
-	rss_cfg->packet_hdr = prof->segs[prof->segs_cnt - 1].hdrs;
+	rss_cfg->hash.hash_flds = prof->segs[prof->segs_cnt - 1].match;
+	rss_cfg->hash.addl_hdrs = prof->segs[prof->segs_cnt - 1].hdrs;
+	rss_cfg->hash.hdr_type = hdr_type;
 	set_bit(vsi_handle, rss_cfg->vsis);
 
 	list_add_tail(&rss_cfg->l_entry, &hw->rss_list_head);
@@ -2022,54 +2070,55 @@ ice_add_rss_list(struct ice_hw *hw, u16 vsi_handle, struct ice_flow_prof *prof)
 }
 
 #define ICE_FLOW_PROF_HASH_S	0
-#define ICE_FLOW_PROF_HASH_M	(0xFFFFFFFFULL << ICE_FLOW_PROF_HASH_S)
+#define ICE_FLOW_PROF_HASH_M	GENMASK_ULL(31, 0)
 #define ICE_FLOW_PROF_HDR_S	32
-#define ICE_FLOW_PROF_HDR_M	(0x3FFFFFFFULL << ICE_FLOW_PROF_HDR_S)
-#define ICE_FLOW_PROF_ENCAP_S	63
-#define ICE_FLOW_PROF_ENCAP_M	(BIT_ULL(ICE_FLOW_PROF_ENCAP_S))
-
-#define ICE_RSS_OUTER_HEADERS	1
-#define ICE_RSS_INNER_HEADERS	2
+#define ICE_FLOW_PROF_HDR_M	GENMASK_ULL(61, 32)
+#define ICE_FLOW_PROF_ENCAP_S	62
+#define ICE_FLOW_PROF_ENCAP_M	GENMASK_ULL(63, 62)
 
 /* Flow profile ID format:
  * [0:31] - Packet match fields
- * [32:62] - Protocol header
- * [63] - Encapsulation flag, 0 if non-tunneled, 1 if tunneled
+ * [32:61] - Protocol header
+ * [62:63] - Encapsulation flag:
+ *	     0 if non-tunneled
+ *	     1 if tunneled
+ *	     2 for tunneled with outer ipv4
+ *	     3 for tunneled with outer ipv6
  */
-#define ICE_FLOW_GEN_PROFID(hash, hdr, segs_cnt) \
-	((u64)(((u64)(hash) & ICE_FLOW_PROF_HASH_M) | \
+#define ICE_FLOW_GEN_PROFID(hash, hdr, encap)                                \
+	((u64)(((u64)(hash) & ICE_FLOW_PROF_HASH_M) |                        \
 	       (((u64)(hdr) << ICE_FLOW_PROF_HDR_S) & ICE_FLOW_PROF_HDR_M) | \
-	       ((u8)((segs_cnt) - 1) ? ICE_FLOW_PROF_ENCAP_M : 0)))
+	       (((u64)(encap) << ICE_FLOW_PROF_ENCAP_S) &                    \
+		ICE_FLOW_PROF_ENCAP_M)))
 
 /**
  * ice_add_rss_cfg_sync - add an RSS configuration
  * @hw: pointer to the hardware structure
  * @vsi_handle: software VSI handle
- * @hashed_flds: hash bit fields (ICE_FLOW_HASH_*) to configure
- * @addl_hdrs: protocol header fields
- * @segs_cnt: packet segment count
+ * @cfg: configure parameters
  *
  * Assumption: lock has already been acquired for RSS list
  */
 static int
-ice_add_rss_cfg_sync(struct ice_hw *hw, u16 vsi_handle, u64 hashed_flds,
-		     u32 addl_hdrs, u8 segs_cnt)
+ice_add_rss_cfg_sync(struct ice_hw *hw, u16 vsi_handle,
+		     const struct ice_rss_hash_cfg *cfg)
 {
 	const enum ice_block blk = ICE_BLK_RSS;
 	struct ice_flow_prof *prof = NULL;
 	struct ice_flow_seg_info *segs;
+	u8 segs_cnt;
 	int status;
 
-	if (!segs_cnt || segs_cnt > ICE_FLOW_SEG_MAX)
-		return -EINVAL;
+	segs_cnt = (cfg->hdr_type == ICE_RSS_OUTER_HEADERS) ?
+			   ICE_FLOW_SEG_SINGLE :
+			   ICE_FLOW_SEG_MAX;
 
 	segs = kcalloc(segs_cnt, sizeof(*segs), GFP_KERNEL);
 	if (!segs)
 		return -ENOMEM;
 
 	/* Construct the packet segment info from the hashed fields */
-	status = ice_flow_set_rss_seg_info(&segs[segs_cnt - 1], hashed_flds,
-					   addl_hdrs);
+	status = ice_flow_set_rss_seg_info(segs, segs_cnt, cfg);
 	if (status)
 		goto exit;
 
@@ -2123,9 +2172,9 @@ ice_add_rss_cfg_sync(struct ice_hw *hw, u16 vsi_handle, u64 hashed_flds,
 	 * segment information.
 	 */
 	status = ice_flow_add_prof(hw, blk, ICE_FLOW_RX,
-				   ICE_FLOW_GEN_PROFID(hashed_flds,
+				   ICE_FLOW_GEN_PROFID(cfg->hash_flds,
 						       segs[segs_cnt - 1].hdrs,
-						       segs_cnt),
+						       cfg->hdr_type),
 				   segs, segs_cnt, &prof);
 	if (status)
 		goto exit;
@@ -2150,29 +2199,37 @@ ice_add_rss_cfg_sync(struct ice_hw *hw, u16 vsi_handle, u64 hashed_flds,
  * ice_add_rss_cfg - add an RSS configuration with specified hashed fields
  * @hw: pointer to the hardware structure
  * @vsi_handle: software VSI handle
- * @hashed_flds: hash bit fields (ICE_FLOW_HASH_*) to configure
- * @addl_hdrs: protocol header fields
+ * @cfg: configure parameters
  *
  * This function will generate a flow profile based on fields associated with
  * the input fields to hash on, the flow type and use the VSI number to add
  * a flow entry to the profile.
  */
 int
-ice_add_rss_cfg(struct ice_hw *hw, u16 vsi_handle, u64 hashed_flds,
-		u32 addl_hdrs)
+ice_add_rss_cfg(struct ice_hw *hw, u16 vsi_handle,
+		const struct ice_rss_hash_cfg *cfg)
 {
+	struct ice_rss_hash_cfg local_cfg;
 	int status;
 
-	if (hashed_flds == ICE_HASH_INVALID ||
-	    !ice_is_vsi_valid(hw, vsi_handle))
+	if (!ice_is_vsi_valid(hw, vsi_handle) || !cfg ||
+	    cfg->hdr_type > ICE_RSS_ANY_HEADERS ||
+	    cfg->hash_flds == ICE_HASH_INVALID)
 		return -EINVAL;
 
 	mutex_lock(&hw->rss_locks);
-	status = ice_add_rss_cfg_sync(hw, vsi_handle, hashed_flds, addl_hdrs,
-				      ICE_RSS_OUTER_HEADERS);
-	if (!status)
-		status = ice_add_rss_cfg_sync(hw, vsi_handle, hashed_flds,
-					      addl_hdrs, ICE_RSS_INNER_HEADERS);
+	local_cfg = *cfg;
+	if (cfg->hdr_type < ICE_RSS_ANY_HEADERS) {
+		status = ice_add_rss_cfg_sync(hw, vsi_handle, &local_cfg);
+	} else {
+		local_cfg.hdr_type = ICE_RSS_OUTER_HEADERS;
+		status = ice_add_rss_cfg_sync(hw, vsi_handle, &local_cfg);
+		if (!status) {
+			local_cfg.hdr_type = ICE_RSS_INNER_HEADERS;
+			status = ice_add_rss_cfg_sync(hw, vsi_handle,
+						      &local_cfg);
+		}
+	}
 	mutex_unlock(&hw->rss_locks);
 
 	return status;
@@ -2182,28 +2239,29 @@ ice_add_rss_cfg(struct ice_hw *hw, u16 vsi_handle, u64 hashed_flds,
  * ice_rem_rss_cfg_sync - remove an existing RSS configuration
  * @hw: pointer to the hardware structure
  * @vsi_handle: software VSI handle
- * @hashed_flds: Packet hash types (ICE_FLOW_HASH_*) to remove
- * @addl_hdrs: Protocol header fields within a packet segment
- * @segs_cnt: packet segment count
+ * @cfg: configure parameters
  *
  * Assumption: lock has already been acquired for RSS list
  */
 static int
-ice_rem_rss_cfg_sync(struct ice_hw *hw, u16 vsi_handle, u64 hashed_flds,
-		     u32 addl_hdrs, u8 segs_cnt)
+ice_rem_rss_cfg_sync(struct ice_hw *hw, u16 vsi_handle,
+		     const struct ice_rss_hash_cfg *cfg)
 {
 	const enum ice_block blk = ICE_BLK_RSS;
 	struct ice_flow_seg_info *segs;
 	struct ice_flow_prof *prof;
+	u8 segs_cnt;
 	int status;
 
+	segs_cnt = (cfg->hdr_type == ICE_RSS_OUTER_HEADERS) ?
+			   ICE_FLOW_SEG_SINGLE :
+			   ICE_FLOW_SEG_MAX;
 	segs = kcalloc(segs_cnt, sizeof(*segs), GFP_KERNEL);
 	if (!segs)
 		return -ENOMEM;
 
 	/* Construct the packet segment info from the hashed fields */
-	status = ice_flow_set_rss_seg_info(&segs[segs_cnt - 1], hashed_flds,
-					   addl_hdrs);
+	status = ice_flow_set_rss_seg_info(segs, segs_cnt, cfg);
 	if (status)
 		goto out;
 
@@ -2236,31 +2294,39 @@ ice_rem_rss_cfg_sync(struct ice_hw *hw, u16 vsi_handle, u64 hashed_flds,
  * ice_rem_rss_cfg - remove an existing RSS config with matching hashed fields
  * @hw: pointer to the hardware structure
  * @vsi_handle: software VSI handle
- * @hashed_flds: Packet hash types (ICE_FLOW_HASH_*) to remove
- * @addl_hdrs: Protocol header fields within a packet segment
+ * @cfg: configure parameters
  *
  * This function will lookup the flow profile based on the input
  * hash field bitmap, iterate through the profile entry list of
  * that profile and find entry associated with input VSI to be
- * removed. Calls are made to underlying flow s which will APIs
+ * removed. Calls are made to underlying flow apis which will in
  * turn build or update buffers for RSS XLT1 section.
  */
-int __maybe_unused
-ice_rem_rss_cfg(struct ice_hw *hw, u16 vsi_handle, u64 hashed_flds,
-		u32 addl_hdrs)
+int
+ice_rem_rss_cfg(struct ice_hw *hw, u16 vsi_handle,
+		const struct ice_rss_hash_cfg *cfg)
 {
+	struct ice_rss_hash_cfg local_cfg;
 	int status;
 
-	if (hashed_flds == ICE_HASH_INVALID ||
-	    !ice_is_vsi_valid(hw, vsi_handle))
+	if (!ice_is_vsi_valid(hw, vsi_handle) || !cfg ||
+	    cfg->hdr_type > ICE_RSS_ANY_HEADERS ||
+	    cfg->hash_flds == ICE_HASH_INVALID)
 		return -EINVAL;
 
 	mutex_lock(&hw->rss_locks);
-	status = ice_rem_rss_cfg_sync(hw, vsi_handle, hashed_flds, addl_hdrs,
-				      ICE_RSS_OUTER_HEADERS);
-	if (!status)
-		status = ice_rem_rss_cfg_sync(hw, vsi_handle, hashed_flds,
-					      addl_hdrs, ICE_RSS_INNER_HEADERS);
+	local_cfg = *cfg;
+	if (cfg->hdr_type < ICE_RSS_ANY_HEADERS) {
+		status = ice_rem_rss_cfg_sync(hw, vsi_handle, &local_cfg);
+	} else {
+		local_cfg.hdr_type = ICE_RSS_OUTER_HEADERS;
+		status = ice_rem_rss_cfg_sync(hw, vsi_handle, &local_cfg);
+		if (!status) {
+			local_cfg.hdr_type = ICE_RSS_INNER_HEADERS;
+			status = ice_rem_rss_cfg_sync(hw, vsi_handle,
+						      &local_cfg);
+		}
+	}
 	mutex_unlock(&hw->rss_locks);
 
 	return status;
@@ -2310,6 +2376,7 @@ ice_rem_rss_cfg(struct ice_hw *hw, u16 vsi_handle, u64 hashed_flds,
  */
 int ice_add_avf_rss_cfg(struct ice_hw *hw, u16 vsi_handle, u64 avf_hash)
 {
+	struct ice_rss_hash_cfg hcfg;
 	int status = 0;
 	u64 hash_flds;
 
@@ -2382,8 +2449,10 @@ int ice_add_avf_rss_cfg(struct ice_hw *hw, u16 vsi_handle, u64 avf_hash)
 		if (rss_hash == ICE_HASH_INVALID)
 			return -EIO;
 
-		status = ice_add_rss_cfg(hw, vsi_handle, rss_hash,
-					 ICE_FLOW_SEG_HDR_NONE);
+		hcfg.addl_hdrs = ICE_FLOW_SEG_HDR_NONE;
+		hcfg.hash_flds = rss_hash;
+		hcfg.hdr_type = ICE_RSS_ANY_HEADERS;
+		status = ice_add_rss_cfg(hw, vsi_handle, &hcfg);
 		if (status)
 			break;
 	}
@@ -2407,16 +2476,7 @@ int ice_replay_rss_cfg(struct ice_hw *hw, u16 vsi_handle)
 	mutex_lock(&hw->rss_locks);
 	list_for_each_entry(r, &hw->rss_list_head, l_entry) {
 		if (test_bit(vsi_handle, r->vsis)) {
-			status = ice_add_rss_cfg_sync(hw, vsi_handle,
-						      r->hashed_flds,
-						      r->packet_hdr,
-						      ICE_RSS_OUTER_HEADERS);
-			if (status)
-				break;
-			status = ice_add_rss_cfg_sync(hw, vsi_handle,
-						      r->hashed_flds,
-						      r->packet_hdr,
-						      ICE_RSS_INNER_HEADERS);
+			status = ice_add_rss_cfg_sync(hw, vsi_handle, &r->hash);
 			if (status)
 				break;
 		}
@@ -2447,8 +2507,8 @@ u64 ice_get_rss_cfg(struct ice_hw *hw, u16 vsi_handle, u32 hdrs)
 	mutex_lock(&hw->rss_locks);
 	list_for_each_entry(r, &hw->rss_list_head, l_entry)
 		if (test_bit(vsi_handle, r->vsis) &&
-		    r->packet_hdr == hdrs) {
-			rss_hash = r->hashed_flds;
+		    r->hash.addl_hdrs == hdrs) {
+			rss_hash = r->hash.hash_flds;
 			break;
 		}
 	mutex_unlock(&hw->rss_locks);
diff --git a/drivers/net/ethernet/intel/ice/ice_flow.h b/drivers/net/ethernet/intel/ice/ice_flow.h
index b465d27d9b80..7a1db1df760e 100644
--- a/drivers/net/ethernet/intel/ice/ice_flow.h
+++ b/drivers/net/ethernet/intel/ice/ice_flow.h
@@ -34,6 +34,8 @@
 #define ICE_HASH_TCP_IPV6	(ICE_FLOW_HASH_IPV6 | ICE_FLOW_HASH_TCP_PORT)
 #define ICE_HASH_UDP_IPV4	(ICE_FLOW_HASH_IPV4 | ICE_FLOW_HASH_UDP_PORT)
 #define ICE_HASH_UDP_IPV6	(ICE_FLOW_HASH_IPV6 | ICE_FLOW_HASH_UDP_PORT)
+#define ICE_HASH_SCTP_IPV4	(ICE_FLOW_HASH_IPV4 | ICE_FLOW_HASH_SCTP_PORT)
+#define ICE_HASH_SCTP_IPV6	(ICE_FLOW_HASH_IPV6 | ICE_FLOW_HASH_SCTP_PORT)
 
 #define ICE_FLOW_HASH_GTP_TEID \
 	(BIT_ULL(ICE_FLOW_FIELD_IDX_GTPC_TEID))
@@ -279,6 +281,23 @@ enum ice_flow_avf_hdr_field {
 	BIT_ULL(ICE_AVF_FLOW_FIELD_UNICAST_IPV6_UDP) | \
 	BIT_ULL(ICE_AVF_FLOW_FIELD_MULTICAST_IPV6_UDP))
 
+enum ice_rss_cfg_hdr_type {
+	ICE_RSS_OUTER_HEADERS, /* take outer headers as inputset. */
+	ICE_RSS_INNER_HEADERS, /* take inner headers as inputset. */
+	/* take inner headers as inputset for packet with outer ipv4. */
+	ICE_RSS_INNER_HEADERS_W_OUTER_IPV4,
+	/* take inner headers as inputset for packet with outer ipv6. */
+	ICE_RSS_INNER_HEADERS_W_OUTER_IPV6,
+	/* take outer headers first then inner headers as inputset */
+	ICE_RSS_ANY_HEADERS
+};
+
+struct ice_rss_hash_cfg {
+	u32 addl_hdrs; /* protocol header fields */
+	u64 hash_flds; /* hash bit field (ICE_FLOW_HASH_*) to configure */
+	enum ice_rss_cfg_hdr_type hdr_type; /* to specify inner or outer */
+};
+
 enum ice_flow_dir {
 	ICE_FLOW_RX		= 0x02,
 };
@@ -289,6 +308,7 @@ enum ice_flow_priority {
 	ICE_FLOW_PRIO_HIGH
 };
 
+#define ICE_FLOW_SEG_SINGLE		1
 #define ICE_FLOW_SEG_MAX		2
 #define ICE_FLOW_SEG_RAW_FLD_MAX	2
 #define ICE_FLOW_FV_EXTRACT_SZ		2
@@ -381,8 +401,7 @@ struct ice_rss_cfg {
 	struct list_head l_entry;
 	/* bitmap of VSIs added to the RSS entry */
 	DECLARE_BITMAP(vsis, ICE_MAX_VSI);
-	u64 hashed_flds;
-	u32 packet_hdr;
+	struct ice_rss_hash_cfg hash;
 };
 
 int
@@ -406,11 +425,9 @@ void ice_rem_vsi_rss_list(struct ice_hw *hw, u16 vsi_handle);
 int ice_replay_rss_cfg(struct ice_hw *hw, u16 vsi_handle);
 int ice_add_avf_rss_cfg(struct ice_hw *hw, u16 vsi_handle, u64 hashed_flds);
 int ice_rem_vsi_rss_cfg(struct ice_hw *hw, u16 vsi_handle);
-int
-ice_add_rss_cfg(struct ice_hw *hw, u16 vsi_handle, u64 hashed_flds,
-		u32 addl_hdrs);
-int
-ice_rem_rss_cfg(struct ice_hw *hw, u16 vsi_handle, u64 hashed_flds,
-		u32 addl_hdrs);
+int ice_add_rss_cfg(struct ice_hw *hw, u16 vsi_handle,
+		    const struct ice_rss_hash_cfg *cfg);
+int ice_rem_rss_cfg(struct ice_hw *hw, u16 vsi_handle,
+		    const struct ice_rss_hash_cfg *cfg);
 u64 ice_get_rss_cfg(struct ice_hw *hw, u16 vsi_handle, u32 hdrs);
 #endif /* _ICE_FLOW_H_ */
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 64b6d9f7a46e..f7a22e0bb505 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -1607,6 +1607,38 @@ static void ice_vsi_set_vf_rss_flow_fld(struct ice_vsi *vsi)
 			vsi->vsi_num, status);
 }
 
+static const struct ice_rss_hash_cfg default_rss_cfgs[] = {
+	/* configure RSS for IPv4 with input set IP src/dst */
+	{ICE_FLOW_SEG_HDR_IPV4, ICE_FLOW_HASH_IPV4, ICE_RSS_ANY_HEADERS},
+	/* configure RSS for IPv6 with input set IPv6 src/dst */
+	{ICE_FLOW_SEG_HDR_IPV6, ICE_FLOW_HASH_IPV6, ICE_RSS_ANY_HEADERS},
+	/* configure RSS for tcp4 with input set IP src/dst, TCP src/dst */
+	{ICE_FLOW_SEG_HDR_TCP | ICE_FLOW_SEG_HDR_IPV4,
+				ICE_HASH_TCP_IPV4,  ICE_RSS_ANY_HEADERS},
+	/* configure RSS for udp4 with input set IP src/dst, UDP src/dst */
+	{ICE_FLOW_SEG_HDR_UDP | ICE_FLOW_SEG_HDR_IPV4,
+				ICE_HASH_UDP_IPV4,  ICE_RSS_ANY_HEADERS},
+	/* configure RSS for sctp4 with input set IP src/dst - only support
+	 * RSS on SCTPv4 on outer headers (non-tunneled)
+	 */
+	{ICE_FLOW_SEG_HDR_SCTP | ICE_FLOW_SEG_HDR_IPV4,
+		ICE_HASH_SCTP_IPV4, ICE_RSS_OUTER_HEADERS},
+	/* configure RSS for tcp6 with input set IPv6 src/dst, TCP src/dst */
+	{ICE_FLOW_SEG_HDR_TCP | ICE_FLOW_SEG_HDR_IPV6,
+				ICE_HASH_TCP_IPV6,  ICE_RSS_ANY_HEADERS},
+	/* configure RSS for udp6 with input set IPv6 src/dst, UDP src/dst */
+	{ICE_FLOW_SEG_HDR_UDP | ICE_FLOW_SEG_HDR_IPV6,
+				ICE_HASH_UDP_IPV6,  ICE_RSS_ANY_HEADERS},
+	/* configure RSS for sctp6 with input set IPv6 src/dst - only support
+	 * RSS on SCTPv6 on outer headers (non-tunneled)
+	 */
+	{ICE_FLOW_SEG_HDR_SCTP | ICE_FLOW_SEG_HDR_IPV6,
+		ICE_HASH_SCTP_IPV6, ICE_RSS_OUTER_HEADERS},
+	/* configure RSS for IPSEC ESP SPI with input set MAC_IPV4_SPI */
+	{ICE_FLOW_SEG_HDR_ESP,
+		ICE_FLOW_HASH_ESP_SPI, ICE_RSS_OUTER_HEADERS},
+};
+
 /**
  * ice_vsi_set_rss_flow_fld - Sets RSS input set for different flows
  * @vsi: VSI to be configured
@@ -1625,6 +1657,7 @@ static void ice_vsi_set_rss_flow_fld(struct ice_vsi *vsi)
 	struct ice_hw *hw = &pf->hw;
 	struct device *dev;
 	int status;
+	u32 i;
 
 	dev = ice_pf_to_dev(pf);
 	if (ice_is_safe_mode(pf)) {
@@ -1632,67 +1665,14 @@ static void ice_vsi_set_rss_flow_fld(struct ice_vsi *vsi)
 			vsi_num);
 		return;
 	}
-	/* configure RSS for IPv4 with input set IP src/dst */
-	status = ice_add_rss_cfg(hw, vsi_handle, ICE_FLOW_HASH_IPV4,
-				 ICE_FLOW_SEG_HDR_IPV4);
-	if (status)
-		dev_dbg(dev, "ice_add_rss_cfg failed for ipv4 flow, vsi = %d, error = %d\n",
-			vsi_num, status);
-
-	/* configure RSS for IPv6 with input set IPv6 src/dst */
-	status = ice_add_rss_cfg(hw, vsi_handle, ICE_FLOW_HASH_IPV6,
-				 ICE_FLOW_SEG_HDR_IPV6);
-	if (status)
-		dev_dbg(dev, "ice_add_rss_cfg failed for ipv6 flow, vsi = %d, error = %d\n",
-			vsi_num, status);
-
-	/* configure RSS for tcp4 with input set IP src/dst, TCP src/dst */
-	status = ice_add_rss_cfg(hw, vsi_handle, ICE_HASH_TCP_IPV4,
-				 ICE_FLOW_SEG_HDR_TCP | ICE_FLOW_SEG_HDR_IPV4);
-	if (status)
-		dev_dbg(dev, "ice_add_rss_cfg failed for tcp4 flow, vsi = %d, error = %d\n",
-			vsi_num, status);
+	for (i = 0; i < ARRAY_SIZE(default_rss_cfgs); i++) {
+		const struct ice_rss_hash_cfg *cfg = &default_rss_cfgs[i];
 
-	/* configure RSS for udp4 with input set IP src/dst, UDP src/dst */
-	status = ice_add_rss_cfg(hw, vsi_handle, ICE_HASH_UDP_IPV4,
-				 ICE_FLOW_SEG_HDR_UDP | ICE_FLOW_SEG_HDR_IPV4);
-	if (status)
-		dev_dbg(dev, "ice_add_rss_cfg failed for udp4 flow, vsi = %d, error = %d\n",
-			vsi_num, status);
-
-	/* configure RSS for sctp4 with input set IP src/dst */
-	status = ice_add_rss_cfg(hw, vsi_handle, ICE_FLOW_HASH_IPV4,
-				 ICE_FLOW_SEG_HDR_SCTP | ICE_FLOW_SEG_HDR_IPV4);
-	if (status)
-		dev_dbg(dev, "ice_add_rss_cfg failed for sctp4 flow, vsi = %d, error = %d\n",
-			vsi_num, status);
-
-	/* configure RSS for tcp6 with input set IPv6 src/dst, TCP src/dst */
-	status = ice_add_rss_cfg(hw, vsi_handle, ICE_HASH_TCP_IPV6,
-				 ICE_FLOW_SEG_HDR_TCP | ICE_FLOW_SEG_HDR_IPV6);
-	if (status)
-		dev_dbg(dev, "ice_add_rss_cfg failed for tcp6 flow, vsi = %d, error = %d\n",
-			vsi_num, status);
-
-	/* configure RSS for udp6 with input set IPv6 src/dst, UDP src/dst */
-	status = ice_add_rss_cfg(hw, vsi_handle, ICE_HASH_UDP_IPV6,
-				 ICE_FLOW_SEG_HDR_UDP | ICE_FLOW_SEG_HDR_IPV6);
-	if (status)
-		dev_dbg(dev, "ice_add_rss_cfg failed for udp6 flow, vsi = %d, error = %d\n",
-			vsi_num, status);
-
-	/* configure RSS for sctp6 with input set IPv6 src/dst */
-	status = ice_add_rss_cfg(hw, vsi_handle, ICE_FLOW_HASH_IPV6,
-				 ICE_FLOW_SEG_HDR_SCTP | ICE_FLOW_SEG_HDR_IPV6);
-	if (status)
-		dev_dbg(dev, "ice_add_rss_cfg failed for sctp6 flow, vsi = %d, error = %d\n",
-			vsi_num, status);
-
-	status = ice_add_rss_cfg(hw, vsi_handle, ICE_FLOW_HASH_ESP_SPI,
-				 ICE_FLOW_SEG_HDR_ESP);
-	if (status)
-		dev_dbg(dev, "ice_add_rss_cfg failed for esp/spi flow, vsi = %d, error = %d\n",
-			vsi_num, status);
+		status = ice_add_rss_cfg(hw, vsi_handle, cfg);
+		if (status)
+			dev_dbg(dev, "ice_add_rss_cfg failed, addl_hdrs = %x, hash_flds = %llx, hdr_type = %d\n",
+				cfg->addl_hdrs, cfg->hash_flds, cfg->hdr_type);
+	}
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.c b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
index 55bf7891981f..f14aae2fe06c 100644
--- a/drivers/net/ethernet/intel/ice/ice_virtchnl.c
+++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
@@ -689,9 +689,7 @@ ice_vc_validate_pattern(struct ice_vf *vf, struct virtchnl_proto_hdrs *proto)
  * a specific virtchnl RSS cfg
  * @hw: pointer to the hardware
  * @rss_cfg: pointer to the virtchnl RSS cfg
- * @addl_hdrs: pointer to the protocol header fields (ICE_FLOW_SEG_HDR_*)
- * to configure
- * @hash_flds: pointer to the hash bit fields (ICE_FLOW_HASH_*) to configure
+ * @hash_cfg: pointer to the HW hash configuration
  *
  * Return true if all the protocol header and hash fields in the RSS cfg could
  * be parsed, else return false
@@ -699,13 +697,18 @@ ice_vc_validate_pattern(struct ice_vf *vf, struct virtchnl_proto_hdrs *proto)
  * This function parses the virtchnl RSS cfg to be the intended
  * hash fields and the intended header for RSS configuration
  */
-static bool
-ice_vc_parse_rss_cfg(struct ice_hw *hw, struct virtchnl_rss_cfg *rss_cfg,
-		     u32 *addl_hdrs, u64 *hash_flds)
+static bool ice_vc_parse_rss_cfg(struct ice_hw *hw,
+				 struct virtchnl_rss_cfg *rss_cfg,
+				 struct ice_rss_hash_cfg *hash_cfg)
 {
 	const struct ice_vc_hash_field_match_type *hf_list;
 	const struct ice_vc_hdr_match_type *hdr_list;
 	int i, hf_list_len, hdr_list_len;
+	u32 *addl_hdrs = &hash_cfg->addl_hdrs;
+	u64 *hash_flds = &hash_cfg->hash_flds;
+
+	/* set outer layer RSS as default */
+	hash_cfg->hdr_type = ICE_RSS_OUTER_HEADERS;
 
 	hf_list = ice_vc_hash_field_list;
 	hf_list_len = ARRAY_SIZE(ice_vc_hash_field_list);
@@ -856,18 +859,24 @@ static int ice_vc_handle_rss_cfg(struct ice_vf *vf, u8 *msg, bool add)
 
 		kfree(ctx);
 	} else {
-		u32 addl_hdrs = ICE_FLOW_SEG_HDR_NONE;
-		u64 hash_flds = ICE_HASH_INVALID;
+		struct ice_rss_hash_cfg cfg;
+
+		/* Only check for none raw pattern case */
+		if (!ice_vc_validate_pattern(vf, &rss_cfg->proto_hdrs)) {
+			v_ret = VIRTCHNL_STATUS_ERR_PARAM;
+			goto error_param;
+		}
+		cfg.addl_hdrs = ICE_FLOW_SEG_HDR_NONE;
+		cfg.hash_flds = ICE_HASH_INVALID;
+		cfg.hdr_type = ICE_RSS_ANY_HEADERS;
 
-		if (!ice_vc_parse_rss_cfg(hw, rss_cfg, &addl_hdrs,
-					  &hash_flds)) {
+		if (!ice_vc_parse_rss_cfg(hw, rss_cfg, &cfg)) {
 			v_ret = VIRTCHNL_STATUS_ERR_PARAM;
 			goto error_param;
 		}
 
 		if (add) {
-			if (ice_add_rss_cfg(hw, vsi->idx, hash_flds,
-					    addl_hdrs)) {
+			if (ice_add_rss_cfg(hw, vsi->idx, &cfg)) {
 				v_ret = VIRTCHNL_STATUS_ERR_PARAM;
 				dev_err(dev, "ice_add_rss_cfg failed for vsi = %d, v_ret = %d\n",
 					vsi->vsi_num, v_ret);
@@ -875,8 +884,7 @@ static int ice_vc_handle_rss_cfg(struct ice_vf *vf, u8 *msg, bool add)
 		} else {
 			int status;
 
-			status = ice_rem_rss_cfg(hw, vsi->idx, hash_flds,
-						 addl_hdrs);
+			status = ice_rem_rss_cfg(hw, vsi->idx, &cfg);
 			/* We just ignore -ENOENT, because if two configurations
 			 * share the same profile remove one of them actually
 			 * removes both, since the profile is deleted.
diff --git a/include/linux/avf/virtchnl.h b/include/linux/avf/virtchnl.h
index 6b3acf15be5c..b0e060cc79ac 100644
--- a/include/linux/avf/virtchnl.h
+++ b/include/linux/avf/virtchnl.h
@@ -911,6 +911,14 @@ struct virtchnl_rss_hena {
 
 VIRTCHNL_CHECK_STRUCT_LEN(8, virtchnl_rss_hena);
 
+/* Type of RSS algorithm */
+enum virtchnl_rss_algorithm {
+	VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC	= 0,
+	VIRTCHNL_RSS_ALG_R_ASYMMETRIC		= 1,
+	VIRTCHNL_RSS_ALG_TOEPLITZ_SYMMETRIC	= 2,
+	VIRTCHNL_RSS_ALG_XOR_SYMMETRIC		= 3,
+};
+
 /* VIRTCHNL_OP_ENABLE_CHANNELS
  * VIRTCHNL_OP_DISABLE_CHANNELS
  * VF sends these messages to enable or disable channels based on
@@ -1095,14 +1103,6 @@ enum virtchnl_vfr_states {
 	VIRTCHNL_VFR_VFACTIVE,
 };
 
-/* Type of RSS algorithm */
-enum virtchnl_rss_algorithm {
-	VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC	= 0,
-	VIRTCHNL_RSS_ALG_R_ASYMMETRIC		= 1,
-	VIRTCHNL_RSS_ALG_TOEPLITZ_SYMMETRIC	= 2,
-	VIRTCHNL_RSS_ALG_XOR_SYMMETRIC		= 3,
-};
-
 #define VIRTCHNL_MAX_NUM_PROTO_HDRS	32
 #define PROTO_HDR_SHIFT			5
 #define PROTO_HDR_FIELD_START(proto_hdr_type) ((proto_hdr_type) << PROTO_HDR_SHIFT)
-- 
2.34.1

[PATCH net-next v4 4/6] ice: refactor the FD and RSS flow ID generation

[Ahmed Zaki]

The flow director and RSS blocks use separate methods to generate a
unique 64 bit ID for the flow. This is not extendable, especially for
the RSS that already uses all 64 bit space.

Refactor the flow generation API so that the ID is generated within
ice_flow_add_prof(). The FD and RSS blocks caches the generated ID for
later use.

Suggested-by: Dan Nowlin <dan.nowlin@intel.com>
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com>
---
 .../net/ethernet/intel/ice/ice_ethtool_fdir.c | 31 +++++-------
 .../net/ethernet/intel/ice/ice_flex_pipe.c    | 10 ++++
 .../net/ethernet/intel/ice/ice_flex_type.h    |  6 +++
 drivers/net/ethernet/intel/ice/ice_flow.c     | 47 +++++--------------
 drivers/net/ethernet/intel/ice/ice_flow.h     |  2 +-
 drivers/net/ethernet/intel/ice/ice_main.c     |  5 +-
 drivers/net/ethernet/intel/ice/ice_type.h     |  1 +
 .../ethernet/intel/ice/ice_virtchnl_fdir.c    | 33 ++++---------
 8 files changed, 54 insertions(+), 81 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c b/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c
index 8c6e13f87b7d..c9c72f8dbcb2 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c
@@ -302,9 +302,7 @@ void ice_fdir_rem_adq_chnl(struct ice_hw *hw, u16 vsi_idx)
 			continue;
 
 		for (tun = 0; tun < ICE_FD_HW_SEG_MAX; tun++) {
-			u64 prof_id;
-
-			prof_id = flow + tun * ICE_FLTR_PTYPE_MAX;
+			u64 prof_id = prof->prof_id[tun];
 
 			for (i = 0; i < prof->cnt; i++) {
 				if (prof->vsi_h[i] != vsi_idx)
@@ -362,10 +360,9 @@ ice_fdir_erase_flow_from_hw(struct ice_hw *hw, enum ice_block blk, int flow)
 		return;
 
 	for (tun = 0; tun < ICE_FD_HW_SEG_MAX; tun++) {
-		u64 prof_id;
+		u64 prof_id = prof->prof_id[tun];
 		int j;
 
-		prof_id = flow + tun * ICE_FLTR_PTYPE_MAX;
 		for (j = 0; j < prof->cnt; j++) {
 			u16 vsi_num;
 
@@ -439,12 +436,10 @@ void ice_fdir_replay_flows(struct ice_hw *hw)
 		for (tun = 0; tun < ICE_FD_HW_SEG_MAX; tun++) {
 			struct ice_flow_prof *hw_prof;
 			struct ice_fd_hw_prof *prof;
-			u64 prof_id;
 			int j;
 
 			prof = hw->fdir_prof[flow];
-			prof_id = flow + tun * ICE_FLTR_PTYPE_MAX;
-			ice_flow_add_prof(hw, ICE_BLK_FD, ICE_FLOW_RX, prof_id,
+			ice_flow_add_prof(hw, ICE_BLK_FD, ICE_FLOW_RX,
 					  prof->fdir_seg[tun], TNL_SEG_CNT(tun),
 					  &hw_prof);
 			for (j = 0; j < prof->cnt; j++) {
@@ -454,7 +449,7 @@ void ice_fdir_replay_flows(struct ice_hw *hw)
 
 				prio = ICE_FLOW_PRIO_NORMAL;
 				err = ice_flow_add_entry(hw, ICE_BLK_FD,
-							 prof_id,
+							 hw_prof->id,
 							 prof->vsi_h[0],
 							 prof->vsi_h[j],
 							 prio, prof->fdir_seg,
@@ -464,6 +459,7 @@ void ice_fdir_replay_flows(struct ice_hw *hw)
 						flow);
 					continue;
 				}
+				prof->prof_id[tun] = hw_prof->id;
 				prof->entry_h[j][tun] = entry_h;
 			}
 		}
@@ -630,7 +626,6 @@ ice_fdir_set_hw_fltr_rule(struct ice_pf *pf, struct ice_flow_seg_info *seg,
 	u64 entry1_h = 0;
 	u64 entry2_h = 0;
 	bool del_last;
-	u64 prof_id;
 	int err;
 	int idx;
 
@@ -678,23 +673,23 @@ ice_fdir_set_hw_fltr_rule(struct ice_pf *pf, struct ice_flow_seg_info *seg,
 	 * That is the final parameters are 1 header (segment), no
 	 * actions (NULL) and zero actions 0.
 	 */
-	prof_id = flow + tun * ICE_FLTR_PTYPE_MAX;
-	err = ice_flow_add_prof(hw, ICE_BLK_FD, ICE_FLOW_RX, prof_id, seg,
+	err = ice_flow_add_prof(hw, ICE_BLK_FD, ICE_FLOW_RX, seg,
 				TNL_SEG_CNT(tun), &prof);
 	if (err)
 		return err;
-	err = ice_flow_add_entry(hw, ICE_BLK_FD, prof_id, main_vsi->idx,
+	err = ice_flow_add_entry(hw, ICE_BLK_FD, prof->id, main_vsi->idx,
 				 main_vsi->idx, ICE_FLOW_PRIO_NORMAL,
 				 seg, &entry1_h);
 	if (err)
 		goto err_prof;
-	err = ice_flow_add_entry(hw, ICE_BLK_FD, prof_id, main_vsi->idx,
+	err = ice_flow_add_entry(hw, ICE_BLK_FD, prof->id, main_vsi->idx,
 				 ctrl_vsi->idx, ICE_FLOW_PRIO_NORMAL,
 				 seg, &entry2_h);
 	if (err)
 		goto err_entry;
 
 	hw_prof->fdir_seg[tun] = seg;
+	hw_prof->prof_id[tun] = prof->id;
 	hw_prof->entry_h[0][tun] = entry1_h;
 	hw_prof->entry_h[1][tun] = entry2_h;
 	hw_prof->vsi_h[0] = main_vsi->idx;
@@ -711,7 +706,7 @@ ice_fdir_set_hw_fltr_rule(struct ice_pf *pf, struct ice_flow_seg_info *seg,
 
 		entry1_h = 0;
 		vsi_h = main_vsi->tc_map_vsi[idx]->idx;
-		err = ice_flow_add_entry(hw, ICE_BLK_FD, prof_id,
+		err = ice_flow_add_entry(hw, ICE_BLK_FD, prof->id,
 					 main_vsi->idx, vsi_h,
 					 ICE_FLOW_PRIO_NORMAL, seg,
 					 &entry1_h);
@@ -748,7 +743,7 @@ ice_fdir_set_hw_fltr_rule(struct ice_pf *pf, struct ice_flow_seg_info *seg,
 
 		if (!hw_prof->entry_h[idx][tun])
 			continue;
-		ice_rem_prof_id_flow(hw, ICE_BLK_FD, vsi_num, prof_id);
+		ice_rem_prof_id_flow(hw, ICE_BLK_FD, vsi_num, prof->id);
 		ice_flow_rem_entry(hw, ICE_BLK_FD, hw_prof->entry_h[idx][tun]);
 		hw_prof->entry_h[idx][tun] = 0;
 		if (del_last)
@@ -758,10 +753,10 @@ ice_fdir_set_hw_fltr_rule(struct ice_pf *pf, struct ice_flow_seg_info *seg,
 		hw_prof->cnt = 0;
 err_entry:
 	ice_rem_prof_id_flow(hw, ICE_BLK_FD,
-			     ice_get_hw_vsi_num(hw, main_vsi->idx), prof_id);
+			     ice_get_hw_vsi_num(hw, main_vsi->idx), prof->id);
 	ice_flow_rem_entry(hw, ICE_BLK_FD, entry1_h);
 err_prof:
-	ice_flow_rem_prof(hw, ICE_BLK_FD, prof_id);
+	ice_flow_rem_prof(hw, ICE_BLK_FD, prof->id);
 	dev_err(dev, "Failed to add filter.  Flow director filters on each port must have the same input set.\n");
 
 	return err;
diff --git a/drivers/net/ethernet/intel/ice/ice_flex_pipe.c b/drivers/net/ethernet/intel/ice/ice_flex_pipe.c
index 5ce413965930..41260110c29c 100644
--- a/drivers/net/ethernet/intel/ice/ice_flex_pipe.c
+++ b/drivers/net/ethernet/intel/ice/ice_flex_pipe.c
@@ -2150,6 +2150,7 @@ void ice_clear_hw_tbls(struct ice_hw *hw)
 
 	for (i = 0; i < ICE_BLK_COUNT; i++) {
 		struct ice_prof_redir *prof_redir = &hw->blk[i].prof_redir;
+		struct ice_prof_id *prof_id = &hw->blk[i].prof_id;
 		struct ice_prof_tcam *prof = &hw->blk[i].prof;
 		struct ice_xlt1 *xlt1 = &hw->blk[i].xlt1;
 		struct ice_xlt2 *xlt2 = &hw->blk[i].xlt2;
@@ -2180,6 +2181,8 @@ void ice_clear_hw_tbls(struct ice_hw *hw)
 		memset(es->ref_count, 0, es->count * sizeof(*es->ref_count));
 		memset(es->written, 0, es->count * sizeof(*es->written));
 		memset(es->mask_ena, 0, es->count * sizeof(*es->mask_ena));
+
+		memset(prof_id->id, 0, prof_id->count * sizeof(*prof_id->id));
 	}
 }
 
@@ -2196,6 +2199,7 @@ int ice_init_hw_tbls(struct ice_hw *hw)
 	ice_init_all_prof_masks(hw);
 	for (i = 0; i < ICE_BLK_COUNT; i++) {
 		struct ice_prof_redir *prof_redir = &hw->blk[i].prof_redir;
+		struct ice_prof_id *prof_id = &hw->blk[i].prof_id;
 		struct ice_prof_tcam *prof = &hw->blk[i].prof;
 		struct ice_xlt1 *xlt1 = &hw->blk[i].xlt1;
 		struct ice_xlt2 *xlt2 = &hw->blk[i].xlt2;
@@ -2301,6 +2305,12 @@ int ice_init_hw_tbls(struct ice_hw *hw)
 					    sizeof(*es->mask_ena), GFP_KERNEL);
 		if (!es->mask_ena)
 			goto err;
+
+		prof_id->count = blk_sizes[i].prof_id;
+		prof_id->id = devm_kcalloc(ice_hw_to_dev(hw), prof_id->count,
+					   sizeof(*prof_id->id), GFP_KERNEL);
+		if (!prof_id->id)
+			goto err;
 	}
 	return 0;
 
diff --git a/drivers/net/ethernet/intel/ice/ice_flex_type.h b/drivers/net/ethernet/intel/ice/ice_flex_type.h
index 4f42e14ed3ae..395ebb18950a 100644
--- a/drivers/net/ethernet/intel/ice/ice_flex_type.h
+++ b/drivers/net/ethernet/intel/ice/ice_flex_type.h
@@ -304,10 +304,16 @@ struct ice_masks {
 	struct ice_mask masks[ICE_PROF_MASK_COUNT];
 };
 
+struct ice_prof_id {
+	unsigned long *id;
+	int count;
+};
+
 /* Tables per block */
 struct ice_blk_info {
 	struct ice_xlt1 xlt1;
 	struct ice_xlt2 xlt2;
+	struct ice_prof_id prof_id;
 	struct ice_prof_tcam prof;
 	struct ice_prof_redir prof_redir;
 	struct ice_es es;
diff --git a/drivers/net/ethernet/intel/ice/ice_flow.c b/drivers/net/ethernet/intel/ice/ice_flow.c
index ff76eaabfb69..44f90d243fe7 100644
--- a/drivers/net/ethernet/intel/ice/ice_flow.c
+++ b/drivers/net/ethernet/intel/ice/ice_flow.c
@@ -1329,7 +1329,6 @@ ice_flow_rem_entry_sync(struct ice_hw *hw, enum ice_block __always_unused blk,
  * @hw: pointer to the HW struct
  * @blk: classification stage
  * @dir: flow direction
- * @prof_id: unique ID to identify this flow profile
  * @segs: array of one or more packet segments that describe the flow
  * @segs_cnt: number of packet segments provided
  * @prof: stores the returned flow profile added
@@ -1338,17 +1337,24 @@ ice_flow_rem_entry_sync(struct ice_hw *hw, enum ice_block __always_unused blk,
  */
 static int
 ice_flow_add_prof_sync(struct ice_hw *hw, enum ice_block blk,
-		       enum ice_flow_dir dir, u64 prof_id,
+		       enum ice_flow_dir dir,
 		       struct ice_flow_seg_info *segs, u8 segs_cnt,
 		       struct ice_flow_prof **prof)
 {
 	struct ice_flow_prof_params *params;
+	struct ice_prof_id *ids;
 	int status;
+	u64 prof_id;
 	u8 i;
 
 	if (!prof)
 		return -EINVAL;
 
+	ids = &hw->blk[blk].prof_id;
+	prof_id = find_first_zero_bit(ids->id, ids->count);
+	if (prof_id >= ids->count)
+		return -ENOSPC;
+
 	params = kzalloc(sizeof(*params), GFP_KERNEL);
 	if (!params)
 		return -ENOMEM;
@@ -1394,6 +1400,7 @@ ice_flow_add_prof_sync(struct ice_hw *hw, enum ice_block blk,
 
 	INIT_LIST_HEAD(&params->prof->entries);
 	mutex_init(&params->prof->entries_lock);
+	set_bit(prof_id, ids->id);
 	*prof = params->prof;
 
 out:
@@ -1437,6 +1444,7 @@ ice_flow_rem_prof_sync(struct ice_hw *hw, enum ice_block blk,
 	/* Remove all hardware profiles associated with this flow profile */
 	status = ice_rem_prof(hw, blk, prof->id);
 	if (!status) {
+		clear_bit(prof->id, hw->blk[blk].prof_id.id);
 		list_del(&prof->l_entry);
 		mutex_destroy(&prof->entries_lock);
 		devm_kfree(ice_hw_to_dev(hw), prof);
@@ -1512,14 +1520,13 @@ ice_flow_disassoc_prof(struct ice_hw *hw, enum ice_block blk,
  * @hw: pointer to the HW struct
  * @blk: classification stage
  * @dir: flow direction
- * @prof_id: unique ID to identify this flow profile
  * @segs: array of one or more packet segments that describe the flow
  * @segs_cnt: number of packet segments provided
  * @prof: stores the returned flow profile added
  */
 int
 ice_flow_add_prof(struct ice_hw *hw, enum ice_block blk, enum ice_flow_dir dir,
-		  u64 prof_id, struct ice_flow_seg_info *segs, u8 segs_cnt,
+		  struct ice_flow_seg_info *segs, u8 segs_cnt,
 		  struct ice_flow_prof **prof)
 {
 	int status;
@@ -1539,8 +1546,7 @@ ice_flow_add_prof(struct ice_hw *hw, enum ice_block blk, enum ice_flow_dir dir,
 
 	mutex_lock(&hw->fl_profs_locks[blk]);
 
-	status = ice_flow_add_prof_sync(hw, blk, dir, prof_id, segs, segs_cnt,
-					prof);
+	status = ice_flow_add_prof_sync(hw, blk, dir, segs, segs_cnt, prof);
 	if (!status)
 		list_add(&(*prof)->l_entry, &hw->fl_profs[blk]);
 
@@ -2069,28 +2075,6 @@ ice_add_rss_list(struct ice_hw *hw, u16 vsi_handle, struct ice_flow_prof *prof)
 	return 0;
 }
 
-#define ICE_FLOW_PROF_HASH_S	0
-#define ICE_FLOW_PROF_HASH_M	GENMASK_ULL(31, 0)
-#define ICE_FLOW_PROF_HDR_S	32
-#define ICE_FLOW_PROF_HDR_M	GENMASK_ULL(61, 32)
-#define ICE_FLOW_PROF_ENCAP_S	62
-#define ICE_FLOW_PROF_ENCAP_M	GENMASK_ULL(63, 62)
-
-/* Flow profile ID format:
- * [0:31] - Packet match fields
- * [32:61] - Protocol header
- * [62:63] - Encapsulation flag:
- *	     0 if non-tunneled
- *	     1 if tunneled
- *	     2 for tunneled with outer ipv4
- *	     3 for tunneled with outer ipv6
- */
-#define ICE_FLOW_GEN_PROFID(hash, hdr, encap)                                \
-	((u64)(((u64)(hash) & ICE_FLOW_PROF_HASH_M) |                        \
-	       (((u64)(hdr) << ICE_FLOW_PROF_HDR_S) & ICE_FLOW_PROF_HDR_M) | \
-	       (((u64)(encap) << ICE_FLOW_PROF_ENCAP_S) &                    \
-		ICE_FLOW_PROF_ENCAP_M)))
-
 /**
  * ice_add_rss_cfg_sync - add an RSS configuration
  * @hw: pointer to the hardware structure
@@ -2168,13 +2152,8 @@ ice_add_rss_cfg_sync(struct ice_hw *hw, u16 vsi_handle,
 		goto exit;
 	}
 
-	/* Create a new flow profile with generated profile and packet
-	 * segment information.
-	 */
+	/* Create a new flow profile with packet segment information. */
 	status = ice_flow_add_prof(hw, blk, ICE_FLOW_RX,
-				   ICE_FLOW_GEN_PROFID(cfg->hash_flds,
-						       segs[segs_cnt - 1].hdrs,
-						       cfg->hdr_type),
 				   segs, segs_cnt, &prof);
 	if (status)
 		goto exit;
diff --git a/drivers/net/ethernet/intel/ice/ice_flow.h b/drivers/net/ethernet/intel/ice/ice_flow.h
index 7a1db1df760e..73a54e1dd31f 100644
--- a/drivers/net/ethernet/intel/ice/ice_flow.h
+++ b/drivers/net/ethernet/intel/ice/ice_flow.h
@@ -406,7 +406,7 @@ struct ice_rss_cfg {
 
 int
 ice_flow_add_prof(struct ice_hw *hw, enum ice_block blk, enum ice_flow_dir dir,
-		  u64 prof_id, struct ice_flow_seg_info *segs, u8 segs_cnt,
+		  struct ice_flow_seg_info *segs, u8 segs_cnt,
 		  struct ice_flow_prof **prof);
 int ice_flow_rem_prof(struct ice_hw *hw, enum ice_block blk, u64 prof_id);
 int
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index c726913bc635..20fb53ea4e5b 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -8117,13 +8117,12 @@ static int ice_add_vsi_to_fdir(struct ice_pf *pf, struct ice_vsi *vsi)
 
 		for (tun = 0; tun < ICE_FD_HW_SEG_MAX; tun++) {
 			enum ice_flow_priority prio;
-			u64 prof_id;
 
 			/* add this VSI to FDir profile for this flow */
 			prio = ICE_FLOW_PRIO_NORMAL;
 			prof = hw->fdir_prof[flow];
-			prof_id = flow + tun * ICE_FLTR_PTYPE_MAX;
-			status = ice_flow_add_entry(hw, ICE_BLK_FD, prof_id,
+			status = ice_flow_add_entry(hw, ICE_BLK_FD,
+						    prof->prof_id[tun],
 						    prof->vsi_h[0], vsi->idx,
 						    prio, prof->fdir_seg[tun],
 						    &entry_h);
diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
index bb5d8b681bc2..af468b2f1da6 100644
--- a/drivers/net/ethernet/intel/ice/ice_type.h
+++ b/drivers/net/ethernet/intel/ice/ice_type.h
@@ -245,6 +245,7 @@ struct ice_fd_hw_prof {
 	int cnt;
 	u64 entry_h[ICE_MAX_FDIR_VSI_PER_FILTER][ICE_FD_HW_SEG_MAX];
 	u16 vsi_h[ICE_MAX_FDIR_VSI_PER_FILTER];
+	u64 prof_id[ICE_FD_HW_SEG_MAX];
 };
 
 /* Common HW capabilities for SW use */
diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
index daa6a1e894cf..5cd143863b21 100644
--- a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
+++ b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
@@ -10,19 +10,6 @@
 #define to_fltr_conf_from_desc(p) \
 	container_of(p, struct virtchnl_fdir_fltr_conf, input)
 
-#define ICE_FLOW_PROF_TYPE_S	0
-#define ICE_FLOW_PROF_TYPE_M	(0xFFFFFFFFULL << ICE_FLOW_PROF_TYPE_S)
-#define ICE_FLOW_PROF_VSI_S	32
-#define ICE_FLOW_PROF_VSI_M	(0xFFFFFFFFULL << ICE_FLOW_PROF_VSI_S)
-
-/* Flow profile ID format:
- * [0:31] - flow type, flow + tun_offs
- * [32:63] - VSI index
- */
-#define ICE_FLOW_PROF_FD(vsi, flow, tun_offs) \
-	((u64)(((((flow) + (tun_offs)) & ICE_FLOW_PROF_TYPE_M)) | \
-	      (((u64)(vsi) << ICE_FLOW_PROF_VSI_S) & ICE_FLOW_PROF_VSI_M)))
-
 #define GTPU_TEID_OFFSET 4
 #define GTPU_EH_QFI_OFFSET 1
 #define GTPU_EH_QFI_MASK 0x3F
@@ -493,6 +480,7 @@ ice_vc_fdir_rem_prof(struct ice_vf *vf, enum ice_fltr_ptype flow, int tun)
 		return;
 
 	vf_prof = fdir->fdir_prof[flow];
+	prof_id = vf_prof->prof_id[tun];
 
 	vf_vsi = ice_get_vf_vsi(vf);
 	if (!vf_vsi) {
@@ -503,9 +491,6 @@ ice_vc_fdir_rem_prof(struct ice_vf *vf, enum ice_fltr_ptype flow, int tun)
 	if (!fdir->prof_entry_cnt[flow][tun])
 		return;
 
-	prof_id = ICE_FLOW_PROF_FD(vf_vsi->vsi_num,
-				   flow, tun ? ICE_FLTR_PTYPE_MAX : 0);
-
 	for (i = 0; i < fdir->prof_entry_cnt[flow][tun]; i++)
 		if (vf_prof->entry_h[i][tun]) {
 			u16 vsi_num = ice_get_hw_vsi_num(hw, vf_prof->vsi_h[i]);
@@ -647,7 +632,6 @@ ice_vc_fdir_write_flow_prof(struct ice_vf *vf, enum ice_fltr_ptype flow,
 	struct ice_hw *hw;
 	u64 entry1_h = 0;
 	u64 entry2_h = 0;
-	u64 prof_id;
 	int ret;
 
 	pf = vf->pf;
@@ -681,10 +665,7 @@ ice_vc_fdir_write_flow_prof(struct ice_vf *vf, enum ice_fltr_ptype flow,
 		ice_vc_fdir_rem_prof(vf, flow, tun);
 	}
 
-	prof_id = ICE_FLOW_PROF_FD(vf_vsi->vsi_num, flow,
-				   tun ? ICE_FLTR_PTYPE_MAX : 0);
-
-	ret = ice_flow_add_prof(hw, ICE_BLK_FD, ICE_FLOW_RX, prof_id, seg,
+	ret = ice_flow_add_prof(hw, ICE_BLK_FD, ICE_FLOW_RX, seg,
 				tun + 1, &prof);
 	if (ret) {
 		dev_dbg(dev, "Could not add VSI flow 0x%x for VF %d\n",
@@ -692,7 +673,7 @@ ice_vc_fdir_write_flow_prof(struct ice_vf *vf, enum ice_fltr_ptype flow,
 		goto err_exit;
 	}
 
-	ret = ice_flow_add_entry(hw, ICE_BLK_FD, prof_id, vf_vsi->idx,
+	ret = ice_flow_add_entry(hw, ICE_BLK_FD, prof->id, vf_vsi->idx,
 				 vf_vsi->idx, ICE_FLOW_PRIO_NORMAL,
 				 seg, &entry1_h);
 	if (ret) {
@@ -701,7 +682,7 @@ ice_vc_fdir_write_flow_prof(struct ice_vf *vf, enum ice_fltr_ptype flow,
 		goto err_prof;
 	}
 
-	ret = ice_flow_add_entry(hw, ICE_BLK_FD, prof_id, vf_vsi->idx,
+	ret = ice_flow_add_entry(hw, ICE_BLK_FD, prof->id, vf_vsi->idx,
 				 ctrl_vsi->idx, ICE_FLOW_PRIO_NORMAL,
 				 seg, &entry2_h);
 	if (ret) {
@@ -725,14 +706,16 @@ ice_vc_fdir_write_flow_prof(struct ice_vf *vf, enum ice_fltr_ptype flow,
 	vf_prof->cnt++;
 	fdir->prof_entry_cnt[flow][tun]++;
 
+	vf_prof->prof_id[tun] = prof->id;
+
 	return 0;
 
 err_entry_1:
 	ice_rem_prof_id_flow(hw, ICE_BLK_FD,
-			     ice_get_hw_vsi_num(hw, vf_vsi->idx), prof_id);
+			     ice_get_hw_vsi_num(hw, vf_vsi->idx), prof->id);
 	ice_flow_rem_entry(hw, ICE_BLK_FD, entry1_h);
 err_prof:
-	ice_flow_rem_prof(hw, ICE_BLK_FD, prof_id);
+	ice_flow_rem_prof(hw, ICE_BLK_FD, prof->id);
 err_exit:
 	return ret;
 }
-- 
2.34.1

[PATCH net-next v4 5/6] ice: enable symmetric RSS Toeplitz hash for any flow type

[Ahmed Zaki]

From: Jeff Guo <jia.guo@intel.com>

The hash function in the E800 NICs is set per-VSI and a specific AQ
command is needed to modify the hash function. Use the AQ command to
enable setting the symmetric Toeplitz RSS hash function for any VSI.

When the Symmetric Toeplitz hash function is used, the hardware sets the
input set of the RSS (Toeplitz) algorithm to be the XOR of the fields
index by HSYMM and the fields index by the INSET registers. We use this
to create a symmetric hash by setting the HSYMM registers to point to
their counterparts in the INSET registers:

 HSYMM [src_fv] = dst_fv;
 HSYMM [dst_fv] = src_fv;

where src_fv and dst_fv are the indexes of the protocol's src and dst
fields.

Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Signed-off-by: Jeff Guo <jia.guo@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Co-developed-by: Ahmed Zaki <ahmed.zaki@intel.com>
Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com>
---
 drivers/net/ethernet/intel/ice/ice.h          |   2 +
 drivers/net/ethernet/intel/ice/ice_common.h   |   1 +
 drivers/net/ethernet/intel/ice/ice_ethtool.c  |  10 +-
 .../net/ethernet/intel/ice/ice_ethtool_fdir.c |   4 +-
 .../net/ethernet/intel/ice/ice_flex_pipe.c    |  33 ++-
 .../net/ethernet/intel/ice/ice_flex_pipe.h    |   4 +-
 .../net/ethernet/intel/ice/ice_flex_type.h    |   1 +
 drivers/net/ethernet/intel/ice/ice_flow.c     | 222 +++++++++++++++---
 drivers/net/ethernet/intel/ice/ice_flow.h     |  13 +-
 .../net/ethernet/intel/ice/ice_hw_autogen.h   |   4 +
 drivers/net/ethernet/intel/ice/ice_lib.c      |  41 ++--
 drivers/net/ethernet/intel/ice/ice_main.c     |  44 ++++
 drivers/net/ethernet/intel/ice/ice_virtchnl.c |   9 +-
 .../ethernet/intel/ice/ice_virtchnl_fdir.c    |   2 +-
 14 files changed, 322 insertions(+), 68 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index fcaa5c3b8ec0..c131fe24f959 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -360,6 +360,7 @@ struct ice_vsi {
 	/* RSS config */
 	u16 rss_table_size;	/* HW RSS table size */
 	u16 rss_size;		/* Allocated RSS queues */
+	u8 rss_hfunc;		/* User configured hash type */
 	u8 *rss_hkey_user;	/* User configured hash keys */
 	u8 *rss_lut_user;	/* User configured lookup table entries */
 	u8 rss_lut_type;	/* used to configure Get/Set RSS LUT AQ call */
@@ -910,6 +911,7 @@ int ice_set_rss_lut(struct ice_vsi *vsi, u8 *lut, u16 lut_size);
 int ice_get_rss_lut(struct ice_vsi *vsi, u8 *lut, u16 lut_size);
 int ice_set_rss_key(struct ice_vsi *vsi, u8 *seed);
 int ice_get_rss_key(struct ice_vsi *vsi, u8 *seed);
+int ice_set_rss_hfunc(struct ice_vsi *vsi, u8 hfunc);
 void ice_fill_rss_lut(u8 *lut, u16 rss_table_size, u16 rss_size);
 int ice_schedule_reset(struct ice_pf *pf, enum ice_reset_req reset);
 void ice_print_link_msg(struct ice_vsi *vsi, bool isup);
diff --git a/drivers/net/ethernet/intel/ice/ice_common.h b/drivers/net/ethernet/intel/ice/ice_common.h
index 31fdcac33986..8d08a34dfb65 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.h
+++ b/drivers/net/ethernet/intel/ice/ice_common.h
@@ -6,6 +6,7 @@
 
 #include <linux/bitfield.h>
 
+#include "ice.h"
 #include "ice_type.h"
 #include "ice_nvm.h"
 #include "ice_flex_pipe.h"
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index d30d1b2a819b..64a93a91319b 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -2538,7 +2538,9 @@ ice_set_rss_hash_opt(struct ice_vsi *vsi, struct ethtool_rxnfc *nfc)
 	cfg.hash_flds = hashed_flds;
 	cfg.addl_hdrs = hdrs;
 	cfg.hdr_type = ICE_RSS_ANY_HEADERS;
-	status = ice_add_rss_cfg(&pf->hw, vsi->idx, &cfg);
+	cfg.symm = !!(nfc->data & RXH_SYMMETRIC_XOR);
+
+	status = ice_add_rss_cfg(&pf->hw, vsi, &cfg);
 	if (status) {
 		dev_dbg(dev, "ice_add_rss_cfg failed, vsi num = %d, error = %d\n",
 			vsi->vsi_num, status);
@@ -2559,6 +2561,7 @@ ice_get_rss_hash_opt(struct ice_vsi *vsi, struct ethtool_rxnfc *nfc)
 	struct ice_pf *pf = vsi->back;
 	struct device *dev;
 	u64 hash_flds;
+	bool symm;
 	u32 hdrs;
 
 	dev = ice_pf_to_dev(pf);
@@ -2577,7 +2580,7 @@ ice_get_rss_hash_opt(struct ice_vsi *vsi, struct ethtool_rxnfc *nfc)
 		return;
 	}
 
-	hash_flds = ice_get_rss_cfg(&pf->hw, vsi->idx, hdrs);
+	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",
 			vsi->vsi_num);
@@ -2601,6 +2604,9 @@ ice_get_rss_hash_opt(struct ice_vsi *vsi, struct ethtool_rxnfc *nfc)
 	    hash_flds & ICE_FLOW_HASH_FLD_UDP_DST_PORT ||
 	    hash_flds & ICE_FLOW_HASH_FLD_SCTP_DST_PORT)
 		nfc->data |= (u64)RXH_L4_B_2_3;
+
+	if (symm)
+		nfc->data |= (u64)RXH_SYMMETRIC_XOR;
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c b/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c
index c9c72f8dbcb2..20a2b768e2d8 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool_fdir.c
@@ -441,7 +441,7 @@ void ice_fdir_replay_flows(struct ice_hw *hw)
 			prof = hw->fdir_prof[flow];
 			ice_flow_add_prof(hw, ICE_BLK_FD, ICE_FLOW_RX,
 					  prof->fdir_seg[tun], TNL_SEG_CNT(tun),
-					  &hw_prof);
+					  false, &hw_prof);
 			for (j = 0; j < prof->cnt; j++) {
 				enum ice_flow_priority prio;
 				u64 entry_h = 0;
@@ -674,7 +674,7 @@ ice_fdir_set_hw_fltr_rule(struct ice_pf *pf, struct ice_flow_seg_info *seg,
 	 * actions (NULL) and zero actions 0.
 	 */
 	err = ice_flow_add_prof(hw, ICE_BLK_FD, ICE_FLOW_RX, seg,
-				TNL_SEG_CNT(tun), &prof);
+				TNL_SEG_CNT(tun), false, &prof);
 	if (err)
 		return err;
 	err = ice_flow_add_entry(hw, ICE_BLK_FD, prof->id, main_vsi->idx,
diff --git a/drivers/net/ethernet/intel/ice/ice_flex_pipe.c b/drivers/net/ethernet/intel/ice/ice_flex_pipe.c
index 41260110c29c..7f0bd4e42e40 100644
--- a/drivers/net/ethernet/intel/ice/ice_flex_pipe.c
+++ b/drivers/net/ethernet/intel/ice/ice_flex_pipe.c
@@ -1218,11 +1218,13 @@ ice_prof_has_mask(struct ice_hw *hw, enum ice_block blk, u8 prof, u16 *masks)
  * @blk: HW block
  * @fv: field vector to search for
  * @masks: masks for FV
+ * @symm: symmetric setting for RSS flows
  * @prof_id: receives the profile ID
  */
 static int
 ice_find_prof_id_with_mask(struct ice_hw *hw, enum ice_block blk,
-			   struct ice_fv_word *fv, u16 *masks, u8 *prof_id)
+			   struct ice_fv_word *fv, u16 *masks, bool symm,
+			   u8 *prof_id)
 {
 	struct ice_es *es = &hw->blk[blk].es;
 	u8 i;
@@ -1236,6 +1238,9 @@ ice_find_prof_id_with_mask(struct ice_hw *hw, enum ice_block blk,
 	for (i = 0; i < (u8)es->count; i++) {
 		u16 off = i * es->fvw;
 
+		if (blk == ICE_BLK_RSS && es->symm[i] != symm)
+			continue;
+
 		if (memcmp(&es->t[off], fv, es->fvw * sizeof(*fv)))
 			continue;
 
@@ -1716,15 +1721,16 @@ ice_update_prof_masking(struct ice_hw *hw, enum ice_block blk, u16 prof_id,
 }
 
 /**
- * ice_write_es - write an extraction sequence to hardware
+ * ice_write_es - write an extraction sequence and symmetric setting to hardware
  * @hw: pointer to the HW struct
  * @blk: the block in which to write the extraction sequence
  * @prof_id: the profile ID to write
  * @fv: pointer to the extraction sequence to write - NULL to clear extraction
+ * @symm: symmetric setting for RSS profiles
  */
 static void
 ice_write_es(struct ice_hw *hw, enum ice_block blk, u8 prof_id,
-	     struct ice_fv_word *fv)
+	     struct ice_fv_word *fv, bool symm)
 {
 	u16 off;
 
@@ -1737,6 +1743,9 @@ ice_write_es(struct ice_hw *hw, enum ice_block blk, u8 prof_id,
 		memcpy(&hw->blk[blk].es.t[off], fv,
 		       hw->blk[blk].es.fvw * sizeof(*fv));
 	}
+
+	if (blk == ICE_BLK_RSS)
+		hw->blk[blk].es.symm[prof_id] = symm;
 }
 
 /**
@@ -1753,7 +1762,7 @@ ice_prof_dec_ref(struct ice_hw *hw, enum ice_block blk, u8 prof_id)
 
 	if (hw->blk[blk].es.ref_count[prof_id] > 0) {
 		if (!--hw->blk[blk].es.ref_count[prof_id]) {
-			ice_write_es(hw, blk, prof_id, NULL);
+			ice_write_es(hw, blk, prof_id, NULL, false);
 			ice_free_prof_masks(hw, blk, prof_id);
 			return ice_free_prof_id(hw, blk, prof_id);
 		}
@@ -2116,6 +2125,7 @@ void ice_free_hw_tbls(struct ice_hw *hw)
 		devm_kfree(ice_hw_to_dev(hw), hw->blk[i].prof_redir.t);
 		devm_kfree(ice_hw_to_dev(hw), hw->blk[i].es.t);
 		devm_kfree(ice_hw_to_dev(hw), hw->blk[i].es.ref_count);
+		devm_kfree(ice_hw_to_dev(hw), hw->blk[i].es.symm);
 		devm_kfree(ice_hw_to_dev(hw), hw->blk[i].es.written);
 		devm_kfree(ice_hw_to_dev(hw), hw->blk[i].es.mask_ena);
 	}
@@ -2179,6 +2189,7 @@ void ice_clear_hw_tbls(struct ice_hw *hw)
 
 		memset(es->t, 0, es->count * sizeof(*es->t) * es->fvw);
 		memset(es->ref_count, 0, es->count * sizeof(*es->ref_count));
+		memset(es->symm, 0, es->count * sizeof(*es->symm));
 		memset(es->written, 0, es->count * sizeof(*es->written));
 		memset(es->mask_ena, 0, es->count * sizeof(*es->mask_ena));
 
@@ -2296,6 +2307,11 @@ int ice_init_hw_tbls(struct ice_hw *hw)
 		if (!es->ref_count)
 			goto err;
 
+		es->symm = devm_kcalloc(ice_hw_to_dev(hw), es->count,
+					sizeof(*es->symm), GFP_KERNEL);
+		if (!es->symm)
+			goto err;
+
 		es->written = devm_kcalloc(ice_hw_to_dev(hw), es->count,
 					   sizeof(*es->written), GFP_KERNEL);
 		if (!es->written)
@@ -2973,6 +2989,7 @@ ice_add_prof_attrib(struct ice_prof_map *prof, u8 ptg, u16 ptype,
  * @attr_cnt: number of elements in attr array
  * @es: extraction sequence (length of array is determined by the block)
  * @masks: mask for extraction sequence
+ * @symm: symmetric setting for RSS profiles
  *
  * This function registers a profile, which matches a set of PTYPES with a
  * particular extraction sequence. While the hardware profile is allocated
@@ -2982,7 +2999,7 @@ ice_add_prof_attrib(struct ice_prof_map *prof, u8 ptg, u16 ptype,
 int
 ice_add_prof(struct ice_hw *hw, enum ice_block blk, u64 id, u8 ptypes[],
 	     const struct ice_ptype_attributes *attr, u16 attr_cnt,
-	     struct ice_fv_word *es, u16 *masks)
+	     struct ice_fv_word *es, u16 *masks, bool symm)
 {
 	u32 bytes = DIV_ROUND_UP(ICE_FLOW_PTYPE_MAX, BITS_PER_BYTE);
 	DECLARE_BITMAP(ptgs_used, ICE_XLT1_CNT);
@@ -2996,7 +3013,7 @@ ice_add_prof(struct ice_hw *hw, enum ice_block blk, u64 id, u8 ptypes[],
 	mutex_lock(&hw->blk[blk].es.prof_map_lock);
 
 	/* search for existing profile */
-	status = ice_find_prof_id_with_mask(hw, blk, es, masks, &prof_id);
+	status = ice_find_prof_id_with_mask(hw, blk, es, masks, symm, &prof_id);
 	if (status) {
 		/* allocate profile ID */
 		status = ice_alloc_prof_id(hw, blk, &prof_id);
@@ -3019,7 +3036,7 @@ ice_add_prof(struct ice_hw *hw, enum ice_block blk, u64 id, u8 ptypes[],
 			goto err_ice_add_prof;
 
 		/* and write new es */
-		ice_write_es(hw, blk, prof_id, es);
+		ice_write_es(hw, blk, prof_id, es, symm);
 	}
 
 	ice_prof_inc_ref(hw, blk, prof_id);
@@ -3107,7 +3124,7 @@ ice_add_prof(struct ice_hw *hw, enum ice_block blk, u64 id, u8 ptypes[],
  * This will search for a profile tracking ID which was previously added.
  * The profile map lock should be held before calling this function.
  */
-static struct ice_prof_map *
+struct ice_prof_map *
 ice_search_prof_id(struct ice_hw *hw, enum ice_block blk, u64 id)
 {
 	struct ice_prof_map *entry = NULL;
diff --git a/drivers/net/ethernet/intel/ice/ice_flex_pipe.h b/drivers/net/ethernet/intel/ice/ice_flex_pipe.h
index 7af7c8e9aa4e..b39d7cdc381f 100644
--- a/drivers/net/ethernet/intel/ice/ice_flex_pipe.h
+++ b/drivers/net/ethernet/intel/ice/ice_flex_pipe.h
@@ -42,7 +42,9 @@ bool ice_hw_ptype_ena(struct ice_hw *hw, u16 ptype);
 int
 ice_add_prof(struct ice_hw *hw, enum ice_block blk, u64 id, u8 ptypes[],
 	     const struct ice_ptype_attributes *attr, u16 attr_cnt,
-	     struct ice_fv_word *es, u16 *masks);
+	     struct ice_fv_word *es, u16 *masks, bool symm);
+struct ice_prof_map *
+ice_search_prof_id(struct ice_hw *hw, enum ice_block blk, u64 id);
 int
 ice_add_prof_id_flow(struct ice_hw *hw, enum ice_block blk, u16 vsi, u64 hdl);
 int
diff --git a/drivers/net/ethernet/intel/ice/ice_flex_type.h b/drivers/net/ethernet/intel/ice/ice_flex_type.h
index 395ebb18950a..d427a79d001a 100644
--- a/drivers/net/ethernet/intel/ice/ice_flex_type.h
+++ b/drivers/net/ethernet/intel/ice/ice_flex_type.h
@@ -146,6 +146,7 @@ struct ice_es {
 	u32 *mask_ena;
 	struct list_head prof_map;
 	struct ice_fv_word *t;
+	u8 *symm;	/* symmetric setting per profile (RSS blk)*/
 	struct mutex prof_map_lock;	/* protect access to profiles list */
 	u8 *written;
 	u8 reverse; /* set to true to reverse FV order */
diff --git a/drivers/net/ethernet/intel/ice/ice_flow.c b/drivers/net/ethernet/intel/ice/ice_flow.c
index 44f90d243fe7..efce56368083 100644
--- a/drivers/net/ethernet/intel/ice/ice_flow.c
+++ b/drivers/net/ethernet/intel/ice/ice_flow.c
@@ -1235,6 +1235,7 @@ ice_flow_proc_segs(struct ice_hw *hw, struct ice_flow_prof_params *params)
 #define ICE_FLOW_FIND_PROF_CHK_FLDS	0x00000001
 #define ICE_FLOW_FIND_PROF_CHK_VSI	0x00000002
 #define ICE_FLOW_FIND_PROF_NOT_CHK_DIR	0x00000004
+#define ICE_FLOW_FIND_PROF_CHK_SYMM	0x00000008
 
 /**
  * ice_flow_find_prof_conds - Find a profile matching headers and conditions
@@ -1243,13 +1244,14 @@ ice_flow_proc_segs(struct ice_hw *hw, struct ice_flow_prof_params *params)
  * @dir: flow direction
  * @segs: array of one or more packet segments that describe the flow
  * @segs_cnt: number of packet segments provided
+ * @symm: symmetric setting for RSS profiles
  * @vsi_handle: software VSI handle to check VSI (ICE_FLOW_FIND_PROF_CHK_VSI)
  * @conds: additional conditions to be checked (ICE_FLOW_FIND_PROF_CHK_*)
  */
 static struct ice_flow_prof *
 ice_flow_find_prof_conds(struct ice_hw *hw, enum ice_block blk,
 			 enum ice_flow_dir dir, struct ice_flow_seg_info *segs,
-			 u8 segs_cnt, u16 vsi_handle, u32 conds)
+			 u8 segs_cnt, bool symm, u16 vsi_handle, u32 conds)
 {
 	struct ice_flow_prof *p, *prof = NULL;
 
@@ -1265,6 +1267,11 @@ ice_flow_find_prof_conds(struct ice_hw *hw, enum ice_block blk,
 			    !test_bit(vsi_handle, p->vsis))
 				continue;
 
+			/* Check for symmetric settings */
+			if ((conds & ICE_FLOW_FIND_PROF_CHK_SYMM) &&
+			    p->symm != symm)
+				continue;
+
 			/* Protocol headers must be checked. Matched fields are
 			 * checked if specified.
 			 */
@@ -1331,6 +1338,7 @@ ice_flow_rem_entry_sync(struct ice_hw *hw, enum ice_block __always_unused blk,
  * @dir: flow direction
  * @segs: array of one or more packet segments that describe the flow
  * @segs_cnt: number of packet segments provided
+ * @symm: symmetric setting for RSS profiles
  * @prof: stores the returned flow profile added
  *
  * Assumption: the caller has acquired the lock to the profile list
@@ -1339,7 +1347,7 @@ static int
 ice_flow_add_prof_sync(struct ice_hw *hw, enum ice_block blk,
 		       enum ice_flow_dir dir,
 		       struct ice_flow_seg_info *segs, u8 segs_cnt,
-		       struct ice_flow_prof **prof)
+		       bool symm, struct ice_flow_prof **prof)
 {
 	struct ice_flow_prof_params *params;
 	struct ice_prof_id *ids;
@@ -1376,6 +1384,7 @@ ice_flow_add_prof_sync(struct ice_hw *hw, enum ice_block blk,
 	params->prof->id = prof_id;
 	params->prof->dir = dir;
 	params->prof->segs_cnt = segs_cnt;
+	params->prof->symm = symm;
 
 	/* Make a copy of the segments that need to be persistent in the flow
 	 * profile instance
@@ -1392,7 +1401,7 @@ ice_flow_add_prof_sync(struct ice_hw *hw, enum ice_block blk,
 	/* Add a HW profile for this flow profile */
 	status = ice_add_prof(hw, blk, prof_id, (u8 *)params->ptypes,
 			      params->attr, params->attr_cnt, params->es,
-			      params->mask);
+			      params->mask, symm);
 	if (status) {
 		ice_debug(hw, ICE_DBG_FLOW, "Error adding a HW flow profile\n");
 		goto out;
@@ -1522,12 +1531,13 @@ ice_flow_disassoc_prof(struct ice_hw *hw, enum ice_block blk,
  * @dir: flow direction
  * @segs: array of one or more packet segments that describe the flow
  * @segs_cnt: number of packet segments provided
+ * @symm: symmetric setting for RSS profiles
  * @prof: stores the returned flow profile added
  */
 int
 ice_flow_add_prof(struct ice_hw *hw, enum ice_block blk, enum ice_flow_dir dir,
 		  struct ice_flow_seg_info *segs, u8 segs_cnt,
-		  struct ice_flow_prof **prof)
+		  bool symm, struct ice_flow_prof **prof)
 {
 	int status;
 
@@ -1546,7 +1556,8 @@ ice_flow_add_prof(struct ice_hw *hw, enum ice_block blk, enum ice_flow_dir dir,
 
 	mutex_lock(&hw->fl_profs_locks[blk]);
 
-	status = ice_flow_add_prof_sync(hw, blk, dir, segs, segs_cnt, prof);
+	status = ice_flow_add_prof_sync(hw, blk, dir, segs, segs_cnt,
+					symm, prof);
 	if (!status)
 		list_add(&(*prof)->l_entry, &hw->fl_profs[blk]);
 
@@ -2068,6 +2079,7 @@ ice_add_rss_list(struct ice_hw *hw, u16 vsi_handle, struct ice_flow_prof *prof)
 	rss_cfg->hash.hash_flds = prof->segs[prof->segs_cnt - 1].match;
 	rss_cfg->hash.addl_hdrs = prof->segs[prof->segs_cnt - 1].hdrs;
 	rss_cfg->hash.hdr_type = hdr_type;
+	rss_cfg->hash.symm = prof->symm;
 	set_bit(vsi_handle, rss_cfg->vsis);
 
 	list_add_tail(&rss_cfg->l_entry, &hw->rss_list_head);
@@ -2075,6 +2087,139 @@ ice_add_rss_list(struct ice_hw *hw, u16 vsi_handle, struct ice_flow_prof *prof)
 	return 0;
 }
 
+/**
+ * ice_rss_config_xor_word - set the HSYMM registers for one input set word
+ * @hw: pointer to the hardware structure
+ * @prof_id: RSS hardware profile id
+ * @src: the FV index used by the protocol's source field
+ * @dst: the FV index used by the protocol's destination field
+ *
+ * Write to the HSYMM register with the index of @src FV the value of the @dst
+ * FV index. This will tell the hardware to XOR HSYMM[src] with INSET[dst]
+ * while calculating the RSS input set.
+ */
+static void
+ice_rss_config_xor_word(struct ice_hw *hw, u8 prof_id, u8 src, u8 dst)
+{
+	u32 val, reg, bits_shift;
+	u8 reg_idx;
+
+	reg_idx = src / GLQF_HSYMM_REG_SIZE;
+	bits_shift = ((src % GLQF_HSYMM_REG_SIZE) << 3);
+	val = dst | GLQF_HSYMM_ENABLE_BIT;
+
+	reg = rd32(hw, GLQF_HSYMM(prof_id, reg_idx));
+	reg = (reg & ~(0xff << bits_shift)) | (val << bits_shift);
+	wr32(hw, GLQF_HSYMM(prof_id, reg_idx), reg);
+}
+
+/**
+ * ice_rss_config_xor - set the symmetric registers for a profile's protocol
+ * @hw: pointer to the hardware structure
+ * @prof_id: RSS hardware profile id
+ * @src: the FV index used by the protocol's source field
+ * @dst: the FV index used by the protocol's destination field
+ * @len: length of the source/destination fields in words
+ */
+static void
+ice_rss_config_xor(struct ice_hw *hw, u8 prof_id, u8 src, u8 dst, u8 len)
+{
+	int fv_last_word =
+		ICE_FLOW_SW_FIELD_VECTOR_MAX / ICE_FLOW_FV_EXTRACT_SZ - 1;
+	int i;
+
+	for (i = 0; i < len; i++) {
+		ice_rss_config_xor_word(hw, prof_id,
+					/* Yes, field vector in GLQF_HSYMM and
+					 * GLQF_HINSET is inversed!
+					 */
+					fv_last_word - (src + i),
+					fv_last_word - (dst + i));
+		ice_rss_config_xor_word(hw, prof_id,
+					fv_last_word - (dst + i),
+					fv_last_word - (src + i));
+	}
+}
+
+/**
+ * ice_rss_set_symm - set the symmetric settings for an RSS profile
+ * @hw: pointer to the hardware structure
+ * @prof: pointer to flow profile
+ *
+ * The symmetric hash will result from XORing the protocol's fields with
+ * indexes in GLQF_HSYMM and GLQF_HINSET. This function configures the profile's
+ * GLQF_HSYMM registers.
+ */
+static void ice_rss_set_symm(struct ice_hw *hw, struct ice_flow_prof *prof)
+{
+	struct ice_prof_map *map;
+	u8 prof_id, m;
+
+	mutex_lock(&hw->blk[ICE_BLK_RSS].es.prof_map_lock);
+	map = ice_search_prof_id(hw, ICE_BLK_RSS, prof->id);
+	if (map)
+		prof_id = map->prof_id;
+	mutex_unlock(&hw->blk[ICE_BLK_RSS].es.prof_map_lock);
+
+	if (!map)
+		return;
+
+	/* clear to default */
+	for (m = 0; m < GLQF_HSYMM_REG_PER_PROF; m++)
+		wr32(hw, GLQF_HSYMM(prof_id, m), 0);
+
+	if (prof->symm) {
+		struct ice_flow_seg_xtrct *ipv4_src, *ipv4_dst;
+		struct ice_flow_seg_xtrct *ipv6_src, *ipv6_dst;
+		struct ice_flow_seg_xtrct *sctp_src, *sctp_dst;
+		struct ice_flow_seg_xtrct *tcp_src, *tcp_dst;
+		struct ice_flow_seg_xtrct *udp_src, *udp_dst;
+		struct ice_flow_seg_info *seg;
+
+		seg = &prof->segs[prof->segs_cnt - 1];
+
+		ipv4_src = &seg->fields[ICE_FLOW_FIELD_IDX_IPV4_SA].xtrct;
+		ipv4_dst = &seg->fields[ICE_FLOW_FIELD_IDX_IPV4_DA].xtrct;
+
+		ipv6_src = &seg->fields[ICE_FLOW_FIELD_IDX_IPV6_SA].xtrct;
+		ipv6_dst = &seg->fields[ICE_FLOW_FIELD_IDX_IPV6_DA].xtrct;
+
+		tcp_src = &seg->fields[ICE_FLOW_FIELD_IDX_TCP_SRC_PORT].xtrct;
+		tcp_dst = &seg->fields[ICE_FLOW_FIELD_IDX_TCP_DST_PORT].xtrct;
+
+		udp_src = &seg->fields[ICE_FLOW_FIELD_IDX_UDP_SRC_PORT].xtrct;
+		udp_dst = &seg->fields[ICE_FLOW_FIELD_IDX_UDP_DST_PORT].xtrct;
+
+		sctp_src = &seg->fields[ICE_FLOW_FIELD_IDX_SCTP_SRC_PORT].xtrct;
+		sctp_dst = &seg->fields[ICE_FLOW_FIELD_IDX_SCTP_DST_PORT].xtrct;
+
+		/* xor IPv4 */
+		if (ipv4_src->prot_id != 0 && ipv4_dst->prot_id != 0)
+			ice_rss_config_xor(hw, prof_id,
+					   ipv4_src->idx, ipv4_dst->idx, 2);
+
+		/* xor IPv6 */
+		if (ipv6_src->prot_id != 0 && ipv6_dst->prot_id != 0)
+			ice_rss_config_xor(hw, prof_id,
+					   ipv6_src->idx, ipv6_dst->idx, 8);
+
+		/* xor TCP */
+		if (tcp_src->prot_id != 0 && tcp_dst->prot_id != 0)
+			ice_rss_config_xor(hw, prof_id,
+					   tcp_src->idx, tcp_dst->idx, 1);
+
+		/* xor UDP */
+		if (udp_src->prot_id != 0 && udp_dst->prot_id != 0)
+			ice_rss_config_xor(hw, prof_id,
+					   udp_src->idx, udp_dst->idx, 1);
+
+		/* xor SCTP */
+		if (sctp_src->prot_id != 0 && sctp_dst->prot_id != 0)
+			ice_rss_config_xor(hw, prof_id,
+					   sctp_src->idx, sctp_dst->idx, 1);
+	}
+}
+
 /**
  * ice_add_rss_cfg_sync - add an RSS configuration
  * @hw: pointer to the hardware structure
@@ -2094,8 +2239,7 @@ ice_add_rss_cfg_sync(struct ice_hw *hw, u16 vsi_handle,
 	int status;
 
 	segs_cnt = (cfg->hdr_type == ICE_RSS_OUTER_HEADERS) ?
-			   ICE_FLOW_SEG_SINGLE :
-			   ICE_FLOW_SEG_MAX;
+			ICE_FLOW_SEG_SINGLE : ICE_FLOW_SEG_MAX;
 
 	segs = kcalloc(segs_cnt, sizeof(*segs), GFP_KERNEL);
 	if (!segs)
@@ -2106,13 +2250,14 @@ ice_add_rss_cfg_sync(struct ice_hw *hw, u16 vsi_handle,
 	if (status)
 		goto exit;
 
-	/* Search for a flow profile that has matching headers, hash fields
-	 * and has the input VSI associated to it. If found, no further
+	/* Search for a flow profile that has matching headers, hash fields,
+	 * symm and has the input VSI associated to it. If found, no further
 	 * operations required and exit.
 	 */
 	prof = ice_flow_find_prof_conds(hw, blk, ICE_FLOW_RX, segs, segs_cnt,
-					vsi_handle,
+					cfg->symm, vsi_handle,
 					ICE_FLOW_FIND_PROF_CHK_FLDS |
+					ICE_FLOW_FIND_PROF_CHK_SYMM |
 					ICE_FLOW_FIND_PROF_CHK_VSI);
 	if (prof)
 		goto exit;
@@ -2123,7 +2268,8 @@ ice_add_rss_cfg_sync(struct ice_hw *hw, u16 vsi_handle,
 	 * the protocol header and new hash field configuration.
 	 */
 	prof = ice_flow_find_prof_conds(hw, blk, ICE_FLOW_RX, segs, segs_cnt,
-					vsi_handle, ICE_FLOW_FIND_PROF_CHK_VSI);
+					cfg->symm, vsi_handle,
+					ICE_FLOW_FIND_PROF_CHK_VSI);
 	if (prof) {
 		status = ice_flow_disassoc_prof(hw, blk, prof, vsi_handle);
 		if (!status)
@@ -2139,11 +2285,12 @@ ice_add_rss_cfg_sync(struct ice_hw *hw, u16 vsi_handle,
 		}
 	}
 
-	/* Search for a profile that has same match fields only. If this
-	 * exists then associate the VSI to this profile.
+	/* Search for a profile that has the same match fields and symmetric
+	 * setting. If this exists then associate the VSI to this profile.
 	 */
 	prof = ice_flow_find_prof_conds(hw, blk, ICE_FLOW_RX, segs, segs_cnt,
-					vsi_handle,
+					cfg->symm, vsi_handle,
+					ICE_FLOW_FIND_PROF_CHK_SYMM |
 					ICE_FLOW_FIND_PROF_CHK_FLDS);
 	if (prof) {
 		status = ice_flow_assoc_prof(hw, blk, prof, vsi_handle);
@@ -2154,10 +2301,12 @@ ice_add_rss_cfg_sync(struct ice_hw *hw, u16 vsi_handle,
 
 	/* Create a new flow profile with packet segment information. */
 	status = ice_flow_add_prof(hw, blk, ICE_FLOW_RX,
-				   segs, segs_cnt, &prof);
+				   segs, segs_cnt, cfg->symm, &prof);
 	if (status)
 		goto exit;
 
+	prof->symm = cfg->symm;
+	ice_rss_set_symm(hw, prof);
 	status = ice_flow_assoc_prof(hw, blk, prof, vsi_handle);
 	/* If association to a new flow profile failed then this profile can
 	 * be removed.
@@ -2177,7 +2326,7 @@ ice_add_rss_cfg_sync(struct ice_hw *hw, u16 vsi_handle,
 /**
  * ice_add_rss_cfg - add an RSS configuration with specified hashed fields
  * @hw: pointer to the hardware structure
- * @vsi_handle: software VSI handle
+ * @vsi: VSI to add the RSS configuration to
  * @cfg: configure parameters
  *
  * This function will generate a flow profile based on fields associated with
@@ -2185,14 +2334,19 @@ ice_add_rss_cfg_sync(struct ice_hw *hw, u16 vsi_handle,
  * a flow entry to the profile.
  */
 int
-ice_add_rss_cfg(struct ice_hw *hw, u16 vsi_handle,
+ice_add_rss_cfg(struct ice_hw *hw, struct ice_vsi *vsi,
 		const struct ice_rss_hash_cfg *cfg)
 {
 	struct ice_rss_hash_cfg local_cfg;
+	u16 vsi_handle;
 	int status;
 
-	if (!ice_is_vsi_valid(hw, vsi_handle) || !cfg ||
-	    cfg->hdr_type > ICE_RSS_ANY_HEADERS ||
+	if (!vsi)
+		return -EINVAL;
+
+	vsi_handle = vsi->idx;
+	if (!ice_is_vsi_valid(hw, vsi_handle) ||
+	    !cfg || cfg->hdr_type > ICE_RSS_ANY_HEADERS ||
 	    cfg->hash_flds == ICE_HASH_INVALID)
 		return -EINVAL;
 
@@ -2211,6 +2365,11 @@ ice_add_rss_cfg(struct ice_hw *hw, u16 vsi_handle,
 	}
 	mutex_unlock(&hw->rss_locks);
 
+	/* Update the VSI's hash function */
+	if (cfg->symm && vsi->rss_hfunc != ICE_AQ_VSI_Q_OPT_RSS_HASH_SYM_TPLZ)
+		return ice_set_rss_hfunc(vsi,
+					 ICE_AQ_VSI_Q_OPT_RSS_HASH_SYM_TPLZ);
+
 	return status;
 }
 
@@ -2233,8 +2392,7 @@ ice_rem_rss_cfg_sync(struct ice_hw *hw, u16 vsi_handle,
 	int status;
 
 	segs_cnt = (cfg->hdr_type == ICE_RSS_OUTER_HEADERS) ?
-			   ICE_FLOW_SEG_SINGLE :
-			   ICE_FLOW_SEG_MAX;
+			ICE_FLOW_SEG_SINGLE : ICE_FLOW_SEG_MAX;
 	segs = kcalloc(segs_cnt, sizeof(*segs), GFP_KERNEL);
 	if (!segs)
 		return -ENOMEM;
@@ -2245,7 +2403,7 @@ ice_rem_rss_cfg_sync(struct ice_hw *hw, u16 vsi_handle,
 		goto out;
 
 	prof = ice_flow_find_prof_conds(hw, blk, ICE_FLOW_RX, segs, segs_cnt,
-					vsi_handle,
+					cfg->symm, vsi_handle,
 					ICE_FLOW_FIND_PROF_CHK_FLDS);
 	if (!prof) {
 		status = -ENOENT;
@@ -2288,8 +2446,8 @@ ice_rem_rss_cfg(struct ice_hw *hw, u16 vsi_handle,
 	struct ice_rss_hash_cfg local_cfg;
 	int status;
 
-	if (!ice_is_vsi_valid(hw, vsi_handle) || !cfg ||
-	    cfg->hdr_type > ICE_RSS_ANY_HEADERS ||
+	if (!ice_is_vsi_valid(hw, vsi_handle) ||
+	    !cfg || cfg->hdr_type > ICE_RSS_ANY_HEADERS ||
 	    cfg->hash_flds == ICE_HASH_INVALID)
 		return -EINVAL;
 
@@ -2346,19 +2504,24 @@ ice_rem_rss_cfg(struct ice_hw *hw, u16 vsi_handle,
 /**
  * ice_add_avf_rss_cfg - add an RSS configuration for AVF driver
  * @hw: pointer to the hardware structure
- * @vsi_handle: software VSI handle
+ * @vsi: VF's VSI
  * @avf_hash: hash bit fields (ICE_AVF_FLOW_FIELD_*) to configure
  *
  * This function will take the hash bitmap provided by the AVF driver via a
  * message, convert it to ICE-compatible values, and configure RSS flow
  * profiles.
  */
-int ice_add_avf_rss_cfg(struct ice_hw *hw, u16 vsi_handle, u64 avf_hash)
+int ice_add_avf_rss_cfg(struct ice_hw *hw, struct ice_vsi *vsi, u64 avf_hash)
 {
 	struct ice_rss_hash_cfg hcfg;
+	u16 vsi_handle;
 	int status = 0;
 	u64 hash_flds;
 
+	if (!vsi)
+		return -EINVAL;
+
+	vsi_handle = vsi->idx;
 	if (avf_hash == ICE_AVF_FLOW_FIELD_INVALID ||
 	    !ice_is_vsi_valid(hw, vsi_handle))
 		return -EINVAL;
@@ -2431,7 +2594,8 @@ int ice_add_avf_rss_cfg(struct ice_hw *hw, u16 vsi_handle, u64 avf_hash)
 		hcfg.addl_hdrs = ICE_FLOW_SEG_HDR_NONE;
 		hcfg.hash_flds = rss_hash;
 		hcfg.hdr_type = ICE_RSS_ANY_HEADERS;
-		status = ice_add_rss_cfg(hw, vsi_handle, &hcfg);
+		hcfg.symm = false;
+		status = ice_add_rss_cfg(hw, vsi, &hcfg);
 		if (status)
 			break;
 	}
@@ -2470,11 +2634,12 @@ int ice_replay_rss_cfg(struct ice_hw *hw, u16 vsi_handle)
  * @hw: pointer to the hardware structure
  * @vsi_handle: software VSI handle
  * @hdrs: protocol header type
+ * @symm: whether the RSS is symmetric (bool, output)
  *
  * This function will return the match fields of the first instance of flow
  * profile having the given header types and containing input VSI
  */
-u64 ice_get_rss_cfg(struct ice_hw *hw, u16 vsi_handle, u32 hdrs)
+u64 ice_get_rss_cfg(struct ice_hw *hw, u16 vsi_handle, u32 hdrs, bool *symm)
 {
 	u64 rss_hash = ICE_HASH_INVALID;
 	struct ice_rss_cfg *r;
@@ -2488,6 +2653,7 @@ u64 ice_get_rss_cfg(struct ice_hw *hw, u16 vsi_handle, u32 hdrs)
 		if (test_bit(vsi_handle, r->vsis) &&
 		    r->hash.addl_hdrs == hdrs) {
 			rss_hash = r->hash.hash_flds;
+			*symm = r->hash.symm;
 			break;
 		}
 	mutex_unlock(&hw->rss_locks);
diff --git a/drivers/net/ethernet/intel/ice/ice_flow.h b/drivers/net/ethernet/intel/ice/ice_flow.h
index 73a54e1dd31f..f0ac023ed37e 100644
--- a/drivers/net/ethernet/intel/ice/ice_flow.h
+++ b/drivers/net/ethernet/intel/ice/ice_flow.h
@@ -296,6 +296,7 @@ struct ice_rss_hash_cfg {
 	u32 addl_hdrs; /* protocol header fields */
 	u64 hash_flds; /* hash bit field (ICE_FLOW_HASH_*) to configure */
 	enum ice_rss_cfg_hdr_type hdr_type; /* to specify inner or outer */
+	bool symm; /* symmetric or asymmetric hash */
 };
 
 enum ice_flow_dir {
@@ -311,6 +312,7 @@ enum ice_flow_priority {
 #define ICE_FLOW_SEG_SINGLE		1
 #define ICE_FLOW_SEG_MAX		2
 #define ICE_FLOW_SEG_RAW_FLD_MAX	2
+#define ICE_FLOW_SW_FIELD_VECTOR_MAX	48
 #define ICE_FLOW_FV_EXTRACT_SZ		2
 
 #define ICE_FLOW_SET_HDRS(seg, val)	((seg)->hdrs |= (u32)(val))
@@ -395,6 +397,8 @@ struct ice_flow_prof {
 
 	/* software VSI handles referenced by this flow profile */
 	DECLARE_BITMAP(vsis, ICE_MAX_VSI);
+
+	bool symm; /* Symmetric Hash for RSS */
 };
 
 struct ice_rss_cfg {
@@ -407,7 +411,7 @@ struct ice_rss_cfg {
 int
 ice_flow_add_prof(struct ice_hw *hw, enum ice_block blk, enum ice_flow_dir dir,
 		  struct ice_flow_seg_info *segs, u8 segs_cnt,
-		  struct ice_flow_prof **prof);
+		  bool symm, struct ice_flow_prof **prof);
 int ice_flow_rem_prof(struct ice_hw *hw, enum ice_block blk, u64 prof_id);
 int
 ice_flow_add_entry(struct ice_hw *hw, enum ice_block blk, u64 prof_id,
@@ -423,11 +427,12 @@ ice_flow_add_fld_raw(struct ice_flow_seg_info *seg, u16 off, u8 len,
 int ice_flow_rem_vsi_prof(struct ice_hw *hw, u16 vsi_handle, u64 prof_id);
 void ice_rem_vsi_rss_list(struct ice_hw *hw, u16 vsi_handle);
 int ice_replay_rss_cfg(struct ice_hw *hw, u16 vsi_handle);
-int ice_add_avf_rss_cfg(struct ice_hw *hw, u16 vsi_handle, u64 hashed_flds);
+int ice_add_avf_rss_cfg(struct ice_hw *hw, struct ice_vsi *vsi,
+			u64 hashed_flds);
 int ice_rem_vsi_rss_cfg(struct ice_hw *hw, u16 vsi_handle);
-int ice_add_rss_cfg(struct ice_hw *hw, u16 vsi_handle,
+int ice_add_rss_cfg(struct ice_hw *hw, struct ice_vsi *vsi,
 		    const struct ice_rss_hash_cfg *cfg);
 int ice_rem_rss_cfg(struct ice_hw *hw, u16 vsi_handle,
 		    const struct ice_rss_hash_cfg *cfg);
-u64 ice_get_rss_cfg(struct ice_hw *hw, u16 vsi_handle, u32 hdrs);
+u64 ice_get_rss_cfg(struct ice_hw *hw, u16 vsi_handle, u32 hdrs, bool *symm);
 #endif /* _ICE_FLOW_H_ */
diff --git a/drivers/net/ethernet/intel/ice/ice_hw_autogen.h b/drivers/net/ethernet/intel/ice/ice_hw_autogen.h
index 6756f3d51d14..ac5aef1ea300 100644
--- a/drivers/net/ethernet/intel/ice/ice_hw_autogen.h
+++ b/drivers/net/ethernet/intel/ice/ice_hw_autogen.h
@@ -389,6 +389,10 @@
 #define GLQF_HMASK_SEL(_i)			(0x00410000 + ((_i) * 4))
 #define GLQF_HMASK_SEL_MAX_INDEX		127
 #define GLQF_HMASK_SEL_MASK_SEL_S		0
+#define GLQF_HSYMM(_i, _j)			(0x0040F000 + ((_i) * 4 + (_j) * 512))
+#define GLQF_HSYMM_REG_SIZE			4
+#define GLQF_HSYMM_REG_PER_PROF			6
+#define GLQF_HSYMM_ENABLE_BIT			BIT(7)
 #define PFQF_FD_ENA				0x0043A000
 #define PFQF_FD_ENA_FD_ENA_M			BIT(0)
 #define PFQF_FD_SIZE				0x00460100
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index f7a22e0bb505..639845bdcca6 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -1186,12 +1186,10 @@ static void ice_set_rss_vsi_ctx(struct ice_vsi_ctx *ctxt, struct ice_vsi *vsi)
 	case ICE_VSI_PF:
 		/* PF VSI will inherit RSS instance of PF */
 		lut_type = ICE_AQ_VSI_Q_OPT_RSS_LUT_PF;
-		hash_type = ICE_AQ_VSI_Q_OPT_RSS_HASH_TPLZ;
 		break;
 	case ICE_VSI_VF:
 		/* VF VSI will gets a small RSS table which is a VSI LUT type */
 		lut_type = ICE_AQ_VSI_Q_OPT_RSS_LUT_VSI;
-		hash_type = ICE_AQ_VSI_Q_OPT_RSS_HASH_TPLZ;
 		break;
 	default:
 		dev_dbg(dev, "Unsupported VSI type %s\n",
@@ -1199,10 +1197,12 @@ static void ice_set_rss_vsi_ctx(struct ice_vsi_ctx *ctxt, struct ice_vsi *vsi)
 		return;
 	}
 
-	ctxt->info.q_opt_rss = ((lut_type << ICE_AQ_VSI_Q_OPT_RSS_LUT_S) &
-				ICE_AQ_VSI_Q_OPT_RSS_LUT_M) |
-				((hash_type << ICE_AQ_VSI_Q_OPT_RSS_HASH_S) &
-				 ICE_AQ_VSI_Q_OPT_RSS_HASH_M);
+	hash_type = ICE_AQ_VSI_Q_OPT_RSS_HASH_TPLZ;
+	vsi->rss_hfunc = hash_type;
+
+	ctxt->info.q_opt_rss =
+		FIELD_PREP(ICE_AQ_VSI_Q_OPT_RSS_LUT_M, lut_type) |
+		FIELD_PREP(ICE_AQ_VSI_Q_OPT_RSS_HASH_M, hash_type);
 }
 
 static void
@@ -1601,7 +1601,7 @@ static void ice_vsi_set_vf_rss_flow_fld(struct ice_vsi *vsi)
 		return;
 	}
 
-	status = ice_add_avf_rss_cfg(&pf->hw, vsi->idx, ICE_DEFAULT_RSS_HENA);
+	status = ice_add_avf_rss_cfg(&pf->hw, vsi, ICE_DEFAULT_RSS_HENA);
 	if (status)
 		dev_dbg(dev, "ice_add_avf_rss_cfg failed for vsi = %d, error = %d\n",
 			vsi->vsi_num, status);
@@ -1609,34 +1609,34 @@ static void ice_vsi_set_vf_rss_flow_fld(struct ice_vsi *vsi)
 
 static const struct ice_rss_hash_cfg default_rss_cfgs[] = {
 	/* configure RSS for IPv4 with input set IP src/dst */
-	{ICE_FLOW_SEG_HDR_IPV4, ICE_FLOW_HASH_IPV4, ICE_RSS_ANY_HEADERS},
+	{ICE_FLOW_SEG_HDR_IPV4, ICE_FLOW_HASH_IPV4, ICE_RSS_ANY_HEADERS, false},
 	/* configure RSS for IPv6 with input set IPv6 src/dst */
-	{ICE_FLOW_SEG_HDR_IPV6, ICE_FLOW_HASH_IPV6, ICE_RSS_ANY_HEADERS},
+	{ICE_FLOW_SEG_HDR_IPV6, ICE_FLOW_HASH_IPV6, ICE_RSS_ANY_HEADERS, false},
 	/* configure RSS for tcp4 with input set IP src/dst, TCP src/dst */
 	{ICE_FLOW_SEG_HDR_TCP | ICE_FLOW_SEG_HDR_IPV4,
-				ICE_HASH_TCP_IPV4,  ICE_RSS_ANY_HEADERS},
+				ICE_HASH_TCP_IPV4,  ICE_RSS_ANY_HEADERS, false},
 	/* configure RSS for udp4 with input set IP src/dst, UDP src/dst */
 	{ICE_FLOW_SEG_HDR_UDP | ICE_FLOW_SEG_HDR_IPV4,
-				ICE_HASH_UDP_IPV4,  ICE_RSS_ANY_HEADERS},
+				ICE_HASH_UDP_IPV4,  ICE_RSS_ANY_HEADERS, false},
 	/* configure RSS for sctp4 with input set IP src/dst - only support
 	 * RSS on SCTPv4 on outer headers (non-tunneled)
 	 */
 	{ICE_FLOW_SEG_HDR_SCTP | ICE_FLOW_SEG_HDR_IPV4,
-		ICE_HASH_SCTP_IPV4, ICE_RSS_OUTER_HEADERS},
+		ICE_HASH_SCTP_IPV4, ICE_RSS_OUTER_HEADERS, false},
 	/* configure RSS for tcp6 with input set IPv6 src/dst, TCP src/dst */
 	{ICE_FLOW_SEG_HDR_TCP | ICE_FLOW_SEG_HDR_IPV6,
-				ICE_HASH_TCP_IPV6,  ICE_RSS_ANY_HEADERS},
+				ICE_HASH_TCP_IPV6,  ICE_RSS_ANY_HEADERS, false},
 	/* configure RSS for udp6 with input set IPv6 src/dst, UDP src/dst */
 	{ICE_FLOW_SEG_HDR_UDP | ICE_FLOW_SEG_HDR_IPV6,
-				ICE_HASH_UDP_IPV6,  ICE_RSS_ANY_HEADERS},
+				ICE_HASH_UDP_IPV6,  ICE_RSS_ANY_HEADERS, false},
 	/* configure RSS for sctp6 with input set IPv6 src/dst - only support
 	 * RSS on SCTPv6 on outer headers (non-tunneled)
 	 */
 	{ICE_FLOW_SEG_HDR_SCTP | ICE_FLOW_SEG_HDR_IPV6,
-		ICE_HASH_SCTP_IPV6, ICE_RSS_OUTER_HEADERS},
+		ICE_HASH_SCTP_IPV6, ICE_RSS_OUTER_HEADERS, false},
 	/* configure RSS for IPSEC ESP SPI with input set MAC_IPV4_SPI */
 	{ICE_FLOW_SEG_HDR_ESP,
-		ICE_FLOW_HASH_ESP_SPI, ICE_RSS_OUTER_HEADERS},
+		ICE_FLOW_HASH_ESP_SPI, ICE_RSS_OUTER_HEADERS, false},
 };
 
 /**
@@ -1652,7 +1652,7 @@ static const struct ice_rss_hash_cfg default_rss_cfgs[] = {
  */
 static void ice_vsi_set_rss_flow_fld(struct ice_vsi *vsi)
 {
-	u16 vsi_handle = vsi->idx, vsi_num = vsi->vsi_num;
+	u16 vsi_num = vsi->vsi_num;
 	struct ice_pf *pf = vsi->back;
 	struct ice_hw *hw = &pf->hw;
 	struct device *dev;
@@ -1668,10 +1668,11 @@ static void ice_vsi_set_rss_flow_fld(struct ice_vsi *vsi)
 	for (i = 0; i < ARRAY_SIZE(default_rss_cfgs); i++) {
 		const struct ice_rss_hash_cfg *cfg = &default_rss_cfgs[i];
 
-		status = ice_add_rss_cfg(hw, vsi_handle, cfg);
+		status = ice_add_rss_cfg(hw, vsi, cfg);
 		if (status)
-			dev_dbg(dev, "ice_add_rss_cfg failed, addl_hdrs = %x, hash_flds = %llx, hdr_type = %d\n",
-				cfg->addl_hdrs, cfg->hash_flds, cfg->hdr_type);
+			dev_dbg(dev, "ice_add_rss_cfg failed, addl_hdrs = %x, hash_flds = %llx, hdr_type = %d, symm = %d\n",
+				cfg->addl_hdrs, cfg->hash_flds,
+				cfg->hdr_type, cfg->symm);
 	}
 }
 
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 20fb53ea4e5b..437278a1ec0d 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -7682,6 +7682,50 @@ int ice_get_rss_key(struct ice_vsi *vsi, u8 *seed)
 	return status;
 }
 
+/**
+ * ice_set_rss_hfunc - Set RSS HASH function
+ * @vsi: Pointer to VSI structure
+ * @hfunc: hash function (ICE_AQ_VSI_Q_OPT_RSS_*)
+ *
+ * Returns 0 on success, negative on failure
+ */
+int ice_set_rss_hfunc(struct ice_vsi *vsi, u8 hfunc)
+{
+	struct ice_vsi_ctx *ctx;
+	int err;
+
+	if (hfunc == vsi->rss_hfunc)
+		return 0;
+
+	if (hfunc != ICE_AQ_VSI_Q_OPT_RSS_HASH_TPLZ &&
+	    hfunc != ICE_AQ_VSI_Q_OPT_RSS_HASH_SYM_TPLZ)
+		return -EOPNOTSUPP;
+
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ctx->info.valid_sections = cpu_to_le16(ICE_AQ_VSI_PROP_Q_OPT_VALID);
+	ctx->info.q_opt_rss = vsi->info.q_opt_rss;
+	ctx->info.q_opt_rss &= ~ICE_AQ_VSI_Q_OPT_RSS_HASH_M;
+	ctx->info.q_opt_rss |=
+		FIELD_PREP(ICE_AQ_VSI_Q_OPT_RSS_HASH_M, hfunc);
+	ctx->info.q_opt_tc = vsi->info.q_opt_tc;
+	ctx->info.q_opt_flags = vsi->info.q_opt_rss;
+
+	err = ice_update_vsi(&vsi->back->hw, vsi->idx, ctx, NULL);
+	if (err) {
+		dev_err(ice_pf_to_dev(vsi->back), "Failed to configure RSS hash for VSI %d, error %d\n",
+			vsi->vsi_num, err);
+	} else {
+		vsi->info.q_opt_rss = ctx->info.q_opt_rss;
+		vsi->rss_hfunc = hfunc;
+	}
+
+	kfree(ctx);
+	return err;
+}
+
 /**
  * ice_bridge_getlink - Get the hardware bridge mode
  * @skb: skb buff
diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.c b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
index f14aae2fe06c..1014679eb147 100644
--- a/drivers/net/ethernet/intel/ice/ice_virtchnl.c
+++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
@@ -710,6 +710,11 @@ static bool ice_vc_parse_rss_cfg(struct ice_hw *hw,
 	/* set outer layer RSS as default */
 	hash_cfg->hdr_type = ICE_RSS_OUTER_HEADERS;
 
+	if (rss_cfg->rss_algorithm == VIRTCHNL_RSS_ALG_TOEPLITZ_SYMMETRIC)
+		hash_cfg->symm = true;
+	else
+		hash_cfg->symm = false;
+
 	hf_list = ice_vc_hash_field_list;
 	hf_list_len = ARRAY_SIZE(ice_vc_hash_field_list);
 	hdr_list = ice_vc_hdr_list;
@@ -876,7 +881,7 @@ static int ice_vc_handle_rss_cfg(struct ice_vf *vf, u8 *msg, bool add)
 		}
 
 		if (add) {
-			if (ice_add_rss_cfg(hw, vsi->idx, &cfg)) {
+			if (ice_add_rss_cfg(hw, vsi, &cfg)) {
 				v_ret = VIRTCHNL_STATUS_ERR_PARAM;
 				dev_err(dev, "ice_add_rss_cfg failed for vsi = %d, v_ret = %d\n",
 					vsi->vsi_num, v_ret);
@@ -2616,7 +2621,7 @@ static int ice_vc_set_rss_hena(struct ice_vf *vf, u8 *msg)
 	}
 
 	if (vrh->hena) {
-		status = ice_add_avf_rss_cfg(&pf->hw, vsi->idx, vrh->hena);
+		status = ice_add_avf_rss_cfg(&pf->hw, vsi, vrh->hena);
 		v_ret = ice_err_to_virt_err(status);
 	}
 
diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
index 5cd143863b21..bcf5d85d5013 100644
--- a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
+++ b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
@@ -666,7 +666,7 @@ ice_vc_fdir_write_flow_prof(struct ice_vf *vf, enum ice_fltr_ptype flow,
 	}
 
 	ret = ice_flow_add_prof(hw, ICE_BLK_FD, ICE_FLOW_RX, seg,
-				tun + 1, &prof);
+				tun + 1, false, &prof);
 	if (ret) {
 		dev_dbg(dev, "Could not add VSI flow 0x%x for VF %d\n",
 			flow, vf->vf_id);
-- 
2.34.1

[PATCH net-next v4 6/6] iavf: enable symmetric RSS Toeplitz hash

[Ahmed Zaki]

Allow the VFs to support symmetric RSS for any flow type. The symmetric
RSS will not be supported on PFs not advertising the ADV RSS Offload
flag (ADV_RSS_SUPPORT()), for example the E700 series (i40e).

Reviewed-by: Madhu Chittim <madhu.chittim@intel.com>
Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com>
---
 .../net/ethernet/intel/iavf/iavf_adv_rss.c    |  8 +++++--
 .../net/ethernet/intel/iavf/iavf_adv_rss.h    |  3 ++-
 .../net/ethernet/intel/iavf/iavf_ethtool.c    | 22 +++++++++++++++----
 3 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/iavf/iavf_adv_rss.c b/drivers/net/ethernet/intel/iavf/iavf_adv_rss.c
index 6edbf134b73f..a9e1da35e248 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_adv_rss.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_adv_rss.c
@@ -95,17 +95,21 @@ iavf_fill_adv_rss_sctp_hdr(struct virtchnl_proto_hdr *hdr, u64 hash_flds)
  * @rss_cfg: the virtchnl message to be filled with RSS configuration setting
  * @packet_hdrs: the RSS configuration protocol header types
  * @hash_flds: the RSS configuration protocol hash fields
+ * @symm: if true, symmetric hash is required
  *
  * Returns 0 if the RSS configuration virtchnl message is filled successfully
  */
 int
 iavf_fill_adv_rss_cfg_msg(struct virtchnl_rss_cfg *rss_cfg,
-			  u32 packet_hdrs, u64 hash_flds)
+			  u32 packet_hdrs, u64 hash_flds, bool symm)
 {
 	struct virtchnl_proto_hdrs *proto_hdrs = &rss_cfg->proto_hdrs;
 	struct virtchnl_proto_hdr *hdr;
 
-	rss_cfg->rss_algorithm = VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
+	if (symm)
+		rss_cfg->rss_algorithm = VIRTCHNL_RSS_ALG_TOEPLITZ_SYMMETRIC;
+	else
+		rss_cfg->rss_algorithm = VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
 
 	proto_hdrs->tunnel_level = 0;	/* always outer layer */
 
diff --git a/drivers/net/ethernet/intel/iavf/iavf_adv_rss.h b/drivers/net/ethernet/intel/iavf/iavf_adv_rss.h
index 4d3be11af7aa..e31eb2afebea 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_adv_rss.h
+++ b/drivers/net/ethernet/intel/iavf/iavf_adv_rss.h
@@ -80,13 +80,14 @@ struct iavf_adv_rss {
 
 	u32 packet_hdrs;
 	u64 hash_flds;
+	bool symm;
 
 	struct virtchnl_rss_cfg cfg_msg;
 };
 
 int
 iavf_fill_adv_rss_cfg_msg(struct virtchnl_rss_cfg *rss_cfg,
-			  u32 packet_hdrs, u64 hash_flds);
+			  u32 packet_hdrs, u64 hash_flds, bool symm);
 struct iavf_adv_rss *
 iavf_find_adv_rss_cfg_by_hdrs(struct iavf_adapter *adapter, u32 packet_hdrs);
 void
diff --git a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
index 90397293525f..8b81c5510bae 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
@@ -1618,6 +1618,7 @@ iavf_set_adv_rss_hash_opt(struct iavf_adapter *adapter,
 	struct iavf_adv_rss *rss_old, *rss_new;
 	bool rss_new_add = false;
 	int count = 50, err = 0;
+	bool symm = false;
 	u64 hash_flds;
 	u32 hdrs;
 
@@ -1632,11 +1633,15 @@ iavf_set_adv_rss_hash_opt(struct iavf_adapter *adapter,
 	if (hash_flds == IAVF_ADV_RSS_HASH_INVALID)
 		return -EINVAL;
 
+	if (cmd->data & RXH_SYMMETRIC_XOR)
+		symm = true;
+
 	rss_new = kzalloc(sizeof(*rss_new), GFP_KERNEL);
 	if (!rss_new)
 		return -ENOMEM;
 
-	if (iavf_fill_adv_rss_cfg_msg(&rss_new->cfg_msg, hdrs, hash_flds)) {
+	if (iavf_fill_adv_rss_cfg_msg(&rss_new->cfg_msg, hdrs, hash_flds,
+				      symm)) {
 		kfree(rss_new);
 		return -EINVAL;
 	}
@@ -1655,9 +1660,11 @@ iavf_set_adv_rss_hash_opt(struct iavf_adapter *adapter,
 	if (rss_old) {
 		if (rss_old->state != IAVF_ADV_RSS_ACTIVE) {
 			err = -EBUSY;
-		} else if (rss_old->hash_flds != hash_flds) {
+		} else if (rss_old->hash_flds != hash_flds ||
+			   rss_old->symm != symm) {
 			rss_old->state = IAVF_ADV_RSS_ADD_REQUEST;
 			rss_old->hash_flds = hash_flds;
+			rss_old->symm = symm;
 			memcpy(&rss_old->cfg_msg, &rss_new->cfg_msg,
 			       sizeof(rss_new->cfg_msg));
 			adapter->aq_required |= IAVF_FLAG_AQ_ADD_ADV_RSS_CFG;
@@ -1669,6 +1676,7 @@ iavf_set_adv_rss_hash_opt(struct iavf_adapter *adapter,
 		rss_new->state = IAVF_ADV_RSS_ADD_REQUEST;
 		rss_new->packet_hdrs = hdrs;
 		rss_new->hash_flds = hash_flds;
+		rss_new->symm = symm;
 		list_add_tail(&rss_new->list, &adapter->adv_rss_list_head);
 		adapter->aq_required |= IAVF_FLAG_AQ_ADD_ADV_RSS_CFG;
 	}
@@ -1698,6 +1706,7 @@ iavf_get_adv_rss_hash_opt(struct iavf_adapter *adapter,
 {
 	struct iavf_adv_rss *rss;
 	u64 hash_flds;
+	bool symm;
 	u32 hdrs;
 
 	if (!ADV_RSS_SUPPORT(adapter))
@@ -1711,10 +1720,12 @@ iavf_get_adv_rss_hash_opt(struct iavf_adapter *adapter,
 
 	spin_lock_bh(&adapter->adv_rss_lock);
 	rss = iavf_find_adv_rss_cfg_by_hdrs(adapter, hdrs);
-	if (rss)
+	if (rss) {
 		hash_flds = rss->hash_flds;
-	else
+		symm = rss->symm;
+	} else {
 		hash_flds = IAVF_ADV_RSS_HASH_INVALID;
+	}
 	spin_unlock_bh(&adapter->adv_rss_lock);
 
 	if (hash_flds == IAVF_ADV_RSS_HASH_INVALID)
@@ -1738,6 +1749,9 @@ iavf_get_adv_rss_hash_opt(struct iavf_adapter *adapter,
 			 IAVF_ADV_RSS_HASH_FLD_SCTP_DST_PORT))
 		cmd->data |= (u64)RXH_L4_B_2_3;
 
+	if (symm)
+		cmd->data |= (u64)RXH_SYMMETRIC_XOR;
+
 	return 0;
 }
 
-- 
2.34.1