DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/i40e: fix flow FDIR enable issue
@ 2020-05-19  7:16 Wei Zhao
  2020-05-19  9:36 ` Cui, LunyuanX
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Wei Zhao @ 2020-05-19  7:16 UTC (permalink / raw)
  To: dev; +Cc: stable, beilei.xing, jia.guo, Wei Zhao

When we flush flow FDIR, all queues are disabled for FDIR.
If FDIR rule is created again, then the flow list is empty,
as it is the first time to create rule after flush fdir filter,
so we need to enable FDRI for all queues. And also, disable FDIR
for queues should be done in function i40e_flow_flush_fdir_filter().

Cc: stable@dpdk.org
Fixes: 1491f63c7559 ("net/i40e: fix flush of flow director filter")
Fixes: 6ae9b2b5e8c2 ("net/i40e: cache flow director enable value in Rx queue")

Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
---
 drivers/net/i40e/i40e_flow.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c
index 25c77e7aa..94fc73092 100644
--- a/drivers/net/i40e/i40e_flow.c
+++ b/drivers/net/i40e/i40e_flow.c
@@ -3462,6 +3462,12 @@ i40e_flow_parse_fdir_filter(struct rte_eth_dev *dev,
 		}
 	}
 
+	/* If the flow list is empty, it is the first time to create
+	 * rule after flush fdir filter, so enable fdir.
+	 */
+	if (TAILQ_EMPTY(&pf->fdir.fdir_list))
+		i40e_fdir_rx_proc_enable(dev, 1);
+
 	return 0;
 err:
 	i40e_fdir_teardown(pf);
@@ -5330,9 +5336,6 @@ i40e_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
 		return -rte_errno;
 	}
 
-	/* Disable FDIR processing as all FDIR rules are now flushed */
-	i40e_fdir_rx_proc_enable(dev, 0);
-
 	return ret;
 }
 
@@ -5368,6 +5371,9 @@ i40e_flow_flush_fdir_filter(struct i40e_pf *pf)
 		for (pctype = I40E_FILTER_PCTYPE_NONF_IPV4_UDP;
 		     pctype <= I40E_FILTER_PCTYPE_L2_PAYLOAD; pctype++)
 			pf->fdir.inset_flag[pctype] = 0;
+
+		/* Disable FDIR processing as all FDIR rules are now flushed */
+		i40e_fdir_rx_proc_enable(dev, 0);
 	}
 
 	return ret;
-- 
2.19.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] net/i40e: fix flow FDIR enable issue
  2020-05-19  7:16 [dpdk-dev] [PATCH] net/i40e: fix flow FDIR enable issue Wei Zhao
@ 2020-05-19  9:36 ` Cui, LunyuanX
  2020-05-20  3:18 ` Jeff Guo
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 13+ messages in thread
From: Cui, LunyuanX @ 2020-05-19  9:36 UTC (permalink / raw)
  To: Zhao1, Wei, dev; +Cc: stable, Xing, Beilei, Guo, Jia, Zhao1, Wei

Tested-by: Cui, Lunyuan <lunyuanx.cui@intel.com>

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao
> Sent: Tuesday, May 19, 2020 3:17 PM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; Guo, Jia
> <jia.guo@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
> Subject: [dpdk-dev] [PATCH] net/i40e: fix flow FDIR enable issue
> 
> When we flush flow FDIR, all queues are disabled for FDIR.
> If FDIR rule is created again, then the flow list is empty, as it is the first time to
> create rule after flush fdir filter, so we need to enable FDRI for all queues.
> And also, disable FDIR for queues should be done in function
> i40e_flow_flush_fdir_filter().
> 
> Cc: stable@dpdk.org
> Fixes: 1491f63c7559 ("net/i40e: fix flush of flow director filter")
> Fixes: 6ae9b2b5e8c2 ("net/i40e: cache flow director enable value in Rx
> queue")
> 
> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> ---
>  drivers/net/i40e/i40e_flow.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c
> index 25c77e7aa..94fc73092 100644
> --- a/drivers/net/i40e/i40e_flow.c
> +++ b/drivers/net/i40e/i40e_flow.c
> @@ -3462,6 +3462,12 @@ i40e_flow_parse_fdir_filter(struct rte_eth_dev
> *dev,
>  		}
>  	}
> 
> +	/* If the flow list is empty, it is the first time to create
> +	 * rule after flush fdir filter, so enable fdir.
> +	 */
> +	if (TAILQ_EMPTY(&pf->fdir.fdir_list))
> +		i40e_fdir_rx_proc_enable(dev, 1);
> +
>  	return 0;
>  err:
>  	i40e_fdir_teardown(pf);
> @@ -5330,9 +5336,6 @@ i40e_flow_flush(struct rte_eth_dev *dev, struct
> rte_flow_error *error)
>  		return -rte_errno;
>  	}
> 
> -	/* Disable FDIR processing as all FDIR rules are now flushed */
> -	i40e_fdir_rx_proc_enable(dev, 0);
> -
>  	return ret;
>  }
> 
> @@ -5368,6 +5371,9 @@ i40e_flow_flush_fdir_filter(struct i40e_pf *pf)
>  		for (pctype = I40E_FILTER_PCTYPE_NONF_IPV4_UDP;
>  		     pctype <= I40E_FILTER_PCTYPE_L2_PAYLOAD; pctype++)
>  			pf->fdir.inset_flag[pctype] = 0;
> +
> +		/* Disable FDIR processing as all FDIR rules are now flushed
> */
> +		i40e_fdir_rx_proc_enable(dev, 0);
>  	}
> 
>  	return ret;
> --
> 2.19.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] net/i40e: fix flow FDIR enable issue
  2020-05-19  7:16 [dpdk-dev] [PATCH] net/i40e: fix flow FDIR enable issue Wei Zhao
  2020-05-19  9:36 ` Cui, LunyuanX
@ 2020-05-20  3:18 ` Jeff Guo
  2020-05-20  3:22   ` Zhao1, Wei
  2020-05-20  4:13 ` Xing, Beilei
  2020-05-21  8:03 ` [dpdk-dev] [PATCH v2] " Wei Zhao
  3 siblings, 1 reply; 13+ messages in thread
From: Jeff Guo @ 2020-05-20  3:18 UTC (permalink / raw)
  To: Wei Zhao, dev; +Cc: stable, beilei.xing

hi, zhaowei

On 5/19/2020 3:16 PM, Wei Zhao wrote:
> When we flush flow FDIR, all queues are disabled for FDIR.
> If FDIR rule is created again, then the flow list is empty,
> as it is the first time to create rule after flush fdir filter,
> so we need to enable FDRI for all queues. And also, disable FDIR


typo: FDRI -> FDIR, and could you modify something commit if you agree 
on below inline comment. Thanks.


> for queues should be done in function i40e_flow_flush_fdir_filter().
>
> Cc: stable@dpdk.org
> Fixes: 1491f63c7559 ("net/i40e: fix flush of flow director filter")
> Fixes: 6ae9b2b5e8c2 ("net/i40e: cache flow director enable value in Rx queue")
>
> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> ---
>   drivers/net/i40e/i40e_flow.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c
> index 25c77e7aa..94fc73092 100644
> --- a/drivers/net/i40e/i40e_flow.c
> +++ b/drivers/net/i40e/i40e_flow.c
> @@ -3462,6 +3462,12 @@ i40e_flow_parse_fdir_filter(struct rte_eth_dev *dev,
>   		}
>   	}
>   
> +	/* If the flow list is empty, it is the first time to create
> +	 * rule after flush fdir filter, so enable fdir.
> +	 */


So either flush fdir filter or destroy the last fdir, it will result the 
flow list to be empty, could it simply as "If create the first fdir 
rule, enable fdir" or?


> +	if (TAILQ_EMPTY(&pf->fdir.fdir_list))
> +		i40e_fdir_rx_proc_enable(dev, 1);
> +
>   	return 0;
>   err:
>   	i40e_fdir_teardown(pf);
> @@ -5330,9 +5336,6 @@ i40e_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
>   		return -rte_errno;
>   	}
>   
> -	/* Disable FDIR processing as all FDIR rules are now flushed */
> -	i40e_fdir_rx_proc_enable(dev, 0);
> -
>   	return ret;
>   }
>   
> @@ -5368,6 +5371,9 @@ i40e_flow_flush_fdir_filter(struct i40e_pf *pf)
>   		for (pctype = I40E_FILTER_PCTYPE_NONF_IPV4_UDP;
>   		     pctype <= I40E_FILTER_PCTYPE_L2_PAYLOAD; pctype++)
>   			pf->fdir.inset_flag[pctype] = 0;
> +
> +		/* Disable FDIR processing as all FDIR rules are now flushed */
> +		i40e_fdir_rx_proc_enable(dev, 0);
>   	}
>   
>   	return ret;

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] net/i40e: fix flow FDIR enable issue
  2020-05-20  3:18 ` Jeff Guo
@ 2020-05-20  3:22   ` Zhao1, Wei
  0 siblings, 0 replies; 13+ messages in thread
From: Zhao1, Wei @ 2020-05-20  3:22 UTC (permalink / raw)
  To: Guo, Jia, dev; +Cc: stable, Xing, Beilei


Hi, guojia

> -----Original Message-----
> From: Guo, Jia <jia.guo@intel.com>
> Sent: Wednesday, May 20, 2020 11:19 AM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; Xing, Beilei <beilei.xing@intel.com>
> Subject: Re: [PATCH] net/i40e: fix flow FDIR enable issue
> 
> hi, zhaowei
> 
> On 5/19/2020 3:16 PM, Wei Zhao wrote:
> > When we flush flow FDIR, all queues are disabled for FDIR.
> > If FDIR rule is created again, then the flow list is empty, as it is
> > the first time to create rule after flush fdir filter, so we need to
> > enable FDRI for all queues. And also, disable FDIR
> 
> 
> typo: FDRI -> FDIR, and could you modify something commit if you agree on
> below inline comment. Thanks.

Ok,

> 
> 
> > for queues should be done in function i40e_flow_flush_fdir_filter().
> >
> > Cc: stable@dpdk.org
> > Fixes: 1491f63c7559 ("net/i40e: fix flush of flow director filter")
> > Fixes: 6ae9b2b5e8c2 ("net/i40e: cache flow director enable value in Rx
> queue")
> >
> > Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> > ---
> >   drivers/net/i40e/i40e_flow.c | 12 +++++++++---
> >   1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c
> > index 25c77e7aa..94fc73092 100644
> > --- a/drivers/net/i40e/i40e_flow.c
> > +++ b/drivers/net/i40e/i40e_flow.c
> > @@ -3462,6 +3462,12 @@ i40e_flow_parse_fdir_filter(struct rte_eth_dev
> *dev,
> >   		}
> >   	}
> >
> > +	/* If the flow list is empty, it is the first time to create
> > +	 * rule after flush fdir filter, so enable fdir.
> > +	 */
> 
> 
> So either flush fdir filter or destroy the last fdir, it will result the
> flow list to be empty, could it simply as "If create the first fdir
> rule, enable fdir" or?


Ok


> 
> 
> > +	if (TAILQ_EMPTY(&pf->fdir.fdir_list))
> > +		i40e_fdir_rx_proc_enable(dev, 1);
> > +
> >   	return 0;
> >   err:
> >   	i40e_fdir_teardown(pf);
> > @@ -5330,9 +5336,6 @@ i40e_flow_flush(struct rte_eth_dev *dev, struct
> rte_flow_error *error)
> >   		return -rte_errno;
> >   	}
> >
> > -	/* Disable FDIR processing as all FDIR rules are now flushed */
> > -	i40e_fdir_rx_proc_enable(dev, 0);
> > -
> >   	return ret;
> >   }
> >
> > @@ -5368,6 +5371,9 @@ i40e_flow_flush_fdir_filter(struct i40e_pf *pf)
> >   		for (pctype = I40E_FILTER_PCTYPE_NONF_IPV4_UDP;
> >   		     pctype <= I40E_FILTER_PCTYPE_L2_PAYLOAD; pctype++)
> >   			pf->fdir.inset_flag[pctype] = 0;
> > +
> > +		/* Disable FDIR processing as all FDIR rules are now flushed */
> > +		i40e_fdir_rx_proc_enable(dev, 0);
> >   	}
> >
> >   	return ret;

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] net/i40e: fix flow FDIR enable issue
  2020-05-19  7:16 [dpdk-dev] [PATCH] net/i40e: fix flow FDIR enable issue Wei Zhao
  2020-05-19  9:36 ` Cui, LunyuanX
  2020-05-20  3:18 ` Jeff Guo
@ 2020-05-20  4:13 ` Xing, Beilei
  2020-05-20  5:13   ` Zhao1, Wei
  2020-05-21  8:03 ` [dpdk-dev] [PATCH v2] " Wei Zhao
  3 siblings, 1 reply; 13+ messages in thread
From: Xing, Beilei @ 2020-05-20  4:13 UTC (permalink / raw)
  To: Zhao1, Wei, dev; +Cc: stable, Guo, Jia



> -----Original Message-----
> From: Zhao1, Wei <wei.zhao1@intel.com>
> Sent: Tuesday, May 19, 2020 3:17 PM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; Guo, Jia
> <jia.guo@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
> Subject: [PATCH] net/i40e: fix flow FDIR enable issue
> 
> When we flush flow FDIR, all queues are disabled for FDIR.
> If FDIR rule is created again, then the flow list is empty, as it is the first time to
> create rule after flush fdir filter, so we need to enable FDRI for all queues. And
> also, disable FDIR for queues should be done in function
> i40e_flow_flush_fdir_filter().
> 
> Cc: stable@dpdk.org
> Fixes: 1491f63c7559 ("net/i40e: fix flush of flow director filter")
> Fixes: 6ae9b2b5e8c2 ("net/i40e: cache flow director enable value in Rx
> queue")
> 
> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> ---
>  drivers/net/i40e/i40e_flow.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c index
> 25c77e7aa..94fc73092 100644
> --- a/drivers/net/i40e/i40e_flow.c
> +++ b/drivers/net/i40e/i40e_flow.c
> @@ -3462,6 +3462,12 @@ i40e_flow_parse_fdir_filter(struct rte_eth_dev
> *dev,
>  		}
>  	}
> 
> +	/* If the flow list is empty, it is the first time to create
> +	 * rule after flush fdir filter, so enable fdir.
> +	 */
> +	if (TAILQ_EMPTY(&pf->fdir.fdir_list))
> +		i40e_fdir_rx_proc_enable(dev, 1);
> +

i40e_fdir_rx_proc_enable is called in i40e_fdir_configure, so could you check if we can optimize this and if statement (pf->fdir.fdir_vsi == NULL) above?

>  	return 0;
>  err:
>  	i40e_fdir_teardown(pf);
> @@ -5330,9 +5336,6 @@ i40e_flow_flush(struct rte_eth_dev *dev, struct
> rte_flow_error *error)
>  		return -rte_errno;
>  	}
> 
> -	/* Disable FDIR processing as all FDIR rules are now flushed */
> -	i40e_fdir_rx_proc_enable(dev, 0);
> -
>  	return ret;
>  }
> 
> @@ -5368,6 +5371,9 @@ i40e_flow_flush_fdir_filter(struct i40e_pf *pf)
>  		for (pctype = I40E_FILTER_PCTYPE_NONF_IPV4_UDP;
>  		     pctype <= I40E_FILTER_PCTYPE_L2_PAYLOAD; pctype++)
>  			pf->fdir.inset_flag[pctype] = 0;
> +
> +		/* Disable FDIR processing as all FDIR rules are now flushed */
> +		i40e_fdir_rx_proc_enable(dev, 0);
>  	}
> 
>  	return ret;
> --
> 2.19.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] net/i40e: fix flow FDIR enable issue
  2020-05-20  4:13 ` Xing, Beilei
@ 2020-05-20  5:13   ` Zhao1, Wei
  2020-05-20  7:37     ` Xing, Beilei
  0 siblings, 1 reply; 13+ messages in thread
From: Zhao1, Wei @ 2020-05-20  5:13 UTC (permalink / raw)
  To: Xing, Beilei, dev; +Cc: stable, Guo, Jia



> -----Original Message-----
> From: Xing, Beilei <beilei.xing@intel.com>
> Sent: Wednesday, May 20, 2020 12:14 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; Guo, Jia <jia.guo@intel.com>
> Subject: RE: [PATCH] net/i40e: fix flow FDIR enable issue
> 
> 
> 
> > -----Original Message-----
> > From: Zhao1, Wei <wei.zhao1@intel.com>
> > Sent: Tuesday, May 19, 2020 3:17 PM
> > To: dev@dpdk.org
> > Cc: stable@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; Guo, Jia
> > <jia.guo@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
> > Subject: [PATCH] net/i40e: fix flow FDIR enable issue
> >
> > When we flush flow FDIR, all queues are disabled for FDIR.
> > If FDIR rule is created again, then the flow list is empty, as it is
> > the first time to create rule after flush fdir filter, so we need to
> > enable FDRI for all queues. And also, disable FDIR for queues should
> > be done in function i40e_flow_flush_fdir_filter().
> >
> > Cc: stable@dpdk.org
> > Fixes: 1491f63c7559 ("net/i40e: fix flush of flow director filter")
> > Fixes: 6ae9b2b5e8c2 ("net/i40e: cache flow director enable value in Rx
> > queue")
> >
> > Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> > ---
> >  drivers/net/i40e/i40e_flow.c | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_flow.c
> > b/drivers/net/i40e/i40e_flow.c index
> > 25c77e7aa..94fc73092 100644
> > --- a/drivers/net/i40e/i40e_flow.c
> > +++ b/drivers/net/i40e/i40e_flow.c
> > @@ -3462,6 +3462,12 @@ i40e_flow_parse_fdir_filter(struct rte_eth_dev
> > *dev,
> >  		}
> >  	}
> >
> > +	/* If the flow list is empty, it is the first time to create
> > +	 * rule after flush fdir filter, so enable fdir.
> > +	 */
> > +	if (TAILQ_EMPTY(&pf->fdir.fdir_list))
> > +		i40e_fdir_rx_proc_enable(dev, 1);
> > +
> 
> i40e_fdir_rx_proc_enable is called in i40e_fdir_configure, so could you check if
> we can optimize this and if statement (pf->fdir.fdir_vsi == NULL) above?

No, when "flow flush 0", the pf->fdir.fdir_vs is not NULL, but list pf->fdir.fdir_list is empty.

> 
> >  	return 0;
> >  err:
> >  	i40e_fdir_teardown(pf);
> > @@ -5330,9 +5336,6 @@ i40e_flow_flush(struct rte_eth_dev *dev, struct
> > rte_flow_error *error)
> >  		return -rte_errno;
> >  	}
> >
> > -	/* Disable FDIR processing as all FDIR rules are now flushed */
> > -	i40e_fdir_rx_proc_enable(dev, 0);
> > -
> >  	return ret;
> >  }
> >
> > @@ -5368,6 +5371,9 @@ i40e_flow_flush_fdir_filter(struct i40e_pf *pf)
> >  		for (pctype = I40E_FILTER_PCTYPE_NONF_IPV4_UDP;
> >  		     pctype <= I40E_FILTER_PCTYPE_L2_PAYLOAD; pctype++)
> >  			pf->fdir.inset_flag[pctype] = 0;
> > +
> > +		/* Disable FDIR processing as all FDIR rules are now flushed */
> > +		i40e_fdir_rx_proc_enable(dev, 0);
> >  	}
> >
> >  	return ret;
> > --
> > 2.19.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] net/i40e: fix flow FDIR enable issue
  2020-05-20  5:13   ` Zhao1, Wei
@ 2020-05-20  7:37     ` Xing, Beilei
  2020-05-20  7:43       ` Zhao1, Wei
  0 siblings, 1 reply; 13+ messages in thread
From: Xing, Beilei @ 2020-05-20  7:37 UTC (permalink / raw)
  To: Zhao1, Wei, dev; +Cc: stable, Guo, Jia



> -----Original Message-----
> From: Zhao1, Wei <wei.zhao1@intel.com>
> Sent: Wednesday, May 20, 2020 1:13 PM
> To: Xing, Beilei <beilei.xing@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; Guo, Jia <jia.guo@intel.com>
> Subject: RE: [PATCH] net/i40e: fix flow FDIR enable issue
> 
> 
> 
> > -----Original Message-----
> > From: Xing, Beilei <beilei.xing@intel.com>
> > Sent: Wednesday, May 20, 2020 12:14 PM
> > To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> > Cc: stable@dpdk.org; Guo, Jia <jia.guo@intel.com>
> > Subject: RE: [PATCH] net/i40e: fix flow FDIR enable issue
> >
> >
> >
> > > -----Original Message-----
> > > From: Zhao1, Wei <wei.zhao1@intel.com>
> > > Sent: Tuesday, May 19, 2020 3:17 PM
> > > To: dev@dpdk.org
> > > Cc: stable@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; Guo, Jia
> > > <jia.guo@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
> > > Subject: [PATCH] net/i40e: fix flow FDIR enable issue
> > >
> > > When we flush flow FDIR, all queues are disabled for FDIR.
> > > If FDIR rule is created again, then the flow list is empty, as it is
> > > the first time to create rule after flush fdir filter, so we need to
> > > enable FDRI for all queues. And also, disable FDIR for queues should
> > > be done in function i40e_flow_flush_fdir_filter().
> > >
> > > Cc: stable@dpdk.org
> > > Fixes: 1491f63c7559 ("net/i40e: fix flush of flow director filter")
> > > Fixes: 6ae9b2b5e8c2 ("net/i40e: cache flow director enable value in
> > > Rx
> > > queue")
> > >
> > > Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> > > ---
> > >  drivers/net/i40e/i40e_flow.c | 12 +++++++++---
> > >  1 file changed, 9 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/net/i40e/i40e_flow.c
> > > b/drivers/net/i40e/i40e_flow.c index
> > > 25c77e7aa..94fc73092 100644
> > > --- a/drivers/net/i40e/i40e_flow.c
> > > +++ b/drivers/net/i40e/i40e_flow.c
> > > @@ -3462,6 +3462,12 @@ i40e_flow_parse_fdir_filter(struct
> > > rte_eth_dev *dev,
> > >  		}
> > >  	}
> > >
> > > +	/* If the flow list is empty, it is the first time to create
> > > +	 * rule after flush fdir filter, so enable fdir.
> > > +	 */
> > > +	if (TAILQ_EMPTY(&pf->fdir.fdir_list))
> > > +		i40e_fdir_rx_proc_enable(dev, 1);
> > > +
> >
> > i40e_fdir_rx_proc_enable is called in i40e_fdir_configure, so could
> > you check if we can optimize this and if statement (pf->fdir.fdir_vsi == NULL)
> above?
> 
> No, when "flow flush 0", the pf->fdir.fdir_vs is not NULL, but list pf-
> >fdir.fdir_list is empty.

Then seems i40e_fdir_rx_proc_enable(dev, 1) can be removed from i40e_fdir_configure, since i40e_fdir_rx_proc_enable(dev, 1) will always be called when the first FDIR rule is created and fdir_list is empty.
Or we can use 'else if (TAILQ_EMPTY(&pf->fdir.fdir_list))' to avoid duplicate execution of i40e_fdir_rx_proc_enable(dev, 1).

> 
> >
> > >  	return 0;
> > >  err:
> > >  	i40e_fdir_teardown(pf);
> > > @@ -5330,9 +5336,6 @@ i40e_flow_flush(struct rte_eth_dev *dev,
> > > struct rte_flow_error *error)
> > >  		return -rte_errno;
> > >  	}
> > >
> > > -	/* Disable FDIR processing as all FDIR rules are now flushed */
> > > -	i40e_fdir_rx_proc_enable(dev, 0);
> > > -
> > >  	return ret;
> > >  }
> > >
> > > @@ -5368,6 +5371,9 @@ i40e_flow_flush_fdir_filter(struct i40e_pf *pf)
> > >  		for (pctype = I40E_FILTER_PCTYPE_NONF_IPV4_UDP;
> > >  		     pctype <= I40E_FILTER_PCTYPE_L2_PAYLOAD; pctype++)
> > >  			pf->fdir.inset_flag[pctype] = 0;
> > > +
> > > +		/* Disable FDIR processing as all FDIR rules are now flushed
> */
> > > +		i40e_fdir_rx_proc_enable(dev, 0);
> > >  	}
> > >
> > >  	return ret;
> > > --
> > > 2.19.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH] net/i40e: fix flow FDIR enable issue
  2020-05-20  7:37     ` Xing, Beilei
@ 2020-05-20  7:43       ` Zhao1, Wei
  0 siblings, 0 replies; 13+ messages in thread
From: Zhao1, Wei @ 2020-05-20  7:43 UTC (permalink / raw)
  To: Xing, Beilei, dev; +Cc: stable, Guo, Jia

Hi beilei

> -----Original Message-----
> From: Xing, Beilei <beilei.xing@intel.com>
> Sent: Wednesday, May 20, 2020 3:37 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; Guo, Jia <jia.guo@intel.com>
> Subject: RE: [PATCH] net/i40e: fix flow FDIR enable issue
> 
> 
> 
> > -----Original Message-----
> > From: Zhao1, Wei <wei.zhao1@intel.com>
> > Sent: Wednesday, May 20, 2020 1:13 PM
> > To: Xing, Beilei <beilei.xing@intel.com>; dev@dpdk.org
> > Cc: stable@dpdk.org; Guo, Jia <jia.guo@intel.com>
> > Subject: RE: [PATCH] net/i40e: fix flow FDIR enable issue
> >
> >
> >
> > > -----Original Message-----
> > > From: Xing, Beilei <beilei.xing@intel.com>
> > > Sent: Wednesday, May 20, 2020 12:14 PM
> > > To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> > > Cc: stable@dpdk.org; Guo, Jia <jia.guo@intel.com>
> > > Subject: RE: [PATCH] net/i40e: fix flow FDIR enable issue
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Zhao1, Wei <wei.zhao1@intel.com>
> > > > Sent: Tuesday, May 19, 2020 3:17 PM
> > > > To: dev@dpdk.org
> > > > Cc: stable@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; Guo,
> > > > Jia <jia.guo@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
> > > > Subject: [PATCH] net/i40e: fix flow FDIR enable issue
> > > >
> > > > When we flush flow FDIR, all queues are disabled for FDIR.
> > > > If FDIR rule is created again, then the flow list is empty, as it
> > > > is the first time to create rule after flush fdir filter, so we
> > > > need to enable FDRI for all queues. And also, disable FDIR for
> > > > queues should be done in function i40e_flow_flush_fdir_filter().
> > > >
> > > > Cc: stable@dpdk.org
> > > > Fixes: 1491f63c7559 ("net/i40e: fix flush of flow director
> > > > filter")
> > > > Fixes: 6ae9b2b5e8c2 ("net/i40e: cache flow director enable value
> > > > in Rx
> > > > queue")
> > > >
> > > > Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> > > > ---
> > > >  drivers/net/i40e/i40e_flow.c | 12 +++++++++---
> > > >  1 file changed, 9 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/net/i40e/i40e_flow.c
> > > > b/drivers/net/i40e/i40e_flow.c index
> > > > 25c77e7aa..94fc73092 100644
> > > > --- a/drivers/net/i40e/i40e_flow.c
> > > > +++ b/drivers/net/i40e/i40e_flow.c
> > > > @@ -3462,6 +3462,12 @@ i40e_flow_parse_fdir_filter(struct
> > > > rte_eth_dev *dev,
> > > >  		}
> > > >  	}
> > > >
> > > > +	/* If the flow list is empty, it is the first time to create
> > > > +	 * rule after flush fdir filter, so enable fdir.
> > > > +	 */
> > > > +	if (TAILQ_EMPTY(&pf->fdir.fdir_list))
> > > > +		i40e_fdir_rx_proc_enable(dev, 1);
> > > > +
> > >
> > > i40e_fdir_rx_proc_enable is called in i40e_fdir_configure, so could
> > > you check if we can optimize this and if statement
> > > (pf->fdir.fdir_vsi == NULL)
> > above?
> >
> > No, when "flow flush 0", the pf->fdir.fdir_vs is not NULL, but list
> > pf-
> > >fdir.fdir_list is empty.
> 
> Then seems i40e_fdir_rx_proc_enable(dev, 1) can be removed from
> i40e_fdir_configure, since i40e_fdir_rx_proc_enable(dev, 1) will always be
> called when the first FDIR rule is created and fdir_list is empty.
> Or we can use 'else if (TAILQ_EMPTY(&pf->fdir.fdir_list))' to avoid duplicate
> execution of i40e_fdir_rx_proc_enable(dev, 1).

i40e_fdir_rx_proc_enable(dev, 1) can not be remove by now, because i40e_fdir_configure() will be used by legacy fdir.
We can remove it only after i40 legacy API disappear. May be some user not using rte-flow FDIR.


> 
> >
> > >
> > > >  	return 0;
> > > >  err:
> > > >  	i40e_fdir_teardown(pf);
> > > > @@ -5330,9 +5336,6 @@ i40e_flow_flush(struct rte_eth_dev *dev,
> > > > struct rte_flow_error *error)
> > > >  		return -rte_errno;
> > > >  	}
> > > >
> > > > -	/* Disable FDIR processing as all FDIR rules are now flushed */
> > > > -	i40e_fdir_rx_proc_enable(dev, 0);
> > > > -
> > > >  	return ret;
> > > >  }
> > > >
> > > > @@ -5368,6 +5371,9 @@ i40e_flow_flush_fdir_filter(struct i40e_pf *pf)
> > > >  		for (pctype = I40E_FILTER_PCTYPE_NONF_IPV4_UDP;
> > > >  		     pctype <= I40E_FILTER_PCTYPE_L2_PAYLOAD; pctype++)
> > > >  			pf->fdir.inset_flag[pctype] = 0;
> > > > +
> > > > +		/* Disable FDIR processing as all FDIR rules are now flushed
> > */
> > > > +		i40e_fdir_rx_proc_enable(dev, 0);
> > > >  	}
> > > >
> > > >  	return ret;
> > > > --
> > > > 2.19.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [dpdk-dev] [PATCH v2] net/i40e: fix flow FDIR enable issue
  2020-05-19  7:16 [dpdk-dev] [PATCH] net/i40e: fix flow FDIR enable issue Wei Zhao
                   ` (2 preceding siblings ...)
  2020-05-20  4:13 ` Xing, Beilei
@ 2020-05-21  8:03 ` Wei Zhao
  2020-05-21 14:27   ` Jeff Guo
  2020-05-22  9:18   ` Ye Xiaolong
  3 siblings, 2 replies; 13+ messages in thread
From: Wei Zhao @ 2020-05-21  8:03 UTC (permalink / raw)
  To: dev; +Cc: stable, beilei.xing, jia.guo, xiaolong.ye, Wei Zhao

When we flush flow FDIR, all queues are disabled for FDIR.
If FDIR rule is created again, then the flow list is empty,
as it is the first time to create rule after flush fdir filter,
so we need to enable FDIR for all queues. And also, disable FDIR
for queues should be done in function i40e_flow_flush_fdir_filter().

Fixes: 1491f63c7559 ("net/i40e: fix flush of flow director filter")
Fixes: 6ae9b2b5e8c2 ("net/i40e: cache flow director enable value in Rx queue")
Cc: stable@dpdk.org

Signed-off-by: Wei Zhao <wei.zhao1@intel.com>

---

v2:
update log info
---
 drivers/net/i40e/i40e_flow.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c
index 25c77e7aa..8f8df6fae 100644
--- a/drivers/net/i40e/i40e_flow.c
+++ b/drivers/net/i40e/i40e_flow.c
@@ -3462,6 +3462,10 @@ i40e_flow_parse_fdir_filter(struct rte_eth_dev *dev,
 		}
 	}
 
+	/* If create the first fdir rule, enable fdir check for rx queues */
+	if (TAILQ_EMPTY(&pf->fdir.fdir_list))
+		i40e_fdir_rx_proc_enable(dev, 1);
+
 	return 0;
 err:
 	i40e_fdir_teardown(pf);
@@ -5330,9 +5334,6 @@ i40e_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
 		return -rte_errno;
 	}
 
-	/* Disable FDIR processing as all FDIR rules are now flushed */
-	i40e_fdir_rx_proc_enable(dev, 0);
-
 	return ret;
 }
 
@@ -5368,6 +5369,9 @@ i40e_flow_flush_fdir_filter(struct i40e_pf *pf)
 		for (pctype = I40E_FILTER_PCTYPE_NONF_IPV4_UDP;
 		     pctype <= I40E_FILTER_PCTYPE_L2_PAYLOAD; pctype++)
 			pf->fdir.inset_flag[pctype] = 0;
+
+		/* Disable FDIR processing as all FDIR rules are now flushed */
+		i40e_fdir_rx_proc_enable(dev, 0);
 	}
 
 	return ret;
-- 
2.19.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH v2] net/i40e: fix flow FDIR enable issue
  2020-05-21  8:03 ` [dpdk-dev] [PATCH v2] " Wei Zhao
@ 2020-05-21 14:27   ` Jeff Guo
  2020-05-22  0:59     ` Zhao1, Wei
  2020-05-22  9:18   ` Ye Xiaolong
  1 sibling, 1 reply; 13+ messages in thread
From: Jeff Guo @ 2020-05-21 14:27 UTC (permalink / raw)
  To: Wei Zhao, dev; +Cc: stable, beilei.xing, xiaolong.ye

hi, zhaowei

On 5/21/2020 4:03 PM, Wei Zhao wrote:
> When we flush flow FDIR, all queues are disabled for FDIR.
> If FDIR rule is created again, then the flow list is empty,
> as it is the first time to create rule after flush fdir filter,
> so we need to enable FDIR for all queues. And also, disable FDIR
> for queues should be done in function i40e_flow_flush_fdir_filter().
>
> Fixes: 1491f63c7559 ("net/i40e: fix flush of flow director filter")
> Fixes: 6ae9b2b5e8c2 ("net/i40e: cache flow director enable value in Rx queue")
> Cc: stable@dpdk.org
>
> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
>
> ---
>
> v2:
> update log info
> ---
>   drivers/net/i40e/i40e_flow.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c
> index 25c77e7aa..8f8df6fae 100644
> --- a/drivers/net/i40e/i40e_flow.c
> +++ b/drivers/net/i40e/i40e_flow.c
> @@ -3462,6 +3462,10 @@ i40e_flow_parse_fdir_filter(struct rte_eth_dev *dev,
>   		}
>   	}
>   
> +	/* If create the first fdir rule, enable fdir check for rx queues */
> +	if (TAILQ_EMPTY(&pf->fdir.fdir_list))
> +		i40e_fdir_rx_proc_enable(dev, 1);
> +


Do you think it is make sense that move the configure out of the priors 
condition? If so the list empty check is no need here.

     if (pf->fdir.fdir_vsi == NULL) {
.....
     }

     ret = i40e_fdir_configure(dev);


>   	return 0;
>   err:
>   	i40e_fdir_teardown(pf);
> @@ -5330,9 +5334,6 @@ i40e_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
>   		return -rte_errno;
>   	}
>   
> -	/* Disable FDIR processing as all FDIR rules are now flushed */
> -	i40e_fdir_rx_proc_enable(dev, 0);
> -
>   	return ret;
>   }
>   
> @@ -5368,6 +5369,9 @@ i40e_flow_flush_fdir_filter(struct i40e_pf *pf)
>   		for (pctype = I40E_FILTER_PCTYPE_NONF_IPV4_UDP;
>   		     pctype <= I40E_FILTER_PCTYPE_L2_PAYLOAD; pctype++)
>   			pf->fdir.inset_flag[pctype] = 0;
> +
> +		/* Disable FDIR processing as all FDIR rules are now flushed */
> +		i40e_fdir_rx_proc_enable(dev, 0);
>   	}
>   
>   	return ret;

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH v2] net/i40e: fix flow FDIR enable issue
  2020-05-21 14:27   ` Jeff Guo
@ 2020-05-22  0:59     ` Zhao1, Wei
  2020-05-22  2:16       ` Jeff Guo
  0 siblings, 1 reply; 13+ messages in thread
From: Zhao1, Wei @ 2020-05-22  0:59 UTC (permalink / raw)
  To: Guo, Jia, dev; +Cc: stable, Xing, Beilei, Ye, Xiaolong

Hi, jia

> -----Original Message-----
> From: Guo, Jia <jia.guo@intel.com>
> Sent: Thursday, May 21, 2020 10:27 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; Ye, Xiaolong
> <xiaolong.ye@intel.com>
> Subject: Re: [PATCH v2] net/i40e: fix flow FDIR enable issue
> 
> hi, zhaowei
> 
> On 5/21/2020 4:03 PM, Wei Zhao wrote:
> > When we flush flow FDIR, all queues are disabled for FDIR.
> > If FDIR rule is created again, then the flow list is empty, as it is
> > the first time to create rule after flush fdir filter, so we need to
> > enable FDIR for all queues. And also, disable FDIR for queues should
> > be done in function i40e_flow_flush_fdir_filter().
> >
> > Fixes: 1491f63c7559 ("net/i40e: fix flush of flow director filter")
> > Fixes: 6ae9b2b5e8c2 ("net/i40e: cache flow director enable value in Rx
> > queue")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> >
> > ---
> >
> > v2:
> > update log info
> > ---
> >   drivers/net/i40e/i40e_flow.c | 10 +++++++---
> >   1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_flow.c
> > b/drivers/net/i40e/i40e_flow.c index 25c77e7aa..8f8df6fae 100644
> > --- a/drivers/net/i40e/i40e_flow.c
> > +++ b/drivers/net/i40e/i40e_flow.c
> > @@ -3462,6 +3462,10 @@ i40e_flow_parse_fdir_filter(struct rte_eth_dev
> *dev,
> >   		}
> >   	}
> >
> > +	/* If create the first fdir rule, enable fdir check for rx queues */
> > +	if (TAILQ_EMPTY(&pf->fdir.fdir_list))
> > +		i40e_fdir_rx_proc_enable(dev, 1);
> > +
> 
> 
> Do you think it is make sense that move the configure out of the priors
> condition? If so the list empty check is no need here.
> 
>      if (pf->fdir.fdir_vsi == NULL) {
> .....
>      }
> 
>      ret = i40e_fdir_configure(dev);
> 

That seems more reasonable, but I do not know what will happen for this , because
That will cause i40e_fdir_configure() be called every time when download rule. 
I dare not make this change for fear of introducing more problems.

> 
> >   	return 0;
> >   err:
> >   	i40e_fdir_teardown(pf);
> > @@ -5330,9 +5334,6 @@ i40e_flow_flush(struct rte_eth_dev *dev, struct
> rte_flow_error *error)
> >   		return -rte_errno;
> >   	}
> >
> > -	/* Disable FDIR processing as all FDIR rules are now flushed */
> > -	i40e_fdir_rx_proc_enable(dev, 0);
> > -
> >   	return ret;
> >   }
> >
> > @@ -5368,6 +5369,9 @@ i40e_flow_flush_fdir_filter(struct i40e_pf *pf)
> >   		for (pctype = I40E_FILTER_PCTYPE_NONF_IPV4_UDP;
> >   		     pctype <= I40E_FILTER_PCTYPE_L2_PAYLOAD; pctype++)
> >   			pf->fdir.inset_flag[pctype] = 0;
> > +
> > +		/* Disable FDIR processing as all FDIR rules are now flushed */
> > +		i40e_fdir_rx_proc_enable(dev, 0);
> >   	}
> >
> >   	return ret;

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH v2] net/i40e: fix flow FDIR enable issue
  2020-05-22  0:59     ` Zhao1, Wei
@ 2020-05-22  2:16       ` Jeff Guo
  0 siblings, 0 replies; 13+ messages in thread
From: Jeff Guo @ 2020-05-22  2:16 UTC (permalink / raw)
  To: Zhao1, Wei, dev; +Cc: stable, Xing, Beilei, Ye, Xiaolong

hi, zhaowei

On 5/22/2020 8:59 AM, Zhao1, Wei wrote:
> Hi, jia
>
>> -----Original Message-----
>> From: Guo, Jia <jia.guo@intel.com>
>> Sent: Thursday, May 21, 2020 10:27 PM
>> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
>> Cc: stable@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; Ye, Xiaolong
>> <xiaolong.ye@intel.com>
>> Subject: Re: [PATCH v2] net/i40e: fix flow FDIR enable issue
>>
>> hi, zhaowei
>>
>> On 5/21/2020 4:03 PM, Wei Zhao wrote:
>>> When we flush flow FDIR, all queues are disabled for FDIR.
>>> If FDIR rule is created again, then the flow list is empty, as it is
>>> the first time to create rule after flush fdir filter, so we need to
>>> enable FDIR for all queues. And also, disable FDIR for queues should
>>> be done in function i40e_flow_flush_fdir_filter().
>>>
>>> Fixes: 1491f63c7559 ("net/i40e: fix flush of flow director filter")
>>> Fixes: 6ae9b2b5e8c2 ("net/i40e: cache flow director enable value in Rx
>>> queue")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
>>>
>>> ---
>>>
>>> v2:
>>> update log info
>>> ---
>>>    drivers/net/i40e/i40e_flow.c | 10 +++++++---
>>>    1 file changed, 7 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/i40e/i40e_flow.c
>>> b/drivers/net/i40e/i40e_flow.c index 25c77e7aa..8f8df6fae 100644
>>> --- a/drivers/net/i40e/i40e_flow.c
>>> +++ b/drivers/net/i40e/i40e_flow.c
>>> @@ -3462,6 +3462,10 @@ i40e_flow_parse_fdir_filter(struct rte_eth_dev
>> *dev,
>>>    }
>>>    }
>>>
>>> +/* If create the first fdir rule, enable fdir check for rx queues */
>>> +if (TAILQ_EMPTY(&pf->fdir.fdir_list))
>>> +i40e_fdir_rx_proc_enable(dev, 1);
>>> +
>>
>> Do you think it is make sense that move the configure out of the priors
>> condition? If so the list empty check is no need here.
>>
>>       if (pf->fdir.fdir_vsi == NULL) {
>> .....
>>       }
>>
>>       ret = i40e_fdir_configure(dev);
>>
> That seems more reasonable, but I do not know what will happen for this , because
> That will cause i40e_fdir_configure() be called every time when download rule.
> I dare not make this change for fear of introducing more problems.


make sense.


>>>    return 0;
>>>    err:
>>>    i40e_fdir_teardown(pf);
>>> @@ -5330,9 +5334,6 @@ i40e_flow_flush(struct rte_eth_dev *dev, struct
>> rte_flow_error *error)
>>>    return -rte_errno;
>>>    }
>>>
>>> -/* Disable FDIR processing as all FDIR rules are now flushed */
>>> -i40e_fdir_rx_proc_enable(dev, 0);
>>> -
>>>    return ret;
>>>    }
>>>
>>> @@ -5368,6 +5369,9 @@ i40e_flow_flush_fdir_filter(struct i40e_pf *pf)
>>>    for (pctype = I40E_FILTER_PCTYPE_NONF_IPV4_UDP;
>>>         pctype <= I40E_FILTER_PCTYPE_L2_PAYLOAD; pctype++)
>>>    pf->fdir.inset_flag[pctype] = 0;
>>> +
>>> +/* Disable FDIR processing as all FDIR rules are now flushed */
>>> +i40e_fdir_rx_proc_enable(dev, 0);
>>>    }
>>>
>>>    return ret;


Reviewed-by: Jeff Guo <jia.guo@intel.com>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [dpdk-dev] [PATCH v2] net/i40e: fix flow FDIR enable issue
  2020-05-21  8:03 ` [dpdk-dev] [PATCH v2] " Wei Zhao
  2020-05-21 14:27   ` Jeff Guo
@ 2020-05-22  9:18   ` Ye Xiaolong
  1 sibling, 0 replies; 13+ messages in thread
From: Ye Xiaolong @ 2020-05-22  9:18 UTC (permalink / raw)
  To: Wei Zhao; +Cc: dev, stable, beilei.xing, jia.guo

On 05/21, Wei Zhao wrote:
>When we flush flow FDIR, all queues are disabled for FDIR.
>If FDIR rule is created again, then the flow list is empty,
>as it is the first time to create rule after flush fdir filter,
>so we need to enable FDIR for all queues. And also, disable FDIR
>for queues should be done in function i40e_flow_flush_fdir_filter().
>
>Fixes: 1491f63c7559 ("net/i40e: fix flush of flow director filter")
>Fixes: 6ae9b2b5e8c2 ("net/i40e: cache flow director enable value in Rx queue")
>Cc: stable@dpdk.org
>
>Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
>
>---
>
>v2:
>update log info
>---
> drivers/net/i40e/i40e_flow.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c
>index 25c77e7aa..8f8df6fae 100644
>--- a/drivers/net/i40e/i40e_flow.c
>+++ b/drivers/net/i40e/i40e_flow.c
>@@ -3462,6 +3462,10 @@ i40e_flow_parse_fdir_filter(struct rte_eth_dev *dev,
> 		}
> 	}
> 
>+	/* If create the first fdir rule, enable fdir check for rx queues */
>+	if (TAILQ_EMPTY(&pf->fdir.fdir_list))
>+		i40e_fdir_rx_proc_enable(dev, 1);
>+
> 	return 0;
> err:
> 	i40e_fdir_teardown(pf);
>@@ -5330,9 +5334,6 @@ i40e_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
> 		return -rte_errno;
> 	}
> 
>-	/* Disable FDIR processing as all FDIR rules are now flushed */
>-	i40e_fdir_rx_proc_enable(dev, 0);
>-
> 	return ret;
> }
> 
>@@ -5368,6 +5369,9 @@ i40e_flow_flush_fdir_filter(struct i40e_pf *pf)
> 		for (pctype = I40E_FILTER_PCTYPE_NONF_IPV4_UDP;
> 		     pctype <= I40E_FILTER_PCTYPE_L2_PAYLOAD; pctype++)
> 			pf->fdir.inset_flag[pctype] = 0;
>+
>+		/* Disable FDIR processing as all FDIR rules are now flushed */
>+		i40e_fdir_rx_proc_enable(dev, 0);
> 	}
> 
> 	return ret;
>-- 
>2.19.1
>

Applied to dpdk-next-net-intel, Thanks.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-05-22  9:27 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-19  7:16 [dpdk-dev] [PATCH] net/i40e: fix flow FDIR enable issue Wei Zhao
2020-05-19  9:36 ` Cui, LunyuanX
2020-05-20  3:18 ` Jeff Guo
2020-05-20  3:22   ` Zhao1, Wei
2020-05-20  4:13 ` Xing, Beilei
2020-05-20  5:13   ` Zhao1, Wei
2020-05-20  7:37     ` Xing, Beilei
2020-05-20  7:43       ` Zhao1, Wei
2020-05-21  8:03 ` [dpdk-dev] [PATCH v2] " Wei Zhao
2020-05-21 14:27   ` Jeff Guo
2020-05-22  0:59     ` Zhao1, Wei
2020-05-22  2:16       ` Jeff Guo
2020-05-22  9:18   ` Ye Xiaolong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).