From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 6D8A12BB3 for ; Tue, 30 Aug 2016 15:27:59 +0200 (CEST) Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga102.jf.intel.com with ESMTP; 30 Aug 2016 06:27:57 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,256,1470726000"; d="scan'208";a="2712690" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.25]) ([10.237.220.25]) by fmsmga006.fm.intel.com with ESMTP; 30 Aug 2016 06:27:55 -0700 To: Shreyansh Jain , dev@dpdk.org References: <1466510566-9240-1-git-send-email-shreyansh.jain@nxp.com> <1472219823-29486-1-git-send-email-shreyansh.jain@nxp.com> Cc: viktorin@rehivetech.com, david.marchand@6wind.com, thomas.monjalon@6wind.com, hemant.agrawal@nxp.com From: Ferruh Yigit Message-ID: <57C589DB.6010704@intel.com> Date: Tue, 30 Aug 2016 14:27:55 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <1472219823-29486-1-git-send-email-shreyansh.jain@nxp.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v8 00/25] Introducing rte_driver/rte_device generalization X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 30 Aug 2016 13:28:00 -0000 On 8/26/2016 2:56 PM, Shreyansh Jain wrote: > Based on master (e22856313fff2) > > Background: > =========== > > It includes two different patch-sets floated on ML earlier: > * Original patch series is from David Marchand [1], [2]. > `- This focused mainly on PCI (PDEV) part > `- v7 of this was posted by me [8] in August/2016 > * Patch series [4] from Jan Viktorin > `- This focused on VDEV and rte_device integration > > Introduction: > ============= > > This patch series introduces a generic device model, moving away from PCI > centric code layout. Key change is to introduce rte_driver/rte_device > structures at the top level which are inherited by > rte_XXX_driver/rte_XXX_device - where XXX belongs to {pci, vdev, soc (in > future),...}. > > Key motivation for this series is to move away from PCI centric design of > EAL to a more hierarchical device model - pivoted around a generic driver > and device. Each specific driver and device can inherit the common > properties of the generic set and build upon it through driver/device > specific functions. > > Earlier, the EAL device initialization model was: > (Refer: [3]) > > -- > Constructor: > |- PMD_DRIVER_REGISTER(rte_driver) > `- insert into dev_driver_list, rte_driver object > > rte_eal_init(): > |- rte_eal_pci_init() > | `- scan and fill pci_device_list from sysfs > | > |- rte_eal_dev_init() > | `- For each rte_driver in dev_driver_list > | `- call the rte_driver->init() function > | |- PMDs designed to call rte_eth_driver_register(eth_driver) > | |- eth_driver have rte_pci_driver embedded in them > | `- rte_eth_driver_register installs the > | rte_pci_driver->devinit/devuninit callbacks. > | > |- rte_eal_pci_probe() > | |- For each device detected, dev_driver_list is parsed and matching is > | | done. > | |- For each matching device, the rte_pci_driver->devinit() is called. > | |- Default map is to rte_eth_dev_init() which in turn creates a > | | new ethernet device (eth_dev) > | | `- eth_drv->eth_dev_init() is called which is implemented by > `--| individual PMD drivers. > > -- > > The structure of driver looks something like: > > +------------+ ._____. > | rte_driver <-----| PMD |___ > | .init | `-----` \ > +----.-------+ | \ > `-. | What PMD actually is > \ | | > +----------v----+ | > | eth_driver | | > | .eth_dev_init | | > +----.----------+ | > `-. | > \ | > +------------v---+ > | rte_pci_driver | > | .pci_devinit | > +----------------+ > > and all devices are part of a following linked lists: > - dev_driver_list for all rte_drivers > - pci_device_list for all devices, whether PCI or VDEV > > > From the above: > * a PMD initializes a rte_driver, eth_driver even though actually it is a > pci_driver > * initialization routines are passed from rte_driver->pci_driver->eth_driver > even though they should ideally be rte_eal_init()->rte_pci_driver() > * For a single driver/device type model, this is not necessarily a > functional issue - but more of a design language. > * But, when number of driver/device type increase, this would create problem > in how driver<=>device links are represented. > > Proposed Architecture: > ====================== > > A nice representation has already been created by David in [3]. Copying that > here: > > +------------------+ +-------------------------------+ > | | | | > | rte_pci_device | | rte_pci_driver | > | | | | > +-------------+ | +--------------+ | | +---------------------------+ | > | | | | | | | | | | > | rte_eth_dev +---> rte_device +-----> rte_driver | | > | | | | char name[] | | | | char name[] | | > +-------------+ | | | | | | int init(rte_device *) | | > | +--------------+ | | | int uninit(rte_device *) | | > | | | | | | > +------------------+ | +---------------------------+ | > | | > +-------------------------------+ > > - for ethdev on top of vdev devices > > +------------------+ +-------------------------------+ > | | | | > | drv specific | | rte_vdev_driver | > | | | | > +-------------+ | +--------------+ | | +---------------------------+ | > | | | | | | | | | | > | rte_eth_dev +---> rte_device +-----> rte_driver | | > | | | | char name[] | | | | char name[] | | > +-------------+ | | | | | | int init(rte_device *) | | > | +--------------+ | | | int uninit(rte_device *) | | > | | | | | | > +------------------+ | +---------------------------+ | > | | > | int priv_size | > | | > +-------------------------------+ There are also "eth_driver" and "rte_cryptodev_driver", which can be represent as: +-----------------------------------+ | | | eth_driver / rte_cryptodev_driver | | | | +-------------------------------+ | | | | | | | rte_pci_driver | | | | | | | | +---------------------------+ | | | | | | | | | | | rte_driver | | | | | | char name[] | | | | | | int init(rte_device *) | | | | | | int uninit(rte_device *) | | | | | | | | | | | +---------------------------+ | | | | | | | +-------------------------------+ | | | +-----------------------------------+ Is eth_driver really needs to be inherited from rte_pci_driver? It is possible to have ethernet driver, without PCI bus, at least we have virtual driver samples. How about: +-------------------------------+ | | | rte_pci_driver / | | rte_vdev_driver | | +---------------------------+ | +-------------------+ | | | | | eth_driver | | | rte_driver |<---------- driver | | | char name[] | | | eth_dev_init | | | int init(rte_device *) | | | eth_dev_uninit | | | int uninit(rte_device *) | | | | | | | | | | | +---------------------------+ | | | | functional_driver ------------------>| | +-------------------------------+ +-------------------+ Currently there is no way to reference rte_vdev_driver from eth_driver, since it only has pci_drv field. With this update, it can be possible to create eth_driver and rte_eth_vdev_probe() for virtual drivers for example. (Although this may not necessary right now, this gives ability to do.) Another think, what do you think moving "dev_private_size" from eth_driver and rte_cryptodev_driver to rte_driver, since this looks like generic need for drivers. And last think, what do you think renaming eth_driver to rte_eth_driver to be consistent? Thanks, ferruh