[dpdk-dev] [PATCH v3] net/i40e: Improved FDIR programming times

[dpdk-dev] [PATCH v3] net/i40e: Improved FDIR programming times

[Michael Lilja]

Previously, the FDIR programming time is +11ms on i40e.
This patch will result in an average programming time of
22usec with a max of 60usec .

Signed-off-by: Michael Lilja <ml@napatech.com>

---
v3:
* Code style fix
---
 drivers/net/i40e/i40e_fdir.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 28cc554f5..2162443f5 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -1296,23 +1296,28 @@ i40e_fdir_filter_programming(struct i40e_pf *pf,
        rte_wmb();
        I40E_PCI_REG_WRITE(txq->qtx_tail, txq->tx_tail);

-       for (i = 0; i < I40E_FDIR_WAIT_COUNT; i++) {
-               rte_delay_us(I40E_FDIR_WAIT_INTERVAL_US);
+       for (i = 0; i < (I40E_FDIR_WAIT_COUNT * I40E_FDIR_WAIT_INTERVAL_US); i++) {
                if ((txdp->cmd_type_offset_bsz &
-                               rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) ==
-                               rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))
+                       rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) ==
+                       rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))
                        break;
+               rte_delay_us(1);
        }
-       if (i >= I40E_FDIR_WAIT_COUNT) {
+       if (i >= (I40E_FDIR_WAIT_COUNT * I40E_FDIR_WAIT_INTERVAL_US)) {
                PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
-                           " time out to get DD on tx queue.");
+                       " time out to get DD on tx queue.");
                return -ETIMEDOUT;
        }
        /* totally delay 10 ms to check programming status*/
-       rte_delay_us((I40E_FDIR_WAIT_COUNT - i) * I40E_FDIR_WAIT_INTERVAL_US);
+       for (i = 0; i < (I40E_FDIR_WAIT_COUNT * I40E_FDIR_WAIT_INTERVAL_US); i++) {
+               if (i40e_check_fdir_programming_status(rxq) >= 0) {
+                       break;
+               }
+               rte_delay_us(1);
+       }
        if (i40e_check_fdir_programming_status(rxq) < 0) {
                PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
-                           " programming status reported.");
+                               " programming status reported.");
                return -ENOSYS;
        }

--
2.12.2

Disclaimer: This email and any files transmitted with it may contain confidential information intended for the addressee(s) only. The information is not to be surrendered or copied to unauthorized persons. If you have received this communication in error, please notify the sender immediately and delete this e-mail from your system.

Re: [dpdk-dev] [PATCH v3] net/i40e: Improved FDIR programming times

[Xing, Beilei]

Hi,

Seems my comments in v2 are not addressed, add the comments here again.

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Michael Lilja
> Sent: Wednesday, May 17, 2017 6:02 AM
> To: dev@dpdk.org
> Cc: Michael Lilja <ml@napatech.com>
> Subject: [dpdk-dev] [PATCH v3] net/i40e: Improved FDIR programming times
> 
> Previously, the FDIR programming time is +11ms on i40e.
> This patch will result in an average programming time of 22usec with a max of
> 60usec .
> 
> Signed-off-by: Michael Lilja <ml@napatech.com>
> 
> ---
> v3:
> * Code style fix
> ---
>  drivers/net/i40e/i40e_fdir.c | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c index
> 28cc554f5..2162443f5 100644
> --- a/drivers/net/i40e/i40e_fdir.c
> +++ b/drivers/net/i40e/i40e_fdir.c
> @@ -1296,23 +1296,28 @@ i40e_fdir_filter_programming(struct i40e_pf *pf,
>         rte_wmb();
>         I40E_PCI_REG_WRITE(txq->qtx_tail, txq->tx_tail);
> 
> -       for (i = 0; i < I40E_FDIR_WAIT_COUNT; i++) {
> -               rte_delay_us(I40E_FDIR_WAIT_INTERVAL_US);
> +       for (i = 0; i < (I40E_FDIR_WAIT_COUNT *
> + I40E_FDIR_WAIT_INTERVAL_US); i++) {
>                 if ((txdp->cmd_type_offset_bsz &
> -
> rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) ==
> -
> rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))
> +
> rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) ==
> +
> rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))
>                         break;
> +               rte_delay_us(1);
>         }
> -       if (i >= I40E_FDIR_WAIT_COUNT) {
> +       if (i >= (I40E_FDIR_WAIT_COUNT * I40E_FDIR_WAIT_INTERVAL_US))
> {
>                 PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
> -                           " time out to get DD on tx queue.");
> +                       " time out to get DD on tx queue.");
>                 return -ETIMEDOUT;
>         }
>         /* totally delay 10 ms to check programming status*/
> -       rte_delay_us((I40E_FDIR_WAIT_COUNT - i) *
> I40E_FDIR_WAIT_INTERVAL_US);
> +       for (i = 0; i < (I40E_FDIR_WAIT_COUNT *
> I40E_FDIR_WAIT_INTERVAL_US); i++) {
> +               if (i40e_check_fdir_programming_status(rxq) >= 0) {
> +                       break;

Braces {} should be removed here according to the coding style.
Besides, I think we can return 0 here directly.

> +               }
> +               rte_delay_us(1);
> +       }
>         if (i40e_check_fdir_programming_status(rxq) < 0) {

This condition can be removed, just keep the following error log.

>                 PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
> -                           " programming status reported.");
> +                               " programming status reported.");
>                 return -ENOSYS;
>         }
> 
> --
> 2.12.2
> 
> Disclaimer: This email and any files transmitted with it may contain confidential
> information intended for the addressee(s) only. The information is not to be
> surrendered or copied to unauthorized persons. If you have received this
> communication in error, please notify the sender immediately and delete this
> e-mail from your system.

Re: [dpdk-dev] [PATCH v3] net/i40e: Improved FDIR programming times

[Ferruh Yigit]

On 5/17/2017 3:22 AM, Xing, Beilei wrote:
> Hi,
> 
> Seems my comments in v2 are not addressed, add the comments here again.

Hi Michael,

And can you please use "--in-reply-to" option of the git send-email when
sending the new version of the patch.
To make new version of the patch in same mail thread, as a reply to
previous version.

Thanks,
ferruh

> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Michael Lilja
>> Sent: Wednesday, May 17, 2017 6:02 AM
>> To: dev@dpdk.org
>> Cc: Michael Lilja <ml@napatech.com>
>> Subject: [dpdk-dev] [PATCH v3] net/i40e: Improved FDIR programming times
>>
>> Previously, the FDIR programming time is +11ms on i40e.
>> This patch will result in an average programming time of 22usec with a max of
>> 60usec .
>>
>> Signed-off-by: Michael Lilja <ml@napatech.com>
<...>

[dpdk-dev] [PATCH v4] net/i40e: improved FDIR programming times

[Michael Lilja]

Previously, the FDIR programming time is +11ms on i40e.
This patch will result in an average programming time of
22usec with a max of 60usec .

Signed-off-by: Michael Lilja <ml@napatech.com>

---
v4:
* Code style fix
---
---
 drivers/net/i40e/i40e_fdir.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 28cc554f5..32f6aeafb 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -1296,27 +1296,27 @@ i40e_fdir_filter_programming(struct i40e_pf *pf,
 	rte_wmb();
 	I40E_PCI_REG_WRITE(txq->qtx_tail, txq->tx_tail);
 
-	for (i = 0; i < I40E_FDIR_WAIT_COUNT; i++) {
-		rte_delay_us(I40E_FDIR_WAIT_INTERVAL_US);
+	for (i = 0; i < (I40E_FDIR_WAIT_COUNT * I40E_FDIR_WAIT_INTERVAL_US); i++) {
 		if ((txdp->cmd_type_offset_bsz &
 				rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) ==
 				rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))
 			break;
+		rte_delay_us(1);
 	}
-	if (i >= I40E_FDIR_WAIT_COUNT) {
+	if (i >= (I40E_FDIR_WAIT_COUNT * I40E_FDIR_WAIT_INTERVAL_US)) {
 		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
 			    " time out to get DD on tx queue.");
 		return -ETIMEDOUT;
 	}
 	/* totally delay 10 ms to check programming status*/
-	rte_delay_us((I40E_FDIR_WAIT_COUNT - i) * I40E_FDIR_WAIT_INTERVAL_US);
-	if (i40e_check_fdir_programming_status(rxq) < 0) {
-		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
-			    " programming status reported.");
-		return -ENOSYS;
+	for (i = 0; i < (I40E_FDIR_WAIT_COUNT * I40E_FDIR_WAIT_INTERVAL_US); i++) {
+		if (i40e_check_fdir_programming_status(rxq) >= 0)
+			return 0;
+		rte_delay_us(1);
 	}
-
-	return 0;
+	PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
+		" programming status reported.");
+	return -ENOSYS;
 }
 
 /*
-- 
2.12.2

Re: [dpdk-dev] [PATCH v4] net/i40e: improved FDIR programming times

[Xing, Beilei]

Hi Michael,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Michael Lilja
> Sent: Wednesday, May 17, 2017 5:12 PM
> To: dev@dpdk.org
> Cc: Michael Lilja <ml@napatech.com>
> Subject: [dpdk-dev] [PATCH v4] net/i40e: improved FDIR programming times
> 
> Previously, the FDIR programming time is +11ms on i40e.
> This patch will result in an average programming time of 22usec with a max of
> 60usec .
> 
> Signed-off-by: Michael Lilja <ml@napatech.com>
> 
> ---
> v4:
> * Code style fix
> ---
> ---
>  drivers/net/i40e/i40e_fdir.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c index
> 28cc554f5..32f6aeafb 100644
> --- a/drivers/net/i40e/i40e_fdir.c
> +++ b/drivers/net/i40e/i40e_fdir.c
> @@ -1296,27 +1296,27 @@ i40e_fdir_filter_programming(struct i40e_pf *pf,
>  	rte_wmb();
>  	I40E_PCI_REG_WRITE(txq->qtx_tail, txq->tx_tail);
> 
> -	for (i = 0; i < I40E_FDIR_WAIT_COUNT; i++) {
> -		rte_delay_us(I40E_FDIR_WAIT_INTERVAL_US);
> +	for (i = 0; i < (I40E_FDIR_WAIT_COUNT *
> I40E_FDIR_WAIT_INTERVAL_US);
> +i++) {
>  		if ((txdp->cmd_type_offset_bsz &
> 
> 	rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) ==
> 
> 	rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))
>  			break;
> +		rte_delay_us(1);
>  	}
> -	if (i >= I40E_FDIR_WAIT_COUNT) {
> +	if (i >= (I40E_FDIR_WAIT_COUNT * I40E_FDIR_WAIT_INTERVAL_US))
> {
>  		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
>  			    " time out to get DD on tx queue.");
>  		return -ETIMEDOUT;
>  	}
>  	/* totally delay 10 ms to check programming status*/
> -	rte_delay_us((I40E_FDIR_WAIT_COUNT - i) *
> I40E_FDIR_WAIT_INTERVAL_US);
> -	if (i40e_check_fdir_programming_status(rxq) < 0) {
> -		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
> -			    " programming status reported.");
> -		return -ENOSYS;
> +	for (i = 0; i < (I40E_FDIR_WAIT_COUNT *
> I40E_FDIR_WAIT_INTERVAL_US); i++) {

To keep the original intention, "i" shouldn't be set to 0 again but keep above value.
Please refer to " rte_delay_us((I40E_FDIR_WAIT_COUNT - i) *> I40E_FDIR_WAIT_INTERVAL_US)".
Sorry for missing it before.

Overall it's OK for me, thanks.

Beilei

[dpdk-dev] [PATCH v5] net/i40e: improved FDIR programming times

[Michael Lilja]

Previously, the FDIR programming time is +11ms on i40e.
This patch will result in an average programming time of
22usec with a max of 60usec .

Signed-off-by: Michael Lilja <ml@napatech.com>

---
v5:
* Reinitialization of "i" inconsistent with original intent
---
---
 drivers/net/i40e/i40e_fdir.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 28cc554f5..85fd827e1 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -1296,27 +1296,27 @@ i40e_fdir_filter_programming(struct i40e_pf *pf,
 	rte_wmb();
 	I40E_PCI_REG_WRITE(txq->qtx_tail, txq->tx_tail);
 
-	for (i = 0; i < I40E_FDIR_WAIT_COUNT; i++) {
-		rte_delay_us(I40E_FDIR_WAIT_INTERVAL_US);
+	for (i = 0; i < (I40E_FDIR_WAIT_COUNT * I40E_FDIR_WAIT_INTERVAL_US); i++) {
 		if ((txdp->cmd_type_offset_bsz &
 				rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) ==
 				rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))
 			break;
+		rte_delay_us(1);
 	}
-	if (i >= I40E_FDIR_WAIT_COUNT) {
+	if (i >= (I40E_FDIR_WAIT_COUNT * I40E_FDIR_WAIT_INTERVAL_US)) {
 		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
 			    " time out to get DD on tx queue.");
 		return -ETIMEDOUT;
 	}
 	/* totally delay 10 ms to check programming status*/
-	rte_delay_us((I40E_FDIR_WAIT_COUNT - i) * I40E_FDIR_WAIT_INTERVAL_US);
-	if (i40e_check_fdir_programming_status(rxq) < 0) {
-		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
-			    " programming status reported.");
-		return -ENOSYS;
+	for (; i < (I40E_FDIR_WAIT_COUNT * I40E_FDIR_WAIT_INTERVAL_US); i++) {
+		if (i40e_check_fdir_programming_status(rxq) >= 0)
+			return 0;
+		rte_delay_us(1);
 	}
-
-	return 0;
+	PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
+		" programming status reported.");
+	return -ENOSYS;
 }
 
 /*
-- 
2.12.2

Re: [dpdk-dev] [PATCH v5] net/i40e: improved FDIR programming times

[Xing, Beilei]

Hi,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Michael Lilja
> Sent: Wednesday, May 17, 2017 6:38 PM
> To: dev@dpdk.org
> Cc: Michael Lilja <ml@napatech.com>
> Subject: [dpdk-dev] [PATCH v5] net/i40e: improved FDIR programming times
> 
> Previously, the FDIR programming time is +11ms on i40e.
> This patch will result in an average programming time of 22usec with a max of
> 60usec .
> 
> Signed-off-by: Michael Lilja <ml@napatech.com>

Acked-by: Beilei Xing <beilei.xing@intel.com>

Re: [dpdk-dev] [PATCH v5] net/i40e: improved FDIR programming times

[Ferruh Yigit]

On 5/17/2017 11:38 AM, Michael Lilja wrote:
> Previously, the FDIR programming time is +11ms on i40e.
> This patch will result in an average programming time of
> 22usec with a max of 60usec .
> 
> Signed-off-by: Michael Lilja <ml@napatech.com>

Please keeps maintainers in CC while sending patches.

> 
> ---
> v5:
> * Reinitialization of "i" inconsistent with original intent

It can be useful to keep history about older versions.

> ---

There are two checkpatch warnings, can you please fix them [1], you can
keep Beilei's ack in next version.

[1]
WARNING:LONG_LINE: line over 80 characters
#39: FILE: drivers/net/i40e/i40e_fdir.c:1299:
+       for (i = 0; i < (I40E_FDIR_WAIT_COUNT *
I40E_FDIR_WAIT_INTERVAL_US); i++) {

WARNING:ENOSYS: ENOSYS means 'invalid syscall nr' and nothing else
#67: FILE: drivers/net/i40e/i40e_fdir.c:1319:
+       return -ENOSYS;

<...>

[dpdk-dev] [PATCH v6] net/i40e: improved FDIR programming times

[Michael Lilja]

Previously, the FDIR programming time is +11ms on i40e.
This patch will result in an average programming time of
22usec with a max of 60usec .

Signed-off-by: Michael Lilja <ml@napatech.com>

---
v6:
* Fixed code style issues

v5:
* Reinitialization of "i" inconsistent with original intent

v4:
* Code style fix

v3:
* Replaced commit message

v2:
*  Code style fix

v1:
* Initial version
---
---
 drivers/net/i40e/i40e_fdir.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 28cc554f5..16cb963ce 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -1295,28 +1295,28 @@ i40e_fdir_filter_programming(struct i40e_pf *pf,
 	/* Update the tx tail register */
 	rte_wmb();
 	I40E_PCI_REG_WRITE(txq->qtx_tail, txq->tx_tail);
-
-	for (i = 0; i < I40E_FDIR_WAIT_COUNT; i++) {
-		rte_delay_us(I40E_FDIR_WAIT_INTERVAL_US);
+	i = 0;
+	for (; i < (I40E_FDIR_WAIT_COUNT * I40E_FDIR_WAIT_INTERVAL_US); i++) {
 		if ((txdp->cmd_type_offset_bsz &
-				rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) ==
-				rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))
+			rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) ==
+			rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))
 			break;
+		rte_delay_us(1);
 	}
-	if (i >= I40E_FDIR_WAIT_COUNT) {
+	if (i >= (I40E_FDIR_WAIT_COUNT * I40E_FDIR_WAIT_INTERVAL_US)) {
 		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
 			    " time out to get DD on tx queue.");
 		return -ETIMEDOUT;
 	}
 	/* totally delay 10 ms to check programming status*/
-	rte_delay_us((I40E_FDIR_WAIT_COUNT - i) * I40E_FDIR_WAIT_INTERVAL_US);
-	if (i40e_check_fdir_programming_status(rxq) < 0) {
-		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
-			    " programming status reported.");
-		return -ENOSYS;
+	for (; i < (I40E_FDIR_WAIT_COUNT * I40E_FDIR_WAIT_INTERVAL_US); i++) {
+		if (i40e_check_fdir_programming_status(rxq) >= 0)
+			return 0;
+		rte_delay_us(1);
 	}
-
-	return 0;
+	PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
+		" programming status reported.");
+	return -ETIMEDOUT;
 }
 
 /*
-- 
2.12.2

Re: [dpdk-dev] [PATCH v6] net/i40e: improved FDIR programming times

[Ferruh Yigit]

On 5/17/2017 2:45 PM, Michael Lilja wrote:
> Previously, the FDIR programming time is +11ms on i40e.
> This patch will result in an average programming time of
> 22usec with a max of 60usec .
> 
> Signed-off-by: Michael Lilja <ml@napatech.com>

Please cc maintainers in the patch.

> 
> ---
> v6:
> * Fixed code style issues
> 
> v5:
> * Reinitialization of "i" inconsistent with original intent
> 
> v4:
> * Code style fix
> 
> v3:
> * Replaced commit message
> 
> v2:
> *  Code style fix
> 
> v1:
> * Initial version
> ---
> ---
>  drivers/net/i40e/i40e_fdir.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
> index 28cc554f5..16cb963ce 100644
> --- a/drivers/net/i40e/i40e_fdir.c
> +++ b/drivers/net/i40e/i40e_fdir.c
> @@ -1295,28 +1295,28 @@ i40e_fdir_filter_programming(struct i40e_pf *pf,
>  	/* Update the tx tail register */
>  	rte_wmb();
>  	I40E_PCI_REG_WRITE(txq->qtx_tail, txq->tx_tail);
> -
> -	for (i = 0; i < I40E_FDIR_WAIT_COUNT; i++) {
> -		rte_delay_us(I40E_FDIR_WAIT_INTERVAL_US);
> +	i = 0;

This is extracted out of "for" to stay in 80 columns limit, but instead
what do you think:

Create a variable, something like "wait_us_count":
wait_us_count = I40E_FDIR_WAIT_COUNT * I40E_FDIR_WAIT_INTERVAL_US;

and used it below three times, and lines will stay in limit.

> +	for (; i < (I40E_FDIR_WAIT_COUNT * I40E_FDIR_WAIT_INTERVAL_US); i++) {
>  		if ((txdp->cmd_type_offset_bsz &
> -				rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) ==
> -				rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))
> +			rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) ==
> +			rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))

Old indentation was correct I think, that is to differentiate the code
in below line easily.

>  			break;
> +		rte_delay_us(1);
>  	}
> -	if (i >= I40E_FDIR_WAIT_COUNT) {
> +	if (i >= (I40E_FDIR_WAIT_COUNT * I40E_FDIR_WAIT_INTERVAL_US)) {
>  		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
>  			    " time out to get DD on tx queue.");
>  		return -ETIMEDOUT;
>  	}
>  	/* totally delay 10 ms to check programming status*/
> -	rte_delay_us((I40E_FDIR_WAIT_COUNT - i) * I40E_FDIR_WAIT_INTERVAL_US);
> -	if (i40e_check_fdir_programming_status(rxq) < 0) {
> -		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
> -			    " programming status reported.");
> -		return -ENOSYS;
> +	for (; i < (I40E_FDIR_WAIT_COUNT * I40E_FDIR_WAIT_INTERVAL_US); i++) {
> +		if (i40e_check_fdir_programming_status(rxq) >= 0)
> +			return 0;
> +		rte_delay_us(1);
>  	}
> -
> -	return 0;
> +	PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
> +		" programming status reported.");
> +	return -ETIMEDOUT;
>  }
>  
>  /*
> 

[dpdk-dev] [PATCH v7] net/i40e: improved FDIR programming times

[Michael Lilja]

Previously, the FDIR programming time is +11ms on i40e.
This patch will result in an average programming time of
22usec with a max of 60usec .

Signed-off-by: Michael Lilja <ml@napatech.com>

---
v7:
* Code style changes

v6:
* Fixed code style issues

v5:
* Reinitialization of "i" inconsistent with original intent

v4:
* Code style fix

v3:
* Replaced commit message

v2:
*  Code style fix

v1:
* Initial version
---
---
 drivers/net/i40e/i40e_fdir.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 28cc554f5..1192d5831 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -76,6 +76,7 @@
 /* Wait count and interval for fdir filter programming */
 #define I40E_FDIR_WAIT_COUNT       10
 #define I40E_FDIR_WAIT_INTERVAL_US 1000
+#define I40E_FDIR_MAX_WAIT (I40E_FDIR_WAIT_COUNT * I40E_FDIR_WAIT_INTERVAL_US)
 
 /* Wait count and interval for fdir filter flush */
 #define I40E_FDIR_FLUSH_RETRY       50
@@ -1295,28 +1296,27 @@ i40e_fdir_filter_programming(struct i40e_pf *pf,
 	/* Update the tx tail register */
 	rte_wmb();
 	I40E_PCI_REG_WRITE(txq->qtx_tail, txq->tx_tail);
-
-	for (i = 0; i < I40E_FDIR_WAIT_COUNT; i++) {
-		rte_delay_us(I40E_FDIR_WAIT_INTERVAL_US);
+	for (i = 0; i < I40E_FDIR_MAX_WAIT; i++) {
 		if ((txdp->cmd_type_offset_bsz &
 				rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) ==
 				rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))
 			break;
+		rte_delay_us(1);
 	}
-	if (i >= I40E_FDIR_WAIT_COUNT) {
+	if (i >= I40E_FDIR_MAX_WAIT) {
 		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
 			    " time out to get DD on tx queue.");
 		return -ETIMEDOUT;
 	}
 	/* totally delay 10 ms to check programming status*/
-	rte_delay_us((I40E_FDIR_WAIT_COUNT - i) * I40E_FDIR_WAIT_INTERVAL_US);
-	if (i40e_check_fdir_programming_status(rxq) < 0) {
-		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
-			    " programming status reported.");
-		return -ENOSYS;
+	for (; i < I40E_FDIR_MAX_WAIT; i++) {
+		if (i40e_check_fdir_programming_status(rxq) >= 0)
+			return 0;
+		rte_delay_us(1);
 	}
-
-	return 0;
+	PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
+		" programming status reported.");
+	return -ETIMEDOUT;
 }
 
 /*
-- 
2.12.2

Re: [dpdk-dev] [PATCH v7] net/i40e: improved FDIR programming times

[Ferruh Yigit]

On 5/17/2017 3:31 PM, Michael Lilja wrote:
> Previously, the FDIR programming time is +11ms on i40e.
> This patch will result in an average programming time of
> 22usec with a max of 60usec .
> 
> Signed-off-by: Michael Lilja <ml@napatech.com>

Sorry for multiple, minor change requests ...

> 
> ---
> v7:
> * Code style changes
> 
> v6:
> * Fixed code style issues
> 
> v5:
> * Reinitialization of "i" inconsistent with original intent
> 
> v4:
> * Code style fix
> 
> v3:
> * Replaced commit message
> 
> v2:
> *  Code style fix
> 
> v1:
> * Initial version
> ---
> ---
>  drivers/net/i40e/i40e_fdir.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
> index 28cc554f5..1192d5831 100644
> --- a/drivers/net/i40e/i40e_fdir.c
> +++ b/drivers/net/i40e/i40e_fdir.c
> @@ -76,6 +76,7 @@
>  /* Wait count and interval for fdir filter programming */
>  #define I40E_FDIR_WAIT_COUNT       10
>  #define I40E_FDIR_WAIT_INTERVAL_US 1000
> +#define I40E_FDIR_MAX_WAIT (I40E_FDIR_WAIT_COUNT * I40E_FDIR_WAIT_INTERVAL_US)

It looks like I40E_FDIR_WAIT_COUNT and I40E_FDIR_WAIT_INTERVAL_US not
used anywhere else, is there any value to keep them?

why not:
#define I40E_FDIR_MAX_WAIT_US 10000 /* 10 ms */

>  
>  /* Wait count and interval for fdir filter flush */
>  #define I40E_FDIR_FLUSH_RETRY       50
> @@ -1295,28 +1296,27 @@ i40e_fdir_filter_programming(struct i40e_pf *pf,
>  	/* Update the tx tail register */
>  	rte_wmb();
>  	I40E_PCI_REG_WRITE(txq->qtx_tail, txq->tx_tail);
> -
> -	for (i = 0; i < I40E_FDIR_WAIT_COUNT; i++) {
> -		rte_delay_us(I40E_FDIR_WAIT_INTERVAL_US);
> +	for (i = 0; i < I40E_FDIR_MAX_WAIT; i++) {
>  		if ((txdp->cmd_type_offset_bsz &
>  				rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) ==
>  				rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))
>  			break;
> +		rte_delay_us(1);
>  	}
> -	if (i >= I40E_FDIR_WAIT_COUNT) {
> +	if (i >= I40E_FDIR_MAX_WAIT) {
>  		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
>  			    " time out to get DD on tx queue.");
>  		return -ETIMEDOUT;
>  	}
>  	/* totally delay 10 ms to check programming status*/
> -	rte_delay_us((I40E_FDIR_WAIT_COUNT - i) * I40E_FDIR_WAIT_INTERVAL_US);
> -	if (i40e_check_fdir_programming_status(rxq) < 0) {
> -		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
> -			    " programming status reported.");
> -		return -ENOSYS;
> +	for (; i < I40E_FDIR_MAX_WAIT; i++) {
> +		if (i40e_check_fdir_programming_status(rxq) >= 0)
> +			return 0;
> +		rte_delay_us(1);
>  	}
> -
> -	return 0;
> +	PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
> +		" programming status reported.");
> +	return -ETIMEDOUT;
>  }
>  
>  /*
> 

Re: [dpdk-dev] [PATCH v7] net/i40e: improved FDIR programming times

[Michael Lilja]

It's ok. I didn't write the original code so I cannot tell why the two defines were made in the initial case. It make sense to remove them, but the maintainers must have had a reason, maybe they are needed in a future version of the code?

/Michael

-----Original Message-----
From: Ferruh Yigit [mailto:ferruh.yigit@intel.com] 
Sent: 17 May 2017 16:44
To: Michael Lilja <ml@napatech.com>; helin.zhang@intel.com; jingjing.wu@intel.com
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v7] net/i40e: improved FDIR programming times

On 5/17/2017 3:31 PM, Michael Lilja wrote:
> Previously, the FDIR programming time is +11ms on i40e.
> This patch will result in an average programming time of 22usec with a 
> max of 60usec .
> 
> Signed-off-by: Michael Lilja <ml@napatech.com>

Sorry for multiple, minor change requests ...

> 
> ---
> v7:
> * Code style changes
> 
> v6:
> * Fixed code style issues
> 
> v5:
> * Reinitialization of "i" inconsistent with original intent
> 
> v4:
> * Code style fix
> 
> v3:
> * Replaced commit message
> 
> v2:
> *  Code style fix
> 
> v1:
> * Initial version
> ---
> ---
>  drivers/net/i40e/i40e_fdir.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_fdir.c 
> b/drivers/net/i40e/i40e_fdir.c index 28cc554f5..1192d5831 100644
> --- a/drivers/net/i40e/i40e_fdir.c
> +++ b/drivers/net/i40e/i40e_fdir.c
> @@ -76,6 +76,7 @@
>  /* Wait count and interval for fdir filter programming */
>  #define I40E_FDIR_WAIT_COUNT       10
>  #define I40E_FDIR_WAIT_INTERVAL_US 1000
> +#define I40E_FDIR_MAX_WAIT (I40E_FDIR_WAIT_COUNT * 
> +I40E_FDIR_WAIT_INTERVAL_US)

It looks like I40E_FDIR_WAIT_COUNT and I40E_FDIR_WAIT_INTERVAL_US not used anywhere else, is there any value to keep them?

why not:
#define I40E_FDIR_MAX_WAIT_US 10000 /* 10 ms */

>  
>  /* Wait count and interval for fdir filter flush */
>  #define I40E_FDIR_FLUSH_RETRY       50
> @@ -1295,28 +1296,27 @@ i40e_fdir_filter_programming(struct i40e_pf *pf,
>  	/* Update the tx tail register */
>  	rte_wmb();
>  	I40E_PCI_REG_WRITE(txq->qtx_tail, txq->tx_tail);
> -
> -	for (i = 0; i < I40E_FDIR_WAIT_COUNT; i++) {
> -		rte_delay_us(I40E_FDIR_WAIT_INTERVAL_US);
> +	for (i = 0; i < I40E_FDIR_MAX_WAIT; i++) {
>  		if ((txdp->cmd_type_offset_bsz &
>  				rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) ==
>  				rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))
>  			break;
> +		rte_delay_us(1);
>  	}
> -	if (i >= I40E_FDIR_WAIT_COUNT) {
> +	if (i >= I40E_FDIR_MAX_WAIT) {
>  		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
>  			    " time out to get DD on tx queue.");
>  		return -ETIMEDOUT;
>  	}
>  	/* totally delay 10 ms to check programming status*/
> -	rte_delay_us((I40E_FDIR_WAIT_COUNT - i) * I40E_FDIR_WAIT_INTERVAL_US);
> -	if (i40e_check_fdir_programming_status(rxq) < 0) {
> -		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
> -			    " programming status reported.");
> -		return -ENOSYS;
> +	for (; i < I40E_FDIR_MAX_WAIT; i++) {
> +		if (i40e_check_fdir_programming_status(rxq) >= 0)
> +			return 0;
> +		rte_delay_us(1);
>  	}
> -
> -	return 0;
> +	PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
> +		" programming status reported.");
> +	return -ETIMEDOUT;
>  }
>  
>  /*
> 

Re: [dpdk-dev] [PATCH v7] net/i40e: improved FDIR programming times

[Ferruh Yigit]

On 5/17/2017 3:46 PM, Michael Lilja wrote:
> It's ok. I didn't write the original code so I cannot tell why the two defines were made in the initial case. It make sense to remove them, but the maintainers must have had a reason, maybe they are needed in a future version of the code?

In original code, they have a meaning:
for (i = 0; i < I40E_FDIR_WAIT_COUNT; i++)
	rte_delay_us(I40E_FDIR_WAIT_INTERVAL_US);

wait step is I40E_FDIR_WAIT_INTERVAL_US.

But you changed to fixes 1us stepping. So WAIT_COUNT and
WAIT_INTERVAL_US are no more meaningful. And since they are not used
anywhere else, I think they can go away.

And we can wait from maintainers ack for any "plan to use in the future"
case.

Thanks,
ferruh

> 
> /Michael
> 
> -----Original Message-----
> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: 17 May 2017 16:44
> To: Michael Lilja <ml@napatech.com>; helin.zhang@intel.com; jingjing.wu@intel.com
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v7] net/i40e: improved FDIR programming times
> 
> On 5/17/2017 3:31 PM, Michael Lilja wrote:
>> Previously, the FDIR programming time is +11ms on i40e.
>> This patch will result in an average programming time of 22usec with a
>> max of 60usec .
>>
>> Signed-off-by: Michael Lilja <ml@napatech.com>
> 
> Sorry for multiple, minor change requests ...
> 
>>
>> ---
>> v7:
>> * Code style changes
>>
>> v6:
>> * Fixed code style issues
>>
>> v5:
>> * Reinitialization of "i" inconsistent with original intent
>>
>> v4:
>> * Code style fix
>>
>> v3:
>> * Replaced commit message
>>
>> v2:
>> *  Code style fix
>>
>> v1:
>> * Initial version
>> ---
>> ---
>>  drivers/net/i40e/i40e_fdir.c | 22 +++++++++++-----------
>>  1 file changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/net/i40e/i40e_fdir.c
>> b/drivers/net/i40e/i40e_fdir.c index 28cc554f5..1192d5831 100644
>> --- a/drivers/net/i40e/i40e_fdir.c
>> +++ b/drivers/net/i40e/i40e_fdir.c
>> @@ -76,6 +76,7 @@
>>  /* Wait count and interval for fdir filter programming */
>>  #define I40E_FDIR_WAIT_COUNT       10
>>  #define I40E_FDIR_WAIT_INTERVAL_US 1000
>> +#define I40E_FDIR_MAX_WAIT (I40E_FDIR_WAIT_COUNT *
>> +I40E_FDIR_WAIT_INTERVAL_US)
> 
> It looks like I40E_FDIR_WAIT_COUNT and I40E_FDIR_WAIT_INTERVAL_US not used anywhere else, is there any value to keep them?
> 
> why not:
> #define I40E_FDIR_MAX_WAIT_US 10000 /* 10 ms */
> 
>>
>>  /* Wait count and interval for fdir filter flush */
>>  #define I40E_FDIR_FLUSH_RETRY       50
>> @@ -1295,28 +1296,27 @@ i40e_fdir_filter_programming(struct i40e_pf *pf,
>>  /* Update the tx tail register */
>>  rte_wmb();
>>  I40E_PCI_REG_WRITE(txq->qtx_tail, txq->tx_tail);
>> -
>> -for (i = 0; i < I40E_FDIR_WAIT_COUNT; i++) {
>> -rte_delay_us(I40E_FDIR_WAIT_INTERVAL_US);
>> +for (i = 0; i < I40E_FDIR_MAX_WAIT; i++) {
>>  if ((txdp->cmd_type_offset_bsz &
>>  rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) ==
>>  rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))
>>  break;
>> +rte_delay_us(1);
>>  }
>> -if (i >= I40E_FDIR_WAIT_COUNT) {
>> +if (i >= I40E_FDIR_MAX_WAIT) {
>>  PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
>>      " time out to get DD on tx queue.");
>>  return -ETIMEDOUT;
>>  }
>>  /* totally delay 10 ms to check programming status*/
>> -rte_delay_us((I40E_FDIR_WAIT_COUNT - i) * I40E_FDIR_WAIT_INTERVAL_US);
>> -if (i40e_check_fdir_programming_status(rxq) < 0) {
>> -PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
>> -    " programming status reported.");
>> -return -ENOSYS;
>> +for (; i < I40E_FDIR_MAX_WAIT; i++) {
>> +if (i40e_check_fdir_programming_status(rxq) >= 0)
>> +return 0;
>> +rte_delay_us(1);
>>  }
>> -
>> -return 0;
>> +PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
>> +" programming status reported.");
>> +return -ETIMEDOUT;
>>  }
>>
>>  /*
>>
> 
> Disclaimer: This email and any files transmitted with it may contain confidential information intended for the addressee(s) only. The information is not to be surrendered or copied to unauthorized persons. If you have received this communication in error, please notify the sender immediately and delete this e-mail from your system.
> 

Re: [dpdk-dev] [PATCH v7] net/i40e: improved FDIR programming times

[Michael Lilja]

Ok, I'll make a v8 removing the define.

/Michael

-----Original Message-----
From: Ferruh Yigit [mailto:ferruh.yigit@intel.com] 
Sent: 17 May 2017 16:50
To: Michael Lilja <ml@napatech.com>; helin.zhang@intel.com; jingjing.wu@intel.com
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v7] net/i40e: improved FDIR programming times

On 5/17/2017 3:46 PM, Michael Lilja wrote:
> It's ok. I didn't write the original code so I cannot tell why the two defines were made in the initial case. It make sense to remove them, but the maintainers must have had a reason, maybe they are needed in a future version of the code?

In original code, they have a meaning:
for (i = 0; i < I40E_FDIR_WAIT_COUNT; i++)
	rte_delay_us(I40E_FDIR_WAIT_INTERVAL_US);

wait step is I40E_FDIR_WAIT_INTERVAL_US.

But you changed to fixes 1us stepping. So WAIT_COUNT and WAIT_INTERVAL_US are no more meaningful. And since they are not used anywhere else, I think they can go away.

And we can wait from maintainers ack for any "plan to use in the future"
case.

Thanks,
ferruh

> 
> /Michael
> 
> -----Original Message-----
> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: 17 May 2017 16:44
> To: Michael Lilja <ml@napatech.com>; helin.zhang@intel.com; 
> jingjing.wu@intel.com
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v7] net/i40e: improved FDIR programming 
> times
> 
> On 5/17/2017 3:31 PM, Michael Lilja wrote:
>> Previously, the FDIR programming time is +11ms on i40e.
>> This patch will result in an average programming time of 22usec with 
>> a max of 60usec .
>>
>> Signed-off-by: Michael Lilja <ml@napatech.com>
> 
> Sorry for multiple, minor change requests ...
> 
>>
>> ---
>> v7:
>> * Code style changes
>>
>> v6:
>> * Fixed code style issues
>>
>> v5:
>> * Reinitialization of "i" inconsistent with original intent
>>
>> v4:
>> * Code style fix
>>
>> v3:
>> * Replaced commit message
>>
>> v2:
>> *  Code style fix
>>
>> v1:
>> * Initial version
>> ---
>> ---
>>  drivers/net/i40e/i40e_fdir.c | 22 +++++++++++-----------
>>  1 file changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/net/i40e/i40e_fdir.c 
>> b/drivers/net/i40e/i40e_fdir.c index 28cc554f5..1192d5831 100644
>> --- a/drivers/net/i40e/i40e_fdir.c
>> +++ b/drivers/net/i40e/i40e_fdir.c
>> @@ -76,6 +76,7 @@
>>  /* Wait count and interval for fdir filter programming */
>>  #define I40E_FDIR_WAIT_COUNT       10
>>  #define I40E_FDIR_WAIT_INTERVAL_US 1000
>> +#define I40E_FDIR_MAX_WAIT (I40E_FDIR_WAIT_COUNT *
>> +I40E_FDIR_WAIT_INTERVAL_US)
> 
> It looks like I40E_FDIR_WAIT_COUNT and I40E_FDIR_WAIT_INTERVAL_US not used anywhere else, is there any value to keep them?
> 
> why not:
> #define I40E_FDIR_MAX_WAIT_US 10000 /* 10 ms */
> 
>>
>>  /* Wait count and interval for fdir filter flush */
>>  #define I40E_FDIR_FLUSH_RETRY       50
>> @@ -1295,28 +1296,27 @@ i40e_fdir_filter_programming(struct i40e_pf 
>> *pf,
>>  /* Update the tx tail register */
>>  rte_wmb();
>>  I40E_PCI_REG_WRITE(txq->qtx_tail, txq->tx_tail);
>> -
>> -for (i = 0; i < I40E_FDIR_WAIT_COUNT; i++) { 
>> -rte_delay_us(I40E_FDIR_WAIT_INTERVAL_US);
>> +for (i = 0; i < I40E_FDIR_MAX_WAIT; i++) {
>>  if ((txdp->cmd_type_offset_bsz &
>>  rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) ==
>>  rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))
>>  break;
>> +rte_delay_us(1);
>>  }
>> -if (i >= I40E_FDIR_WAIT_COUNT) {
>> +if (i >= I40E_FDIR_MAX_WAIT) {
>>  PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
>>      " time out to get DD on tx queue.");  return -ETIMEDOUT;  }
>>  /* totally delay 10 ms to check programming status*/ 
>> -rte_delay_us((I40E_FDIR_WAIT_COUNT - i) * 
>> I40E_FDIR_WAIT_INTERVAL_US); -if 
>> (i40e_check_fdir_programming_status(rxq) < 0) { -PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
>> -    " programming status reported.");
>> -return -ENOSYS;
>> +for (; i < I40E_FDIR_MAX_WAIT; i++) { if 
>> +(i40e_check_fdir_programming_status(rxq) >= 0) return 0; 
>> +rte_delay_us(1);
>>  }
>> -
>> -return 0;
>> +PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
>> +" programming status reported.");
>> +return -ETIMEDOUT;
>>  }
>>
>>  /*
>>
> 
> Disclaimer: This email and any files transmitted with it may contain confidential information intended for the addressee(s) only. The information is not to be surrendered or copied to unauthorized persons. If you have received this communication in error, please notify the sender immediately and delete this e-mail from your system.
> 

[dpdk-dev] [PATCH v8] net/i40e: improved FDIR programming times

[Michael Lilja]

Previously, the FDIR programming time is +11ms on i40e.
This patch will result in an average programming time of
22usec with a max of 60usec .

Signed-off-by: Michael Lilja <ml@napatech.com>

---
v8:
* Merged two defines into one handling max wait time

v7:
* Code style changes

v6:
* Fixed code style issues

v5:
* Reinitialization of "i" inconsistent with original intent

v4:
* Code style fix

v3:
* Replaced commit message

v2:
*  Code style fix

v1:
* Initial version
---
---
 drivers/net/i40e/i40e_fdir.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 28cc554f5..f94e1c3b8 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -73,9 +73,8 @@
 #define I40E_FDIR_IPv6_PAYLOAD_LEN          380
 #define I40E_FDIR_UDP_DEFAULT_LEN           400
 
-/* Wait count and interval for fdir filter programming */
-#define I40E_FDIR_WAIT_COUNT       10
-#define I40E_FDIR_WAIT_INTERVAL_US 1000
+/* Wait time for fdir filter programming */
+#define I40E_FDIR_MAX_WAIT_US 10000
 
 /* Wait count and interval for fdir filter flush */
 #define I40E_FDIR_FLUSH_RETRY       50
@@ -1295,28 +1294,27 @@ i40e_fdir_filter_programming(struct i40e_pf *pf,
 	/* Update the tx tail register */
 	rte_wmb();
 	I40E_PCI_REG_WRITE(txq->qtx_tail, txq->tx_tail);
-
-	for (i = 0; i < I40E_FDIR_WAIT_COUNT; i++) {
-		rte_delay_us(I40E_FDIR_WAIT_INTERVAL_US);
+	for (i = 0; i < I40E_FDIR_MAX_WAIT_US; i++) {
 		if ((txdp->cmd_type_offset_bsz &
 				rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) ==
 				rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))
 			break;
+		rte_delay_us(1);
 	}
-	if (i >= I40E_FDIR_WAIT_COUNT) {
+	if (i >= I40E_FDIR_MAX_WAIT_US) {
 		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
 			    " time out to get DD on tx queue.");
 		return -ETIMEDOUT;
 	}
 	/* totally delay 10 ms to check programming status*/
-	rte_delay_us((I40E_FDIR_WAIT_COUNT - i) * I40E_FDIR_WAIT_INTERVAL_US);
-	if (i40e_check_fdir_programming_status(rxq) < 0) {
-		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
-			    " programming status reported.");
-		return -ENOSYS;
+	for (; i < I40E_FDIR_MAX_WAIT_US; i++) {
+		if (i40e_check_fdir_programming_status(rxq) >= 0)
+			return 0;
+		rte_delay_us(1);
 	}
-
-	return 0;
+	PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
+		" programming status reported.");
+	return -ETIMEDOUT;
 }
 
 /*
-- 
2.12.2

Re: [dpdk-dev] [PATCH v8] net/i40e: improved FDIR programming times

[Ferruh Yigit]

On 5/17/2017 3:57 PM, Michael Lilja wrote:
> Previously, the FDIR programming time is +11ms on i40e.
> This patch will result in an average programming time of
> 22usec with a max of 60usec .
> 
> Signed-off-by: Michael Lilja <ml@napatech.com>

<...>

>  	/* totally delay 10 ms to check programming status*/
> -	rte_delay_us((I40E_FDIR_WAIT_COUNT - i) * I40E_FDIR_WAIT_INTERVAL_US);
> -	if (i40e_check_fdir_programming_status(rxq) < 0) {
> -		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
> -			    " programming status reported.");
> -		return -ENOSYS;
> +	for (; i < I40E_FDIR_MAX_WAIT_US; i++) {
> +		if (i40e_check_fdir_programming_status(rxq) >= 0)
> +			return 0;
> +		rte_delay_us(1);
>  	}
> -
> -	return 0;
> +	PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
> +		" programming status reported."
I am aware that you just moved this log, but since you have touched to
it, can you please fix it too [1]:

PMD_DRV_LOG(ERR,
	"Failed to program FDIR filter: programming status reported.");

[1] Or if you prefer please let me know, so I can fix it while applying.

> +	return -ETIMEDOUT;
>  }
>  
>  /*
> 

Re: [dpdk-dev] [PATCH v8] net/i40e: improved FDIR programming times

[Michael Lilja]

Please fix while applying.

-----Original Message-----
From: Ferruh Yigit [mailto:ferruh.yigit@intel.com] 
Sent: 17 May 2017 17:17
To: Michael Lilja <ml@napatech.com>; helin.zhang@intel.com; jingjing.wu@intel.com
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v8] net/i40e: improved FDIR programming times

On 5/17/2017 3:57 PM, Michael Lilja wrote:
> Previously, the FDIR programming time is +11ms on i40e.
> This patch will result in an average programming time of 22usec with a 
> max of 60usec .
> 
> Signed-off-by: Michael Lilja <ml@napatech.com>

<...>

>  	/* totally delay 10 ms to check programming status*/
> -	rte_delay_us((I40E_FDIR_WAIT_COUNT - i) * I40E_FDIR_WAIT_INTERVAL_US);
> -	if (i40e_check_fdir_programming_status(rxq) < 0) {
> -		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
> -			    " programming status reported.");
> -		return -ENOSYS;
> +	for (; i < I40E_FDIR_MAX_WAIT_US; i++) {
> +		if (i40e_check_fdir_programming_status(rxq) >= 0)
> +			return 0;
> +		rte_delay_us(1);
>  	}
> -
> -	return 0;
> +	PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
> +		" programming status reported."
I am aware that you just moved this log, but since you have touched to it, can you please fix it too [1]:

PMD_DRV_LOG(ERR,
	"Failed to program FDIR filter: programming status reported.");

[1] Or if you prefer please let me know, so I can fix it while applying.

> +	return -ETIMEDOUT;
>  }
>  
>  /*
> 

Re: [dpdk-dev] [PATCH v8] net/i40e: improved FDIR programming times

[Xing, Beilei]

Hi,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Michael Lilja
> Sent: Wednesday, May 17, 2017 10:58 PM
> To: Zhang, Helin <helin.zhang@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; Michael Lilja <ml@napatech.com>
> Subject: [dpdk-dev] [PATCH v8] net/i40e: improved FDIR programming times
> 
> Previously, the FDIR programming time is +11ms on i40e.
> This patch will result in an average programming time of 22usec with a max of
> 60usec .
> 
> Signed-off-by: Michael Lilja <ml@napatech.com>
> 
> ---
> v8:
> * Merged two defines into one handling max wait time
> 
> v7:
> * Code style changes
> 
> v6:
> * Fixed code style issues
> 
> v5:
> * Reinitialization of "i" inconsistent with original intent
> 
> v4:
> * Code style fix
> 
> v3:
> * Replaced commit message
> 
> v2:
> *  Code style fix
> 
> v1:
> * Initial version
> ---
> ---
>  drivers/net/i40e/i40e_fdir.c | 26 ++++++++++++--------------
>  1 file changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c index
> 28cc554f5..f94e1c3b8 100644
> --- a/drivers/net/i40e/i40e_fdir.c
> +++ b/drivers/net/i40e/i40e_fdir.c
> @@ -73,9 +73,8 @@
>  #define I40E_FDIR_IPv6_PAYLOAD_LEN          380
>  #define I40E_FDIR_UDP_DEFAULT_LEN           400
> 
> -/* Wait count and interval for fdir filter programming */
> -#define I40E_FDIR_WAIT_COUNT       10
> -#define I40E_FDIR_WAIT_INTERVAL_US 1000
> +/* Wait time for fdir filter programming */ #define
> +I40E_FDIR_MAX_WAIT_US 10000
> 
>  /* Wait count and interval for fdir filter flush */
>  #define I40E_FDIR_FLUSH_RETRY       50
> @@ -1295,28 +1294,27 @@ i40e_fdir_filter_programming(struct i40e_pf *pf,
>  	/* Update the tx tail register */
>  	rte_wmb();
>  	I40E_PCI_REG_WRITE(txq->qtx_tail, txq->tx_tail);
> -
> -	for (i = 0; i < I40E_FDIR_WAIT_COUNT; i++) {
> -		rte_delay_us(I40E_FDIR_WAIT_INTERVAL_US);
> +	for (i = 0; i < I40E_FDIR_MAX_WAIT_US; i++) {
>  		if ((txdp->cmd_type_offset_bsz &
> 
> 	rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) ==
> 
> 	rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))
>  			break;
> +		rte_delay_us(1);
>  	}
> -	if (i >= I40E_FDIR_WAIT_COUNT) {
> +	if (i >= I40E_FDIR_MAX_WAIT_US) {
>  		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
>  			    " time out to get DD on tx queue.");
>  		return -ETIMEDOUT;
>  	}
>  	/* totally delay 10 ms to check programming status*/
> -	rte_delay_us((I40E_FDIR_WAIT_COUNT - i) *
> I40E_FDIR_WAIT_INTERVAL_US);
> -	if (i40e_check_fdir_programming_status(rxq) < 0) {
> -		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
> -			    " programming status reported.");
> -		return -ENOSYS;
> +	for (; i < I40E_FDIR_MAX_WAIT_US; i++) {
> +		if (i40e_check_fdir_programming_status(rxq) >= 0)
> +			return 0;
> +		rte_delay_us(1);
>  	}
> -
> -	return 0;
> +	PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
> +		" programming status reported.");
> +	return -ETIMEDOUT;
>  }
> 
>  /*
> --
> 2.12.2

Acked-by: Beilei Xing <beilei.xing@intel.com>

Re: [dpdk-dev] [PATCH v8] net/i40e: improved FDIR programming times

[Ferruh Yigit]

On 5/18/2017 2:38 AM, Xing, Beilei wrote:
> Hi,
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Michael Lilja
>> Sent: Wednesday, May 17, 2017 10:58 PM
>> To: Zhang, Helin <helin.zhang@intel.com>; Wu, Jingjing
>> <jingjing.wu@intel.com>
>> Cc: dev@dpdk.org; Michael Lilja <ml@napatech.com>
>> Subject: [dpdk-dev] [PATCH v8] net/i40e: improved FDIR programming times
>>
>> Previously, the FDIR programming time is +11ms on i40e.
>> This patch will result in an average programming time of 22usec with a max of
>> 60usec .
>>
>> Signed-off-by: Michael Lilja <ml@napatech.com>

> Acked-by: Beilei Xing <beilei.xing@intel.com>

Applied to dpdk-next-net/master, thanks.