On 2020-04-28 12:11 GMT+0300 tal...@mellanox.com wrote: [snip] > + switch (dev->kdrv) { > + case RTE_KDRV_NONE: > + /* Get NUMA node using DEVPKEY_Device_Numa_Node */ > + bResult = SetupDiGetDevicePropertyW(hDevInfo, pDeviceInfoData, > + &DEVPKEY_Device_Numa_Node, &uPropertyType, > + (BYTE *)&uNumaNode, sizeof(uNumaNode), NULL, 0); > + if (!bResult) { > + ret = GetLastError(); > + goto end; > + } > + dev->device.numa_node = uNumaNode;
Note: NUMA node != socket ID, but this field is used as socket ID by PMDs. I suggest adding Windows-only EAL API to do the translation. [snip] > + /* We now have the various hw IDs in tokens in the str[] array */ > + /* Isolate the numerical IDs - '_' as the delimiter */ > + if (parse_hardware_ids(str[0], strlen(str[0]), vendorID, NULL)) > + goto end; > + > + if (parse_hardware_ids(str[1], strlen(str[1]), deviceID, NULL)) > + goto end; > + > + if (parse_hardware_ids(str[2], strlen(str[2]), subdeviceID, > + subvendorID)) { > + goto end; > + } You could avoid hand-written string parsing by using sscanf(), because hardware ID formats are fixed, limited, and documented: https://docs.microsoft.com/en-us/windows-hardware/drivers/install/identifiers-for-pci-devices [snip] > + > + bResult = SetupDiGetDeviceRegistryPropertyA(hDevInfo, pDeviceInfoData, > + SPDRP_LOCATION_INFORMATION, NULL, (BYTE *)&strPCIDeviceInfo, > + sizeof(strPCIDeviceInfo), NULL); > + > + /* Some devices don't have location information - simply return 0 */ > + /* Also, if we don't find 'PCI' in the description, we'll skip it */ > + if (!bResult) { > + ret = (GetLastError() == ERROR_INVALID_DATA) ? ERROR_CONTINUE > + : -1; > + goto end; > + } else if (!strstr(strPCIDeviceInfo, "PCI")) { > + ret = ERROR_CONTINUE; > + goto end; > + } You could get PCI address without parsing strings with SPDRP_BUSNUMBER and SPDRP_ADDRESS. > + > + struct rte_pci_addr addr; > + > + if (parse_pci_addr_format((const char *)&strPCIDeviceInfo, > + sizeof(strPCIDeviceInfo), &addr) != 0) { > + ret = -1; > + goto end; > + } > + > + dev->addr.domain = addr.domain; > + dev->addr.bus = addr.bus; > + dev->addr.devid = addr.devid; > + dev->addr.function = addr.function; Why not dev->addr = addr? [snip] > diff --git a/lib/librte_eal/rte_eal_exports.def > b/lib/librte_eal/rte_eal_exports.def > index edbb6b277..7a0468a02 100644 > --- a/lib/librte_eal/rte_eal_exports.def > +++ b/lib/librte_eal/rte_eal_exports.def > @@ -18,6 +18,7 @@ EXPORTS > rte_eal_tailq_lookup > rte_eal_tailq_register > rte_eal_using_phys_addrs > + rte_find_numerical_value > rte_free > rte_log > rte_malloc This belongs to the patch that added the API, doesn't it? -- Dmitry Kozlyuk