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 58A13A0521; Tue, 3 Nov 2020 16:52:51 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 3ED46CBDB; Tue, 3 Nov 2020 16:52:50 +0100 (CET) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id 19B8ACBAB for ; Tue, 3 Nov 2020 16:52:47 +0100 (CET) IronPort-SDR: DRKrXHs8Vhvg+CgrmfBf5LUbmJo1mTE6yZugjkrLV2AGTe1VWkWpOMp4SYCPhLqzgn4Q8SLf7x 6Qz0ON/ZMTwQ== X-IronPort-AV: E=McAfee;i="6000,8403,9794"; a="148354243" X-IronPort-AV: E=Sophos;i="5.77,448,1596524400"; d="scan'208";a="148354243" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Nov 2020 07:52:46 -0800 IronPort-SDR: idgxb8ZI6/7r8Ou5HP29IzYj5LoG+zwInZlmWoq2UeyzF0AD+5l5dtUNfrxUasddvxxt0ZyaIq pUbaMkvmxPtw== X-IronPort-AV: E=Sophos;i="5.77,448,1596524400"; d="scan'208";a="538532688" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.218.178]) ([10.213.218.178]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Nov 2020 07:52:44 -0800 To: Ivan Malov , dev@dpdk.org, Andrey Vesnovaty Cc: Xueming Li , Ori Kam , Thomas Monjalon , Andrew Rybchenko References: <20201029114644.22169-1-ivan.malov@oktetlabs.ru> <20201102114317.24492-1-ivan.malov@oktetlabs.ru> <914ca03f-69ed-9cb0-44a3-1a3bf9af79f7@intel.com> <7a9b6c06-ac7e-e1f7-e6e9-6856d2bb90a3@oktetlabs.ru> From: Ferruh Yigit Message-ID: <3bcf1f2f-869c-590b-9c03-c2dc9b32d97b@intel.com> Date: Tue, 3 Nov 2020 15:52:40 +0000 MIME-Version: 1.0 In-Reply-To: <7a9b6c06-ac7e-e1f7-e6e9-6856d2bb90a3@oktetlabs.ru> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v3 1/2] ethdev: introduce transfer attribute to shared action conf 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/3/2020 2:10 PM, Ivan Malov wrote: > Hi Ferruh, > > On 02/11/2020 21:54, Ferruh Yigit wrote: >> On 11/2/2020 11:43 AM, Ivan Malov wrote: >>> In a flow rule, attribute "transfer" means operation level >>> at which both traffic is matched and actions are conducted. >>> >>> Add the very same attribute to shared action configuration. >>> If a driver needs to prepare HW resources in two different >>> ways, depending on the operation level, in order to set up >>> an action, then this new attribute will indicate the level. >>> Also, when handling a flow rule insertion, the driver will >>> be able to turn down a shared action if its level is unfit. >>> >>> Signed-off-by: Ivan Malov >>> Acked-by: Ori Kam >>> --- >>>   lib/librte_ethdev/rte_flow.h | 8 ++++++++ >>>   1 file changed, 8 insertions(+) >>> >>> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h >>> index a8eac4deb..8b970ba0b 100644 >>> --- a/lib/librte_ethdev/rte_flow.h >>> +++ b/lib/librte_ethdev/rte_flow.h >>> @@ -3487,6 +3487,14 @@ struct rte_flow_shared_action_conf { >>>       /**< Action valid for rules applied to ingress traffic. */ >>>       uint32_t egress:1; >>>       /**< Action valid for rules applied to egress traffic. */ >>> + >>> +    /** >>> +     * When set to 1, indicates that the action is valid for >>> +     * transfer traffic; otherwise, for non-transfer traffic. >>> +     * >>> +     * See struct rte_flow_attr. >>> +     */ >>> +    uint32_t transfer:1; >> >> Is this require any documentation update? >> >> Also cc'ed Andrey, as he is author of the shared action feature, @Andrey can >> you please review this update? > > Many-many thanks to you for reviewing the patch. And thanks for inviting > @Andrey. I should've done that from the very beginning. > > What's for documentation update, = I did take a look at > "doc/guides/prog_guide/rte_flow.rst" after Ori had suggested to do so. As far as > I can learn from the file, the convention is to describe the action structure > itself, i.e. not any auxiliary structures. > > For example, direct input to action SHARED is "struct rte_flow_shared_action". > And it's already described by an empty table ("table:: SHARED") in the doc file. > > On the other hand, documentation of "struct rte_flow_shared_action_conf" belongs > in a place where the API rte_flow_shared_action_create() is documented. This API > is only mentioned by "doc/guides/prog_guide/rte_flow.rst", and it looks like > it's not documented anywhere but in the header file itself > ("lib/librte_ethdev/rte_flow.h"). > > So, it looks like this particular patch does not need to provide an update to > documentation other that the existing comment before the field in the structure. > I missed that Ori requested same before, and I agree on your documentation analysis above, there is not clear location to put this other than doxygen comment, so OK to proceed as it is.