From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wj0-f174.google.com (mail-wj0-f174.google.com [209.85.210.174]) by dpdk.org (Postfix) with ESMTP id E70A9378E for ; Fri, 18 Nov 2016 11:28:10 +0100 (CET) Received: by mail-wj0-f174.google.com with SMTP id mp19so748846wjc.1 for ; Fri, 18 Nov 2016 02:28:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=eqCxSUABMdrBxRZAJEd8T5UiNRaoqgcPj84+T7WWvG0=; b=eNnWlSH9ZA3Iz76AszBQxF2Wy4BMqMWH5Pm3TjhR3Jj04T728FM4jbTF8X0Jpe6W29 RxFUhMsQpKdGp8PwD7jNQ1HaYm6hhYQSyf3mZeimhzaJsBsuOaYCpzPDckfIjv9V6qmh HjA0HkyeBBtpCX3VWvnANBbtT1LmYzxvUuUXtkym9p+zSnYssTn6vwyFLXP+k4IY7ztB P4Fuvd1b4Yg4BjxLNN9IWjSLRzK8J3Eoqjhan+mB+fbx8CGTLGl9fbeZ2dq5dqp5CMao hIdNe2KzWnsPyctcFI5hElySi9xB0QMUP5lhpIBOxWiMnMojgWRonzHf2koFuyuUjb+O 3x9Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=eqCxSUABMdrBxRZAJEd8T5UiNRaoqgcPj84+T7WWvG0=; b=Lc5Jpmw3hdTontUTve4LEjl12yN61xMerzRZE/oB8TJ2rOH98cgKMqkllgHpShB7KS Y9umTZKujoJ8NLUJNpcby7KVcuGg0upK2dAbcD0bhhF196THqG0BPnuD3Bg+HEx2SiKC D4b2zD5w3GVGjZkh8T2rXyksgeK1IGl6A/Q04/BBL2i7xe5JhttlrJ/Y19/WboFEu822 YY2xmPYS0Lk02pCj+7tEasU+M6XHGGD0nadnOyoMy7YfII6eE+j9foxfjLQh7kkwzMSs dYhVEPysJZMLJpJ/O86r4yriTyZttkc+zu/0J8D2fX9W/HN7Qk78xdI3fyj8kDvdFJ+q 2lcw== X-Gm-Message-State: AKaTC02/wp+KwXQC2jv2DbJ93xZ1eZ6JqR+ThMzY702JpLU21KT1KSOsBxzNJhJ92jaEmsD+ X-Received: by 10.194.59.71 with SMTP id x7mr6375872wjq.74.1479464890326; Fri, 18 Nov 2016 02:28:10 -0800 (PST) Received: from 6wind.com (guy78-3-82-239-227-177.fbx.proxad.net. [82.239.227.177]) by smtp.gmail.com with ESMTPSA id yg1sm8092859wjb.12.2016.11.18.02.28.08 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 18 Nov 2016 02:28:08 -0800 (PST) Date: Fri, 18 Nov 2016 11:28:01 +0100 From: Adrien Mazarguil To: "Xing, Beilei" Cc: "dev@dpdk.org" , Thomas Monjalon , "De Lara Guarch, Pablo" , Olivier Matz Message-ID: <20161118102801.GB16215@6wind.com> References: <1c8a8e4fec73ed33836f1da9525b1b8b53048518.1479309720.git.adrien.mazarguil@6wind.com> <94479800C636CB44BD422CB454846E012EA1C37D@SHSMSX101.ccr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <94479800C636CB44BD422CB454846E012EA1C37D@SHSMSX101.ccr.corp.intel.com> Subject: Re: [dpdk-dev] [PATCH 01/22] ethdev: introduce generic flow API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 18 Nov 2016 10:28:11 -0000 Hi Beilei, On Fri, Nov 18, 2016 at 06:36:31AM +0000, Xing, Beilei wrote: > Hi Adrien, > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien Mazarguil > > Sent: Thursday, November 17, 2016 12:23 AM > > To: dev@dpdk.org > > Cc: Thomas Monjalon ; De Lara Guarch, > > Pablo ; Olivier Matz > > > > Subject: [dpdk-dev] [PATCH 01/22] ethdev: introduce generic flow API > > > > This new API supersedes all the legacy filter types described in rte_eth_ctrl.h. > > It is slightly higher level and as a result relies more on PMDs to process and > > validate flow rules. > > > > Benefits: > > > > - A unified API is easier to program for, applications do not have to be > > written for a specific filter type which may or may not be supported by > > the underlying device. > > > > - The behavior of a flow rule is the same regardless of the underlying > > device, applications do not need to be aware of hardware quirks. > > > > - Extensible by design, API/ABI breakage should rarely occur if at all. > > > > - Documentation is self-standing, no need to look up elsewhere. > > > > Existing filter types will be deprecated and removed in the near future. > > > > Signed-off-by: Adrien Mazarguil > > > > + > > +/** > > + * Opaque type returned after successfully creating a flow. > > + * > > + * This handle can be used to manage and query the related flow (e.g. > > +to > > + * destroy it or retrieve counters). > > + */ > > +struct rte_flow; > > + > > As we talked before, we use attr/pattern/actions to create and destroy a flow in PMD, > but I don't think it's easy to clone the user-provided parameters and return the result > to the application as a rte_flow pointer. As you suggested: > /* PMD-specific code. */ > struct rte_flow { > struct rte_flow_attr attr; > struct rte_flow_item *pattern; > struct rte_flow_action *actions; > }; Just to provide some context to the community since the above snippet comes from private exchanges, I've suggested the above structure as a mean to create and remove rules in the same fashion as FDIR, by providing the rule used for creation to the destroy callback. As an opaque type, each PMD currently needs to implement its own version of struct rte_flow. The above definition may ease transition from FDIR to rte_flow for some PMDs, however they need to clone the entire application-provided rule to do so because there is no requirement for it to be kept allocated. I've implemented such a function in testpmd (port_flow_new() in commit [1]) as an example. [1] http://dpdk.org/ml/archives/dev/2016-November/050266.html However my suggestion is for PMDs to use their own HW-specific structure that only contains relevant information instead of being forced to drag large, non-native data around, missing useful context and that requires parsing every time. This is one benefit of using an opaque type in the first place, the other being ABI breakage avoidance. > Because both pattern and actions are pointers, and there're also pointers in structure > rte_flow_item and struct rte_flow_action. We need to iterate allocation during clone > and iterate free during destroy, then seems that the code is something ugly, right? Well since I wrote that code, I won't easily admit it's ugly. I think PMDs should not require the duplication of generic rules actually, which are only defined as a common language between applications and PMDs. Both are free to store rules in their own preferred and efficient format internally. > I think application saves info when creating a flow rule, so why not application provide > attr/pattern/actions info to PMD before calling PMD API? They have to do so temporarily (e.g. allocated on the stack) while calling rte_flow_create() and rte_flow_validate(), that's it. Once a rule is created, there's no requirement for applications to keep anything around. For simple applications such as testpmd, the generic format is probably enough. More complex and existing applications such as ovs-dpdk may rather choose to keep using their internal format that already fits their needs, partially duplicating this information in rte_flow_attr and rte_flow_item/rte_flow_action lists would waste memory. The conversion in this case should only be performed when creating/validating flow rules. In short, I fail to see any downside with maintaining struct rte_flow opaque to applications. Best regards, -- Adrien Mazarguil 6WIND