From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 7532F8E91 for ; Fri, 30 Oct 2015 15:00:15 +0100 (CET) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga101.fm.intel.com with ESMTP; 30 Oct 2015 07:00:14 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,218,1444719600"; d="scan'208";a="838863171" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.208.63]) by orsmga002.jf.intel.com with SMTP; 30 Oct 2015 07:00:12 -0700 Received: by (sSMTP sendmail emulation); Fri, 30 Oct 2015 14:00:11 +0025 Date: Fri, 30 Oct 2015 14:00:11 +0000 From: Bruce Richardson To: David Marchand Message-ID: <20151030140011.GB10248@bricha3-MOBL3> References: <1446143795-31841-1-git-send-email-bernard.iremonger@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v6 00/28] remove pci driver from vdevs 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: Fri, 30 Oct 2015 14:00:16 -0000 On Fri, Oct 30, 2015 at 02:32:35PM +0100, David Marchand wrote: > Hello Bernard, > > On Thu, Oct 29, 2015 at 7:36 PM, Bernard Iremonger < > bernard.iremonger@intel.com> wrote: > > > There is a dummy pci driver in the vdev PMD's at present. > > This patch set removes the pci driver from the vdev PMD's. > > Changes have been made to librte_ether to handle vdevs and pdevs in the > > same way. > > > > [snip] > > > > Bernard Iremonger (28): > > librte_eal: add RTE_KDRV_NONE for vdevs > > librte_ether: add fields from rte_pci_driver to rte_eth_dev_data > > librte_ether: add function rte_eth_copy_dev_info > > ixgbe: copy pci device info to eth_dev data > > e1000: copy pci device info to eth_dev data > > i40e: copy pci device info to eth_dev data > > fm10k: copy pci device info to eth_dev data > > bnx2x: copy pci device info to eth_dev data > > cxgbe: copy pci device info to eth_dev data > > enic: copy pci device info to eth_dev data > > mlx4: copy pci device info to eth_dev data > > virtio: copy pci device info to eth_dev data > > vmxnet3: copy pci device info to eth_dev data > > null: copy device info to eth_dev data > > ring: copy device info to eth_dev data > > pcap: copy device info to eth_dev data > > af_packet: copy device info to eth_dev data > > xenvirt: copy device info to eth_dev data > > mpipe: copy device info to eth_dev data > > bonding: copy device info to eth_dev data > > librte_ether: remove branches on pci_dev > > null: remove pci device > > ring: remove pci device > > pcap: remove pci device > > af_packet: remove pci device > > xenvirt: remove pci device > > mpipe: remove pci device > > bonding: remove pci device > > > > We end up with kdrv none, while for virtual devices, I can't see a case > where we would need it. > > And these flags end up in ethdev : > +/** Device needs PCI BAR mapping (done with either IGB_UIO or VFIO) */ > +#define RTE_ETH_DEV_DRV_NEED_MAPPING RTE_PCI_DRV_NEED_MAPPING > +/** Device needs to be unbound even if no module is provided */ > +#define RTE_ETH_DEV_DRV_FORCE_UNBIND RTE_PCI_DRV_FORCE_UNBIND > +/** Device supports link state interrupt */ > +#define RTE_ETH_DEV_INTR_LSC RTE_PCI_DRV_INTR_LSC > +/** Device supports detaching capability */ > +#define RTE_ETH_DEV_DETACHABLE RTE_PCI_DRV_DETACHABLE > > I can't see the point of all this, you are just moving pci specific fields > to ethdev and I don't see how it would ease future cleanups or > introductions of other bus types. > > A real cleanup would be something like what Neil already proposed before. > Hi David, this is perhaps not a full solution to enable other device types, but it's certainly a step along the way. The primary goals were to: * remove all references to the pci_dev structure from ethdev, so that no checks need to be explicitly made for the underlying device type. In this, I believe it largely succeeds, as the flags above are checked for each device directly, and a number of them are completely generic across device types, e.g. whether it supports a link state interrupt or is hotpluggable. The others may be of use to other device types too, just without any non-pci devices to look at as reference in DPDK, it's hard to know for sure. * Remove the need to have a pci_dev structure within each vdev, and again this set does that cleanup, simplifying all the vdevs. I'm sure there is more work to do be done to get more and different device support in there, but this patch has been through 6 revisions and multiple reviews and I believe is worthwhile including in 2.2 for the reasons stated above. Regards, /Bruce