[dpdk-dev] [PATCH v2 06/25] app/testpmd: implement basic support for rte_flow

Xing, Beilei beilei.xing at intel.com
Tue Dec 20 02:57:46 CET 2016



> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com]
> Sent: Monday, December 19, 2016 6:20 PM
> To: Xing, Beilei <beilei.xing at intel.com>
> Cc: dev at dpdk.org; Pei, Yulong <yulong.pei at intel.com>
> 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 at dpdk.org] On Behalf Of Adrien
> > > Mazarguil
> > > Sent: Saturday, December 17, 2016 12:25 AM
> > > To: dev at 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 <adrien.mazarguil at 6wind.com>
> > > +/** 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?

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 application? then PMD can judge if it's need to allocate or overlap memory.

> 
> > > +	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


More information about the dev mailing list