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 AEFA9A04E7; Mon, 2 Nov 2020 18:55:41 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 326DF28EE; Mon, 2 Nov 2020 18:55:40 +0100 (CET) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by dpdk.org (Postfix) with ESMTP id 1EA001E2B for ; Mon, 2 Nov 2020 18:55:38 +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 2CF497F528; Mon, 2 Nov 2020 20:55:36 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 2CF497F528 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1604339736; bh=GJSX4SiCWvFwcpNB4xZr6dTD5feXaWY+mS/15zk8oY0=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=aTZPPSRTapSVIPB5ayUQ3UfHsm/cx1kU4wFxajr+Imygt5ETsy59eG+HvCp41hpHA tSBaOk0eTvbywHNlZ7zm5YVbuc+1Wb2LN8mwwPAH41L3suen+i3XDHLAsSl/wMDVtX EO9Xad1cH45ZhRrDDoSOEKMIkGTWMX+oGCfmgOkA= 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> <77ee1d41-e3f4-b7c4-e268-c51448db7384@oktetlabs.ru> From: Andrew Rybchenko Message-ID: <673524bd-67e3-1997-018c-70e4c830f9e3@oktetlabs.ru> Date: Mon, 2 Nov 2020 20:55:35 +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 8:48 PM, Slava Ovsiienko wrote: >> -----Original Message----- >> From: Andrew Rybchenko >> Sent: Monday, November 2, 2020 18:40 >> 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 >> 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 >>>> 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. >> > 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. >>> >>> >