From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 8F2FBA052A; Thu, 26 Nov 2020 13:23:25 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C88F5C93E; Thu, 26 Nov 2020 13:23:22 +0100 (CET) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id B93DBC93C for ; Thu, 26 Nov 2020 13:23:20 +0100 (CET) IronPort-SDR: S1MUgu+N5V1cDinXVPaTVReOAnT14vw2DZuOIIAkctjAPfqRMqQ+0vxSF+ka86YVJyB8CJwATk KulELHofRn6A== X-IronPort-AV: E=McAfee;i="6000,8403,9816"; a="233883391" X-IronPort-AV: E=Sophos;i="5.78,372,1599548400"; d="scan'208";a="233883391" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Nov 2020 04:23:18 -0800 IronPort-SDR: EOOs02bJVwjFYcJhNMXhscug44cTR178GFUX5MJhdV+YTThiM/pp+Mw7dCN3O5WP3M10yqI2T0 k32vSbTv9yUw== X-IronPort-AV: E=Sophos;i="5.78,372,1599548400"; d="scan'208";a="333359841" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.229.67]) ([10.213.229.67]) by orsmga006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Nov 2020 04:23:13 -0800 To: Ori Kam , Ajit Khaparde Cc: Andrew Rybchenko , "Xueming(Steven) Li" , Andrew Rybchenko , "dev@dpdk.org" , "declan.doherty@intel.com" , 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 References: <1603961577-13264-1-git-send-email-arybchenko@solarflare.com> <362bab6b-1dcc-2254-20a0-4ee62b0cd69b@oktetlabs.ru> From: Ferruh Yigit Message-ID: <2a54c24c-fce8-ac5c-5a93-2892c4033633@intel.com> Date: Thu, 26 Nov 2020 12:23:10 +0000 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH] ethdev: deprecate shared counters using action attribute X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 11/26/2020 9:08 AM, Ori Kam wrote: > Hi All > >> -----Original Message----- >> From: Ajit Khaparde >> 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 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 On Behalf Of Andrew Rybchenko >>>>>> Sent: Thursday, October 29, 2020 4:53 PM >>>>>> To: dev@dpdk.org >>>>>> Cc: Andrey Vesnovaty ; NBU-Contact-Thomas >>>>>> Monjalon ; Ferruh Yigit >> ; >>>>>> Ray Kinsella ; Neil Horman >> ; >>>>>> Ori Kam ; Andrew Rybchenko >>>>>> >>>>>> 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 >>>>>> --- >>>>>> 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 >> >> Acked-by: Ajit Khaparde > > 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.