Message ID | 20231006224726.443836-2-ahmed.zaki@intel.com |
---|---|
State | Handled Elsewhere |
Headers | show |
Series | Support symmetric RSS (Toeplitz) hash | expand |
On Fri, 6 Oct 2023 16:47:21 -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. > > 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 > > or turn symmetry off (asymmetric) by: > > # ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n Thanks for the changes, code looks good! The question left unanswered is whether we should care about the exact implementation of the symmetry (xor, xor duplicate, sort fields). Toeplitz-based RSS is very precisely specified, so we may want to carry that precision into the symmetric behavior. I have a weak preference to do so... but no willingness to argue with you, so let me put Willem on the spot and have him make a decision :) Please make sure to CC Willem and anyone else who commented on previous revisions on future versions!
On Fri, Oct 6, 2023 at 7:22 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Fri, 6 Oct 2023 16:47:21 -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. > > > > 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 > > > > or turn symmetry off (asymmetric) by: > > > > # ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n > > Thanks for the changes, code looks good! > > The question left unanswered is whether we should care about the exact > implementation of the symmetry (xor, xor duplicate, sort fields). > Toeplitz-based RSS is very precisely specified, so we may want to carry > that precision into the symmetric behavior. I have a weak preference > to do so... but no willingness to argue with you, so let me put Willem > on the spot and have him make a decision :) I do have a stronger willingness to argue, thanks ;-) Can we give a more precise name, such as symmetric-xor? In case another device would implement another mode, such as the symmetric toeplitz of __flow_hash_consistentify, it would be good to be able to discern the modes.
On 2023-10-07 03:01, Willem de Bruijn wrote: > On Fri, Oct 6, 2023 at 7:22 PM Jakub Kicinski <kuba@kernel.org> wrote: >> On Fri, 6 Oct 2023 16:47:21 -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. >>> >>> 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 >>> >>> or turn symmetry off (asymmetric) by: >>> >>> # ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n >> Thanks for the changes, code looks good! >> >> The question left unanswered is whether we should care about the exact >> implementation of the symmetry (xor, xor duplicate, sort fields). >> Toeplitz-based RSS is very precisely specified, so we may want to carry >> that precision into the symmetric behavior. I have a weak preference >> to do so... but no willingness to argue with you, so let me put Willem >> on the spot and have him make a decision :) > I do have a stronger willingness to argue, thanks ;-) > > Can we give a more precise name, such as symmetric-xor? In case > another device would implement another mode, such as the symmetric > toeplitz of __flow_hash_consistentify, it would be good to be able to > discern the modes. I agree that implementation matters. I changed "symmetric" to "symmetric-xor" in v3. Thanks for the review.
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..bf67c8094ae0 100644 --- a/include/uapi/linux/ethtool.h +++ b/include/uapi/linux/ethtool.h @@ -2025,6 +2025,7 @@ static inline int ethtool_validate_duplex(__u8 duplex) #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 (1 << 30) #define RXH_DISCARD (1 << 31) #define RX_CLS_FLOW_DISC 0xffffffffffffffffULL diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index 0b0ce4f81c01..44742653a4bd 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) && + ((info.data & ~(RXH_SYMMETRIC | 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;