DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Kadam, Pallavi" <pallavi.kadam@intel.com>
To: Tal Shnaiderman <talshn@nvidia.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	NBU-Contact-Thomas Monjalon <thomas@monjalon.net>
Cc: "ranjit.menon@intel.com" <ranjit.menon@intel.com>,
	"John.Alexander@datapath.co.uk" <John.Alexander@datapath.co.uk>,
	"dmitry.kozliuk@gmail.com" <dmitry.kozliuk@gmail.com>,
	"Narcisa.Vasile@microsoft.com" <Narcisa.Vasile@microsoft.com>
Subject: Re: [dpdk-dev] [PATCH v5] bus/pci: netuio interface for windows
Date: Tue, 6 Oct 2020 16:31:52 -0700
Message-ID: <23b18aa9-cd8a-1df1-2310-bfa1f9b129ab@intel.com> (raw)
In-Reply-To: <DM6PR12MB43296561DE798C84C966BD5AA4320@DM6PR12MB4329.namprd12.prod.outlook.com>


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 <John.Alexander@datapath.co.uk>
>> Signed-off-by: Pallavi Kadam <pallavi.kadam@intel.com>
>> Reviewed-by: Ranjit Menon <ranjit.menon@intel.com>
>> ---
> [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)
>

  parent reply	other threads:[~2020-10-06 23:32 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-11  1:59 [dpdk-dev] [PATCH] " Pallavi Kadam
2020-09-13 18:42 ` Tal Shnaiderman
2020-09-15 20:13   ` Ranjit Menon
2020-09-15 23:28 ` [dpdk-dev] [PATCH v2] " Pallavi Kadam
2020-09-16  1:54   ` Stephen Hemminger
2020-09-17  0:48     ` Ranjit Menon
2020-09-17  1:20       ` Ranjit Menon
2020-09-21 21:08   ` [dpdk-dev] [PATCH v3] " Pallavi Kadam
2020-09-22  3:05     ` [dpdk-dev] [PATCH v4] " Pallavi Kadam
2020-09-25  1:53       ` [dpdk-dev] [PATCH v5] " Pallavi Kadam
2020-09-29  8:28         ` Tal Shnaiderman
2020-09-29 17:29           ` Ranjit Menon
2020-09-30  7:58             ` Tal Shnaiderman
2020-10-06 23:31           ` Kadam, Pallavi [this message]
2020-10-06 21:57         ` [dpdk-dev] [PATCH v6] " Pallavi Kadam
2020-10-08 17:46           ` Tal Shnaiderman
2020-10-08 18:56           ` [dpdk-dev] [PATCH v7] " Pallavi Kadam
2020-10-08 21:50             ` Tal Shnaiderman
2020-10-09  2:12             ` Narcisa Ana Maria Vasile
2020-10-14 20:27               ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=23b18aa9-cd8a-1df1-2310-bfa1f9b129ab@intel.com \
    --to=pallavi.kadam@intel.com \
    --cc=John.Alexander@datapath.co.uk \
    --cc=Narcisa.Vasile@microsoft.com \
    --cc=dev@dpdk.org \
    --cc=dmitry.kozliuk@gmail.com \
    --cc=ranjit.menon@intel.com \
    --cc=talshn@nvidia.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git