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

Message ID 20170516220132.3813-1-ml@napatech.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation fail apply patch file failure

Commit Message

Michael Lilja May 16, 2017, 10:01 p.m. UTC
  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(-)

--
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.
  

Comments

Xing, Beilei May 17, 2017, 2:22 a.m. UTC | #1
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.
  
Ferruh Yigit May 17, 2017, 8:44 a.m. UTC | #2
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>
<...>
  

Patch

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;
        }