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

[Shreyansh Jain]

On Friday 06 April 2018 08:20 PM, Remy Horton wrote:
> 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 at intel.com>
> ---
>   app/test-pmd/cmdline.c    | 31 ++++++++++++++++++++++++++++---
>   app/test-pmd/parameters.c | 38 +++++++++++++++++++++++++++++++++-----
>   app/test-pmd/testpmd.c    |  5 +++--
>   3 files changed, 64 insertions(+), 10 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 40b31ad..0914425 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -2599,6 +2599,8 @@ cmd_config_burst_parsed(void *parsed_result,
>   			__attribute__((unused)) void *data)
>   {
>   	struct cmd_config_burst *res = parsed_result;
> +	struct rte_eth_dev_info dev_info;
> +	uint16_t rec_nb_pkts;
>   
>   	if (!all_ports_stopped()) {
>   		printf("Please stop all ports first\n");
> @@ -2606,11 +2608,34 @@ cmd_config_burst_parsed(void *parsed_result,
>   	}
>   
>   	if (!strcmp(res->name, "burst")) {
> -		if (res->value < 1 || res->value > MAX_PKT_BURST) {
> +		if (res->value == 0) {

Documentation for burst mode changes to testpmd would need an update.
I guess, only when the user explicitly sets 'set burst 0' would the 
driver defaults be picked up - isn't it?

Maybe something like this:

--->8---
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -372,7 +372,9 @@ The commandline options are:
  *   ``--burst=N``

      Set the number of packets per burst to N, where 1 <= N <= 512.
-    The default value is 16.
+    The default value is 32.
+    If set to 0, driver default is used if defined. Else, if driver default
+    is not defined, default of 32 is used.

  *   ``--mbcache=N``
--->8---

In the above, I think the existing documented default value needs to be 
changed. It is set to '#define DEF_PKT_BURST 32'

If you add that, please use my ack for next revision.
(For patch 1/4, I had already given my Ack in v2)

> +			/* If user gives a value of zero, query the PMD for
> +			 * its recommended Rx burst size. Testpmd uses a single
> +			 * size for all ports, so assume all ports are the same
> +			 * NIC model and use the values from Port 0.
> +			 */
> +			rte_eth_dev_info_get(0, &dev_info);
> +			rec_nb_pkts = dev_info.default_rxportconf.burst_size;
> +
> +			if (rec_nb_pkts == 0) {
> +				printf("PMD does not recommend a burst size.\n"
> +					"User provided value must be between"
> +					" 1 and %d\n", MAX_PKT_BURST);
> +				return;
> +			} else if (rec_nb_pkts > MAX_PKT_BURST) {
> +				printf("PMD recommended burst size of %d"
> +					" exceeds maximum value of %d\n",
> +					rec_nb_pkts, MAX_PKT_BURST);
> +				return;
> +			}
> +			printf("Using PMD-provided burst value of %d\n",
> +				rec_nb_pkts);
> +			nb_pkt_per_burst = rec_nb_pkts;
> +		} else if (res->value > MAX_PKT_BURST) {
>   			printf("burst must be >= 1 && <= %d\n", MAX_PKT_BURST);
>   			return;
> -		}
> -		nb_pkt_per_burst = res->value;
> +		} else
> +			nb_pkt_per_burst = res->value;
>   	} else {
>   		printf("Unknown parameter\n");
>   		return;
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index 2192bdc..cb6a229 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -544,6 +544,8 @@ launch_args_parse(int argc, char** argv)
>   	/* Default offloads for all ports. */
>   	uint64_t rx_offloads = rx_mode.offloads;
>   	uint64_t tx_offloads = tx_mode.offloads;
> +	struct rte_eth_dev_info dev_info;
> +	uint16_t rec_nb_pkts;
>   
>   	static struct option lgopts[] = {
>   		{ "help",			0, 0, 0 },
> @@ -947,12 +949,38 @@ launch_args_parse(int argc, char** argv)
>   			}
>   			if (!strcmp(lgopts[opt_idx].name, "burst")) {
>   				n = atoi(optarg);
> -				if ((n >= 1) && (n <= MAX_PKT_BURST))
> -					nb_pkt_per_burst = (uint16_t) n;
> -				else
> +				if (n == 0) {
> +					/* A burst size of zero means that the
> +					 * PMD should be queried for
> +					 * recommended Rx burst size. Since
> +					 * testpmd uses a single size for all
> +					 * ports, port 0 is queried for the
> +					 * value, on the assumption that all
> +					 * ports are of the same NIC model.
> +					 */
> +					rte_eth_dev_info_get(0, &dev_info);
> +					rec_nb_pkts = dev_info
> +						.default_rxportconf.burst_size;
> +
> +					if (rec_nb_pkts == 0)
> +						rte_exit(EXIT_FAILURE,
> +							"PMD does not recommend a burst size. "
> +							"Provided value must be between "
> +							"1 and %d\n", MAX_PKT_BURST);
> +					else if (rec_nb_pkts > MAX_PKT_BURST)
> +						rte_exit(EXIT_FAILURE,
> +							"PMD recommended burst size of %d"
> +							" exceeds maximum value of %d\n",
> +							rec_nb_pkts, MAX_PKT_BURST);
> +					printf("Using PMD-provided burst value of %d\n",
> +						rec_nb_pkts);
> +					nb_pkt_per_burst = rec_nb_pkts;
> +				} else if (n > MAX_PKT_BURST)
>   					rte_exit(EXIT_FAILURE,
> -						 "burst must >= 1 and <= %d]",
> -						 MAX_PKT_BURST);
> +						"burst must be between1 and %d\n",
> +						MAX_PKT_BURST);
> +				else
> +					nb_pkt_per_burst = (uint16_t) n;
>   			}
>   			if (!strcmp(lgopts[opt_idx].name, "mbcache")) {
>   				n = atoi(optarg);
> 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. */
>   
>