Re: [dpdk-dev] [PATCH 21.05] app/testpmd: support show Rx queue count

[Ferruh Yigit <ferruh.yigit@intel.com>]

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
>   ~~~~~~~~~~~
>