On Tue, Dec 27, 2016 at 02:26:22PM +0800, Beilei Xing wrote:
> This patch adds i40e_flow_flush function to flush all
> filters for users. And flow director flush function
> is involved first.
>
> Signed-off-by: Beilei Xing <beilei.x...@intel.com>
> ---
> drivers/net/i40e/i40e_ethdev.h | 3 +++
> drivers/net/i40e/i40e_fdir.c | 8 ++------
> drivers/net/i40e/i40e_flow.c | 46
> ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 51 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
> index b8c7d41..0b736d5 100644
> --- a/drivers/net/i40e/i40e_ethdev.h
> +++ b/drivers/net/i40e/i40e_ethdev.h
> @@ -786,6 +786,9 @@ i40e_sw_tunnel_filter_lookup(struct i40e_tunnel_rule
> *tunnel_rule,
> const struct i40e_tunnel_filter_input *input);
> int i40e_sw_tunnel_filter_del(struct i40e_pf *pf,
> struct i40e_tunnel_filter *tunnel_filter);
> +int i40e_sw_fdir_filter_del(struct i40e_pf *pf,
> + struct i40e_fdir_filter *filter);
> +int i40e_fdir_flush(struct rte_eth_dev *dev);
>
Why don't declare them as the global functions at the beginning?
> /* I40E_DEV_PRIVATE_TO */
> #define I40E_DEV_PRIVATE_TO_PF(adapter) \
> diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
> index 6c1bb18..f10aeee 100644
> --- a/drivers/net/i40e/i40e_fdir.c
> +++ b/drivers/net/i40e/i40e_fdir.c
> @@ -119,8 +119,6 @@ static int i40e_fdir_filter_programming(struct i40e_pf
> *pf,
> enum i40e_filter_pctype pctype,
> const struct rte_eth_fdir_filter *filter,
> bool add);
> -static int i40e_fdir_flush(struct rte_eth_dev *dev);
> -
> static int i40e_fdir_filter_convert(const struct rte_eth_fdir_filter *input,
> struct i40e_fdir_filter *filter);
> static struct i40e_fdir_filter *
> @@ -128,8 +126,6 @@ i40e_sw_fdir_filter_lookup(struct i40e_fdir_info
> *fdir_info,
> const struct rte_eth_fdir_input *input);
> static int i40e_sw_fdir_filter_insert(struct i40e_pf *pf,
> struct i40e_fdir_filter *filter);
> -static int i40e_sw_fdir_filter_del(struct i40e_pf *pf,
> - struct i40e_fdir_filter *filter);
>
> static int
> i40e_fdir_rx_queue_init(struct i40e_rx_queue *rxq)
> @@ -1070,7 +1066,7 @@ i40e_sw_fdir_filter_insert(struct i40e_pf *pf, struct
> i40e_fdir_filter *filter)
> }
>
> /* Delete a flow director filter from the SW list */
> -static int
> +int
> i40e_sw_fdir_filter_del(struct i40e_pf *pf, struct i40e_fdir_filter *filter)
> {
> struct i40e_fdir_info *fdir_info = &pf->fdir;
> @@ -1318,7 +1314,7 @@ i40e_fdir_filter_programming(struct i40e_pf *pf,
> * i40e_fdir_flush - clear all filters of Flow Director table
> * @pf: board private structure
> */
> -static int
> +int
> i40e_fdir_flush(struct rte_eth_dev *dev)
> {
> struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c
> index 4c7856c..1d9f603 100644
> --- a/drivers/net/i40e/i40e_flow.c
> +++ b/drivers/net/i40e/i40e_flow.c
> @@ -68,6 +68,8 @@ static struct rte_flow *i40e_flow_create(struct rte_eth_dev
> *dev,
> const struct rte_flow_item pattern[],
> const struct rte_flow_action actions[],
> struct rte_flow_error *error);
> +static int i40e_flow_flush(struct rte_eth_dev *dev,
> + struct rte_flow_error *error);
> static int i40e_flow_destroy(struct rte_eth_dev *dev,
> struct rte_flow *flow,
> struct rte_flow_error *error);
> @@ -95,11 +97,13 @@ static int i40e_dev_destroy_ethertype_filter(struct
> i40e_pf *pf,
> struct i40e_ethertype_filter *filter);
> static int i40e_dev_destroy_tunnel_filter(struct i40e_pf *pf,
> struct i40e_tunnel_filter *filter);
> +static int i40e_fdir_filter_flush(struct i40e_pf *pf);
>
> const struct rte_flow_ops i40e_flow_ops = {
> .validate = i40e_flow_validate,
> .create = i40e_flow_create,
> .destroy = i40e_flow_destroy,
> + .flush = i40e_flow_flush,
> };
>
> enum rte_filter_type cons_filter_type = RTE_ETH_FILTER_NONE;
> @@ -1603,3 +1607,45 @@ i40e_dev_destroy_tunnel_filter(struct i40e_pf *pf,
>
> return ret;
> }
> +
> +static int
> +i40e_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
> +{
> + struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> + int ret = 0;
> +
Meaningless initialization.
> + ret = i40e_fdir_filter_flush(pf);
> + if (!ret)
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
> + "Failed to flush FDIR flows.");
Just curious. What's the relationship between `ret' and the code (EINVAL)
passed to rte_flow_error_set()? Is `-ret' acceptable as the parameter?
Because the `-ret' which is actually returned by i40e_fdir_flush() is also
some standard UNIX errno. When error occurs, user should use which one to
figure out the failure reason? `-ret' or `rte_errno'?
> +
> + return ret;
> +}
> +
> +static int
> +i40e_fdir_filter_flush(struct i40e_pf *pf)
> +{
> + struct rte_eth_dev *dev = pf->adapter->eth_dev;
> + struct i40e_fdir_info *fdir_info = &pf->fdir;
> + struct i40e_fdir_filter *fdir_filter;
> + struct i40e_flow *flow;
> + int ret = 0;
> +
Meaningless initialization.
> + ret = i40e_fdir_flush(dev);
> + if (!ret) {
> + /* Delete FDIR filters in FDIR list. */
> + while ((fdir_filter = TAILQ_FIRST(&fdir_info->fdir_list)))
> + i40e_sw_fdir_filter_del(pf, fdir_filter);
> +
The i40e_sw_fdir_filter_del() may fail, in which case fdir_filter won't
be removed from fdir_info->fdir_list. Will it lead to an infinite loop?
Should you check the retval of i40e_sw_fdir_filter_del() and break the
loop when it fails?
Best regards,
Tiwei Bie
