From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shreyansh Jain Subject: Re: [PATCH v2 2/2] eal: rename dev init API for consistency Date: Mon, 5 Dec 2016 19:33:13 +0530 Message-ID: <38b932b8-fe75-289f-b2a0-5dd0fbec60ba@nxp.com> References: <1479628850-27202-1-git-send-email-jerin.jacob@caviumnetworks.com> <1480798539-13360-1-git-send-email-jerin.jacob@caviumnetworks.com> <1480798539-13360-3-git-send-email-jerin.jacob@caviumnetworks.com> <20161205102404.GA29487@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Cc: , , , To: Jerin Jacob Return-path: Received: from NAM02-SN1-obe.outbound.protection.outlook.com (mail-sn1nam02on0068.outbound.protection.outlook.com [104.47.36.68]) by dpdk.org (Postfix) with ESMTP id 256EE377C for ; Mon, 5 Dec 2016 15:00:31 +0100 (CET) In-Reply-To: <20161205102404.GA29487@localhost.localdomain> 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 Monday 05 December 2016 03:54 PM, Jerin Jacob wrote: > On Mon, Dec 05, 2016 at 03:42:18PM +0530, Shreyansh Jain wrote: >> Hello Jerin, > > Hello Shreyansh, > >> >> On Sunday 04 December 2016 02:25 AM, Jerin Jacob wrote: >>> rte_eal_dev_init() is a misleading name. >>> It actually performs the driver->probe for vdev, >>> which is parallel to rte_eal_pci_probe. >>> >>> Changed to rte_eal_vdev_probe for consistency and >>> moved the vdev specific probe to eal_common_vdev.c >>> >>> Suggested-by: Shreyansh Jain >>> Signed-off-by: Jerin Jacob >>> --- >>> +int >>> +rte_eal_vdev_probe(void) >>> +{ >>> + struct rte_devargs *devargs; >>> + >>> + /* >>> + * Note that the dev_driver_list is populated here >>> + * from calls made to rte_eal_driver_register from constructor functions >>> + * embedded into PMD modules via the RTE_PMD_REGISTER_VDEV macro >>> + */ >>> + >>> + /* call the init function for each virtual device */ >>> + TAILQ_FOREACH(devargs, &devargs_list, next) { >>> + >>> + if (devargs->type != RTE_DEVTYPE_VIRTUAL) >>> + continue; >>> + >>> + if (rte_eal_vdev_init(devargs->virt.drv_name, >> >> The situation now is: >> rte_eal_init=>rte_eal_vdev_probe()=>rte_eal_vdev_init()=> driver->probe() >> >> Even though I had suggested this, my intention was to completely do away >> with rte_*_[v]dev_init as it is misleading. >> >> rte_eal_init=>rte_eal_vdev_probe=>driver->probe() > > IMO, We don't need to remove rte_eal_vdev_init() as it is an > application API that uses to create vdev driver instance.Moreover, > change and removing that name will result in ABI breakage. > > grep -ri "rte_eal_vdev_init" app/ > app/test/test_cryptodev.c: ret = rte_eal_vdev_init( > app/test/test_cryptodev.c: TEST_ASSERT_SUCCESS(rte_eal_vdev_init( > app/test/test_cryptodev.c: TEST_ASSERT_SUCCESS(rte_eal_vdev_init( > app/test/test_cryptodev.c: TEST_ASSERT_SUCCESS(rte_eal_vdev_init( > app/test/test_cryptodev.c: TEST_ASSERT_SUCCESS(rte_eal_vdev_init( > app/test/test_cryptodev.c: int dev_id = rte_eal_vdev_init( > app/test/test_cryptodev.c: ret = rte_eal_vdev_init( > app/test/test_cryptodev_perf.c: ret = rte_eal_vdev_init( > app/test/test_cryptodev_perf.c: ret = rte_eal_vdev_init( > app/test/test_cryptodev_perf.c: ret = rte_eal_vdev_init( > app/test/test_cryptodev_perf.c: ret = rte_eal_vdev_init( > Got it. Have you noticed patches from Ben which actually merges init and probe all together? [1]. It is for PCI right now (and that too would break ABIs, I am assuming). [1] http://dpdk.org/dev/patchwork/patch/17206/ > >> >> should be the ideal order, IMO. >> Apologies, I was not completely clear then. >> >>> + devargs->args)) { >>> + RTE_LOG(ERR, EAL, "failed to initialize %s device\n", >>> + devargs->virt.drv_name); >>> + return -1; >>> + } >>> + } >>> + >>> + return 0; >>> +} >>> diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h >>> index 8840380..146f505 100644 >>> --- a/lib/librte_eal/common/include/rte_dev.h >>> +++ b/lib/librte_eal/common/include/rte_dev.h >>> @@ -171,9 +171,9 @@ void rte_eal_driver_register(struct rte_driver *driver); >>> void rte_eal_driver_unregister(struct rte_driver *driver); >>> >>> /** >>> - * Initalize all the registered drivers in this process >>> + * Probe all the registered vdev drivers in this process >>> */ >>> -int rte_eal_dev_init(void); >>> +int rte_eal_vdev_probe(void); >>> >>> /** >>> * Initialize a driver specified by name. >>> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c >>> index 16dd5b9..faf75cf 100644 >>> --- a/lib/librte_eal/linuxapp/eal/eal.c >>> +++ b/lib/librte_eal/linuxapp/eal/eal.c >>> @@ -884,8 +884,8 @@ rte_eal_init(int argc, char **argv) >>> if (rte_eal_pci_probe()) >>> rte_panic("Cannot probe PCI\n"); >>> >>> - if (rte_eal_dev_init() < 0) >>> - rte_panic("Cannot init pmd devices\n"); >>> + if (rte_eal_vdev_probe() < 0) >>> + rte_panic("Cannot probe vdev drivers\n"); >>> >>> rte_eal_mcfg_complete(); >>> >>> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map >>> index 83721ba..67fc95b 100644 >>> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map >>> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map >>> @@ -22,7 +22,7 @@ DPDK_2.0 { >>> rte_dump_tailq; >>> rte_eal_alarm_cancel; >>> rte_eal_alarm_set; >>> - rte_eal_dev_init; >>> + rte_eal_vdev_probe; >>> rte_eal_devargs_add; >>> rte_eal_devargs_dump; >>> rte_eal_devargs_type_count; >>> >> >