|SUCCESS| Windows: fix core count on NUMA with more than 64 cores per node

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/163978

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-05-14

Reviewing patch: Windows: fix core count on NUMA with more than 64 cores per node

commit d2a2c2d091
Author: Gena Tertychnyi <genter at microsoft.com>
Subject: [PATCH] Windows: fix core count on NUMA with more than 64 cores per node

This patch fixes core enumeration on Windows NUMA systems with more than
64 cores per node by using NumaNode.GroupMasks[] array and RelationAll
instead of RelationNumaNode when querying processor information.

> diff --git a/lib/eal/windows/eal_lcore.c b/lib/eal/windows/eal_lcore.c
> index a498044620..d2a2c2d091 100644
> --- a/lib/eal/windows/eal_lcore.c
> +++ b/lib/eal/windows/eal_lcore.c

[ ... ]

> +/*
> + * NUMA_NODE_RELATIONSHIP layout differs:
> + *  - MSVC + modern SDK: GroupCount + GroupMasks[]
> + *  - MinGW-w64: only GroupMask (MinGW headers lag behind Windows SDK ABI changes)
> + */
> +#ifdef RTE_TOOLCHAIN_GCC
> +#define EAL_NUMA_GROUP_COUNT(numa) (RTE_SET_USED(numa), 1)
> +#define EAL_NUMA_GROUP_MASKS(numa) (&((numa).GroupMask))
> +#else
> +#define EAL_NUMA_GROUP_COUNT(numa) ((numa).GroupCount)
> +#define EAL_NUMA_GROUP_MASKS(numa) ((numa).GroupMasks)
> +#endif
> +
>  struct lcore_map {
> -	uint8_t socket_id;
> -	uint8_t core_id;
> +	unsigned int socket_id;
> +	unsigned int core_id;
>  };
>  
>  struct socket_map {
>  	uint16_t node_id;
> +	unsigned int lcore_count;
>  };

Does changing socket_id and core_id from uint8_t to unsigned int cause
any ABI compatibility issues in the cpu_map structure? Are these fields
ever exposed through any API or stored persistently?

> @@ -132,20 +150,33 @@ eal_create_lcore_map(const SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX *info)
>  			return true;
>  
>  		cpu_map.sockets[socket_id].node_id = node_id;
> +		cpu_map.sockets[socket_id].lcore_count = 0;
>  		cpu_map.socket_count++;
>  	}
>  
> -	for (i = 0; i < EAL_PROCESSOR_GROUP_SIZE; i++) {
> -		if ((cores->Mask & ((KAFFINITY)1 << i)) == 0)
> -			continue;
> +	/* Old Windows versions report NUMA nodes with GroupCount == 0 */
> +	if (group_count == 0) {
> +		group_count = 1;
> +		group_masks = &info->NumaNode.GroupMask;
> +	}

Does this code correctly handle the MinGW case where GroupCount doesn't
exist? EAL_NUMA_GROUP_COUNT(info->NumaNode) with RTE_TOOLCHAIN_GCC
returns 1, but then this code overwrites group_masks with
&info->NumaNode.GroupMask, potentially setting it twice (once in
EAL_NUMA_GROUP_MASKS and once here).

> +	for (group_no = 0; group_no < group_count; group_no++) {
> +		const GROUP_AFFINITY *cores = &group_masks[group_no];
> +		for (i = 0; i < EAL_PROCESSOR_GROUP_SIZE; i++) {
> +			if ((cores->Mask & ((KAFFINITY)1 << i)) == 0)
> +				continue;
> +
> +			if (cpu_map.lcore_count == RTE_DIM(cpu_map.lcores))
> +				return true;
>  
> -		lcore = &cpu_map.lcores[cpu_map.lcore_count];
> -		lcore->socket_id = socket_id;
> -		lcore->core_id = cores->Group * EAL_PROCESSOR_GROUP_SIZE + i;
> -		cpu_map.lcore_count++;
> +			lcore = &cpu_map.lcores[cpu_map.lcore_count];
> +			lcore->socket_id = socket_id;
> +
> +			/* core_id within the socket */
> +			lcore->core_id = cpu_map.sockets[socket_id].lcore_count;
> +			cpu_map.sockets[socket_id].lcore_count++;
> +			cpu_map.lcore_count++;
> +		}
>  	}

The new core_id assignment uses per-socket lcore_count instead of the
global processor group position. Does this change the semantics of
core_id for existing code that may depend on the previous calculation?

> @@ -181,7 +213,7 @@ eal_create_cpu_map(void)
>  	}
>  
>  	if (!GetLogicalProcessorInformationEx(
> -			RelationNumaNode, infos, &infos_size)) {
> +			RelationAll, infos, &infos_size)) {
>  		log_early("Cannot get NUMA node information, error %lu\n",
>  			GetLastError());
>  		rte_errno = EINVAL;
> @@ -191,9 +223,11 @@ eal_create_cpu_map(void)
>  
>  	info = infos;
>  	while ((uint8_t *)info - (uint8_t *)infos < infos_size) {
> -		if (eal_create_lcore_map(info)) {
> -			full = true;
> -			break;
> +		if (info->Relationship == RelationNumaNode) {
> +			if (eal_create_lcore_map(info)) {
> +				full = true;
> +				break;
> +			}
>  		}

When using RelationAll, does this code skip non-NUMA relationship types
correctly? Are there any relationship types that should be processed but
are now being silently ignored?