Re: [dpdk-dev] [PATCH v2 2/3] net/ice: add generic flow API

["Xing, Beilei" <beilei.xing@intel.com>]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Qiming Yang
> Sent: Wednesday, June 12, 2019 3:50 PM
> To: dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>
> Subject: [dpdk-dev] [PATCH v2 2/3] net/ice: add generic flow API
> 
> This patch adds ice_flow_create, ice_flow_destroy, ice_flow_flush and
> ice_flow_validate support, these are going to used to handle all the generic
> filters.
> 

'going to' can be removed.

> Signed-off-by: Qiming Yang <qiming.yang@intel.com>
> ---
>  drivers/net/ice/Makefile           |   1 +
>  drivers/net/ice/ice_ethdev.c       |  44 +++
>  drivers/net/ice/ice_ethdev.h       |   7 +-
>  drivers/net/ice/ice_generic_flow.c | 567
> +++++++++++++++++++++++++++++++++++++
>  drivers/net/ice/ice_generic_flow.h | 404 ++++++++++++++++++++++++++
>  5 files changed, 1022 insertions(+), 1 deletion(-)  create mode 100644
> drivers/net/ice/ice_generic_flow.c
>  create mode 100644 drivers/net/ice/ice_generic_flow.h
> 
> diff --git a/drivers/net/ice/Makefile b/drivers/net/ice/Makefile index
> b10d826..32abeb6 100644
> --- a/drivers/net/ice/Makefile
> +++ b/drivers/net/ice/Makefile
> @@ -79,5 +79,6 @@ endif
>  ifeq ($(CC_AVX2_SUPPORT), 1)
>  	SRCS-$(CONFIG_RTE_LIBRTE_ICE_PMD) += ice_rxtx_vec_avx2.c  endif
> +SRCS-$(CONFIG_RTE_LIBRTE_ICE_PMD) += ice_generic_flow.c
> 

It should be added in meson.build, too.

>  include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c index
> bdbceb4..cf6bb1d 100644
> --- a/drivers/net/ice/ice_ethdev.c
> +++ b/drivers/net/ice/ice_ethdev.c
> @@ -15,6 +15,7 @@

...

> 
> +extern const struct rte_flow_ops ice_flow_ops;
> +
>  /* Struct to store flow created. */
>  struct rte_flow {
>  	TAILQ_ENTRY(rte_flow) node;
> -void *rule;
> +	void *rule;
>  };

Why not change it in patch 1?

> 
> +TAILQ_HEAD(ice_flow_list, rte_flow);
> +
>  struct ice_pf {
>  	struct ice_adapter *adapter; /* The adapter this PF associate to */
>  	struct ice_vsi *main_vsi; /* pointer to main VSI structure */ @@ -
> 265,6 +269,7 @@ struct ice_pf {
>  	struct ice_eth_stats internal_stats;
>  	bool offset_loaded;
>  	bool adapter_stopped;
> +	struct ice_flow_list flow_list;
>  };
> 
>  /**
> diff --git a/drivers/net/ice/ice_generic_flow.c
> b/drivers/net/ice/ice_generic_flow.c
> new file mode 100644
> index 0000000..4fb50b2
> --- /dev/null
> +++ b/drivers/net/ice/ice_generic_flow.c

...

> +
> +static int ice_flow_valid_action(const struct rte_flow_action *actions,
> +				       struct rte_flow_error *error) {
> +	switch (actions->type) {
> +	case RTE_FLOW_ACTION_TYPE_QUEUE:
> +		break;
> +	case RTE_FLOW_ACTION_TYPE_DROP:
> +		break;
> +	default:
> +		rte_flow_error_set(error, EINVAL,
> +				   RTE_FLOW_ERROR_TYPE_ACTION, actions,
> +				   "Invalid action.");
> +		return -rte_errno;
> +	}
> +
> +	return 0;
> +}

Do we need a supported array for action?

> +
> +static int
> +ice_flow_validate(__rte_unused struct rte_eth_dev *dev,

...