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 10701A04E7; Mon, 2 Nov 2020 17:40:11 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id ABC012D41; Mon, 2 Nov 2020 17:40:09 +0100 (CET) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by dpdk.org (Postfix) with ESMTP id 6DC3D2C01 for ; Mon, 2 Nov 2020 17:40:07 +0100 (CET) Received: from [192.168.1.192] (unknown [188.242.181.57]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 5E4347F66A; Mon, 2 Nov 2020 19:40:05 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 5E4347F66A DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1604335206; bh=w+FYvYkknsLNf/Bm2MCWNOVga+aX3tx4wgz3M5lskzA=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=a7CtaCiU9cQTO4968mk9OSoiB0P/QtgpMBk9zA5WSuuYgEnGoZK8h/6+HSgxautZ+ ClUEQhPpLzgEZdaGasTJXZo+5vh30ZsH5j6u1k5T1baPQ5HN2u5DfG7Yiua6DUG/VA gigiitCvEmz4841Jy66RJWs/u28kufs1N8778nSU= To: Slava Ovsiienko , Ferruh Yigit , "Xueming(Steven) Li" , Andrew Rybchenko , "dev@dpdk.org" , "declan.doherty@intel.com" 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 References: <1603961577-13264-1-git-send-email-arybchenko@solarflare.com> <362bab6b-1dcc-2254-20a0-4ee62b0cd69b@oktetlabs.ru> From: Andrew Rybchenko Message-ID: <77ee1d41-e3f4-b7c4-e268-c51448db7384@oktetlabs.ru> Date: Mon, 2 Nov 2020 19:40:04 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1 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/2/20 7:12 PM, Slava Ovsiienko wrote: >> -----Original Message----- >> From: Ferruh Yigit >> Sent: Monday, November 2, 2020 18:01 >> To: Andrew Rybchenko ; Xueming(Steven) >> Li ; Andrew Rybchenko ; >> dev@dpdk.org; declan.doherty@intel.com >> 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 ; >> Slava Ovsiienko ; Jasvinder Singh >> ; Cristian Dumitrescu >> ; Ajit Khaparde >> ; Somnath Kotur >> ; Qiming Yang ; Qi >> Zhang >> 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 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 > > 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. > >