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 <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 > > +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 >
