From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Richardson, Bruce" Subject: Re: [RFC PATCH 1/6] librte_ether: add fields from rte_pci_driver to rte_eth_dev and rte_eth_dev_data. Date: Tue, 1 Sep 2015 12:43:30 +0000 Message-ID: <59AF69C657FD0841A61C55336867B5B035911E04@IRSMSX103.ger.corp.intel.com> References: <1440690041-32391-2-git-send-email-bernard.iremonger@intel.com> <3040981.rvdZqrEcfG@xps13> <8CEF83825BEC744B83065625E567D7C219F49A14@IRSMSX108.ger.corp.intel.com> <2601191342CEEE43887BDE71AB97725836A818B6@irsmsx105.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" To: "Ananyev, Konstantin" , "Iremonger, Bernard" , Thomas Monjalon Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 4A3A05323 for ; Tue, 1 Sep 2015 14:43:33 +0200 (CEST) In-Reply-To: <2601191342CEEE43887BDE71AB97725836A818B6@irsmsx105.ger.corp.intel.com> Content-Language: en-US List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev, Konstantin > Sent: Tuesday, September 1, 2015 1:37 PM > To: Iremonger, Bernard; Thomas Monjalon > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [RFC PATCH 1/6] librte_ether: add fields from > rte_pci_driver to rte_eth_dev and rte_eth_dev_data. >=20 >=20 >=20 > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Iremonger, > > Bernard > > Sent: Tuesday, September 01, 2015 12:39 PM > > To: Thomas Monjalon > > Cc: dev@dpdk.org > > Subject: Re: [dpdk-dev] [RFC PATCH 1/6] librte_ether: add fields from > rte_pci_driver to rte_eth_dev and rte_eth_dev_data. > > > > Hi THomas, > > > > > > > > > > @@ -424,7 +425,10 @@ rte_eth_dev_socket_id(uint8_t port_id) { > > > > if (!rte_eth_dev_is_valid_port(port_id)) > > > > return -1; > > > > - return rte_eth_devices[port_id].pci_dev->numa_node; > > > > + if (rte_eth_devices[port_id].dev_type =3D=3D RTE_ETH_DEV_PCI) > > > > + return rte_eth_devices[port_id].pci_dev->numa_node; > > > > + else > > > > + return rte_eth_devices[port_id].data->numa_node; > > > > > > Clearly not the way to go. > > > We should remove the special cases (PCI, PDEV, VDEV) instead of > > > adding more checks. > > > > The dev_type field is not new, just using it now to distinguish between > PCI and non PCI devices. > > > > I have moved some of the RTE_PCI_DRV flags to a new dev_flags field in > > struct rte_eth_dev{}, These flags are independent of the driver type > (PCI or non PCI). > > Do you have view on this new dev_flags field and macros? >=20 > What looks strange here to me, I that we now we duplicate some fields > here? > Let say for PCI devices numa_node would be precent in pci_dev and in data= , > right? > If there are some fields that are common for all device types (PCI, VDEV, > etc) why not to create some unified structure for them that would be used > by all device types and remove subsequent fields from pci_dev? > Or alternatively create a union of structs (one struct per device type)? > Then, as was pointed before, we wouldn't these branches by device_type at > all. > Konstantin >=20 I wouldn't worry so much about the duplicated data, so long as the ethdev A= PIs only have to look for the data in a single place when necessary. [Some data may = be naturally present in the pci_dev structure, for instance, and then be copied by the d= river into the common ethdev structure. If however, that copy and duplication can be a= voided, great!] /Bruce