[dpdk-dev,v2,4/4] testpmd: make use of per-PMD TxRx parameters

  The optimal values of several transmission & reception related
parameters, such as burst sizes, descriptor ring sizes, and number
of queues, varies between different network interface devices. This
patch allows testpmd to make use of per-PMD tuned parameter values.

Signed-off-by: Remy Horton <remy.horton@intel.com>
---
 app/test-pmd/testpmd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
  

Hello Remy,

> -----Original Message-----
> From: Remy Horton [mailto:remy.horton@intel.com]
> Sent: Wednesday, March 21, 2018 7:58 PM
> To: dev@dpdk.org
> Cc: John McNamara <john.mcnamara@intel.com>; Wenzhuo Lu
> <wenzhuo.lu@intel.com>; Jingjing Wu <jingjing.wu@intel.com>; Qi Zhang
> <qi.z.zhang@intel.com>; Beilei Xing <beilei.xing@intel.com>; Shreyansh
> Jain <shreyansh.jain@nxp.com>; Thomas Monjalon <thomas@monjalon.net>
> Subject: [PATCH v2 4/4] testpmd: make use of per-PMD TxRx parameters
> 
> The optimal values of several transmission & reception related
> parameters, such as burst sizes, descriptor ring sizes, and number
> of queues, varies between different network interface devices. This
> patch allows testpmd to make use of per-PMD tuned parameter values.
> 
> Signed-off-by: Remy Horton <remy.horton@intel.com>
> ---
>  app/test-pmd/testpmd.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 4c0e258..82eb197 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -210,9 +210,10 @@ queueid_t nb_txq = 1; /**< Number of TX queues per
> port. */
> 
>  /*
>   * Configurable number of RX/TX ring descriptors.
> + * Defaults are supplied by drivers via ethdev.
>   */
> -#define RTE_TEST_RX_DESC_DEFAULT 1024
> -#define RTE_TEST_TX_DESC_DEFAULT 1024
> +#define RTE_TEST_RX_DESC_DEFAULT 0
> +#define RTE_TEST_TX_DESC_DEFAULT 0
>  uint16_t nb_rxd = RTE_TEST_RX_DESC_DEFAULT; /**< Number of RX
> descriptors. */
>  uint16_t nb_txd = RTE_TEST_TX_DESC_DEFAULT; /**< Number of TX
> descriptors. */

Can the change for burst size too be accommodated in this patch?
I looked through them and they might not be as trivial as the change above - but if that is incorporated in this, it might serve as example for other applications.

Or, I am also OK sending a separate patch for that change (maybe for iofwd case, at least)

Either way:

Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
  

21/03/2018 15:27, Remy Horton:
>  /*
>   * Configurable number of RX/TX ring descriptors.

Configurable, really?

> + * Defaults are supplied by drivers via ethdev.

And fallback values are in ethdev.

>   */
> -#define RTE_TEST_RX_DESC_DEFAULT 1024
> -#define RTE_TEST_TX_DESC_DEFAULT 1024
> +#define RTE_TEST_RX_DESC_DEFAULT 0
> +#define RTE_TEST_TX_DESC_DEFAULT 0

We do not need a define for 0.
Better to rework a bit above and below comments.

>  uint16_t nb_rxd = RTE_TEST_RX_DESC_DEFAULT; /**< Number of RX descriptors. */
>  uint16_t nb_txd = RTE_TEST_TX_DESC_DEFAULT; /**< Number of TX descriptors. */

These doxygen comments in the middle of the code are totally useless.
  

On 31/03/2018 01:01, Thomas Monjalon wrote:
[..]
>>  uint16_t nb_rxd = RTE_TEST_RX_DESC_DEFAULT; /**< Number of RX descriptors. */
>>  uint16_t nb_txd = RTE_TEST_TX_DESC_DEFAULT; /**< Number of TX descriptors. */
>
> These doxygen comments in the middle of the code are totally useless.

Did wonder why they were there. However these lines are existing code, 
and since testpmd.c uses Doxygen tags extensively, removing them in my 
view ought to be done via a separate clean-up patch.
  

On 28/03/2018 08:18, Shreyansh Jain wrote:
[..]
> Can the change for burst size too be accommodated in this patch? I
> looked through them and they might not be as trivial as the change
> above - but if that is incorporated in this, it might serve as
> example for other applications.

Testpmd doesn't really distinguish between Rx and TX burst sizes, having 
just a single user-specifiable nb_pkt_per_burst parameter. However 
filling it with the preferred Rx burst size seems a good balance between 
providing a usage example and amount of code changes required.