[dpdk-dev,v2,15/17] net/i40e: add flow flush function
Checks
Commit Message
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.xing@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(-)
Comments
Hi Beilei,
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.xing@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_flow.c b/drivers/net/i40e/i40e_flow.c
[...]
> +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;
> +
> + 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);
> +
> + /* Delete FDIR flows in flow list. */
> + TAILQ_FOREACH(flow, &pf->flow_list, node) {
> + if (flow->filter_type == RTE_ETH_FILTER_FDIR) {
> + TAILQ_REMOVE(&pf->flow_list, flow, node);
> + rte_free(flow);
> + }
> + }
Be careful, I'm not sure calling TAILQ_REMOVE() followed by rte_free()
inside a TAILQ_FOREACH() is safe. BSD has the _SAFE() variant for this
purpose but Linux does not.
> + }
> +
> + return ret;
> +}
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.xing@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
> -----Original Message-----
> From: Bie, Tiwei
> Sent: Wednesday, December 28, 2016 1:36 PM
> To: Xing, Beilei <beilei.xing@intel.com>
> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Helin
> <helin.zhang@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 15/17] net/i40e: add flow flush function
>
> 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.xing@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?
When I implement the store/restore function, I plan this function is only used in i40e_ethdev.c.
I change them to the global functions since I add i40e_flow.c to rework all the flow ops.
>
> > /* 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.
Yes, you're right, will change in next version. Thanks.
>
> > + 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'?
rte_errno. I need to rework all rte_flow_error_set() according to Adrien's comments.
>
> > +
> > + 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?
Yes, thanks for catching this.
>
> Best regards,
> Tiwei Bie
On Wed, Dec 28, 2016 at 02:48:02PM +0800, Xing, Beilei wrote:
> > -----Original Message-----
> > From: Bie, Tiwei
> > Sent: Wednesday, December 28, 2016 1:36 PM
> > To: Xing, Beilei <beilei.xing@intel.com>
> > Cc: Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Helin
> > <helin.zhang@intel.com>; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v2 15/17] net/i40e: add flow flush function
> >
> > 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.xing@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?
>
> When I implement the store/restore function, I plan this function is only used in i40e_ethdev.c.
> I change them to the global functions since I add i40e_flow.c to rework all the flow ops.
>
These functions are also introduced in this patch set. There is no
particular reason to mark them as static at first and then turn them
into the global functions in the later patches. So it would be better
to declare them as the global ones when introducing them.
Best regards,
Tiwei Bie
> -----Original Message-----
> From: Bie, Tiwei
> Sent: Wednesday, December 28, 2016 3:01 PM
> To: Xing, Beilei <beilei.xing@intel.com>
> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Helin
> <helin.zhang@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 15/17] net/i40e: add flow flush function
>
> On Wed, Dec 28, 2016 at 02:48:02PM +0800, Xing, Beilei wrote:
> > > -----Original Message-----
> > > From: Bie, Tiwei
> > > Sent: Wednesday, December 28, 2016 1:36 PM
> > > To: Xing, Beilei <beilei.xing@intel.com>
> > > Cc: Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Helin
> > > <helin.zhang@intel.com>; dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH v2 15/17] net/i40e: add flow flush
> > > function
> > >
> > > 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.xing@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?
> >
> > When I implement the store/restore function, I plan this function is only
> used in i40e_ethdev.c.
> > I change them to the global functions since I add i40e_flow.c to rework all
> the flow ops.
> >
>
> These functions are also introduced in this patch set. There is no particular
> reason to mark them as static at first and then turn them into the global
> functions in the later patches. So it would be better to declare them as the
> global ones when introducing them.
Yes, make sense. Will update in next version.
>
> Best regards,
> Tiwei Bie
> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> Sent: Tuesday, December 27, 2016 8:40 PM
> To: Xing, Beilei <beilei.xing@intel.com>
> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Helin
> <helin.zhang@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 15/17] net/i40e: add flow flush function
>
> Hi Beilei,
>
> 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.xing@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_flow.c
> > b/drivers/net/i40e/i40e_flow.c
> [...]
> > +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;
> > +
> > + 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);
> > +
> > + /* Delete FDIR flows in flow list. */
> > + TAILQ_FOREACH(flow, &pf->flow_list, node) {
> > + if (flow->filter_type == RTE_ETH_FILTER_FDIR) {
> > + TAILQ_REMOVE(&pf->flow_list, flow, node);
> > + rte_free(flow);
> > + }
> > + }
>
> Be careful, I'm not sure calling TAILQ_REMOVE() followed by rte_free()
> inside a TAILQ_FOREACH() is safe. BSD has the _SAFE() variant for this
> purpose but Linux does not.
>
Yes, thanks for reminder, I'll check it later:)
> > + }
> > +
> > + return ret;
> > +}
>
> --
> Adrien Mazarguil
> 6WIND
@@ -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);
/* I40E_DEV_PRIVATE_TO */
#define I40E_DEV_PRIVATE_TO_PF(adapter) \
@@ -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);
@@ -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;
+
+ 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.");
+
+ 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;
+
+ 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);
+
+ /* Delete FDIR flows in flow list. */
+ TAILQ_FOREACH(flow, &pf->flow_list, node) {
+ if (flow->filter_type == RTE_ETH_FILTER_FDIR) {
+ TAILQ_REMOVE(&pf->flow_list, flow, node);
+ rte_free(flow);
+ }
+ }
+ }
+
+ return ret;
+}