From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adrien Mazarguil Subject: Re: [PATCH v2 06/25] app/testpmd: implement basic support for rte_flow Date: Mon, 19 Dec 2016 11:19:43 +0100 Message-ID: <20161219101943.GJ10340@6wind.com> References: <3318a43c9e105caaf4d5b13d6e4fcce774fb522f.1481903839.git.adrien.mazarguil@6wind.com> <94479800C636CB44BD422CB454846E013157433C@SHSMSX101.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: "dev@dpdk.org" , "Pei, Yulong" To: "Xing, Beilei" Return-path: Received: from mail-wm0-f44.google.com (mail-wm0-f44.google.com [74.125.82.44]) by dpdk.org (Postfix) with ESMTP id 48D4DF614 for ; Mon, 19 Dec 2016 11:19:52 +0100 (CET) Received: by mail-wm0-f44.google.com with SMTP id f82so96573580wmf.1 for ; Mon, 19 Dec 2016 02:19:52 -0800 (PST) Content-Disposition: inline In-Reply-To: <94479800C636CB44BD422CB454846E013157433C@SHSMSX101.ccr.corp.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 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=' -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? > > + flow = rte_flow_create(port_id, attr, pattern, actions, &error); > > + if (!flow) > > + return port_flow_complain(&error); > > + port = &ports[port_id]; > > + if (port->flow_list) { > > + if (port->flow_list->id == UINT32_MAX) { > > + printf("Highest rule ID is already assigned, delete" > > + " it first"); > > + rte_flow_destroy(port_id, flow, NULL); > > + return -ENOMEM; > > + } > > + id = port->flow_list->id + 1; > > + } else > > + id = 0; > > + pf = port_flow_new(attr, pattern, actions); > > + if (!pf) { > > + int err = rte_errno; > > + > > + printf("Cannot allocate flow: %s\n", rte_strerror(err)); > > + rte_flow_destroy(port_id, flow, NULL); > > + return -err; > > + } > > + pf->next = port->flow_list; > > + pf->id = id; > > + pf->flow = flow; > > + port->flow_list = pf; > > + printf("Flow rule #%u created\n", pf->id); > > + return 0; > > +} > > + -- Adrien Mazarguil 6WIND