[dpdk-dev,v2] examples/l3fwd: pass flow arguments when start app

Message ID 1506736748-50515-1-git-send-email-xiaoyun.li@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Li, Xiaoyun Sept. 30, 2017, 1:59 a.m. UTC
  To make the performance can be tuning on different NICs or platforms. We
need to make the number of descriptors and Rx/TX threshold as arguments
when starting l3fwd application.

Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
---
v2
* Modify commit log.
* Update the change in guides doc.

 doc/guides/sample_app_ug/l3_forward.rst |  15 +++
 examples/l3fwd/main.c                   | 191 +++++++++++++++++++++++++++++++-
 2 files changed, 204 insertions(+), 2 deletions(-)
  

Comments

Stephen Hemminger Oct. 1, 2017, 5:24 p.m. UTC | #1
On Sat, 30 Sep 2017 09:59:08 +0800
Xiaoyun Li <xiaoyun.li@intel.com> wrote:

> To make the performance can be tuning on different NICs or platforms. We
> need to make the number of descriptors and Rx/TX threshold as arguments
> when starting l3fwd application.
> 
> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>

Not sure about this. The point of l3fwd is to make it as simple
an application as possible to help users.

Given that drivers can now supply default values for thresholds, I think
the l3fwd sample should get rid of all the special descriptor values it
is setting. Then if the values are not right for best performance that should
be pushed back to the driver writer to fix.
  
Jingjing Wu Oct. 5, 2017, 8:35 a.m. UTC | #2
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Monday, October 2, 2017 1:24 AM
> To: Li, Xiaoyun <xiaoyun.li@intel.com>
> Cc: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] examples/l3fwd: pass flow arguments when start app
> 
> On Sat, 30 Sep 2017 09:59:08 +0800
> Xiaoyun Li <xiaoyun.li@intel.com> wrote:
> 
> > To make the performance can be tuning on different NICs or platforms. We
> > need to make the number of descriptors and Rx/TX threshold as arguments
> > when starting l3fwd application.
> >
> > Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> 
> Not sure about this. The point of l3fwd is to make it as simple
> an application as possible to help users.
> 
> Given that drivers can now supply default values for thresholds, I think
> the l3fwd sample should get rid of all the special descriptor values it
> is setting. Then if the values are not right for best performance that should
> be pushed back to the driver writer to fix.

But now what the driver using are the arguments passed from l3fwd application,
such as RTE_TEST_RX_DESC_DEFAULT. About the threshold, I guess it is already
done by driver to use default value. For number of descriptors, any ideas? Diver to
provide a suggestion one?
  
Jingjing Wu Oct. 10, 2017, 7:57 a.m. UTC | #3
> -----Original Message-----
> From: Wu, Jingjing
> Sent: Thursday, October 5, 2017 4:36 PM
> To: Stephen Hemminger <stephen@networkplumber.org>; Li, Xiaoyun
> <xiaoyun.li@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2] examples/l3fwd: pass flow arguments when
> start app
> 
> 
> 
> > -----Original Message-----
> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > Sent: Monday, October 2, 2017 1:24 AM
> > To: Li, Xiaoyun <xiaoyun.li@intel.com>
> > Cc: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v2] examples/l3fwd: pass flow arguments
> > when start app
> >
> > On Sat, 30 Sep 2017 09:59:08 +0800
> > Xiaoyun Li <xiaoyun.li@intel.com> wrote:
> >
> > > To make the performance can be tuning on different NICs or
> > > platforms. We need to make the number of descriptors and Rx/TX
> > > threshold as arguments when starting l3fwd application.
> > >
> > > Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> >
> > Not sure about this. The point of l3fwd is to make it as simple an
> > application as possible to help users.
> >
> > Given that drivers can now supply default values for thresholds, I
> > think the l3fwd sample should get rid of all the special descriptor
> > values it is setting. Then if the values are not right for best
> > performance that should be pushed back to the driver writer to fix.
> 
> But now what the driver using are the arguments passed from l3fwd
> application, such as RTE_TEST_RX_DESC_DEFAULT. About the threshold, I guess
> it is already done by driver to use default value. For number of descriptors, any
> ideas? Diver to provide a suggestion one?
> 

And additionally, even driver provides a suggestion value for descriptors or threshold,
on different platform, the performance result also varies. So I think xiaoyun's change to
pass those are arguments makes sense.


Thanks
Jingjing
  

Patch

diff --git a/doc/guides/sample_app_ug/l3_forward.rst b/doc/guides/sample_app_ug/l3_forward.rst
index 6a6b8fb..668b7d0 100644
--- a/doc/guides/sample_app_ug/l3_forward.rst
+++ b/doc/guides/sample_app_ug/l3_forward.rst
@@ -101,6 +101,11 @@  The application has a number of command line options::
                              [--hash-entry-num]
                              [--ipv6]
                              [--parse-ptype]
+                             [--nb-rxd]
+                             [--nb-txd]
+                             [--rx-free-thresh]
+                             [--tx-free-thresh]
+                             [--tx-rs-thresh]
 
 Where,
 
@@ -129,6 +134,16 @@  Where,
 
 * ``--parse-ptype:`` Optional, set to use software to analyze packet type. Without this option, hardware will check the packet type.
 
+* ``--nb-rxd:`` Optional, specifies the number of Rx queue discriptors in decimal.
+
+* ``--nb-txd:`` Optional, specifies the number of Tx queue discriptors in decimal.
+
+* ``--rx-free-thresh:`` Optional, specifies the number of Rx free threshold in decimal.
+
+* ``--tx-free-thresh:`` Optional, specifies the number of Tx free threshold in decimal.
+
+* ``--tx-rs-thresh:`` Optional, specifies the number of Tx RS bit threshold in decimal.
+
 For example, consider a dual processor socket platform with 8 physical cores, where cores 0-7 and 16-23 appear on socket 0,
 while cores 8-15 and 24-31 appear on socket 1.
 
diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
index 81995fd..bafac29 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -83,6 +83,7 @@ 
  */
 #define RTE_TEST_RX_DESC_DEFAULT 128
 #define RTE_TEST_TX_DESC_DEFAULT 512
+#define RTE_PMD_PARAM_UNSET -1
 
 #define MAX_TX_QUEUE_PER_PORT RTE_MAX_ETHPORTS
 #define MAX_RX_QUEUE_PER_PORT 128
@@ -92,6 +93,9 @@ 
 /* Static global variables used within this file. */
 static uint16_t nb_rxd = RTE_TEST_RX_DESC_DEFAULT;
 static uint16_t nb_txd = RTE_TEST_TX_DESC_DEFAULT;
+static int16_t rx_free_thresh = RTE_PMD_PARAM_UNSET;
+static int16_t tx_free_thresh = RTE_PMD_PARAM_UNSET;
+static int16_t tx_rs_thresh = RTE_PMD_PARAM_UNSET;
 
 /**< Ports set in promiscuous mode off by default. */
 static int promiscuous_on;
@@ -320,7 +324,12 @@  print_usage(const char *prgname)
 		" [--no-numa]"
 		" [--hash-entry-num]"
 		" [--ipv6]"
-		" [--parse-ptype]\n\n"
+		" [--parse-ptype]"
+		" [--nb-rxd]"
+		" [--nb-txd]"
+		" [--rx-free-thresh]"
+		" [--tx-free-thresh]"
+		" [--tx-rs-thresh]\n\n"
 
 		"  -p PORTMASK: Hexadecimal bitmask of ports to configure\n"
 		"  -P : Enable promiscuous mode\n"
@@ -334,7 +343,12 @@  print_usage(const char *prgname)
 		"  --no-numa: Disable numa awareness\n"
 		"  --hash-entry-num: Specify the hash entry number in hexadecimal to be setup\n"
 		"  --ipv6: Set if running ipv6 packets\n"
-		"  --parse-ptype: Set to use software to analyze packet type\n\n",
+		"  --parse-ptype: Set to use software to analyze packet type\n\n"
+		"  --nb-rxd: Set number of descriptors of Rx queue\n"
+		"  --nb-txd: Set number of descriptors of Tx queue\n"
+		"  --rx-free-thresh: Set value of Rx free threshold\n"
+		"  --tx-free-thresh: Set value of Tx free threshold\n"
+		"  --tx-rs-thresh: Set value of Tx RS bit threshold\n\n",
 		prgname);
 }
 
@@ -389,6 +403,91 @@  parse_hash_entry_number(const char *hash_entry_num)
 }
 
 static int
+parse_nb_rxd(const char *nb_rxd_c)
+{
+	char *end = NULL;
+	unsigned int nb_rxd_t;
+
+	/* parse hexadecimal string */
+	nb_rxd_t = strtoul(nb_rxd_c, &end, 10);
+	if ((nb_rxd_c[0] == '\0') || (end == NULL) || (*end != '\0'))
+		return -1;
+
+	if (nb_rxd_t == 0)
+		return -1;
+
+	return nb_rxd_t;
+}
+
+static int
+parse_nb_txd(const char *nb_txd_c)
+{
+	char *end = NULL;
+	unsigned int nb_txd_t;
+
+	/* parse hexadecimal string */
+	nb_txd_t = strtoul(nb_txd_c, &end, 10);
+	if ((nb_txd_c[0] == '\0') || (end == NULL) || (*end != '\0'))
+		return -1;
+
+	if (nb_txd_t == 0)
+		return -1;
+
+	return nb_txd_t;
+}
+
+static int
+parse_rx_free_thresh(const char *rx_free_thresh_c)
+{
+	char *end = NULL;
+	unsigned int rx_free_thresh_t;
+
+	/* parse hexadecimal string */
+	rx_free_thresh_t = strtoul(rx_free_thresh_c, &end, 10);
+	if ((rx_free_thresh_c[0] == '\0') || (end == NULL) || (*end != '\0'))
+		return -1;
+
+	if (rx_free_thresh_t == 0)
+		return -1;
+
+	return rx_free_thresh_t;
+}
+
+static int
+parse_tx_free_thresh(const char *tx_free_thresh_c)
+{
+	char *end = NULL;
+	unsigned int tx_free_thresh_t;
+
+	/* parse hexadecimal string */
+	tx_free_thresh_t = strtoul(tx_free_thresh_c, &end, 10);
+	if ((tx_free_thresh_c[0] == '\0') || (end == NULL) || (*end != '\0'))
+		return -1;
+
+	if (tx_free_thresh_t == 0)
+		return -1;
+
+	return tx_free_thresh_t;
+}
+
+static int
+parse_tx_rs_thresh(const char *tx_rs_thresh_c)
+{
+	char *end = NULL;
+	unsigned int tx_rs_thresh_t;
+
+	/* parse hexadecimal string */
+	tx_rs_thresh_t = strtoul(tx_rs_thresh_c, &end, 10);
+	if ((tx_rs_thresh_c[0] == '\0') || (end == NULL) || (*end != '\0'))
+		return -1;
+
+	if (tx_rs_thresh_t == 0)
+		return -1;
+
+	return tx_rs_thresh_t;
+}
+
+static int
 parse_config(const char *q_arg)
 {
 	char s[256];
@@ -487,6 +586,11 @@  static const char short_options[] =
 #define CMD_LINE_OPT_ENABLE_JUMBO "enable-jumbo"
 #define CMD_LINE_OPT_HASH_ENTRY_NUM "hash-entry-num"
 #define CMD_LINE_OPT_PARSE_PTYPE "parse-ptype"
+#define CMD_LINE_OPT_NB_RXD "nb-rxd"
+#define CMD_LINE_OPT_NB_TXD "nb-txd"
+#define CMD_LINE_OPT_RX_FREE_THRESH "rx-free-thresh"
+#define CMD_LINE_OPT_TX_FREE_THRESH "tx-free-thresh"
+#define CMD_LINE_OPT_TX_RS_THRESH "tx-rs-thresh"
 enum {
 	/* long options mapped to a short option */
 
@@ -500,6 +604,11 @@  enum {
 	CMD_LINE_OPT_ENABLE_JUMBO_NUM,
 	CMD_LINE_OPT_HASH_ENTRY_NUM_NUM,
 	CMD_LINE_OPT_PARSE_PTYPE_NUM,
+	CMD_LINE_OPT_NB_RXD_NUM,
+	CMD_LINE_OPT_NB_TXD_NUM,
+	CMD_LINE_OPT_RX_FREE_THRESH_NUM,
+	CMD_LINE_OPT_TX_FREE_THRESH_NUM,
+	CMD_LINE_OPT_TX_RS_THRESH_NUM,
 };
 
 static const struct option lgopts[] = {
@@ -510,6 +619,11 @@  static const struct option lgopts[] = {
 	{CMD_LINE_OPT_ENABLE_JUMBO, 0, 0, CMD_LINE_OPT_ENABLE_JUMBO_NUM},
 	{CMD_LINE_OPT_HASH_ENTRY_NUM, 1, 0, CMD_LINE_OPT_HASH_ENTRY_NUM_NUM},
 	{CMD_LINE_OPT_PARSE_PTYPE, 0, 0, CMD_LINE_OPT_PARSE_PTYPE_NUM},
+	{CMD_LINE_OPT_NB_RXD, 1, 0, CMD_LINE_OPT_NB_RXD_NUM},
+	{CMD_LINE_OPT_NB_TXD, 1, 0, CMD_LINE_OPT_NB_TXD_NUM},
+	{CMD_LINE_OPT_RX_FREE_THRESH, 1, 0, CMD_LINE_OPT_RX_FREE_THRESH_NUM},
+	{CMD_LINE_OPT_TX_FREE_THRESH, 1, 0, CMD_LINE_OPT_TX_FREE_THRESH_NUM},
+	{CMD_LINE_OPT_TX_RS_THRESH, 1, 0, CMD_LINE_OPT_TX_RS_THRESH_NUM},
 	{NULL, 0, 0, 0}
 };
 
@@ -554,6 +668,11 @@  parse_args(int argc, char **argv)
 	const char *str12 =
 		"L3FWD: LPM and EM are mutually exclusive, select only one";
 	const char *str13 = "L3FWD: LPM or EM none selected, default LPM on";
+	const char *str14 = "L3FWD: Invalid Rx descriptors number";
+	const char *str15 = "L3FWD: Invalid Tx descriptors number";
+	const char *str16 = "L3FWD: Invalid Rx free threshold value";
+	const char *str17 = "L3FWD: Invalid Tx free threshold value";
+	const char *str18 = "L3FWD: Invalid Tx RS bit threshold value";
 
 	while ((opt = getopt_long(argc, argvopt, short_options,
 				lgopts, &option_index)) != EOF) {
@@ -652,6 +771,61 @@  parse_args(int argc, char **argv)
 			parse_ptype = 1;
 			break;
 
+		case CMD_LINE_OPT_NB_RXD_NUM:
+			ret = parse_nb_rxd(optarg);
+			if (ret > 0)
+				nb_rxd = ret;
+			else{
+				printf("%s\n", str14);
+				print_usage(prgname);
+				return -1;
+			}
+			break;
+
+		case CMD_LINE_OPT_NB_TXD_NUM:
+			ret = parse_nb_txd(optarg);
+			if (ret > 0)
+				nb_txd = ret;
+			else{
+				printf("%s\n", str15);
+				print_usage(prgname);
+				return -1;
+			}
+			break;
+
+		case CMD_LINE_OPT_RX_FREE_THRESH_NUM:
+			ret = parse_rx_free_thresh(optarg);
+			if (ret > 0)
+				rx_free_thresh = ret;
+			else{
+				printf("%s\n", str16);
+				print_usage(prgname);
+				return -1;
+			}
+			break;
+
+		case CMD_LINE_OPT_TX_FREE_THRESH_NUM:
+			ret = parse_tx_free_thresh(optarg);
+			if (ret > 0)
+				tx_free_thresh = ret;
+			else{
+				printf("%s\n", str17);
+				print_usage(prgname);
+				return -1;
+			}
+			break;
+
+		case CMD_LINE_OPT_TX_RS_THRESH_NUM:
+			ret = parse_tx_rs_thresh(optarg);
+			if (ret > 0)
+				tx_rs_thresh = ret;
+			else{
+				printf("%s\n", str18);
+				print_usage(prgname);
+				return -1;
+			}
+			break;
+
 		default:
 			print_usage(prgname);
 			return -1;
@@ -894,6 +1068,19 @@  main(int argc, char **argv)
 
 	/* initialize all ports */
 	for (portid = 0; portid < nb_ports; portid++) {
+		rte_eth_dev_info_get(portid, &dev_info);
+
+		if (rx_free_thresh != RTE_PMD_PARAM_UNSET)
+			dev_info.default_rxconf.rx_free_thresh = rx_free_thresh;
+
+		if (tx_free_thresh != RTE_PMD_PARAM_UNSET)
+			dev_info.default_txconf.tx_free_thresh = tx_free_thresh;
+
+		if (tx_rs_thresh != RTE_PMD_PARAM_UNSET)
+			dev_info.default_txconf.tx_rs_thresh = tx_rs_thresh;
+	}
+
+	for (portid = 0; portid < nb_ports; portid++) {
 		/* skip ports that are not enabled */
 		if ((enabled_port_mask & (1 << portid)) == 0) {
 			printf("\nSkipping disabled port %d\n", portid);