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

[Ferruh Yigit]

On 8/29/2019 5:56 PM, Thomas Monjalon wrote:
> 28/08/2019 16:39, Andrew Rybchenko:
>> On 8/28/19 2:26 PM, Jan Viktorin wrote:
>>> Andrew Rybchenko <arybchenko at solarflare.com> wrote:
>>>> On 8/28/19 12:51 PM, Jan Viktorin wrote:
>>>>> Andrew Rybchenko <arybchenko at solarflare.com> wrote:
>>>>>> From: Ivan Ilchenko <Ivan.Ilchenko at oktetlabs.ru>
>>>>>> + * @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?
>>> One way would be to fail to register a PMD without that callback.
>>> Such PMD driver would be simply wrong. This is what I mean by saying
>>> "mandatory" - the callback MUST be provided.
>>
>> Typically callbacks are set on probe and
>> rte_eth_dev_pci_generic_probe() and similar functions could
>> be updated to reject devices with missing dev_infos_get callback.
>> However, there is a secondary process cases where dev_infos_get
>> is not essential since control path may be unsupported in secondary
>> process.
>>
>> Anyway, I think it is a separate story.
>>
>>> DPDK could be so nice to provide a default callback named like
>>> default_dev_infos_get_when_you_are_incompetent_pmd_author() returning
>>> mostly zeros and some always "known metadata" like device pointer,
>>> driver_name, ...
>>
>> Thomas, Ferruh, what do you think?
> 
> I like the idea of making some functions mandatory.
> If we need to provide a default callback, why not.
> 
> I'm also thinking we need to better enforce a standardization
> of basic features to be implemented. It would make DPDK more mature.
> 
> 

+1 to make some dev_ops mandatory. At first I can think of:
dev_infos_get
dev_configure
rx_queue_setup
tx_queue_setup
dev_start
dev_stop

specific to 'dev_infos_get', it is to get device info, not sure a default
callback is good idea for it.

And overall agree that 'rte_eth_dev_info_get()' can be documented more/better ...