[dpdk-dev,v2] drivers/net/i40e/i40e_fdir.c: Improved i40e FDIR programming times

  During my work (https://www.napatech.com/hw-acceleration-via-rte_flow/)
on a flowtable application example that use rte_flow I discovered
that the rte_flow programming times on a i40e was +11ms. The patch
below result in an average programming time of 22usec with a max of
60usec instead of +11ms.

Could the following patch be useful? There might be a good reason
for the original code, I'm unable to tell, so I will let it up
to the maintainer to decide.

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

---
v2:
* 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.
  

Hi Lijia,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Michael Lilja
> Sent: Thursday, May 11, 2017 6:21 PM
> To: dev@dpdk.org
> Cc: Michael Lilja <ml@napatech.com>
> Subject: [dpdk-dev] [PATCH v2] drivers/net/i40e/i40e_fdir.c: Improved i40e
> FDIR programming times
> 
> During my work (https://www.napatech.com/hw-acceleration-via-rte_flow/)
> on a flowtable application example that use rte_flow I discovered that the
> rte_flow programming times on a i40e was +11ms. The patch below result in
> an average programming time of 22usec with a max of 60usec instead of
> +11ms.
> 
> Could the following patch be useful? There might be a good reason for the
> original code, I'm unable to tell, so I will let it up to the maintainer to decide.

Thanks for the patch, it's useful, and this can be removed from the commit log.

> 
> Signed-off-by: Michael Lilja <ml@napatech.com>
> 
> ---
> v2:
> * 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++) {
> +               i
f (i40e_check_fdir_programming_status(rxq) >= 0) {

Braces {} can be removed here according to the coding style.

> +                       break;
 
How about "return 0;" here?

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

How about removing the if statement? as i40e_check_fdir_programming_status(rxq) has been executed in the above for loop.

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