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.yi...@intel.com] Sent: 17 May 2017 16:44 To: Michael Lilja <m...@napatech.com>; helin.zh...@intel.com; jingjing...@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 <m...@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; > } > > /* >
