From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 30F61A2E1B for ; Tue, 3 Sep 2019 14:09:29 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C10D91D50A; Tue, 3 Sep 2019 14:09:27 +0200 (CEST) Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id 9E8BD1D165 for ; Tue, 3 Sep 2019 14:09:26 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us3.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTPS id B4C859C0071; Tue, 3 Sep 2019 12:09:23 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Tue, 3 Sep 2019 13:09:17 +0100 To: Ferruh Yigit , Thomas Monjalon , Jan Viktorin CC: Neil Horman , John McNamara , Marko Kovacevic , , Ivan Ilchenko References: <1566915962-5472-2-git-send-email-arybchenko@solarflare.com> <20190828132638.24193266@coaster.localdomain> <1886728.S05C2o22q7@xps> <8259ef3a-73ac-2bdc-f7f1-83af0aa1cb16@intel.com> From: Andrew Rybchenko Message-ID: <16856b26-23dd-46bf-8875-153c03ad7f6e@solarflare.com> Date: Tue, 3 Sep 2019 15:09:13 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <8259ef3a-73ac-2bdc-f7f1-83af0aa1cb16@intel.com> Content-Language: en-GB X-Originating-IP: [91.220.146.112] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24886.003 X-TM-AS-Result: No-13.083700-8.000000-10 X-TMASE-MatchedRID: Rp71wniPtoM4Q7K/HmUF53bspjK6JP6qJuDBbd4NSqQNWA0aQ3FdiCGU b2JNxi1qy0hhEFQalUD2Meo1KIrzyHnPiFSNe6xclrGv+knQDl8GchEhVwJY32AMM0WKD4asUS0 xEYUYNYODWfNv+ApTBNtxq7Yo5FhMN68Ooe9V0NsoPrDxs+3NLDoSfZud5+GgT7zqZowzdpKsyG kbPqnO09tQRyknqHlewBjAcGBTroW1IvkXV6/2TiZm6wdY+F8K5GNm5cTRaUd0+4P4W65ejkh4F FTKCL9hY0HeqffloVgdlUItd0X5jN/jKAW/2v78U3sFLicCFhUO9z+P2gwiBYJM8C7JLK1JvjEZ PIkHSfrc2mne0Tkg/B0VZ3HnAifxVMcggo9p3kytNKGdmwMhA8MA9JsxaUa3/HqTI96kSD6+mFU WJD5GAkWFICE5GJQ0P31UHmNXNmhh44VlLBLudMmR5yDJkPg4qQd+Hs7Yv7v61Ei0kvuTLcT8ZA 0pwEMipEYBIO560cqv8TAvJrvuV3Ho2p42VhyY5rnnDTmVbKog/CIfleX9D4qxMBrs575iLFlXZ d2HA7CdfbUqhtxNN/rap8Wg8afC0+Gv1tc3Op1mNCKOCsW/OlReZlHerCDGOyxd6V963VS48WuR FQVdauLzNWBegCW2RYvisGWbbS+3sNbcHjySQeJGF26G8SWyyy60Xy+RYeE6PFo4N4x6suFhbsr lIrZVTgNLXt+w9C7CJ0ZQvReeEiQ7cxSDj7Z8XGI6o4r+k7i7xSjjSvx+eLt25YdPKsEuCfkx1H +pA7M= X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--13.083700-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24886.003 X-MDID: 1567512566-s8bhHWPNc1ld Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH 01/51] ethdev: change rte_eth_dev_info_get() return value to int X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 wrote: >>>>> On 8/28/19 12:51 PM, Jan Viktorin wrote: >>>>>> Andrew Rybchenko wrote: >>>>>>> From: Ivan Ilchenko >>>>>>> + * @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. > 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