From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Xing, Beilei" Subject: Re: [PATCH v2 06/25] app/testpmd: implement basic support for rte_flow Date: Wed, 21 Dec 2016 05:23:11 +0000 Message-ID: <94479800C636CB44BD422CB454846E013158055A@SHSMSX101.ccr.corp.intel.com> References: <3318a43c9e105caaf4d5b13d6e4fcce774fb522f.1481903839.git.adrien.mazarguil@6wind.com> <94479800C636CB44BD422CB454846E013157433C@SHSMSX101.ccr.corp.intel.com> <20161219101943.GJ10340@6wind.com> <94479800C636CB44BD422CB454846E0131574785@SHSMSX101.ccr.corp.intel.com> <20161220093815.GO10340@6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" , "Pei, Yulong" To: Adrien Mazarguil Return-path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 5B1F198 for ; Wed, 21 Dec 2016 06:23:14 +0100 (CET) In-Reply-To: <20161220093815.GO10340@6wind.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > -----Original Message----- > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com] > Sent: Tuesday, December 20, 2016 5:38 PM > To: Xing, Beilei > Cc: dev@dpdk.org; Pei, Yulong > Subject: Re: [dpdk-dev] [PATCH v2 06/25] app/testpmd: implement basic > support for rte_flow >=20 > On Tue, Dec 20, 2016 at 01:57:46AM +0000, Xing, Beilei wrote: > > > -----Original Message----- > > > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com] > > > Sent: Monday, December 19, 2016 6:20 PM > > > To: Xing, Beilei > > > Cc: dev@dpdk.org; Pei, Yulong > > > Subject: Re: [dpdk-dev] [PATCH v2 06/25] app/testpmd: implement > > > basic support for rte_flow > > > > > > Hi Beilei, > > > > > > On Mon, Dec 19, 2016 at 08:37:20AM +0000, Xing, Beilei wrote: > > > > Hi Adrien, > > > > > > > > > -----Original Message----- > > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien > > > > > Mazarguil > > > > > Sent: Saturday, December 17, 2016 12:25 AM > > > > > To: dev@dpdk.org > > > > > Subject: [dpdk-dev] [PATCH v2 06/25] app/testpmd: implement > > > > > basic support for rte_flow > > > > > > > > > > Add basic management functions for the generic flow API > > > > > (validate, create, destroy, flush, query and list). Flow rule > > > > > objects and properties are arranged in lists associated with each= port. > > > > > > > > > > Signed-off-by: Adrien Mazarguil > > > > > +/** Create flow rule. */ > > > > > +int > > > > > +port_flow_create(portid_t port_id, > > > > > + const struct rte_flow_attr *attr, > > > > > + const struct rte_flow_item *pattern, > > > > > + const struct rte_flow_action *actions) { > > > > > + struct rte_flow *flow; > > > > > + struct rte_port *port; > > > > > + struct port_flow *pf; > > > > > + uint32_t id; > > > > > + struct rte_flow_error error; > > > > > + > > > > > > > > I think there should be memset for error here, e.g. memset(&error, > > > > 0, sizeof(struct rte_flow_error)); Since both cause and message > > > > may be NULL > > > regardless of the error type, if there's no error.cause and > > > error.message returned from PMD, Segmentation fault will happen in > port_flow_complain. > > > > PS: This issue doesn't happen if add "export EXTRA_CFLAGS=3D' -g > > > > O0'" when > > > compiling. > > > > > > Actually, PMDs must fill the error structure only in case of error > > > if the application provides one, it's not optional. I didn't > > > initialize this structure for this reason. > > > > > > I suggest we initialize it with a known poisoning value for > > > debugging purposes though, to make it fail every time. Does it sound > reasonable? >=20 > Done for v3 by the way. >=20 > > OK, I see. Do you want PMD to allocate the memory for cause and message > of error, and must fill the cause and message if error exists, right? > > So is it possible to set NULL for pointers of cause and message in appl= ication? > then PMD can judge if it's need to allocate or overlap memory. >=20 > PMDs never allocate this structure, applications do and initialize it how= ever > they want. They only provide a non-NULL pointer if they want additional > details in case of error. >=20 > It will likely be allocated on the stack in most cases (as in testpmd). >=20 > From a PMD standpoint, the contents of this structure must be updated in > case of non-NULL pointer and error state. >=20 > Now the message pointer can be allocated dynamically but it's not > recommended, it's far easier to make it point to some constant string. > Applications won't free it anyway, so PMDs would have to do it during > dev_close(). Please see "Verbose error reporting" documentation section. Got it, thanks. Seems the rte_flow_error_set function can be used for PMD. Regards, Beilei