From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adrien Mazarguil Subject: Re: [PATCH v3 2/4] net/mlx5: add software support for rte_flow Date: Fri, 23 Dec 2016 14:24:19 +0100 Message-ID: <20161223132419.GM10340@6wind.com> References: <21927e17-26fc-6c45-8953-3215fe47ce85@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-f53.google.com (mail-wm0-f53.google.com [74.125.82.53]) by dpdk.org (Postfix) with ESMTP id BF1562C8 for ; Fri, 23 Dec 2016 14:24:28 +0100 (CET) Received: by mail-wm0-f53.google.com with SMTP id a197so199398049wmd.0 for ; Fri, 23 Dec 2016 05:24:28 -0800 (PST) Content-Disposition: inline In-Reply-To: <21927e17-26fc-6c45-8953-3215fe47ce85@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" On Fri, Dec 23, 2016 at 12:19:30PM +0000, Ferruh Yigit wrote: > On 12/21/2016 3:19 PM, Nelio Laranjeiro wrote: > > Introduce initial software validation for rte_flow rules. > > > > Signed-off-by: Nelio Laranjeiro > > --- > > drivers/net/mlx5/mlx5.h | 2 + > > drivers/net/mlx5/mlx5_flow.c | 202 ++++++++++++++++++++++++++++++++++------ > > drivers/net/mlx5/mlx5_trigger.c | 2 + > > 3 files changed, 177 insertions(+), 29 deletions(-) > > <...> > > > + for (; items->type != RTE_FLOW_ITEM_TYPE_END; ++items) { > > + if (items->type == RTE_FLOW_ITEM_TYPE_VOID) { > > + continue; > > + } else if (items->type == RTE_FLOW_ITEM_TYPE_ETH) { > > + if (ilast) > > + goto exit_item_not_supported; > > + ilast = items; > > + } else if ((items->type == RTE_FLOW_ITEM_TYPE_IPV4) || > > + (items->type == RTE_FLOW_ITEM_TYPE_IPV6)) { > > + if (!ilast) > > + goto exit_item_not_supported; > > + else if (ilast->type != RTE_FLOW_ITEM_TYPE_ETH) > > + goto exit_item_not_supported; > > + ilast = items; > > + } else if ((items->type == RTE_FLOW_ITEM_TYPE_UDP) || > > + (items->type == RTE_FLOW_ITEM_TYPE_TCP)) { > > + if (!ilast) > > + goto exit_item_not_supported; > > + else if ((ilast->type != RTE_FLOW_ITEM_TYPE_IPV4) && > > + (ilast->type != RTE_FLOW_ITEM_TYPE_IPV6)) > > + goto exit_item_not_supported; > > + ilast = items; > > + } else { > > + goto exit_item_not_supported; > > + } > > + } > > I was thinking rte_flow_validate() is validating rule against hardware / > PMD, but here the API input validation is also done. > In patch 3/4 API input validation continues with validating each item > one by one. > > Shouldn't each PMD needs to do this kind of input validation? > Why not move generic input validation to rte_flow API? > And if it is valid, call PMD specific one. I think we'll add one eventually, but such a generic function would be called by PMDs not by applications. PMDs must have the ability to optimize validate() and create() however they want. In the meantime in my opinion it's better to let PMDs implement their own to determine what can be shared later without cluttering rte_flow from the start. -- Adrien Mazarguil 6WIND