From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id A96D9A04BC; Wed, 7 Oct 2020 01:32:00 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 3E07EE07; Wed, 7 Oct 2020 01:31:59 +0200 (CEST) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id D5AC4160 for ; Wed, 7 Oct 2020 01:31:56 +0200 (CEST) IronPort-SDR: m36/ZnoMfGaO6NucikGbY6Fwmp2YFBJQ75Mx/XOBAbm11EgOWLjz6JwNisDce75kFNvTgAjCxW uMo2Buh5iEGw== X-IronPort-AV: E=McAfee;i="6000,8403,9766"; a="163974122" X-IronPort-AV: E=Sophos;i="5.77,344,1596524400"; d="scan'208";a="163974122" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Oct 2020 16:31:55 -0700 IronPort-SDR: 38Zu+gLxtWwe56myjKVBzg/6lanTCrB2OscCQMAYCBFikkkqWV13WUPi9Omsr7I3Zp7sGI1pWh hZ/i9iKlLI0A== X-IronPort-AV: E=Sophos;i="5.77,344,1596524400"; d="scan'208";a="348741348" Received: from pkadam-mobl1.amr.corp.intel.com (HELO [10.209.179.53]) ([10.209.179.53]) by fmsmga002-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Oct 2020 16:31:54 -0700 To: Tal Shnaiderman , "dev@dpdk.org" , NBU-Contact-Thomas Monjalon Cc: "ranjit.menon@intel.com" , "John.Alexander@datapath.co.uk" , "dmitry.kozliuk@gmail.com" , "Narcisa.Vasile@microsoft.com" References: <20200922030531.6928-1-pallavi.kadam@intel.com> <20200925015327.2916-1-pallavi.kadam@intel.com> From: "Kadam, Pallavi" Message-ID: <23b18aa9-cd8a-1df1-2310-bfa1f9b129ab@intel.com> Date: Tue, 6 Oct 2020 16:31:52 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.12.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-GB Subject: Re: [dpdk-dev] [PATCH v5] bus/pci: netuio interface for windows X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list 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 9/29/2020 1:28 AM, Tal Shnaiderman wrote: >> Subject: [PATCH v5] bus/pci: netuio interface for windows >> >> This patch adds implementations to probe PCI devices bound to netuio with >> the help of "netuio" class device changes. >> Now Windows will support both "netuio" and "net" device class and can set >> kernel driver type based on the device class selection. >> >> Note: Few definitions and structures have been copied from >> netuio_interface.h file from ("[v4] windows/netuio: add Windows NetUIO >> kernel driver") series and this will be fixed once the exact path for netuio >> source code is known. >> >> v5 changes: >> Changed when netuio driver handle is closed >> >> v4 changes: >> Removed 'reserved' member as it is not used >> >> v3 changes: >> Removed the casts >> >> v2 changes: >> - Moved all netuio specific definitions and functions to >> pci_netuio.c and pci_netuio.h files >> - Added a single function call to scan all the devices >> >> Signed-off-by: John Alexander >> Signed-off-by: Pallavi Kadam >> Reviewed-by: Ranjit Menon >> --- > [snip] > >> +/* >> + * get device resource information by sending ioctl to netuio driver >> +*/ int get_netuio_device_info(HDEVINFO dev_info, PSP_DEVINFO_DATA >> +dev_info_data, >> + struct rte_pci_device *dev) >> +{ > [snip] > >> + /* get NUMA node using DEVPKEY_Device_Numa_Node */ >> + res = SetupDiGetDevicePropertyW(dev_info, dev_info_data, >> + &DEVPKEY_Device_Numa_Node, &property_type, >> + (BYTE *)&numa_node, sizeof(numa_node), NULL, 0); >> + if (!res) { >> + RTE_LOG_WIN32_ERR( >> + >> "SetupDiGetDevicePropertyW(DEVPKEY_Device_Numa_Node)"); >> + goto end; >> + } >> + dev->device.numa_node = numa_node; > The is the same code RTE_PCI_KDRV_NONE devices are using to get numa node id, I suggest removing it and changing the original code in get_device_resource_info to work on both RTE_PCI_KDRV_NONE and RTE_PCI_KDRV_NIC_UIO Thanks, Tal. We have modified get numa node id code to support both RTE_PCI_KDRV_NONE and RTE_PCI_KDRV_NIC_UIO and split up the get_netuio_device_info function into multiple functions in v6. > > In general: > > Regarding the issue Ranjit mentioned in the last community call on duplicated detection of netuio devices both as GUID_DEVCLASS_NETUIO and GUID_DEVCLASS_NET, in pci_scan_one there is actually code that take cares of duplicated detection by leaving only one in the device list, up until nor I saw it in action only in virtualization where BDF of several devices was equal. > > Assuming this is resolved with the addition of segment id as domain id the change we need is to give precedence to GUID_DEVCLASS_NETUIO, something like: > > @@ -372,11 +372,15 @@ pci_scan_one(HDEVINFO dev_info, PSP_DEVINFO_DATA device_info_data) > } else if (ret < 0) { > rte_pci_insert_device(dev2, dev); > } else { /* already registered */ > + if (IsEqualGUID(&(device_info_data.ClassGuid), > + &GUID_DEVCLASS_NETUIO) { > dev2->kdrv = dev->kdrv; > dev2->max_vfs = dev->max_vfs; > memmove(dev2->mem_resource, dev->mem_resource, > sizeof(dev->mem_resource)); > + } > free(dev); > } > return 0; > } > > The other pci.c changes of the patch LGTM, I'd prefer someone from MSFT to review in depth the pci_netuio.c/.h, the only other comment I have on it is a suggestion to break down the get_netuio_device_info function into several helper functions (get_netuio_device_information_set, allocate_netuio_interface_detail, etc) >