Re: [PATCH v3 2/2] app/testpmd: add API for configuration of queue region

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

On 9/15/2017 4:13 AM, Wei Zhao wrote:
> This patch add a API configuration of queue region in rss.
> It can parse the parameters of region index, queue number,
> queue start index, user priority, traffic classes and so on.
> According to commands from command line, it will call i40e
> private API and start the process of set or flush queue region
> configure. As this feature is specific for i40e, so private API
> will be used.
> 
> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> ---
>  app/test-pmd/cmdline.c | 328 +++++++++++++++++++++++++++++++++++++++++++++++++

Testpmd documentation also needs to be updated.

>  1 file changed, 328 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 0144191..060fcb1 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -637,6 +637,21 @@ static void cmd_help_long_parsed(void *parsed_result,
>  			"ptype mapping update (port_id) (hw_ptype) (sw_ptype)\n"
>  			"    Update a ptype mapping item on a port\n\n"
>  
> +			"queue-region set port (port_id) region_id (value) "
> +			"queue_start_index (value) queue_num (value)\n"
> +			"    Set a queue region on a port\n\n"
> +
> +			"queue-region set (pf|vf) port (port_id) region_id (value) "
> +			"flowtype (value)\n"
> +			"    Set a flowtype region index on a port\n\n"
> +
> +			"queue-region set port (port_id) UP (value) region_id (value)\n"
> +			"    Set the mapping of User Priority to "
> +			"queue region on a port\n\n"
> +
> +			"queue-region flush (on|off) port (port_id)\n"
> +			"    flush all queue region related configuration\n\n"

I keep doing same comment but I will do it again...

Each patch adding a new feature looking from its own context and adding
a new root level command and this is making overall testpmd confusing.

Since this is to set an option of the port, what do you think making
this command part of existing commands, like:
"port config #P queue-region ...."
OR
"set port #P queue-region ..." ?

> +
>  			, list_pkt_forwarding_modes()
>  		);
>  	}
> @@ -8224,6 +8239,315 @@ cmdline_parse_inst_t cmd_syn_filter = {
>  		NULL,
>  	},
>  };
> +/* *** queue region set *** */
> +struct cmd_queue_region_result {
> +	cmdline_fixed_string_t cmd;
> +	cmdline_fixed_string_t set;
> +	cmdline_fixed_string_t port;
> +	uint8_t  port_id;
> +	cmdline_fixed_string_t region;
> +	uint8_t  region_id;
> +	cmdline_fixed_string_t queue_start_index;
> +	uint8_t  queue_id;
> +	cmdline_fixed_string_t queue_num;
> +	uint8_t  queue_num_value;
> +};
> +
> +static void
> +cmd_queue_region_parsed(void *parsed_result,
> +			__attribute__((unused)) struct cmdline *cl,
> +			__attribute__((unused)) void *data)
> +{
> +	struct cmd_queue_region_result *res = parsed_result;
> +	struct rte_i40e_rss_region_conf region_conf;
> +	int ret = 0;
> +
> +	memset(&region_conf, 0, sizeof(region_conf));
> +	region_conf.op = RTE_PMD_I40E_QUEUE_REGION_SET;
> +	region_conf.region_id = res->region_id;
> +	region_conf.queue_num = res->queue_num_value;
> +	region_conf.queue_start_index = res->queue_id;
> +
> +	ret = rte_pmd_i40e_queue_region_conf(res->port_id, &region_conf);

It is not safe to directly call PMD specific APIs from testpmd? What if
that PMD is not enabled? There are samples how to do this, can you
please check them?

<...>