DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute
@ 2020-10-29  8:52 Andrew Rybchenko
  2020-10-29 14:39 ` Ori Kam
                   ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: Andrew Rybchenko @ 2020-10-29  8:52 UTC (permalink / raw)
  To: dev
  Cc: Andrey Vesnovaty, Thomas Monjalon, Ferruh Yigit, Ray Kinsella,
	Neil Horman, Ori Kam, Andrew Rybchenko

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 <arybchenko@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. */
 };
-- 
2.17.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute
  2020-10-29  8:52 [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute Andrew Rybchenko
@ 2020-10-29 14:39 ` Ori Kam
  2020-10-29 16:11   ` Thomas Monjalon
  2020-10-30 16:12 ` Xueming(Steven) Li
  2020-11-03 14:26 ` [dpdk-dev] [PATCH v2] " Andrew Rybchenko
  2 siblings, 1 reply; 26+ messages in thread
From: Ori Kam @ 2020-10-29 14:39 UTC (permalink / raw)
  To: Andrew Rybchenko, dev
  Cc: Andrey Vesnovaty, NBU-Contact-Thomas Monjalon, Ferruh Yigit,
	Ray Kinsella, Neil Horman, Andrew Rybchenko

Hi Andrew,

> -----Original Message-----
> From: Andrew Rybchenko <arybchenko@solarflare.com>
> Sent: Thursday, October 29, 2020 10:53 AM
> Subject: [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 <arybchenko@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. */
>  };
> --
> 2.17.1

As much as I agree with your patch, I don't think we should push it since
not all PMD made the move to support count action, so the application still needs
to use this API.

I think this patch should be done but in next LTS release.

Best,
Ori


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute
  2020-10-29 14:39 ` Ori Kam
@ 2020-10-29 16:11   ` Thomas Monjalon
  2020-11-01  7:49     ` Ori Kam
  0 siblings, 1 reply; 26+ messages in thread
From: Thomas Monjalon @ 2020-10-29 16:11 UTC (permalink / raw)
  To: Ori Kam
  Cc: Andrew Rybchenko, dev, Andrey Vesnovaty, Ferruh Yigit,
	Ray Kinsella, Neil Horman, Andrew Rybchenko

29/10/2020 15:39, Ori Kam:
> Hi Andrew,
> 
[...]
> > 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 <arybchenko@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.
[...]
> > +* 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).
> > +
[...]
> > + * @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. */
> >  };
> > --
> > 2.17.1
> 
> As much as I agree with your patch, I don't think we should push it since
> not all PMD made the move to support count action, so the application still needs
> to use this API.
> 
> I think this patch should be done but in next LTS release.

The patch is not removing the field,
it is just warning it will be removed in next year.



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute
  2020-10-29  8:52 [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute Andrew Rybchenko
  2020-10-29 14:39 ` Ori Kam
@ 2020-10-30 16:12 ` Xueming(Steven) Li
  2020-11-01 10:45   ` Andrew Rybchenko
  2020-11-03 14:26 ` [dpdk-dev] [PATCH v2] " Andrew Rybchenko
  2 siblings, 1 reply; 26+ messages in thread
From: Xueming(Steven) Li @ 2020-10-30 16:12 UTC (permalink / raw)
  To: Andrew Rybchenko, dev
  Cc: Andrey Vesnovaty, NBU-Contact-Thomas Monjalon, Ferruh Yigit,
	Ray Kinsella, Neil Horman, Ori Kam, Andrew Rybchenko

Hi Andrew,

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Andrew Rybchenko
> Sent: Thursday, October 29, 2020 4:53 PM
> To: dev@dpdk.org
> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-Thomas
> Monjalon <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>;
> Ray Kinsella <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>;
> Ori Kam <orika@nvidia.com>; Andrew Rybchenko
> <andrew.rybchenko@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 <arybchenko@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.

>  };
> --
> 2.17.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute
  2020-10-29 16:11   ` Thomas Monjalon
@ 2020-11-01  7:49     ` Ori Kam
  2020-11-03 17:21       ` Thomas Monjalon
  0 siblings, 1 reply; 26+ messages in thread
From: Ori Kam @ 2020-11-01  7:49 UTC (permalink / raw)
  To: NBU-Contact-Thomas Monjalon
  Cc: Andrew Rybchenko, dev, Andrey Vesnovaty, Ferruh Yigit,
	Ray Kinsella, Neil Horman, Andrew Rybchenko



> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, October 29, 2020 6:12 PM
> To: Ori Kam <orika@nvidia.com>
> Subject: Re: [PATCH] ethdev: deprecate shared counters using action attribute
> 
> 29/10/2020 15:39, Ori Kam:
> > Hi Andrew,
> >
> [...]
> > > 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 <arybchenko@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.
> [...]
> > > +* 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).
> > > +
> [...]
> > > + * @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. */
> > >  };
> > > --
> > > 2.17.1
> >
> > As much as I agree with your patch, I don't think we should push it since
> > not all PMD made the move to support count action, so the application still
> needs
> > to use this API.
> >
> > I think this patch should be done but in next LTS release.
> 
> The patch is not removing the field,
> it is just warning it will be removed in next year.
> 

Yes I know, but I don't think it is correct to issue such a warning without support.
The application still must use this API, the warning should be added as soon as
at least one PMD support shared counters with the new API.

Best,
Ori

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute
  2020-10-30 16:12 ` Xueming(Steven) Li
@ 2020-11-01 10:45   ` Andrew Rybchenko
  2020-11-02 16:01     ` Ferruh Yigit
  0 siblings, 1 reply; 26+ messages in thread
From: Andrew Rybchenko @ 2020-11-01 10:45 UTC (permalink / raw)
  To: Xueming(Steven) Li, Andrew Rybchenko, dev
  Cc: Andrey Vesnovaty, NBU-Contact-Thomas Monjalon, Ferruh Yigit,
	Ray Kinsella, Neil Horman, Ori Kam

On 10/30/20 7:12 PM, Xueming(Steven) Li wrote:
> Hi Andrew,
> 
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Andrew Rybchenko
>> Sent: Thursday, October 29, 2020 4:53 PM
>> To: dev@dpdk.org
>> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-Thomas
>> Monjalon <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>;
>> Ray Kinsella <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>;
>> Ori Kam <orika@nvidia.com>; Andrew Rybchenko
>> <andrew.rybchenko@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 <arybchenko@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.



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute
  2020-11-01 10:45   ` Andrew Rybchenko
@ 2020-11-02 16:01     ` Ferruh Yigit
  2020-11-02 16:12       ` Slava Ovsiienko
  2020-11-24 18:09       ` Ajit Khaparde
  0 siblings, 2 replies; 26+ messages in thread
From: Ferruh Yigit @ 2020-11-02 16:01 UTC (permalink / raw)
  To: Andrew Rybchenko, Xueming(Steven) Li, Andrew Rybchenko, dev,
	declan.doherty
  Cc: Andrey Vesnovaty, NBU-Contact-Thomas Monjalon, Ray Kinsella,
	Neil Horman, Ori Kam, Wei Hu (Xavier), Min Hu (Connor),
	Yisen Zhuang, Lijun Ou, Matan Azrad, Shahaf Shuler,
	Viacheslav Ovsiienko, Jasvinder Singh, Cristian Dumitrescu,
	Ajit Khaparde, Somnath Kotur, Qiming Yang, Qi Zhang

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-bounces@dpdk.org> On Behalf Of Andrew Rybchenko
>>> Sent: Thursday, October 29, 2020 4:53 PM
>>> To: dev@dpdk.org
>>> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-Thomas
>>> Monjalon <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>;
>>> Ray Kinsella <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>;
>>> Ori Kam <orika@nvidia.com>; Andrew Rybchenko
>>> <andrew.rybchenko@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 <arybchenko@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.yigit@intel.com>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute
  2020-11-02 16:01     ` Ferruh Yigit
@ 2020-11-02 16:12       ` Slava Ovsiienko
  2020-11-02 16:40         ` Andrew Rybchenko
  2020-11-24 18:09       ` Ajit Khaparde
  1 sibling, 1 reply; 26+ messages in thread
From: Slava Ovsiienko @ 2020-11-02 16:12 UTC (permalink / raw)
  To: Ferruh Yigit, Andrew Rybchenko, Xueming(Steven) Li,
	Andrew Rybchenko, dev, declan.doherty
  Cc: Andrey Vesnovaty, NBU-Contact-Thomas Monjalon, Ray Kinsella,
	Neil Horman, Ori Kam, Wei Hu (Xavier), Min Hu (Connor),
	Yisen Zhuang, Lijun Ou, Matan Azrad, Shahaf Shuler,
	Jasvinder Singh, Cristian Dumitrescu, Ajit Khaparde,
	Somnath Kotur, Qiming Yang, Qi Zhang

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Monday, November 2, 2020 18:01
> To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; Xueming(Steven)
> Li <xuemingl@nvidia.com>; Andrew Rybchenko <arybchenko@solarflare.com>;
> dev@dpdk.org; declan.doherty@intel.com
> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-Thomas Monjalon
> <thomas@monjalon.net>; Ray Kinsella <mdr@ashroe.eu>; Neil Horman
> <nhorman@tuxdriver.com>; Ori Kam <orika@nvidia.com>; Wei Hu (Xavier)
> <xavier.huwei@huawei.com>; Min Hu (Connor) <humin29@huawei.com>;
> Yisen Zhuang <yisen.zhuang@huawei.com>; Lijun Ou <oulijun@huawei.com>;
> Matan Azrad <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>;
> Slava Ovsiienko <viacheslavo@nvidia.com>; Jasvinder Singh
> <jasvinder.singh@intel.com>; Cristian Dumitrescu
> <cristian.dumitrescu@intel.com>; Ajit Khaparde
> <ajit.khaparde@broadcom.com>; Somnath Kotur
> <somnath.kotur@broadcom.com>; Qiming Yang <qiming.yang@intel.com>; Qi
> Zhang <qi.z.zhang@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-bounces@dpdk.org> On Behalf Of Andrew Rybchenko
> >>> Sent: Thursday, October 29, 2020 4:53 PM
> >>> To: dev@dpdk.org
> >>> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-Thomas
> >>> Monjalon <thomas@monjalon.net>; Ferruh Yigit
> >>> <ferruh.yigit@intel.com>; Ray Kinsella <mdr@ashroe.eu>; Neil Horman
> >>> <nhorman@tuxdriver.com>; Ori Kam <orika@nvidia.com>; Andrew
> >>> Rybchenko <andrew.rybchenko@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 <arybchenko@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.yigit@intel.com>

It depends whether we are going to support multiple counters for the same flow.
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.



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute
  2020-11-02 16:12       ` Slava Ovsiienko
@ 2020-11-02 16:40         ` Andrew Rybchenko
  2020-11-02 17:48           ` Slava Ovsiienko
  0 siblings, 1 reply; 26+ messages in thread
From: Andrew Rybchenko @ 2020-11-02 16:40 UTC (permalink / raw)
  To: Slava Ovsiienko, Ferruh Yigit, Xueming(Steven) Li,
	Andrew Rybchenko, dev, declan.doherty
  Cc: Andrey Vesnovaty, NBU-Contact-Thomas Monjalon, Ray Kinsella,
	Neil Horman, Ori Kam, Wei Hu (Xavier), Min Hu (Connor),
	Yisen Zhuang, Lijun Ou, Matan Azrad, Shahaf Shuler,
	Jasvinder Singh, Cristian Dumitrescu, Ajit Khaparde,
	Somnath Kotur, Qiming Yang, Qi Zhang

On 11/2/20 7:12 PM, Slava Ovsiienko wrote:
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Monday, November 2, 2020 18:01
>> To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; Xueming(Steven)
>> Li <xuemingl@nvidia.com>; Andrew Rybchenko <arybchenko@solarflare.com>;
>> dev@dpdk.org; declan.doherty@intel.com
>> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-Thomas Monjalon
>> <thomas@monjalon.net>; Ray Kinsella <mdr@ashroe.eu>; Neil Horman
>> <nhorman@tuxdriver.com>; Ori Kam <orika@nvidia.com>; Wei Hu (Xavier)
>> <xavier.huwei@huawei.com>; Min Hu (Connor) <humin29@huawei.com>;
>> Yisen Zhuang <yisen.zhuang@huawei.com>; Lijun Ou <oulijun@huawei.com>;
>> Matan Azrad <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>;
>> Slava Ovsiienko <viacheslavo@nvidia.com>; Jasvinder Singh
>> <jasvinder.singh@intel.com>; Cristian Dumitrescu
>> <cristian.dumitrescu@intel.com>; Ajit Khaparde
>> <ajit.khaparde@broadcom.com>; Somnath Kotur
>> <somnath.kotur@broadcom.com>; Qiming Yang <qiming.yang@intel.com>; Qi
>> Zhang <qi.z.zhang@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-bounces@dpdk.org> On Behalf Of Andrew Rybchenko
>>>>> Sent: Thursday, October 29, 2020 4:53 PM
>>>>> To: dev@dpdk.org
>>>>> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-Thomas
>>>>> Monjalon <thomas@monjalon.net>; Ferruh Yigit
>>>>> <ferruh.yigit@intel.com>; Ray Kinsella <mdr@ashroe.eu>; Neil Horman
>>>>> <nhorman@tuxdriver.com>; Ori Kam <orika@nvidia.com>; Andrew
>>>>> Rybchenko <andrew.rybchenko@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 <arybchenko@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.yigit@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.
> 
> 


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute
  2020-11-02 16:40         ` Andrew Rybchenko
@ 2020-11-02 17:48           ` Slava Ovsiienko
  2020-11-02 17:55             ` Andrew Rybchenko
  0 siblings, 1 reply; 26+ messages in thread
From: Slava Ovsiienko @ 2020-11-02 17:48 UTC (permalink / raw)
  To: Andrew Rybchenko, Ferruh Yigit, Xueming(Steven) Li,
	Andrew Rybchenko, dev, declan.doherty
  Cc: Andrey Vesnovaty, NBU-Contact-Thomas Monjalon, Ray Kinsella,
	Neil Horman, Ori Kam, Wei Hu (Xavier), Min Hu (Connor),
	Yisen Zhuang, Lijun Ou, Matan Azrad, Shahaf Shuler,
	Jasvinder Singh, Cristian Dumitrescu, Ajit Khaparde,
	Somnath Kotur, Qiming Yang, Qi Zhang

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Monday, November 2, 2020 18:40
> To: Slava Ovsiienko <viacheslavo@nvidia.com>; Ferruh Yigit
> <ferruh.yigit@intel.com>; Xueming(Steven) Li <xuemingl@nvidia.com>;
> Andrew Rybchenko <arybchenko@solarflare.com>; dev@dpdk.org;
> declan.doherty@intel.com
> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-Thomas Monjalon
> <thomas@monjalon.net>; Ray Kinsella <mdr@ashroe.eu>; Neil Horman
> <nhorman@tuxdriver.com>; Ori Kam <orika@nvidia.com>; Wei Hu (Xavier)
> <xavier.huwei@huawei.com>; Min Hu (Connor) <humin29@huawei.com>;
> Yisen Zhuang <yisen.zhuang@huawei.com>; Lijun Ou <oulijun@huawei.com>;
> Matan Azrad <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>;
> Jasvinder Singh <jasvinder.singh@intel.com>; Cristian Dumitrescu
> <cristian.dumitrescu@intel.com>; Ajit Khaparde
> <ajit.khaparde@broadcom.com>; Somnath Kotur
> <somnath.kotur@broadcom.com>; Qiming Yang <qiming.yang@intel.com>; Qi
> Zhang <qi.z.zhang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using
> action attribute
> 
> On 11/2/20 7:12 PM, Slava Ovsiienko wrote:
> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >> Sent: Monday, November 2, 2020 18:01
> >> To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>;
> Xueming(Steven)
> >> Li <xuemingl@nvidia.com>; Andrew Rybchenko
> >> <arybchenko@solarflare.com>; dev@dpdk.org; declan.doherty@intel.com
> >> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-Thomas
> >> Monjalon <thomas@monjalon.net>; Ray Kinsella <mdr@ashroe.eu>; Neil
> >> Horman <nhorman@tuxdriver.com>; Ori Kam <orika@nvidia.com>; Wei Hu
> >> (Xavier) <xavier.huwei@huawei.com>; Min Hu (Connor)
> >> <humin29@huawei.com>; Yisen Zhuang <yisen.zhuang@huawei.com>;
> Lijun
> >> Ou <oulijun@huawei.com>; Matan Azrad <matan@nvidia.com>; Shahaf
> >> Shuler <shahafs@nvidia.com>; Slava Ovsiienko
> >> <viacheslavo@nvidia.com>; Jasvinder Singh
> >> <jasvinder.singh@intel.com>; Cristian Dumitrescu
> >> <cristian.dumitrescu@intel.com>; Ajit Khaparde
> >> <ajit.khaparde@broadcom.com>; Somnath Kotur
> >> <somnath.kotur@broadcom.com>; Qiming Yang <qiming.yang@intel.com>;
> Qi
> >> Zhang <qi.z.zhang@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-bounces@dpdk.org> On Behalf Of Andrew Rybchenko
> >>>>> Sent: Thursday, October 29, 2020 4:53 PM
> >>>>> To: dev@dpdk.org
> >>>>> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-Thomas
> >>>>> Monjalon <thomas@monjalon.net>; Ferruh Yigit
> >>>>> <ferruh.yigit@intel.com>; Ray Kinsella <mdr@ashroe.eu>; Neil
> >>>>> Horman <nhorman@tuxdriver.com>; Ori Kam <orika@nvidia.com>;
> Andrew
> >>>>> Rybchenko <andrew.rybchenko@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 <arybchenko@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.yigit@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.
> 
Honestly, I wonder if someone wants to use multiple counters in the same flow.
That might happen if we add some unique attributes to the counter action
(say, count some complex events/traffic params).

Action pointer in the rte_flow_query() is just a pointer to some counter action
describing the counter. If we drop id field (and only action type COUNTER remains
in action description) - there would be no way to distinguish two (regular, not shared)
counters in the flow. Which one should be returned on query?  

> > 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.
> >
> >


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute
  2020-11-02 17:48           ` Slava Ovsiienko
@ 2020-11-02 17:55             ` Andrew Rybchenko
  2020-11-03 13:30               ` Xueming(Steven) Li
  2020-11-03 13:34               ` Xueming(Steven) Li
  0 siblings, 2 replies; 26+ messages in thread
From: Andrew Rybchenko @ 2020-11-02 17:55 UTC (permalink / raw)
  To: Slava Ovsiienko, Ferruh Yigit, Xueming(Steven) Li,
	Andrew Rybchenko, dev, declan.doherty
  Cc: Andrey Vesnovaty, NBU-Contact-Thomas Monjalon, Ray Kinsella,
	Neil Horman, Ori Kam, Wei Hu (Xavier), Min Hu (Connor),
	Yisen Zhuang, Lijun Ou, Matan Azrad, Shahaf Shuler,
	Jasvinder Singh, Cristian Dumitrescu, Ajit Khaparde,
	Somnath Kotur, Qiming Yang, Qi Zhang

On 11/2/20 8:48 PM, Slava Ovsiienko wrote:
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Monday, November 2, 2020 18:40
>> To: Slava Ovsiienko <viacheslavo@nvidia.com>; Ferruh Yigit
>> <ferruh.yigit@intel.com>; Xueming(Steven) Li <xuemingl@nvidia.com>;
>> Andrew Rybchenko <arybchenko@solarflare.com>; dev@dpdk.org;
>> declan.doherty@intel.com
>> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-Thomas Monjalon
>> <thomas@monjalon.net>; Ray Kinsella <mdr@ashroe.eu>; Neil Horman
>> <nhorman@tuxdriver.com>; Ori Kam <orika@nvidia.com>; Wei Hu (Xavier)
>> <xavier.huwei@huawei.com>; Min Hu (Connor) <humin29@huawei.com>;
>> Yisen Zhuang <yisen.zhuang@huawei.com>; Lijun Ou <oulijun@huawei.com>;
>> Matan Azrad <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>;
>> Jasvinder Singh <jasvinder.singh@intel.com>; Cristian Dumitrescu
>> <cristian.dumitrescu@intel.com>; Ajit Khaparde
>> <ajit.khaparde@broadcom.com>; Somnath Kotur
>> <somnath.kotur@broadcom.com>; Qiming Yang <qiming.yang@intel.com>; Qi
>> Zhang <qi.z.zhang@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using
>> action attribute
>>
>> On 11/2/20 7:12 PM, Slava Ovsiienko wrote:
>>>> -----Original Message-----
>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>> Sent: Monday, November 2, 2020 18:01
>>>> To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>;
>> Xueming(Steven)
>>>> Li <xuemingl@nvidia.com>; Andrew Rybchenko
>>>> <arybchenko@solarflare.com>; dev@dpdk.org; declan.doherty@intel.com
>>>> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-Thomas
>>>> Monjalon <thomas@monjalon.net>; Ray Kinsella <mdr@ashroe.eu>; Neil
>>>> Horman <nhorman@tuxdriver.com>; Ori Kam <orika@nvidia.com>; Wei Hu
>>>> (Xavier) <xavier.huwei@huawei.com>; Min Hu (Connor)
>>>> <humin29@huawei.com>; Yisen Zhuang <yisen.zhuang@huawei.com>;
>> Lijun
>>>> Ou <oulijun@huawei.com>; Matan Azrad <matan@nvidia.com>; Shahaf
>>>> Shuler <shahafs@nvidia.com>; Slava Ovsiienko
>>>> <viacheslavo@nvidia.com>; Jasvinder Singh
>>>> <jasvinder.singh@intel.com>; Cristian Dumitrescu
>>>> <cristian.dumitrescu@intel.com>; Ajit Khaparde
>>>> <ajit.khaparde@broadcom.com>; Somnath Kotur
>>>> <somnath.kotur@broadcom.com>; Qiming Yang <qiming.yang@intel.com>;
>> Qi
>>>> Zhang <qi.z.zhang@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-bounces@dpdk.org> On Behalf Of Andrew Rybchenko
>>>>>>> Sent: Thursday, October 29, 2020 4:53 PM
>>>>>>> To: dev@dpdk.org
>>>>>>> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-Thomas
>>>>>>> Monjalon <thomas@monjalon.net>; Ferruh Yigit
>>>>>>> <ferruh.yigit@intel.com>; Ray Kinsella <mdr@ashroe.eu>; Neil
>>>>>>> Horman <nhorman@tuxdriver.com>; Ori Kam <orika@nvidia.com>;
>> Andrew
>>>>>>> Rybchenko <andrew.rybchenko@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 <arybchenko@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.yigit@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.
>>
> Honestly, I wonder if someone wants to use multiple counters in the same flow.
> That might happen if we add some unique attributes to the counter action
> (say, count some complex events/traffic params).
> 

It might make sense for shared counters, but it they are done using
shared actions, we don't need ID as well.

> Action pointer in the rte_flow_query() is just a pointer to some counter action
> describing the counter. If we drop id field (and only action type COUNTER remains
> in action description) - there would be no way to distinguish two (regular, not shared)
> counters in the flow. Which one should be returned on query?
>

I thought that 'action' must be exactly the same pointer to actions
array element specified on flow creation. It looks like I'm wrong.
If so, yes, id is still required for non-shared many COUNT actions
in the same flow. I doubt that we need it as well.
Thanks for clarification.

>>> 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.
>>>
>>>
> 


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute
  2020-11-02 17:55             ` Andrew Rybchenko
@ 2020-11-03 13:30               ` Xueming(Steven) Li
  2020-11-03 13:34               ` Xueming(Steven) Li
  1 sibling, 0 replies; 26+ messages in thread
From: Xueming(Steven) Li @ 2020-11-03 13:30 UTC (permalink / raw)
  To: Andrew Rybchenko, Slava Ovsiienko, Ferruh Yigit,
	Andrew Rybchenko, dev, declan.doherty
  Cc: Andrey Vesnovaty, NBU-Contact-Thomas Monjalon, Ray Kinsella,
	Neil Horman, Ori Kam, Wei Hu (Xavier), Min Hu (Connor),
	Yisen Zhuang, Lijun Ou, Matan Azrad, Shahaf Shuler,
	Jasvinder Singh, Cristian Dumitrescu, Ajit Khaparde,
	Somnath Kotur, Qiming Yang, Qi Zhang


> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Tuesday, November 3, 2020 1:56 AM
> To: Slava Ovsiienko <viacheslavo@nvidia.com>; Ferruh Yigit
> <ferruh.yigit@intel.com>; Xueming(Steven) Li <xuemingl@nvidia.com>;
> Andrew Rybchenko <arybchenko@solarflare.com>; dev@dpdk.org;
> declan.doherty@intel.com
> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-Thomas
> Monjalon <thomas@monjalon.net>; Ray Kinsella <mdr@ashroe.eu>; Neil
> Horman <nhorman@tuxdriver.com>; Ori Kam <orika@nvidia.com>; Wei Hu
> (Xavier) <xavier.huwei@huawei.com>; Min Hu (Connor)
> <humin29@huawei.com>; Yisen Zhuang <yisen.zhuang@huawei.com>; Lijun
> Ou <oulijun@huawei.com>; Matan Azrad <matan@nvidia.com>; Shahaf
> Shuler <shahafs@nvidia.com>; Jasvinder Singh <jasvinder.singh@intel.com>;
> Cristian Dumitrescu <cristian.dumitrescu@intel.com>; Ajit Khaparde
> <ajit.khaparde@broadcom.com>; Somnath Kotur
> <somnath.kotur@broadcom.com>; Qiming Yang <qiming.yang@intel.com>;
> Qi Zhang <qi.z.zhang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using
> action attribute
> 
> On 11/2/20 8:48 PM, Slava Ovsiienko wrote:
> >> -----Original Message-----
> >> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Sent: Monday, November 2, 2020 18:40
> >> To: Slava Ovsiienko <viacheslavo@nvidia.com>; Ferruh Yigit
> >> <ferruh.yigit@intel.com>; Xueming(Steven) Li <xuemingl@nvidia.com>;
> >> Andrew Rybchenko <arybchenko@solarflare.com>; dev@dpdk.org;
> >> declan.doherty@intel.com
> >> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-Thomas
> >> Monjalon <thomas@monjalon.net>; Ray Kinsella <mdr@ashroe.eu>; Neil
> >> Horman <nhorman@tuxdriver.com>; Ori Kam <orika@nvidia.com>; Wei
> Hu
> >> (Xavier) <xavier.huwei@huawei.com>; Min Hu (Connor)
> >> <humin29@huawei.com>; Yisen Zhuang <yisen.zhuang@huawei.com>;
> Lijun
> >> Ou <oulijun@huawei.com>; Matan Azrad <matan@nvidia.com>; Shahaf
> >> Shuler <shahafs@nvidia.com>; Jasvinder Singh
> >> <jasvinder.singh@intel.com>; Cristian Dumitrescu
> >> <cristian.dumitrescu@intel.com>; Ajit Khaparde
> >> <ajit.khaparde@broadcom.com>; Somnath Kotur
> >> <somnath.kotur@broadcom.com>; Qiming Yang
> <qiming.yang@intel.com>; Qi
> >> Zhang <qi.z.zhang@intel.com>
> >> Subject: Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters
> >> using action attribute
> >>
> >> On 11/2/20 7:12 PM, Slava Ovsiienko wrote:
> >>>> -----Original Message-----
> >>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >>>> Sent: Monday, November 2, 2020 18:01
> >>>> To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>;
> >> Xueming(Steven)
> >>>> Li <xuemingl@nvidia.com>; Andrew Rybchenko
> >>>> <arybchenko@solarflare.com>; dev@dpdk.org;
> declan.doherty@intel.com
> >>>> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-Thomas
> >>>> Monjalon <thomas@monjalon.net>; Ray Kinsella <mdr@ashroe.eu>;
> Neil
> >>>> Horman <nhorman@tuxdriver.com>; Ori Kam <orika@nvidia.com>;
> Wei Hu
> >>>> (Xavier) <xavier.huwei@huawei.com>; Min Hu (Connor)
> >>>> <humin29@huawei.com>; Yisen Zhuang <yisen.zhuang@huawei.com>;
> >> Lijun
> >>>> Ou <oulijun@huawei.com>; Matan Azrad <matan@nvidia.com>;
> Shahaf
> >>>> Shuler <shahafs@nvidia.com>; Slava Ovsiienko
> >>>> <viacheslavo@nvidia.com>; Jasvinder Singh
> >>>> <jasvinder.singh@intel.com>; Cristian Dumitrescu
> >>>> <cristian.dumitrescu@intel.com>; Ajit Khaparde
> >>>> <ajit.khaparde@broadcom.com>; Somnath Kotur
> >>>> <somnath.kotur@broadcom.com>; Qiming Yang
> <qiming.yang@intel.com>;
> >> Qi
> >>>> Zhang <qi.z.zhang@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-bounces@dpdk.org> On Behalf Of Andrew
> Rybchenko
> >>>>>>> Sent: Thursday, October 29, 2020 4:53 PM
> >>>>>>> To: dev@dpdk.org
> >>>>>>> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-
> Thomas
> >>>>>>> Monjalon <thomas@monjalon.net>; Ferruh Yigit
> >>>>>>> <ferruh.yigit@intel.com>; Ray Kinsella <mdr@ashroe.eu>; Neil
> >>>>>>> Horman <nhorman@tuxdriver.com>; Ori Kam <orika@nvidia.com>;
> >> Andrew
> >>>>>>> Rybchenko <andrew.rybchenko@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 <arybchenko@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.yigit@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.
> >>
> > Honestly, I wonder if someone wants to use multiple counters in the same
> flow.
> > That might happen if we add some unique attributes to the counter
> > action (say, count some complex events/traffic params).
> >
> 
> It might make sense for shared counters, but it they are done using shared
> actions, we don't need ID as well.
> 

Flow 5 tuple flows, besides the per flow counter, some customer wants 
shared counter per stream type, per VM and per customer(owns group of VMs).
Shared counter action should work.


> > Action pointer in the rte_flow_query() is just a pointer to some
> > counter action describing the counter. If we drop id field (and only
> > action type COUNTER remains in action description) - there would be no
> > way to distinguish two (regular, not shared) counters in the flow. Which
> one should be returned on query?
> >
> 
> I thought that 'action' must be exactly the same pointer to actions array
> element specified on flow creation. It looks like I'm wrong.
> If so, yes, id is still required for non-shared many COUNT actions in the same
> flow. I doubt that we need it as well.
> Thanks for clarification.
> 

By using shared counter action, only one per flow counter allowed, no need
to support multiple per flow counters IMHO.

In current design, the user specified counter ID field of shared counter implies
additional mapping with PMD internal counter - another reason to deprecate
counter ID.

> >>> 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.
> >>>
> >>>
> >


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute
  2020-11-02 17:55             ` Andrew Rybchenko
  2020-11-03 13:30               ` Xueming(Steven) Li
@ 2020-11-03 13:34               ` Xueming(Steven) Li
  1 sibling, 0 replies; 26+ messages in thread
From: Xueming(Steven) Li @ 2020-11-03 13:34 UTC (permalink / raw)
  To: Andrew Rybchenko, Slava Ovsiienko, Ferruh Yigit,
	Andrew Rybchenko, dev, declan.doherty
  Cc: Andrey Vesnovaty, NBU-Contact-Thomas Monjalon, Ray Kinsella,
	Neil Horman, Ori Kam, Matan Azrad, Shahaf Shuler,
	Jasvinder Singh, Cristian Dumitrescu, Ajit Khaparde,
	Somnath Kotur, Qiming Yang, Qi Zhang


> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Tuesday, November 3, 2020 1:56 AM
> To: Slava Ovsiienko <viacheslavo@nvidia.com>; Ferruh Yigit
> <ferruh.yigit@intel.com>; Xueming(Steven) Li <xuemingl@nvidia.com>;
> Andrew Rybchenko <arybchenko@solarflare.com>; dev@dpdk.org;
> declan.doherty@intel.com
> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-Thomas
> Monjalon <thomas@monjalon.net>; Ray Kinsella <mdr@ashroe.eu>; Neil
> Horman <nhorman@tuxdriver.com>; Ori Kam <orika@nvidia.com>; Wei Hu
> (Xavier) <xavier.huwei@huawei.com>; Min Hu (Connor)
> <humin29@huawei.com>; Yisen Zhuang <yisen.zhuang@huawei.com>; Lijun
> Ou <oulijun@huawei.com>; Matan Azrad <matan@nvidia.com>; Shahaf
> Shuler <shahafs@nvidia.com>; Jasvinder Singh <jasvinder.singh@intel.com>;
> Cristian Dumitrescu <cristian.dumitrescu@intel.com>; Ajit Khaparde
> <ajit.khaparde@broadcom.com>; Somnath Kotur
> <somnath.kotur@broadcom.com>; Qiming Yang <qiming.yang@intel.com>;
> Qi Zhang <qi.z.zhang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using
> action attribute
> 
> On 11/2/20 8:48 PM, Slava Ovsiienko wrote:
> >> -----Original Message-----
> >> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Sent: Monday, November 2, 2020 18:40
> >> To: Slava Ovsiienko <viacheslavo@nvidia.com>; Ferruh Yigit
> >> <ferruh.yigit@intel.com>; Xueming(Steven) Li <xuemingl@nvidia.com>;
> >> Andrew Rybchenko <arybchenko@solarflare.com>; dev@dpdk.org;
> >> declan.doherty@intel.com
> >> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-Thomas
> >> Monjalon <thomas@monjalon.net>; Ray Kinsella <mdr@ashroe.eu>; Neil
> >> Horman <nhorman@tuxdriver.com>; Ori Kam <orika@nvidia.com>; Wei
> Hu
> >> (Xavier) <xavier.huwei@huawei.com>; Min Hu (Connor)
> >> <humin29@huawei.com>; Yisen Zhuang <yisen.zhuang@huawei.com>;
> Lijun
> >> Ou <oulijun@huawei.com>; Matan Azrad <matan@nvidia.com>; Shahaf
> >> Shuler <shahafs@nvidia.com>; Jasvinder Singh
> >> <jasvinder.singh@intel.com>; Cristian Dumitrescu
> >> <cristian.dumitrescu@intel.com>; Ajit Khaparde
> >> <ajit.khaparde@broadcom.com>; Somnath Kotur
> >> <somnath.kotur@broadcom.com>; Qiming Yang
> <qiming.yang@intel.com>; Qi
> >> Zhang <qi.z.zhang@intel.com>
> >> Subject: Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters
> >> using action attribute
> >>
> >> On 11/2/20 7:12 PM, Slava Ovsiienko wrote:
> >>>> -----Original Message-----
> >>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >>>> Sent: Monday, November 2, 2020 18:01
> >>>> To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>;
> >> Xueming(Steven)
> >>>> Li <xuemingl@nvidia.com>; Andrew Rybchenko
> >>>> <arybchenko@solarflare.com>; dev@dpdk.org;
> declan.doherty@intel.com
> >>>> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-Thomas
> >>>> Monjalon <thomas@monjalon.net>; Ray Kinsella <mdr@ashroe.eu>;
> Neil
> >>>> Horman <nhorman@tuxdriver.com>; Ori Kam <orika@nvidia.com>;
> Wei Hu
> >>>> (Xavier) <xavier.huwei@huawei.com>; Min Hu (Connor)
> >>>> <humin29@huawei.com>; Yisen Zhuang <yisen.zhuang@huawei.com>;
> >> Lijun
> >>>> Ou <oulijun@huawei.com>; Matan Azrad <matan@nvidia.com>;
> Shahaf
> >>>> Shuler <shahafs@nvidia.com>; Slava Ovsiienko
> >>>> <viacheslavo@nvidia.com>; Jasvinder Singh
> >>>> <jasvinder.singh@intel.com>; Cristian Dumitrescu
> >>>> <cristian.dumitrescu@intel.com>; Ajit Khaparde
> >>>> <ajit.khaparde@broadcom.com>; Somnath Kotur
> >>>> <somnath.kotur@broadcom.com>; Qiming Yang
> <qiming.yang@intel.com>;
> >> Qi
> >>>> Zhang <qi.z.zhang@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-bounces@dpdk.org> On Behalf Of Andrew
> Rybchenko
> >>>>>>> Sent: Thursday, October 29, 2020 4:53 PM
> >>>>>>> To: dev@dpdk.org
> >>>>>>> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-
> Thomas
> >>>>>>> Monjalon <thomas@monjalon.net>; Ferruh Yigit
> >>>>>>> <ferruh.yigit@intel.com>; Ray Kinsella <mdr@ashroe.eu>; Neil
> >>>>>>> Horman <nhorman@tuxdriver.com>; Ori Kam <orika@nvidia.com>;
> >> Andrew
> >>>>>>> Rybchenko <andrew.rybchenko@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 <arybchenko@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.yigit@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.
> >>
> > Honestly, I wonder if someone wants to use multiple counters in the same
> flow.
> > That might happen if we add some unique attributes to the counter
> > action (say, count some complex events/traffic params).
> >
> 
> It might make sense for shared counters, but it they are done using shared
> actions, we don't need ID as well.
> 

Flow 5 tuple flows, besides the per flow counter, some customer wants 
shared counter per stream type, per VM and per customer(owns group of VMs).
Shared counter action should work.


> > Action pointer in the rte_flow_query() is just a pointer to some
> > counter action describing the counter. If we drop id field (and only
> > action type COUNTER remains in action description) - there would be no
> > way to distinguish two (regular, not shared) counters in the flow. Which
> one should be returned on query?
> >
> 
> I thought that 'action' must be exactly the same pointer to actions array
> element specified on flow creation. It looks like I'm wrong.
> If so, yes, id is still required for non-shared many COUNT actions in the same
> flow. I doubt that we need it as well.
> Thanks for clarification.
> 

By using shared counter action, only one per flow counter allowed, no need
to support multiple per flow counters IMHO.

In current design, the user specified counter ID field of shared counter implies
additional mapping with PMD internal counter - another reason to deprecate
counter ID.

> >>> 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.
> >>>
> >>>
> >


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [dpdk-dev] [PATCH v2] ethdev: deprecate shared counters using action attribute
  2020-10-29  8:52 [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute Andrew Rybchenko
  2020-10-29 14:39 ` Ori Kam
  2020-10-30 16:12 ` Xueming(Steven) Li
@ 2020-11-03 14:26 ` Andrew Rybchenko
  2020-11-12 13:21   ` Ferruh Yigit
  2 siblings, 1 reply; 26+ messages in thread
From: Andrew Rybchenko @ 2020-11-03 14:26 UTC (permalink / raw)
  To: dev
  Cc: Ori Kam, Xueming(Steven) Li, Slava Ovsiienko, Ray Kinsella,
	Neil Horman, Thomas Monjalon, Ferruh Yigit, Andrew Rybchenko

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 <arybchenko@solarflare.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: Ori Kam <orika@nvidia.com>
Cc: Xueming(Steven) Li <xuemingl@mellanox.com>
Cc: Slava Ovsiienko <viacheslavo@nvidia.com>

Right now it is suggested do not touch ID and keep it as is.
Anyway we have plenty of time before 21.11 when it can be removed.
However it is important to highlight the right direction for shared
counters now.

 doc/guides/prog_guide/rte_flow.rst   | 17 ++++++++++-------
 doc/guides/rel_notes/deprecation.rst |  4 ++++
 lib/librte_ethdev/rte_flow.h         |  6 +++++-
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index ea203e0..64b533b 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1724,17 +1724,20 @@ that counter.
 For ports within the same switch domain then the counter id namespace extends
 to all ports within that switch domain.
 
+The shared flag is DEPRECATED and ``SHARED`` ``COUNT`` action should be used
+to make shared counters.
+
 .. _table_rte_flow_action_count:
 
 .. table:: COUNT
 
-   +------------+---------------------+
-   | Field      | Value               |
-   +============+=====================+
-   | ``shared`` | shared counter flag |
-   +------------+---------------------+
-   | ``id``     | counter id          |
-   +------------+---------------------+
+   +------------+---------------------------------+
+   | Field      | Value                           |
+   +============+=================================+
+   | ``shared`` | DEPRECATED, shared counter flag |
+   +------------+---------------------------------+
+   | ``id``     | counter id                      |
+   +------------+---------------------------------+
 
 Query structure to retrieve and reset flow rule counters:
 
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index c2d3457..7f3c2e7 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -122,6 +122,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 a8eac4d..2bb93d2 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. */
 };
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute
  2020-11-01  7:49     ` Ori Kam
@ 2020-11-03 17:21       ` Thomas Monjalon
  2020-11-03 17:26         ` Andrew Rybchenko
  0 siblings, 1 reply; 26+ messages in thread
From: Thomas Monjalon @ 2020-11-03 17:21 UTC (permalink / raw)
  To: Andrew Rybchenko, Ori Kam
  Cc: dev, Andrew Rybchenko, Andrey Vesnovaty, Ferruh Yigit,
	Ray Kinsella, Neil Horman, techboard

+Cc techboard

There is an interesting case here that we should decide
how to manage in general. Please see below.

01/11/2020 08:49, Ori Kam:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 29/10/2020 15:39, Ori Kam:
> > > >  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. */
> > > >  };
> > >
> > > As much as I agree with your patch, I don't think we should push it since
> > > not all PMD made the move to support count action, so the application still
> > needs
> > > to use this API.
> > >
> > > I think this patch should be done but in next LTS release.
> > 
> > The patch is not removing the field,
> > it is just warning it will be removed in next year.
> 
> Yes I know, but I don't think it is correct to issue such a warning without support.
> The application still must use this API, the warning should be added as soon as
> at least one PMD support shared counters with the new API.

It should be replaced with shared actions API,
but you claim it is not supported yet. Right?
What are the criterias to define the replacement as supported?

What to do in such case?
Can we warn about a deprecation without having a proper replacement?
What is the pre-condition to warn about a deprecation?

About the complete removal, it has already been decided by the techboard
that we cannot remove an API until its replacement is stable.
In other words, the new experimental API must be promoted
in the stable ABI, before removing the deprecated API.



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute
  2020-11-03 17:21       ` Thomas Monjalon
@ 2020-11-03 17:26         ` Andrew Rybchenko
  0 siblings, 0 replies; 26+ messages in thread
From: Andrew Rybchenko @ 2020-11-03 17:26 UTC (permalink / raw)
  To: Thomas Monjalon, Ori Kam
  Cc: dev, Andrew Rybchenko, Andrey Vesnovaty, Ferruh Yigit,
	Ray Kinsella, Neil Horman, techboard

On 11/3/20 8:21 PM, Thomas Monjalon wrote:
> +Cc techboard
> 
> There is an interesting case here that we should decide
> how to manage in general. Please see below.
> 
> 01/11/2020 08:49, Ori Kam:
>> From: Thomas Monjalon <thomas@monjalon.net>
>>> 29/10/2020 15:39, Ori Kam:
>>>>>  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. */
>>>>>  };
>>>>
>>>> As much as I agree with your patch, I don't think we should push it since
>>>> not all PMD made the move to support count action, so the application still
>>> needs
>>>> to use this API.
>>>>
>>>> I think this patch should be done but in next LTS release.
>>>
>>> The patch is not removing the field,
>>> it is just warning it will be removed in next year.
>>
>> Yes I know, but I don't think it is correct to issue such a warning without support.
>> The application still must use this API, the warning should be added as soon as
>> at least one PMD support shared counters with the new API.
> 
> It should be replaced with shared actions API,
> but you claim it is not supported yet. Right?
> What are the criterias to define the replacement as supported?
> 
> What to do in such case?
> Can we warn about a deprecation without having a proper replacement?
> What is the pre-condition to warn about a deprecation?
> 
> About the complete removal, it has already been decided by the techboard
> that we cannot remove an API until its replacement is stable.
> In other words, the new experimental API must be promoted
> in the stable ABI, before removing the deprecated API.
> 

May be the right way here is to remove deprecation mark up,
but add a deprecation notice that it will be deprecated in
21.02 and PMDs are encouraged to switch to shared actions.

Anyway questions above about the criteria are still valid
even in this case and should be answered.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH v2] ethdev: deprecate shared counters using action attribute
  2020-11-03 14:26 ` [dpdk-dev] [PATCH v2] " Andrew Rybchenko
@ 2020-11-12 13:21   ` Ferruh Yigit
  2020-11-24 17:51     ` Ferruh Yigit
                       ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: Ferruh Yigit @ 2020-11-12 13:21 UTC (permalink / raw)
  To: Andrew Rybchenko, dev
  Cc: Ori Kam, Xueming(Steven) Li, Slava Ovsiienko, Ray Kinsella,
	Neil Horman, Thomas Monjalon, Andrew Rybchenko

On 11/3/2020 2:26 PM, Andrew Rybchenko wrote:
> 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 <arybchenko@solarflare.com>
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> Cc: Ori Kam <orika@nvidia.com>
> Cc: Xueming(Steven) Li <xuemingl@mellanox.com>
> Cc: Slava Ovsiienko <viacheslavo@nvidia.com>
> 

Ping for this deprecation notice.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH v2] ethdev: deprecate shared counters using action attribute
  2020-11-12 13:21   ` Ferruh Yigit
@ 2020-11-24 17:51     ` Ferruh Yigit
  2020-11-27 16:07     ` Bruce Richardson
  2020-11-27 16:08     ` Bruce Richardson
  2 siblings, 0 replies; 26+ messages in thread
From: Ferruh Yigit @ 2020-11-24 17:51 UTC (permalink / raw)
  To: Andrew Rybchenko, dev, Thomas Monjalon, Ori Kam, Slava Ovsiienko,
	Zhang, Qi Z, Ajit Khaparde, jerinj
  Cc: Xueming(Steven) Li, Ray Kinsella, Neil Horman, Andrew Rybchenko

On 11/12/2020 1:21 PM, Ferruh Yigit wrote:
> On 11/3/2020 2:26 PM, Andrew Rybchenko wrote:
>> 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 <arybchenko@solarflare.com>
>> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>> Cc: Ori Kam <orika@nvidia.com>
>> Cc: Xueming(Steven) Li <xuemingl@mellanox.com>
>> Cc: Slava Ovsiienko <viacheslavo@nvidia.com>
>>
> 
> Ping for this deprecation notice.
> 

For this one too added a few more people, removal target is 21.11, but it also 
marks the fields as deprecated, so it should be merged in this release.

Can you please review this deprecation notice?

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute
  2020-11-02 16:01     ` Ferruh Yigit
  2020-11-02 16:12       ` Slava Ovsiienko
@ 2020-11-24 18:09       ` Ajit Khaparde
  2020-11-26  9:08         ` Ori Kam
  1 sibling, 1 reply; 26+ messages in thread
From: Ajit Khaparde @ 2020-11-24 18:09 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Andrew Rybchenko, Xueming(Steven) Li, Andrew Rybchenko, dev,
	declan.doherty, Andrey Vesnovaty, NBU-Contact-Thomas Monjalon,
	Ray Kinsella, Neil Horman, Ori Kam, Wei Hu (Xavier),
	Min Hu (Connor),
	Yisen Zhuang, Lijun Ou, Matan Azrad, Shahaf Shuler,
	Viacheslav Ovsiienko, Jasvinder Singh, Cristian Dumitrescu,
	Somnath Kotur, Qiming Yang, Qi Zhang

On Mon, Nov 2, 2020 at 8:01 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> 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-bounces@dpdk.org> On Behalf Of Andrew Rybchenko
> >>> Sent: Thursday, October 29, 2020 4:53 PM
> >>> To: dev@dpdk.org
> >>> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-Thomas
> >>> Monjalon <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>;
> >>> Ray Kinsella <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>;
> >>> Ori Kam <orika@nvidia.com>; Andrew Rybchenko
> >>> <andrew.rybchenko@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 <arybchenko@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.yigit@intel.com>

Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute
  2020-11-24 18:09       ` Ajit Khaparde
@ 2020-11-26  9:08         ` Ori Kam
  2020-11-26 12:23           ` Ferruh Yigit
  0 siblings, 1 reply; 26+ messages in thread
From: Ori Kam @ 2020-11-26  9:08 UTC (permalink / raw)
  To: Ajit Khaparde, Ferruh Yigit
  Cc: Andrew Rybchenko, Xueming(Steven) Li, Andrew Rybchenko, dev,
	declan.doherty, Andrey Vesnovaty, NBU-Contact-Thomas Monjalon,
	Ray Kinsella, Neil Horman, Wei Hu (Xavier), Min Hu (Connor),
	Yisen Zhuang, Lijun Ou, Matan Azrad, Shahaf Shuler,
	Slava Ovsiienko, Jasvinder Singh, Cristian Dumitrescu,
	Somnath Kotur, Qiming Yang, Qi Zhang

Hi All

> -----Original Message-----
> From: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Sent: Tuesday, November 24, 2020 8:10 PM
> Subject: Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action
> attribute
> 
> On Mon, Nov 2, 2020 at 8:01 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >
> > 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-bounces@dpdk.org> On Behalf Of Andrew Rybchenko
> > >>> Sent: Thursday, October 29, 2020 4:53 PM
> > >>> To: dev@dpdk.org
> > >>> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-Thomas
> > >>> Monjalon <thomas@monjalon.net>; Ferruh Yigit
> <ferruh.yigit@intel.com>;
> > >>> Ray Kinsella <mdr@ashroe.eu>; Neil Horman
> <nhorman@tuxdriver.com>;
> > >>> Ori Kam <orika@nvidia.com>; Andrew Rybchenko
> > >>> <andrew.rybchenko@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 <arybchenko@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.yigit@intel.com>
> 
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

One small point, I know it was mention in some thread, can we also deprecate the id?
Maybe the entire struct? since there is not need for configuration any more.


 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute
  2020-11-26  9:08         ` Ori Kam
@ 2020-11-26 12:23           ` Ferruh Yigit
  2020-11-26 16:49             ` Ori Kam
  0 siblings, 1 reply; 26+ messages in thread
From: Ferruh Yigit @ 2020-11-26 12:23 UTC (permalink / raw)
  To: Ori Kam, Ajit Khaparde
  Cc: Andrew Rybchenko, Xueming(Steven) Li, Andrew Rybchenko, dev,
	declan.doherty, Andrey Vesnovaty, NBU-Contact-Thomas Monjalon,
	Ray Kinsella, Neil Horman, Wei Hu (Xavier), Min Hu (Connor),
	Yisen Zhuang, Lijun Ou, Matan Azrad, Shahaf Shuler,
	Slava Ovsiienko, Jasvinder Singh, Cristian Dumitrescu,
	Somnath Kotur, Qiming Yang, Qi Zhang

On 11/26/2020 9:08 AM, Ori Kam wrote:
> Hi All
> 
>> -----Original Message-----
>> From: Ajit Khaparde <ajit.khaparde@broadcom.com>
>> Sent: Tuesday, November 24, 2020 8:10 PM
>> Subject: Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action
>> attribute
>>
>> On Mon, Nov 2, 2020 at 8:01 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>>
>>> 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-bounces@dpdk.org> On Behalf Of Andrew Rybchenko
>>>>>> Sent: Thursday, October 29, 2020 4:53 PM
>>>>>> To: dev@dpdk.org
>>>>>> Cc: Andrey Vesnovaty <andreyv@nvidia.com>; NBU-Contact-Thomas
>>>>>> Monjalon <thomas@monjalon.net>; Ferruh Yigit
>> <ferruh.yigit@intel.com>;
>>>>>> Ray Kinsella <mdr@ashroe.eu>; Neil Horman
>> <nhorman@tuxdriver.com>;
>>>>>> Ori Kam <orika@nvidia.com>; Andrew Rybchenko
>>>>>> <andrew.rybchenko@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 <arybchenko@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.yigit@intel.com>
>>
>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> 
> One small point, I know it was mention in some thread, can we also deprecate the id?
> Maybe the entire struct? since there is not need for configuration any more.
> 

There is alternative usage for 'shared' fields, that is the reason of the 
deprecation. But there are some usage of the 'id' without alternative, we can 
discuss if that usage is right or wrong later, but for now I think better to 
start with just this patch.

And if 'id' also decided to be deprecated later, yes we can remove entire struct 
in that case.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute
  2020-11-26 12:23           ` Ferruh Yigit
@ 2020-11-26 16:49             ` Ori Kam
  0 siblings, 0 replies; 26+ messages in thread
From: Ori Kam @ 2020-11-26 16:49 UTC (permalink / raw)
  To: Ferruh Yigit, Ajit Khaparde
  Cc: Andrew Rybchenko, Xueming(Steven) Li, Andrew Rybchenko, dev,
	declan.doherty, Andrey Vesnovaty, NBU-Contact-Thomas Monjalon,
	Ray Kinsella, Neil Horman, Wei Hu (Xavier), Min Hu (Connor),
	Yisen Zhuang, Lijun Ou, Matan Azrad, Shahaf Shuler,
	Slava Ovsiienko, Jasvinder Singh, Cristian Dumitrescu,
	Somnath Kotur, Qiming Yang, Qi Zhang



> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Thursday, November 26, 2020 2:23 PM
> Subject: Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action
> attribute
> > One small point, I know it was mention in some thread, can we also
> deprecate the id?
> > Maybe the entire struct? since there is not need for configuration any more.
> >
> 
> There is alternative usage for 'shared' fields, that is the reason of the
> deprecation. But there are some usage of the 'id' without alternative, we can
> discuss if that usage is right or wrong later, but for now I think better to
> start with just this patch.
> 
> And if 'id' also decided to be deprecated later, yes we can remove entire struct
> in that case.

Acked-by: Ori Kam <orika@nvidia.com>

Thanks,
Ori

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH v2] ethdev: deprecate shared counters using action attribute
  2020-11-12 13:21   ` Ferruh Yigit
  2020-11-24 17:51     ` Ferruh Yigit
@ 2020-11-27 16:07     ` Bruce Richardson
  2020-11-27 16:08     ` Bruce Richardson
  2 siblings, 0 replies; 26+ messages in thread
From: Bruce Richardson @ 2020-11-27 16:07 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Andrew Rybchenko, dev, Ori Kam, Xueming(Steven) Li,
	Slava Ovsiienko, Ray Kinsella, Neil Horman, Thomas Monjalon,
	Andrew Rybchenko

On Thu, Nov 12, 2020 at 01:21:20PM +0000, Ferruh Yigit wrote:
> On 11/3/2020 2:26 PM, Andrew Rybchenko wrote:
> > 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 <arybchenko@solarflare.com>
> > Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > ---
> > Cc: Ori Kam <orika@nvidia.com>
> > Cc: Xueming(Steven) Li <xuemingl@mellanox.com>
> > Cc: Slava Ovsiienko <viacheslavo@nvidia.com>
> > 
> 
> Ping for this deprecation notice.
> 
I believe this patch v2 is largely the same a v1 just with some added docs,
right? In which case the acks on v1 from Ajit and Ori should be kept, no?

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH v2] ethdev: deprecate shared counters using action attribute
  2020-11-12 13:21   ` Ferruh Yigit
  2020-11-24 17:51     ` Ferruh Yigit
  2020-11-27 16:07     ` Bruce Richardson
@ 2020-11-27 16:08     ` Bruce Richardson
  2020-11-27 17:43       ` Ajit Khaparde
  2 siblings, 1 reply; 26+ messages in thread
From: Bruce Richardson @ 2020-11-27 16:08 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Andrew Rybchenko, dev, Ori Kam, Xueming(Steven) Li,
	Slava Ovsiienko, Ray Kinsella, Neil Horman, Thomas Monjalon,
	Andrew Rybchenko

On Thu, Nov 12, 2020 at 01:21:20PM +0000, Ferruh Yigit wrote:
> On 11/3/2020 2:26 PM, Andrew Rybchenko wrote:
> > 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 <arybchenko@solarflare.com>
> > Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > ---
> > Cc: Ori Kam <orika@nvidia.com>
> > Cc: Xueming(Steven) Li <xuemingl@mellanox.com>
> > Cc: Slava Ovsiienko <viacheslavo@nvidia.com>
> > 
> 
> Ping for this deprecation notice.
> 
Reading through the patch history, seems the consensus is that this is
reasonable.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH v2] ethdev: deprecate shared counters using action attribute
  2020-11-27 16:08     ` Bruce Richardson
@ 2020-11-27 17:43       ` Ajit Khaparde
  2020-11-27 18:11         ` Thomas Monjalon
  0 siblings, 1 reply; 26+ messages in thread
From: Ajit Khaparde @ 2020-11-27 17:43 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: Ferruh Yigit, Andrew Rybchenko, dpdk-dev, Ori Kam,
	Xueming(Steven) Li, Slava Ovsiienko, Ray Kinsella, Neil Horman,
	Thomas Monjalon, Andrew Rybchenko

On Fri, Nov 27, 2020 at 8:08 AM Bruce Richardson <bruce.richardson@intel.com>
wrote:

> On Thu, Nov 12, 2020 at 01:21:20PM +0000, Ferruh Yigit wrote:
> > On 11/3/2020 2:26 PM, Andrew Rybchenko wrote:
> > > 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 <arybchenko@solarflare.com>
> > > Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > > ---
> > > Cc: Ori Kam <orika@nvidia.com>
> > > Cc: Xueming(Steven) Li <xuemingl@mellanox.com>
> > > Cc: Slava Ovsiienko <viacheslavo@nvidia.com>
> > >
> >
> > Ping for this deprecation notice.
> >
> Reading through the patch history, seems the consensus is that this is
> reasonable.
>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [dpdk-dev] [PATCH v2] ethdev: deprecate shared counters using action attribute
  2020-11-27 17:43       ` Ajit Khaparde
@ 2020-11-27 18:11         ` Thomas Monjalon
  0 siblings, 0 replies; 26+ messages in thread
From: Thomas Monjalon @ 2020-11-27 18:11 UTC (permalink / raw)
  To: Andrew Rybchenko
  Cc: Bruce Richardson, dev, Ferruh Yigit, Ori Kam, Xueming(Steven) Li,
	Slava Ovsiienko, Ray Kinsella, Neil Horman, Andrew Rybchenko,
	Ajit Khaparde

> > > > 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 <arybchenko@solarflare.com>
> > > > Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > > > ---
> > > > Cc: Ori Kam <orika@nvidia.com>
> > > > Cc: Xueming(Steven) Li <xuemingl@mellanox.com>
> > > > Cc: Slava Ovsiienko <viacheslavo@nvidia.com>
> > > >
> > >
> > > Ping for this deprecation notice.
> > >
> > Reading through the patch history, seems the consensus is that this is
> > reasonable.
> >
> > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> >
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

Applied, thanks



^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2020-11-27 18:12 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-29  8:52 [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute Andrew Rybchenko
2020-10-29 14:39 ` Ori Kam
2020-10-29 16:11   ` Thomas Monjalon
2020-11-01  7:49     ` Ori Kam
2020-11-03 17:21       ` Thomas Monjalon
2020-11-03 17:26         ` Andrew Rybchenko
2020-10-30 16:12 ` Xueming(Steven) Li
2020-11-01 10:45   ` Andrew Rybchenko
2020-11-02 16:01     ` Ferruh Yigit
2020-11-02 16:12       ` Slava Ovsiienko
2020-11-02 16:40         ` Andrew Rybchenko
2020-11-02 17:48           ` Slava Ovsiienko
2020-11-02 17:55             ` Andrew Rybchenko
2020-11-03 13:30               ` Xueming(Steven) Li
2020-11-03 13:34               ` Xueming(Steven) Li
2020-11-24 18:09       ` Ajit Khaparde
2020-11-26  9:08         ` Ori Kam
2020-11-26 12:23           ` Ferruh Yigit
2020-11-26 16:49             ` Ori Kam
2020-11-03 14:26 ` [dpdk-dev] [PATCH v2] " Andrew Rybchenko
2020-11-12 13:21   ` Ferruh Yigit
2020-11-24 17:51     ` Ferruh Yigit
2020-11-27 16:07     ` Bruce Richardson
2020-11-27 16:08     ` Bruce Richardson
2020-11-27 17:43       ` Ajit Khaparde
2020-11-27 18:11         ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).