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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Oct 10, 2023 at 5:34 PM Ahmed Zaki <ahmed.zaki@xxxxxxxxx> wrote:
>
>
> On 2023-10-10 14:40, Willem de Bruijn wrote:
>
> On Tue, Oct 10, 2023 at 4:05 PM Ahmed Zaki <ahmed.zaki@xxxxxxxxx> 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@xxxxxxxxx>
> Signed-off-by: Ahmed Zaki <ahmed.zaki@xxxxxxxxx>
> ---
>  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.





[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux