[dpdk-dev] [PATCH 01/51] ethdev: change rte_eth_dev_info_get() return value to int

Andrew Rybchenko arybchenko at solarflare.com
Wed Aug 28 12:09:51 CEST 2019


On 8/28/19 12:51 PM, Jan Viktorin wrote:
> On Tue, 27 Aug 2019 15:25:12 +0100
> Andrew Rybchenko <arybchenko at solarflare.com> wrote:
>
>> From: Ivan Ilchenko <Ivan.Ilchenko at oktetlabs.ru>
>>
>> Change rte_eth_dev_info_get() return value from void to int and return
>> negative errno values in case of error conditions.
>> Modify rte_eth_dev_info_get() usage across the ethdev according
>> to new return type.
> Hello Andrew,
>
> I didn't find any cover letter describing the true intentions of this
> patchset. Anyway, see below a short comment...

The cover letter [1] was sent together with the patch.

[1] http://mails.dpdk.org/archives/dev/2019-August/141593.html

>> Signed-off-by: Ivan Ilchenko <Ivan.Ilchenko at oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <arybchenko at solarflare.com>
>> ---
>>   doc/guides/rel_notes/deprecation.rst   |  1 -
>>   doc/guides/rel_notes/release_19_11.rst |  5 ++-
>>   lib/librte_ethdev/rte_ethdev.c         | 71
>> ++++++++++++++++++++++++---------- lib/librte_ethdev/rte_ethdev.h
>>      |  6 ++- 4 files changed, 60 insertions(+), 23 deletions(-)
> [...]
>
>> b/lib/librte_ethdev/rte_ethdev.h index dc6596b..09c278d 100644
>> --- a/lib/librte_ethdev/rte_ethdev.h
>> +++ b/lib/librte_ethdev/rte_ethdev.h
>> @@ -2366,8 +2366,12 @@ int
>> rte_eth_dev_set_rx_queue_stats_mapping(uint16_t port_id,
>>    * @param dev_info
>>    *   A pointer to a structure of type *rte_eth_dev_info* to be
>> filled with
>>    *   the contextual information of the Ethernet device.
>> + * @return
>> + *   - (0) if successful.
>> + *   - (-ENOTSUP) if support for dev_infos_get() does not exist for
>> the device.
> I believe that allowing PMDs to return -ENOTSUP is not a good idea.
> At the moment, all PMDs provides this kind of information. It is not
> always very reliable piece of information but for me, it is a piece of
> gold I would not like to loose when configuring devices.
>
> I think it should be mandatory for all PMDs to provide this function.
> Another possible way, give a sane default contents of this structure.
> But, please, do not return -ENOTSUP.

I agree that dev_infos_get() callback should be mandatory, but
what the function should do if the callback is not provided?
I think that a sane default contents is more harm than an error
(basically that's what we had before the patch).
Since the function may return error, caller should take it into
account anyway. Yes, some error codes could have special
handling, but default error handling is required in any case.



More information about the dev mailing list