Re: [dpdk-dev] [PATCH v2 07/17] net/i40e: add flow validate function

[Tiwei Bie <tiwei.bie@intel.com>]

On Tue, Dec 27, 2016 at 02:26:14PM +0800, Beilei Xing wrote:
> This patch adds i40e_flow_validation function to check if
> a flow is valid according to the flow pattern.
> i40e_parse_ethertype_filter is added first, it also gets
> the ethertype info.
> i40e_flow.c is added to handle all generic filter events.
> 
> Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> ---
>  drivers/net/i40e/Makefile      |   1 +
>  drivers/net/i40e/i40e_ethdev.c |   5 +
>  drivers/net/i40e/i40e_ethdev.h |  20 ++
>  drivers/net/i40e/i40e_flow.c   | 431 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 457 insertions(+)
>  create mode 100644 drivers/net/i40e/i40e_flow.c
> 
> diff --git a/drivers/net/i40e/Makefile b/drivers/net/i40e/Makefile
> index 11175c4..89bd85a 100644
> --- a/drivers/net/i40e/Makefile
> +++ b/drivers/net/i40e/Makefile
> @@ -105,6 +105,7 @@ endif
>  SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_ethdev_vf.c
>  SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_pf.c
>  SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_fdir.c
> +SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_flow.c
>  
>  # vector PMD driver needs SSE4.1 support
>  ifeq ($(findstring RTE_MACHINE_CPUFLAG_SSE4_1,$(CFLAGS)),)
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 7f98b79..80024ed 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -8452,6 +8452,11 @@ i40e_dev_filter_ctrl(struct rte_eth_dev *dev,
>  	case RTE_ETH_FILTER_FDIR:
>  		ret = i40e_fdir_ctrl_func(dev, filter_op, arg);
>  		break;
> +	case RTE_ETH_FILTER_GENERIC:
> +		if (filter_op != RTE_ETH_FILTER_GET)
> +			return -EINVAL;
> +		*(const void **)arg = &i40e_flow_ops;
> +		break;
>  	default:
>  		PMD_DRV_LOG(WARNING, "Filter type (%d) not supported",
>  							filter_type);
> diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
> index 6089895..bbe52f0 100644
> --- a/drivers/net/i40e/i40e_ethdev.h
> +++ b/drivers/net/i40e/i40e_ethdev.h
> @@ -38,6 +38,7 @@
>  #include <rte_time.h>
>  #include <rte_kvargs.h>
>  #include <rte_hash.h>
> +#include <rte_flow_driver.h>
>  
>  #define I40E_VLAN_TAG_SIZE        4
>  
> @@ -629,6 +630,23 @@ struct i40e_adapter {
>  	struct rte_timecounter tx_tstamp_tc;
>  };
>  
> +union i40e_filter_t {
> +	struct rte_eth_ethertype_filter ethertype_filter;
> +	struct rte_eth_fdir_filter fdir_filter;
> +	struct rte_eth_tunnel_filter_conf tunnel_filter;
> +} cons_filter;
> +

Are you sure that you want to define a variable in i40e_ethdev.h?

> +typedef int (*parse_filter_t)(struct rte_eth_dev *dev,
> +			      const struct rte_flow_attr *attr,
> +			      const struct rte_flow_item pattern[],
> +			      const struct rte_flow_action actions[],
> +			      struct rte_flow_error *error,
> +			      union i40e_filter_t *filter);
> +struct i40e_valid_pattern {
> +	enum rte_flow_item_type *items;
> +	parse_filter_t parse_filter;
> +};
> +
>  int i40e_dev_switch_queues(struct i40e_pf *pf, bool on);
>  int i40e_vsi_release(struct i40e_vsi *vsi);
>  struct i40e_vsi *i40e_vsi_setup(struct i40e_pf *pf,
> @@ -823,4 +841,6 @@ i40e_calc_itr_interval(int16_t interval)
>  	((phy_type) & I40E_CAP_PHY_TYPE_25GBASE_SR) || \
>  	((phy_type) & I40E_CAP_PHY_TYPE_25GBASE_LR))
>  
> +const struct rte_flow_ops i40e_flow_ops;
> +

Same here. Are you sure that you want to define a variable in i40e_ethdev.h?
Maybe you should add the `extern' qualifier.

Best regards,
Tiwei Bie