diff mbox series

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

Message ID 20231010200437.9794-2-ahmed.zaki@intel.com
State Handled Elsewhere
Headers show
Series Support symmetric RSS (Toeplitz) hash | expand

Commit Message

Ahmed Zaki Oct. 10, 2023, 8:04 p.m. UTC
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         | 17 +++++++++--------
 net/ethtool/ioctl.c                  | 11 +++++++++++
 3 files changed, 26 insertions(+), 8 deletions(-)

Comments

Willem de Bruijn Oct. 10, 2023, 8:40 p.m. UTC | #1
On Tue, Oct 10, 2023 at 4:05 PM Ahmed Zaki <ahmed.zaki@intel.com> 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         | 17 +++++++++--------
>  net/ethtool/ioctl.c                  | 11 +++++++++++
>  3 files changed, 26 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).
> +

Maybe add a short ethtool example?

>  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..b9ee667ad7e5 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -2018,14 +2018,15 @@ 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 */
> +#define        RXH_SYMMETRIC_XOR       (1 << 30)
> +#define        RXH_DISCARD             (1 << 31)

Are these indentation changes intentional?
Ahmed Zaki Oct. 10, 2023, 9:34 p.m. UTC | #2
On 2023-10-10 14:40, Willem de Bruijn wrote:
> On Tue, Oct 10, 2023 at 4:05 PM Ahmed Zaki<ahmed.zaki@intel.com>  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         | 17 +++++++++--------
>>   net/ethtool/ioctl.c                  | 11 +++++++++++
>>   3 files changed, 26 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).
>> +
> Maybe add a short ethtool example?

Same example as in commit message is OK?

AFAIK, the "ethtool" patch has to be sent after this series is accepted. 
So I am not 100% sure of how the ethtool side will look like, but I can 
add the line above to Doc.


>
>>   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..b9ee667ad7e5 100644
>> --- a/include/uapi/linux/ethtool.h
>> +++ b/include/uapi/linux/ethtool.h
>> @@ -2018,14 +2018,15 @@ 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 */
>> +#define        RXH_SYMMETRIC_XOR       (1 << 30)
>> +#define        RXH_DISCARD             (1 << 31)
> Are these indentation changes intentional?

Yes, for alignment ("RXH_SYMMETRIC_XOR" is too long).
Ahmed Zaki Oct. 11, 2023, 2:49 p.m. UTC | #3
[ Resend - rejected by netdev and linux-doc MLs for HTML content]


On 2023-10-10 14:40, Willem de Bruijn wrote:
> On Tue, Oct 10, 2023 at 4:05 PM Ahmed Zaki <ahmed.zaki@intel.com> 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         | 17 +++++++++--------
>>   net/ethtool/ioctl.c                  | 11 +++++++++++
>>   3 files changed, 26 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).
>> +
> 
> Maybe add a short ethtool example?

Same example as in commit message is OK?

AFAIK, the "ethtool" patch has to be sent after this series is accepted. 
So I am not 100% sure of how the ethtool side will look like, but I can 
add the line above to Doc.


> 
>>   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..b9ee667ad7e5 100644
>> --- a/include/uapi/linux/ethtool.h
>> +++ b/include/uapi/linux/ethtool.h
>> @@ -2018,14 +2018,15 @@ 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 */
>> +#define        RXH_SYMMETRIC_XOR       (1 << 30)
>> +#define        RXH_DISCARD             (1 << 31)
> 
> Are these indentation changes intentional?

Yes, for alignment ("RXH_SYMMETRIC_XOR" is too long).
Willem de Bruijn Oct. 11, 2023, 10:45 p.m. UTC | #4
On Tue, Oct 10, 2023 at 5:34 PM Ahmed Zaki <ahmed.zaki@intel.com> wrote:
>
>
> On 2023-10-10 14:40, Willem de Bruijn wrote:
>
> On Tue, Oct 10, 2023 at 4:05 PM Ahmed Zaki <ahmed.zaki@intel.com> 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         | 17 +++++++++--------
>  net/ethtool/ioctl.c                  | 11 +++++++++++
>  3 files changed, 26 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).
> +
>
> Maybe add a short ethtool example?
>
> Same example as in commit message is OK?
>
> AFAIK, the "ethtool" patch has to be sent after this series is accepted. So I am not 100% sure of how the ethtool side will look like, but I can add the line above to Doc.

Good point. Then let's not if the API is not final yet.
>
>
>  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..b9ee667ad7e5 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -2018,14 +2018,15 @@ 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 */
> +#define        RXH_SYMMETRIC_XOR       (1 << 30)
> +#define        RXH_DISCARD             (1 << 31)
>
> Are these indentation changes intentional?
>
>
> Yes, for alignment ("RXH_SYMMETRIC_XOR" is too long).

I think it's preferable to not touch other lines. Among others, that
messes up git blame. But it's subjective. Follow your preference if no
one else chimes in.
Ahmed Zaki Oct. 14, 2023, 12:19 p.m. UTC | #5
On 2023-10-11 16:45, Willem de Bruijn wrote:
> On Tue, Oct 10, 2023 at 5:34 PM Ahmed Zaki <ahmed.zaki@intel.com> wrote:
>>
>>
>> On 2023-10-10 14:40, Willem de Bruijn wrote:
>>
>> On Tue, Oct 10, 2023 at 4:05 PM Ahmed Zaki <ahmed.zaki@intel.com> 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         | 17 +++++++++--------
>>   net/ethtool/ioctl.c                  | 11 +++++++++++
>>   3 files changed, 26 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).
>> +
>>
>> Maybe add a short ethtool example?
>>
>> Same example as in commit message is OK?
>>
>> AFAIK, the "ethtool" patch has to be sent after this series is accepted. So I am not 100% sure of how the ethtool side will look like, but I can add the line above to Doc.
> 
> Good point. Then let's not if the API is not final yet.
>>
>>
>>   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..b9ee667ad7e5 100644
>> --- a/include/uapi/linux/ethtool.h
>> +++ b/include/uapi/linux/ethtool.h
>> @@ -2018,14 +2018,15 @@ 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 */
>> +#define        RXH_SYMMETRIC_XOR       (1 << 30)
>> +#define        RXH_DISCARD             (1 << 31)
>>
>> Are these indentation changes intentional?
>>
>>
>> Yes, for alignment ("RXH_SYMMETRIC_XOR" is too long).
> 
> I think it's preferable to not touch other lines. Among others, that
> messes up git blame. But it's subjective. Follow your preference if no
> one else chimes in.

Jakub,

Sorry for late reply, I was off for few days.

I'd like to keep this version, I don't see any other comments that needs 
to be addressed. Can you accept this or need a v4/rebase ?

Ahmed
Jakub Kicinski Oct. 16, 2023, 3:02 p.m. UTC | #6
On Sat, 14 Oct 2023 06:19:54 -0600 Ahmed Zaki wrote:
> >> +#define        RXH_SYMMETRIC_XOR       (1 << 30)
> >> +#define        RXH_DISCARD             (1 << 31)
> >>
> >> Are these indentation changes intentional?
> >>
> >>
> >> Yes, for alignment ("RXH_SYMMETRIC_XOR" is too long).  
> > 
> > I think it's preferable to not touch other lines. Among others, that
> > messes up git blame. But it's subjective. Follow your preference if no
> > one else chimes in.  
> 
> Jakub,
> 
> Sorry for late reply, I was off for few days.
> 
> I'd like to keep this version, I don't see any other comments that needs 
> to be addressed. Can you accept this or need a v4/rebase ?

I think you should add a comment above the define explaining what
"symmetric-xor" is. Is this correct?

/* XOR corresponding source and destination fields, both copies 
 * of the XOR'ed fields are fed into the RSS and RXHASH calculation.
 */
Ahmed Zaki Oct. 16, 2023, 3:27 p.m. UTC | #7
On 2023-10-16 09:02, Jakub Kicinski wrote:
> On Sat, 14 Oct 2023 06:19:54 -0600 Ahmed Zaki wrote:
>>>> +#define        RXH_SYMMETRIC_XOR       (1 << 30)
>>>> +#define        RXH_DISCARD             (1 << 31)
>>>>
>>>> Are these indentation changes intentional?
>>>>
>>>>
>>>> Yes, for alignment ("RXH_SYMMETRIC_XOR" is too long).
>>>
>>> I think it's preferable to not touch other lines. Among others, that
>>> messes up git blame. But it's subjective. Follow your preference if no
>>> one else chimes in.
>>
>> Jakub,
>>
>> Sorry for late reply, I was off for few days.
>>
>> I'd like to keep this version, I don't see any other comments that needs
>> to be addressed. Can you accept this or need a v4/rebase ?
> 
> I think you should add a comment above the define explaining what
> "symmetric-xor" is. Is this correct?
> 
> /* XOR corresponding source and destination fields, both copies
>   * of the XOR'ed fields are fed into the RSS and RXHASH calculation.
>   */

Sounds good. I will send a new version shortly.
diff mbox series

Patch

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..b9ee667ad7e5 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -2018,14 +2018,15 @@  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 */
+#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;