| Message ID | 20230823164831.3284341-2-ahmed.zaki@intel.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Support Symmetric Toeplitz RSS hash | expand |
On 23 Aug 10:48, 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. > Can you please shed more light on the use case and configuration? Where do you expect the same flow/connection rx/tx to be received by the same rxq in a nic driver? >Allow ethtool to support symmetric Toeplitz algorithm. A user can set the >RSS function of the netdevice via: > # ethtool -X eth0 hfunc symmetric_toeplitz > What is the expectation of the symmetric toeplitz hash, how do you achieve that? by sorting packet fields? which fields? Can you please provide a link to documentation/spec? We should make sure all vendors agree on implementation and expectation of the symmetric hash function. >Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com> >--- > include/linux/ethtool.h | 4 +++- > net/ethtool/common.c | 1 + > 2 files changed, 4 insertions(+), 1 deletion(-) > >diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h >index 62b61527bcc4..9a8e1fb7170d 100644 >--- a/include/linux/ethtool.h >+++ b/include/linux/ethtool.h >@@ -60,10 +60,11 @@ enum { > 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 */ >+ ETH_RSS_HASH_SYM_TOP_BIT, /* Configurable RSS hash function - Symmetric Toeplitz */ > > /* > * Add your fresh new hash function bits above and remember to update >- * rss_hash_func_strings[] in ethtool.c >+ * rss_hash_func_strings[] in ethtool/common.c > */ > ETH_RSS_HASH_FUNCS_COUNT > }; >@@ -108,6 +109,7 @@ enum ethtool_supported_ring_param { > #define __ETH_RSS_HASH(name) __ETH_RSS_HASH_BIT(ETH_RSS_HASH_##name##_BIT) > > #define ETH_RSS_HASH_TOP __ETH_RSS_HASH(TOP) >+#define ETH_RSS_HASH_SYM_TOP __ETH_RSS_HASH(SYM_TOP) > #define ETH_RSS_HASH_XOR __ETH_RSS_HASH(XOR) > #define ETH_RSS_HASH_CRC32 __ETH_RSS_HASH(CRC32) > >diff --git a/net/ethtool/common.c b/net/ethtool/common.c >index f5598c5f50de..a0e0c6b2980e 100644 >--- a/net/ethtool/common.c >+++ b/net/ethtool/common.c >@@ -81,6 +81,7 @@ rss_hash_func_strings[ETH_RSS_HASH_FUNCS_COUNT][ETH_GSTRING_LEN] = { > [ETH_RSS_HASH_TOP_BIT] = "toeplitz", > [ETH_RSS_HASH_XOR_BIT] = "xor", > [ETH_RSS_HASH_CRC32_BIT] = "crc32", >+ [ETH_RSS_HASH_SYM_TOP_BIT] = "symmetric_toeplitz", > }; > > const char >-- >2.39.2 > >
On 2023-08-23 13:45, Saeed Mahameed wrote: > On 23 Aug 10:48, 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. >> > > Can you please shed more light on the use case and configuration? > Where do you expect the same flow/connection rx/tx to be received by the > same rxq in a nic driver? The use case is usually an application running on a intermediate server (not an endpoint of the flow) monitoring and reading both directions of the flow. Applications like intrusion detection systems or user-space state-full firewalls. For best CPU and cache efficiencies, we would need both flows to land on the same rx queue of that intermediate server. The paper in [1] gives more background on Symmetric Toeplitz (but imposes some restrictions on the LUT keys to get the hash symmetry). > >> Allow ethtool to support symmetric Toeplitz algorithm. A user can set >> the >> RSS function of the netdevice via: >> # ethtool -X eth0 hfunc symmetric_toeplitz >> > > What is the expectation of the symmetric toeplitz hash, how do you > achieve > that? by sorting packet fields? which fields? > > Can you please provide a link to documentation/spec? > We should make sure all vendors agree on implementation and > expectation of > the symmetric hash function. The way the Intel NICs are achieving this hash symmetry is by XORing the source and destination values of the IP and L4 ports and then feeding these values to the regular Toeplitz (in-tree) hash algorithm. For example, for UDP/IPv4, the input fields for the Toeplitz hash would be: (SRC_IP, DST_IP, SRC_PORT, DST_PORT) If symmetric Toeplitz is set, the NIC XOR the src and dst fields: (SRC_IP^DST_IP , SRC_IP^DST_IP, SRC_PORT^DST_PORT, SRC_PORT^DST_PORT) This way, the output hash would be the same for both flow directions. Same is applicable for IPv6, TCP and SCTP. Regarding the documentation, the above is available in our public datasheets [2]. In the final version, I can add similar explanation in the headers (kdoc) and under "Documentation/networking/" so that there is a clear understanding of the algorithm. [1] https://www.ndsl.kaist.edu/~kyoungsoo/papers/TR-symRSS.pdf [2] E810 datasheet: 7.10.10.2 : Symmetric Hash https://www.intel.com/content/www/us/en/content-details/613875/intel-ethernet-controller-e810-datasheet.html > >> Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com> >> --- >> include/linux/ethtool.h | 4 +++- >> net/ethtool/common.c | 1 + >> 2 files changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h >> index 62b61527bcc4..9a8e1fb7170d 100644 >> --- a/include/linux/ethtool.h >> +++ b/include/linux/ethtool.h >> @@ -60,10 +60,11 @@ enum { >> 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 */ >> + ETH_RSS_HASH_SYM_TOP_BIT, /* Configurable RSS hash function - >> Symmetric Toeplitz */ >> >> /* >> * Add your fresh new hash function bits above and remember to >> update >> - * rss_hash_func_strings[] in ethtool.c >> + * rss_hash_func_strings[] in ethtool/common.c >> */ >> ETH_RSS_HASH_FUNCS_COUNT >> }; >> @@ -108,6 +109,7 @@ enum ethtool_supported_ring_param { >> #define __ETH_RSS_HASH(name) >> __ETH_RSS_HASH_BIT(ETH_RSS_HASH_##name##_BIT) >> >> #define ETH_RSS_HASH_TOP __ETH_RSS_HASH(TOP) >> +#define ETH_RSS_HASH_SYM_TOP __ETH_RSS_HASH(SYM_TOP) >> #define ETH_RSS_HASH_XOR __ETH_RSS_HASH(XOR) >> #define ETH_RSS_HASH_CRC32 __ETH_RSS_HASH(CRC32) >> >> diff --git a/net/ethtool/common.c b/net/ethtool/common.c >> index f5598c5f50de..a0e0c6b2980e 100644 >> --- a/net/ethtool/common.c >> +++ b/net/ethtool/common.c >> @@ -81,6 +81,7 @@ >> rss_hash_func_strings[ETH_RSS_HASH_FUNCS_COUNT][ETH_GSTRING_LEN] = { >> [ETH_RSS_HASH_TOP_BIT] = "toeplitz", >> [ETH_RSS_HASH_XOR_BIT] = "xor", >> [ETH_RSS_HASH_CRC32_BIT] = "crc32", >> + [ETH_RSS_HASH_SYM_TOP_BIT] = "symmetric_toeplitz", >> }; >> >> const char >> -- >> 2.39.2 >> >>
CC Willem On Wed, 23 Aug 2023 10:48:29 -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. > > Allow ethtool to support symmetric Toeplitz algorithm. A user can set the > RSS function of the netdevice via: > # ethtool -X eth0 hfunc symmetric_toeplitz Looks fairly reasonable, but there are two questions we need to answer: - what do we do if RXH config includes fields which are by definition not symmetric (l2 DA or in the future flow label)? - my initial thought was the same as Saeed's - that the fields are sorted, so how do we inform user about the exact implementation? One way to fix both problems would be to, instead of changing the hash function, change the RXH config. Add new "xor-ed" fields there. Another would be to name the function "XORSYM_TOP" and make the core check that it cannot be combined with uni-dir fields? I like the first option more. Either way, please make sure to add docs, and extend the toeplitz test for this. > diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h > index 62b61527bcc4..9a8e1fb7170d 100644 > --- a/include/linux/ethtool.h > +++ b/include/linux/ethtool.h > @@ -60,10 +60,11 @@ enum { > 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 */ > + ETH_RSS_HASH_SYM_TOP_BIT, /* Configurable RSS hash function - Symmetric Toeplitz */ > > /* > * Add your fresh new hash function bits above and remember to update > - * rss_hash_func_strings[] in ethtool.c > + * rss_hash_func_strings[] in ethtool/common.c > */ > ETH_RSS_HASH_FUNCS_COUNT > }; > @@ -108,6 +109,7 @@ enum ethtool_supported_ring_param { > #define __ETH_RSS_HASH(name) __ETH_RSS_HASH_BIT(ETH_RSS_HASH_##name##_BIT) > > #define ETH_RSS_HASH_TOP __ETH_RSS_HASH(TOP) > +#define ETH_RSS_HASH_SYM_TOP __ETH_RSS_HASH(SYM_TOP) > #define ETH_RSS_HASH_XOR __ETH_RSS_HASH(XOR) > #define ETH_RSS_HASH_CRC32 __ETH_RSS_HASH(CRC32) > > diff --git a/net/ethtool/common.c b/net/ethtool/common.c > index f5598c5f50de..a0e0c6b2980e 100644 > --- a/net/ethtool/common.c > +++ b/net/ethtool/common.c > @@ -81,6 +81,7 @@ rss_hash_func_strings[ETH_RSS_HASH_FUNCS_COUNT][ETH_GSTRING_LEN] = { > [ETH_RSS_HASH_TOP_BIT] = "toeplitz", > [ETH_RSS_HASH_XOR_BIT] = "xor", > [ETH_RSS_HASH_CRC32_BIT] = "crc32", > + [ETH_RSS_HASH_SYM_TOP_BIT] = "symmetric_toeplitz", > }; > > const char
On 24 Aug 07:14, Ahmed Zaki wrote: > >On 2023-08-23 13:45, Saeed Mahameed wrote: >>On 23 Aug 10:48, 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. >>> ... >> >>What is the expectation of the symmetric toeplitz hash, how do you >>achieve >>that? by sorting packet fields? which fields? >> >>Can you please provide a link to documentation/spec? >>We should make sure all vendors agree on implementation and >>expectation of >>the symmetric hash function. > >The way the Intel NICs are achieving this hash symmetry is by XORing >the source and destination values of the IP and L4 ports and then >feeding these values to the regular Toeplitz (in-tree) hash algorithm. > >For example, for UDP/IPv4, the input fields for the Toeplitz hash would be: > >(SRC_IP, DST_IP, SRC_PORT, DST_PORT) > So you mangle the input. This is different than the paper you referenced below which doesn't change the input but it modifies the RSS algorithm and uses a special hash key. >If symmetric Toeplitz is set, the NIC XOR the src and dst fields: > >(SRC_IP^DST_IP , SRC_IP^DST_IP, SRC_PORT^DST_PORT, SRC_PORT^DST_PORT) > >This way, the output hash would be the same for both flow directions. >Same is applicable for IPv6, TCP and SCTP. > I understand the motivation, I just want to make sure the interpretation is clear, I agree with Jakub, we should use a clear name for the ethtool parameter or allow users to select "xor-ed"/"sorted" fields as Jakub suggested. >Regarding the documentation, the above is available in our public >datasheets [2]. In the final version, I can add similar explanation in >the headers (kdoc) and under "Documentation/networking/" so that there >is a clear understanding of the algorithm. > > >[1] https://www.ndsl.kaist.edu/~kyoungsoo/papers/TR-symRSS.pdf > >[2] E810 datasheet: 7.10.10.2 : Symmetric Hash > >https://www.intel.com/content/www/us/en/content-details/613875/intel-ethernet-controller-e810-datasheet.html > This document doesn't mention anything about implementation.
On 2023-08-24 12:14, Jakub Kicinski wrote: > CC Willem > > On Wed, 23 Aug 2023 10:48:29 -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. >> >> Allow ethtool to support symmetric Toeplitz algorithm. A user can set the >> RSS function of the netdevice via: >> # ethtool -X eth0 hfunc symmetric_toeplitz > Looks fairly reasonable, but there are two questions we need to answer: > - what do we do if RXH config includes fields which are by definition > not symmetric (l2 DA or in the future flow label)? > - my initial thought was the same as Saeed's - that the fields are > sorted, so how do we inform user about the exact implementation? > > One way to fix both problems would be to, instead of changing the hash > function, change the RXH config. Add new "xor-ed" fields there. > > Another would be to name the function "XORSYM_TOP" and make the core > check that it cannot be combined with uni-dir fields? > > I like the first option more. > > Either way, please make sure to add docs, and extend the toeplitz test > for this. When "Symmetric Toeplitz" is set in the NIC, the H/W will yield the same hash as the regular Toeplitz for protocol types that do not have such symmetric fields in both directions (i.e. there will be no RSS hash symmetry and the TX/RX traffic will land on different Rx queues). The goal of this series is to enable the "default" behavior of the whole device ("-X hfunc") to be the symmetric hash (again, only for protocols that have symmetric src/dst counterparts). If I understand the first option correctly, the user would need to manually configure all RXH fields for all flow types (tcp4, udp4, sctp4, tcp6, ..etc), to get symmetric RSS on them, instead of the proposed single "-X" command? The second option is closer to what I had in mind. We can re-name and provide any details. I agree that we will need to take care of some cases like if the user removes only "source IP" or "destination port" from the hash fields, without that field's counterpart (we can prevent this, or show a warning, ..etc). I was planning to address that in a follow-up series; ie. handling the "ethtool -U rx-flow-hash". Do you want that to be included in the same series as well? > >> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h >> index 62b61527bcc4..9a8e1fb7170d 100644 >> --- a/include/linux/ethtool.h >> +++ b/include/linux/ethtool.h >> @@ -60,10 +60,11 @@ enum { >> 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 */ >> + ETH_RSS_HASH_SYM_TOP_BIT, /* Configurable RSS hash function - Symmetric Toeplitz */ >> >> /* >> * Add your fresh new hash function bits above and remember to update >> - * rss_hash_func_strings[] in ethtool.c >> + * rss_hash_func_strings[] in ethtool/common.c >> */ >> ETH_RSS_HASH_FUNCS_COUNT >> }; >> @@ -108,6 +109,7 @@ enum ethtool_supported_ring_param { >> #define __ETH_RSS_HASH(name) __ETH_RSS_HASH_BIT(ETH_RSS_HASH_##name##_BIT) >> >> #define ETH_RSS_HASH_TOP __ETH_RSS_HASH(TOP) >> +#define ETH_RSS_HASH_SYM_TOP __ETH_RSS_HASH(SYM_TOP) >> #define ETH_RSS_HASH_XOR __ETH_RSS_HASH(XOR) >> #define ETH_RSS_HASH_CRC32 __ETH_RSS_HASH(CRC32) >> >> diff --git a/net/ethtool/common.c b/net/ethtool/common.c >> index f5598c5f50de..a0e0c6b2980e 100644 >> --- a/net/ethtool/common.c >> +++ b/net/ethtool/common.c >> @@ -81,6 +81,7 @@ rss_hash_func_strings[ETH_RSS_HASH_FUNCS_COUNT][ETH_GSTRING_LEN] = { >> [ETH_RSS_HASH_TOP_BIT] = "toeplitz", >> [ETH_RSS_HASH_XOR_BIT] = "xor", >> [ETH_RSS_HASH_CRC32_BIT] = "crc32", >> + [ETH_RSS_HASH_SYM_TOP_BIT] = "symmetric_toeplitz", >> }; >> >> const char Thanks, Ahmed
On 2023-08-24 12:36, Saeed Mahameed wrote: > On 24 Aug 07:14, Ahmed Zaki wrote: >> >> On 2023-08-23 13:45, Saeed Mahameed wrote: >>> On 23 Aug 10:48, 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. >>>> > > ... > >>> >>> What is the expectation of the symmetric toeplitz hash, how do you >>> achieve >>> that? by sorting packet fields? which fields? >>> >>> Can you please provide a link to documentation/spec? >>> We should make sure all vendors agree on implementation and >>> expectation of >>> the symmetric hash function. >> >> The way the Intel NICs are achieving this hash symmetry is by XORing >> the source and destination values of the IP and L4 ports and then >> feeding these values to the regular Toeplitz (in-tree) hash algorithm. >> >> For example, for UDP/IPv4, the input fields for the Toeplitz hash >> would be: >> >> (SRC_IP, DST_IP, SRC_PORT, DST_PORT) >> > > So you mangle the input. This is different than the paper you > referenced below which doesn't change the input but it modifies the RSS > algorithm and uses a special hash key. > >> If symmetric Toeplitz is set, the NIC XOR the src and dst fields: >> >> (SRC_IP^DST_IP , SRC_IP^DST_IP, SRC_PORT^DST_PORT, SRC_PORT^DST_PORT) >> >> This way, the output hash would be the same for both flow directions. >> Same is applicable for IPv6, TCP and SCTP. >> > > I understand the motivation, I just want to make sure the > interpretation is > clear, I agree with Jakub, we should use a clear name for the ethtool > parameter or allow users to select "xor-ed"/"sorted" fields as Jakub > suggested. >> Regarding the documentation, the above is available in our public >> datasheets [2]. In the final version, I can add similar explanation >> in the headers (kdoc) and under "Documentation/networking/" so that >> there is a clear understanding of the algorithm. >> >> >> [1] https://www.ndsl.kaist.edu/~kyoungsoo/papers/TR-symRSS.pdf >> >> [2] E810 datasheet: 7.10.10.2 : Symmetric Hash >> >> https://www.intel.com/content/www/us/en/content-details/613875/intel-ethernet-controller-e810-datasheet.html >> >> > > This document doesn't mention anything about implementation. It has all the info regarding which fields are XOR'd using which registers and so on. The hash algorithm itself is the standard Toeplitz, also on section 7.10.10.2.
Ahmed Zaki wrote: > > On 2023-08-24 12:36, Saeed Mahameed wrote: > > On 24 Aug 07:14, Ahmed Zaki wrote: > >> > >> On 2023-08-23 13:45, Saeed Mahameed wrote: > >>> On 23 Aug 10:48, 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. > >>>> > > > > ... > > > >>> > >>> What is the expectation of the symmetric toeplitz hash, how do you > >>> achieve > >>> that? by sorting packet fields? which fields? > >>> > >>> Can you please provide a link to documentation/spec? > >>> We should make sure all vendors agree on implementation and > >>> expectation of > >>> the symmetric hash function. > >> > >> The way the Intel NICs are achieving this hash symmetry is by XORing > >> the source and destination values of the IP and L4 ports and then > >> feeding these values to the regular Toeplitz (in-tree) hash algorithm. > >> > >> For example, for UDP/IPv4, the input fields for the Toeplitz hash > >> would be: > >> > >> (SRC_IP, DST_IP, SRC_PORT, DST_PORT) > >> > > > > So you mangle the input. This is different than the paper you > > referenced below which doesn't change the input but it modifies the RSS > > algorithm and uses a special hash key. > > > >> If symmetric Toeplitz is set, the NIC XOR the src and dst fields: > >> > >> (SRC_IP^DST_IP , SRC_IP^DST_IP, SRC_PORT^DST_PORT, SRC_PORT^DST_PORT) > >> > >> This way, the output hash would be the same for both flow directions. > >> Same is applicable for IPv6, TCP and SCTP. > >> > > > > I understand the motivation, I just want to make sure the > > interpretation is > > clear, I agree with Jakub, we should use a clear name for the ethtool > > parameter or allow users to select "xor-ed"/"sorted" fields as Jakub > > suggested. > >> Regarding the documentation, the above is available in our public > >> datasheets [2]. In the final version, I can add similar explanation > >> in the headers (kdoc) and under "Documentation/networking/" so that > >> there is a clear understanding of the algorithm. Please do define the behavior. When I hear symmetric Toeplitz, my initial assumption was also sorted fields, as implemented in __flow_hash_consistentify. If this is something else, agreed that that is good to make crystal clear in name and somewhere in the kernel Documentation. xor-symmetric hash?
On Thu, 24 Aug 2023 16:55:40 -0600 Ahmed Zaki wrote: > When "Symmetric Toeplitz" is set in the NIC, the H/W will yield the same > hash as the regular Toeplitz for protocol types that do not have such > symmetric fields in both directions (i.e. there will be no RSS hash > symmetry and the TX/RX traffic will land on different Rx queues). > > The goal of this series is to enable the "default" behavior of the whole > device ("-X hfunc") to be the symmetric hash (again, only for protocols > that have symmetric src/dst counterparts). If I understand the first > option correctly, the user would need to manually configure all RXH > fields for all flow types (tcp4, udp4, sctp4, tcp6, ..etc), to get > symmetric RSS on them, instead of the proposed single "-X" command? > The second option is closer to what I had in mind. We can re-name and > provide any details. I'm just trying to help, if you want a single knob you'd need to add new fields to the API and the RXFH API is not netlink-ified. Using hashing algo for configuring fields feels like a dirty hack. > I agree that we will need to take care of some cases like if the user > removes only "source IP" or "destination port" from the hash fields, > without that field's counterpart (we can prevent this, or show a > warning, ..etc). I was planning to address that in a follow-up > series; ie. handling the "ethtool -U rx-flow-hash". Do you want that > to be included in the same series as well? Yes, the validation needs to be part of the same series. But the semantics of selecting only src or dst need to be established, too. You said you feed dst ^ src into the hashing twice - why?
On 2023-08-24 18:43, Jakub Kicinski wrote: > On Thu, 24 Aug 2023 16:55:40 -0600 Ahmed Zaki wrote: >> When "Symmetric Toeplitz" is set in the NIC, the H/W will yield the same >> hash as the regular Toeplitz for protocol types that do not have such >> symmetric fields in both directions (i.e. there will be no RSS hash >> symmetry and the TX/RX traffic will land on different Rx queues). >> >> The goal of this series is to enable the "default" behavior of the whole >> device ("-X hfunc") to be the symmetric hash (again, only for protocols >> that have symmetric src/dst counterparts). If I understand the first >> option correctly, the user would need to manually configure all RXH >> fields for all flow types (tcp4, udp4, sctp4, tcp6, ..etc), to get >> symmetric RSS on them, instead of the proposed single "-X" command? >> The second option is closer to what I had in mind. We can re-name and >> provide any details. > I'm just trying to help, if you want a single knob you'd need to add > new fields to the API and the RXFH API is not netlink-ified. > > Using hashing algo for configuring fields feels like a dirty hack. Ok. Another way to add a single knob is to a flag in "struct ethtool_rxfh" (there are still some reserved bytes) and then: ethtool -X eth0 --symmetric hfunc toeplitz This will also allow drivers/NICs to implement this as they wish (XOR, sorted, ..etc). Better ? > >> I agree that we will need to take care of some cases like if the user >> removes only "source IP" or "destination port" from the hash fields, >> without that field's counterpart (we can prevent this, or show a >> warning, ..etc). I was planning to address that in a follow-up >> series; ie. handling the "ethtool -U rx-flow-hash". Do you want that >> to be included in the same series as well? > Yes, the validation needs to be part of the same series. But the > semantics of selecting only src or dst need to be established, too. > You said you feed dst ^ src into the hashing twice - why? To maintain the same input length (same as the regular Toeplitz input) to the hash H/W block length(src_ip , dst_ip, src_port, dst_port) = length(src_ip ^ dst_ip , src_ip ^ dst_ip, src_port ^ dst_port, src_port ^ dst_port)
On 2023-08-24 17:30, Willem de Bruijn wrote: > Ahmed Zaki wrote: >> On 2023-08-24 12:36, Saeed Mahameed wrote: >>> On 24 Aug 07:14, Ahmed Zaki wrote: >>>> On 2023-08-23 13:45, Saeed Mahameed wrote: >>>>> On 23 Aug 10:48, 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. >>>>>> >>> ... >>> >>>>> What is the expectation of the symmetric toeplitz hash, how do you >>>>> achieve >>>>> that? by sorting packet fields? which fields? >>>>> >>>>> Can you please provide a link to documentation/spec? >>>>> We should make sure all vendors agree on implementation and >>>>> expectation of >>>>> the symmetric hash function. >>>> The way the Intel NICs are achieving this hash symmetry is by XORing >>>> the source and destination values of the IP and L4 ports and then >>>> feeding these values to the regular Toeplitz (in-tree) hash algorithm. >>>> >>>> For example, for UDP/IPv4, the input fields for the Toeplitz hash >>>> would be: >>>> >>>> (SRC_IP, DST_IP, SRC_PORT, DST_PORT) >>>> >>> So you mangle the input. This is different than the paper you >>> referenced below which doesn't change the input but it modifies the RSS >>> algorithm and uses a special hash key. >>> >>>> If symmetric Toeplitz is set, the NIC XOR the src and dst fields: >>>> >>>> (SRC_IP^DST_IP , SRC_IP^DST_IP, SRC_PORT^DST_PORT, SRC_PORT^DST_PORT) >>>> >>>> This way, the output hash would be the same for both flow directions. >>>> Same is applicable for IPv6, TCP and SCTP. >>>> >>> I understand the motivation, I just want to make sure the >>> interpretation is >>> clear, I agree with Jakub, we should use a clear name for the ethtool >>> parameter or allow users to select "xor-ed"/"sorted" fields as Jakub >>> suggested. >>>> Regarding the documentation, the above is available in our public >>>> datasheets [2]. In the final version, I can add similar explanation >>>> in the headers (kdoc) and under "Documentation/networking/" so that >>>> there is a clear understanding of the algorithm. > Please do define the behavior. > > When I hear symmetric Toeplitz, my initial assumption was also > sorted fields, as implemented in __flow_hash_consistentify. > > If this is something else, agreed that that is good to make > crystal clear in name and somewhere in the kernel Documentation. > xor-symmetric hash? Thanks, I was wondering why everyone was assuming "sorted" fileds. If we go with an a new algorithm (-X hfunc) I agree we should name it "xor-symmetric". I also just suggested to Jakub to use a flag instead of a new algorithm, since the underlying algorithm is really just the regular Toeplitz.
On Fri, 25 Aug 2023 14:46:42 -0600 Ahmed Zaki wrote: > > I'm just trying to help, if you want a single knob you'd need to add > > new fields to the API and the RXFH API is not netlink-ified. > > > > Using hashing algo for configuring fields feels like a dirty hack. > > Ok. Another way to add a single knob is to a flag in "struct > ethtool_rxfh" (there are still some reserved bytes) and then: Sorry we do have ETHTOOL_MSG_RSS_GET. It just doesn't cover the flow config now. But you can add the new field there without a problem. > ethtool -X eth0 --symmetric hfunc toeplitz > > This will also allow drivers/NICs to implement this as they wish (XOR, > sorted, ..etc). Better ? We should specify the fields, I reckon, something like: ethtool -X eth0 --symmetric sdfn hfunc toeplitz So that the driver can make sure the user expects symmetry on fields the device supports. > >> I agree that we will need to take care of some cases like if the user > >> removes only "source IP" or "destination port" from the hash fields, > >> without that field's counterpart (we can prevent this, or show a > >> warning, ..etc). I was planning to address that in a follow-up > >> series; ie. handling the "ethtool -U rx-flow-hash". Do you want that > >> to be included in the same series as well? > > Yes, the validation needs to be part of the same series. But the > > semantics of selecting only src or dst need to be established, too. > > You said you feed dst ^ src into the hashing twice - why? > > To maintain the same input length (same as the regular Toeplitz input) > to the hash H/W block But that's a choice, right? We're configuring the input we could as well choose to make it shorter? v4 and v6 use the same key with different input lengths, right?
On 2023-08-25 18:49, Jakub Kicinski wrote: > On Fri, 25 Aug 2023 14:46:42 -0600 Ahmed Zaki wrote: >>> I'm just trying to help, if you want a single knob you'd need to add >>> new fields to the API and the RXFH API is not netlink-ified. >>> >>> Using hashing algo for configuring fields feels like a dirty hack. >> Ok. Another way to add a single knob is to a flag in "struct >> ethtool_rxfh" (there are still some reserved bytes) and then: > Sorry we do have ETHTOOL_MSG_RSS_GET. It just doesn't cover the flow > config now. But you can add the new field there without a problem. > >> ethtool -X eth0 --symmetric hfunc toeplitz >> >> This will also allow drivers/NICs to implement this as they wish (XOR, >> sorted, ..etc). Better ? > We should specify the fields, I reckon, something like: > > ethtool -X eth0 --symmetric sdfn hfunc toeplitz > > So that the driver can make sure the user expects symmetry on fields > the device supports. Seems fair. I will prepare this and the per-flow based config code ("-U|-N") and re-send. > >>>> I agree that we will need to take care of some cases like if the user >>>> removes only "source IP" or "destination port" from the hash fields, >>>> without that field's counterpart (we can prevent this, or show a >>>> warning, ..etc). I was planning to address that in a follow-up >>>> series; ie. handling the "ethtool -U rx-flow-hash". Do you want that >>>> to be included in the same series as well? >>> Yes, the validation needs to be part of the same series. But the >>> semantics of selecting only src or dst need to be established, too. >>> You said you feed dst ^ src into the hashing twice - why? >> To maintain the same input length (same as the regular Toeplitz input) >> to the hash H/W block > But that's a choice, right? We're configuring the input we could as > well choose to make it shorter? v4 and v6 use the same key with > different input lengths, right? Correct. All RSS fields' offsets and lengths are configurable. The example I gave before was from the datasheet, but it seems we can feed the Xored values once. Thanks,
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index 62b61527bcc4..9a8e1fb7170d 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -60,10 +60,11 @@ enum { 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 */ + ETH_RSS_HASH_SYM_TOP_BIT, /* Configurable RSS hash function - Symmetric Toeplitz */ /* * Add your fresh new hash function bits above and remember to update - * rss_hash_func_strings[] in ethtool.c + * rss_hash_func_strings[] in ethtool/common.c */ ETH_RSS_HASH_FUNCS_COUNT }; @@ -108,6 +109,7 @@ enum ethtool_supported_ring_param { #define __ETH_RSS_HASH(name) __ETH_RSS_HASH_BIT(ETH_RSS_HASH_##name##_BIT) #define ETH_RSS_HASH_TOP __ETH_RSS_HASH(TOP) +#define ETH_RSS_HASH_SYM_TOP __ETH_RSS_HASH(SYM_TOP) #define ETH_RSS_HASH_XOR __ETH_RSS_HASH(XOR) #define ETH_RSS_HASH_CRC32 __ETH_RSS_HASH(CRC32) diff --git a/net/ethtool/common.c b/net/ethtool/common.c index f5598c5f50de..a0e0c6b2980e 100644 --- a/net/ethtool/common.c +++ b/net/ethtool/common.c @@ -81,6 +81,7 @@ rss_hash_func_strings[ETH_RSS_HASH_FUNCS_COUNT][ETH_GSTRING_LEN] = { [ETH_RSS_HASH_TOP_BIT] = "toeplitz", [ETH_RSS_HASH_XOR_BIT] = "xor", [ETH_RSS_HASH_CRC32_BIT] = "crc32", + [ETH_RSS_HASH_SYM_TOP_BIT] = "symmetric_toeplitz", }; const char
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. Allow ethtool to support symmetric Toeplitz algorithm. A user can set the RSS function of the netdevice via: # ethtool -X eth0 hfunc symmetric_toeplitz Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com> --- include/linux/ethtool.h | 4 +++- net/ethtool/common.c | 1 + 2 files changed, 4 insertions(+), 1 deletion(-)