From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shreyansh Jain Subject: Re: [PATCH v5 4/4] testpmd: make use of per-PMD TxRx parameters Date: Mon, 9 Apr 2018 18:25:15 +0530 Message-ID: References: <20180406145002.27480-1-remy.horton@intel.com> <20180406145002.27480-5-remy.horton@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: John McNamara , Wenzhuo Lu , Jingjing Wu , Qi Zhang , Beilei Xing , Thomas Monjalon To: Remy Horton , dev@dpdk.org Return-path: Received: from EUR01-VE1-obe.outbound.protection.outlook.com (mail-ve1eur01on0054.outbound.protection.outlook.com [104.47.1.54]) by dpdk.org (Postfix) with ESMTP id BEADD1B7F5 for ; Mon, 9 Apr 2018 14:40:18 +0200 (CEST) In-Reply-To: <20180406145002.27480-5-remy.horton@intel.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > --- > 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. */ > >