On 9/1/2020 3:27 PM, Kiran Kumar Kokkilagadda wrote:


-----Original Message-----
From: Ferruh Yigit <ferruh.yi...@intel.com>
Sent: Tuesday, September 1, 2020 7:08 PM
To: Kiran Kumar Kokkilagadda <kirankum...@marvell.com>; Thomas Monjalon
<tho...@monjalon.net>; Andrew Rybchenko <arybche...@solarflare.com>
Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran <jer...@marvell.com>;
or...@mellanox.com; xuanziya...@huawei.com;
cloud.wangxiao...@huawei.com; zhouguoy...@huawei.com;
rosen...@intel.com; beilei.x...@intel.com; jia....@intel.com; Rasesh Mody
<rm...@marvell.com>; Shahed Shaikh <shsha...@marvell.com>; Nithin Kumar
Dabilpuram <ndabilpu...@marvell.com>; qiming.y...@intel.com;
qi.z.zh...@intel.com; keith.wi...@intel.com; hemant.agra...@nxp.com;
sachin.sax...@nxp.com; wei.zh...@intel.com; johnd...@cisco.com;
hyon...@cisco.com; ch...@att.com; ma...@mellanox.com;
shah...@mellanox.com; viachesl...@mellanox.com;
rahul.lakkire...@chelsio.com; gr...@u256.net; Liron Himi
<lir...@marvell.com>; jingjing...@intel.com; xavier.hu...@huawei.com;
humi...@huawei.com; yisen.zhu...@huawei.com;
ajit.khapa...@broadcom.com; somnath.ko...@broadcom.com;
jasvinder.si...@intel.com; cristian.dumitre...@intel.com
Subject: [EXT] Re: [dpdk-dev][PATCH v7 1/3] ethdev: add level support for RSS
offload types

External Email

----------------------------------------------------------------------
On 9/1/2020 4:27 AM, kirankum...@marvell.com wrote:
From: Kiran Kumar K <kirankum...@marvell.com>

This patch reserves 2 bits as input selection to select Inner and
outer encapsulation level for RSS computation. It is combined with
existing
ETH_RSS_* to choose Inner or outer layers.
This functionality already exists in rte_flow through level parameter
in RSS action configuration rte_flow_action_rss.

Signed-off-by: Kiran Kumar K <kirankum...@marvell.com>
---
V7 Changes:
* Re-worked to keep it in sync with rte_flow_action_rss and support
upto
3 levels.
* Addressed testpmd review comments.

   lib/librte_ethdev/rte_ethdev.h | 27 +++++++++++++++++++++++++++
   1 file changed, 27 insertions(+)

diff --git a/lib/librte_ethdev/rte_ethdev.h
b/lib/librte_ethdev/rte_ethdev.h index 70295d7ab..13e49bbd7 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -552,6 +552,33 @@ struct rte_eth_rss_conf {
   #define RTE_ETH_RSS_L3_PRE64    (1ULL << 53)
   #define RTE_ETH_RSS_L3_PRE96    (1ULL << 52)

+/*
+ * We use the following macros to combine with the above layers to
+choose
+ * inner and outer layers or both for RSS computation.
+ * bit 50 and 51 are reserved for this.
+ */
+
+/** level 0, requests the default behavior. Depending on the packet
+ * type, it can mean outermost, innermost, anything in between or even no
RSS.
+ * It basically stands for the innermost encapsulation level RSS
+ * can be performed on according to PMD and device capabilities.
+ */
+#define ETH_RSS_LEVEL_0         (0ULL << 50)

I can see from history how this is involved, but the 'ETH_RSS_LEVEL_0' naming is
not really clear what it is, the naming in v6 is more clear.

What about following one:
0 -> LEVEL_PMD_DEFAULT
1 -> LEVEL_OUTER
2 -> LEVEL_INNER
3 -> LEVEL_INNER_OUTER

This doesn't exactly match to rte_flow one, but closer than v6 one. This ends
with max level 2. And defines a way to say both inner and outer.

This one looks good to me. If everyone is ok with the proposed changes, I will 
send V8.


Let's wait for more comments before v8.


+
+/** level 1,  requests RSS to be performed on the outermost packet
+ * encapsulation level.
+ */
+#define ETH_RSS_LEVEL_1         (1ULL << 50)
+
+/** level 2,  requests RSS to be performed on the
+ * specified inner packet encapsulation level, from outermost to
+ * innermost (lower to higher values).
+ */
+#define ETH_RSS_LEVEL_2                (2ULL << 50)

I can see you are trying to copy rte_flow usage, but this doesn't really makes
sense here. Where the value of the level is defined in this case? If not defined
how the PMD knows which level to use?

+#define ETH_RSS_LEVEL_MASK     (3ULL << 50)
+
+#define ETH_RSS_LEVEL(rss_hf) ((rss_hf & ETH_RSS_LEVEL_MASK) >> 50)
+
   /**
    * For input set change of hash filter, if SRC_ONLY and DST_ONLY of
    * the same level are used simultaneously, it is the same case as
--
2.25.1



Reply via email to