|SUCCESS| [v3,6/6] eal/memory: add page size VA limits EAL parameter

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

I'll review this DPDK patch series focusing on correctness, coding style, API requirements, and guideline violations.

---

commit 0f4f34f49024583025a28640c770235da6ae1893
Author: Anatoly Burakov <anatoly.burakov at intel.com>

    eal: reject non-numeric input in str to size

This patch adds validation to rte_str_to_size() to reject non-numeric 
input and moves test coverage from malloc tests to string_autotest.

> diff --git a/lib/eal/common/eal_common_string_fns.c b/lib/eal/common/eal_common_string_fns.c
> index fa87831c3a..e0dc48bd80 100644
> --- a/lib/eal/common/eal_common_string_fns.c
> +++ b/lib/eal/common/eal_common_string_fns.c
> @@ -85,6 +85,10 @@ rte_str_to_size(const char *str)
>  
>  	errno = 0;
>  	size = strtoull(str, &endptr, 0);
> +	if (endptr == str) {
> +		errno = EINVAL;
> +		return 0;
> +	}
>  	if (errno)
>  		return 0;

Does this code handle whitespace-only strings correctly? The test suite 
includes "   " as an invalid input case. When strtoull() is called with 
"   " it may skip leading whitespace and then fail to parse anything, 
setting endptr to point past the whitespace but not equal to str. Can 
this lead to the validation being bypassed?

---

commit 449777153a6be94602cf04bc8e3617a5ec92ef2a
Author: Anatoly Burakov <anatoly.burakov at intel.com>

    eal/memory: remove per-list segment and memory limits

This patch removes multi-list logic and consolidates to single segment 
list per memory type.

> diff --git a/lib/eal/common/eal_common_dynmem.c b/lib/eal/common/eal_common_dynmem.c
> index 8f51d6dd4a..c25197f2e9 100644
> --- a/lib/eal/common/eal_common_dynmem.c
> +++ b/lib/eal/common/eal_common_dynmem.c

[ ... ]

> @@ -105,8 +108,7 @@ eal_dynmem_memseg_lists_init(void)
>  	max_mem = (uint64_t)RTE_MAX_MEM_MB << 20;
>  	max_mem_per_type = RTE_MIN((uint64_t)RTE_MAX_MEM_MB_PER_TYPE << 20,
>  			max_mem / n_memtypes);

Is there potential for division by zero at eal_dynmem_memseg_lists_init() 
if n_memtypes is zero? The check for n_memtypes == 0 appears after this 
calculation in the original code.

> @@ -113,91 +103,47 @@ eal_dynmem_memseg_lists_init(void)
> -	/* create all segment lists */
> -	for (cur_seglist = 0; cur_seglist < n_seglists; cur_seglist++) {
> -		if (msl_idx >= RTE_MAX_MEMSEG_LISTS) {
> -			EAL_LOG(ERR,
> -				"No more space in memseg lists, please increase RTE_MAX_MEMSEG_LISTS");
> -			goto out;
> -		}
> -		msl = &mcfg->memsegs[msl_idx++];
> -
> -		if (eal_memseg_list_init(msl, pagesz, n_segs,
> -				socket_id, cur_seglist, true))
> -			goto out;
> -
> -		if (eal_memseg_list_alloc(msl, 0)) {
> -			EAL_LOG(ERR, "Cannot allocate VA space for memseg list");
> -			goto out;
> -		}
> -	}
> +	if (eal_memseg_list_init(msl, pagesz, n_segs, socket_id, msl_idx, true))
> +		goto out;
> +
> +	if (eal_memseg_list_alloc(msl, 0)) {
> +		EAL_LOG(ERR, "Cannot allocate VA space for memseg list");
> +		goto out;

Does this code leak resources from eal_memseg_list_init() if 
eal_memseg_list_alloc() fails? Should there be cleanup of previously 
initialized memseg lists?

---

commit fe2f88f7645f745af7fec6565097b2078c3bcdcb
Author: Anatoly Burakov <anatoly.burakov at intel.com>

    eal/memory: allocate all VA space in one go

This patch allocates all VA space in a single reservation and assigns 
portions to memseg lists.

> diff --git a/lib/eal/common/eal_common_dynmem.c b/lib/eal/common/eal_common_dynmem.c
> index c25197f2e9..40cc989ed9 100644
> --- a/lib/eal/common/eal_common_dynmem.c
> +++ b/lib/eal/common/eal_common_dynmem.c

[ ... ]

> @@ -144,6 +173,15 @@ eal_dynmem_memseg_lists_init(void)
>  	/* we're successful */
>  	ret = 0;
>  out:
> +	if (ret != 0) {
> +		if (mem_va_addr != NULL)
> +			eal_mem_free(mem_va_addr, mem_va_len);
> +	} else {
> +		/* store the VA space data in shared config */
> +		mcfg->mem_va_addr = (uintptr_t)mem_va_addr;
> +		mcfg->mem_va_len = mem_va_len;
> +		mcfg->mem_va_page_sz = mem_va_page_sz;
> +	}
>  	return ret;

Does the error cleanup path at eal_dynmem_memseg_lists_init() also need 
to clean up any memseg lists that were initialized via 
eal_memseg_list_assign() before the failure occurred?

> diff --git a/lib/eal/linux/eal_memory.c b/lib/eal/linux/eal_memory.c
> index 691d8eb3cc..b4960ffa54 100644
> --- a/lib/eal/linux/eal_memory.c
> +++ b/lib/eal/linux/eal_memory.c

[ ... ]

> +static int __rte_unused
> +memseg_secondary_init_dynmem(void)
> +{
> +	struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
> +	int msl_idx = 0;
> +	struct rte_memseg_list *msl;
> +	void *mem_va_addr;
> +	size_t mem_va_len;
> +
> +	if (mcfg->mem_va_addr == 0 || mcfg->mem_va_len == 0 ||
> +			mcfg->mem_va_page_sz == 0) {
> +		EAL_LOG(ERR, "Missing shared dynamic memory VA range from primary process");
> +		return -1;
> +	}
> +
> +	mem_va_addr = (void *)(uintptr_t)mcfg->mem_va_addr;
> +	mem_va_len = mcfg->mem_va_len;
> +
> +	if (eal_get_virtual_area(mem_va_addr, &mem_va_len,
> +			mcfg->mem_va_page_sz, 0, 0) == NULL) {
> +		EAL_LOG(ERR, "Cannot reserve VA space for hugepage memory");
> +		return -1;
> +	}

Does memseg_secondary_init_dynmem() need to clean up the VA reservation 
from eal_get_virtual_area() if the subsequent rte_fbarray_attach() or 
eal_memseg_list_assign() operations fail?

---

commit e5f69c56cc83cb73c443c4f992a325b9e6f1081e
Author: Anatoly Burakov <anatoly.burakov at intel.com>

    eal/memory: get rid of global VA space limits

This patch removes global memory limits and lets VA usage scale with 
NUMA nodes.

No specific correctness issues identified in this patch.

---

commit fd1309dd3e792b5fd97ad3d46872f083b5338554
Author: Anatoly Burakov <anatoly.burakov at intel.com>

    eal/memory: store default segment limits in config

This patch moves memory limit configuration to internal config structure.

No specific correctness issues identified in this patch.

---

commit 1dc272787d685524a1b3abea4fb415a44af3ed9e
Author: Anatoly Burakov <anatoly.burakov at intel.com>

    eal/memory: add page size VA limits EAL parameter

This patch adds --pagesz-mem option for per-page-size memory limits.

> diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
> index 67a7c6fb70..1049838d73 100644
> --- a/lib/eal/common/eal_common_options.c
> +++ b/lib/eal/common/eal_common_options.c

[ ... ]

> +static int
> +eal_parse_pagesz_mem(char *strval, struct internal_config *internal_cfg)
> +{
> +	char strval_cpy[1024];
> +	char *fields[3];
> +	char *pagesz_str, *mem_str;
> +	int arg_num;
> +	int len;
> +	unsigned int i;
> +	uint64_t pagesz, mem_limit;
> +	struct pagesz_mem_override *pmo;
> +
> +	len = strnlen(strval, 1024);
> +	if (len >= 1024) {
> +		EAL_LOG(ERR, "--pagesz-mem parameter is too long");
> +		return -1;
> +	}

Does eal_parse_pagesz_mem() have a buffer overflow risk? The magic 
constant 1024 is used for both the local buffer size and strnlen() 
check, but if len equals 1024, the subsequent rte_strlcpy() may not 
null-terminate strval_cpy properly since strnlen() returns 1024 when 
the string is >= 1024 bytes.