On 11/2/20 7:12 PM, Slava Ovsiienko wrote:
-----Original Message-----
From: Ferruh Yigit <ferruh.yi...@intel.com>
Sent: Monday, November 2, 2020 18:01
To: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>; Xueming(Steven)
Li <xuemi...@nvidia.com>; Andrew Rybchenko <arybche...@solarflare.com>;
dev@dpdk.org; declan.dohe...@intel.com
Cc: Andrey Vesnovaty <andr...@nvidia.com>; NBU-Contact-Thomas Monjalon
<tho...@monjalon.net>; Ray Kinsella <m...@ashroe.eu>; Neil Horman
<nhor...@tuxdriver.com>; Ori Kam <or...@nvidia.com>; Wei Hu (Xavier)
<xavier.hu...@huawei.com>; Min Hu (Connor) <humi...@huawei.com>;
Yisen Zhuang <yisen.zhu...@huawei.com>; Lijun Ou <ouli...@huawei.com>;
Matan Azrad <ma...@nvidia.com>; Shahaf Shuler <shah...@nvidia.com>;
Slava Ovsiienko <viachesl...@nvidia.com>; Jasvinder Singh
<jasvinder.si...@intel.com>; Cristian Dumitrescu
<cristian.dumitre...@intel.com>; Ajit Khaparde
<ajit.khapa...@broadcom.com>; Somnath Kotur
<somnath.ko...@broadcom.com>; Qiming Yang <qiming.y...@intel.com>; Qi
Zhang <qi.z.zh...@intel.com>
Subject: Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using
action attribute

On 11/1/2020 10:45 AM, Andrew Rybchenko wrote:
On 10/30/20 7:12 PM, Xueming(Steven) Li wrote:
Hi Andrew,

-----Original Message-----
From: dev <dev-boun...@dpdk.org> On Behalf Of Andrew Rybchenko
Sent: Thursday, October 29, 2020 4:53 PM
To: dev@dpdk.org
Cc: Andrey Vesnovaty <andr...@nvidia.com>; NBU-Contact-Thomas
Monjalon <tho...@monjalon.net>; Ferruh Yigit
<ferruh.yi...@intel.com>; Ray Kinsella <m...@ashroe.eu>; Neil Horman
<nhor...@tuxdriver.com>; Ori Kam <or...@nvidia.com>; Andrew
Rybchenko <andrew.rybche...@oktetlabs.ru>
Subject: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using
action attribute

A new generic shared actions API may be used to create shared counter.
There is no point to keep duplicate COUNT action specific capability
to create shared counters.

Signed-off-by: Andrew Rybchenko <arybche...@solarflare.com>
---
In fact, it looks like the next logical step is to remove struct
rte_flow_action_count completely since counter ID makes sense for
shared counters only. I think it will just make it easiser to use COUNT
action.
Comments are welcome.

   doc/guides/rel_notes/deprecation.rst | 4 ++++
   lib/librte_ethdev/rte_flow.h         | 6 +++++-
   2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/doc/guides/rel_notes/deprecation.rst
b/doc/guides/rel_notes/deprecation.rst
index 2e082499b8..4f3bac1a6d 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -138,6 +138,10 @@ Deprecation Notices
     will be limited to maximum 256 queues.
     Also compile time flag ``RTE_ETHDEV_QUEUE_STAT_CNTRS`` will be
removed.

+* ethdev: Attribute ``shared`` of the ``struct
+rte_flow_action_count``
+  is deprecated and will be removed in DPDK 21.11. Shared counters
+should
+  be managed using shared actions API
+(``rte_flow_shared_action_create``
etc).
+
   * cryptodev: support for using IV with all sizes is added, J0
still can
     be used but only when IV length in following structs
``rte_crypto_auth_xform``,
     ``rte_crypto_aead_xform`` is set to zero. When IV length is
greater or equal diff --git a/lib/librte_ethdev/rte_flow.h
b/lib/librte_ethdev/rte_flow.h index a8eac4deb8..2bb93d237a 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -2287,6 +2287,9 @@ struct rte_flow_query_age {
    * Counters can be retrieved and reset through
``rte_flow_query()``, see
    * ``struct rte_flow_query_count``.
    *
+ * @deprecated Shared attribute is deprecated, use generic
+ * RTE_FLOW_ACTION_TYPE_SHARED action.
+ *
    * The shared flag indicates whether the counter is unique to the
flow rule the
    * action is specified with, or whether it is a shared counter.
    *
@@ -2299,7 +2302,8 @@ struct rte_flow_query_age {
    * to all ports within that switch domain.
    */
   struct rte_flow_action_count {
-    uint32_t shared:1; /**< Share counter ID with other flow rules.
*/
+    /** @deprecated Share counter ID with other flow rules. */
+    uint32_t shared:1;
       uint32_t reserved:31; /**< Reserved, must be zero. */
       uint32_t id; /**< Counter ID. */
Do you think id could be removed as well? neither non-shared flow
counter query, nor shared action query.

I'm not 100% sure, but yes, as I write above just after my Signed-off-by.


cc'ed Declan + maintainers of PMDs for the 'id' field, but as far as I can see 
it is
used out of the 'shared' context, so I am for going on with existing patch for
now.

Reviewed-by: Ferruh Yigit <ferruh.yi...@intel.com>

It depends whether we are going to support multiple counters for the same flow.

Why? Query refers to a counter using action pointer. There is always one
counter in one action. If you need more counters, just use more actions.

If there is the only counter per flow we could get rid of the "id" field 
either. If it is
still needed, PMDs should generate counter id internally and id should not be 
exposed outside.

With best regards, Slava

PS. What about meters? The next good candidate to shared actions.



Reply via email to