[dpdk-dev] [PATCH v2 12/18] net/ixgbe: parse ethertype filter

Zhao1, Wei wei.zhao1 at intel.com
Wed Jan 11 09:54:46 CET 2017


Hi, Yigit

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Saturday, January 7, 2017 1:11 AM
> To: Zhao1, Wei <wei.zhao1 at intel.com>; dev at dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu at intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2 12/18] net/ixgbe: parse ethertype filter
> 
> On 12/30/2016 7:53 AM, Wei Zhao wrote:
> > check if the rule is a ethertype rule, and get the ethertype info.
> > Signed-off-by: Wei Zhao <wei.zhao1 at intel.com>
> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu at intel.com>
> >
> > ---
> >
> > v2:add new error set function
> > ---
> 
> <...>
> 
> > +static int
> > +ixgbe_parse_ethertype_filter(const struct rte_flow_attr *attr,
> > +			     const struct rte_flow_item pattern[],
> > +			     const struct rte_flow_action actions[],
> > +			     struct rte_eth_ethertype_filter *filter,
> > +			     struct rte_flow_error *error) {
> > +	int ret;
> > +
> > +	ret = cons_parse_ethertype_filter(attr, pattern,
> > +					actions, filter, error);
> > +
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* Ixgbe doesn't support MAC address. */
> > +	if (filter->flags & RTE_ETHTYPE_FLAGS_MAC) {
> > +		memset(filter, 0, sizeof(struct rte_eth_ethertype_filter));
> 
> Is memset required for error cases, if so is other error cases also require it?


Yes, Ok , I will do as your suggestion.

> 
> > +		rte_flow_error_set(error, EINVAL,
> > +				RTE_FLOW_ERROR_TYPE_ITEM,
> > +				NULL, "Not supported by ethertype filter");
> > +		return -rte_errno;
> > +	}
> > +
> > +	if (filter->queue >= IXGBE_MAX_RX_QUEUE_NUM)
> > +		return -rte_errno;
> > +
> > +	if (filter->ether_type == ETHER_TYPE_IPv4 ||
> > +		filter->ether_type == ETHER_TYPE_IPv6) {
> > +		PMD_DRV_LOG(ERR, "unsupported ether_type(0x%04x) in"
> > +			" ethertype filter.", filter->ether_type);
> 
> Not sure about this log here, specially it is in ERR level.
> This function is returning error, and public API will return an error, if we want
> to notify user with a log, should app do this as library
> (here) should do this? More comment welcome?
> 
> btw, should rte_flow_error_set() used here (and below) ?

Yes, I will change to rte_flow_error_set() 

> 
> > +		return -rte_errno;
> > +	}
> > +
> > +	if (filter->flags & RTE_ETHTYPE_FLAGS_MAC) {
> 
> Isn't this duplicate with above check?
> 
> > +		PMD_DRV_LOG(ERR, "mac compare is unsupported.");
> > +		return -rte_errno;
> > +	}
> > +
> > +	if (filter->flags & RTE_ETHTYPE_FLAGS_DROP) {
> 
> Just to double check, isn't drop action by ether filters?

Yse , ixgbe do not, but i40e is.
> 
> > +		PMD_DRV_LOG(ERR, "drop option is unsupported.");
> > +		return -rte_errno;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> <...>


More information about the dev mailing list