> -----Original Message----- > From: Andrew Rybchenko <[email protected]> > Sent: Monday, November 2, 2020 18:40 > To: Slava Ovsiienko <[email protected]>; Ferruh Yigit > <[email protected]>; Xueming(Steven) Li <[email protected]>; > Andrew Rybchenko <[email protected]>; [email protected]; > [email protected] > Cc: Andrey Vesnovaty <[email protected]>; NBU-Contact-Thomas Monjalon > <[email protected]>; Ray Kinsella <[email protected]>; Neil Horman > <[email protected]>; Ori Kam <[email protected]>; Wei Hu (Xavier) > <[email protected]>; Min Hu (Connor) <[email protected]>; > Yisen Zhuang <[email protected]>; Lijun Ou <[email protected]>; > Matan Azrad <[email protected]>; Shahaf Shuler <[email protected]>; > Jasvinder Singh <[email protected]>; Cristian Dumitrescu > <[email protected]>; Ajit Khaparde > <[email protected]>; Somnath Kotur > <[email protected]>; Qiming Yang <[email protected]>; Qi > Zhang <[email protected]> > 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 <[email protected]> > >> Sent: Monday, November 2, 2020 18:01 > >> To: Andrew Rybchenko <[email protected]>; > Xueming(Steven) > >> Li <[email protected]>; Andrew Rybchenko > >> <[email protected]>; [email protected]; [email protected] > >> Cc: Andrey Vesnovaty <[email protected]>; NBU-Contact-Thomas > >> Monjalon <[email protected]>; Ray Kinsella <[email protected]>; Neil > >> Horman <[email protected]>; Ori Kam <[email protected]>; Wei Hu > >> (Xavier) <[email protected]>; Min Hu (Connor) > >> <[email protected]>; Yisen Zhuang <[email protected]>; > Lijun > >> Ou <[email protected]>; Matan Azrad <[email protected]>; Shahaf > >> Shuler <[email protected]>; Slava Ovsiienko > >> <[email protected]>; Jasvinder Singh > >> <[email protected]>; Cristian Dumitrescu > >> <[email protected]>; Ajit Khaparde > >> <[email protected]>; Somnath Kotur > >> <[email protected]>; Qiming Yang <[email protected]>; > Qi > >> Zhang <[email protected]> > >> 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 <[email protected]> On Behalf Of Andrew Rybchenko > >>>>> Sent: Thursday, October 29, 2020 4:53 PM > >>>>> To: [email protected] > >>>>> Cc: Andrey Vesnovaty <[email protected]>; NBU-Contact-Thomas > >>>>> Monjalon <[email protected]>; Ferruh Yigit > >>>>> <[email protected]>; Ray Kinsella <[email protected]>; Neil > >>>>> Horman <[email protected]>; Ori Kam <[email protected]>; > Andrew > >>>>> Rybchenko <[email protected]> > >>>>> 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 <[email protected]> > >>>>> --- > >>>>> 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 <[email protected]> > > > > 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. > > > >
