From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <adrien.mazarguil@6wind.com>
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 <dev@dpdk.org>; Fri, 18 Nov 2016 11:28:10 +0100 (CET)
Received: by mail-wj0-f174.google.com with SMTP id mp19so748846wjc.1
for <dev@dpdk.org>; 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 <adrien.mazarguil@6wind.com>
To: "Xing, Beilei" <beilei.xing@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, Thomas Monjalon <thomas.monjalon@6wind.com>,
"De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>,
Olivier Matz <olivier.matz@6wind.com>
Message-ID: <20161118102801.GB16215@6wind.com>
References: <cover.1471632644.git.adrien.mazarguil@6wind.com>
<cover.1479309719.git.adrien.mazarguil@6wind.com>
<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 <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=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 <thomas.monjalon@6wind.com>; De Lara Guarch,
> > Pablo <pablo.de.lara.guarch@intel.com>; Olivier Matz
> > <olivier.matz@6wind.com>
> > 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 <adrien.mazarguil@6wind.com>
>
>
> > +
> > +/**
> > + * 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