diff mbox

[dpdk-dev,v3,08/10] app/testpmd: remove txqflags

Message ID 079c2831ae6d80d105d7faf548a5683d965459e1.1514281260.git.shahafs@mellanox.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Shahaf Shuler Dec. 26, 2017, 9:44 a.m. UTC 
  Since testpmd is now using the new Ethdev offloads API and there is
a way configure each of the tx offloads from CLI or command line,
there is no need for the txqflags configuration anymore.

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
 app/test-pmd/cmdline.c                      | 69 ------------------------
 app/test-pmd/config.c                       |  7 +--
 app/test-pmd/parameters.c                   | 14 +----
 app/test-pmd/testpmd.c                      |  8 ---
 app/test-pmd/testpmd.h                      |  1 -
 doc/guides/testpmd_app_ug/run_app.rst       | 12 -----
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 11 ----
 7 files changed, 3 insertions(+), 119 deletions(-)

Comments

Wenzhuo Lu Jan. 9, 2018, 7:17 a.m. UTC | #1 
Hi Shahaf,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Shahaf Shuler
> Sent: Tuesday, December 26, 2017 5:44 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v3 08/10] app/testpmd: remove txqflags
> 
> Since testpmd is now using the new Ethdev offloads API and there is a way
> configure each of the tx offloads from CLI or command line, there is no need
> for the txqflags configuration anymore.
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>


> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> 806548196..91dcb31c0 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -259,11 +259,6 @@ int16_t tx_free_thresh = RTE_PMD_PARAM_UNSET;
> int16_t tx_rs_thresh = RTE_PMD_PARAM_UNSET;
> 
>  /*
> - * Configurable value of TX queue flags.
> - */
> -int32_t txq_flags = RTE_PMD_PARAM_UNSET;
> -
> -/*
>   * Receive Side Scaling (RSS) configuration.
>   */
>  uint64_t rss_hf = ETH_RSS_IP; /* RSS IP by default. */ @@ -2084,9 +2079,6
> @@ rxtx_port_config(struct rte_port *port)
> 
>  	if (tx_free_thresh != RTE_PMD_PARAM_UNSET)
>  		port->tx_conf.tx_free_thresh = tx_free_thresh;
> -
> -	if (txq_flags != RTE_PMD_PARAM_UNSET)
> -		port->tx_conf.txq_flags = txq_flags;
I think we have some problem if just remove this code. Because port->tx_conf.txq_flags is used by many NICs. If txqflags is removed, all these NICs have to use the default value and have no chance to change it.  I think it's a good idea to use tx-offloads to replace txqflags, but we may need to clear the drivers' code first.
Shahaf Shuler Jan. 9, 2018, 10:07 a.m. UTC | #2 
Tuesday, January 9, 2018 9:17 AM, Lu, Wenzhuo:

--Shahaf


> -----Original Message-----
> From: Lu, Wenzhuo [mailto:wenzhuo.lu@intel.com]
> Sent: Tuesday, January 9, 2018 9:17 AM
> To: Shahaf Shuler <shahafs@mellanox.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v3 08/10] app/testpmd: remove txqflags
> 
> Hi Shahaf,
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Shahaf Shuler
> > Sent: Tuesday, December 26, 2017 5:44 PM
> > To: Wu, Jingjing <jingjing.wu@intel.com>; Yigit, Ferruh
> > <ferruh.yigit@intel.com>
> > Cc: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH v3 08/10] app/testpmd: remove txqflags
> >
> > Since testpmd is now using the new Ethdev offloads API and there is a
> > way configure each of the tx offloads from CLI or command line, there
> > is no need for the txqflags configuration anymore.
> >
> > Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> > Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> 
> 
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > 806548196..91dcb31c0 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -259,11 +259,6 @@ int16_t tx_free_thresh =
> RTE_PMD_PARAM_UNSET;
> > int16_t tx_rs_thresh = RTE_PMD_PARAM_UNSET;
> >
> >  /*
> > - * Configurable value of TX queue flags.
> > - */
> > -int32_t txq_flags = RTE_PMD_PARAM_UNSET;
> > -
> > -/*
> >   * Receive Side Scaling (RSS) configuration.
> >   */
> >  uint64_t rss_hf = ETH_RSS_IP; /* RSS IP by default. */ @@ -2084,9
> > +2079,6 @@ rxtx_port_config(struct rte_port *port)
> >
> >  	if (tx_free_thresh != RTE_PMD_PARAM_UNSET)
> >  		port->tx_conf.tx_free_thresh = tx_free_thresh;
> > -
> > -	if (txq_flags != RTE_PMD_PARAM_UNSET)
> > -		port->tx_conf.txq_flags = txq_flags;
> I think we have some problem if just remove this code. Because port-
> >tx_conf.txq_flags is used by many NICs. If txqflags is removed, all these
> NICs have to use the default value and have no chance to change it.  I think
> it's a good idea to use tx-offloads to replace txqflags, but we may need to
> clear the drivers' code first.

There is an internal function in ethdev which do translation from the Tx offloads to the txqflags. Checkout ``rte_eth_convert_txq_offloads``.
This is specifically to support PMDs which still use the old offloads API. 

So in fact application needs only to specify the Tx offloads on the tx_conf and those will be converted to txqflags. The PMD see no difference.
Wenzhuo Lu Jan. 9, 2018, 11:51 a.m. UTC | #3 
Hi Shahaf,


> -----Original Message-----
> From: Shahaf Shuler [mailto:shahafs@mellanox.com]
> Sent: Tuesday, January 9, 2018 6:08 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v3 08/10] app/testpmd: remove txqflags
> 
> Tuesday, January 9, 2018 9:17 AM, Lu, Wenzhuo:
> 
> --Shahaf
> 
> 
> > -----Original Message-----
> > From: Lu, Wenzhuo [mailto:wenzhuo.lu@intel.com]
> > Sent: Tuesday, January 9, 2018 9:17 AM
> > To: Shahaf Shuler <shahafs@mellanox.com>; Wu, Jingjing
> > <jingjing.wu@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
> > Cc: dev@dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH v3 08/10] app/testpmd: remove txqflags
> >
> > Hi Shahaf,
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Shahaf Shuler
> > > Sent: Tuesday, December 26, 2017 5:44 PM
> > > To: Wu, Jingjing <jingjing.wu@intel.com>; Yigit, Ferruh
> > > <ferruh.yigit@intel.com>
> > > Cc: dev@dpdk.org
> > > Subject: [dpdk-dev] [PATCH v3 08/10] app/testpmd: remove txqflags
> > >
> > > Since testpmd is now using the new Ethdev offloads API and there is
> > > a way configure each of the tx offloads from CLI or command line,
> > > there is no need for the txqflags configuration anymore.
> > >
> > > Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> > > Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> >
> >
> > > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > > 806548196..91dcb31c0 100644
> > > --- a/app/test-pmd/testpmd.c
> > > +++ b/app/test-pmd/testpmd.c
> > > @@ -259,11 +259,6 @@ int16_t tx_free_thresh =
> > RTE_PMD_PARAM_UNSET;
> > > int16_t tx_rs_thresh = RTE_PMD_PARAM_UNSET;
> > >
> > >  /*
> > > - * Configurable value of TX queue flags.
> > > - */
> > > -int32_t txq_flags = RTE_PMD_PARAM_UNSET;
> > > -
> > > -/*
> > >   * Receive Side Scaling (RSS) configuration.
> > >   */
> > >  uint64_t rss_hf = ETH_RSS_IP; /* RSS IP by default. */ @@ -2084,9
> > > +2079,6 @@ rxtx_port_config(struct rte_port *port)
> > >
> > >  	if (tx_free_thresh != RTE_PMD_PARAM_UNSET)
> > >  		port->tx_conf.tx_free_thresh = tx_free_thresh;
> > > -
> > > -	if (txq_flags != RTE_PMD_PARAM_UNSET)
> > > -		port->tx_conf.txq_flags = txq_flags;
> > I think we have some problem if just remove this code. Because port-
> > >tx_conf.txq_flags is used by many NICs. If txqflags is removed, all
> > >these
> > NICs have to use the default value and have no chance to change it.  I
> > think it's a good idea to use tx-offloads to replace txqflags, but we
> > may need to clear the drivers' code first.
> 
> There is an internal function in ethdev which do translation from the Tx
> offloads to the txqflags. Checkout ``rte_eth_convert_txq_offloads``.
> This is specifically to support PMDs which still use the old offloads API.
> 
> So in fact application needs only to specify the Tx offloads on the tx_conf
> and those will be converted to txqflags. The PMD see no difference.
Yes, you're right, the convert code is already merged.

Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
diff mbox

Patch

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 858482174..b4ef1d0eb 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -3092,74 +3092,6 @@  cmdline_parse_inst_t cmd_set_txsplit = {
 	},
 };
 
-/* *** CONFIG TX QUEUE FLAGS *** */
-
-struct cmd_config_txqflags_result {
-	cmdline_fixed_string_t port;
-	cmdline_fixed_string_t config;
-	cmdline_fixed_string_t all;
-	cmdline_fixed_string_t what;
-	int32_t hexvalue;
-};
-
-static void cmd_config_txqflags_parsed(void *parsed_result,
-				__attribute__((unused)) struct cmdline *cl,
-				__attribute__((unused)) void *data)
-{
-	struct cmd_config_txqflags_result *res = parsed_result;
-
-	if (!all_ports_stopped()) {
-		printf("Please stop all ports first\n");
-		return;
-	}
-
-	if (strcmp(res->what, "txqflags")) {
-		printf("Unknown parameter\n");
-		return;
-	}
-
-	if (res->hexvalue >= 0) {
-		txq_flags = res->hexvalue;
-	} else {
-		printf("txqflags must be >= 0\n");
-		return;
-	}
-
-	init_port_config();
-
-	cmd_reconfig_device_queue(RTE_PORT_ALL, 1, 1);
-}
-
-cmdline_parse_token_string_t cmd_config_txqflags_port =
-	TOKEN_STRING_INITIALIZER(struct cmd_config_txqflags_result, port,
-				 "port");
-cmdline_parse_token_string_t cmd_config_txqflags_config =
-	TOKEN_STRING_INITIALIZER(struct cmd_config_txqflags_result, config,
-				 "config");
-cmdline_parse_token_string_t cmd_config_txqflags_all =
-	TOKEN_STRING_INITIALIZER(struct cmd_config_txqflags_result, all,
-				 "all");
-cmdline_parse_token_string_t cmd_config_txqflags_what =
-	TOKEN_STRING_INITIALIZER(struct cmd_config_txqflags_result, what,
-				 "txqflags");
-cmdline_parse_token_num_t cmd_config_txqflags_value =
-	TOKEN_NUM_INITIALIZER(struct cmd_config_txqflags_result,
-				hexvalue, INT32);
-
-cmdline_parse_inst_t cmd_config_txqflags = {
-	.f = cmd_config_txqflags_parsed,
-	.data = NULL,
-	.help_str = "port config all txqflags <value>",
-	.tokens = {
-		(void *)&cmd_config_txqflags_port,
-		(void *)&cmd_config_txqflags_config,
-		(void *)&cmd_config_txqflags_all,
-		(void *)&cmd_config_txqflags_what,
-		(void *)&cmd_config_txqflags_value,
-		NULL,
-	},
-};
-
 /* *** ADD/REMOVE ALL VLAN IDENTIFIERS TO/FROM A PORT VLAN RX FILTER *** */
 struct cmd_rx_vlan_filter_all_result {
 	cmdline_fixed_string_t rx_vlan;
@@ -15709,7 +15641,6 @@  cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_config_rx_mode_flag,
 	(cmdline_parse_inst_t *)&cmd_config_rss,
 	(cmdline_parse_inst_t *)&cmd_config_rxtx_queue,
-	(cmdline_parse_inst_t *)&cmd_config_txqflags,
 	(cmdline_parse_inst_t *)&cmd_config_rss_reta,
 	(cmdline_parse_inst_t *)&cmd_showport_reta,
 	(cmdline_parse_inst_t *)&cmd_config_burst,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 29115e255..faccb84ca 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -417,7 +417,6 @@  tx_queue_infos_display(portid_t port_id, uint16_t queue_id)
 	printf("\nTX writeback threshold: %hhu", qinfo.conf.tx_thresh.wthresh);
 	printf("\nTX RS threshold: %hu", qinfo.conf.tx_rs_thresh);
 	printf("\nTX free threshold: %hu", qinfo.conf.tx_free_thresh);
-	printf("\nTX flags: %#x", qinfo.conf.txq_flags);
 	printf("\nTX deferred start: %s",
 		(qinfo.conf.tx_deferred_start != 0) ? "on" : "off");
 	printf("\nNumber of TXDs: %hu", qinfo.nb_desc);
@@ -1714,10 +1713,8 @@  rxtx_config_display(void)
 				tx_conf->tx_thresh.pthresh,
 				tx_conf->tx_thresh.hthresh,
 				tx_conf->tx_thresh.wthresh);
-		printf("  TX RS bit threshold=%d - TXQ flags=0x%"PRIx32""
-		       " - TXQ offloads=0x%"PRIx64"\n",
-				tx_conf->tx_rs_thresh, tx_conf->txq_flags,
-				tx_conf->offloads);
+		printf("  TX RS bit threshold=%d - TXQ offloads=0x%"PRIx64"\n",
+				tx_conf->tx_rs_thresh, tx_conf->offloads);
 	}
 }
 
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 58889420f..fa547df66 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -99,7 +99,7 @@  usage(char* progname)
 	       "--rss-ip | --rss-udp | "
 	       "--rxpt= | --rxht= | --rxwt= | --rxfreet= | "
 	       "--txpt= | --txht= | --txwt= | --txfreet= | "
-	       "--txrst= | --txqflags= | --tx-offloads ]\n",
+	       "--txrst= | --tx-offloads ]\n",
 	       progname);
 #ifdef RTE_LIBRTE_CMDLINE
 	printf("  --interactive: run in interactive mode.\n");
@@ -192,8 +192,6 @@  usage(char* progname)
 	       "(0 <= N <= value of txd).\n");
 	printf("  --txrst=N: set the transmit RS bit threshold of TX rings to N "
 	       "(0 <= N <= value of txd).\n");
-	printf("  --txqflags=0xXXXXXXXX: hexadecimal bitmask of TX queue flags "
-	       "(0 <= N <= 0x7FFFFFFF).\n");
 	printf("  --tx-queue-stats-mapping=(port,queue,mapping)[,(port,queue,mapping]: "
 	       "tx queues statistics counters mapping "
 	       "(0 <= mapping <= %d).\n", RTE_ETHDEV_QUEUE_STAT_CNTRS - 1);
@@ -632,7 +630,6 @@  launch_args_parse(int argc, char** argv)
 		{ "txwt",			1, 0, 0 },
 		{ "txfreet",			1, 0, 0 },
 		{ "txrst",			1, 0, 0 },
-		{ "txqflags",			1, 0, 0 },
 		{ "rxpt",			1, 0, 0 },
 		{ "rxht",			1, 0, 0 },
 		{ "rxwt",			1, 0, 0 },
@@ -1006,15 +1003,6 @@  launch_args_parse(int argc, char** argv)
 				else
 					rte_exit(EXIT_FAILURE, "txrst must be >= 0\n");
 			}
-			if (!strcmp(lgopts[opt_idx].name, "txqflags")) {
-				char *end = NULL;
-				n = strtoul(optarg, &end, 16);
-				if (n >= 0)
-					txq_flags = (int32_t)n;
-				else
-					rte_exit(EXIT_FAILURE,
-						 "txqflags must be >= 0\n");
-			}
 			if (!strcmp(lgopts[opt_idx].name, "rxd")) {
 				n = atoi(optarg);
 				if (n > 0) {
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 806548196..91dcb31c0 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -259,11 +259,6 @@  int16_t tx_free_thresh = RTE_PMD_PARAM_UNSET;
 int16_t tx_rs_thresh = RTE_PMD_PARAM_UNSET;
 
 /*
- * Configurable value of TX queue flags.
- */
-int32_t txq_flags = RTE_PMD_PARAM_UNSET;
-
-/*
  * Receive Side Scaling (RSS) configuration.
  */
 uint64_t rss_hf = ETH_RSS_IP; /* RSS IP by default. */
@@ -2084,9 +2079,6 @@  rxtx_port_config(struct rte_port *port)
 
 	if (tx_free_thresh != RTE_PMD_PARAM_UNSET)
 		port->tx_conf.tx_free_thresh = tx_free_thresh;
-
-	if (txq_flags != RTE_PMD_PARAM_UNSET)
-		port->tx_conf.txq_flags = txq_flags;
 }
 
 void
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 3e63edfa1..c9af41cfa 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -409,7 +409,6 @@  extern int16_t rx_free_thresh;
 extern int8_t rx_drop_en;
 extern int16_t tx_free_thresh;
 extern int16_t tx_rs_thresh;
-extern int32_t txq_flags;
 
 extern uint8_t dcb_config;
 extern uint8_t dcb_test;
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index fface6f58..be9a2cdd1 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -447,18 +447,6 @@  The commandline options are:
     Set the transmit RS bit threshold of TX rings to N, where 0 <= N <= value of ``--txd``.
     The default value is 0.
 
-*   ``--txqflags=0xXXXXXXXX``
-
-    Set the hexadecimal bitmask of TX queue flags, where 0 <= N <= 0x7FFFFFFF.
-    The default value is 0.
-
-    .. note::
-
-       When using hardware offload functions such as vlan or checksum
-       add ``txqflags=0`` to force the full-featured TX code path.
-       In some PMDs this may already be the default.
-
-
 *   ``--rx-queue-stats-mapping=(port,queue,mapping)[,(port,queue,mapping)]``
 
     Set the RX queues statistics counters mapping 0 <= mapping <= 15.
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 2b00be81c..7d3a7ae93 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -354,8 +354,6 @@  The available information categories are:
   also modify the default hierarchy or specify the new hierarchy through CLI for
   implementing QoS scheduler.  Requires ``CONFIG_RTE_LIBRTE_PMD_SOFTNIC=y`` ``CONFIG_RTE_LIBRTE_SCHED=y``.
 
-Note: TX timestamping is only available in the "Full Featured" TX path. To force ``testpmd`` into this mode set ``--txqflags=0``.
-
 Example::
 
    testpmd> set fwd rxonly
@@ -1681,15 +1679,6 @@  RX scatter mode is off by default.
 
 The ``on`` option is equivalent to the ``--enable-scatter`` command-line option.
 
-port config - TX queue flags
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-Set a hexadecimal bitmap of TX queue flags for all ports::
-
-   testpmd> port config all txqflags value
-
-This command is equivalent to the ``--txqflags`` command-line option.
-
 port config - RX Checksum
 ~~~~~~~~~~~~~~~~~~~~~~~~~