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@xxxxxxxxx> > Signed-off-by: Ahmed Zaki <ahmed.zaki@xxxxxxxxx> > --- > 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.
