[21.05] app/testpmd: support show Rx queue count

Message ID 20210211194434.172212-1-lance.richardson@broadcom.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [21.05] app/testpmd: support show Rx queue count |

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/iol-broadcom-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/checkpatch success coding style OK

Commit Message

Lance Richardson Feb. 11, 2021, 7:44 p.m. UTC
  Add support for querying receive queue count in order to allow
the rte_eth_dev rx_queue_count() API to be exercised and tested.

Signed-off-by: Lance Richardson <lance.richardson@broadcom.com>
---
 app/test-pmd/cmdline.c                      | 65 +++++++++++++++++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  6 ++
 2 files changed, 71 insertions(+)
  

Comments

Ferruh Yigit Feb. 12, 2021, 11:51 a.m. UTC | #1
On 2/11/2021 7:44 PM, Lance Richardson wrote:
> Add support for querying receive queue count in order to allow
> the rte_eth_dev rx_queue_count() API to be exercised and tested.
> 

+1 to adding this feature, but the naming is a little misleading, "Rx queue 
count", it looks like it will print the number of Rx queues, and the API has the 
same problem indeed.

Can you please clarify it that it is to get number of used descriptor in a Rx queue?
And "used descriptor" part also needs some explanation I think.

> Signed-off-by: Lance Richardson <lance.richardson@broadcom.com>
> ---
>   app/test-pmd/cmdline.c                      | 65 +++++++++++++++++++++
>   doc/guides/testpmd_app_ug/testpmd_funcs.rst |  6 ++
>   2 files changed, 71 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 59722d268..6e2fe57a6 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -16699,6 +16699,70 @@ cmdline_parse_inst_t cmd_show_rx_tx_desc_status = {
>   	},
>   };
>   
> +/* *** display rx queue count *** */
> +struct cmd_show_rx_queue_count_result {
> +	cmdline_fixed_string_t cmd_show;
> +	cmdline_fixed_string_t cmd_port;
> +	cmdline_fixed_string_t cmd_rxq;
> +	cmdline_fixed_string_t cmd_count;
> +	portid_t cmd_pid;
> +	portid_t cmd_qid;
> +};
> +
> +static void
> +cmd_show_rx_queue_count_parsed(void *parsed_result,
> +		__rte_unused struct cmdline *cl,
> +		__rte_unused void *data)
> +{
> +	struct cmd_show_rx_queue_count_result *res = parsed_result;
> +	int rc;
> +
> +	if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
> +		printf("invalid port id %u\n", res->cmd_pid);
> +		return;
> +	}
> +
> +	rc = rte_eth_rx_queue_count(res->cmd_pid, res->cmd_qid);
> +	if (rc < 0) {
> +		printf("Invalid queueid = %d\n", res->cmd_qid);
> +		return;
> +	}
> +	printf("Used desc count = %d\n", rc);
> +}
> +
> +cmdline_parse_token_string_t cmd_show_rx_queue_count_show =
> +	TOKEN_STRING_INITIALIZER(struct cmd_show_rx_queue_count_result,
> +			cmd_show, "show");
> +cmdline_parse_token_string_t cmd_show_rx_queue_count_port =
> +	TOKEN_STRING_INITIALIZER(struct cmd_show_rx_queue_count_result,
> +			cmd_port, "port");
> +cmdline_parse_token_num_t cmd_show_rx_queue_count_pid =
> +	TOKEN_NUM_INITIALIZER(struct cmd_show_rx_queue_count_result,
> +			cmd_pid, RTE_UINT16);
> +cmdline_parse_token_string_t cmd_show_rx_queue_count_rxq =
> +	TOKEN_STRING_INITIALIZER(struct cmd_show_rx_queue_count_result,
> +			cmd_rxq, "rxq");
> +cmdline_parse_token_num_t cmd_show_rx_queue_count_qid =
> +	TOKEN_NUM_INITIALIZER(struct cmd_show_rx_queue_count_result,
> +			cmd_qid, RTE_UINT16);
> +cmdline_parse_token_string_t cmd_show_rx_queue_count_count =
> +	TOKEN_STRING_INITIALIZER(struct cmd_show_rx_queue_count_result,
> +			cmd_count, "count");
> +cmdline_parse_inst_t cmd_show_rx_queue_count = {
> +	.f = cmd_show_rx_queue_count_parsed,
> +	.data = NULL,
> +	.help_str = "show port <port_id> rxq <queue_id> count",

There is already an existing command:
"show port <port_id> rxq|txq <queue_id> desc <desc_id> status"

What do you think adding the new one as something like:
"show port <port_id> rxq <queue_id> desc used count"

> +	.tokens = {
> +		(void *)&cmd_show_rx_queue_count_show,
> +		(void *)&cmd_show_rx_queue_count_port,
> +		(void *)&cmd_show_rx_queue_count_pid,
> +		(void *)&cmd_show_rx_queue_count_rxq,
> +		(void *)&cmd_show_rx_queue_count_qid,
> +		(void *)&cmd_show_rx_queue_count_count,
> +		NULL,
> +	},
> +};
> +
>   /* Common result structure for set port ptypes */
>   struct cmd_set_port_ptypes_result {
>   	cmdline_fixed_string_t set;
> @@ -17098,6 +17162,7 @@ cmdline_parse_ctx_t main_ctx[] = {
>   	(cmdline_parse_inst_t *)&cmd_config_tx_metadata_specific,
>   	(cmdline_parse_inst_t *)&cmd_show_tx_metadata,
>   	(cmdline_parse_inst_t *)&cmd_show_rx_tx_desc_status,
> +	(cmdline_parse_inst_t *)&cmd_show_rx_queue_count,
>   	(cmdline_parse_inst_t *)&cmd_set_raw,
>   	(cmdline_parse_inst_t *)&cmd_show_set_raw,
>   	(cmdline_parse_inst_t *)&cmd_show_set_raw_all,
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index a45910b81..789ee7d27 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -266,6 +266,12 @@ Display information for a given port's RX/TX descriptor status::
>   
>      testpmd> show port (port_id) (rxq|txq) (queue_id) desc (desc_id) status
>   
> +show rxq count
> +~~~~~~~~~~~~~~~~~~~~~~~~~

The '~' line length should be same as header length

> +
> +Display the number of ready descriptors on a given RX queue::

Can you please describe more, what is "ready descriptor"?

The 'rte_eth_rx_queue_count()' API should be returning number of descriptors 
filled by HW.

> +
> +   testpmd> show port (port_id) rxq (queue_id) count
>   
>   show config
>   ~~~~~~~~~~~
>
  
Lance Richardson Feb. 12, 2021, 3:12 p.m. UTC | #2
On Fri, Feb 12, 2021 at 6:51 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 2/11/2021 7:44 PM, Lance Richardson wrote:
> > Add support for querying receive queue count in order to allow
> > the rte_eth_dev rx_queue_count() API to be exercised and tested.
> >
>
> +1 to adding this feature, but the naming is a little misleading, "Rx queue
> count", it looks like it will print the number of Rx queues, and the API has the
> same problem indeed.
>
> Can you please clarify it that it is to get number of used descriptor in a Rx queue?
> And "used descriptor" part also needs some explanation I think.
>
That makes sense, fixed in v2.

>
> There is already an existing command:
> "show port <port_id> rxq|txq <queue_id> desc <desc_id> status"
>
> What do you think adding the new one as something like:
> "show port <port_id> rxq <queue_id> desc used count"

Sounds good, v2 is updated to use that form.

> > +show rxq count
> > +~~~~~~~~~~~~~~~~~~~~~~~~~
>
> The '~' line length should be same as header length
>

Fixed in v2.

> > +
> > +Display the number of ready descriptors on a given RX queue::
>
> Can you please describe more, what is "ready descriptor"?
>
> The 'rte_eth_rx_queue_count()' API should be returning number of descriptors
> filled by HW.
>

I took a stab at this in v2, but maybe it could be expanded more.

As I understand it, the returned descriptor count should correspond to
the number of packets that could be received in the next call to the burst
receive function... not necessarily the hardware-specific notion of a
descriptor, which might include descriptors used for chained mbufs,
LRO metadata, async status messages from firmware, etc.

Thanks,

    Lance
  

Patch

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 59722d268..6e2fe57a6 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -16699,6 +16699,70 @@  cmdline_parse_inst_t cmd_show_rx_tx_desc_status = {
 	},
 };
 
+/* *** display rx queue count *** */
+struct cmd_show_rx_queue_count_result {
+	cmdline_fixed_string_t cmd_show;
+	cmdline_fixed_string_t cmd_port;
+	cmdline_fixed_string_t cmd_rxq;
+	cmdline_fixed_string_t cmd_count;
+	portid_t cmd_pid;
+	portid_t cmd_qid;
+};
+
+static void
+cmd_show_rx_queue_count_parsed(void *parsed_result,
+		__rte_unused struct cmdline *cl,
+		__rte_unused void *data)
+{
+	struct cmd_show_rx_queue_count_result *res = parsed_result;
+	int rc;
+
+	if (!rte_eth_dev_is_valid_port(res->cmd_pid)) {
+		printf("invalid port id %u\n", res->cmd_pid);
+		return;
+	}
+
+	rc = rte_eth_rx_queue_count(res->cmd_pid, res->cmd_qid);
+	if (rc < 0) {
+		printf("Invalid queueid = %d\n", res->cmd_qid);
+		return;
+	}
+	printf("Used desc count = %d\n", rc);
+}
+
+cmdline_parse_token_string_t cmd_show_rx_queue_count_show =
+	TOKEN_STRING_INITIALIZER(struct cmd_show_rx_queue_count_result,
+			cmd_show, "show");
+cmdline_parse_token_string_t cmd_show_rx_queue_count_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_show_rx_queue_count_result,
+			cmd_port, "port");
+cmdline_parse_token_num_t cmd_show_rx_queue_count_pid =
+	TOKEN_NUM_INITIALIZER(struct cmd_show_rx_queue_count_result,
+			cmd_pid, RTE_UINT16);
+cmdline_parse_token_string_t cmd_show_rx_queue_count_rxq =
+	TOKEN_STRING_INITIALIZER(struct cmd_show_rx_queue_count_result,
+			cmd_rxq, "rxq");
+cmdline_parse_token_num_t cmd_show_rx_queue_count_qid =
+	TOKEN_NUM_INITIALIZER(struct cmd_show_rx_queue_count_result,
+			cmd_qid, RTE_UINT16);
+cmdline_parse_token_string_t cmd_show_rx_queue_count_count =
+	TOKEN_STRING_INITIALIZER(struct cmd_show_rx_queue_count_result,
+			cmd_count, "count");
+cmdline_parse_inst_t cmd_show_rx_queue_count = {
+	.f = cmd_show_rx_queue_count_parsed,
+	.data = NULL,
+	.help_str = "show port <port_id> rxq <queue_id> count",
+	.tokens = {
+		(void *)&cmd_show_rx_queue_count_show,
+		(void *)&cmd_show_rx_queue_count_port,
+		(void *)&cmd_show_rx_queue_count_pid,
+		(void *)&cmd_show_rx_queue_count_rxq,
+		(void *)&cmd_show_rx_queue_count_qid,
+		(void *)&cmd_show_rx_queue_count_count,
+		NULL,
+	},
+};
+
 /* Common result structure for set port ptypes */
 struct cmd_set_port_ptypes_result {
 	cmdline_fixed_string_t set;
@@ -17098,6 +17162,7 @@  cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_config_tx_metadata_specific,
 	(cmdline_parse_inst_t *)&cmd_show_tx_metadata,
 	(cmdline_parse_inst_t *)&cmd_show_rx_tx_desc_status,
+	(cmdline_parse_inst_t *)&cmd_show_rx_queue_count,
 	(cmdline_parse_inst_t *)&cmd_set_raw,
 	(cmdline_parse_inst_t *)&cmd_show_set_raw,
 	(cmdline_parse_inst_t *)&cmd_show_set_raw_all,
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index a45910b81..789ee7d27 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -266,6 +266,12 @@  Display information for a given port's RX/TX descriptor status::
 
    testpmd> show port (port_id) (rxq|txq) (queue_id) desc (desc_id) status
 
+show rxq count
+~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Display the number of ready descriptors on a given RX queue::
+
+   testpmd> show port (port_id) rxq (queue_id) count
 
 show config
 ~~~~~~~~~~~