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 <arybche...@solarflare.com> wrote: >>>> On 8/28/19 12:51 PM, Jan Viktorin wrote: >>>>> Andrew Rybchenko <arybche...@solarflare.com> wrote: >>>>>> From: Ivan Ilchenko <ivan.ilche...@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 ...
