[dpdk-dev] [PATCH 02/51] app/testpmd: check status of getting ethdev info

[Andrew Rybchenko]

On 9/2/19 3:40 PM, Ferruh Yigit wrote:
> On 8/27/2019 3:25 PM, Andrew Rybchenko wrote:
>> From: Ivan Ilchenko <Ivan.Ilchenko at oktetlabs.ru>
>>
>> Add eth_dev_info_get_print_err() which is a wrapper for
>> rte_eth_dev_info_get() printing error if rte_eth_dev_info_get()
>> fails and returning its status code.
>>
>> Signed-off-by: Ivan Ilchenko <Ivan.Ilchenko at oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <arybchenko at solarflare.com>
> <...>
>
>> @@ -2410,9 +2414,13 @@ struct cmd_config_rss_hash_key {
>>   	struct rte_eth_dev_info dev_info;
>>   	uint8_t hash_key_size;
>>   	uint32_t key_len;
>> +	int ret;
>>   
>>   	memset(&dev_info, 0, sizeof(dev_info));
> It should be in different most probably, but the 'memset' before
> 'rte_eth_dev_info_get()' looks redundant, since API does the same.

I'll add pre-patch to fix it everywhere I found.

> <...>
>
>> @@ -4746,7 +4780,8 @@ struct cmd_tunnel_tso_set_result {
>>   {
>>   	struct rte_eth_dev_info dev_info;
>>   
>> -	rte_eth_dev_info_get(port_id, &dev_info);
>> +	eth_dev_info_get_print_err(port_id, &dev_info);
> Should we check the return value here too? And return 'dev_info' immediately on
> error instead of going on.

Thanks, missed it.

Also there are two more cases in testpmd to get maximum number
of Rx/Tx queue. I'll skip port and continue if device info get fails.

> <...>
>
>> @@ -1136,7 +1139,10 @@ struct extmem_param {
>>   		/* Apply default TxRx configuration for all ports */
>>   		port->dev_conf.txmode = tx_mode;
>>   		port->dev_conf.rxmode = rx_mode;
>> -		rte_eth_dev_info_get(pid, &port->dev_info);
>> +
>> +		ret = eth_dev_info_get_print_err(pid, &port->dev_info);
>> +		if (ret != 0)
>> +			return;
> What do you think 'rte_exit()' here instead of 'return', 'init_config()' doesn't
> return error, and returning from init will assume everyting setup, 0 number of
> queues for any port eventually will cause an exit in the app, better to fail
> where error occurs.

I agree. I've considered to skip the port and continue with other port, but
it is better to catch such problems earlier instead of hiding behind complex
logic.

Thanks, Andrew.