[v3,1/2] ethdev: extend RSS offload types
Checks
Commit Message
This patch cover two aspects:
(1)decouple RTE_ETH_FLOW_* and ETH_RSS_*. Because both serve
different purposes.
(2)reserve several bits as input set selection from the high
end of the 64 bits. It is combined with exisiting ETH_RSS_*
to represent rss types.
for example:
ETH_RSS_IPV4 | ETH_RSS_L3_SRC_ONLY: hash on src ip address only
ETH_RSS_IPV4_UDP | ETH_RSS_L4_DST_ONLY: hash on src/dst IP and
dst UDP port
ETH_RSS_L2_PAYLOAD | ETH_RSS_L2_DST_ONLY: hash on dst mac address
Signed-off-by: Simei Su <simei.su@intel.com>
---
lib/librte_ethdev/rte_ethdev.h | 63 +++++++++++++++++++++++++-----------------
1 file changed, 37 insertions(+), 26 deletions(-)
Comments
On 9/25/19 5:06 PM, Simei Su wrote:
> This patch cover two aspects:
> (1)decouple RTE_ETH_FLOW_* and ETH_RSS_*. Because both serve
> different purposes.
> (2)reserve several bits as input set selection from the high
> end of the 64 bits. It is combined with exisiting ETH_RSS_*
> to represent rss types.
If the patch covers two aspects why it is one patch instead of two?
It would be useful to motivate decouple a bit getter and provide
details since "different purposes" are hardly very useful. Which
purposes?
> for example:
> ETH_RSS_IPV4 | ETH_RSS_L3_SRC_ONLY: hash on src ip address only
Is (ETH_RSS_IPV4 | ETH_RSS_L3_SRC_ONLY | ETH_RSS_L3_DST_ONLY)
valid and an equivalent to ETH_RSS_IPV4 only?
If yes, shouldn't generic API care about it or each driver should do it?
Similar question is applicable to L4.
> ETH_RSS_IPV4_UDP | ETH_RSS_L4_DST_ONLY: hash on src/dst IP and
> dst UDP port
> ETH_RSS_L2_PAYLOAD | ETH_RSS_L2_DST_ONLY: hash on dst mac address
I'm a bit confused by L2_PAYLOAD | L2_DST_ONLY. Does L2_PAYLOAD
mean entire L2 frame including header and payload?
> Signed-off-by: Simei Su <simei.su@intel.com>
[snip]
From: Andrew Rybchenko [mailto:arybchenko@solarflare.com]
Sent: Thursday, September 26, 2019 5:16 PM
To: Su, Simei <simei.su@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v3 1/2] ethdev: extend RSS offload types
On 9/25/19 5:06 PM, Simei Su wrote:
This patch cover two aspects:
(1)decouple RTE_ETH_FLOW_* and ETH_RSS_*. Because both serve
different purposes.
(2)reserve several bits as input set selection from the high
end of the 64 bits. It is combined with exisiting ETH_RSS_*
to represent rss types.
If the patch covers two aspects why it is one patch instead of two?
It would be useful to motivate decouple a bit getter and provide
details since "different purposes" are hardly very useful. Which
purposes?
[Qi: ]Though the second purpose depends on the first one, but I agree, it’s better to separate them in two patches
for example:
ETH_RSS_IPV4 | ETH_RSS_L3_SRC_ONLY: hash on src ip address only
Is (ETH_RSS_IPV4 | ETH_RSS_L3_SRC_ONLY | ETH_RSS_L3_DST_ONLY)
valid and an equivalent to ETH_RSS_IPV4 only?
[Qi:] The word “only” should be an exclusive attribute , so I don’t think ETH_RSS_L3_SRC_ONLY | ETH_RSS_L3_DST_ONLY is a valid case,
Such configure should be rejected by all PMD ( better be handled in generic API, but I did see a good place to add so far)
But I guess that should not be a problem, since nobody will intend to use with that way, also exist PMD may reject any new bit it don't know.
If yes, shouldn't generic API care about it or each driver should do it?
Similar question is applicable to L4.
ETH_RSS_IPV4_UDP | ETH_RSS_L4_DST_ONLY: hash on src/dst IP and
dst UDP port
ETH_RSS_L2_PAYLOAD | ETH_RSS_L2_DST_ONLY: hash on dst mac address
I'm a bit confused by L2_PAYLOAD | L2_DST_ONLY. Does L2_PAYLOAD
mean entire L2 frame including header and payload?
[Qi:] yes, seems we are missing L2_RSS_ETH.
Regards
Qi
Signed-off-by: Simei Su <simei.su@intel.com>
[snip]
@@ -453,7 +453,6 @@ struct rte_eth_rss_conf {
* possible, and exclusively. For example, if a packet is identified as
* 'RTE_ETH_FLOW_NONFRAG_IPV4_TCP', it will not be any of other flow types,
* though it is an actual IPV4 packet.
- * Note that the flow types are used to define RSS offload types.
*/
#define RTE_ETH_FLOW_UNKNOWN 0
#define RTE_ETH_FLOW_RAW 1
@@ -482,31 +481,43 @@ struct rte_eth_rss_conf {
#define RTE_ETH_FLOW_MAX 23
/*
- * The RSS offload types are defined based on flow types.
- * Different NIC hardware may support different RSS offload
- * types. The supported flow types or RSS offload types can be queried by
- * rte_eth_dev_info_get().
- */
-#define ETH_RSS_IPV4 (1ULL << RTE_ETH_FLOW_IPV4)
-#define ETH_RSS_FRAG_IPV4 (1ULL << RTE_ETH_FLOW_FRAG_IPV4)
-#define ETH_RSS_NONFRAG_IPV4_TCP (1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_TCP)
-#define ETH_RSS_NONFRAG_IPV4_UDP (1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_UDP)
-#define ETH_RSS_NONFRAG_IPV4_SCTP (1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_SCTP)
-#define ETH_RSS_NONFRAG_IPV4_OTHER (1ULL << RTE_ETH_FLOW_NONFRAG_IPV4_OTHER)
-#define ETH_RSS_IPV6 (1ULL << RTE_ETH_FLOW_IPV6)
-#define ETH_RSS_FRAG_IPV6 (1ULL << RTE_ETH_FLOW_FRAG_IPV6)
-#define ETH_RSS_NONFRAG_IPV6_TCP (1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_TCP)
-#define ETH_RSS_NONFRAG_IPV6_UDP (1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_UDP)
-#define ETH_RSS_NONFRAG_IPV6_SCTP (1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_SCTP)
-#define ETH_RSS_NONFRAG_IPV6_OTHER (1ULL << RTE_ETH_FLOW_NONFRAG_IPV6_OTHER)
-#define ETH_RSS_L2_PAYLOAD (1ULL << RTE_ETH_FLOW_L2_PAYLOAD)
-#define ETH_RSS_IPV6_EX (1ULL << RTE_ETH_FLOW_IPV6_EX)
-#define ETH_RSS_IPV6_TCP_EX (1ULL << RTE_ETH_FLOW_IPV6_TCP_EX)
-#define ETH_RSS_IPV6_UDP_EX (1ULL << RTE_ETH_FLOW_IPV6_UDP_EX)
-#define ETH_RSS_PORT (1ULL << RTE_ETH_FLOW_PORT)
-#define ETH_RSS_VXLAN (1ULL << RTE_ETH_FLOW_VXLAN)
-#define ETH_RSS_GENEVE (1ULL << RTE_ETH_FLOW_GENEVE)
-#define ETH_RSS_NVGRE (1ULL << RTE_ETH_FLOW_NVGRE)
+ * Below macros are defined for RSS offload types, they can be used to
+ * fill rte_eth_rss_conf.rss_hf or rte_flow_action_rss.types.
+ */
+#define ETH_RSS_IPV4 (1ULL << 2)
+#define ETH_RSS_FRAG_IPV4 (1ULL << 3)
+#define ETH_RSS_NONFRAG_IPV4_TCP (1ULL << 4)
+#define ETH_RSS_NONFRAG_IPV4_UDP (1ULL << 5)
+#define ETH_RSS_NONFRAG_IPV4_SCTP (1ULL << 6)
+#define ETH_RSS_NONFRAG_IPV4_OTHER (1ULL << 7)
+#define ETH_RSS_IPV6 (1ULL << 8)
+#define ETH_RSS_FRAG_IPV6 (1ULL << 9)
+#define ETH_RSS_NONFRAG_IPV6_TCP (1ULL << 10)
+#define ETH_RSS_NONFRAG_IPV6_UDP (1ULL << 11)
+#define ETH_RSS_NONFRAG_IPV6_SCTP (1ULL << 12)
+#define ETH_RSS_NONFRAG_IPV6_OTHER (1ULL << 13)
+#define ETH_RSS_L2_PAYLOAD (1ULL << 14)
+#define ETH_RSS_IPV6_EX (1ULL << 15)
+#define ETH_RSS_IPV6_TCP_EX (1ULL << 16)
+#define ETH_RSS_IPV6_UDP_EX (1ULL << 17)
+#define ETH_RSS_PORT (1ULL << 18)
+#define ETH_RSS_VXLAN (1ULL << 19)
+#define ETH_RSS_GENEVE (1ULL << 20)
+#define ETH_RSS_NVGRE (1ULL << 21)
+
+/*
+ * We use the following macros to combine with above ETH_RSS_* for
+ * more specific input set selection. These bits are defined starting
+ * from the high end of the 64 bits.
+ * Note: if use above ETH_RSS_* without SRC/DST_ONLY, it represents
+ * both SRC and DST are taken into account.
+ */
+#define ETH_RSS_L2_SRC_ONLY (1ULL << 63)
+#define ETH_RSS_L2_DST_ONLY (1ULL << 62)
+#define ETH_RSS_L3_SRC_ONLY (1ULL << 61)
+#define ETH_RSS_L3_DST_ONLY (1ULL << 60)
+#define ETH_RSS_L4_SRC_ONLY (1ULL << 59)
+#define ETH_RSS_L4_DST_ONLY (1ULL << 58)
#define ETH_RSS_IP ( \
ETH_RSS_IPV4 | \