From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adrien Mazarguil Subject: Re: [PATCH v5 2/6] net/mlx5: support basic flow items and actions Date: Mon, 9 Jan 2017 16:29:58 +0100 Message-ID: <20170109152958.GB12822@6wind.com> References: <025e66266f9baba7515ad4ba54a4a6addc9e2345.1483022600.git.nelio.laranjeiro@6wind.com> <2973bbd2-8db3-f9e4-da9a-359e399527b3@intel.com> <20170104184222.GI12822@6wind.com> <37389e42-030a-a36f-57bc-a7ef15a0ee66@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Nelio Laranjeiro , dev@dpdk.org To: Ferruh Yigit Return-path: Received: from mail-wm0-f49.google.com (mail-wm0-f49.google.com [74.125.82.49]) by dpdk.org (Postfix) with ESMTP id 58879133F for ; Mon, 9 Jan 2017 16:30:07 +0100 (CET) Received: by mail-wm0-f49.google.com with SMTP id a197so101630062wmd.0 for ; Mon, 09 Jan 2017 07:30:07 -0800 (PST) Content-Disposition: inline In-Reply-To: <37389e42-030a-a36f-57bc-a7ef15a0ee66@intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Ferruh, On Fri, Jan 06, 2017 at 01:52:53PM +0000, Ferruh Yigit wrote: > On 1/4/2017 6:42 PM, Adrien Mazarguil wrote: > > Hi Ferruh, > > > > On Wed, Jan 04, 2017 at 05:49:46PM +0000, Ferruh Yigit wrote: > >> Hi Nelio, > >> > >> A quick question. > > > > I'll reply since it's related to the API. > > > >> On 12/29/2016 3:15 PM, Nelio Laranjeiro wrote: > >>> Introduce initial software for rte_flow rules. > >>> > >>> VLAN, VXLAN are still not supported. > >>> > >>> Signed-off-by: Nelio Laranjeiro > >>> Acked-by: Adrien Mazarguil > >> > >> <...> > >> > >>> +static int > >>> +priv_flow_validate(struct priv *priv, > >>> + const struct rte_flow_attr *attr, > >>> + const struct rte_flow_item items[], > >>> + const struct rte_flow_action actions[], > >>> + struct rte_flow_error *error, > >>> + struct mlx5_flow *flow) > >>> +{ > >>> + const struct mlx5_flow_items *cur_item = mlx5_flow_items; > >> > >> <...> > >> > >>> + for (; items->type != RTE_FLOW_ITEM_TYPE_END; ++items) { > >> <...> > >>> + } > >>> + for (; actions->type != RTE_FLOW_ACTION_TYPE_END; ++actions) { > >> <...> > >>> + } > >> > >> Is it guarantied in somewhere that items or actions terminated with > >> TYPE_END? > > > > Yes, since it's now the only way to terminate items/actions lists [1][2]. > > There used to be a "max" value in the original draft but it seemed redundant > > and proved annoying to use, and was therefore dropped. > > > > END items/actions behave like a NUL terminator for C strings. They are > > likewise defined with value 0 for convenience. > > At least it is good idea to set END values to 0, but still if user not > set it, most probably this will crash the app. > > Although most probably this kind of error will be detected easily in > development phase, still it would be nice to return an error instead of > crashing when user provide wrong input. Unfortunately I cannot think of an easy way to do that, even for debugging purposes, this would be like checking for unterminated strings or linked lists without a NULL ending pointer. That's the trade-off of any unbounded data structure. Note PMDs will likely return errors as they iterate on garbage item/action types, crashes will also almost always occur when attempting to dereference the related spec/last/mask/conf pointers. > >> And these fields are direct inputs from user. > >> Is there a way to verify user provided values are with TYPE_END terminated? > > > > No, applications must check for its presence (they normally add it > > themselves) before feeding these lists to PMDs. I think that's safe enough. > > > > Note the testpmd flow command does not allow entering a flow rule without > > "end" tokens in both lists, there is no way around this restriction. > > > > [1] http://dpdk.org/doc/guides/prog_guide/rte_flow.html#matching-pattern > > [2] http://dpdk.org/doc/guides/prog_guide/rte_flow.html#actions -- Adrien Mazarguil 6WIND