From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id A5E22E72 for ; Wed, 30 Sep 2015 18:33:12 +0200 (CEST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga103.fm.intel.com with ESMTP; 30 Sep 2015 09:33:11 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,613,1437462000"; d="scan'208";a="781063133" Received: from irsmsx102.ger.corp.intel.com ([163.33.3.155]) by orsmga001.jf.intel.com with ESMTP; 30 Sep 2015 09:33:10 -0700 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.216]) by IRSMSX102.ger.corp.intel.com ([169.254.2.160]) with mapi id 14.03.0248.002; Wed, 30 Sep 2015 17:33:09 +0100 From: "Iremonger, Bernard" To: "Richardson, Bruce" , Neil Horman Thread-Topic: [dpdk-dev] [PATCH 02/20] librte_ether: add fields from rte_pci_driver to rte_eth_dev_data Thread-Index: AQHQ+4JNeWloV3Jhdku9Ofblv/SIDJ5U/geAgAAn2UA= Date: Wed, 30 Sep 2015 16:33:07 +0000 Message-ID: <8CEF83825BEC744B83065625E567D7C219F632D8@IRSMSX108.ger.corp.intel.com> References: <1443445418-18498-1-git-send-email-bernard.iremonger@intel.com> <1443445418-18498-3-git-send-email-bernard.iremonger@intel.com> <20150929190812.GA3154@hmsreliant.think-freely.org> <20150930095603.GA10264@bricha3-MOBL3> <20150930131448.GA32524@hmsreliant.think-freely.org> <20150930132124.GA11552@bricha3-MOBL3> In-Reply-To: <20150930132124.GA11552@bricha3-MOBL3> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH 02/20] librte_ether: add fields from rte_pci_driver to rte_eth_dev_data 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: Wed, 30 Sep 2015 16:33:13 -0000 Hi Neil > > > > > +++ b/lib/librte_ether/rte_ethdev.h > > > > > @@ -1635,8 +1635,23 @@ struct rte_eth_dev_data { > > > > > all_multicast : 1, /**< RX all multicast mode ON(1) / > OFF(0). */ > > > > > dev_started : 1, /**< Device state: STARTED(1) / > STOPPED(0). */ > > > > > lro : 1; /**< RX LRO is ON(1) / OFF(0) */ > > > > > + uint32_t dev_flags; /**< Flags controlling handling of device. > */ > > > > > + enum rte_kernel_driver kdrv; /**< Kernel driver > passthrough */ > > > > Why add this here? The ennumerated driver types are all variants > > > > on PCI bus types. Not sure why the ethernet interface needs to > > > > know this info > > > > > > > > > + int numa_node; > > > > Ditto, this seems like information that is only relevant if the > > > > device is on a physical bus (i.e. virual devices are likely to not > > > > have a numa node) > > > > > > > Actually, I disagree. For some virtual devices they will have a numa > > > node. For ring or other virtual PMDs the numa node will be the node > > > on which the ring / mempool etc. memory is allocated on, and can be o= f > relevance. > > > > > > /Bruce > > > > > > > I think its fairly clear that some devices (including virtual ones) > > have some relevant relation to a numa_node (There are even some that > > have no numa_node, for which a -1 value makes some sense). That said, > > there are just as many that don't have a relevant numa_node. > > > > 1) There are some drivers for which numa_node make no sense > > (regardless of > > value): > > * af_packet - The numa node is at best determined at run time by the > > interface the socket is bound to > > > > * pcap - same as af_packet > > > > * bonding - multiple interfaces mean multiple numa_nodes, any value > > set here is just as likely to be wrong as right > > > > * mpipe - no real large memory area to associate with a numa node > > > > * virtio - uses iopl for communication, and cannot know its numa_node > > > > * vmxnet3 - same concept as virtio > > > > * xenvirt - same as vmxnet3 > > > > I think its better that you store numa locality information in a pmd's > > private bus data, and export it to applications via a device method. > > that provides the flexibility to tell the application that there is no > > numa locality for a device (by not implementing the method), without > > having to expose an unset data field to the application. > > > > Neil > > >=20 > Sure, that could work. > However, is it really worthwhile asking drivers to implement a new ethdev > API function, rather than just having them set the numa node field correc= tly > in the init function? >=20 > /Bruce The four fields below have been added to struct rte_eth_dev_data uint32_t dev_flags; /**< Flags controlling handling of device. */ enum rte_kernel_driver kdrv; /**< Kernel driver passthrough */ int numa_node; const char *drv_name; The data for these fields is available in the struct rte_pci_device. In order to remove the pci_device from the vdev PMD's this data needs to b= e available in the eth_dev. A new function rte_eth_copy_dev_info() has been added to the eth_dev for us= e by the pdevs to copy this data from the pci_device to the ethdev. In the vdevs the pci_device has been removed and the new fields are set up = directly in the rte_driver.init function. The numa_node is already initialised in the following vdev PMD's: af_packet - initialized from socket_id bonding - initialized from socket_id mpipe - initialized from instance null - initialized from socket_id pcap - initialized from socket_id ring - initialized form socket_id xenvirt - initialized from socket_id Regards, Bernard.