Re: [dpdk-dev] [PATCH 7/7] bus/pci: support Windows with bifurcated drivers

[Narcisa Ana Maria Vasile <navasile@linux.microsoft.com>]

On Wed, Apr 22, 2020 at 10:27:47AM +0300, talshn@mellanox.com wrote:
> From: Tal Shnaiderman <talshn@mellanox.com>
> 
> Uses SetupAPI.h functions to scan PCI tree.
> Uses DEVPKEY_Device_Numa_Node to get the PCI Numa node.
> scanning currently supports types RTE_KDRV_NONE.
> 
> Signed-off-by: Tal Shnaiderman <talshn@mellanox.com>
> ---
>  drivers/bus/pci/windows/pci.c                | 342 ++++++++++++++++++++++++++-
>  lib/librte_eal/rte_eal_exports.def           |   1 +
>  lib/librte_eal/windows/include/rte_windows.h |   1 +
>  3 files changed, 342 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/bus/pci/windows/pci.c b/drivers/bus/pci/windows/pci.c
> index 7eff39173..d5ee938fa 100644
> --- a/drivers/bus/pci/windows/pci.c
> +++ b/drivers/bus/pci/windows/pci.c
> @@ -1,14 +1,24 @@
>  /* SPDX-License-Identifier: BSD-3-Clause
>   * Copyright 2020 Mellanox Technologies, Ltd
>   */
> +
> +static
> +int get_device_resource_info(HDEVINFO hDevInfo,
> +	PSP_DEVINFO_DATA pDeviceInfoData , struct rte_pci_device *dev)
> +{
> +	int  ret = -1;
> +	DEVPROPTYPE uPropertyType;
> +	DWORD uNumaNode;
> +	BOOL  bResult;
> +
> +	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();
> +			break;

Here 'ret' is correctly set to an error code, but after breaking out of the switch, it is overwritten to ERROR_SUCCESS.
Maybe 'goto end' instead of 'break'.

> +		}
> +		dev->device.numa_node = uNumaNode;
> +		/* mem_resource - Uneeded for RTE_KDRV_NONE */
> +		dev->mem_resource[0].phys_addr = 0;
> +		dev->mem_resource[0].len = 0;
> +		dev->mem_resource[0].addr = NULL;
> +		break;
> +	default:
> +		ret = -1;

Same thing here, ret is overwritten after breaking from the switch. 

> +		break;
> +	}
> +
> +	ret = ERROR_SUCCESS;
> +end:
> +	return ret;
> +}
> +
> +
> +/*