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

[Ferruh Yigit]

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?

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

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

> +		PMD_DRV_LOG(ERR, "drop option is unsupported.");
> +		return -rte_errno;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
<...>