[dpdk-dev,v7,4/5] app/testpmd: enable queue ring size configure

Message ID 20180422115824.105219-5-qi.z.zhang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply patch file failure

Commit Message

Qi Zhang April 22, 2018, 11:58 a.m. UTC
  Add command to change specific queue's ring size configure,
the new value will only take effect after command that restart
the device(port stop <port_id>/port start <port_id>) or command
that setup the queue(port <port_id> rxq <qid> setup) at runtime.

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
 app/test-pmd/cmdline.c                      | 102 ++++++++++++++++++++++++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |   9 +++
 2 files changed, 111 insertions(+)
  

Comments

Ferruh Yigit April 23, 2018, 5:45 p.m. UTC | #1
On 4/22/2018 12:58 PM, Qi Zhang wrote:
> Add command to change specific queue's ring size configure,
> the new value will only take effect after command that restart
> the device(port stop <port_id>/port start <port_id>) or command
> that setup the queue(port <port_id> rxq <qid> setup) at runtime.
> 
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> ---
>  app/test-pmd/cmdline.c                      | 102 ++++++++++++++++++++++++++++
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |   9 +++
>  2 files changed, 111 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index b50e11e60..22e4d4585 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -846,6 +846,11 @@ static void cmd_help_long_parsed(void *parsed_result,
>  			"port config mtu X value\n"
>  			"    Set the MTU of port X to a given value\n\n"
>  
> +			"port config (port_id) (rxq|txq) (queue_id) ring_size (value)\n"
> +			"    Set a rx/tx queue's ring size configuration, the new"
> +			" value will take effect after command that (re-)start the port"
> +			" or command that setup the specific queue\n\n"

"port config all rxq|txq|rxd|txd <value>" is used to set number of queues (rxq)
or number of descriptors in queue (rxd).

Problem is this is not flexible and your version is better.

What do you think removing old rxd|txd part with this patch, to prevent duplication?
  
Qi Zhang April 24, 2018, 3:16 a.m. UTC | #2
> -----Original Message-----

> From: Yigit, Ferruh

> Sent: Tuesday, April 24, 2018 1:45 AM

> To: Zhang, Qi Z <qi.z.zhang@intel.com>; thomas@monjalon.net

> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org;

> Xing, Beilei <beilei.xing@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;

> Lu, Wenzhuo <wenzhuo.lu@intel.com>

> Subject: Re: [PATCH v7 4/5] app/testpmd: enable queue ring size configure

> 

> On 4/22/2018 12:58 PM, Qi Zhang wrote:

> > Add command to change specific queue's ring size configure, the new

> > value will only take effect after command that restart the device(port

> > stop <port_id>/port start <port_id>) or command that setup the

> > queue(port <port_id> rxq <qid> setup) at runtime.

> >

> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>

> > ---

> >  app/test-pmd/cmdline.c                      | 102

> ++++++++++++++++++++++++++++

> >  doc/guides/testpmd_app_ug/testpmd_funcs.rst |   9 +++

> >  2 files changed, 111 insertions(+)

> >

> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index

> > b50e11e60..22e4d4585 100644

> > --- a/app/test-pmd/cmdline.c

> > +++ b/app/test-pmd/cmdline.c

> > @@ -846,6 +846,11 @@ static void cmd_help_long_parsed(void

> *parsed_result,

> >  			"port config mtu X value\n"

> >  			"    Set the MTU of port X to a given value\n\n"

> >

> > +			"port config (port_id) (rxq|txq) (queue_id) ring_size (value)\n"

> > +			"    Set a rx/tx queue's ring size configuration, the new"

> > +			" value will take effect after command that (re-)start the port"

> > +			" or command that setup the specific queue\n\n"

> 

> "port config all rxq|txq|rxd|txd <value>" is used to set number of queues

> (rxq) or number of descriptors in queue (rxd).

> 

> Problem is this is not flexible and your version is better.

> 

> What do you think removing old rxd|txd part with this patch, to prevent

> duplication?


I'm not sure.
Do we need some command to reset all queue's ring size to default value.
Probably we need to support "all" syntax on new command before consider remove this.

Also per queue config will be reset to original command line parameter by any command that call init_port_config, (for example: port config all rxq ...)
while "port config all rxd ..." modify the command line value, so they are different
	
Regards
Qi
  
Ferruh Yigit April 24, 2018, 11:05 a.m. UTC | #3
On 4/24/2018 4:16 AM, Zhang, Qi Z wrote:
> 
> 
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Tuesday, April 24, 2018 1:45 AM
>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; thomas@monjalon.net
>> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org;
>> Xing, Beilei <beilei.xing@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
>> Lu, Wenzhuo <wenzhuo.lu@intel.com>
>> Subject: Re: [PATCH v7 4/5] app/testpmd: enable queue ring size configure
>>
>> On 4/22/2018 12:58 PM, Qi Zhang wrote:
>>> Add command to change specific queue's ring size configure, the new
>>> value will only take effect after command that restart the device(port
>>> stop <port_id>/port start <port_id>) or command that setup the
>>> queue(port <port_id> rxq <qid> setup) at runtime.
>>>
>>> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
>>> ---
>>>  app/test-pmd/cmdline.c                      | 102
>> ++++++++++++++++++++++++++++
>>>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |   9 +++
>>>  2 files changed, 111 insertions(+)
>>>
>>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
>>> b50e11e60..22e4d4585 100644
>>> --- a/app/test-pmd/cmdline.c
>>> +++ b/app/test-pmd/cmdline.c
>>> @@ -846,6 +846,11 @@ static void cmd_help_long_parsed(void
>> *parsed_result,
>>>  			"port config mtu X value\n"
>>>  			"    Set the MTU of port X to a given value\n\n"
>>>
>>> +			"port config (port_id) (rxq|txq) (queue_id) ring_size (value)\n"
>>> +			"    Set a rx/tx queue's ring size configuration, the new"
>>> +			" value will take effect after command that (re-)start the port"
>>> +			" or command that setup the specific queue\n\n"
>>
>> "port config all rxq|txq|rxd|txd <value>" is used to set number of queues
>> (rxq) or number of descriptors in queue (rxd).
>>
>> Problem is this is not flexible and your version is better.
>>
>> What do you think removing old rxd|txd part with this patch, to prevent
>> duplication?
> 
> I'm not sure.
> Do we need some command to reset all queue's ring size to default value.
> Probably we need to support "all" syntax on new command before consider remove this.

My concern was having to multiple commands with different syntax for same
result, if both have different usecase that is OK.

> 
> Also per queue config will be reset to original command line parameter by any command that call init_port_config, (for example: port config all rxq ...)
> while "port config all rxd ..." modify the command line value, so they are different
> 	
> Regards
> Qi
>
  

Patch

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index b50e11e60..22e4d4585 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -846,6 +846,11 @@  static void cmd_help_long_parsed(void *parsed_result,
 			"port config mtu X value\n"
 			"    Set the MTU of port X to a given value\n\n"
 
+			"port config (port_id) (rxq|txq) (queue_id) ring_size (value)\n"
+			"    Set a rx/tx queue's ring size configuration, the new"
+			" value will take effect after command that (re-)start the port"
+			" or command that setup the specific queue\n\n"
+
 			"port (port_id) (rxq|txq) (queue_id) (start|stop)\n"
 			"    Start/stop a rx/tx queue of port X. Only take effect"
 			" when port X is started\n\n"
@@ -2191,6 +2196,102 @@  cmdline_parse_inst_t cmd_config_rss_hash_key = {
 	},
 };
 
+/* *** configure port rxq/txq ring size *** */
+struct cmd_config_rxtx_ring_size {
+	cmdline_fixed_string_t port;
+	cmdline_fixed_string_t config;
+	portid_t portid;
+	cmdline_fixed_string_t rxtxq;
+	uint16_t qid;
+	cmdline_fixed_string_t rsize;
+	uint16_t size;
+};
+
+static void
+cmd_config_rxtx_ring_size_parsed(void *parsed_result,
+				 __attribute__((unused)) struct cmdline *cl,
+				 __attribute__((unused)) void *data)
+{
+	struct cmd_config_rxtx_ring_size *res = parsed_result;
+	struct rte_port *port;
+	uint8_t isrx;
+
+	if (port_id_is_invalid(res->portid, ENABLED_WARN))
+		return;
+
+	if (res->portid == (portid_t)RTE_PORT_ALL) {
+		printf("Invalid port id\n");
+		return;
+	}
+
+	port = &ports[res->portid];
+
+	if (!strcmp(res->rxtxq, "rxq"))
+		isrx = 1;
+	else if (!strcmp(res->rxtxq, "txq"))
+		isrx = 0;
+	else {
+		printf("Unknown parameter\n");
+		return;
+	}
+
+	if (isrx && rx_queue_id_is_invalid(res->qid))
+		return;
+	else if (!isrx && tx_queue_id_is_invalid(res->qid))
+		return;
+
+	if (isrx && res->size != 0 && res->size <= rx_free_thresh) {
+		printf("Invalid rx ring_size, must > rx_free_thresh: %d\n",
+		       rx_free_thresh);
+		return;
+	}
+
+	if (isrx)
+		port->nb_rx_desc[res->qid] = res->size;
+	else
+		port->nb_tx_desc[res->qid] = res->size;
+
+	cmd_reconfig_device_queue(res->portid, 0, 1);
+}
+
+cmdline_parse_token_string_t cmd_config_rxtx_ring_size_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_rxtx_ring_size,
+				 port, "port");
+cmdline_parse_token_string_t cmd_config_rxtx_ring_size_config =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_rxtx_ring_size,
+				 config, "config");
+cmdline_parse_token_num_t cmd_config_rxtx_ring_size_portid =
+	TOKEN_NUM_INITIALIZER(struct cmd_config_rxtx_ring_size,
+				 portid, UINT16);
+cmdline_parse_token_string_t cmd_config_rxtx_ring_size_rxtxq =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_rxtx_ring_size,
+				 rxtxq, "rxq#txq");
+cmdline_parse_token_num_t cmd_config_rxtx_ring_size_qid =
+	TOKEN_NUM_INITIALIZER(struct cmd_config_rxtx_ring_size,
+			      qid, UINT16);
+cmdline_parse_token_string_t cmd_config_rxtx_ring_size_rsize =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_rxtx_ring_size,
+				 rsize, "ring_size");
+cmdline_parse_token_num_t cmd_config_rxtx_ring_size_size =
+	TOKEN_NUM_INITIALIZER(struct cmd_config_rxtx_ring_size,
+			      size, UINT16);
+
+cmdline_parse_inst_t cmd_config_rxtx_ring_size = {
+	.f = cmd_config_rxtx_ring_size_parsed,
+	.data = NULL,
+	.help_str = "port config <port_id> rxq|txq <queue_id> ring_size <value>",
+	.tokens = {
+		(void *)&cmd_config_rxtx_ring_size_port,
+		(void *)&cmd_config_rxtx_ring_size_config,
+		(void *)&cmd_config_rxtx_ring_size_portid,
+		(void *)&cmd_config_rxtx_ring_size_rxtxq,
+		(void *)&cmd_config_rxtx_ring_size_qid,
+		(void *)&cmd_config_rxtx_ring_size_rsize,
+		(void *)&cmd_config_rxtx_ring_size_size,
+		NULL,
+	},
+};
+
 /* *** configure port rxq/txq start/stop *** */
 struct cmd_config_rxtx_queue {
 	cmdline_fixed_string_t port;
@@ -16346,6 +16447,7 @@  cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_config_max_pkt_len,
 	(cmdline_parse_inst_t *)&cmd_config_rx_mode_flag,
 	(cmdline_parse_inst_t *)&cmd_config_rss,
+	(cmdline_parse_inst_t *)&cmd_config_rxtx_ring_size,
 	(cmdline_parse_inst_t *)&cmd_config_rxtx_queue,
 	(cmdline_parse_inst_t *)&cmd_setup_rxtx_queue,
 	(cmdline_parse_inst_t *)&cmd_config_rss_reta,
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 07a43aeeb..e0b159bc6 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -1623,6 +1623,15 @@  Close all ports or a specific port::
 
    testpmd> port close (port_id|all)
 
+port config - queue ring size
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Configure a rx/tx queue ring size::
+
+   testpmd> port (port_id) (rxq|txq) (queue_id) ring_size (value)
+
+Only take effect after command that (re-)start the port or command that setup specific queue.
+
 port start/stop queue
 ~~~~~~~~~~~~~~~~~~~~~