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

Ferruh Yigit ferruh.yigit at intel.com
Tue Sep 3 14:36:54 CEST 2019


On 9/3/2019 1:09 PM, Andrew Rybchenko wrote:
> On 9/2/19 12:33 PM, Ferruh Yigit wrote:
>> 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
> 
> +1 as well, but I think it is a separate story.
> I really don't want to complicate so big patchset by introducing
> more logic here.
> 
> As far as I can see dev_infos_get callback is implemented in
> all network PMDs. So, I think the topic is not gating the patchset.

Agreed it is a separate story and not a blocker for this set

> 
>> 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 ...
> 
> Me too
> 



More information about the dev mailing list