[PATCH] eal: handle sysconf() negative return value

[PATCH] eal: handle sysconf() negative return value

[Morten Brørup]

Coverity reports some defects, where the root cause seems to be negative
return value from sysconf() not being handled.

rte_mem_page_size() has been updated to handle negative return value from
sysconf(_SC_PAGESIZE).

All other functions calling sysconf(_SC_PAGESIZE) directly have been
updated to call rte_mem_page_size() instead.

At this time, no other calls to sysconf() have been found in DPDK.

Also, an unrelated drive-by minor fix in eal_mem_set_dump():
When madvise() failed, an incorrect reason was logged.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/eal/freebsd/eal.c          |  3 ++-
 lib/eal/freebsd/eal_memory.c   |  7 +++----
 lib/eal/linux/eal.c            |  3 ++-
 lib/eal/unix/eal_unix_memory.c | 16 ++++++++++++++--
 lib/vhost/vduse.c              |  3 ++-
 5 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index d6fffa2170..ce7f5c3d65 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -26,6 +26,7 @@
 #include <rte_launch.h>
 #include <rte_eal.h>
 #include <rte_eal_memconfig.h>
+#include <rte_eal_paging.h>
 #include <rte_errno.h>
 #include <rte_per_lcore.h>
 #include <rte_lcore.h>
@@ -98,7 +99,7 @@ rte_eal_config_create(void)
 	struct rte_config *config = rte_eal_get_configuration();
 	const struct internal_config *internal_conf =
 		eal_get_internal_configuration();
-	size_t page_sz = sysconf(_SC_PAGE_SIZE);
+	size_t page_sz = rte_mem_page_size();
 	size_t cfg_len = sizeof(struct rte_mem_config);
 	size_t cfg_len_aligned = RTE_ALIGN(cfg_len, page_sz);
 	void *rte_mem_cfg_addr, *mapped_mem_cfg_addr;
diff --git a/lib/eal/freebsd/eal_memory.c b/lib/eal/freebsd/eal_memory.c
index 3b72e13506..6d3d46a390 100644
--- a/lib/eal/freebsd/eal_memory.c
+++ b/lib/eal/freebsd/eal_memory.c
@@ -11,6 +11,7 @@
 #include <fcntl.h>
 
 #include <rte_eal.h>
+#include <rte_eal_paging.h>
 #include <rte_errno.h>
 #include <rte_log.h>
 #include <rte_string_fns.h>
@@ -22,8 +23,6 @@
 #include "eal_memcfg.h"
 #include "eal_options.h"
 
-#define EAL_PAGE_SIZE (sysconf(_SC_PAGESIZE))
-
 uint64_t eal_get_baseaddr(void)
 {
 	/*
@@ -191,7 +190,7 @@ rte_eal_hugepage_init(void)
 			addr = mmap(addr, page_sz, PROT_READ|PROT_WRITE,
 					MAP_SHARED | MAP_FIXED,
 					hpi->lock_descriptor,
-					j * EAL_PAGE_SIZE);
+					j * rte_mem_page_size());
 			if (addr == MAP_FAILED) {
 				EAL_LOG(ERR, "Failed to mmap buffer %u from %s",
 						j, hpi->hugedir);
@@ -243,7 +242,7 @@ attach_segment(const struct rte_memseg_list *msl, const struct rte_memseg *ms,
 
 	addr = mmap(ms->addr, ms->len, PROT_READ | PROT_WRITE,
 			MAP_SHARED | MAP_FIXED, wa->fd_hugepage,
-			wa->seg_idx * EAL_PAGE_SIZE);
+			wa->seg_idx * rte_mem_page_size());
 	if (addr == MAP_FAILED || addr != ms->addr)
 		return -1;
 	wa->seg_idx++;
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 20f777b8b0..ad6de9b7ee 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -31,6 +31,7 @@
 #include <rte_launch.h>
 #include <rte_eal.h>
 #include <rte_eal_memconfig.h>
+#include <rte_eal_paging.h>
 #include <rte_errno.h>
 #include <rte_lcore.h>
 #include <rte_service_component.h>
@@ -179,7 +180,7 @@ static int
 rte_eal_config_create(void)
 {
 	struct rte_config *config = rte_eal_get_configuration();
-	size_t page_sz = sysconf(_SC_PAGE_SIZE);
+	size_t page_sz = rte_mem_page_size();
 	size_t cfg_len = sizeof(*config->mem_config);
 	size_t cfg_len_aligned = RTE_ALIGN(cfg_len, page_sz);
 	void *rte_mem_cfg_addr, *mapped_mem_cfg_addr;
diff --git a/lib/eal/unix/eal_unix_memory.c b/lib/eal/unix/eal_unix_memory.c
index c540f1e838..51de6780f2 100644
--- a/lib/eal/unix/eal_unix_memory.c
+++ b/lib/eal/unix/eal_unix_memory.c
@@ -85,7 +85,7 @@ eal_mem_set_dump(void *virt, size_t size, bool dump)
 	int ret = madvise(virt, size, flags);
 	if (ret) {
 		EAL_LOG(DEBUG, "madvise(%p, %#zx, %d) failed: %s",
-				virt, size, flags, strerror(rte_errno));
+				virt, size, flags, strerror(errno));
 		rte_errno = errno;
 	}
 	return ret;
@@ -141,8 +141,20 @@ rte_mem_page_size(void)
 {
 	static size_t page_size;
 
-	if (!page_size)
+	if (page_size == 0) {
+		/*
+		 * When the sysconf value cannot be determined, sysconf()
+		 * returns -1 without setting errno.
+		 * To distinguish an indeterminate value from an error,
+		 * clear errno before calling sysconf(), and check whether
+		 * errno has been set if sysconf() returns -1.
+		 */
+		errno = 0;
 		page_size = sysconf(_SC_PAGESIZE);
+		if ((ssize_t)page_size < 0)
+			rte_panic("sysconf(_SC_PAGESIZE) failed: %s",
+					errno == 0 ? "Indeterminate" : strerror(errno));
+	}
 
 	return page_size;
 }
diff --git a/lib/vhost/vduse.c b/lib/vhost/vduse.c
index a2a7d73388..9de7f04a4f 100644
--- a/lib/vhost/vduse.c
+++ b/lib/vhost/vduse.c
@@ -16,6 +16,7 @@
 #include <sys/stat.h>
 
 #include <rte_common.h>
+#include <rte_eal_paging.h>
 #include <rte_thread.h>
 
 #include "fd_man.h"
@@ -690,7 +691,7 @@ vduse_device_create(const char *path, bool compliant_ol_flags)
 		dev_config->vendor_id = 0;
 		dev_config->features = features;
 		dev_config->vq_num = total_queues;
-		dev_config->vq_align = sysconf(_SC_PAGE_SIZE);
+		dev_config->vq_align = rte_mem_page_size();
 		dev_config->config_size = sizeof(struct virtio_net_config);
 		memcpy(dev_config->config, &vnet_config, sizeof(vnet_config));
 
-- 
2.43.0

[PATCH v2] eal: handle sysconf(_SC_PAGESIZE) negative return value

[Morten Brørup]

eal: handle sysconf(_SC_PAGESIZE) negative return value

Coverity reports some defects, where the root cause seems to be negative
return value from sysconf(_SC_PAGESIZE) not being handled.

rte_mem_page_size() was updated to handle negative return value from
sysconf(_SC_PAGESIZE).

All other functions calling sysconf(_SC_PAGESIZE) directly were updated to
call rte_mem_page_size() instead.

At this time, no other calls to sysconf(_SC_PAGESIZE) have been found in
the DPDK libraries.

PS: "_SC_PAGESIZE" has the alias "_SC_PAGE_SIZE". Both are covered here.

Moved the PMU library to the correct location in the meson.build file; it
depends on the EAL, not vice versa.

And an unrelated drive-by minor fix in eal_mem_set_dump():
When madvise() failed, an incorrect reason was logged.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
v2:
* Clarify in title and description that only sysconf(_SC_PAGESIZE) is
  covered, and in description that only DPDK libs have been patched.
* Include new PMU library in patch after rebasing.
v1:
* Improve comment about errno for indeterminate sysconf() values.
  (Stephen)
---
 lib/eal/freebsd/eal.c          |  3 ++-
 lib/eal/freebsd/eal_memory.c   |  7 +++----
 lib/eal/linux/eal.c            |  3 ++-
 lib/eal/unix/eal_unix_memory.c | 16 ++++++++++++++--
 lib/meson.build                |  2 +-
 lib/pmu/pmu.c                  |  8 ++++----
 lib/vhost/vduse.c              |  3 ++-
 7 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index 82382d6e8f..c1ab8d86d2 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -26,6 +26,7 @@
 #include <rte_launch.h>
 #include <rte_eal.h>
 #include <rte_eal_memconfig.h>
+#include <rte_eal_paging.h>
 #include <rte_errno.h>
 #include <rte_per_lcore.h>
 #include <rte_lcore.h>
@@ -98,7 +99,7 @@ rte_eal_config_create(void)
 	struct rte_config *config = rte_eal_get_configuration();
 	const struct internal_config *internal_conf =
 		eal_get_internal_configuration();
-	size_t page_sz = sysconf(_SC_PAGE_SIZE);
+	size_t page_sz = rte_mem_page_size();
 	size_t cfg_len = sizeof(struct rte_mem_config);
 	size_t cfg_len_aligned = RTE_ALIGN(cfg_len, page_sz);
 	void *rte_mem_cfg_addr, *mapped_mem_cfg_addr;
diff --git a/lib/eal/freebsd/eal_memory.c b/lib/eal/freebsd/eal_memory.c
index 3b72e13506..6d3d46a390 100644
--- a/lib/eal/freebsd/eal_memory.c
+++ b/lib/eal/freebsd/eal_memory.c
@@ -11,6 +11,7 @@
 #include <fcntl.h>
 
 #include <rte_eal.h>
+#include <rte_eal_paging.h>
 #include <rte_errno.h>
 #include <rte_log.h>
 #include <rte_string_fns.h>
@@ -22,8 +23,6 @@
 #include "eal_memcfg.h"
 #include "eal_options.h"
 
-#define EAL_PAGE_SIZE (sysconf(_SC_PAGESIZE))
-
 uint64_t eal_get_baseaddr(void)
 {
 	/*
@@ -191,7 +190,7 @@ rte_eal_hugepage_init(void)
 			addr = mmap(addr, page_sz, PROT_READ|PROT_WRITE,
 					MAP_SHARED | MAP_FIXED,
 					hpi->lock_descriptor,
-					j * EAL_PAGE_SIZE);
+					j * rte_mem_page_size());
 			if (addr == MAP_FAILED) {
 				EAL_LOG(ERR, "Failed to mmap buffer %u from %s",
 						j, hpi->hugedir);
@@ -243,7 +242,7 @@ attach_segment(const struct rte_memseg_list *msl, const struct rte_memseg *ms,
 
 	addr = mmap(ms->addr, ms->len, PROT_READ | PROT_WRITE,
 			MAP_SHARED | MAP_FIXED, wa->fd_hugepage,
-			wa->seg_idx * EAL_PAGE_SIZE);
+			wa->seg_idx * rte_mem_page_size());
 	if (addr == MAP_FAILED || addr != ms->addr)
 		return -1;
 	wa->seg_idx++;
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 54bee4a2cf..52efb8626b 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -31,6 +31,7 @@
 #include <rte_launch.h>
 #include <rte_eal.h>
 #include <rte_eal_memconfig.h>
+#include <rte_eal_paging.h>
 #include <rte_errno.h>
 #include <rte_lcore.h>
 #include <rte_service_component.h>
@@ -179,7 +180,7 @@ static int
 rte_eal_config_create(void)
 {
 	struct rte_config *config = rte_eal_get_configuration();
-	size_t page_sz = sysconf(_SC_PAGE_SIZE);
+	size_t page_sz = rte_mem_page_size();
 	size_t cfg_len = sizeof(*config->mem_config);
 	size_t cfg_len_aligned = RTE_ALIGN(cfg_len, page_sz);
 	void *rte_mem_cfg_addr, *mapped_mem_cfg_addr;
diff --git a/lib/eal/unix/eal_unix_memory.c b/lib/eal/unix/eal_unix_memory.c
index 61e914b8db..562b2b6d92 100644
--- a/lib/eal/unix/eal_unix_memory.c
+++ b/lib/eal/unix/eal_unix_memory.c
@@ -89,7 +89,7 @@ eal_mem_set_dump(void *virt, size_t size, bool dump)
 	int ret = madvise(virt, size, flags);
 	if (ret) {
 		EAL_LOG(DEBUG, "madvise(%p, %#zx, %d) failed: %s",
-				virt, size, flags, strerror(rte_errno));
+				virt, size, flags, strerror(errno));
 		rte_errno = errno;
 	}
 	return ret;
@@ -147,8 +147,20 @@ rte_mem_page_size(void)
 {
 	static size_t page_size;
 
-	if (!page_size)
+	if (page_size == 0) {
+		/*
+		 * When the sysconf value cannot be determined, sysconf()
+		 * returns -1 without setting errno.
+		 * To distinguish an indeterminate value from an error,
+		 * clear errno before calling sysconf(), and check whether
+		 * errno has been set if sysconf() returns -1.
+		 */
+		errno = 0;
 		page_size = sysconf(_SC_PAGESIZE);
+		if ((ssize_t)page_size < 0)
+			rte_panic("sysconf(_SC_PAGESIZE) failed: %s",
+					errno == 0 ? "Indeterminate" : strerror(errno));
+	}
 
 	return page_size;
 }
diff --git a/lib/meson.build b/lib/meson.build
index 1934cb4a29..7c1c21e6bc 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -13,7 +13,6 @@ libraries = [
         'kvargs', # eal depends on kvargs
         'argparse',
         'telemetry', # basic info querying
-        'pmu',
         'eal', # everything depends on eal
         'ptr_compress',
         'ring',
@@ -49,6 +48,7 @@ libraries = [
         'lpm',
         'member',
         'pcapng',
+        'pmu',
         'power',
         'rawdev',
         'regexdev',
diff --git a/lib/pmu/pmu.c b/lib/pmu/pmu.c
index 46b0b450ac..267d48c3d6 100644
--- a/lib/pmu/pmu.c
+++ b/lib/pmu/pmu.c
@@ -14,6 +14,7 @@
 
 #include <eal_export.h>
 #include <rte_bitops.h>
+#include <rte_eal_paging.h>
 #include <rte_tailq.h>
 #include <rte_log.h>
 
@@ -215,13 +216,12 @@ open_events(struct rte_pmu_event_group *group)
 static int
 mmap_events(struct rte_pmu_event_group *group)
 {
-	long page_size = sysconf(_SC_PAGE_SIZE);
 	unsigned int i;
 	void *addr;
 	int ret;
 
 	for (i = 0; i < rte_pmu.num_group_events; i++) {
-		addr = mmap(0, page_size, PROT_READ, MAP_SHARED, group->fds[i], 0);
+		addr = mmap(0, rte_mem_page_size(), PROT_READ, MAP_SHARED, group->fds[i], 0);
 		if (addr == MAP_FAILED) {
 			ret = -errno;
 			goto out;
@@ -233,7 +233,7 @@ mmap_events(struct rte_pmu_event_group *group)
 	return 0;
 out:
 	for (; i; i--) {
-		munmap(group->mmap_pages[i - 1], page_size);
+		munmap(group->mmap_pages[i - 1], rte_mem_page_size());
 		group->mmap_pages[i - 1] = NULL;
 	}
 
@@ -250,7 +250,7 @@ cleanup_events(struct rte_pmu_event_group *group)
 
 	for (i = 0; i < rte_pmu.num_group_events; i++) {
 		if (group->mmap_pages[i]) {
-			munmap(group->mmap_pages[i], sysconf(_SC_PAGE_SIZE));
+			munmap(group->mmap_pages[i], rte_mem_page_size());
 			group->mmap_pages[i] = NULL;
 		}
 
diff --git a/lib/vhost/vduse.c b/lib/vhost/vduse.c
index a2a7d73388..9de7f04a4f 100644
--- a/lib/vhost/vduse.c
+++ b/lib/vhost/vduse.c
@@ -16,6 +16,7 @@
 #include <sys/stat.h>
 
 #include <rte_common.h>
+#include <rte_eal_paging.h>
 #include <rte_thread.h>
 
 #include "fd_man.h"
@@ -690,7 +691,7 @@ vduse_device_create(const char *path, bool compliant_ol_flags)
 		dev_config->vendor_id = 0;
 		dev_config->features = features;
 		dev_config->vq_num = total_queues;
-		dev_config->vq_align = sysconf(_SC_PAGE_SIZE);
+		dev_config->vq_align = rte_mem_page_size();
 		dev_config->config_size = sizeof(struct virtio_net_config);
 		memcpy(dev_config->config, &vnet_config, sizeof(vnet_config));
 
-- 
2.43.0

Re: [PATCH v2] eal: handle sysconf(_SC_PAGESIZE) negative return value

[Burakov, Anatoly]

On 6/12/2025 4:06 PM, Morten Brørup wrote:
> eal: handle sysconf(_SC_PAGESIZE) negative return value
> 
> Coverity reports some defects, where the root cause seems to be negative
> return value from sysconf(_SC_PAGESIZE) not being handled.
> 
> rte_mem_page_size() was updated to handle negative return value from
> sysconf(_SC_PAGESIZE).
> 
> All other functions calling sysconf(_SC_PAGESIZE) directly were updated to
> call rte_mem_page_size() instead.
> 
> At this time, no other calls to sysconf(_SC_PAGESIZE) have been found in
> the DPDK libraries.
> 
> PS: "_SC_PAGESIZE" has the alias "_SC_PAGE_SIZE". Both are covered here.
> 
> Moved the PMU library to the correct location in the meson.build file; it
> depends on the EAL, not vice versa.
> 
> And an unrelated drive-by minor fix in eal_mem_set_dump():
> When madvise() failed, an incorrect reason was logged.
> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>

Hi Morten,

I don't think including unrelated fixes is a good idea, as it makes it 
more difficult to bisect, trace line history, etc., so I would suggest 
splitting these into 3 patches.

Regarding contents of the patch itself, LGTM

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

> ---
> v2:
> * Clarify in title and description that only sysconf(_SC_PAGESIZE) is
>    covered, and in description that only DPDK libs have been patched.
> * Include new PMU library in patch after rebasing.
> v1:
> * Improve comment about errno for indeterminate sysconf() values.
>    (Stephen)
> ---
>   lib/eal/freebsd/eal.c          |  3 ++-
>   lib/eal/freebsd/eal_memory.c   |  7 +++----
>   lib/eal/linux/eal.c            |  3 ++-
>   lib/eal/unix/eal_unix_memory.c | 16 ++++++++++++++--
>   lib/meson.build                |  2 +-
>   lib/pmu/pmu.c                  |  8 ++++----
>   lib/vhost/vduse.c              |  3 ++-
>   7 files changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
> index 82382d6e8f..c1ab8d86d2 100644
> --- a/lib/eal/freebsd/eal.c
> +++ b/lib/eal/freebsd/eal.c
> @@ -26,6 +26,7 @@
>   #include <rte_launch.h>
>   #include <rte_eal.h>
>   #include <rte_eal_memconfig.h>
> +#include <rte_eal_paging.h>
>   #include <rte_errno.h>
>   #include <rte_per_lcore.h>
>   #include <rte_lcore.h>
> @@ -98,7 +99,7 @@ rte_eal_config_create(void)
>   	struct rte_config *config = rte_eal_get_configuration();
>   	const struct internal_config *internal_conf =
>   		eal_get_internal_configuration();
> -	size_t page_sz = sysconf(_SC_PAGE_SIZE);
> +	size_t page_sz = rte_mem_page_size();
>   	size_t cfg_len = sizeof(struct rte_mem_config);
>   	size_t cfg_len_aligned = RTE_ALIGN(cfg_len, page_sz);
>   	void *rte_mem_cfg_addr, *mapped_mem_cfg_addr;
> diff --git a/lib/eal/freebsd/eal_memory.c b/lib/eal/freebsd/eal_memory.c
> index 3b72e13506..6d3d46a390 100644
> --- a/lib/eal/freebsd/eal_memory.c
> +++ b/lib/eal/freebsd/eal_memory.c
> @@ -11,6 +11,7 @@
>   #include <fcntl.h>
>   
>   #include <rte_eal.h>
> +#include <rte_eal_paging.h>
>   #include <rte_errno.h>
>   #include <rte_log.h>
>   #include <rte_string_fns.h>
> @@ -22,8 +23,6 @@
>   #include "eal_memcfg.h"
>   #include "eal_options.h"
>   
> -#define EAL_PAGE_SIZE (sysconf(_SC_PAGESIZE))
> -
>   uint64_t eal_get_baseaddr(void)
>   {
>   	/*
> @@ -191,7 +190,7 @@ rte_eal_hugepage_init(void)
>   			addr = mmap(addr, page_sz, PROT_READ|PROT_WRITE,
>   					MAP_SHARED | MAP_FIXED,
>   					hpi->lock_descriptor,
> -					j * EAL_PAGE_SIZE);
> +					j * rte_mem_page_size());
>   			if (addr == MAP_FAILED) {
>   				EAL_LOG(ERR, "Failed to mmap buffer %u from %s",
>   						j, hpi->hugedir);
> @@ -243,7 +242,7 @@ attach_segment(const struct rte_memseg_list *msl, const struct rte_memseg *ms,
>   
>   	addr = mmap(ms->addr, ms->len, PROT_READ | PROT_WRITE,
>   			MAP_SHARED | MAP_FIXED, wa->fd_hugepage,
> -			wa->seg_idx * EAL_PAGE_SIZE);
> +			wa->seg_idx * rte_mem_page_size());
>   	if (addr == MAP_FAILED || addr != ms->addr)
>   		return -1;
>   	wa->seg_idx++;
> diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
> index 54bee4a2cf..52efb8626b 100644
> --- a/lib/eal/linux/eal.c
> +++ b/lib/eal/linux/eal.c
> @@ -31,6 +31,7 @@
>   #include <rte_launch.h>
>   #include <rte_eal.h>
>   #include <rte_eal_memconfig.h>
> +#include <rte_eal_paging.h>
>   #include <rte_errno.h>
>   #include <rte_lcore.h>
>   #include <rte_service_component.h>
> @@ -179,7 +180,7 @@ static int
>   rte_eal_config_create(void)
>   {
>   	struct rte_config *config = rte_eal_get_configuration();
> -	size_t page_sz = sysconf(_SC_PAGE_SIZE);
> +	size_t page_sz = rte_mem_page_size();
>   	size_t cfg_len = sizeof(*config->mem_config);
>   	size_t cfg_len_aligned = RTE_ALIGN(cfg_len, page_sz);
>   	void *rte_mem_cfg_addr, *mapped_mem_cfg_addr;
> diff --git a/lib/eal/unix/eal_unix_memory.c b/lib/eal/unix/eal_unix_memory.c
> index 61e914b8db..562b2b6d92 100644
> --- a/lib/eal/unix/eal_unix_memory.c
> +++ b/lib/eal/unix/eal_unix_memory.c
> @@ -89,7 +89,7 @@ eal_mem_set_dump(void *virt, size_t size, bool dump)
>   	int ret = madvise(virt, size, flags);
>   	if (ret) {
>   		EAL_LOG(DEBUG, "madvise(%p, %#zx, %d) failed: %s",
> -				virt, size, flags, strerror(rte_errno));
> +				virt, size, flags, strerror(errno));
>   		rte_errno = errno;
>   	}
>   	return ret;
> @@ -147,8 +147,20 @@ rte_mem_page_size(void)
>   {
>   	static size_t page_size;
>   
> -	if (!page_size)
> +	if (page_size == 0) {
> +		/*
> +		 * When the sysconf value cannot be determined, sysconf()
> +		 * returns -1 without setting errno.
> +		 * To distinguish an indeterminate value from an error,
> +		 * clear errno before calling sysconf(), and check whether
> +		 * errno has been set if sysconf() returns -1.
> +		 */
> +		errno = 0;
>   		page_size = sysconf(_SC_PAGESIZE);
> +		if ((ssize_t)page_size < 0)
> +			rte_panic("sysconf(_SC_PAGESIZE) failed: %s",
> +					errno == 0 ? "Indeterminate" : strerror(errno));
> +	}
>   
>   	return page_size;
>   }
> diff --git a/lib/meson.build b/lib/meson.build
> index 1934cb4a29..7c1c21e6bc 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -13,7 +13,6 @@ libraries = [
>           'kvargs', # eal depends on kvargs
>           'argparse',
>           'telemetry', # basic info querying
> -        'pmu',
>           'eal', # everything depends on eal
>           'ptr_compress',
>           'ring',
> @@ -49,6 +48,7 @@ libraries = [
>           'lpm',
>           'member',
>           'pcapng',
> +        'pmu',
>           'power',
>           'rawdev',
>           'regexdev',
> diff --git a/lib/pmu/pmu.c b/lib/pmu/pmu.c
> index 46b0b450ac..267d48c3d6 100644
> --- a/lib/pmu/pmu.c
> +++ b/lib/pmu/pmu.c
> @@ -14,6 +14,7 @@
>   
>   #include <eal_export.h>
>   #include <rte_bitops.h>
> +#include <rte_eal_paging.h>
>   #include <rte_tailq.h>
>   #include <rte_log.h>
>   
> @@ -215,13 +216,12 @@ open_events(struct rte_pmu_event_group *group)
>   static int
>   mmap_events(struct rte_pmu_event_group *group)
>   {
> -	long page_size = sysconf(_SC_PAGE_SIZE);
>   	unsigned int i;
>   	void *addr;
>   	int ret;
>   
>   	for (i = 0; i < rte_pmu.num_group_events; i++) {
> -		addr = mmap(0, page_size, PROT_READ, MAP_SHARED, group->fds[i], 0);
> +		addr = mmap(0, rte_mem_page_size(), PROT_READ, MAP_SHARED, group->fds[i], 0);
>   		if (addr == MAP_FAILED) {
>   			ret = -errno;
>   			goto out;
> @@ -233,7 +233,7 @@ mmap_events(struct rte_pmu_event_group *group)
>   	return 0;
>   out:
>   	for (; i; i--) {
> -		munmap(group->mmap_pages[i - 1], page_size);
> +		munmap(group->mmap_pages[i - 1], rte_mem_page_size());
>   		group->mmap_pages[i - 1] = NULL;
>   	}
>   
> @@ -250,7 +250,7 @@ cleanup_events(struct rte_pmu_event_group *group)
>   
>   	for (i = 0; i < rte_pmu.num_group_events; i++) {
>   		if (group->mmap_pages[i]) {
> -			munmap(group->mmap_pages[i], sysconf(_SC_PAGE_SIZE));
> +			munmap(group->mmap_pages[i], rte_mem_page_size());
>   			group->mmap_pages[i] = NULL;
>   		}
>   
> diff --git a/lib/vhost/vduse.c b/lib/vhost/vduse.c
> index a2a7d73388..9de7f04a4f 100644
> --- a/lib/vhost/vduse.c
> +++ b/lib/vhost/vduse.c
> @@ -16,6 +16,7 @@
>   #include <sys/stat.h>
>   
>   #include <rte_common.h>
> +#include <rte_eal_paging.h>
>   #include <rte_thread.h>
>   
>   #include "fd_man.h"
> @@ -690,7 +691,7 @@ vduse_device_create(const char *path, bool compliant_ol_flags)
>   		dev_config->vendor_id = 0;
>   		dev_config->features = features;
>   		dev_config->vq_num = total_queues;
> -		dev_config->vq_align = sysconf(_SC_PAGE_SIZE);
> +		dev_config->vq_align = rte_mem_page_size();
>   		dev_config->config_size = sizeof(struct virtio_net_config);
>   		memcpy(dev_config->config, &vnet_config, sizeof(vnet_config));
>   


-- 
Thanks,
Anatoly

[PATCH v3 0/3] handle sysconf(_SC_PAGESIZE) negative return value

[Morten Brørup]

Coverity reports some defects, where the root cause seems to be negative
return value from sysconf(_SC_PAGESIZE) not being handled.
This series addresses those defects in the DPDK libraries.

PS: "_SC_PAGESIZE" has the alias "_SC_PAGE_SIZE". Both are covered here.

Morten Brørup (3):
  eal/unix: fix log message for madvise() failure
  eal: handle sysconf(_SC_PAGESIZE) negative return value
  pmu: handle sysconf(_SC_PAGESIZE) negative return value

 lib/eal/freebsd/eal.c          |  3 ++-
 lib/eal/freebsd/eal_memory.c   |  7 +++----
 lib/eal/linux/eal.c            |  3 ++-
 lib/eal/unix/eal_unix_memory.c | 16 ++++++++++++++--
 lib/pmu/pmu.c                  | 32 ++++++++++++++++++++++++++++++--
 lib/vhost/vduse.c              |  3 ++-
 6 files changed, 53 insertions(+), 11 deletions(-)

-- 
2.43.0

[PATCH v3 1/3] eal/unix: fix log message for madvise() failure

[Morten Brørup]

In eal_mem_set_dump(), when madvise() failed, an incorrect reason was
logged.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/eal/unix/eal_unix_memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/eal/unix/eal_unix_memory.c b/lib/eal/unix/eal_unix_memory.c
index 61e914b8db..650151facb 100644
--- a/lib/eal/unix/eal_unix_memory.c
+++ b/lib/eal/unix/eal_unix_memory.c
@@ -89,7 +89,7 @@ eal_mem_set_dump(void *virt, size_t size, bool dump)
 	int ret = madvise(virt, size, flags);
 	if (ret) {
 		EAL_LOG(DEBUG, "madvise(%p, %#zx, %d) failed: %s",
-				virt, size, flags, strerror(rte_errno));
+				virt, size, flags, strerror(errno));
 		rte_errno = errno;
 	}
 	return ret;
-- 
2.43.0

[PATCH v3 2/3] eal: handle sysconf(_SC_PAGESIZE) negative return value

[Morten Brørup]

Coverity reports some defects, where the root cause seems to be negative
return value from sysconf(_SC_PAGESIZE) not being handled.

rte_mem_page_size() was updated to handle negative return value from
sysconf(_SC_PAGESIZE).

All library functions directly calling sysconf(_SC_PAGESIZE) were updated
to call rte_mem_page_size() instead.
Except the PMU library, because of its dependency chain. (EAL depends on
PMU, because Trace is part of EAL, and Trace depends on PMU; so PMU cannot
call EAL functions.)

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/eal/freebsd/eal.c          |  3 ++-
 lib/eal/freebsd/eal_memory.c   |  7 +++----
 lib/eal/linux/eal.c            |  3 ++-
 lib/eal/unix/eal_unix_memory.c | 14 +++++++++++++-
 lib/vhost/vduse.c              |  3 ++-
 5 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index 82382d6e8f..c1ab8d86d2 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -26,6 +26,7 @@
 #include <rte_launch.h>
 #include <rte_eal.h>
 #include <rte_eal_memconfig.h>
+#include <rte_eal_paging.h>
 #include <rte_errno.h>
 #include <rte_per_lcore.h>
 #include <rte_lcore.h>
@@ -98,7 +99,7 @@ rte_eal_config_create(void)
 	struct rte_config *config = rte_eal_get_configuration();
 	const struct internal_config *internal_conf =
 		eal_get_internal_configuration();
-	size_t page_sz = sysconf(_SC_PAGE_SIZE);
+	size_t page_sz = rte_mem_page_size();
 	size_t cfg_len = sizeof(struct rte_mem_config);
 	size_t cfg_len_aligned = RTE_ALIGN(cfg_len, page_sz);
 	void *rte_mem_cfg_addr, *mapped_mem_cfg_addr;
diff --git a/lib/eal/freebsd/eal_memory.c b/lib/eal/freebsd/eal_memory.c
index 3b72e13506..6d3d46a390 100644
--- a/lib/eal/freebsd/eal_memory.c
+++ b/lib/eal/freebsd/eal_memory.c
@@ -11,6 +11,7 @@
 #include <fcntl.h>
 
 #include <rte_eal.h>
+#include <rte_eal_paging.h>
 #include <rte_errno.h>
 #include <rte_log.h>
 #include <rte_string_fns.h>
@@ -22,8 +23,6 @@
 #include "eal_memcfg.h"
 #include "eal_options.h"
 
-#define EAL_PAGE_SIZE (sysconf(_SC_PAGESIZE))
-
 uint64_t eal_get_baseaddr(void)
 {
 	/*
@@ -191,7 +190,7 @@ rte_eal_hugepage_init(void)
 			addr = mmap(addr, page_sz, PROT_READ|PROT_WRITE,
 					MAP_SHARED | MAP_FIXED,
 					hpi->lock_descriptor,
-					j * EAL_PAGE_SIZE);
+					j * rte_mem_page_size());
 			if (addr == MAP_FAILED) {
 				EAL_LOG(ERR, "Failed to mmap buffer %u from %s",
 						j, hpi->hugedir);
@@ -243,7 +242,7 @@ attach_segment(const struct rte_memseg_list *msl, const struct rte_memseg *ms,
 
 	addr = mmap(ms->addr, ms->len, PROT_READ | PROT_WRITE,
 			MAP_SHARED | MAP_FIXED, wa->fd_hugepage,
-			wa->seg_idx * EAL_PAGE_SIZE);
+			wa->seg_idx * rte_mem_page_size());
 	if (addr == MAP_FAILED || addr != ms->addr)
 		return -1;
 	wa->seg_idx++;
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 54bee4a2cf..52efb8626b 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -31,6 +31,7 @@
 #include <rte_launch.h>
 #include <rte_eal.h>
 #include <rte_eal_memconfig.h>
+#include <rte_eal_paging.h>
 #include <rte_errno.h>
 #include <rte_lcore.h>
 #include <rte_service_component.h>
@@ -179,7 +180,7 @@ static int
 rte_eal_config_create(void)
 {
 	struct rte_config *config = rte_eal_get_configuration();
-	size_t page_sz = sysconf(_SC_PAGE_SIZE);
+	size_t page_sz = rte_mem_page_size();
 	size_t cfg_len = sizeof(*config->mem_config);
 	size_t cfg_len_aligned = RTE_ALIGN(cfg_len, page_sz);
 	void *rte_mem_cfg_addr, *mapped_mem_cfg_addr;
diff --git a/lib/eal/unix/eal_unix_memory.c b/lib/eal/unix/eal_unix_memory.c
index 650151facb..55b647c736 100644
--- a/lib/eal/unix/eal_unix_memory.c
+++ b/lib/eal/unix/eal_unix_memory.c
@@ -147,8 +147,20 @@ rte_mem_page_size(void)
 {
 	static size_t page_size;
 
-	if (!page_size)
+	if (unlikely(page_size == 0)) {
+		/*
+		 * When the sysconf value cannot be determined, sysconf()
+		 * returns -1 without setting errno.
+		 * To distinguish an indeterminate value from an error,
+		 * clear errno before calling sysconf(), and check whether
+		 * errno has been set if sysconf() returns -1.
+		 */
+		errno = 0;
 		page_size = sysconf(_SC_PAGESIZE);
+		if ((ssize_t)page_size < 0)
+			rte_panic("sysconf(_SC_PAGESIZE) failed: %s",
+					errno == 0 ? "Indeterminate" : strerror(errno));
+	}
 
 	return page_size;
 }
diff --git a/lib/vhost/vduse.c b/lib/vhost/vduse.c
index a2a7d73388..9de7f04a4f 100644
--- a/lib/vhost/vduse.c
+++ b/lib/vhost/vduse.c
@@ -16,6 +16,7 @@
 #include <sys/stat.h>
 
 #include <rte_common.h>
+#include <rte_eal_paging.h>
 #include <rte_thread.h>
 
 #include "fd_man.h"
@@ -690,7 +691,7 @@ vduse_device_create(const char *path, bool compliant_ol_flags)
 		dev_config->vendor_id = 0;
 		dev_config->features = features;
 		dev_config->vq_num = total_queues;
-		dev_config->vq_align = sysconf(_SC_PAGE_SIZE);
+		dev_config->vq_align = rte_mem_page_size();
 		dev_config->config_size = sizeof(struct virtio_net_config);
 		memcpy(dev_config->config, &vnet_config, sizeof(vnet_config));
 
-- 
2.43.0

[PATCH v3 3/3] pmu: handle sysconf(_SC_PAGESIZE) negative return value

[Morten Brørup]

Coverity reports some defects, where the root cause seems to be negative
return value from sysconf(_SC_PAGESIZE) not being handled.

The PMU library cannot use the EAL function rte_mem_page_size(), because
of the dependency chain. (EAL depends on PMU, because Trace is part of
EAL, and Trace depends on PMU; so PMU cannot call EAL functions.)
So mem_page_size(), inspired by rte_mem_page_size(), was added to PMU, and
used instead.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/pmu/pmu.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/lib/pmu/pmu.c b/lib/pmu/pmu.c
index 46b0b450ac..4c7271522a 100644
--- a/lib/pmu/pmu.c
+++ b/lib/pmu/pmu.c
@@ -212,14 +212,40 @@ open_events(struct rte_pmu_event_group *group)
 	return ret;
 }
 
+/* Inspired by rte_mem_page_size() from /lib/eal/unix/eal_unix_memory.c */
+static size_t
+mem_page_size(void)
+{
+	static size_t page_size;
+
+	if (unlikely(page_size == 0)) {
+		/*
+		 * When the sysconf value cannot be determined, sysconf()
+		 * returns -1 without setting errno.
+		 * To distinguish an indeterminate value from an error,
+		 * clear errno before calling sysconf(), and check whether
+		 * errno has been set if sysconf() returns -1.
+		 */
+		errno = 0;
+		page_size = sysconf(_SC_PAGESIZE);
+		if ((ssize_t)page_size < 0 && errno == 0)
+			errno = ENOENT;
+	}
+
+	return page_size;
+}
+
 static int
 mmap_events(struct rte_pmu_event_group *group)
 {
-	long page_size = sysconf(_SC_PAGE_SIZE);
+	size_t page_size = mem_page_size();
 	unsigned int i;
 	void *addr;
 	int ret;
 
+	if ((ssize_t)page_size < 0)
+		return -errno;
+
 	for (i = 0; i < rte_pmu.num_group_events; i++) {
 		addr = mmap(0, page_size, PROT_READ, MAP_SHARED, group->fds[i], 0);
 		if (addr == MAP_FAILED) {
@@ -243,6 +269,7 @@ mmap_events(struct rte_pmu_event_group *group)
 static void
 cleanup_events(struct rte_pmu_event_group *group)
 {
+	size_t page_size = mem_page_size();
 	unsigned int i;
 
 	if (group->fds[0] != -1)
@@ -250,7 +277,8 @@ cleanup_events(struct rte_pmu_event_group *group)
 
 	for (i = 0; i < rte_pmu.num_group_events; i++) {
 		if (group->mmap_pages[i]) {
-			munmap(group->mmap_pages[i], sysconf(_SC_PAGE_SIZE));
+			__rte_assume((ssize_t)page_size >= 0);
+			munmap(group->mmap_pages[i], page_size);
 			group->mmap_pages[i] = NULL;
 		}
 
-- 
2.43.0

Re: [PATCH v3 1/3] eal/unix: fix log message for madvise() failure

[Thomas Monjalon]

24/06/2025 10:03, Morten Brørup:
> In eal_mem_set_dump(), when madvise() failed, an incorrect reason was
> logged.
> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

We need to track the root cause with Fixes: please.

Re: [PATCH v3 2/3] eal: handle sysconf(_SC_PAGESIZE) negative return value

[Thomas Monjalon]

24/06/2025 10:03, Morten Brørup:
> +		if ((ssize_t)page_size < 0)
> +			rte_panic("sysconf(_SC_PAGESIZE) failed: %s",
> +					errno == 0 ? "Indeterminate" : strerror(errno));

We don't want more rte_panic().
You could log the problem and return 0 here.
It will be a problem later, but it may allow the application to cleanup
instead of abrupting crashing.

RE: [PATCH v3 2/3] eal: handle sysconf(_SC_PAGESIZE) negative return value

[Morten Brørup]

> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Friday, 27 June 2025 17.58
> 
> 24/06/2025 10:03, Morten Brørup:
> > +		if ((ssize_t)page_size < 0)
> > +			rte_panic("sysconf(_SC_PAGESIZE) failed: %s",
> > +					errno == 0 ? "Indeterminate" :
> strerror(errno));
> 
> We don't want more rte_panic().
> You could log the problem and return 0 here.
> It will be a problem later, but it may allow the application to cleanup
> instead of abrupting crashing.

Disagree.
That would be likely to cause crash with division by zero later.
Better to fail early.

RE: [PATCH v3 1/3] eal/unix: fix log message for madvise() failure

[Morten Brørup]

> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Friday, 27 June 2025 17.56
> 
> 24/06/2025 10:03, Morten Brørup:
> > In eal_mem_set_dump(), when madvise() failed, an incorrect reason was
> > logged.
> >
> > Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> > Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> 
> We need to track the root cause with Fixes: please.
> 

I already looked into it, and the bug was there from the initial commit of this function:
https://git.dpdk.org/dpdk/commit/lib/librte_eal/unix/eal_unix_memory.c?id=c4b89ecb64eae773337f3b1367dd8b1f09737f2e

I didn't think the Fixes tag was required if that was the case, but I may have misunderstood this.

Also, it's an insignificant bug, so no need to backport.

Re: [PATCH v3 1/3] eal/unix: fix log message for madvise() failure

[Thomas Monjalon]

27/06/2025 18:47, Morten Brørup:
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > Sent: Friday, 27 June 2025 17.56
> > 
> > 24/06/2025 10:03, Morten Brørup:
> > > In eal_mem_set_dump(), when madvise() failed, an incorrect reason was
> > > logged.
> > >
> > > Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> > > Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > 
> > We need to track the root cause with Fixes: please.
> > 
> 
> I already looked into it, and the bug was there from the initial commit of this function:
> https://git.dpdk.org/dpdk/commit/lib/librte_eal/unix/eal_unix_memory.c?id=c4b89ecb64eae773337f3b1367dd8b1f09737f2e

If we don't track this origin in the commit log, it is difficult to know.

> I didn't think the Fixes tag was required if that was the case, but I may have misunderstood this.

Yes we must be explicit and add it.

> Also, it's an insignificant bug, so no need to backport.

A bug is a bug.
LTS maintainers or anyone maintaining his own branch will decide whether to backport.

Re: [PATCH v3 2/3] eal: handle sysconf(_SC_PAGESIZE) negative return value

[Thomas Monjalon]

27/06/2025 18:38, Morten Brørup:
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > Sent: Friday, 27 June 2025 17.58
> > 
> > 24/06/2025 10:03, Morten Brørup:
> > > +		if ((ssize_t)page_size < 0)
> > > +			rte_panic("sysconf(_SC_PAGESIZE) failed: %s",
> > > +					errno == 0 ? "Indeterminate" :
> > strerror(errno));
> > 
> > We don't want more rte_panic().
> > You could log the problem and return 0 here.
> > It will be a problem later, but it may allow the application to cleanup
> > instead of abrupting crashing.
> 
> Disagree.
> That would be likely to cause crash with division by zero later.
> Better to fail early.

Which division by zero?

I don't think a library should take this decision on behalf of the app.

RE: [PATCH v3 2/3] eal: handle sysconf(_SC_PAGESIZE) negative return value

[Morten Brørup]

> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Friday, 27 June 2025 19.35
> 
> 27/06/2025 18:38, Morten Brørup:
> > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > Sent: Friday, 27 June 2025 17.58
> > >
> > > 24/06/2025 10:03, Morten Brørup:
> > > > +		if ((ssize_t)page_size < 0)
> > > > +			rte_panic("sysconf(_SC_PAGESIZE) failed: %s",
> > > > +					errno == 0 ? "Indeterminate" :
> > > strerror(errno));
> > >
> > > We don't want more rte_panic().
> > > You could log the problem and return 0 here.
> > > It will be a problem later, but it may allow the application to
> cleanup
> > > instead of abrupting crashing.
> >
> > Disagree.
> > That would be likely to cause crash with division by zero later.
> > Better to fail early.
> 
> Which division by zero?

Functions dividing by page size. E.g.:
https://elixir.bootlin.com/dpdk/v25.03/source/lib/eal/common/eal_common_memory.c#L313

> 
> I don't think a library should take this decision on behalf of the app.

I expect lots of things to break if sysconf(_SC_PAGESIZE) fails, so the purpose of this patch is to centralize error handling here, and only continue/return with non-failing values.

Otherwise, everywhere using rte_mem_page_size() or sysconf(_SC_PAGESIZE) should implement error handling (or ignore errors).
That's a lot of places, so I'm not going to provide a patch doing that.

RE: [PATCH v3 1/3] eal/unix: fix log message for madvise() failure

[Morten Brørup]

> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Friday, 27 June 2025 19.34
> 
> 27/06/2025 18:47, Morten Brørup:
> > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > Sent: Friday, 27 June 2025 17.56
> > >
> > > 24/06/2025 10:03, Morten Brørup:
> > > > In eal_mem_set_dump(), when madvise() failed, an incorrect reason
> was
> > > > logged.
> > > >
> > > > Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> > > > Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > >
> > > We need to track the root cause with Fixes: please.
> > >
> >
> > I already looked into it, and the bug was there from the initial
> commit of this function:
> >
> https://git.dpdk.org/dpdk/commit/lib/librte_eal/unix/eal_unix_memory.c?i
> d=c4b89ecb64eae773337f3b1367dd8b1f09737f2e
> 
> If we don't track this origin in the commit log, it is difficult to
> know.
> 
> > I didn't think the Fixes tag was required if that was the case, but I
> may have misunderstood this.
> 
> Yes we must be explicit and add it.
> 
> > Also, it's an insignificant bug, so no need to backport.
> 
> A bug is a bug.
> LTS maintainers or anyone maintaining his own branch will decide whether
> to backport.
> 

OK. Thank you for clarifying, Thomas.

Re: [PATCH v3 2/3] eal: handle sysconf(_SC_PAGESIZE) negative return value

[Thomas Monjalon]

27/06/2025 19:49, Morten Brørup:
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > Sent: Friday, 27 June 2025 19.35
> > 
> > 27/06/2025 18:38, Morten Brørup:
> > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > Sent: Friday, 27 June 2025 17.58
> > > >
> > > > 24/06/2025 10:03, Morten Brørup:
> > > > > +		if ((ssize_t)page_size < 0)
> > > > > +			rte_panic("sysconf(_SC_PAGESIZE) failed: %s",
> > > > > +					errno == 0 ? "Indeterminate" :
> > > > strerror(errno));
> > > >
> > > > We don't want more rte_panic().
> > > > You could log the problem and return 0 here.
> > > > It will be a problem later, but it may allow the application to
> > cleanup
> > > > instead of abrupting crashing.
> > >
> > > Disagree.
> > > That would be likely to cause crash with division by zero later.
> > > Better to fail early.
> > 
> > Which division by zero?
> 
> Functions dividing by page size. E.g.:
> https://elixir.bootlin.com/dpdk/v25.03/source/lib/eal/common/eal_common_memory.c#L313
> 
> > 
> > I don't think a library should take this decision on behalf of the app.
> 
> I expect lots of things to break if sysconf(_SC_PAGESIZE) fails, so the purpose of this patch is to centralize error handling here, and only continue/return with non-failing values.
> 
> Otherwise, everywhere using rte_mem_page_size() or sysconf(_SC_PAGESIZE) should implement error handling (or ignore errors).
> That's a lot of places, so I'm not going to provide a patch doing that.

I understand.

The problem is that we don't have an exception mechanism in this language.

RE: [PATCH v3 1/3] eal/unix: fix log message for madvise() failure

[Morten Brørup]

> From: Morten Brørup
> Sent: Friday, 27 June 2025 19.51
> 
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > Sent: Friday, 27 June 2025 19.34
> >
> > 27/06/2025 18:47, Morten Brørup:
> > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > Sent: Friday, 27 June 2025 17.56
> > > >
> > > > 24/06/2025 10:03, Morten Brørup:
> > > > > In eal_mem_set_dump(), when madvise() failed, an incorrect
> reason
> > was
> > > > > logged.
> > > > >
> > > > > Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> > > > > Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > > >
> > > > We need to track the root cause with Fixes: please.
> > > >
> > >
> > > I already looked into it, and the bug was there from the initial
> > commit of this function:
> > >
> >
> https://git.dpdk.org/dpdk/commit/lib/librte_eal/unix/eal_unix_memory.c?i
> > d=c4b89ecb64eae773337f3b1367dd8b1f09737f2e
> >
> > If we don't track this origin in the commit log, it is difficult to
> > know.
> >
> > > I didn't think the Fixes tag was required if that was the case, but
> I
> > may have misunderstood this.
> >
> > Yes we must be explicit and add it.
> >
> > > Also, it's an insignificant bug, so no need to backport.
> >
> > A bug is a bug.
> > LTS maintainers or anyone maintaining his own branch will decide
> whether
> > to backport.
> >
> 
> OK. Thank you for clarifying, Thomas.

Fixes: c4b89ecb64ea ("eal: introduce memory management wrappers")

RE: [PATCH v3 2/3] eal: handle sysconf(_SC_PAGESIZE) negative return value

[Morten Brørup]

> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Friday, 27 June 2025 20.30
> 
> 27/06/2025 19:49, Morten Brørup:
> > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > Sent: Friday, 27 June 2025 19.35
> > >
> > > 27/06/2025 18:38, Morten Brørup:
> > > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > > Sent: Friday, 27 June 2025 17.58
> > > > >
> > > > > 24/06/2025 10:03, Morten Brørup:
> > > > > > +		if ((ssize_t)page_size < 0)
> > > > > > +			rte_panic("sysconf(_SC_PAGESIZE) failed: %s",
> > > > > > +					errno == 0 ? "Indeterminate" :
> > > > > strerror(errno));
> > > > >
> > > > > We don't want more rte_panic().
> > > > > You could log the problem and return 0 here.
> > > > > It will be a problem later, but it may allow the application to
> > > cleanup
> > > > > instead of abrupting crashing.
> > > >
> > > > Disagree.
> > > > That would be likely to cause crash with division by zero later.
> > > > Better to fail early.
> > >
> > > Which division by zero?
> >
> > Functions dividing by page size. E.g.:
> >
> https://elixir.bootlin.com/dpdk/v25.03/source/lib/eal/common/eal_common_
> memory.c#L313
> >
> > >
> > > I don't think a library should take this decision on behalf of the
> app.
> >
> > I expect lots of things to break if sysconf(_SC_PAGESIZE) fails, so
> the purpose of this patch is to centralize error handling here, and only
> continue/return with non-failing values.
> >
> > Otherwise, everywhere using rte_mem_page_size() or
> sysconf(_SC_PAGESIZE) should implement error handling (or ignore
> errors).
> > That's a lot of places, so I'm not going to provide a patch doing
> that.
> 
> I understand.
> 
> The problem is that we don't have an exception mechanism in this
> language.

Yep.
And everyone assumes sysconf(_SC_PAGESIZE) never fails, which is probably correct, so nobody implemented error handling for it. Not even in rte_mem_page_size().
Coverity detected the missing error handling, and warns: "Although rte_mem_page_size() is declared to return unsigned int, it may actually return a negative value." This defect applies to all functions calling rte_mem_page_size().
This patch adds error handling to ensure that rte_mem_page_size() only returns non-negative values, or doesn’t return at all - i.e. fails with rte_panic() - so Coverity is satisfied with callers not implementing error handling for it.

It would be borderline waste of time fixing all the callers, so I fixed the root cause to satisfy Coverity.

From an higher level perspective:
This is a low level EAL function to determine the page size. I would consider it reasonable for such a low level EAL function to never fail.
If some O/S decides to not have a "system page size", and fail with "Indeterminate", e.g. to support multiple page sizes, we would need to handle that somehow. But let's ignore that until it actually happens, if ever.

If you are skeptical about this patch 2/3 in the series, we can escalate the discussion to the tech board. If you really hate this patch 2/3, I will honor a NAK from you. The patch is not important for me; I'm just trying to clean up.

Re: [PATCH v3 2/3] eal: handle sysconf(_SC_PAGESIZE) negative return value

[Stephen Hemminger]

On Sat, 28 Jun 2025 18:45:44 +0200
Morten Brørup <mb@smartsharesystems.com> wrote:

> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > Sent: Friday, 27 June 2025 20.30
> > 
> > 27/06/2025 19:49, Morten Brørup:  
> > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > Sent: Friday, 27 June 2025 19.35
> > > >
> > > > 27/06/2025 18:38, Morten Brørup:  
> > > > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > > > Sent: Friday, 27 June 2025 17.58
> > > > > >
> > > > > > 24/06/2025 10:03, Morten Brørup:  
> > > > > > > +		if ((ssize_t)page_size < 0)
> > > > > > > +			rte_panic("sysconf(_SC_PAGESIZE) failed: %s",
> > > > > > > +					errno == 0 ? "Indeterminate" :  
> > > > > > strerror(errno));
> > > > > >
> > > > > > We don't want more rte_panic().
> > > > > > You could log the problem and return 0 here.
> > > > > > It will be a problem later, but it may allow the application to  
> > > > cleanup  
> > > > > > instead of abrupting crashing.  
> > > > >
> > > > > Disagree.
> > > > > That would be likely to cause crash with division by zero later.
> > > > > Better to fail early.  
> > > >
> > > > Which division by zero?  
> > >
> > > Functions dividing by page size. E.g.:
> > >  
> > https://elixir.bootlin.com/dpdk/v25.03/source/lib/eal/common/eal_common_
> > memory.c#L313  
> > >  
> > > >
> > > > I don't think a library should take this decision on behalf of the  
> > app.  
> > >
> > > I expect lots of things to break if sysconf(_SC_PAGESIZE) fails, so  
> > the purpose of this patch is to centralize error handling here, and only
> > continue/return with non-failing values.  
> > >
> > > Otherwise, everywhere using rte_mem_page_size() or  
> > sysconf(_SC_PAGESIZE) should implement error handling (or ignore
> > errors).  
> > > That's a lot of places, so I'm not going to provide a patch doing  
> > that.
> > 
> > I understand.
> > 
> > The problem is that we don't have an exception mechanism in this
> > language.  
> 
> Yep.
> And everyone assumes sysconf(_SC_PAGESIZE) never fails, which is probably correct, so nobody implemented error handling for it. Not even in rte_mem_page_size().
> Coverity detected the missing error handling, and warns: "Although rte_mem_page_size() is declared to return unsigned int, it may actually return a negative value." This defect applies to all functions calling rte_mem_page_size().
> This patch adds error handling to ensure that rte_mem_page_size() only returns non-negative values, or doesn’t return at all - i.e. fails with rte_panic() - so Coverity is satisfied with callers not implementing error handling for it.
> 
> It would be borderline waste of time fixing all the callers, so I fixed the root cause to satisfy Coverity.
> 
> From an higher level perspective:
> This is a low level EAL function to determine the page size. I would consider it reasonable for such a low level EAL function to never fail.
> If some O/S decides to not have a "system page size", and fail with "Indeterminate", e.g. to support multiple page sizes, we would need to handle that somehow. But let's ignore that until it actually happens, if ever.
> 
> If you are skeptical about this patch 2/3 in the series, we can escalate the discussion to the tech board. If you really hate this patch 2/3, I will honor a NAK from you. The patch is not important for me; I'm just trying to clean up.
> 

In such cases, I look at glibc source and see if handles it or not.
Looks like only used a couple of places there, the result of sysconf(_SC_PAGE_SIZE) is checked
in one of the tests; but is not checked in the loading of locale's.  It expects a valid power of 2
value there.

Ok to just die if value isn't valid.