[PATCH 0/9] consolidate sysfs access

[PATCH 0/9] consolidate sysfs access

[Stephen Hemminger]

After reviewing lots of drivers and seeing sloppy string handling
resorted to AI assistance to unify and consolidate the parsing
of sysfs values. Many drivers open code this with similar pattern
but lacked any coherent error handling. The kernel API is
consistent and won't give bad data, but it make sense to
check for garbage.

Add one set of routines in EAL that build the path from a printf
style format and convert with strtoul()/strtol(), then convert the
existing callers.

Note: existing eal_parse_sysfs_value() goes away with this.
It was exported as a stable symbol, which was a mistake:
the eal_ prefix and the private eal_filesystem.h both say it is internal,
and nothing outside the tree should have been calling it.
The new code does export rte_sysfs_parse_XXX are marked with
as internal use only.

Stephen Hemminger (9):
  eal: add common sysfs value routines
  dma/idxd: use common sysfs routines
  common/ionic: use common sysfs routines
  bus/vmbus: use common sysfs routines
  power: use common sysfs routines
  drivers/bus: remove duplicate sysfs string helpers
  common/mlx5: use common sysfs routines
  net/mlx5: use common sysfs routines
  net/mana: use common sysfs routines

 app/test/test_eal_fs.c                        | 135 ++++++++++---
 drivers/bus/auxiliary/linux/auxiliary.c       |  13 +-
 drivers/bus/cdx/cdx.c                         |   7 +-
 drivers/bus/pci/linux/pci.c                   |  44 ++---
 drivers/bus/platform/platform.c               |  43 +---
 drivers/bus/vmbus/linux/vmbus_bus.c           |  64 ++----
 drivers/bus/vmbus/linux/vmbus_uio.c           |  24 +--
 drivers/common/cnxk/roc_model.c               |  40 +---
 drivers/common/cnxk/roc_platform.h            |   3 +-
 drivers/common/ionic/ionic_common_uio.c       |  63 ++----
 .../common/mlx5/linux/mlx5_common_auxiliary.c |  13 +-
 drivers/common/mlx5/linux/mlx5_common_os.c    |  39 ++--
 drivers/dma/idxd/idxd_bus.c                   |  80 ++------
 drivers/net/af_xdp/rte_eth_af_xdp.c           |  13 +-
 drivers/net/mana/mana.c                       |  14 +-
 drivers/net/mlx5/linux/mlx5_ethdev_os.c       |  17 +-
 drivers/net/mlx5/linux/mlx5_os.c              |  13 +-
 drivers/power/acpi/acpi_cpufreq.c             |  21 +-
 drivers/power/amd_pstate/amd_pstate_cpufreq.c |  63 +-----
 drivers/power/cppc/cppc_cpufreq.c             |  69 +------
 .../power/intel_pstate/intel_pstate_cpufreq.c |  99 ++--------
 drivers/power/intel_uncore/intel_uncore.c     |  32 +--
 lib/eal/common/eal_filesystem.h               |   4 +-
 lib/eal/include/meson.build                   |   1 +
 lib/eal/include/rte_sysfs.h                   | 121 ++++++++++++
 lib/eal/linux/eal_hugepage_info.c             |  32 +--
 lib/eal/linux/eal_lcore.c                     |  18 +-
 lib/eal/unix/eal_filesystem.c                 |  31 ---
 lib/eal/unix/eal_unix_sysfs.c                 | 187 ++++++++++++++++++
 lib/eal/unix/meson.build                      |   1 +
 lib/power/power_common.c                      |  46 ++---
 lib/power/power_common.h                      |   6 +
 lib/power/rte_power_qos.c                     |  29 +--
 33 files changed, 639 insertions(+), 746 deletions(-)
 create mode 100644 lib/eal/include/rte_sysfs.h
 create mode 100644 lib/eal/unix/eal_unix_sysfs.c

-- 
2.53.0

[PATCH 1/9] eal: add common sysfs value routines

[Stephen Hemminger]

Many drivers in DPDK read single values, and each one
open codes the same sequence: build a path with snprintf, open the
file, read a line, and convert it. Several drivers grew their own
private copy of that helper, and most of them parse with fscanf()
which cannot tell a failed conversion from a legitimate zero and
silently accepts trailing garbage.

Add a single set of routines in EAL that build the path from a
printf style format and do the conversion with strtoul()/strtol():

  rte_sysfs_parse_uint()    unsigned value
  rte_sysfs_parse_int()     signed value
  rte_sysfs_parse_string()  string value, newline stripped
  rte_sysfs_write_string()  write a string

rte_sysfs_vparse_uint() takes a va_list, so that a wrapper such
as the one in the power library can forward its arguments without
formatting the path a second time.

Folding the path construction into the routine removes the separate
snprintf() and its truncation check from every caller.

The signed variant exists because some attributes are genuinely
signed: numa_node is -1 when the device is not tied to a node.

These replace eal_parse_sysfs_value(), which was declared in the
internal eal_filesystem.h yet exported as a stable symbol, and was
reached by drivers through that private header.

Convert all of the existing callers.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_eal_fs.c                        | 135 ++++++++++---
 drivers/bus/auxiliary/linux/auxiliary.c       |  13 +-
 drivers/bus/cdx/cdx.c                         |   7 +-
 drivers/bus/pci/linux/pci.c                   |  44 ++---
 drivers/bus/platform/platform.c               |  10 +-
 drivers/bus/vmbus/linux/vmbus_bus.c           |  25 +--
 drivers/common/cnxk/roc_model.c               |  40 +---
 drivers/common/cnxk/roc_platform.h            |   3 +-
 .../common/mlx5/linux/mlx5_common_auxiliary.c |  13 +-
 drivers/net/af_xdp/rte_eth_af_xdp.c           |  13 +-
 lib/eal/common/eal_filesystem.h               |   4 +-
 lib/eal/include/meson.build                   |   1 +
 lib/eal/include/rte_sysfs.h                   | 121 ++++++++++++
 lib/eal/linux/eal_hugepage_info.c             |  32 +--
 lib/eal/linux/eal_lcore.c                     |  18 +-
 lib/eal/unix/eal_filesystem.c                 |  31 ---
 lib/eal/unix/eal_unix_sysfs.c                 | 187 ++++++++++++++++++
 lib/eal/unix/meson.build                      |   1 +
 18 files changed, 493 insertions(+), 205 deletions(-)
 create mode 100644 lib/eal/include/rte_sysfs.h
 create mode 100644 lib/eal/unix/eal_unix_sysfs.c

diff --git a/app/test/test_eal_fs.c b/app/test/test_eal_fs.c
index 62eea98677..00c676c4d6 100644
--- a/app/test/test_eal_fs.c
+++ b/app/test/test_eal_fs.c
@@ -7,8 +7,10 @@
 #include <stdlib.h>
 #include <string.h>
 #include <errno.h>
+#include <limits.h>
+#include <unistd.h>
 
-#include "eal_filesystem.h"
+#include <rte_sysfs.h>
 
 #ifdef RTE_EXEC_ENV_WINDOWS
 static int
@@ -30,13 +32,15 @@ test_parse_sysfs_value(void)
 	FILE *fd = NULL;
 	unsigned valid_number;
 	unsigned long retval = 0;
+	char strval[64];
+	long sretval = 0;
 
 #ifdef RTE_EXEC_ENV_FREEBSD
 	/* BSD doesn't have /proc/pid/fd */
 	return 0;
 #endif
 
-	printf("Testing function eal_parse_sysfs_value()\n");
+	printf("Testing sysfs value functions\n");
 
 	/* get a temporary filename to use for all tests - create temp file handle and then
 	 * use /proc to get the actual file that we can open */
@@ -54,8 +58,8 @@ test_parse_sysfs_value(void)
 
 	/* test we get an error value if we use file before it's created */
 	printf("Test reading a missing file ...\n");
-	if (eal_parse_sysfs_value("/dev/not-quite-null", &retval) == 0) {
-		printf("Error with eal_parse_sysfs_value() - returned success on reading empty file\n");
+	if (rte_sysfs_parse_uint(&retval, "/dev/not-quite-null") == 0) {
+		printf("Error with rte_sysfs_parse_uint() - returned success on reading empty file\n");
 		goto error;
 	}
 	printf("Confirmed return error when reading empty file\n");
@@ -71,12 +75,12 @@ test_parse_sysfs_value(void)
 	fprintf(fd,"%u\n", valid_number);
 	fclose(fd);
 	fd = NULL;
-	if (eal_parse_sysfs_value(filename, &retval) < 0) {
-		printf("eal_parse_sysfs_value() returned error - test failed\n");
+	if (rte_sysfs_parse_uint(&retval, "%s", filename) < 0) {
+		printf("rte_sysfs_parse_uint() returned error - test failed\n");
 		goto error;
 	}
 	if (retval != valid_number) {
-		printf("Invalid value read by eal_parse_sysfs_value() - test failed\n");
+		printf("Invalid value read by rte_sysfs_parse_uint() - test failed\n");
 		goto error;
 	}
 	printf("Read '%u\\n' ok\n", valid_number);
@@ -91,43 +95,93 @@ test_parse_sysfs_value(void)
 	fprintf(fd,"0x%x\n", valid_number);
 	fclose(fd);
 	fd = NULL;
-	if (eal_parse_sysfs_value(filename, &retval) < 0) {
-		printf("eal_parse_sysfs_value() returned error - test failed\n");
+	if (rte_sysfs_parse_uint(&retval, "%s", filename) < 0) {
+		printf("rte_sysfs_parse_uint() returned error - test failed\n");
 		goto error;
 	}
 	if (retval != valid_number) {
-		printf("Invalid value read by eal_parse_sysfs_value() - test failed\n");
+		printf("Invalid value read by rte_sysfs_parse_uint() - test failed\n");
 		goto error;
 	}
 	printf("Read '0x%x\\n' ok\n", valid_number);
 
-	printf("Test reading invalid values ...\n");
+	/* a value without a trailing newline is accepted */
+	valid_number = 3;
+	fd = fopen(filename, "w");
+	if (fd == NULL) {
+		printf("line %d, Error opening %s: %s\n", __LINE__, filename, strerror(errno));
+		goto error;
+	}
+	fprintf(fd, "%u", valid_number);
+	fclose(fd);
+	fd = NULL;
+	if (rte_sysfs_parse_uint(&retval, "%s", filename) < 0 || retval != valid_number) {
+		printf("rte_sysfs_parse_uint() failed without trailing newline - test failed\n");
+		goto error;
+	}
+	printf("Read '%u' (no newline) ok\n", valid_number);
 
-	/* test reading an empty file - expect failure!*/
-	fd = fopen(filename,"w");
+	/* a negative value is rejected by the unsigned variant ... */
+	fd = fopen(filename, "w");
 	if (fd == NULL) {
 		printf("line %d, Error opening %s: %s\n", __LINE__, filename, strerror(errno));
 		goto error;
 	}
+	fprintf(fd, "-1\n");
 	fclose(fd);
 	fd = NULL;
-	if (eal_parse_sysfs_value(filename, &retval) == 0) {
-		printf("eal_parse_sysfs_value() read invalid value  - test failed\n");
+	if (rte_sysfs_parse_uint(&retval, "%s", filename) == 0) {
+		printf("rte_sysfs_parse_uint() accepted a negative value - test failed\n");
 		goto error;
 	}
 
-	/* test reading a valid number value *without* "\n" on the end - expect failure!*/
-	valid_number = 3;
+	/* ... and read correctly by the signed one, as for numa_node */
+	if (rte_sysfs_parse_int(&sretval, "%s", filename) < 0 || sretval != -1) {
+		printf("rte_sysfs_parse_int() failed to read -1 - test failed\n");
+		goto error;
+	}
+	printf("Read '-1' as signed ok\n");
+
+	/* string read strips the trailing newline */
+	fd = fopen(filename, "w");
+	if (fd == NULL) {
+		printf("line %d, Error opening %s: %s\n", __LINE__, filename, strerror(errno));
+		goto error;
+	}
+	fprintf(fd, "performance\n");
+	fclose(fd);
+	fd = NULL;
+	if (rte_sysfs_parse_string(strval, sizeof(strval), "%s", filename) < 0 ||
+			strcmp(strval, "performance") != 0) {
+		printf("rte_sysfs_parse_string() returned '%s' - test failed\n", strval);
+		goto error;
+	}
+	printf("Read 'performance' ok\n");
+
+	/* write it back and read it again */
+	if (rte_sysfs_write_string("powersave", "%s", filename) < 0) {
+		printf("rte_sysfs_write_string() returned error - test failed\n");
+		goto error;
+	}
+	if (rte_sysfs_parse_string(strval, sizeof(strval), "%s", filename) < 0 ||
+			strcmp(strval, "powersave") != 0) {
+		printf("read back '%s' after write - test failed\n", strval);
+		goto error;
+	}
+	printf("Wrote and read back 'powersave' ok\n");
+
+	printf("Test reading invalid values ...\n");
+
+	/* test reading an empty file - expect failure!*/
 	fd = fopen(filename,"w");
 	if (fd == NULL) {
 		printf("line %d, Error opening %s: %s\n", __LINE__, filename, strerror(errno));
 		goto error;
 	}
-	fprintf(fd,"%u", valid_number);
 	fclose(fd);
 	fd = NULL;
-	if (eal_parse_sysfs_value(filename, &retval) == 0) {
-		printf("eal_parse_sysfs_value() read invalid value  - test failed\n");
+	if (rte_sysfs_parse_uint(&retval, "%s", filename) == 0) {
+		printf("rte_sysfs_parse_uint() read invalid value  - test failed\n");
 		goto error;
 	}
 
@@ -141,8 +195,8 @@ test_parse_sysfs_value(void)
 	fprintf(fd,"%uJ\n", valid_number);
 	fclose(fd);
 	fd = NULL;
-	if (eal_parse_sysfs_value(filename, &retval) == 0) {
-		printf("eal_parse_sysfs_value() read invalid value  - test failed\n");
+	if (rte_sysfs_parse_uint(&retval, "%s", filename) == 0) {
+		printf("rte_sysfs_parse_uint() read invalid value  - test failed\n");
 		goto error;
 	}
 
@@ -155,14 +209,45 @@ test_parse_sysfs_value(void)
 	fprintf(fd,"error\n");
 	fclose(fd);
 	fd = NULL;
-	if (eal_parse_sysfs_value(filename, &retval) == 0) {
-		printf("eal_parse_sysfs_value() read invalid value  - test failed\n");
+	if (rte_sysfs_parse_uint(&retval, "%s", filename) == 0) {
+		printf("rte_sysfs_parse_uint() read invalid value  - test failed\n");
+		goto error;
+	}
+
+	/* test reading a negative value as unsigned - expect failure! */
+	fd = fopen(filename, "w");
+	if (fd == NULL) {
+		printf("line %d, Error opening %s: %s\n", __LINE__, filename, strerror(errno));
+		goto error;
+	}
+	fprintf(fd, "-1\n");
+	fclose(fd);
+	fd = NULL;
+	if (rte_sysfs_parse_uint(&retval, "%s", filename) == 0) {
+		printf("rte_sysfs_parse_uint() read negative value  - test failed\n");
+		goto error;
+	}
+
+	/*
+	 * Same, but with leading whitespace: strtoul() skips it before it
+	 * negates, so the sign has to be looked for past the whitespace.
+	 */
+	fd = fopen(filename, "w");
+	if (fd == NULL) {
+		printf("line %d, Error opening %s: %s\n", __LINE__, filename, strerror(errno));
+		goto error;
+	}
+	fprintf(fd, " -1\n");
+	fclose(fd);
+	fd = NULL;
+	if (rte_sysfs_parse_uint(&retval, "%s", filename) == 0) {
+		printf("rte_sysfs_parse_uint() read negative value  - test failed\n");
 		goto error;
 	}
 
 	close(tmp_file_handle);
 	unlink(filename);
-	printf("eal_parse_sysfs_value() - OK\n");
+	printf("sysfs value functions - OK\n");
 	return 0;
 
 error:
diff --git a/drivers/bus/auxiliary/linux/auxiliary.c b/drivers/bus/auxiliary/linux/auxiliary.c
index 3a2dca2865..c8d289eecb 100644
--- a/drivers/bus/auxiliary/linux/auxiliary.c
+++ b/drivers/bus/auxiliary/linux/auxiliary.c
@@ -9,6 +9,7 @@
 #include <rte_malloc.h>
 #include <rte_devargs.h>
 #include <rte_memcpy.h>
+#include <rte_sysfs.h>
 #include <eal_filesystem.h>
 
 #include "../private.h"
@@ -21,8 +22,7 @@ auxiliary_scan_one(const char *dirname, const char *name)
 {
 	struct rte_auxiliary_device *dev;
 	struct rte_auxiliary_device *dev2;
-	char filename[PATH_MAX];
-	unsigned long tmp;
+	long num;
 	int ret;
 
 	dev = malloc(sizeof(*dev));
@@ -36,12 +36,9 @@ auxiliary_scan_one(const char *dirname, const char *name)
 	}
 	dev->device.name = dev->name;
 
-	/* Get NUMA node, default to 0 if not present */
-	snprintf(filename, sizeof(filename), "%s/%s/numa_node",
-		 dirname, name);
-	if (access(filename, F_OK) == 0 &&
-	    eal_parse_sysfs_value(filename, &tmp) == 0)
-		dev->device.numa_node = tmp;
+	/* Get NUMA node, default to SOCKET_ID_ANY if not present */
+	if (rte_sysfs_parse_int(&num, "%s/%s/numa_node", dirname, name) == 0)
+		dev->device.numa_node = num;
 	else
 		dev->device.numa_node = SOCKET_ID_ANY;
 
diff --git a/drivers/bus/cdx/cdx.c b/drivers/bus/cdx/cdx.c
index c0b46a41ad..8cbe2473a9 100644
--- a/drivers/bus/cdx/cdx.c
+++ b/drivers/bus/cdx/cdx.c
@@ -74,6 +74,7 @@
 #include <rte_kvargs.h>
 #include <rte_malloc.h>
 #include <rte_vfio.h>
+#include <rte_sysfs.h>
 
 #include <eal_export.h>
 #include <eal_filesystem.h>
@@ -179,16 +180,14 @@ cdx_scan_one(const char *dirname, const char *dev_name)
 	}
 
 	/* get vendor id */
-	snprintf(filename, sizeof(filename), "%s/vendor", dirname);
-	if (eal_parse_sysfs_value(filename, &tmp) < 0) {
+	if (rte_sysfs_parse_uint(&tmp, "%s/vendor", dirname) < 0) {
 		ret = -1;
 		goto err;
 	}
 	dev->id.vendor_id = (uint16_t)tmp;
 
 	/* get device id */
-	snprintf(filename, sizeof(filename), "%s/device", dirname);
-	if (eal_parse_sysfs_value(filename, &tmp) < 0) {
+	if (rte_sysfs_parse_uint(&tmp, "%s/device", dirname) < 0) {
 		ret = -1;
 		goto err;
 	}
diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index 9aae0a5d14..d40a09e78a 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -12,6 +12,7 @@
 #include <rte_devargs.h>
 #include <rte_memcpy.h>
 #include <rte_vfio.h>
+#include <rte_sysfs.h>
 
 #include <eal_export.h>
 #include "eal_filesystem.h"
@@ -205,6 +206,7 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 {
 	char filename[PATH_MAX];
 	unsigned long tmp;
+	long num;
 	struct rte_pci_device_internal *pdev;
 	struct rte_pci_device *dev;
 	char driver[PATH_MAX];
@@ -221,43 +223,35 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 	dev->addr = *addr;
 
 	/* get vendor id */
-	snprintf(filename, sizeof(filename), "%s/vendor", dirname);
-	if (eal_parse_sysfs_value(filename, &tmp) < 0) {
+	if (rte_sysfs_parse_uint(&tmp, "%s/vendor", dirname) < 0) {
 		pci_free(pdev);
 		return -1;
 	}
 	dev->id.vendor_id = (uint16_t)tmp;
 
 	/* get device id */
-	snprintf(filename, sizeof(filename), "%s/device", dirname);
-	if (eal_parse_sysfs_value(filename, &tmp) < 0) {
+	if (rte_sysfs_parse_uint(&tmp, "%s/device", dirname) < 0) {
 		pci_free(pdev);
 		return -1;
 	}
 	dev->id.device_id = (uint16_t)tmp;
 
 	/* get subsystem_vendor id */
-	snprintf(filename, sizeof(filename), "%s/subsystem_vendor",
-		 dirname);
-	if (eal_parse_sysfs_value(filename, &tmp) < 0) {
+	if (rte_sysfs_parse_uint(&tmp, "%s/subsystem_vendor", dirname) < 0) {
 		pci_free(pdev);
 		return -1;
 	}
 	dev->id.subsystem_vendor_id = (uint16_t)tmp;
 
 	/* get subsystem_device id */
-	snprintf(filename, sizeof(filename), "%s/subsystem_device",
-		 dirname);
-	if (eal_parse_sysfs_value(filename, &tmp) < 0) {
+	if (rte_sysfs_parse_uint(&tmp, "%s/subsystem_device", dirname) < 0) {
 		pci_free(pdev);
 		return -1;
 	}
 	dev->id.subsystem_device_id = (uint16_t)tmp;
 
 	/* get class_id */
-	snprintf(filename, sizeof(filename), "%s/class",
-		 dirname);
-	if (eal_parse_sysfs_value(filename, &tmp) < 0) {
+	if (rte_sysfs_parse_uint(&tmp, "%s/class", dirname) < 0) {
 		pci_free(pdev);
 		return -1;
 	}
@@ -266,25 +260,15 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 
 	/* get max_vfs */
 	dev->max_vfs = 0;
-	snprintf(filename, sizeof(filename), "%s/max_vfs", dirname);
-	if (!access(filename, F_OK) &&
-	    eal_parse_sysfs_value(filename, &tmp) == 0)
+	if (rte_sysfs_parse_uint(&tmp, "%s/max_vfs", dirname) == 0)
+		dev->max_vfs = (uint16_t)tmp;
+	/* for non igb_uio driver, need kernel version >= 3.8 */
+	else if (rte_sysfs_parse_uint(&tmp, "%s/sriov_numvfs", dirname) == 0)
 		dev->max_vfs = (uint16_t)tmp;
-	else {
-		/* for non igb_uio driver, need kernel version >= 3.8 */
-		snprintf(filename, sizeof(filename),
-			 "%s/sriov_numvfs", dirname);
-		if (!access(filename, F_OK) &&
-		    eal_parse_sysfs_value(filename, &tmp) == 0)
-			dev->max_vfs = (uint16_t)tmp;
-	}
-
-	/* get numa node, default to 0 if not present */
-	snprintf(filename, sizeof(filename), "%s/numa_node", dirname);
 
-	if (access(filename, F_OK) == 0 &&
-	    eal_parse_sysfs_value(filename, &tmp) == 0)
-		dev->device.numa_node = tmp;
+	/* get numa node, default to SOCKET_ID_ANY if not present */
+	if (rte_sysfs_parse_int(&num, "%s/numa_node", dirname) == 0)
+		dev->device.numa_node = num;
 	else
 		dev->device.numa_node = SOCKET_ID_ANY;
 
diff --git a/drivers/bus/platform/platform.c b/drivers/bus/platform/platform.c
index 90d865a8df..9585fb79e9 100644
--- a/drivers/bus/platform/platform.c
+++ b/drivers/bus/platform/platform.c
@@ -24,6 +24,7 @@
 #include <rte_memory.h>
 #include <rte_string_fns.h>
 #include <rte_vfio.h>
+#include <rte_sysfs.h>
 
 #include "private.h"
 
@@ -49,8 +50,7 @@ static int
 dev_add(const char *dev_name)
 {
 	struct rte_platform_device *pdev, *tmp;
-	char path[PATH_MAX];
-	unsigned long val;
+	long val;
 
 	pdev = calloc(1, sizeof(*pdev));
 	if (pdev == NULL)
@@ -59,8 +59,10 @@ dev_add(const char *dev_name)
 	rte_strscpy(pdev->name, dev_name, sizeof(pdev->name));
 	pdev->device.name = pdev->name;
 	pdev->device.devargs = rte_bus_find_devargs(&platform_bus, dev_name);
-	snprintf(path, sizeof(path), PLATFORM_BUS_DEVICES_PATH "/%s/numa_node", dev_name);
-	pdev->device.numa_node = eal_parse_sysfs_value(path, &val) ? rte_socket_id() : val;
+	if (rte_sysfs_parse_int(&val, PLATFORM_BUS_DEVICES_PATH "/%s/numa_node", dev_name) == 0)
+		pdev->device.numa_node = val;
+	else
+		pdev->device.numa_node = rte_socket_id();
 
 	RTE_BUS_FOREACH_DEV(tmp, &platform_bus) {
 		if (!strcmp(tmp->name, pdev->name)) {
diff --git a/drivers/bus/vmbus/linux/vmbus_bus.c b/drivers/bus/vmbus/linux/vmbus_bus.c
index 779ea50b92..9ee7983eb6 100644
--- a/drivers/bus/vmbus/linux/vmbus_bus.c
+++ b/drivers/bus/vmbus/linux/vmbus_bus.c
@@ -19,6 +19,7 @@
 #include <rte_malloc.h>
 #include <rte_bus_vmbus.h>
 #include <rte_kvargs.h>
+#include <rte_sysfs.h>
 
 #include <eal_export.h>
 #include "eal_filesystem.h"
@@ -202,11 +203,8 @@ rte_vmbus_map_device(struct rte_vmbus_device *dev)
 			return -1;
 		}
 
-		snprintf(filename, sizeof(filename),
-			 "%s/size", dirname);
-		if (eal_parse_sysfs_value(filename, &len) < 0) {
-			VMBUS_LOG(ERR,
-				"could not read %s", filename);
+		if (rte_sysfs_parse_uint(&len, "%s/size", dirname) < 0) {
+			VMBUS_LOG(ERR, "could not read size of %s", dirname);
 			return -1;
 		}
 		res->len = len;
@@ -280,6 +278,7 @@ vmbus_scan_one(const char *name)
 	char filename[PATH_MAX];
 	char dirname[PATH_MAX];
 	unsigned long tmp;
+	long num;
 
 	dev = calloc(1, sizeof(*dev));
 	if (dev == NULL)
@@ -314,14 +313,12 @@ vmbus_scan_one(const char *name)
 		goto error;
 
 	/* get relid */
-	snprintf(filename, sizeof(filename), "%s/id", dirname);
-	if (eal_parse_sysfs_value(filename, &tmp) < 0)
+	if (rte_sysfs_parse_uint(&tmp, "%s/id", dirname) < 0)
 		goto error;
 	dev->relid = tmp;
 
 	/* get monitor id */
-	snprintf(filename, sizeof(filename), "%s/monitor_id", dirname);
-	if (eal_parse_sysfs_value(filename, &tmp) >= 0) {
+	if (rte_sysfs_parse_uint(&tmp, "%s/monitor_id", dirname) >= 0) {
 		dev->monitor_id = tmp;
 	} else {
 		VMBUS_LOG(NOTICE, "monitor disabled on %s", name);
@@ -333,14 +330,8 @@ vmbus_scan_one(const char *name)
 	dev->device.numa_node = SOCKET_ID_ANY;
 	if (vmbus_use_numa(dev)) {
 		/* get numa node (if present) */
-		snprintf(filename, sizeof(filename), "%s/numa_node",
-			 dirname);
-
-		if (access(filename, R_OK) == 0) {
-			if (eal_parse_sysfs_value(filename, &tmp) < 0)
-				goto error;
-			dev->device.numa_node = tmp;
-		}
+		if (rte_sysfs_parse_int(&num, "%s/numa_node", dirname) == 0)
+			dev->device.numa_node = num;
 	}
 
 	/* device is valid, add in list (sorted) */
diff --git a/drivers/common/cnxk/roc_model.c b/drivers/common/cnxk/roc_model.c
index f0312a5400..800e1ef013 100644
--- a/drivers/common/cnxk/roc_model.c
+++ b/drivers/common/cnxk/roc_model.c
@@ -105,20 +105,17 @@ is_rvu_device(unsigned long val)
 static int
 rvu_device_lookup(const char *dirname, uint32_t *part, uint32_t *pass)
 {
-	char filename[PATH_MAX];
 	unsigned long val;
 
 	/* Check if vendor id is cavium */
-	snprintf(filename, sizeof(filename), "%s/vendor", dirname);
-	if (plt_sysfs_value_parse(filename, &val) < 0)
+	if (plt_sysfs_value_parse(&val, "%s/vendor", dirname) < 0)
 		goto error;
 
 	if (val != PCI_VENDOR_ID_CAVIUM)
 		goto error;
 
 	/* Get device id  */
-	snprintf(filename, sizeof(filename), "%s/device", dirname);
-	if (plt_sysfs_value_parse(filename, &val) < 0)
+	if (plt_sysfs_value_parse(&val, "%s/device", dirname) < 0)
 		goto error;
 
 	/* Check if device ID belongs to any RVU device */
@@ -126,15 +123,13 @@ rvu_device_lookup(const char *dirname, uint32_t *part, uint32_t *pass)
 		goto error;
 
 	/* Get subsystem_device id */
-	snprintf(filename, sizeof(filename), "%s/subsystem_device", dirname);
-	if (plt_sysfs_value_parse(filename, &val) < 0)
+	if (plt_sysfs_value_parse(&val, "%s/subsystem_device", dirname) < 0)
 		goto error;
 
 	*part = val >> MODEL_CN10K_PART_SHIFT;
 
 	/* Get revision for pass value*/
-	snprintf(filename, sizeof(filename), "%s/revision", dirname);
-	if (plt_sysfs_value_parse(filename, &val) < 0)
+	if (plt_sysfs_value_parse(&val, "%s/revision", dirname) < 0)
 		goto error;
 
 	*pass = val & MODEL_CN10K_PASS_MASK;
@@ -230,31 +225,14 @@ populate_model(struct roc_model *model, uint32_t midr)
 static int
 midr_get(unsigned long *val)
 {
-	const char *file =
-		"/sys/devices/system/cpu/cpu0/regs/identification/midr_el1";
-	int rc = UTIL_ERR_FS;
-	char buf[BUFSIZ];
-	char *end = NULL;
-	FILE *f;
-
 	if (val == NULL)
-		goto err;
-	f = fopen(file, "r");
-	if (f == NULL)
-		goto err;
-
-	if (fgets(buf, sizeof(buf), f) == NULL)
-		goto fclose;
+		return UTIL_ERR_FS;
 
-	*val = strtoul(buf, &end, 0);
-	if ((buf[0] == '\0') || (end == NULL) || (*end != '\n'))
-		goto fclose;
+	if (plt_sysfs_value_parse(val,
+			"/sys/devices/system/cpu/cpu0/regs/identification/midr_el1") < 0)
+		return UTIL_ERR_FS;
 
-	rc = 0;
-fclose:
-	fclose(f);
-err:
-	return rc;
+	return 0;
 }
 
 static void
diff --git a/drivers/common/cnxk/roc_platform.h b/drivers/common/cnxk/roc_platform.h
index ac4f76473f..71f9d9a256 100644
--- a/drivers/common/cnxk/roc_platform.h
+++ b/drivers/common/cnxk/roc_platform.h
@@ -23,6 +23,7 @@
 #include <rte_seqcount.h>
 #include <rte_spinlock.h>
 #include <rte_string_fns.h>
+#include <rte_sysfs.h>
 #include <rte_tailq.h>
 #include <rte_telemetry.h>
 
@@ -109,7 +110,7 @@
 #define plt_pci_device		    rte_pci_device
 #define plt_pci_read_config	    rte_pci_read_config
 #define plt_pci_find_ext_capability rte_pci_find_ext_capability
-#define plt_sysfs_value_parse	    eal_parse_sysfs_value
+#define plt_sysfs_value_parse	    rte_sysfs_parse_uint
 
 #define plt_log2_u32	 rte_log2_u32
 #define plt_cpu_to_be_16 rte_cpu_to_be_16
diff --git a/drivers/common/mlx5/linux/mlx5_common_auxiliary.c b/drivers/common/mlx5/linux/mlx5_common_auxiliary.c
index 3ee2f4638a..07bb899f9e 100644
--- a/drivers/common/mlx5/linux/mlx5_common_auxiliary.c
+++ b/drivers/common/mlx5/linux/mlx5_common_auxiliary.c
@@ -10,6 +10,7 @@
 #include <rte_errno.h>
 #include <bus_auxiliary_driver.h>
 #include <rte_common.h>
+#include <rte_sysfs.h>
 #include <eal_export.h>
 #include "eal_filesystem.h"
 
@@ -93,16 +94,12 @@ mlx5_auxiliary_get_pci_str(const struct rte_auxiliary_device *dev,
 static int
 mlx5_auxiliary_get_numa(const struct rte_auxiliary_device *dev)
 {
-	unsigned long numa;
-	char numa_path[PATH_MAX];
+	char pci_path[PATH_MAX];
+	long numa;
 
-	if (mlx5_auxiliary_get_pci_path(dev, numa_path, sizeof(numa_path)) != 0)
+	if (mlx5_auxiliary_get_pci_path(dev, pci_path, sizeof(pci_path)) != 0)
 		return SOCKET_ID_ANY;
-	if (strcat(numa_path, "/numa_node") == NULL) {
-		rte_errno = ENAMETOOLONG;
-		return SOCKET_ID_ANY;
-	}
-	if (eal_parse_sysfs_value(numa_path, &numa) != 0) {
+	if (rte_sysfs_parse_int(&numa, "%s/numa_node", pci_path) != 0) {
 		rte_errno = EINVAL;
 		return SOCKET_ID_ANY;
 	}
diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
index 2cdb533276..9fc48e113b 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -38,6 +38,7 @@
 #include <rte_ring.h>
 #include <rte_spinlock.h>
 #include <rte_power_intrinsics.h>
+#include <rte_sysfs.h>
 
 #include "compat.h"
 #include "eal_filesystem.h"
@@ -2568,15 +2569,13 @@ rte_pmd_af_xdp_probe(struct rte_vdev_device *dev)
 
 	/* get numa node id from net sysfs */
 	if (dev->device.numa_node == SOCKET_ID_ANY) {
-		unsigned long numa = 0;
-		char numa_path[PATH_MAX];
+		long numa;
 
-		snprintf(numa_path, sizeof(numa_path), "/sys/class/net/%s/device/numa_node",
-			 if_name);
-		if (access(numa_path, R_OK) != 0 || eal_parse_sysfs_value(numa_path, &numa) != 0)
-			dev->device.numa_node = rte_socket_id();
-		else
+		if (rte_sysfs_parse_int(&numa, "/sys/class/net/%s/device/numa_node",
+					if_name) == 0)
 			dev->device.numa_node = numa;
+		else
+			dev->device.numa_node = rte_socket_id();
 	}
 
 	busy_budget = busy_budget == -1 ? ETH_AF_XDP_DFLT_BUSY_BUDGET :
diff --git a/lib/eal/common/eal_filesystem.h b/lib/eal/common/eal_filesystem.h
index 912f446f64..9859fe7241 100644
--- a/lib/eal/common/eal_filesystem.h
+++ b/lib/eal/common/eal_filesystem.h
@@ -128,8 +128,6 @@ eal_get_hugefile_list_seg_path(char *buffer, size_t buflen,
 /** define the default filename prefix for the %s values above */
 #define HUGEFILE_PREFIX_DEFAULT "rte"
 
-/** Function to read a single numeric value from a file on the filesystem.
- * Used to read information from files on /sys */
-int eal_parse_sysfs_value(const char *filename, unsigned long *val);
+/* Reading and writing of sysfs values is in <rte_sysfs.h> */
 
 #endif /* EAL_FILESYSTEM_H */
diff --git a/lib/eal/include/meson.build b/lib/eal/include/meson.build
index aef5824e5f..8c0a59f3d7 100644
--- a/lib/eal/include/meson.build
+++ b/lib/eal/include/meson.build
@@ -61,6 +61,7 @@ headers += files(
 driver_sdk_headers = files(
         'bus_driver.h',
         'dev_driver.h',
+        'rte_sysfs.h',
 )
 
 # special case install the generic headers, since they go in a subdir
diff --git a/lib/eal/include/rte_sysfs.h b/lib/eal/include/rte_sysfs.h
new file mode 100644
index 0000000000..d9b59f58c3
--- /dev/null
+++ b/lib/eal/include/rte_sysfs.h
@@ -0,0 +1,121 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2026 Stephen Hemminger
+ */
+
+#ifndef RTE_SYSFS_H
+#define RTE_SYSFS_H
+
+/**
+ * @file
+ * @internal
+ *
+ * Helpers to read and write single values in sysfs, used across DPDK.
+ *
+ * All of these build the path from a printf-style format, so that
+ * callers do not have to construct it separately.
+ */
+
+#include <stdarg.h>
+#include <stddef.h>
+
+#include <rte_common.h>
+#include <rte_compat.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * Read an unsigned numeric value from a file, typically under /sys.
+ *
+ * The value is parsed with strtoul() using base 0, so decimal, octal
+ * and 0x-prefixed hexadecimal are all accepted. A negative value is
+ * rejected rather than wrapping; use rte_sysfs_parse_int() for the
+ * attributes that are signed.
+ *
+ * @param val
+ *   Where to store the parsed value, unmodified on failure.
+ * @param format
+ *   printf-style format describing the path to read.
+ * @return
+ *   0 on success, -1 on error.
+ */
+__rte_internal
+__rte_format_printf(2, 3)
+int rte_sysfs_parse_uint(unsigned long *val, const char *format, ...);
+
+/**
+ * Read an unsigned numeric value from a file, typically under /sys.
+ *
+ * As rte_sysfs_parse_uint(), but takes a va_list, so that a wrapper
+ * can forward its arguments without formatting the path itself.
+ *
+ * @param val
+ *   Where to store the parsed value, unmodified on failure.
+ * @param format
+ *   printf-style format describing the path to read.
+ * @param ap
+ *   Arguments for the format.
+ * @return
+ *   0 on success, -1 on error.
+ */
+__rte_internal
+__rte_format_printf(2, 0)
+int rte_sysfs_vparse_uint(unsigned long *val, const char *format, va_list ap);
+
+/**
+ * Read a signed numeric value from a file, typically under /sys.
+ *
+ * The value is parsed with strtol() using base 0. Some sysfs
+ * attributes are signed, most notably "numa_node" which is -1 when
+ * the device is not associated with any NUMA node.
+ *
+ * @param val
+ *   Where to store the parsed value, unmodified on failure.
+ * @param format
+ *   printf-style format describing the path to read.
+ * @return
+ *   0 on success, -1 on error.
+ */
+__rte_internal
+__rte_format_printf(2, 3)
+int rte_sysfs_parse_int(long *val, const char *format, ...);
+
+/**
+ * Read a string value from a file, typically under /sys.
+ *
+ * The trailing newline, if any, is stripped.
+ *
+ * @param buf
+ *   Where to store the NUL-terminated value. The contents are
+ *   indeterminate on failure.
+ * @param buflen
+ *   Size of buf, the value is truncated if it does not fit.
+ * @param format
+ *   printf-style format describing the path to read.
+ * @return
+ *   0 on success, -1 on error.
+ */
+__rte_internal
+__rte_format_printf(3, 4)
+int rte_sysfs_parse_string(char *buf, size_t buflen, const char *format, ...);
+
+/**
+ * Write a string value to a file, typically under /sys.
+ *
+ * @param str
+ *   The NUL-terminated value to write.
+ * @param format
+ *   printf-style format describing the path to write.
+ * @return
+ *   0 on success, -1 on error.
+ */
+__rte_internal
+__rte_format_printf(2, 3)
+int rte_sysfs_write_string(const char *str, const char *format, ...);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* RTE_SYSFS_H */
diff --git a/lib/eal/linux/eal_hugepage_info.c b/lib/eal/linux/eal_hugepage_info.c
index 05c5b3f613..a8da0d5455 100644
--- a/lib/eal/linux/eal_hugepage_info.c
+++ b/lib/eal/linux/eal_hugepage_info.c
@@ -25,6 +25,7 @@
 #include <rte_debug.h>
 #include <rte_log.h>
 #include <rte_common.h>
+#include <rte_sysfs.h>
 #include "rte_string_fns.h"
 
 #include "eal_private.h"
@@ -68,18 +69,6 @@ create_shared_memory(const char *filename, const size_t mem_size)
 	return map_shared_memory(filename, mem_size, O_RDWR | O_CREAT);
 }
 
-static int get_hp_sysfs_value(const char *subdir, const char *file, unsigned long *val)
-{
-	char *path = NULL;
-	int ret;
-
-	if (asprintf(&path, "%s/%s/%s", sys_dir_path, subdir, file) < 0)
-		return -1;
-	ret = eal_parse_sysfs_value(path, val);
-	free(path);
-	return ret;
-}
-
 /* this function is only called from eal_hugepage_info_init which itself
  * is only called from a primary process */
 static uint32_t
@@ -92,16 +81,16 @@ get_num_hugepages(const char *subdir, size_t sz, unsigned int reusable_pages)
 	const char *nr_splus_file = "surplus_hugepages";
 
 	/* first, check how many reserved pages kernel reports */
-	if (get_hp_sysfs_value(subdir, nr_rsvd_file, &resv_pages) < 0)
+	if (rte_sysfs_parse_uint(&resv_pages, "%s/%s/%s", sys_dir_path, subdir, nr_rsvd_file) < 0)
 		return 0;
 
-	if (get_hp_sysfs_value(subdir, nr_hp_file, &num_pages) < 0)
+	if (rte_sysfs_parse_uint(&num_pages, "%s/%s/%s", sys_dir_path, subdir, nr_hp_file) < 0)
 		return 0;
 
-	if (get_hp_sysfs_value(subdir, nr_over_file, &over_pages) < 0)
+	if (rte_sysfs_parse_uint(&over_pages, "%s/%s/%s", sys_dir_path, subdir, nr_over_file) < 0)
 		over_pages = 0;
 
-	if (get_hp_sysfs_value(subdir, nr_splus_file, &surplus_pages) < 0)
+	if (rte_sysfs_parse_uint(&surplus_pages, "%s/%s/%s", sys_dir_path, subdir, nr_splus_file) < 0)
 		surplus_pages = 0;
 
 	/* adjust num_pages */
@@ -138,7 +127,7 @@ get_num_hugepages(const char *subdir, size_t sz, unsigned int reusable_pages)
 static uint32_t
 get_num_hugepages_on_node(const char *subdir, unsigned int socket, size_t sz)
 {
-	char *path = NULL, *socketpath = NULL;
+	char *socketpath = NULL;
 	DIR *socketdir;
 	unsigned long num_pages = 0;
 	const char *nr_hp_file = "free_hugepages";
@@ -158,13 +147,7 @@ get_num_hugepages_on_node(const char *subdir, unsigned int socket, size_t sz)
 		goto nopages;
 	}
 
-	if (asprintf(&path, "%s/%s/%s", socketpath, subdir, nr_hp_file) < 0) {
-		EAL_LOG(ERR, "Can not format free hugepages path");
-		path = NULL;
-		goto nopages;
-	}
-
-	if (eal_parse_sysfs_value(path, &num_pages) < 0)
+	if (rte_sysfs_parse_uint(&num_pages, "%s/%s/%s", socketpath, subdir, nr_hp_file) < 0)
 		goto nopages;
 
 	if (num_pages == 0)
@@ -179,7 +162,6 @@ get_num_hugepages_on_node(const char *subdir, unsigned int socket, size_t sz)
 		num_pages = UINT32_MAX;
 
 nopages:
-	free(path);
 	free(socketpath);
 
 	return num_pages;
diff --git a/lib/eal/linux/eal_lcore.c b/lib/eal/linux/eal_lcore.c
index 29b36dd610..ada1c408f4 100644
--- a/lib/eal/linux/eal_lcore.c
+++ b/lib/eal/linux/eal_lcore.c
@@ -6,6 +6,7 @@
 #include <limits.h>
 
 #include <rte_log.h>
+#include <rte_sysfs.h>
 
 #include "eal_private.h"
 #include "eal_filesystem.h"
@@ -57,18 +58,13 @@ eal_cpu_socket_id(unsigned lcore_id)
 unsigned
 eal_cpu_core_id(unsigned lcore_id)
 {
-	char path[PATH_MAX];
 	unsigned long id;
 
-	int len = snprintf(path, sizeof(path), SYS_CPU_DIR "/%s", lcore_id, CORE_ID_FILE);
-	if (len <= 0 || (unsigned)len >= sizeof(path))
-		goto err;
-	if (eal_parse_sysfs_value(path, &id) != 0)
-		goto err;
-	return (unsigned)id;
+	if (rte_sysfs_parse_uint(&id, SYS_CPU_DIR "/%s", lcore_id, CORE_ID_FILE) != 0) {
+		EAL_LOG(ERR, "Error reading core id value from %s "
+				"for lcore %u - assuming core 0", SYS_CPU_DIR, lcore_id);
+		return 0;
+	}
 
-err:
-	EAL_LOG(ERR, "Error reading core id value from %s "
-			"for lcore %u - assuming core 0", SYS_CPU_DIR, lcore_id);
-	return 0;
+	return (unsigned int)id;
 }
diff --git a/lib/eal/unix/eal_filesystem.c b/lib/eal/unix/eal_filesystem.c
index 6b8451cd3e..c6e827f805 100644
--- a/lib/eal/unix/eal_filesystem.c
+++ b/lib/eal/unix/eal_filesystem.c
@@ -76,34 +76,3 @@ int eal_create_runtime_dir(void)
 
 	return 0;
 }
-
-/* parse a sysfs (or other) file containing one integer value */
-RTE_EXPORT_SYMBOL(eal_parse_sysfs_value)
-int eal_parse_sysfs_value(const char *filename, unsigned long *val)
-{
-	FILE *f;
-	char buf[BUFSIZ];
-	char *end = NULL;
-
-	if ((f = fopen(filename, "r")) == NULL) {
-		EAL_LOG(ERR, "%s(): cannot open sysfs value %s",
-			__func__, filename);
-		return -1;
-	}
-
-	if (fgets(buf, sizeof(buf), f) == NULL) {
-		EAL_LOG(ERR, "%s(): cannot read sysfs value %s",
-			__func__, filename);
-		fclose(f);
-		return -1;
-	}
-	*val = strtoul(buf, &end, 0);
-	if ((buf[0] == '\0') || (end == NULL) || (*end != '\n')) {
-		EAL_LOG(ERR, "%s(): cannot parse sysfs value %s",
-				__func__, filename);
-		fclose(f);
-		return -1;
-	}
-	fclose(f);
-	return 0;
-}
diff --git a/lib/eal/unix/eal_unix_sysfs.c b/lib/eal/unix/eal_unix_sysfs.c
new file mode 100644
index 0000000000..444155b793
--- /dev/null
+++ b/lib/eal/unix/eal_unix_sysfs.c
@@ -0,0 +1,187 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2026 Stephen Hemminger
+ */
+
+#include <ctype.h>
+#include <errno.h>
+#include <limits.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <rte_log.h>
+#include <rte_sysfs.h>
+
+#include <eal_export.h>
+#include "eal_private.h"
+
+/* build the path from the format, then read the first line of that file */
+static int
+sysfs_read_line(char *buf, size_t buflen, const char *format, va_list ap)
+{
+	char path[PATH_MAX];
+	FILE *f;
+	int len;
+
+	len = vsnprintf(path, sizeof(path), format, ap);
+	if (len < 0 || len >= (int)sizeof(path)) {
+		EAL_LOG(ERR, "sysfs path too long");
+		return -1;
+	}
+
+	f = fopen(path, "r");
+	if (f == NULL) {
+		/*
+		 * A missing attribute is normal: callers probe for optional
+		 * ones such as max_vfs or numa_node. Anything else, such as
+		 * a permission problem, is worth reporting.
+		 */
+		if (errno == ENOENT)
+			EAL_LOG(DEBUG, "cannot open %s: %s", path, strerror(errno));
+		else
+			EAL_LOG(ERR, "cannot open %s: %s", path, strerror(errno));
+		return -1;
+	}
+
+	if (fgets(buf, buflen, f) == NULL) {
+		EAL_LOG(ERR, "cannot read %s", path);
+		fclose(f);
+		return -1;
+	}
+	fclose(f);
+
+	/* sysfs values are newline terminated, strip it */
+	*strchrnul(buf, '\n') = '\0';
+
+	return 0;
+}
+
+RTE_EXPORT_INTERNAL_SYMBOL(rte_sysfs_vparse_uint)
+int
+rte_sysfs_vparse_uint(unsigned long *val, const char *format, va_list ap)
+{
+	const char *start;
+	char buf[BUFSIZ];
+	unsigned long tmp;
+	char *end;
+
+	if (sysfs_read_line(buf, sizeof(buf), format, ap) < 0)
+		return -1;
+
+	/*
+	 * strtoul() skips leading whitespace and then silently negates a
+	 * leading '-', so " -1" would come back as ULONG_MAX. Look for the
+	 * sign past any whitespace: attributes that are really signed, such
+	 * as numa_node, must use rte_sysfs_parse_int() instead.
+	 */
+	start = buf;
+	while (isspace((unsigned char)*start))
+		++start;
+
+	errno = 0;
+	tmp = strtoul(start, &end, 0);
+	if (end == start || *end != '\0' || errno != 0 || *start == '-') {
+		EAL_LOG(ERR, "cannot parse sysfs value '%s'", buf);
+		return -1;
+	}
+
+	*val = tmp;
+	return 0;
+}
+
+RTE_EXPORT_INTERNAL_SYMBOL(rte_sysfs_parse_uint)
+int
+rte_sysfs_parse_uint(unsigned long *val, const char *format, ...)
+{
+	va_list ap;
+	int ret;
+
+	va_start(ap, format);
+	ret = rte_sysfs_vparse_uint(val, format, ap);
+	va_end(ap);
+
+	return ret;
+}
+
+RTE_EXPORT_INTERNAL_SYMBOL(rte_sysfs_parse_int)
+int
+rte_sysfs_parse_int(long *val, const char *format, ...)
+{
+	char buf[BUFSIZ];
+	va_list ap;
+	char *end;
+	long tmp;
+	int ret;
+
+	va_start(ap, format);
+	ret = sysfs_read_line(buf, sizeof(buf), format, ap);
+	va_end(ap);
+	if (ret < 0)
+		return -1;
+
+	errno = 0;
+	tmp = strtol(buf, &end, 0);
+	if (end == buf || *end != '\0' || errno != 0) {
+		EAL_LOG(ERR, "cannot parse sysfs value '%s'", buf);
+		return -1;
+	}
+
+	*val = tmp;
+	return 0;
+}
+
+RTE_EXPORT_INTERNAL_SYMBOL(rte_sysfs_parse_string)
+int
+rte_sysfs_parse_string(char *buf, size_t buflen, const char *format, ...)
+{
+	va_list ap;
+	int ret;
+
+	va_start(ap, format);
+	ret = sysfs_read_line(buf, buflen, format, ap);
+	va_end(ap);
+
+	return ret;
+}
+
+RTE_EXPORT_INTERNAL_SYMBOL(rte_sysfs_write_string)
+int
+rte_sysfs_write_string(const char *str, const char *format, ...)
+{
+	char path[PATH_MAX];
+	va_list ap;
+	FILE *f;
+	int len;
+
+	va_start(ap, format);
+	len = vsnprintf(path, sizeof(path), format, ap);
+	va_end(ap);
+	if (len < 0 || len >= (int)sizeof(path)) {
+		EAL_LOG(ERR, "sysfs path too long");
+		return -1;
+	}
+
+	f = fopen(path, "w");
+	if (f == NULL) {
+		if (errno == ENOENT)
+			EAL_LOG(DEBUG, "cannot open %s: %s", path, strerror(errno));
+		else
+			EAL_LOG(ERR, "cannot open %s: %s", path, strerror(errno));
+		return -1;
+	}
+
+	if (fputs(str, f) < 0) {
+		EAL_LOG(ERR, "cannot write '%s' to %s", str, path);
+		fclose(f);
+		return -1;
+	}
+
+	/* errors on a sysfs write are reported at close time */
+	if (fclose(f) != 0) {
+		EAL_LOG(ERR, "cannot write '%s' to %s: %s", str, path, strerror(errno));
+		return -1;
+	}
+
+	return 0;
+}
diff --git a/lib/eal/unix/meson.build b/lib/eal/unix/meson.build
index 70af352dab..2498e9266e 100644
--- a/lib/eal/unix/meson.build
+++ b/lib/eal/unix/meson.build
@@ -7,6 +7,7 @@ sources += files(
         'eal_filesystem.c',
         'eal_firmware.c',
         'eal_unix_memory.c',
+        'eal_unix_sysfs.c',
         'eal_unix_thread.c',
         'eal_unix_timer.c',
         'rte_basename.c',
-- 
2.53.0

[PATCH 2/9] dma/idxd: use common sysfs routines

[Stephen Hemminger]

The three local helpers each rebuilt a path and read a value; they
now wrap the EAL routines instead. Note that "numa_node" is -1 when
the device is not tied to a node, so the integer reads use the
signed routine.

The wq "size" and "max_batch_size" attributes were read with
fscanf("%u") and are now converted with base 0, so a value with a
leading zero would parse as octal. The kernel does not print either
with leading zeros.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/dma/idxd/idxd_bus.c | 80 ++++++-------------------------------
 1 file changed, 12 insertions(+), 68 deletions(-)

diff --git a/drivers/dma/idxd/idxd_bus.c b/drivers/dma/idxd/idxd_bus.c
index 2ec526ec09..684dee3d6d 100644
--- a/drivers/dma/idxd/idxd_bus.c
+++ b/drivers/dma/idxd/idxd_bus.c
@@ -16,6 +16,7 @@
 #include <rte_log.h>
 #include <rte_dmadev_pmd.h>
 #include <rte_string_fns.h>
+#include <rte_sysfs.h>
 
 #include "idxd_internal.h"
 
@@ -121,82 +122,24 @@ static int
 read_wq_string(const struct rte_dsa_device *dev, const char *filename,
 		char *value, size_t valuelen)
 {
-	char sysfs_node[PATH_MAX];
-	int len;
-	int fd;
-
-	snprintf(sysfs_node, sizeof(sysfs_node), "%s/%s/%s",
+	return rte_sysfs_parse_string(value, valuelen, "%s/%s/%s",
 			dsa_get_sysfs_path(), dev->wq_name, filename);
-	fd = open(sysfs_node, O_RDONLY);
-	if (fd < 0) {
-		IDXD_PMD_ERR("%s(): opening file '%s' failed: %s",
-				__func__, sysfs_node, strerror(errno));
-		return -1;
-	}
-
-	len = read(fd, value, valuelen - 1);
-	close(fd);
-	if (len < 0) {
-		IDXD_PMD_ERR("%s(): error reading file '%s': %s",
-				__func__, sysfs_node, strerror(errno));
-		return -1;
-	}
-	value[len] = '\0';
-	return 0;
 }
 
 static int
 read_wq_int(struct rte_dsa_device *dev, const char *filename,
-		int *value)
+		long *value)
 {
-	char sysfs_node[PATH_MAX];
-	FILE *f;
-	int ret = 0;
-
-	snprintf(sysfs_node, sizeof(sysfs_node), "%s/%s/%s",
+	return rte_sysfs_parse_int(value, "%s/%s/%s",
 			dsa_get_sysfs_path(), dev->wq_name, filename);
-	f = fopen(sysfs_node, "r");
-	if (f == NULL) {
-		IDXD_PMD_ERR("%s(): opening file '%s' failed: %s",
-				__func__, sysfs_node, strerror(errno));
-		return -1;
-	}
-
-	if (fscanf(f, "%d", value) != 1) {
-		IDXD_PMD_ERR("%s(): error reading file '%s': %s",
-				__func__, sysfs_node, strerror(errno));
-		ret = -1;
-	}
-
-	fclose(f);
-	return ret;
 }
 
 static int
 read_device_int(struct rte_dsa_device *dev, const char *filename,
-		int *value)
+		long *value)
 {
-	char sysfs_node[PATH_MAX];
-	FILE *f;
-	int ret = 0;
-
-	snprintf(sysfs_node, sizeof(sysfs_node), "%s/dsa%d/%s",
+	return rte_sysfs_parse_int(value, "%s/dsa%d/%s",
 			dsa_get_sysfs_path(), dev->addr.device_id, filename);
-	f = fopen(sysfs_node, "r");
-	if (f == NULL) {
-		IDXD_PMD_ERR("%s(): opening file '%s' failed: %s",
-				__func__, sysfs_node, strerror(errno));
-		return -1;
-	}
-
-	if (fscanf(f, "%d", value) != 1) {
-		IDXD_PMD_ERR("%s(): error reading file '%s': %s",
-				__func__, sysfs_node, strerror(errno));
-		ret = -1;
-	}
-
-	fclose(f);
-	return ret;
 }
 
 static int
@@ -205,15 +148,16 @@ dsa_probe_device(__rte_unused struct rte_driver *drv, struct rte_device *dev)
 	struct rte_dsa_device *dsa_dev = RTE_BUS_DEVICE(dev, *dsa_dev);
 	struct idxd_dmadev idxd = {0};
 	int ret = 0;
+	long val;
 
 	IDXD_PMD_INFO("Probing device %s on numa node %d",
 			dsa_dev->wq_name, dsa_dev->device.numa_node);
-	if (read_wq_int(dsa_dev, "size", &ret) < 0)
+	if (read_wq_int(dsa_dev, "size", &val) < 0)
 		return -1;
-	idxd.max_batches = ret;
-	if (read_wq_int(dsa_dev, "max_batch_size", &ret) < 0)
+	idxd.max_batches = val;
+	if (read_wq_int(dsa_dev, "max_batch_size", &val) < 0)
 		return -1;
-	idxd.max_batch_size = ret;
+	idxd.max_batch_size = val;
 	idxd.qid = dsa_dev->addr.wq_id;
 	idxd.u.bus.dsa_id = dsa_dev->addr.device_id;
 	idxd.sva_support = 1;
@@ -286,7 +230,7 @@ dsa_scan(void)
 
 	while ((wq = readdir(dev_dir)) != NULL) {
 		struct rte_dsa_device *dev;
-		int numa_node = SOCKET_ID_ANY;
+		long numa_node = SOCKET_ID_ANY;
 
 		if (strncmp(wq->d_name, "wq", 2) != 0)
 			continue;
-- 
2.53.0

[PATCH 3/9] common/ionic: use common sysfs routines

[Stephen Hemminger]

The three resource helpers and the UIO name lookup each built a path
with an unchecked sprintf() into a 64 byte buffer and then read the
value with fscanf(). Use the EAL routines instead; base 0 conversion
handles the "0x" prefix these files use.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/common/ionic/ionic_common_uio.c | 63 +++++--------------------
 1 file changed, 12 insertions(+), 51 deletions(-)

diff --git a/drivers/common/ionic/ionic_common_uio.c b/drivers/common/ionic/ionic_common_uio.c
index aaefab918c..a796281bdd 100644
--- a/drivers/common/ionic/ionic_common_uio.c
+++ b/drivers/common/ionic/ionic_common_uio.c
@@ -19,6 +19,7 @@
 #include <rte_common.h>
 #include <rte_eal.h>
 #include <rte_string_fns.h>
+#include <rte_sysfs.h>
 
 #include "ionic_common.h"
 
@@ -63,30 +64,17 @@ struct uio_name {
 static void
 uio_fill_name_cache(struct uio_name *name_cache, const char *pfx)
 {
-	char file[64];
-	FILE *fp;
-	char *ret;
 	int name_idx = 0;
 	int i;
 
 	for (i = 0; i < IONIC_UIO_MAX_TRIES &&
 			name_idx < IONIC_MAX_DEVICES; i++) {
-		sprintf(file, "/sys/class/uio/uio%d/name", i);
-
-		fp = fopen(file, "r");
-		if (fp == NULL)
-			continue;
-
-		ret = fgets(name_cache[name_idx].name, IONIC_MAX_NAME_LEN, fp);
-		if (ret == NULL) {
-			fclose(fp);
+		if (rte_sysfs_parse_string(name_cache[name_idx].name, IONIC_MAX_NAME_LEN,
+				"/sys/class/uio/uio%d/name", i) < 0)
 			continue;
-		}
 
 		name_cache[name_idx].idx = i;
 
-		fclose(fp);
-
 		if (strncmp(name_cache[name_idx].name, pfx, strlen(pfx)) == 0)
 			name_idx++;
 	}
@@ -215,21 +203,12 @@ static unsigned long
 uio_get_res_size(int uio_idx, int res_idx)
 {
 	unsigned long size;
-	char file[64];
-	FILE *fp;
-
-	sprintf(file, "/sys/class/uio/uio%d/maps/map%d/size",
-		uio_idx, res_idx);
 
-	fp = fopen(file, "r");
-	if (fp == NULL)
+	/* zero is the error value for all of these */
+	if (rte_sysfs_parse_uint(&size, "/sys/class/uio/uio%d/maps/map%d/size",
+			uio_idx, res_idx) < 0)
 		return 0;
 
-	if (fscanf(fp, "0x%lx", &size) != 1)
-		size = 0;
-
-	fclose(fp);
-
 	return size;
 }
 
@@ -237,21 +216,12 @@ static unsigned long
 uio_get_res_phy_addr_offs(int uio_idx, int res_idx)
 {
 	unsigned long offset;
-	char file[64];
-	FILE *fp;
-
-	sprintf(file, "/sys/class/uio/uio%d/maps/map%d/offset",
-		uio_idx, res_idx);
 
-	fp = fopen(file, "r");
-	if (fp == NULL)
+	/* zero is the error value for all of these */
+	if (rte_sysfs_parse_uint(&offset, "/sys/class/uio/uio%d/maps/map%d/offset",
+			uio_idx, res_idx) < 0)
 		return 0;
 
-	if (fscanf(fp, "0x%lx", &offset) != 1)
-		offset = 0;
-
-	fclose(fp);
-
 	return offset;
 }
 
@@ -259,21 +229,12 @@ static unsigned long
 uio_get_res_phy_addr(int uio_idx, int res_idx)
 {
 	unsigned long addr;
-	char file[64];
-	FILE *fp;
 
-	sprintf(file, "/sys/class/uio/uio%d/maps/map%d/addr",
-		uio_idx, res_idx);
-
-	fp = fopen(file, "r");
-	if (fp == NULL)
+	/* zero is the error value for all of these */
+	if (rte_sysfs_parse_uint(&addr, "/sys/class/uio/uio%d/maps/map%d/addr",
+			uio_idx, res_idx) < 0)
 		return 0;
 
-	if (fscanf(fp, "0x%lx", &addr) != 1)
-		addr = 0;
-
-	fclose(fp);
-
 	return addr;
 }
 
-- 
2.53.0

[PATCH 4/9] bus/vmbus: use common sysfs routines

[Stephen Hemminger]

Replace the open coded read in vmbus_uio_sysfs_read() with the EAL
routine. The range check and the -ERANGE return are unchanged.

Two things do change. On an open failure the helper now returns -EIO
rather than -errno; both callers only pass it to strerror(-err) in a
log message, so this is message text only. And subchannel_id and
monitor_id were read with fscanf("%u") and are now converted with
base 0, so a value with a leading zero would parse as octal. Neither
attribute is printed with leading zeros by the kernel.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/bus/vmbus/linux/vmbus_uio.c | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/drivers/bus/vmbus/linux/vmbus_uio.c b/drivers/bus/vmbus/linux/vmbus_uio.c
index fbafc5027d..50e58f9b65 100644
--- a/drivers/bus/vmbus/linux/vmbus_uio.c
+++ b/drivers/bus/vmbus/linux/vmbus_uio.c
@@ -18,6 +18,7 @@
 #include <rte_malloc.h>
 #include <rte_bus_vmbus.h>
 #include <rte_string_fns.h>
+#include <rte_sysfs.h>
 
 #include "private.h"
 
@@ -334,27 +335,12 @@ int vmbus_uio_map_rings(struct vmbus_channel *chan)
 static int vmbus_uio_sysfs_read(const char *dir, const char *name,
 				unsigned long *val, unsigned long max_range)
 {
-	char path[PATH_MAX];
-	FILE *f;
-	int ret;
-
-	snprintf(path, sizeof(path), "%s/%s", dir, name);
-	f = fopen(path, "r");
-	if (!f) {
-		VMBUS_LOG(ERR, "can't open %s:%s",
-			  path, strerror(errno));
-		return -errno;
+	if (rte_sysfs_parse_uint(val, "%s/%s", dir, name) < 0) {
+		VMBUS_LOG(ERR, "can't read %s/%s", dir, name);
+		return -EIO;
 	}
 
-	if (fscanf(f, "%lu", val) != 1)
-		ret = -EIO;
-	else if (*val > max_range)
-		ret = -ERANGE;
-	else
-		ret = 0;
-	fclose(f);
-
-	return ret;
+	return *val > max_range ? -ERANGE : 0;
 }
 
 static bool vmbus_uio_ring_present(const struct rte_vmbus_device *dev,
-- 
2.53.0

[PATCH 5/9] power: use common sysfs routines

[Stephen Hemminger]

The power library and its backends open a sysfs file, read one value
from it, and close it again in many places. Where the file is only
read once, use the EAL routines instead.

Add power_sysfs_read_u32() as a thin wrapper so the backends keep
reading into a uint32_t.

The files that are held open for the lifetime of the device, the
"rw+" handles used to repeatedly read and write the scaling
frequency, are left alone: they are not a single read, and closing
and reopening them on every frequency change would be a behaviour
change on a hot path.

This also removes a few cases where the FILE pointer was left
uninitialised and then tested against NULL in the cleanup path.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/power/acpi/acpi_cpufreq.c             | 21 ++--
 drivers/power/amd_pstate/amd_pstate_cpufreq.c | 63 ++----------
 drivers/power/cppc/cppc_cpufreq.c             | 69 ++-----------
 .../power/intel_pstate/intel_pstate_cpufreq.c | 99 +++----------------
 drivers/power/intel_uncore/intel_uncore.c     | 32 ++----
 lib/power/power_common.c                      | 46 ++++-----
 lib/power/power_common.h                      |  6 ++
 lib/power/rte_power_qos.c                     | 29 +-----
 8 files changed, 81 insertions(+), 284 deletions(-)

diff --git a/drivers/power/acpi/acpi_cpufreq.c b/drivers/power/acpi/acpi_cpufreq.c
index af85a8cdec..84e33375d0 100644
--- a/drivers/power/acpi/acpi_cpufreq.c
+++ b/drivers/power/acpi/acpi_cpufreq.c
@@ -9,6 +9,7 @@
 #include <rte_memcpy.h>
 #include <rte_stdatomic.h>
 #include <rte_string_fns.h>
+#include <rte_sysfs.h>
 
 #include "acpi_cpufreq.h"
 #include "power_common.h"
@@ -111,21 +112,14 @@ power_set_governor_original(struct acpi_power_info *pi)
 static int
 power_get_available_freqs(struct acpi_power_info *pi)
 {
-	FILE *f;
+	char *freqs[RTE_MAX_LCORE_FREQS];
 	int ret = -1, i, count;
-	char *p;
 	char buf[BUFSIZ];
-	char *freqs[RTE_MAX_LCORE_FREQS];
-
-	open_core_sysfs_file(&f, "r", POWER_SYSFILE_AVAIL_FREQ, pi->lcore_id);
-	if (f == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_AVAIL_FREQ);
-		goto out;
-	}
+	char *p;
 
-	ret = read_core_sysfs_s(f, buf, sizeof(buf));
-	if ((ret) < 0) {
+	ret = rte_sysfs_parse_string(buf, sizeof(buf), POWER_SYSFILE_AVAIL_FREQ,
+			pi->lcore_id);
+	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_AVAIL_FREQ);
 		goto out;
@@ -169,9 +163,6 @@ power_get_available_freqs(struct acpi_power_info *pi)
 	POWER_DEBUG_LOG("%d frequency(s) of lcore %u are available",
 			count, pi->lcore_id);
 out:
-	if (f != NULL)
-		fclose(f);
-
 	return ret;
 }
 
diff --git a/drivers/power/amd_pstate/amd_pstate_cpufreq.c b/drivers/power/amd_pstate/amd_pstate_cpufreq.c
index af9c1309f3..1d9d4cb495 100644
--- a/drivers/power/amd_pstate/amd_pstate_cpufreq.c
+++ b/drivers/power/amd_pstate/amd_pstate_cpufreq.c
@@ -103,34 +103,19 @@ power_set_governor_userspace(struct amd_pstate_power_info *pi)
 static int
 power_check_turbo(struct amd_pstate_power_info *pi)
 {
-	FILE *f_nom = NULL, *f_max = NULL;
 	int ret = -1;
 	uint32_t nominal_perf = 0, highest_perf = 0;
 
-	open_core_sysfs_file(&f_max, "r", POWER_SYSFILE_HIGHEST_PERF,
+	ret = power_sysfs_read_u32(&highest_perf, POWER_SYSFILE_HIGHEST_PERF,
 			pi->lcore_id);
-	if (f_max == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_HIGHEST_PERF);
-		goto err;
-	}
-
-	open_core_sysfs_file(&f_nom, "r", POWER_SYSFILE_NOMINAL_PERF,
-			pi->lcore_id);
-	if (f_nom == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_NOMINAL_PERF);
-		goto err;
-	}
-
-	ret = read_core_sysfs_u32(f_max, &highest_perf);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_HIGHEST_PERF);
 		goto err;
 	}
 
-	ret = read_core_sysfs_u32(f_nom, &nominal_perf);
+	ret = power_sysfs_read_u32(&nominal_perf, POWER_SYSFILE_NOMINAL_PERF,
+			pi->lcore_id);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_NOMINAL_PERF);
@@ -156,10 +141,6 @@ power_check_turbo(struct amd_pstate_power_info *pi)
 	}
 
 err:
-	if (f_max != NULL)
-		fclose(f_max);
-	if (f_nom != NULL)
-		fclose(f_nom);
 
 	return ret;
 }
@@ -171,52 +152,30 @@ power_check_turbo(struct amd_pstate_power_info *pi)
 static int
 power_get_available_freqs(struct amd_pstate_power_info *pi)
 {
-	FILE *f_min = NULL, *f_max = NULL, *f_nom = NULL;
 	int ret = -1, nominal_idx = -1;
 	uint32_t scaling_min_freq = 0, scaling_max_freq = 0;
 	uint32_t i, num_freqs = RTE_MAX_LCORE_FREQS;
 	uint32_t nominal_freq = 0, scaling_freq = 0;
 	uint32_t freq_calc = 0;
 
-	open_core_sysfs_file(&f_max, "r", POWER_SYSFILE_SCALING_MAX_FREQ,
+	ret = power_sysfs_read_u32(&scaling_max_freq, POWER_SYSFILE_SCALING_MAX_FREQ,
 			pi->lcore_id);
-	if (f_max == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_SCALING_MAX_FREQ);
-		goto out;
-	}
-
-	open_core_sysfs_file(&f_min, "r", POWER_SYSFILE_SCALING_MIN_FREQ,
-			pi->lcore_id);
-	if (f_min == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_SCALING_MIN_FREQ);
-		goto out;
-	}
-
-	open_core_sysfs_file(&f_nom, "r", POWER_SYSFILE_NOMINAL_FREQ,
-			pi->lcore_id);
-	if (f_nom == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_NOMINAL_FREQ);
-		goto out;
-	}
-
-	ret = read_core_sysfs_u32(f_max, &scaling_max_freq);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_SCALING_MAX_FREQ);
 		goto out;
 	}
 
-	ret = read_core_sysfs_u32(f_min, &scaling_min_freq);
+	ret = power_sysfs_read_u32(&scaling_min_freq, POWER_SYSFILE_SCALING_MIN_FREQ,
+			pi->lcore_id);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_SCALING_MIN_FREQ);
 		goto out;
 	}
 
-	ret = read_core_sysfs_u32(f_nom, &nominal_freq);
+	ret = power_sysfs_read_u32(&nominal_freq, POWER_SYSFILE_NOMINAL_FREQ,
+			pi->lcore_id);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_NOMINAL_FREQ);
@@ -272,12 +231,6 @@ power_get_available_freqs(struct amd_pstate_power_info *pi)
 			num_freqs, pi->lcore_id);
 
 out:
-	if (f_min != NULL)
-		fclose(f_min);
-	if (f_max != NULL)
-		fclose(f_max);
-	if (f_nom != NULL)
-		fclose(f_nom);
 
 	return ret;
 }
diff --git a/drivers/power/cppc/cppc_cpufreq.c b/drivers/power/cppc/cppc_cpufreq.c
index aed44c1212..a25dc7eaf1 100644
--- a/drivers/power/cppc/cppc_cpufreq.c
+++ b/drivers/power/cppc/cppc_cpufreq.c
@@ -106,49 +106,27 @@ power_set_governor_userspace(struct cppc_power_info *pi)
 static int
 power_check_turbo(struct cppc_power_info *pi)
 {
-	FILE *f_nom = NULL, *f_max = NULL, *f_cmax = NULL;
-	int ret = -1;
 	uint32_t nominal_perf = 0, highest_perf = 0, cpuinfo_max_freq = 0;
+	int ret = -1;
 
-	open_core_sysfs_file(&f_max, "r", POWER_SYSFILE_HIGHEST_PERF,
-			pi->lcore_id);
-	if (f_max == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_HIGHEST_PERF);
-		goto err;
-	}
-
-	open_core_sysfs_file(&f_nom, "r", POWER_SYSFILE_NOMINAL_PERF,
-			pi->lcore_id);
-	if (f_nom == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_NOMINAL_PERF);
-		goto err;
-	}
-
-	open_core_sysfs_file(&f_cmax, "r", POWER_SYSFILE_SYS_MAX,
+	ret = power_sysfs_read_u32(&highest_perf, POWER_SYSFILE_HIGHEST_PERF,
 			pi->lcore_id);
-	if (f_cmax == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_SYS_MAX);
-		goto err;
-	}
-
-	ret = read_core_sysfs_u32(f_max, &highest_perf);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_HIGHEST_PERF);
 		goto err;
 	}
 
-	ret = read_core_sysfs_u32(f_nom, &nominal_perf);
+	ret = power_sysfs_read_u32(&nominal_perf, POWER_SYSFILE_NOMINAL_PERF,
+			pi->lcore_id);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_NOMINAL_PERF);
 		goto err;
 	}
 
-	ret = read_core_sysfs_u32(f_cmax, &cpuinfo_max_freq);
+	ret = power_sysfs_read_u32(&cpuinfo_max_freq, POWER_SYSFILE_SYS_MAX,
+			pi->lcore_id);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_SYS_MAX);
@@ -175,13 +153,6 @@ power_check_turbo(struct cppc_power_info *pi)
 	}
 
 err:
-	if (f_max != NULL)
-		fclose(f_max);
-	if (f_nom != NULL)
-		fclose(f_nom);
-	if (f_cmax != NULL)
-		fclose(f_cmax);
-
 	return ret;
 }
 
@@ -192,35 +163,20 @@ power_check_turbo(struct cppc_power_info *pi)
 static int
 power_get_available_freqs(struct cppc_power_info *pi)
 {
-	FILE *f_min = NULL, *f_max = NULL;
-	int ret = -1;
 	uint32_t scaling_min_freq = 0, scaling_max_freq = 0, nominal_perf = 0;
 	uint32_t i, num_freqs = 0;
+	int ret = -1;
 
-	open_core_sysfs_file(&f_max, "r", POWER_SYSFILE_SCALING_MAX_FREQ,
-			pi->lcore_id);
-	if (f_max == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_SCALING_MAX_FREQ);
-		goto out;
-	}
-
-	open_core_sysfs_file(&f_min, "r", POWER_SYSFILE_SCALING_MIN_FREQ,
+	ret = power_sysfs_read_u32(&scaling_max_freq, POWER_SYSFILE_SCALING_MAX_FREQ,
 			pi->lcore_id);
-	if (f_min == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_SCALING_MIN_FREQ);
-		goto out;
-	}
-
-	ret = read_core_sysfs_u32(f_max, &scaling_max_freq);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_SCALING_MAX_FREQ);
 		goto out;
 	}
 
-	ret = read_core_sysfs_u32(f_min, &scaling_min_freq);
+	ret = power_sysfs_read_u32(&scaling_min_freq, POWER_SYSFILE_SCALING_MIN_FREQ,
+			pi->lcore_id);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_SCALING_MIN_FREQ);
@@ -260,11 +216,6 @@ power_get_available_freqs(struct cppc_power_info *pi)
 			num_freqs, pi->lcore_id);
 
 out:
-	if (f_min != NULL)
-		fclose(f_min);
-	if (f_max != NULL)
-		fclose(f_max);
-
 	return ret;
 }
 
diff --git a/drivers/power/intel_pstate/intel_pstate_cpufreq.c b/drivers/power/intel_pstate/intel_pstate_cpufreq.c
index dfbb5635a1..c8d0ed2c21 100644
--- a/drivers/power/intel_pstate/intel_pstate_cpufreq.c
+++ b/drivers/power/intel_pstate/intel_pstate_cpufreq.c
@@ -108,29 +108,12 @@ out:	close(fd);
 static int
 power_init_for_setting_freq(struct pstate_power_info *pi)
 {
-	FILE *f_base = NULL, *f_base_min = NULL, *f_base_max = NULL,
-	     *f_min = NULL, *f_max = NULL;
+	FILE *f_min = NULL, *f_max = NULL;
 	uint32_t base_ratio, base_min_ratio, base_max_ratio;
 	uint64_t max_non_turbo = 0;
 	int ret;
 
-	/* open all files we expect to have open */
-	open_core_sysfs_file(&f_base_max, "r", POWER_SYSFILE_BASE_MAX_FREQ,
-			pi->lcore_id);
-	if (f_base_max == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_BASE_MAX_FREQ);
-		goto err;
-	}
-
-	open_core_sysfs_file(&f_base_min, "r", POWER_SYSFILE_BASE_MIN_FREQ,
-			pi->lcore_id);
-	if (f_base_min == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_BASE_MIN_FREQ);
-		goto err;
-	}
-
+	/* open the files that are kept open for setting the frequency */
 	open_core_sysfs_file(&f_min, "rw+", POWER_SYSFILE_MIN_FREQ,
 			pi->lcore_id);
 	if (f_min == NULL) {
@@ -147,12 +130,9 @@ power_init_for_setting_freq(struct pstate_power_info *pi)
 		goto err;
 	}
 
-	open_core_sysfs_file(&f_base, "r", POWER_SYSFILE_BASE_FREQ,
-			pi->lcore_id);
-	/* base ratio file may not exist in some kernels, so no error check */
-
 	/* read base max ratio */
-	ret = read_core_sysfs_u32(f_base_max, &base_max_ratio);
+	ret = power_sysfs_read_u32(&base_max_ratio, POWER_SYSFILE_BASE_MAX_FREQ,
+			pi->lcore_id);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_BASE_MAX_FREQ);
@@ -160,24 +140,18 @@ power_init_for_setting_freq(struct pstate_power_info *pi)
 	}
 
 	/* read base min ratio */
-	ret = read_core_sysfs_u32(f_base_min, &base_min_ratio);
+	ret = power_sysfs_read_u32(&base_min_ratio, POWER_SYSFILE_BASE_MIN_FREQ,
+			pi->lcore_id);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_BASE_MIN_FREQ);
 		goto err;
 	}
 
-	/* base ratio may not exist */
-	if (f_base != NULL) {
-		ret = read_core_sysfs_u32(f_base, &base_ratio);
-		if (ret < 0) {
-			POWER_LOG(ERR, "Failed to read %s",
-					POWER_SYSFILE_BASE_FREQ);
-			goto err;
-		}
-	} else {
+	/* base ratio file may not exist in some kernels, so no error check */
+	if (power_sysfs_read_u32(&base_ratio, POWER_SYSFILE_BASE_FREQ,
+			pi->lcore_id) < 0)
 		base_ratio = 0;
-	}
 
 	/* convert ratios to bins */
 	base_max_ratio /= BUS_FREQ;
@@ -222,20 +196,10 @@ power_init_for_setting_freq(struct pstate_power_info *pi)
 	pi->core_base_freq = base_ratio * BUS_FREQ;
 
 out:
-	if (f_base != NULL)
-		fclose(f_base);
-	fclose(f_base_max);
-	fclose(f_base_min);
 	/* f_min and f_max are stored, no need to close */
 	return 0;
 
 err:
-	if (f_base != NULL)
-		fclose(f_base);
-	if (f_base_min != NULL)
-		fclose(f_base_min);
-	if (f_base_max != NULL)
-		fclose(f_base_max);
 	if (f_min != NULL)
 		fclose(f_min);
 	if (f_max != NULL)
@@ -366,38 +330,22 @@ power_set_governor_original(struct pstate_power_info *pi)
 static int
 power_get_available_freqs(struct pstate_power_info *pi)
 {
-	FILE *f_min = NULL, *f_max = NULL;
-	int ret = -1;
 	uint32_t sys_min_freq = 0, sys_max_freq = 0, base_max_freq = 0;
 	int config_min_freq, config_max_freq;
 	uint32_t i, num_freqs = 0;
-
-	/* open all files */
-	open_core_sysfs_file(&f_max, "r", POWER_SYSFILE_BASE_MAX_FREQ,
-			pi->lcore_id);
-	if (f_max == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_BASE_MAX_FREQ);
-		goto out;
-	}
-
-	open_core_sysfs_file(&f_min, "r", POWER_SYSFILE_BASE_MIN_FREQ,
-			pi->lcore_id);
-	if (f_min == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_BASE_MIN_FREQ);
-		goto out;
-	}
+	int ret = -1;
 
 	/* read base ratios */
-	ret = read_core_sysfs_u32(f_max, &sys_max_freq);
+	ret = power_sysfs_read_u32(&sys_max_freq, POWER_SYSFILE_BASE_MAX_FREQ,
+			pi->lcore_id);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_BASE_MAX_FREQ);
 		goto out;
 	}
 
-	ret = read_core_sysfs_u32(f_min, &sys_min_freq);
+	ret = power_sysfs_read_u32(&sys_min_freq, POWER_SYSFILE_BASE_MIN_FREQ,
+			pi->lcore_id);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_BASE_MIN_FREQ);
@@ -467,31 +415,18 @@ power_get_available_freqs(struct pstate_power_info *pi)
 			num_freqs, pi->lcore_id);
 
 out:
-	if (f_min != NULL)
-		fclose(f_min);
-	if (f_max != NULL)
-		fclose(f_max);
-
 	return ret;
 }
 
 static int
 power_get_cur_idx(struct pstate_power_info *pi)
 {
-	FILE *f_cur;
-	int ret = -1;
 	uint32_t sys_cur_freq = 0;
 	unsigned int i;
+	int ret = -1;
 
-	open_core_sysfs_file(&f_cur, "r", POWER_SYSFILE_CUR_FREQ,
+	ret = power_sysfs_read_u32(&sys_cur_freq, POWER_SYSFILE_CUR_FREQ,
 			pi->lcore_id);
-	if (f_cur == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_CUR_FREQ);
-		goto fail;
-	}
-
-	ret = read_core_sysfs_u32(f_cur, &sys_cur_freq);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_CUR_FREQ);
@@ -517,8 +452,6 @@ power_get_cur_idx(struct pstate_power_info *pi)
 
 	ret = 0;
 fail:
-	if (f_cur != NULL)
-		fclose(f_cur);
 	return ret;
 }
 
diff --git a/drivers/power/intel_uncore/intel_uncore.c b/drivers/power/intel_uncore/intel_uncore.c
index 6759ea1445..0e758312cf 100644
--- a/drivers/power/intel_uncore/intel_uncore.c
+++ b/drivers/power/intel_uncore/intel_uncore.c
@@ -111,20 +111,14 @@ set_uncore_freq_internal(struct uncore_power_info *ui, uint32_t idx)
 static int
 power_init_for_setting_uncore_freq(struct uncore_power_info *ui)
 {
-	FILE *f_base_min = NULL, *f_base_max = NULL, *f_min = NULL, *f_max = NULL;
+	FILE *f_min = NULL, *f_max = NULL;
 	uint32_t base_min_freq = 0, base_max_freq = 0, min_freq = 0, max_freq = 0;
 	int ret;
 
 	/* open and read all uncore sys files */
 	/* Base max */
-	open_core_sysfs_file(&f_base_max, "r", POWER_INTEL_UNCORE_SYSFILE_BASE_MAX_FREQ,
+	ret = power_sysfs_read_u32(&base_max_freq, POWER_INTEL_UNCORE_SYSFILE_BASE_MAX_FREQ,
 			ui->pkg, ui->die);
-	if (f_base_max == NULL) {
-		POWER_LOG(DEBUG, "failed to open %s",
-				POWER_INTEL_UNCORE_SYSFILE_BASE_MAX_FREQ);
-		goto err;
-	}
-	ret = read_core_sysfs_u32(f_base_max, &base_max_freq);
 	if (ret < 0) {
 		POWER_LOG(DEBUG, "Failed to read %s",
 				POWER_INTEL_UNCORE_SYSFILE_BASE_MAX_FREQ);
@@ -132,21 +126,13 @@ power_init_for_setting_uncore_freq(struct uncore_power_info *ui)
 	}
 
 	/* Base min */
-	open_core_sysfs_file(&f_base_min, "r", POWER_INTEL_UNCORE_SYSFILE_BASE_MIN_FREQ,
-		ui->pkg, ui->die);
-	if (f_base_min == NULL) {
-		POWER_LOG(DEBUG, "failed to open %s",
+	ret = power_sysfs_read_u32(&base_min_freq, POWER_INTEL_UNCORE_SYSFILE_BASE_MIN_FREQ,
+			ui->pkg, ui->die);
+	if (ret < 0) {
+		POWER_LOG(DEBUG, "Failed to read %s",
 				POWER_INTEL_UNCORE_SYSFILE_BASE_MIN_FREQ);
 		goto err;
 	}
-	if (f_base_min != NULL) {
-		ret = read_core_sysfs_u32(f_base_min, &base_min_freq);
-		if (ret < 0) {
-			POWER_LOG(DEBUG, "Failed to read %s",
-					POWER_INTEL_UNCORE_SYSFILE_BASE_MIN_FREQ);
-			goto err;
-		}
-	}
 
 	/* Curr min */
 	open_core_sysfs_file(&f_min, "rw+", POWER_INTEL_UNCORE_SYSFILE_MIN_FREQ,
@@ -191,17 +177,11 @@ power_init_for_setting_uncore_freq(struct uncore_power_info *ui)
 	ui->init_max_freq = base_max_freq;
 	ui->init_min_freq = base_min_freq;
 
-	fclose(f_base_min);
-	fclose(f_base_max);
 	/* f_min and f_max are stored, no need to close */
 
 	return 0;
 
 err:
-	if (f_base_min != NULL)
-		fclose(f_base_min);
-	if (f_base_max != NULL)
-		fclose(f_base_max);
 	if (f_min != NULL)
 		fclose(f_min);
 	if (f_max != NULL)
diff --git a/lib/power/power_common.c b/lib/power/power_common.c
index da22a4d160..f05bc48048 100644
--- a/lib/power/power_common.c
+++ b/lib/power/power_common.c
@@ -6,11 +6,13 @@
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
+#include <stdarg.h>
 
 #include <eal_export.h>
 #include <rte_log.h>
 #include <rte_string_fns.h>
 #include <rte_lcore.h>
+#include <rte_sysfs.h>
 
 #include "power_common.h"
 
@@ -28,34 +30,16 @@ cpufreq_check_scaling_driver(const char *driver_name)
 {
 	unsigned int lcore_id = 0; /* always check core 0 */
 	char readbuf[PATH_MAX];
-	size_t end_idx;
-	char *s;
-	FILE *f;
 
 	/*
 	 * Check if scaling driver matches what we expect.
+	 * If there is no driver at all, or it can't be read,
+	 * consider the system unsupported.
 	 */
-	open_core_sysfs_file(&f, "r", POWER_SYSFILE_SCALING_DRIVER,
-			lcore_id);
-	/* if there's no driver at all, bail out */
-	if (f == NULL)
-		return 0;
-
-	s = fgets(readbuf, sizeof(readbuf), f);
-	/* don't need it any more */
-	fclose(f);
-
-	/* if we can't read it, consider unsupported */
-	if (s == NULL)
+	if (rte_sysfs_parse_string(readbuf, sizeof(readbuf),
+			POWER_SYSFILE_SCALING_DRIVER, lcore_id) < 0)
 		return 0;
 
-	/* when read from sysfs, driver name has an extra newline at the end */
-	end_idx = strnlen(readbuf, sizeof(readbuf));
-	if (end_idx > 0 && readbuf[end_idx - 1] == '\n') {
-		end_idx--;
-		readbuf[end_idx] = '\0';
-	}
-
 	/* does the driver name match? */
 	if (strncmp(readbuf, driver_name, sizeof(readbuf)) != 0)
 		return 0;
@@ -87,6 +71,24 @@ open_core_sysfs_file(FILE **f, const char *mode, const char *format, ...)
 	return 0;
 }
 
+RTE_EXPORT_INTERNAL_SYMBOL(power_sysfs_read_u32)
+int
+power_sysfs_read_u32(uint32_t *val, const char *format, ...)
+{
+	unsigned long tmp;
+	va_list ap;
+	int ret;
+
+	va_start(ap, format);
+	ret = rte_sysfs_vparse_uint(&tmp, format, ap);
+	va_end(ap);
+	if (ret < 0)
+		return -1;
+
+	*val = tmp;
+	return 0;
+}
+
 RTE_EXPORT_INTERNAL_SYMBOL(read_core_sysfs_u32)
 int
 read_core_sysfs_u32(FILE *f, uint32_t *val)
diff --git a/lib/power/power_common.h b/lib/power/power_common.h
index 370c5246c6..6ca91658cd 100644
--- a/lib/power/power_common.h
+++ b/lib/power/power_common.h
@@ -8,6 +8,7 @@
 #include <rte_common.h>
 #include <rte_compat.h>
 #include <rte_log.h>
+#include <stdint.h>
 
 #define RTE_POWER_INVALID_FREQ_INDEX (~0)
 
@@ -48,6 +49,11 @@ int open_core_sysfs_file(FILE **f, const char *mode, const char *format, ...)
 __rte_internal
 int read_core_sysfs_u32(FILE *f, uint32_t *val);
 
+/* read a 32 bit value from a sysfs file given by a printf style path */
+__rte_internal
+int power_sysfs_read_u32(uint32_t *val, const char *format, ...)
+		__rte_format_printf(2, 3);
+
 __rte_internal
 int read_core_sysfs_s(FILE *f, char *buf, unsigned int len);
 
diff --git a/lib/power/rte_power_qos.c b/lib/power/rte_power_qos.c
index d8d8d36a76..dd1b8f7384 100644
--- a/lib/power/rte_power_qos.c
+++ b/lib/power/rte_power_qos.c
@@ -9,6 +9,7 @@
 #include <eal_export.h>
 #include <rte_lcore.h>
 #include <rte_log.h>
+#include <rte_sysfs.h>
 
 #include "power_common.h"
 #include "rte_power_qos.h"
@@ -24,7 +25,6 @@ rte_power_qos_set_cpu_resume_latency(uint16_t lcore_id, int latency)
 {
 	char buf[PM_QOS_CPU_RESUME_LATENCY_BUF_LEN];
 	uint32_t cpu_id;
-	FILE *f;
 	int ret;
 
 	RTE_POWER_VALID_LCOREID_OR_ERR_RET(lcore_id, -EINVAL);
@@ -37,13 +37,6 @@ rte_power_qos_set_cpu_resume_latency(uint16_t lcore_id, int latency)
 		return -EINVAL;
 	}
 
-	ret = open_core_sysfs_file(&f, "w", PM_QOS_SYSFILE_RESUME_LATENCY_US, cpu_id);
-	if (ret != 0) {
-		POWER_LOG(ERR, "Failed to open "PM_QOS_SYSFILE_RESUME_LATENCY_US" : %s",
-			  cpu_id, strerror(errno));
-		return ret;
-	}
-
 	/*
 	 * Based on the sysfs interface pm_qos_resume_latency_us under
 	 * @PM_QOS_SYSFILE_RESUME_LATENCY_US directory in kernel, their meaning
@@ -59,13 +52,11 @@ rte_power_qos_set_cpu_resume_latency(uint16_t lcore_id, int latency)
 	else
 		snprintf(buf, sizeof(buf), "%u", latency);
 
-	ret = write_core_sysfs_s(f, buf);
+	ret = rte_sysfs_write_string(buf, PM_QOS_SYSFILE_RESUME_LATENCY_US, cpu_id);
 	if (ret != 0)
 		POWER_LOG(ERR, "Failed to write "PM_QOS_SYSFILE_RESUME_LATENCY_US" : %s",
 			  cpu_id, strerror(errno));
 
-	fclose(f);
-
 	return ret;
 }
 
@@ -76,7 +67,6 @@ rte_power_qos_get_cpu_resume_latency(uint16_t lcore_id)
 	char buf[PM_QOS_CPU_RESUME_LATENCY_BUF_LEN];
 	int latency = -1;
 	uint32_t cpu_id;
-	FILE *f;
 	int ret;
 
 	RTE_POWER_VALID_LCOREID_OR_ERR_RET(lcore_id, -EINVAL);
@@ -84,18 +74,12 @@ rte_power_qos_get_cpu_resume_latency(uint16_t lcore_id)
 	if (ret != 0)
 		return ret;
 
-	ret = open_core_sysfs_file(&f, "r", PM_QOS_SYSFILE_RESUME_LATENCY_US, cpu_id);
-	if (ret != 0) {
-		POWER_LOG(ERR, "Failed to open "PM_QOS_SYSFILE_RESUME_LATENCY_US" : %s",
-			  cpu_id, strerror(errno));
-		return ret;
-	}
-
-	ret = read_core_sysfs_s(f, buf, sizeof(buf));
+	ret = rte_sysfs_parse_string(buf, sizeof(buf), PM_QOS_SYSFILE_RESUME_LATENCY_US,
+			cpu_id);
 	if (ret != 0) {
 		POWER_LOG(ERR, "Failed to read "PM_QOS_SYSFILE_RESUME_LATENCY_US" : %s",
 			  cpu_id, strerror(errno));
-		goto out;
+		return ret;
 	}
 
 	/*
@@ -113,8 +97,5 @@ rte_power_qos_get_cpu_resume_latency(uint16_t lcore_id)
 		latency = latency == 0 ? RTE_POWER_QOS_RESUME_LATENCY_NO_CONSTRAINT : latency;
 	}
 
-out:
-	fclose(f);
-
 	return latency != -1 ? latency : ret;
 }
-- 
2.53.0

[PATCH 6/9] drivers/bus: remove duplicate sysfs string helpers

[Stephen Hemminger]

get_sysfs_string() in the vmbus bus and read_sysfs_string() in the
platform bus are both open coded copies of what the EAL routine now
provides. Each had a single caller.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/bus/platform/platform.c     | 33 +++---------------------
 drivers/bus/vmbus/linux/vmbus_bus.c | 39 +++--------------------------
 2 files changed, 8 insertions(+), 64 deletions(-)

diff --git a/drivers/bus/platform/platform.c b/drivers/bus/platform/platform.c
index 9585fb79e9..c511893aab 100644
--- a/drivers/bus/platform/platform.c
+++ b/drivers/bus/platform/platform.c
@@ -187,40 +187,15 @@ device_unmap_resources(struct rte_platform_device *pdev)
 	pdev->num_resource = 0;
 }
 
-static int
-read_sysfs_string(const char *path, char *buf, size_t size)
-{
-	FILE *f;
-	char *p;
-
-	f = fopen(path, "r");
-	if (f == NULL)
-		return -errno;
-
-	if (fgets(buf, size, f) == NULL) {
-		fclose(f);
-		return -ENODATA;
-	}
-
-	fclose(f);
-
-	p = strrchr(buf, '\n');
-	if (p != NULL)
-		*p = '\0';
-
-	return 0;
-}
-
 static char *
 of_resource_name(const char *dev_name, int index)
 {
-	char path[PATH_MAX], buf[BUFSIZ] = { };
-	int num = 0, ret;
+	char buf[BUFSIZ] = { };
+	int num = 0;
 	char *name;
 
-	snprintf(path, sizeof(path), PLATFORM_BUS_DEVICES_PATH "/%s/of_node/reg-names", dev_name);
-	ret = read_sysfs_string(path, buf, sizeof(buf) - 1);
-	if (ret)
+	if (rte_sysfs_parse_string(buf, sizeof(buf) - 1,
+			PLATFORM_BUS_DEVICES_PATH "/%s/of_node/reg-names", dev_name) < 0)
 		return NULL;
 
 	for (name = buf; *name != 0; name += strlen(name) + 1) {
diff --git a/drivers/bus/vmbus/linux/vmbus_bus.c b/drivers/bus/vmbus/linux/vmbus_bus.c
index 9ee7983eb6..00f369120f 100644
--- a/drivers/bus/vmbus/linux/vmbus_bus.c
+++ b/drivers/bus/vmbus/linux/vmbus_bus.c
@@ -84,35 +84,6 @@ parse_sysfs_uuid(const char *filename, rte_uuid_t uu)
 	return 0;
 }
 
-static int
-get_sysfs_string(const char *filename, char *buf, size_t buflen)
-{
-	char *cp;
-	FILE *f;
-
-	f = fopen(filename, "r");
-	if (f == NULL) {
-		VMBUS_LOG(ERR, "cannot open sysfs value %s:%s",
-			  filename, strerror(errno));
-		return -1;
-	}
-
-	if (fgets(buf, buflen, f) == NULL) {
-		VMBUS_LOG(ERR, "cannot read sysfs value %s",
-				filename);
-		fclose(f);
-		return -1;
-	}
-	fclose(f);
-
-	/* remove trailing newline */
-	cp = memchr(buf, '\n', buflen);
-	if (cp)
-		*cp = '\0';
-
-	return 0;
-}
-
 static int
 vmbus_get_uio_dev(const struct rte_vmbus_device *dev,
 		  char *dstbuf, size_t buflen)
@@ -169,7 +140,7 @@ RTE_EXPORT_SYMBOL(rte_vmbus_map_device)
 int
 rte_vmbus_map_device(struct rte_vmbus_device *dev)
 {
-	char uioname[PATH_MAX], filename[PATH_MAX];
+	char uioname[PATH_MAX];
 	char dirname[PATH_MAX], mapname[64];
 	int i;
 
@@ -188,11 +159,9 @@ rte_vmbus_map_device(struct rte_vmbus_device *dev)
 		snprintf(dirname, sizeof(dirname),
 			 "%s/maps/map%d", uioname, i);
 
-		snprintf(filename, sizeof(filename),
-			 "%s/name", dirname);
-
-		if (get_sysfs_string(filename, mapname, sizeof(mapname)) < 0) {
-			VMBUS_LOG(ERR, "could not read %s", filename);
+		if (rte_sysfs_parse_string(mapname, sizeof(mapname),
+				"%s/name", dirname) < 0) {
+			VMBUS_LOG(ERR, "could not read name of %s", dirname);
 			return -1;
 		}
 
-- 
2.53.0

[PATCH 7/9] common/mlx5: use common sysfs routines

[Stephen Hemminger]

mlx5_sys_roce_disable() read the roce_enable attribute and then
reopened the same file to write it. Use the EAL read and write
routines instead.

The dev_port lookup is left alone: it selects the scanf format at
runtime and distinguishes ENOENT from other errors to fall back to
dev_id on older kernels.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/common/mlx5/linux/mlx5_common_os.c | 39 ++++++++--------------
 1 file changed, 14 insertions(+), 25 deletions(-)

diff --git a/drivers/common/mlx5/linux/mlx5_common_os.c b/drivers/common/mlx5/linux/mlx5_common_os.c
index 3e9cd86062..cf496b0b58 100644
--- a/drivers/common/mlx5/linux/mlx5_common_os.c
+++ b/drivers/common/mlx5/linux/mlx5_common_os.c
@@ -16,6 +16,7 @@
 #include <eal_export.h>
 #include <rte_errno.h>
 #include <rte_string_fns.h>
+#include <rte_sysfs.h>
 #include <bus_pci_driver.h>
 #include <bus_auxiliary_driver.h>
 
@@ -658,45 +659,33 @@ mlx5_nl_roce_disable(const char *addr)
 	return ret;
 }
 
+#define MLX5_ROCE_ENABLE_PATH "/sys/bus/pci/devices/%s/roce_enable"
+
 /* Try to disable ROCE by sysfs. */
 static int
 mlx5_sys_roce_disable(const char *addr)
 {
-	FILE *file_o;
-	int enable;
+	unsigned long enable;
 	int ret;
 
-	MKSTR(file_p, "/sys/bus/pci/devices/%s/roce_enable", addr);
-	file_o = fopen(file_p, "rb");
-	if (!file_o) {
+	if (rte_sysfs_parse_uint(&enable, MLX5_ROCE_ENABLE_PATH, addr) != 0) {
 		rte_errno = ENOTSUP;
 		return -ENOTSUP;
 	}
-	ret = fscanf(file_o, "%d", &enable);
-	if (ret != 1) {
-		rte_errno = EINVAL;
-		ret = EINVAL;
-		goto close;
-	} else if (!enable) {
-		ret = 0;
+	if (enable == 0) {
 		DRV_LOG(INFO, "ROCE has already disabled(sysfs).");
-		goto close;
+		return 0;
 	}
-	fclose(file_o);
-	file_o = fopen(file_p, "wb");
-	if (!file_o) {
+
+	ret = rte_sysfs_write_string("0\n", MLX5_ROCE_ENABLE_PATH, addr);
+	if (ret != 0) {
 		rte_errno = ENOTSUP;
+		DRV_LOG(DEBUG, "Failed to disable ROCE by sysfs.");
 		return -ENOTSUP;
 	}
-	fprintf(file_o, "0\n");
-	ret = 0;
-close:
-	if (ret)
-		DRV_LOG(DEBUG, "Failed to disable ROCE by sysfs: %d.", ret);
-	else
-		DRV_LOG(INFO, "ROCE is disabled by sysfs successfully.");
-	fclose(file_o);
-	return ret;
+
+	DRV_LOG(INFO, "ROCE is disabled by sysfs successfully.");
+	return 0;
 }
 
 static int
-- 
2.53.0

[PATCH 8/9] net/mlx5: use common sysfs routines

[Stephen Hemminger]

Read the bonding interface indexes with the EAL routine rather than
opening each file and parsing it with fscanf().

The index was read with fscanf("%u") and is now converted with base
0, so a value with a leading zero would parse as octal. The kernel
does not print ifindex with leading zeros.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/mlx5/linux/mlx5_ethdev_os.c | 17 +++++------------
 drivers/net/mlx5/linux/mlx5_os.c        | 13 +++++--------
 2 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/net/mlx5/linux/mlx5_ethdev_os.c b/drivers/net/mlx5/linux/mlx5_ethdev_os.c
index 4bbc590e91..d1c03d66af 100644
--- a/drivers/net/mlx5/linux/mlx5_ethdev_os.c
+++ b/drivers/net/mlx5/linux/mlx5_ethdev_os.c
@@ -36,6 +36,7 @@
 #include <rte_string_fns.h>
 #include <rte_rwlock.h>
 #include <rte_cycles.h>
+#include <rte_sysfs.h>
 
 #include <mlx5_glue.h>
 #include <mlx5_devx_cmds.h>
@@ -1192,27 +1193,19 @@ mlx5_sysfs_bond_info(unsigned int pf_ifindex, unsigned int *ifindex,
 		     char *ifname)
 {
 	char name[IF_NAMESIZE];
-	FILE *file;
 	unsigned int index;
-	int ret;
+	unsigned long val;
 
 	if (!if_indextoname(pf_ifindex, name) || !strlen(name)) {
 		rte_errno = errno;
 		return -rte_errno;
 	}
-	MKSTR(bond_if, "/sys/class/net/%s/master/ifindex", name);
 	/* read bond ifindex */
-	file = fopen(bond_if, "rb");
-	if (file == NULL) {
-		rte_errno = errno;
-		return -rte_errno;
-	}
-	ret = fscanf(file, "%u", &index);
-	fclose(file);
-	if (ret <= 0) {
-		rte_errno = errno;
+	if (rte_sysfs_parse_uint(&val, "/sys/class/net/%s/master/ifindex", name) != 0) {
+		rte_errno = ENODEV;
 		return -rte_errno;
 	}
+	index = val;
 	if (ifindex)
 		*ifindex = index;
 
diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index adc5878296..203e9339bb 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -29,6 +29,7 @@
 #include <rte_string_fns.h>
 #include <rte_alarm.h>
 #include <rte_eal_paging.h>
+#include <rte_sysfs.h>
 
 #include <mlx5_glue.h>
 #include <mlx5_devx_cmds.h>
@@ -2103,6 +2104,7 @@ mlx5_device_bond_pci_match(const struct ibv_device *ibdev,
 		char tmp_str[IF_NAMESIZE + 32];
 		struct rte_pci_addr pci_addr;
 		struct mlx5_switch_info	info;
+		unsigned long val;
 		int ret;
 
 		/* Process slave interface names in the loop. */
@@ -2140,15 +2142,10 @@ mlx5_device_bond_pci_match(const struct ibv_device *ibdev,
 			break;
 		}
 		/* Get ifindex. */
-		snprintf(tmp_str, sizeof(tmp_str),
-			 "/sys/class/net/%s/ifindex", ifname);
-		file = fopen(tmp_str, "rb");
-		if (!file)
-			break;
-		ret = fscanf(file, "%u", &ifindex);
-		fclose(file);
-		if (ret != 1)
+		if (rte_sysfs_parse_uint(&val, "/sys/class/net/%s/ifindex",
+				ifname) != 0)
 			break;
+		ifindex = val;
 		/* Save bonding info. */
 		snprintf(bond_info->ports[info.port_name].ifname,
 			 sizeof(bond_info->ports[0].ifname), "%s", ifname);
-- 
2.53.0

[PATCH 9/9] net/mana: use common sysfs routines

[Stephen Hemminger]

Read dev_port with the EAL routine rather than opening the file and
parsing it with fscanf().

The value was read with fscanf("%d") and is now converted with base
0, so a value with a leading zero would parse as octal. The kernel
does not print dev_port with leading zeros.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/mana/mana.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/net/mana/mana.c b/drivers/net/mana/mana.c
index 0b72f711a1..39bfb5b4b7 100644
--- a/drivers/net/mana/mana.c
+++ b/drivers/net/mana/mana.c
@@ -14,6 +14,7 @@
 #include <rte_kvargs.h>
 #include <rte_eal_paging.h>
 #include <rte_pci.h>
+#include <rte_sysfs.h>
 
 #include <infiniband/verbs.h>
 #include <infiniband/manadv.h>
@@ -1282,6 +1283,7 @@ get_port_mac(struct ibv_device *device, unsigned int port,
 	DIR *dir;
 	struct dirent *dent;
 	unsigned int dev_port;
+	unsigned long val;
 
 	MANA_MKSTR(path, "%s/device/net", device->ibdev_path);
 
@@ -1293,23 +1295,15 @@ get_port_mac(struct ibv_device *device, unsigned int port,
 		char *name = dent->d_name;
 		char *mac = NULL;
 
-		MANA_MKSTR(port_path, "%s/%s/dev_port", path, name);
-
 		/* Ignore . and .. */
 		if ((name[0] == '.') &&
 		    ((name[1] == '\0') ||
 		     ((name[1] == '.') && (name[2] == '\0'))))
 			continue;
 
-		file = fopen(port_path, "r");
-		if (!file)
-			continue;
-
-		ret = fscanf(file, "%u", &dev_port);
-		fclose(file);
-
-		if (ret != 1)
+		if (rte_sysfs_parse_uint(&val, "%s/%s/dev_port", path, name) != 0)
 			continue;
+		dev_port = val;
 
 		/* Ethernet ports start at 0, IB port start at 1 */
 		if (dev_port == port - 1) {
-- 
2.53.0

[PATCH v2 0/9] consolidate sysfs access

[Stephen Hemminger]

After reviewing lots of drivers and seeing sloppy string handling
resorted to AI assistance to unify and consolidate the parsing
of sysfs values. Many drivers open code this with similar pattern
but lacked any coherent error handling. The kernel API is
consistent and won't give bad data, but it make sense to
check for garbage.

Add one set of routines in EAL that build the path from a printf
style format and convert with strtoul()/strtol(), then convert the
existing callers.

Since the new routines are Linux only, a driver that uses sysfs while
advertising support on FreeBSD or Windows now fails to link. Two such
cases turned up and are fixed here: the eal_fs test and common/ionic.

Note: existing eal_parse_sysfs_value() goes away with this.
It was exported as a stable symbol, which was a mistake:
the eal_ prefix and the private eal_filesystem.h both say it is internal,
and nothing outside the tree should have been calling it.
The new rte_sysfs_parse_XXX routines are exported, but marked
as internal use only.

v2 - checkpatch fixes
   - clang format attribute fix
   - sysfs is Linux only, not unix

Stephen Hemminger (9):
  eal: add common sysfs value routines
  dma/idxd: use common sysfs routines
  common/ionic: use common sysfs routines
  bus/vmbus: use common sysfs routines
  power: use common sysfs routines
  drivers/bus: remove duplicate sysfs string helpers
  common/mlx5: use common sysfs routines
  net/mlx5: use common sysfs routines
  net/mana: use common sysfs routines

 app/test/test_eal_fs.c                        | 148 ++++++++++----
 drivers/bus/auxiliary/linux/auxiliary.c       |  13 +-
 drivers/bus/cdx/cdx.c                         |   7 +-
 drivers/bus/pci/linux/pci.c                   |  44 ++--
 drivers/bus/platform/platform.c               |  43 +---
 drivers/bus/vmbus/linux/vmbus_bus.c           |  64 ++----
 drivers/bus/vmbus/linux/vmbus_uio.c           |  24 +--
 drivers/common/cnxk/roc_model.c               |  40 +---
 drivers/common/cnxk/roc_platform.h            |   3 +-
 drivers/common/ionic/ionic_common_uio.c       | 102 +++++-----
 .../common/mlx5/linux/mlx5_common_auxiliary.c |  13 +-
 drivers/common/mlx5/linux/mlx5_common_os.c    |  39 ++--
 drivers/dma/idxd/idxd_bus.c                   |  80 ++------
 drivers/net/af_xdp/rte_eth_af_xdp.c           |  13 +-
 drivers/net/mana/mana.c                       |  14 +-
 drivers/net/mlx5/linux/mlx5_ethdev_os.c       |  17 +-
 drivers/net/mlx5/linux/mlx5_os.c              |  13 +-
 drivers/power/acpi/acpi_cpufreq.c             |  21 +-
 drivers/power/amd_pstate/amd_pstate_cpufreq.c |  63 +-----
 drivers/power/cppc/cppc_cpufreq.c             |  69 +------
 .../power/intel_pstate/intel_pstate_cpufreq.c |  99 ++-------
 drivers/power/intel_uncore/intel_uncore.c     |  32 +--
 lib/eal/common/eal_filesystem.h               |   4 +-
 lib/eal/include/meson.build                   |   1 +
 lib/eal/include/rte_sysfs.h                   | 121 +++++++++++
 lib/eal/linux/eal_hugepage_info.c             |  33 +--
 lib/eal/linux/eal_lcore.c                     |  18 +-
 lib/eal/linux/eal_sysfs.c                     | 188 ++++++++++++++++++
 lib/eal/linux/meson.build                     |   1 +
 lib/eal/unix/eal_filesystem.c                 |  31 ---
 lib/power/power_common.c                      |  52 +++--
 lib/power/power_common.h                      |   6 +
 lib/power/rte_power_qos.c                     |  29 +--
 33 files changed, 690 insertions(+), 755 deletions(-)
 create mode 100644 lib/eal/include/rte_sysfs.h
 create mode 100644 lib/eal/linux/eal_sysfs.c

-- 
2.53.0

[PATCH v2 1/9] eal: add common sysfs value routines

[Stephen Hemminger]

Many drivers in DPDK read single values, and each one
open codes the same sequence: build a path with snprintf, open the
file, read a line, and convert it. Several drivers grew their own
private copy of that helper, and most of them parse with fscanf()
which cannot tell a failed conversion from a legitimate zero and
silently accepts trailing garbage.

Add a single set of routines in EAL that build the path from a
printf style format and do the conversion with strtoul()/strtol():

  rte_sysfs_parse_uint()    unsigned value
  rte_sysfs_parse_int()     signed value
  rte_sysfs_parse_string()  string value, newline stripped
  rte_sysfs_write_string()  write a string

rte_sysfs_vparse_uint() takes a va_list, so that a wrapper such
as the one in the power library can forward its arguments without
formatting the path a second time.

Folding the path construction into the routine removes the separate
snprintf() and its truncation check from every caller.

The signed variant exists because some attributes are genuinely
signed: numa_node is -1 when the device is not tied to a node.

These replace eal_parse_sysfs_value(), which was declared in the
internal eal_filesystem.h yet exported as a stable symbol, and was
reached by drivers through that private header.

sysfs is a Linux filesystem, so the implementation belongs in
lib/eal/linux rather than lib/eal/unix. That also settles the use
of strchrnul(), which is a GNU extension that FreeBSD only grew
recently. The declarations stay in lib/eal/include since they are
part of the driver SDK headers, and the test is skipped on the
platforms that no longer provide the routines.

Convert all of the existing callers.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_eal_fs.c                        | 148 ++++++++++----
 drivers/bus/auxiliary/linux/auxiliary.c       |  13 +-
 drivers/bus/cdx/cdx.c                         |   7 +-
 drivers/bus/pci/linux/pci.c                   |  44 ++--
 drivers/bus/platform/platform.c               |  10 +-
 drivers/bus/vmbus/linux/vmbus_bus.c           |  25 +--
 drivers/common/cnxk/roc_model.c               |  40 +---
 drivers/common/cnxk/roc_platform.h            |   3 +-
 .../common/mlx5/linux/mlx5_common_auxiliary.c |  13 +-
 drivers/net/af_xdp/rte_eth_af_xdp.c           |  13 +-
 lib/eal/common/eal_filesystem.h               |   4 +-
 lib/eal/include/meson.build                   |   1 +
 lib/eal/include/rte_sysfs.h                   | 121 +++++++++++
 lib/eal/linux/eal_hugepage_info.c             |  33 +--
 lib/eal/linux/eal_lcore.c                     |  18 +-
 lib/eal/linux/eal_sysfs.c                     | 188 ++++++++++++++++++
 lib/eal/linux/meson.build                     |   1 +
 lib/eal/unix/eal_filesystem.c                 |  31 ---
 18 files changed, 499 insertions(+), 214 deletions(-)
 create mode 100644 lib/eal/include/rte_sysfs.h
 create mode 100644 lib/eal/linux/eal_sysfs.c

diff --git a/app/test/test_eal_fs.c b/app/test/test_eal_fs.c
index 62eea98677..d2e94ff0db 100644
--- a/app/test/test_eal_fs.c
+++ b/app/test/test_eal_fs.c
@@ -7,19 +7,21 @@
 #include <stdlib.h>
 #include <string.h>
 #include <errno.h>
+#include <limits.h>
+#include <unistd.h>
 
-#include "eal_filesystem.h"
-
-#ifdef RTE_EXEC_ENV_WINDOWS
+#ifndef RTE_EXEC_ENV_LINUX
 static int
 test_eal_fs(void)
 {
-	printf("eal_fs not supported on Windows, skipping test\n");
+	printf("sysfs is only available on Linux, skipping test\n");
 	return TEST_SKIPPED;
 }
 
 #else
 
+#include <rte_sysfs.h>
+
 static int
 test_parse_sysfs_value(void)
 {
@@ -30,13 +32,10 @@ test_parse_sysfs_value(void)
 	FILE *fd = NULL;
 	unsigned valid_number;
 	unsigned long retval = 0;
+	char strval[64];
+	long sretval = 0;
 
-#ifdef RTE_EXEC_ENV_FREEBSD
-	/* BSD doesn't have /proc/pid/fd */
-	return 0;
-#endif
-
-	printf("Testing function eal_parse_sysfs_value()\n");
+	printf("Testing sysfs value functions\n");
 
 	/* get a temporary filename to use for all tests - create temp file handle and then
 	 * use /proc to get the actual file that we can open */
@@ -54,8 +53,8 @@ test_parse_sysfs_value(void)
 
 	/* test we get an error value if we use file before it's created */
 	printf("Test reading a missing file ...\n");
-	if (eal_parse_sysfs_value("/dev/not-quite-null", &retval) == 0) {
-		printf("Error with eal_parse_sysfs_value() - returned success on reading empty file\n");
+	if (rte_sysfs_parse_uint(&retval, "/dev/not-quite-null") == 0) {
+		printf("rte_sysfs_parse_uint() returned success on a missing file - test failed\n");
 		goto error;
 	}
 	printf("Confirmed return error when reading empty file\n");
@@ -71,12 +70,12 @@ test_parse_sysfs_value(void)
 	fprintf(fd,"%u\n", valid_number);
 	fclose(fd);
 	fd = NULL;
-	if (eal_parse_sysfs_value(filename, &retval) < 0) {
-		printf("eal_parse_sysfs_value() returned error - test failed\n");
+	if (rte_sysfs_parse_uint(&retval, "%s", filename) < 0) {
+		printf("rte_sysfs_parse_uint() returned error - test failed\n");
 		goto error;
 	}
 	if (retval != valid_number) {
-		printf("Invalid value read by eal_parse_sysfs_value() - test failed\n");
+		printf("Invalid value read by rte_sysfs_parse_uint() - test failed\n");
 		goto error;
 	}
 	printf("Read '%u\\n' ok\n", valid_number);
@@ -91,43 +90,93 @@ test_parse_sysfs_value(void)
 	fprintf(fd,"0x%x\n", valid_number);
 	fclose(fd);
 	fd = NULL;
-	if (eal_parse_sysfs_value(filename, &retval) < 0) {
-		printf("eal_parse_sysfs_value() returned error - test failed\n");
+	if (rte_sysfs_parse_uint(&retval, "%s", filename) < 0) {
+		printf("rte_sysfs_parse_uint() returned error - test failed\n");
 		goto error;
 	}
 	if (retval != valid_number) {
-		printf("Invalid value read by eal_parse_sysfs_value() - test failed\n");
+		printf("Invalid value read by rte_sysfs_parse_uint() - test failed\n");
 		goto error;
 	}
 	printf("Read '0x%x\\n' ok\n", valid_number);
 
-	printf("Test reading invalid values ...\n");
+	/* a value without a trailing newline is accepted */
+	valid_number = 3;
+	fd = fopen(filename, "w");
+	if (fd == NULL) {
+		printf("line %d, Error opening %s: %s\n", __LINE__, filename, strerror(errno));
+		goto error;
+	}
+	fprintf(fd, "%u", valid_number);
+	fclose(fd);
+	fd = NULL;
+	if (rte_sysfs_parse_uint(&retval, "%s", filename) < 0 || retval != valid_number) {
+		printf("rte_sysfs_parse_uint() failed without trailing newline - test failed\n");
+		goto error;
+	}
+	printf("Read '%u' (no newline) ok\n", valid_number);
 
-	/* test reading an empty file - expect failure!*/
-	fd = fopen(filename,"w");
+	/* a negative value is rejected by the unsigned variant ... */
+	fd = fopen(filename, "w");
 	if (fd == NULL) {
 		printf("line %d, Error opening %s: %s\n", __LINE__, filename, strerror(errno));
 		goto error;
 	}
+	fprintf(fd, "-1\n");
 	fclose(fd);
 	fd = NULL;
-	if (eal_parse_sysfs_value(filename, &retval) == 0) {
-		printf("eal_parse_sysfs_value() read invalid value  - test failed\n");
+	if (rte_sysfs_parse_uint(&retval, "%s", filename) == 0) {
+		printf("rte_sysfs_parse_uint() accepted a negative value - test failed\n");
 		goto error;
 	}
 
-	/* test reading a valid number value *without* "\n" on the end - expect failure!*/
-	valid_number = 3;
+	/* ... and read correctly by the signed one, as for numa_node */
+	if (rte_sysfs_parse_int(&sretval, "%s", filename) < 0 || sretval != -1) {
+		printf("rte_sysfs_parse_int() failed to read -1 - test failed\n");
+		goto error;
+	}
+	printf("Read '-1' as signed ok\n");
+
+	/* string read strips the trailing newline */
+	fd = fopen(filename, "w");
+	if (fd == NULL) {
+		printf("line %d, Error opening %s: %s\n", __LINE__, filename, strerror(errno));
+		goto error;
+	}
+	fprintf(fd, "performance\n");
+	fclose(fd);
+	fd = NULL;
+	if (rte_sysfs_parse_string(strval, sizeof(strval), "%s", filename) < 0 ||
+			strcmp(strval, "performance") != 0) {
+		printf("rte_sysfs_parse_string() returned '%s' - test failed\n", strval);
+		goto error;
+	}
+	printf("Read 'performance' ok\n");
+
+	/* write it back and read it again */
+	if (rte_sysfs_write_string("powersave", "%s", filename) < 0) {
+		printf("rte_sysfs_write_string() returned error - test failed\n");
+		goto error;
+	}
+	if (rte_sysfs_parse_string(strval, sizeof(strval), "%s", filename) < 0 ||
+			strcmp(strval, "powersave") != 0) {
+		printf("read back '%s' after write - test failed\n", strval);
+		goto error;
+	}
+	printf("Wrote and read back 'powersave' ok\n");
+
+	printf("Test reading invalid values ...\n");
+
+	/* test reading an empty file - expect failure!*/
 	fd = fopen(filename,"w");
 	if (fd == NULL) {
 		printf("line %d, Error opening %s: %s\n", __LINE__, filename, strerror(errno));
 		goto error;
 	}
-	fprintf(fd,"%u", valid_number);
 	fclose(fd);
 	fd = NULL;
-	if (eal_parse_sysfs_value(filename, &retval) == 0) {
-		printf("eal_parse_sysfs_value() read invalid value  - test failed\n");
+	if (rte_sysfs_parse_uint(&retval, "%s", filename) == 0) {
+		printf("rte_sysfs_parse_uint() read invalid value  - test failed\n");
 		goto error;
 	}
 
@@ -141,8 +190,8 @@ test_parse_sysfs_value(void)
 	fprintf(fd,"%uJ\n", valid_number);
 	fclose(fd);
 	fd = NULL;
-	if (eal_parse_sysfs_value(filename, &retval) == 0) {
-		printf("eal_parse_sysfs_value() read invalid value  - test failed\n");
+	if (rte_sysfs_parse_uint(&retval, "%s", filename) == 0) {
+		printf("rte_sysfs_parse_uint() read invalid value  - test failed\n");
 		goto error;
 	}
 
@@ -155,14 +204,45 @@ test_parse_sysfs_value(void)
 	fprintf(fd,"error\n");
 	fclose(fd);
 	fd = NULL;
-	if (eal_parse_sysfs_value(filename, &retval) == 0) {
-		printf("eal_parse_sysfs_value() read invalid value  - test failed\n");
+	if (rte_sysfs_parse_uint(&retval, "%s", filename) == 0) {
+		printf("rte_sysfs_parse_uint() read invalid value  - test failed\n");
+		goto error;
+	}
+
+	/* test reading a negative value as unsigned - expect failure! */
+	fd = fopen(filename, "w");
+	if (fd == NULL) {
+		printf("line %d, Error opening %s: %s\n", __LINE__, filename, strerror(errno));
+		goto error;
+	}
+	fprintf(fd, "-1\n");
+	fclose(fd);
+	fd = NULL;
+	if (rte_sysfs_parse_uint(&retval, "%s", filename) == 0) {
+		printf("rte_sysfs_parse_uint() read negative value  - test failed\n");
+		goto error;
+	}
+
+	/*
+	 * Same, but with leading whitespace: strtoul() skips it before it
+	 * negates, so the sign has to be looked for past the whitespace.
+	 */
+	fd = fopen(filename, "w");
+	if (fd == NULL) {
+		printf("line %d, Error opening %s: %s\n", __LINE__, filename, strerror(errno));
+		goto error;
+	}
+	fprintf(fd, " -1\n");
+	fclose(fd);
+	fd = NULL;
+	if (rte_sysfs_parse_uint(&retval, "%s", filename) == 0) {
+		printf("rte_sysfs_parse_uint() read negative value  - test failed\n");
 		goto error;
 	}
 
 	close(tmp_file_handle);
 	unlink(filename);
-	printf("eal_parse_sysfs_value() - OK\n");
+	printf("sysfs value functions - OK\n");
 	return 0;
 
 error:
@@ -183,6 +263,6 @@ test_eal_fs(void)
 	return 0;
 }
 
-#endif /* !RTE_EXEC_ENV_WINDOWS */
+#endif /* RTE_EXEC_ENV_LINUX */
 
 REGISTER_FAST_TEST(eal_fs_autotest, NOHUGE_OK, ASAN_OK, test_eal_fs);
diff --git a/drivers/bus/auxiliary/linux/auxiliary.c b/drivers/bus/auxiliary/linux/auxiliary.c
index 3a2dca2865..c8d289eecb 100644
--- a/drivers/bus/auxiliary/linux/auxiliary.c
+++ b/drivers/bus/auxiliary/linux/auxiliary.c
@@ -9,6 +9,7 @@
 #include <rte_malloc.h>
 #include <rte_devargs.h>
 #include <rte_memcpy.h>
+#include <rte_sysfs.h>
 #include <eal_filesystem.h>
 
 #include "../private.h"
@@ -21,8 +22,7 @@ auxiliary_scan_one(const char *dirname, const char *name)
 {
 	struct rte_auxiliary_device *dev;
 	struct rte_auxiliary_device *dev2;
-	char filename[PATH_MAX];
-	unsigned long tmp;
+	long num;
 	int ret;
 
 	dev = malloc(sizeof(*dev));
@@ -36,12 +36,9 @@ auxiliary_scan_one(const char *dirname, const char *name)
 	}
 	dev->device.name = dev->name;
 
-	/* Get NUMA node, default to 0 if not present */
-	snprintf(filename, sizeof(filename), "%s/%s/numa_node",
-		 dirname, name);
-	if (access(filename, F_OK) == 0 &&
-	    eal_parse_sysfs_value(filename, &tmp) == 0)
-		dev->device.numa_node = tmp;
+	/* Get NUMA node, default to SOCKET_ID_ANY if not present */
+	if (rte_sysfs_parse_int(&num, "%s/%s/numa_node", dirname, name) == 0)
+		dev->device.numa_node = num;
 	else
 		dev->device.numa_node = SOCKET_ID_ANY;
 
diff --git a/drivers/bus/cdx/cdx.c b/drivers/bus/cdx/cdx.c
index c0b46a41ad..8cbe2473a9 100644
--- a/drivers/bus/cdx/cdx.c
+++ b/drivers/bus/cdx/cdx.c
@@ -74,6 +74,7 @@
 #include <rte_kvargs.h>
 #include <rte_malloc.h>
 #include <rte_vfio.h>
+#include <rte_sysfs.h>
 
 #include <eal_export.h>
 #include <eal_filesystem.h>
@@ -179,16 +180,14 @@ cdx_scan_one(const char *dirname, const char *dev_name)
 	}
 
 	/* get vendor id */
-	snprintf(filename, sizeof(filename), "%s/vendor", dirname);
-	if (eal_parse_sysfs_value(filename, &tmp) < 0) {
+	if (rte_sysfs_parse_uint(&tmp, "%s/vendor", dirname) < 0) {
 		ret = -1;
 		goto err;
 	}
 	dev->id.vendor_id = (uint16_t)tmp;
 
 	/* get device id */
-	snprintf(filename, sizeof(filename), "%s/device", dirname);
-	if (eal_parse_sysfs_value(filename, &tmp) < 0) {
+	if (rte_sysfs_parse_uint(&tmp, "%s/device", dirname) < 0) {
 		ret = -1;
 		goto err;
 	}
diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index 9aae0a5d14..d40a09e78a 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -12,6 +12,7 @@
 #include <rte_devargs.h>
 #include <rte_memcpy.h>
 #include <rte_vfio.h>
+#include <rte_sysfs.h>
 
 #include <eal_export.h>
 #include "eal_filesystem.h"
@@ -205,6 +206,7 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 {
 	char filename[PATH_MAX];
 	unsigned long tmp;
+	long num;
 	struct rte_pci_device_internal *pdev;
 	struct rte_pci_device *dev;
 	char driver[PATH_MAX];
@@ -221,43 +223,35 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 	dev->addr = *addr;
 
 	/* get vendor id */
-	snprintf(filename, sizeof(filename), "%s/vendor", dirname);
-	if (eal_parse_sysfs_value(filename, &tmp) < 0) {
+	if (rte_sysfs_parse_uint(&tmp, "%s/vendor", dirname) < 0) {
 		pci_free(pdev);
 		return -1;
 	}
 	dev->id.vendor_id = (uint16_t)tmp;
 
 	/* get device id */
-	snprintf(filename, sizeof(filename), "%s/device", dirname);
-	if (eal_parse_sysfs_value(filename, &tmp) < 0) {
+	if (rte_sysfs_parse_uint(&tmp, "%s/device", dirname) < 0) {
 		pci_free(pdev);
 		return -1;
 	}
 	dev->id.device_id = (uint16_t)tmp;
 
 	/* get subsystem_vendor id */
-	snprintf(filename, sizeof(filename), "%s/subsystem_vendor",
-		 dirname);
-	if (eal_parse_sysfs_value(filename, &tmp) < 0) {
+	if (rte_sysfs_parse_uint(&tmp, "%s/subsystem_vendor", dirname) < 0) {
 		pci_free(pdev);
 		return -1;
 	}
 	dev->id.subsystem_vendor_id = (uint16_t)tmp;
 
 	/* get subsystem_device id */
-	snprintf(filename, sizeof(filename), "%s/subsystem_device",
-		 dirname);
-	if (eal_parse_sysfs_value(filename, &tmp) < 0) {
+	if (rte_sysfs_parse_uint(&tmp, "%s/subsystem_device", dirname) < 0) {
 		pci_free(pdev);
 		return -1;
 	}
 	dev->id.subsystem_device_id = (uint16_t)tmp;
 
 	/* get class_id */
-	snprintf(filename, sizeof(filename), "%s/class",
-		 dirname);
-	if (eal_parse_sysfs_value(filename, &tmp) < 0) {
+	if (rte_sysfs_parse_uint(&tmp, "%s/class", dirname) < 0) {
 		pci_free(pdev);
 		return -1;
 	}
@@ -266,25 +260,15 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 
 	/* get max_vfs */
 	dev->max_vfs = 0;
-	snprintf(filename, sizeof(filename), "%s/max_vfs", dirname);
-	if (!access(filename, F_OK) &&
-	    eal_parse_sysfs_value(filename, &tmp) == 0)
+	if (rte_sysfs_parse_uint(&tmp, "%s/max_vfs", dirname) == 0)
+		dev->max_vfs = (uint16_t)tmp;
+	/* for non igb_uio driver, need kernel version >= 3.8 */
+	else if (rte_sysfs_parse_uint(&tmp, "%s/sriov_numvfs", dirname) == 0)
 		dev->max_vfs = (uint16_t)tmp;
-	else {
-		/* for non igb_uio driver, need kernel version >= 3.8 */
-		snprintf(filename, sizeof(filename),
-			 "%s/sriov_numvfs", dirname);
-		if (!access(filename, F_OK) &&
-		    eal_parse_sysfs_value(filename, &tmp) == 0)
-			dev->max_vfs = (uint16_t)tmp;
-	}
-
-	/* get numa node, default to 0 if not present */
-	snprintf(filename, sizeof(filename), "%s/numa_node", dirname);
 
-	if (access(filename, F_OK) == 0 &&
-	    eal_parse_sysfs_value(filename, &tmp) == 0)
-		dev->device.numa_node = tmp;
+	/* get numa node, default to SOCKET_ID_ANY if not present */
+	if (rte_sysfs_parse_int(&num, "%s/numa_node", dirname) == 0)
+		dev->device.numa_node = num;
 	else
 		dev->device.numa_node = SOCKET_ID_ANY;
 
diff --git a/drivers/bus/platform/platform.c b/drivers/bus/platform/platform.c
index 90d865a8df..9585fb79e9 100644
--- a/drivers/bus/platform/platform.c
+++ b/drivers/bus/platform/platform.c
@@ -24,6 +24,7 @@
 #include <rte_memory.h>
 #include <rte_string_fns.h>
 #include <rte_vfio.h>
+#include <rte_sysfs.h>
 
 #include "private.h"
 
@@ -49,8 +50,7 @@ static int
 dev_add(const char *dev_name)
 {
 	struct rte_platform_device *pdev, *tmp;
-	char path[PATH_MAX];
-	unsigned long val;
+	long val;
 
 	pdev = calloc(1, sizeof(*pdev));
 	if (pdev == NULL)
@@ -59,8 +59,10 @@ dev_add(const char *dev_name)
 	rte_strscpy(pdev->name, dev_name, sizeof(pdev->name));
 	pdev->device.name = pdev->name;
 	pdev->device.devargs = rte_bus_find_devargs(&platform_bus, dev_name);
-	snprintf(path, sizeof(path), PLATFORM_BUS_DEVICES_PATH "/%s/numa_node", dev_name);
-	pdev->device.numa_node = eal_parse_sysfs_value(path, &val) ? rte_socket_id() : val;
+	if (rte_sysfs_parse_int(&val, PLATFORM_BUS_DEVICES_PATH "/%s/numa_node", dev_name) == 0)
+		pdev->device.numa_node = val;
+	else
+		pdev->device.numa_node = rte_socket_id();
 
 	RTE_BUS_FOREACH_DEV(tmp, &platform_bus) {
 		if (!strcmp(tmp->name, pdev->name)) {
diff --git a/drivers/bus/vmbus/linux/vmbus_bus.c b/drivers/bus/vmbus/linux/vmbus_bus.c
index 779ea50b92..9ee7983eb6 100644
--- a/drivers/bus/vmbus/linux/vmbus_bus.c
+++ b/drivers/bus/vmbus/linux/vmbus_bus.c
@@ -19,6 +19,7 @@
 #include <rte_malloc.h>
 #include <rte_bus_vmbus.h>
 #include <rte_kvargs.h>
+#include <rte_sysfs.h>
 
 #include <eal_export.h>
 #include "eal_filesystem.h"
@@ -202,11 +203,8 @@ rte_vmbus_map_device(struct rte_vmbus_device *dev)
 			return -1;
 		}
 
-		snprintf(filename, sizeof(filename),
-			 "%s/size", dirname);
-		if (eal_parse_sysfs_value(filename, &len) < 0) {
-			VMBUS_LOG(ERR,
-				"could not read %s", filename);
+		if (rte_sysfs_parse_uint(&len, "%s/size", dirname) < 0) {
+			VMBUS_LOG(ERR, "could not read size of %s", dirname);
 			return -1;
 		}
 		res->len = len;
@@ -280,6 +278,7 @@ vmbus_scan_one(const char *name)
 	char filename[PATH_MAX];
 	char dirname[PATH_MAX];
 	unsigned long tmp;
+	long num;
 
 	dev = calloc(1, sizeof(*dev));
 	if (dev == NULL)
@@ -314,14 +313,12 @@ vmbus_scan_one(const char *name)
 		goto error;
 
 	/* get relid */
-	snprintf(filename, sizeof(filename), "%s/id", dirname);
-	if (eal_parse_sysfs_value(filename, &tmp) < 0)
+	if (rte_sysfs_parse_uint(&tmp, "%s/id", dirname) < 0)
 		goto error;
 	dev->relid = tmp;
 
 	/* get monitor id */
-	snprintf(filename, sizeof(filename), "%s/monitor_id", dirname);
-	if (eal_parse_sysfs_value(filename, &tmp) >= 0) {
+	if (rte_sysfs_parse_uint(&tmp, "%s/monitor_id", dirname) >= 0) {
 		dev->monitor_id = tmp;
 	} else {
 		VMBUS_LOG(NOTICE, "monitor disabled on %s", name);
@@ -333,14 +330,8 @@ vmbus_scan_one(const char *name)
 	dev->device.numa_node = SOCKET_ID_ANY;
 	if (vmbus_use_numa(dev)) {
 		/* get numa node (if present) */
-		snprintf(filename, sizeof(filename), "%s/numa_node",
-			 dirname);
-
-		if (access(filename, R_OK) == 0) {
-			if (eal_parse_sysfs_value(filename, &tmp) < 0)
-				goto error;
-			dev->device.numa_node = tmp;
-		}
+		if (rte_sysfs_parse_int(&num, "%s/numa_node", dirname) == 0)
+			dev->device.numa_node = num;
 	}
 
 	/* device is valid, add in list (sorted) */
diff --git a/drivers/common/cnxk/roc_model.c b/drivers/common/cnxk/roc_model.c
index f0312a5400..800e1ef013 100644
--- a/drivers/common/cnxk/roc_model.c
+++ b/drivers/common/cnxk/roc_model.c
@@ -105,20 +105,17 @@ is_rvu_device(unsigned long val)
 static int
 rvu_device_lookup(const char *dirname, uint32_t *part, uint32_t *pass)
 {
-	char filename[PATH_MAX];
 	unsigned long val;
 
 	/* Check if vendor id is cavium */
-	snprintf(filename, sizeof(filename), "%s/vendor", dirname);
-	if (plt_sysfs_value_parse(filename, &val) < 0)
+	if (plt_sysfs_value_parse(&val, "%s/vendor", dirname) < 0)
 		goto error;
 
 	if (val != PCI_VENDOR_ID_CAVIUM)
 		goto error;
 
 	/* Get device id  */
-	snprintf(filename, sizeof(filename), "%s/device", dirname);
-	if (plt_sysfs_value_parse(filename, &val) < 0)
+	if (plt_sysfs_value_parse(&val, "%s/device", dirname) < 0)
 		goto error;
 
 	/* Check if device ID belongs to any RVU device */
@@ -126,15 +123,13 @@ rvu_device_lookup(const char *dirname, uint32_t *part, uint32_t *pass)
 		goto error;
 
 	/* Get subsystem_device id */
-	snprintf(filename, sizeof(filename), "%s/subsystem_device", dirname);
-	if (plt_sysfs_value_parse(filename, &val) < 0)
+	if (plt_sysfs_value_parse(&val, "%s/subsystem_device", dirname) < 0)
 		goto error;
 
 	*part = val >> MODEL_CN10K_PART_SHIFT;
 
 	/* Get revision for pass value*/
-	snprintf(filename, sizeof(filename), "%s/revision", dirname);
-	if (plt_sysfs_value_parse(filename, &val) < 0)
+	if (plt_sysfs_value_parse(&val, "%s/revision", dirname) < 0)
 		goto error;
 
 	*pass = val & MODEL_CN10K_PASS_MASK;
@@ -230,31 +225,14 @@ populate_model(struct roc_model *model, uint32_t midr)
 static int
 midr_get(unsigned long *val)
 {
-	const char *file =
-		"/sys/devices/system/cpu/cpu0/regs/identification/midr_el1";
-	int rc = UTIL_ERR_FS;
-	char buf[BUFSIZ];
-	char *end = NULL;
-	FILE *f;
-
 	if (val == NULL)
-		goto err;
-	f = fopen(file, "r");
-	if (f == NULL)
-		goto err;
-
-	if (fgets(buf, sizeof(buf), f) == NULL)
-		goto fclose;
+		return UTIL_ERR_FS;
 
-	*val = strtoul(buf, &end, 0);
-	if ((buf[0] == '\0') || (end == NULL) || (*end != '\n'))
-		goto fclose;
+	if (plt_sysfs_value_parse(val,
+			"/sys/devices/system/cpu/cpu0/regs/identification/midr_el1") < 0)
+		return UTIL_ERR_FS;
 
-	rc = 0;
-fclose:
-	fclose(f);
-err:
-	return rc;
+	return 0;
 }
 
 static void
diff --git a/drivers/common/cnxk/roc_platform.h b/drivers/common/cnxk/roc_platform.h
index ac4f76473f..71f9d9a256 100644
--- a/drivers/common/cnxk/roc_platform.h
+++ b/drivers/common/cnxk/roc_platform.h
@@ -23,6 +23,7 @@
 #include <rte_seqcount.h>
 #include <rte_spinlock.h>
 #include <rte_string_fns.h>
+#include <rte_sysfs.h>
 #include <rte_tailq.h>
 #include <rte_telemetry.h>
 
@@ -109,7 +110,7 @@
 #define plt_pci_device		    rte_pci_device
 #define plt_pci_read_config	    rte_pci_read_config
 #define plt_pci_find_ext_capability rte_pci_find_ext_capability
-#define plt_sysfs_value_parse	    eal_parse_sysfs_value
+#define plt_sysfs_value_parse	    rte_sysfs_parse_uint
 
 #define plt_log2_u32	 rte_log2_u32
 #define plt_cpu_to_be_16 rte_cpu_to_be_16
diff --git a/drivers/common/mlx5/linux/mlx5_common_auxiliary.c b/drivers/common/mlx5/linux/mlx5_common_auxiliary.c
index 3ee2f4638a..07bb899f9e 100644
--- a/drivers/common/mlx5/linux/mlx5_common_auxiliary.c
+++ b/drivers/common/mlx5/linux/mlx5_common_auxiliary.c
@@ -10,6 +10,7 @@
 #include <rte_errno.h>
 #include <bus_auxiliary_driver.h>
 #include <rte_common.h>
+#include <rte_sysfs.h>
 #include <eal_export.h>
 #include "eal_filesystem.h"
 
@@ -93,16 +94,12 @@ mlx5_auxiliary_get_pci_str(const struct rte_auxiliary_device *dev,
 static int
 mlx5_auxiliary_get_numa(const struct rte_auxiliary_device *dev)
 {
-	unsigned long numa;
-	char numa_path[PATH_MAX];
+	char pci_path[PATH_MAX];
+	long numa;
 
-	if (mlx5_auxiliary_get_pci_path(dev, numa_path, sizeof(numa_path)) != 0)
+	if (mlx5_auxiliary_get_pci_path(dev, pci_path, sizeof(pci_path)) != 0)
 		return SOCKET_ID_ANY;
-	if (strcat(numa_path, "/numa_node") == NULL) {
-		rte_errno = ENAMETOOLONG;
-		return SOCKET_ID_ANY;
-	}
-	if (eal_parse_sysfs_value(numa_path, &numa) != 0) {
+	if (rte_sysfs_parse_int(&numa, "%s/numa_node", pci_path) != 0) {
 		rte_errno = EINVAL;
 		return SOCKET_ID_ANY;
 	}
diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
index 2cdb533276..9fc48e113b 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -38,6 +38,7 @@
 #include <rte_ring.h>
 #include <rte_spinlock.h>
 #include <rte_power_intrinsics.h>
+#include <rte_sysfs.h>
 
 #include "compat.h"
 #include "eal_filesystem.h"
@@ -2568,15 +2569,13 @@ rte_pmd_af_xdp_probe(struct rte_vdev_device *dev)
 
 	/* get numa node id from net sysfs */
 	if (dev->device.numa_node == SOCKET_ID_ANY) {
-		unsigned long numa = 0;
-		char numa_path[PATH_MAX];
+		long numa;
 
-		snprintf(numa_path, sizeof(numa_path), "/sys/class/net/%s/device/numa_node",
-			 if_name);
-		if (access(numa_path, R_OK) != 0 || eal_parse_sysfs_value(numa_path, &numa) != 0)
-			dev->device.numa_node = rte_socket_id();
-		else
+		if (rte_sysfs_parse_int(&numa, "/sys/class/net/%s/device/numa_node",
+					if_name) == 0)
 			dev->device.numa_node = numa;
+		else
+			dev->device.numa_node = rte_socket_id();
 	}
 
 	busy_budget = busy_budget == -1 ? ETH_AF_XDP_DFLT_BUSY_BUDGET :
diff --git a/lib/eal/common/eal_filesystem.h b/lib/eal/common/eal_filesystem.h
index 912f446f64..9859fe7241 100644
--- a/lib/eal/common/eal_filesystem.h
+++ b/lib/eal/common/eal_filesystem.h
@@ -128,8 +128,6 @@ eal_get_hugefile_list_seg_path(char *buffer, size_t buflen,
 /** define the default filename prefix for the %s values above */
 #define HUGEFILE_PREFIX_DEFAULT "rte"
 
-/** Function to read a single numeric value from a file on the filesystem.
- * Used to read information from files on /sys */
-int eal_parse_sysfs_value(const char *filename, unsigned long *val);
+/* Reading and writing of sysfs values is in <rte_sysfs.h> */
 
 #endif /* EAL_FILESYSTEM_H */
diff --git a/lib/eal/include/meson.build b/lib/eal/include/meson.build
index aef5824e5f..8c0a59f3d7 100644
--- a/lib/eal/include/meson.build
+++ b/lib/eal/include/meson.build
@@ -61,6 +61,7 @@ headers += files(
 driver_sdk_headers = files(
         'bus_driver.h',
         'dev_driver.h',
+        'rte_sysfs.h',
 )
 
 # special case install the generic headers, since they go in a subdir
diff --git a/lib/eal/include/rte_sysfs.h b/lib/eal/include/rte_sysfs.h
new file mode 100644
index 0000000000..d9b59f58c3
--- /dev/null
+++ b/lib/eal/include/rte_sysfs.h
@@ -0,0 +1,121 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2026 Stephen Hemminger
+ */
+
+#ifndef RTE_SYSFS_H
+#define RTE_SYSFS_H
+
+/**
+ * @file
+ * @internal
+ *
+ * Helpers to read and write single values in sysfs, used across DPDK.
+ *
+ * All of these build the path from a printf-style format, so that
+ * callers do not have to construct it separately.
+ */
+
+#include <stdarg.h>
+#include <stddef.h>
+
+#include <rte_common.h>
+#include <rte_compat.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * Read an unsigned numeric value from a file, typically under /sys.
+ *
+ * The value is parsed with strtoul() using base 0, so decimal, octal
+ * and 0x-prefixed hexadecimal are all accepted. A negative value is
+ * rejected rather than wrapping; use rte_sysfs_parse_int() for the
+ * attributes that are signed.
+ *
+ * @param val
+ *   Where to store the parsed value, unmodified on failure.
+ * @param format
+ *   printf-style format describing the path to read.
+ * @return
+ *   0 on success, -1 on error.
+ */
+__rte_internal
+__rte_format_printf(2, 3)
+int rte_sysfs_parse_uint(unsigned long *val, const char *format, ...);
+
+/**
+ * Read an unsigned numeric value from a file, typically under /sys.
+ *
+ * As rte_sysfs_parse_uint(), but takes a va_list, so that a wrapper
+ * can forward its arguments without formatting the path itself.
+ *
+ * @param val
+ *   Where to store the parsed value, unmodified on failure.
+ * @param format
+ *   printf-style format describing the path to read.
+ * @param ap
+ *   Arguments for the format.
+ * @return
+ *   0 on success, -1 on error.
+ */
+__rte_internal
+__rte_format_printf(2, 0)
+int rte_sysfs_vparse_uint(unsigned long *val, const char *format, va_list ap);
+
+/**
+ * Read a signed numeric value from a file, typically under /sys.
+ *
+ * The value is parsed with strtol() using base 0. Some sysfs
+ * attributes are signed, most notably "numa_node" which is -1 when
+ * the device is not associated with any NUMA node.
+ *
+ * @param val
+ *   Where to store the parsed value, unmodified on failure.
+ * @param format
+ *   printf-style format describing the path to read.
+ * @return
+ *   0 on success, -1 on error.
+ */
+__rte_internal
+__rte_format_printf(2, 3)
+int rte_sysfs_parse_int(long *val, const char *format, ...);
+
+/**
+ * Read a string value from a file, typically under /sys.
+ *
+ * The trailing newline, if any, is stripped.
+ *
+ * @param buf
+ *   Where to store the NUL-terminated value. The contents are
+ *   indeterminate on failure.
+ * @param buflen
+ *   Size of buf, the value is truncated if it does not fit.
+ * @param format
+ *   printf-style format describing the path to read.
+ * @return
+ *   0 on success, -1 on error.
+ */
+__rte_internal
+__rte_format_printf(3, 4)
+int rte_sysfs_parse_string(char *buf, size_t buflen, const char *format, ...);
+
+/**
+ * Write a string value to a file, typically under /sys.
+ *
+ * @param str
+ *   The NUL-terminated value to write.
+ * @param format
+ *   printf-style format describing the path to write.
+ * @return
+ *   0 on success, -1 on error.
+ */
+__rte_internal
+__rte_format_printf(2, 3)
+int rte_sysfs_write_string(const char *str, const char *format, ...);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* RTE_SYSFS_H */
diff --git a/lib/eal/linux/eal_hugepage_info.c b/lib/eal/linux/eal_hugepage_info.c
index 05c5b3f613..9c05d86f6e 100644
--- a/lib/eal/linux/eal_hugepage_info.c
+++ b/lib/eal/linux/eal_hugepage_info.c
@@ -25,6 +25,7 @@
 #include <rte_debug.h>
 #include <rte_log.h>
 #include <rte_common.h>
+#include <rte_sysfs.h>
 #include "rte_string_fns.h"
 
 #include "eal_private.h"
@@ -68,18 +69,6 @@ create_shared_memory(const char *filename, const size_t mem_size)
 	return map_shared_memory(filename, mem_size, O_RDWR | O_CREAT);
 }
 
-static int get_hp_sysfs_value(const char *subdir, const char *file, unsigned long *val)
-{
-	char *path = NULL;
-	int ret;
-
-	if (asprintf(&path, "%s/%s/%s", sys_dir_path, subdir, file) < 0)
-		return -1;
-	ret = eal_parse_sysfs_value(path, val);
-	free(path);
-	return ret;
-}
-
 /* this function is only called from eal_hugepage_info_init which itself
  * is only called from a primary process */
 static uint32_t
@@ -92,16 +81,17 @@ get_num_hugepages(const char *subdir, size_t sz, unsigned int reusable_pages)
 	const char *nr_splus_file = "surplus_hugepages";
 
 	/* first, check how many reserved pages kernel reports */
-	if (get_hp_sysfs_value(subdir, nr_rsvd_file, &resv_pages) < 0)
+	if (rte_sysfs_parse_uint(&resv_pages, "%s/%s/%s", sys_dir_path, subdir, nr_rsvd_file) < 0)
 		return 0;
 
-	if (get_hp_sysfs_value(subdir, nr_hp_file, &num_pages) < 0)
+	if (rte_sysfs_parse_uint(&num_pages, "%s/%s/%s", sys_dir_path, subdir, nr_hp_file) < 0)
 		return 0;
 
-	if (get_hp_sysfs_value(subdir, nr_over_file, &over_pages) < 0)
+	if (rte_sysfs_parse_uint(&over_pages, "%s/%s/%s", sys_dir_path, subdir, nr_over_file) < 0)
 		over_pages = 0;
 
-	if (get_hp_sysfs_value(subdir, nr_splus_file, &surplus_pages) < 0)
+	if (rte_sysfs_parse_uint(&surplus_pages, "%s/%s/%s",
+			sys_dir_path, subdir, nr_splus_file) < 0)
 		surplus_pages = 0;
 
 	/* adjust num_pages */
@@ -138,7 +128,7 @@ get_num_hugepages(const char *subdir, size_t sz, unsigned int reusable_pages)
 static uint32_t
 get_num_hugepages_on_node(const char *subdir, unsigned int socket, size_t sz)
 {
-	char *path = NULL, *socketpath = NULL;
+	char *socketpath = NULL;
 	DIR *socketdir;
 	unsigned long num_pages = 0;
 	const char *nr_hp_file = "free_hugepages";
@@ -158,13 +148,7 @@ get_num_hugepages_on_node(const char *subdir, unsigned int socket, size_t sz)
 		goto nopages;
 	}
 
-	if (asprintf(&path, "%s/%s/%s", socketpath, subdir, nr_hp_file) < 0) {
-		EAL_LOG(ERR, "Can not format free hugepages path");
-		path = NULL;
-		goto nopages;
-	}
-
-	if (eal_parse_sysfs_value(path, &num_pages) < 0)
+	if (rte_sysfs_parse_uint(&num_pages, "%s/%s/%s", socketpath, subdir, nr_hp_file) < 0)
 		goto nopages;
 
 	if (num_pages == 0)
@@ -179,7 +163,6 @@ get_num_hugepages_on_node(const char *subdir, unsigned int socket, size_t sz)
 		num_pages = UINT32_MAX;
 
 nopages:
-	free(path);
 	free(socketpath);
 
 	return num_pages;
diff --git a/lib/eal/linux/eal_lcore.c b/lib/eal/linux/eal_lcore.c
index 29b36dd610..ada1c408f4 100644
--- a/lib/eal/linux/eal_lcore.c
+++ b/lib/eal/linux/eal_lcore.c
@@ -6,6 +6,7 @@
 #include <limits.h>
 
 #include <rte_log.h>
+#include <rte_sysfs.h>
 
 #include "eal_private.h"
 #include "eal_filesystem.h"
@@ -57,18 +58,13 @@ eal_cpu_socket_id(unsigned lcore_id)
 unsigned
 eal_cpu_core_id(unsigned lcore_id)
 {
-	char path[PATH_MAX];
 	unsigned long id;
 
-	int len = snprintf(path, sizeof(path), SYS_CPU_DIR "/%s", lcore_id, CORE_ID_FILE);
-	if (len <= 0 || (unsigned)len >= sizeof(path))
-		goto err;
-	if (eal_parse_sysfs_value(path, &id) != 0)
-		goto err;
-	return (unsigned)id;
+	if (rte_sysfs_parse_uint(&id, SYS_CPU_DIR "/%s", lcore_id, CORE_ID_FILE) != 0) {
+		EAL_LOG(ERR, "Error reading core id value from %s "
+				"for lcore %u - assuming core 0", SYS_CPU_DIR, lcore_id);
+		return 0;
+	}
 
-err:
-	EAL_LOG(ERR, "Error reading core id value from %s "
-			"for lcore %u - assuming core 0", SYS_CPU_DIR, lcore_id);
-	return 0;
+	return (unsigned int)id;
 }
diff --git a/lib/eal/linux/eal_sysfs.c b/lib/eal/linux/eal_sysfs.c
new file mode 100644
index 0000000000..a5e411585c
--- /dev/null
+++ b/lib/eal/linux/eal_sysfs.c
@@ -0,0 +1,188 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2026 Stephen Hemminger
+ */
+
+#include <ctype.h>
+#include <errno.h>
+#include <limits.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <rte_log.h>
+#include <rte_sysfs.h>
+
+#include <eal_export.h>
+#include "eal_private.h"
+
+/* build the path from the format, then read the first line of that file */
+static int
+__rte_format_printf(3, 0)
+sysfs_read_line(char *buf, size_t buflen, const char *format, va_list ap)
+{
+	char path[PATH_MAX];
+	FILE *f;
+	int len;
+
+	len = vsnprintf(path, sizeof(path), format, ap);
+	if (len < 0 || len >= (int)sizeof(path)) {
+		EAL_LOG(ERR, "sysfs path too long");
+		return -1;
+	}
+
+	f = fopen(path, "r");
+	if (f == NULL) {
+		/*
+		 * A missing attribute is normal: callers probe for optional
+		 * ones such as max_vfs or numa_node. Anything else, such as
+		 * a permission problem, is worth reporting.
+		 */
+		if (errno == ENOENT)
+			EAL_LOG(DEBUG, "cannot open %s: %s", path, strerror(errno));
+		else
+			EAL_LOG(ERR, "cannot open %s: %s", path, strerror(errno));
+		return -1;
+	}
+
+	if (fgets(buf, buflen, f) == NULL) {
+		EAL_LOG(ERR, "cannot read %s", path);
+		fclose(f);
+		return -1;
+	}
+	fclose(f);
+
+	/* sysfs values are newline terminated, strip it */
+	*strchrnul(buf, '\n') = '\0';
+
+	return 0;
+}
+
+RTE_EXPORT_INTERNAL_SYMBOL(rte_sysfs_vparse_uint)
+int
+rte_sysfs_vparse_uint(unsigned long *val, const char *format, va_list ap)
+{
+	const char *start;
+	char buf[BUFSIZ];
+	unsigned long tmp;
+	char *end;
+
+	if (sysfs_read_line(buf, sizeof(buf), format, ap) < 0)
+		return -1;
+
+	/*
+	 * strtoul() skips leading whitespace and then silently negates a
+	 * leading '-', so " -1" would come back as ULONG_MAX. Look for the
+	 * sign past any whitespace: attributes that are really signed, such
+	 * as numa_node, must use rte_sysfs_parse_int() instead.
+	 */
+	start = buf;
+	while (isspace((unsigned char)*start))
+		++start;
+
+	errno = 0;
+	tmp = strtoul(start, &end, 0);
+	if (end == start || *end != '\0' || errno != 0 || *start == '-') {
+		EAL_LOG(ERR, "cannot parse sysfs value '%s'", buf);
+		return -1;
+	}
+
+	*val = tmp;
+	return 0;
+}
+
+RTE_EXPORT_INTERNAL_SYMBOL(rte_sysfs_parse_uint)
+int
+rte_sysfs_parse_uint(unsigned long *val, const char *format, ...)
+{
+	va_list ap;
+	int ret;
+
+	va_start(ap, format);
+	ret = rte_sysfs_vparse_uint(val, format, ap);
+	va_end(ap);
+
+	return ret;
+}
+
+RTE_EXPORT_INTERNAL_SYMBOL(rte_sysfs_parse_int)
+int
+rte_sysfs_parse_int(long *val, const char *format, ...)
+{
+	char buf[BUFSIZ];
+	va_list ap;
+	char *end;
+	long tmp;
+	int ret;
+
+	va_start(ap, format);
+	ret = sysfs_read_line(buf, sizeof(buf), format, ap);
+	va_end(ap);
+	if (ret < 0)
+		return -1;
+
+	errno = 0;
+	tmp = strtol(buf, &end, 0);
+	if (end == buf || *end != '\0' || errno != 0) {
+		EAL_LOG(ERR, "cannot parse sysfs value '%s'", buf);
+		return -1;
+	}
+
+	*val = tmp;
+	return 0;
+}
+
+RTE_EXPORT_INTERNAL_SYMBOL(rte_sysfs_parse_string)
+int
+rte_sysfs_parse_string(char *buf, size_t buflen, const char *format, ...)
+{
+	va_list ap;
+	int ret;
+
+	va_start(ap, format);
+	ret = sysfs_read_line(buf, buflen, format, ap);
+	va_end(ap);
+
+	return ret;
+}
+
+RTE_EXPORT_INTERNAL_SYMBOL(rte_sysfs_write_string)
+int
+rte_sysfs_write_string(const char *str, const char *format, ...)
+{
+	char path[PATH_MAX];
+	va_list ap;
+	FILE *f;
+	int len;
+
+	va_start(ap, format);
+	len = vsnprintf(path, sizeof(path), format, ap);
+	va_end(ap);
+	if (len < 0 || len >= (int)sizeof(path)) {
+		EAL_LOG(ERR, "sysfs path too long");
+		return -1;
+	}
+
+	f = fopen(path, "w");
+	if (f == NULL) {
+		if (errno == ENOENT)
+			EAL_LOG(DEBUG, "cannot open %s: %s", path, strerror(errno));
+		else
+			EAL_LOG(ERR, "cannot open %s: %s", path, strerror(errno));
+		return -1;
+	}
+
+	if (fputs(str, f) < 0) {
+		EAL_LOG(ERR, "cannot write '%s' to %s", str, path);
+		fclose(f);
+		return -1;
+	}
+
+	/* errors on a sysfs write are reported at close time */
+	if (fclose(f) != 0) {
+		EAL_LOG(ERR, "cannot write '%s' to %s: %s", str, path, strerror(errno));
+		return -1;
+	}
+
+	return 0;
+}
diff --git a/lib/eal/linux/meson.build b/lib/eal/linux/meson.build
index 29ba313218..2313cd7488 100644
--- a/lib/eal/linux/meson.build
+++ b/lib/eal/linux/meson.build
@@ -13,6 +13,7 @@ sources += files(
         'eal_lcore.c',
         'eal_memalloc.c',
         'eal_memory.c',
+        'eal_sysfs.c',
         'eal_thread.c',
         'eal_timer.c',
         'eal_vfio.c',
diff --git a/lib/eal/unix/eal_filesystem.c b/lib/eal/unix/eal_filesystem.c
index 6b8451cd3e..c6e827f805 100644
--- a/lib/eal/unix/eal_filesystem.c
+++ b/lib/eal/unix/eal_filesystem.c
@@ -76,34 +76,3 @@ int eal_create_runtime_dir(void)
 
 	return 0;
 }
-
-/* parse a sysfs (or other) file containing one integer value */
-RTE_EXPORT_SYMBOL(eal_parse_sysfs_value)
-int eal_parse_sysfs_value(const char *filename, unsigned long *val)
-{
-	FILE *f;
-	char buf[BUFSIZ];
-	char *end = NULL;
-
-	if ((f = fopen(filename, "r")) == NULL) {
-		EAL_LOG(ERR, "%s(): cannot open sysfs value %s",
-			__func__, filename);
-		return -1;
-	}
-
-	if (fgets(buf, sizeof(buf), f) == NULL) {
-		EAL_LOG(ERR, "%s(): cannot read sysfs value %s",
-			__func__, filename);
-		fclose(f);
-		return -1;
-	}
-	*val = strtoul(buf, &end, 0);
-	if ((buf[0] == '\0') || (end == NULL) || (*end != '\n')) {
-		EAL_LOG(ERR, "%s(): cannot parse sysfs value %s",
-				__func__, filename);
-		fclose(f);
-		return -1;
-	}
-	fclose(f);
-	return 0;
-}
-- 
2.53.0

[PATCH v2 2/9] dma/idxd: use common sysfs routines

[Stephen Hemminger]

The three local helpers each rebuilt a path and read a value; they
now wrap the EAL routines instead. Note that "numa_node" is -1 when
the device is not tied to a node, so the integer reads use the
signed routine.

The wq "size" and "max_batch_size" attributes were read with
fscanf("%u") and are now converted with base 0, so a value with a
leading zero would parse as octal. The kernel does not print either
with leading zeros.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/dma/idxd/idxd_bus.c | 80 ++++++-------------------------------
 1 file changed, 12 insertions(+), 68 deletions(-)

diff --git a/drivers/dma/idxd/idxd_bus.c b/drivers/dma/idxd/idxd_bus.c
index 2ec526ec09..684dee3d6d 100644
--- a/drivers/dma/idxd/idxd_bus.c
+++ b/drivers/dma/idxd/idxd_bus.c
@@ -16,6 +16,7 @@
 #include <rte_log.h>
 #include <rte_dmadev_pmd.h>
 #include <rte_string_fns.h>
+#include <rte_sysfs.h>
 
 #include "idxd_internal.h"
 
@@ -121,82 +122,24 @@ static int
 read_wq_string(const struct rte_dsa_device *dev, const char *filename,
 		char *value, size_t valuelen)
 {
-	char sysfs_node[PATH_MAX];
-	int len;
-	int fd;
-
-	snprintf(sysfs_node, sizeof(sysfs_node), "%s/%s/%s",
+	return rte_sysfs_parse_string(value, valuelen, "%s/%s/%s",
 			dsa_get_sysfs_path(), dev->wq_name, filename);
-	fd = open(sysfs_node, O_RDONLY);
-	if (fd < 0) {
-		IDXD_PMD_ERR("%s(): opening file '%s' failed: %s",
-				__func__, sysfs_node, strerror(errno));
-		return -1;
-	}
-
-	len = read(fd, value, valuelen - 1);
-	close(fd);
-	if (len < 0) {
-		IDXD_PMD_ERR("%s(): error reading file '%s': %s",
-				__func__, sysfs_node, strerror(errno));
-		return -1;
-	}
-	value[len] = '\0';
-	return 0;
 }
 
 static int
 read_wq_int(struct rte_dsa_device *dev, const char *filename,
-		int *value)
+		long *value)
 {
-	char sysfs_node[PATH_MAX];
-	FILE *f;
-	int ret = 0;
-
-	snprintf(sysfs_node, sizeof(sysfs_node), "%s/%s/%s",
+	return rte_sysfs_parse_int(value, "%s/%s/%s",
 			dsa_get_sysfs_path(), dev->wq_name, filename);
-	f = fopen(sysfs_node, "r");
-	if (f == NULL) {
-		IDXD_PMD_ERR("%s(): opening file '%s' failed: %s",
-				__func__, sysfs_node, strerror(errno));
-		return -1;
-	}
-
-	if (fscanf(f, "%d", value) != 1) {
-		IDXD_PMD_ERR("%s(): error reading file '%s': %s",
-				__func__, sysfs_node, strerror(errno));
-		ret = -1;
-	}
-
-	fclose(f);
-	return ret;
 }
 
 static int
 read_device_int(struct rte_dsa_device *dev, const char *filename,
-		int *value)
+		long *value)
 {
-	char sysfs_node[PATH_MAX];
-	FILE *f;
-	int ret = 0;
-
-	snprintf(sysfs_node, sizeof(sysfs_node), "%s/dsa%d/%s",
+	return rte_sysfs_parse_int(value, "%s/dsa%d/%s",
 			dsa_get_sysfs_path(), dev->addr.device_id, filename);
-	f = fopen(sysfs_node, "r");
-	if (f == NULL) {
-		IDXD_PMD_ERR("%s(): opening file '%s' failed: %s",
-				__func__, sysfs_node, strerror(errno));
-		return -1;
-	}
-
-	if (fscanf(f, "%d", value) != 1) {
-		IDXD_PMD_ERR("%s(): error reading file '%s': %s",
-				__func__, sysfs_node, strerror(errno));
-		ret = -1;
-	}
-
-	fclose(f);
-	return ret;
 }
 
 static int
@@ -205,15 +148,16 @@ dsa_probe_device(__rte_unused struct rte_driver *drv, struct rte_device *dev)
 	struct rte_dsa_device *dsa_dev = RTE_BUS_DEVICE(dev, *dsa_dev);
 	struct idxd_dmadev idxd = {0};
 	int ret = 0;
+	long val;
 
 	IDXD_PMD_INFO("Probing device %s on numa node %d",
 			dsa_dev->wq_name, dsa_dev->device.numa_node);
-	if (read_wq_int(dsa_dev, "size", &ret) < 0)
+	if (read_wq_int(dsa_dev, "size", &val) < 0)
 		return -1;
-	idxd.max_batches = ret;
-	if (read_wq_int(dsa_dev, "max_batch_size", &ret) < 0)
+	idxd.max_batches = val;
+	if (read_wq_int(dsa_dev, "max_batch_size", &val) < 0)
 		return -1;
-	idxd.max_batch_size = ret;
+	idxd.max_batch_size = val;
 	idxd.qid = dsa_dev->addr.wq_id;
 	idxd.u.bus.dsa_id = dsa_dev->addr.device_id;
 	idxd.sva_support = 1;
@@ -286,7 +230,7 @@ dsa_scan(void)
 
 	while ((wq = readdir(dev_dir)) != NULL) {
 		struct rte_dsa_device *dev;
-		int numa_node = SOCKET_ID_ANY;
+		long numa_node = SOCKET_ID_ANY;
 
 		if (strncmp(wq->d_name, "wq", 2) != 0)
 			continue;
-- 
2.53.0

[PATCH v2 3/9] common/ionic: use common sysfs routines

[Stephen Hemminger]

The three resource helpers and the UIO name lookup each built a path
with an unchecked sprintf() into a 64 byte buffer and then read the
value with fscanf(). Use the EAL routines instead; base 0 conversion
handles the "0x" prefix these files use.

The scan walks /sys/class/uio, so it only ever found anything on
Linux. Build the implementation there only, and keep no-op stubs
for the rest so that the vdev probe in net/ionic and crypto/ionic
still links where it did before.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/common/ionic/ionic_common_uio.c | 102 ++++++++++++------------
 1 file changed, 51 insertions(+), 51 deletions(-)

diff --git a/drivers/common/ionic/ionic_common_uio.c b/drivers/common/ionic/ionic_common_uio.c
index aaefab918c..04c184c2ee 100644
--- a/drivers/common/ionic/ionic_common_uio.c
+++ b/drivers/common/ionic/ionic_common_uio.c
@@ -22,6 +22,16 @@
 
 #include "ionic_common.h"
 
+/*
+ * The mnic and mcrypt devices are found by walking /sys/class/uio,
+ * so this only ever does anything on Linux. The stubs at the end of
+ * the file keep the vdev probe in net/ionic and crypto/ionic linking
+ * on other systems, where the scan would have found nothing anyway.
+ */
+#ifdef RTE_EXEC_ENV_LINUX
+
+#include <rte_sysfs.h>
+
 #define IONIC_MDEV_UNK      "mdev_unknown"
 #define IONIC_MNIC          "cpu_mnic"
 #define IONIC_MCRYPT        "cpu_mcrypt"
@@ -63,30 +73,17 @@ struct uio_name {
 static void
 uio_fill_name_cache(struct uio_name *name_cache, const char *pfx)
 {
-	char file[64];
-	FILE *fp;
-	char *ret;
 	int name_idx = 0;
 	int i;
 
 	for (i = 0; i < IONIC_UIO_MAX_TRIES &&
 			name_idx < IONIC_MAX_DEVICES; i++) {
-		sprintf(file, "/sys/class/uio/uio%d/name", i);
-
-		fp = fopen(file, "r");
-		if (fp == NULL)
+		if (rte_sysfs_parse_string(name_cache[name_idx].name, IONIC_MAX_NAME_LEN,
+				"/sys/class/uio/uio%d/name", i) < 0)
 			continue;
 
-		ret = fgets(name_cache[name_idx].name, IONIC_MAX_NAME_LEN, fp);
-		if (ret == NULL) {
-			fclose(fp);
-			continue;
-		}
-
 		name_cache[name_idx].idx = i;
 
-		fclose(fp);
-
 		if (strncmp(name_cache[name_idx].name, pfx, strlen(pfx)) == 0)
 			name_idx++;
 	}
@@ -215,21 +212,12 @@ static unsigned long
 uio_get_res_size(int uio_idx, int res_idx)
 {
 	unsigned long size;
-	char file[64];
-	FILE *fp;
 
-	sprintf(file, "/sys/class/uio/uio%d/maps/map%d/size",
-		uio_idx, res_idx);
-
-	fp = fopen(file, "r");
-	if (fp == NULL)
+	/* zero is the error value for all of these */
+	if (rte_sysfs_parse_uint(&size, "/sys/class/uio/uio%d/maps/map%d/size",
+			uio_idx, res_idx) < 0)
 		return 0;
 
-	if (fscanf(fp, "0x%lx", &size) != 1)
-		size = 0;
-
-	fclose(fp);
-
 	return size;
 }
 
@@ -237,21 +225,12 @@ static unsigned long
 uio_get_res_phy_addr_offs(int uio_idx, int res_idx)
 {
 	unsigned long offset;
-	char file[64];
-	FILE *fp;
-
-	sprintf(file, "/sys/class/uio/uio%d/maps/map%d/offset",
-		uio_idx, res_idx);
 
-	fp = fopen(file, "r");
-	if (fp == NULL)
+	/* zero is the error value for all of these */
+	if (rte_sysfs_parse_uint(&offset, "/sys/class/uio/uio%d/maps/map%d/offset",
+			uio_idx, res_idx) < 0)
 		return 0;
 
-	if (fscanf(fp, "0x%lx", &offset) != 1)
-		offset = 0;
-
-	fclose(fp);
-
 	return offset;
 }
 
@@ -259,21 +238,12 @@ static unsigned long
 uio_get_res_phy_addr(int uio_idx, int res_idx)
 {
 	unsigned long addr;
-	char file[64];
-	FILE *fp;
-
-	sprintf(file, "/sys/class/uio/uio%d/maps/map%d/addr",
-		uio_idx, res_idx);
 
-	fp = fopen(file, "r");
-	if (fp == NULL)
+	/* zero is the error value for all of these */
+	if (rte_sysfs_parse_uint(&addr, "/sys/class/uio/uio%d/maps/map%d/addr",
+			uio_idx, res_idx) < 0)
 		return 0;
 
-	if (fscanf(fp, "0x%lx", &addr) != 1)
-		addr = 0;
-
-	fclose(fp);
-
 	return addr;
 }
 
@@ -338,3 +308,33 @@ ionic_uio_rel_rsrc(const char *name, int idx, struct ionic_dev_bar *bar)
 	offs = uio_get_res_phy_addr_offs(num, idx);
 	munmap(((char *)bar->vaddr) - offs, bar->len);
 }
+
+#else /* !RTE_EXEC_ENV_LINUX */
+
+RTE_EXPORT_INTERNAL_SYMBOL(ionic_uio_scan_mnet_devices)
+void
+ionic_uio_scan_mnet_devices(void)
+{
+}
+
+RTE_EXPORT_INTERNAL_SYMBOL(ionic_uio_scan_mcrypt_devices)
+void
+ionic_uio_scan_mcrypt_devices(void)
+{
+}
+
+RTE_EXPORT_INTERNAL_SYMBOL(ionic_uio_get_rsrc)
+void
+ionic_uio_get_rsrc(const char *name __rte_unused, int idx __rte_unused,
+		struct ionic_dev_bar *bar __rte_unused)
+{
+}
+
+RTE_EXPORT_INTERNAL_SYMBOL(ionic_uio_rel_rsrc)
+void
+ionic_uio_rel_rsrc(const char *name __rte_unused, int idx __rte_unused,
+		struct ionic_dev_bar *bar __rte_unused)
+{
+}
+
+#endif /* RTE_EXEC_ENV_LINUX */
-- 
2.53.0

[PATCH v2 4/9] bus/vmbus: use common sysfs routines

[Stephen Hemminger]

Replace the open coded read in vmbus_uio_sysfs_read() with the EAL
routine. The range check and the -ERANGE return are unchanged.

Two things do change. On an open failure the helper now returns -EIO
rather than -errno; both callers only pass it to strerror(-err) in a
log message, so this is message text only. And subchannel_id and
monitor_id were read with fscanf("%u") and are now converted with
base 0, so a value with a leading zero would parse as octal. Neither
attribute is printed with leading zeros by the kernel.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/bus/vmbus/linux/vmbus_uio.c | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/drivers/bus/vmbus/linux/vmbus_uio.c b/drivers/bus/vmbus/linux/vmbus_uio.c
index fbafc5027d..50e58f9b65 100644
--- a/drivers/bus/vmbus/linux/vmbus_uio.c
+++ b/drivers/bus/vmbus/linux/vmbus_uio.c
@@ -18,6 +18,7 @@
 #include <rte_malloc.h>
 #include <rte_bus_vmbus.h>
 #include <rte_string_fns.h>
+#include <rte_sysfs.h>
 
 #include "private.h"
 
@@ -334,27 +335,12 @@ int vmbus_uio_map_rings(struct vmbus_channel *chan)
 static int vmbus_uio_sysfs_read(const char *dir, const char *name,
 				unsigned long *val, unsigned long max_range)
 {
-	char path[PATH_MAX];
-	FILE *f;
-	int ret;
-
-	snprintf(path, sizeof(path), "%s/%s", dir, name);
-	f = fopen(path, "r");
-	if (!f) {
-		VMBUS_LOG(ERR, "can't open %s:%s",
-			  path, strerror(errno));
-		return -errno;
+	if (rte_sysfs_parse_uint(val, "%s/%s", dir, name) < 0) {
+		VMBUS_LOG(ERR, "can't read %s/%s", dir, name);
+		return -EIO;
 	}
 
-	if (fscanf(f, "%lu", val) != 1)
-		ret = -EIO;
-	else if (*val > max_range)
-		ret = -ERANGE;
-	else
-		ret = 0;
-	fclose(f);
-
-	return ret;
+	return *val > max_range ? -ERANGE : 0;
 }
 
 static bool vmbus_uio_ring_present(const struct rte_vmbus_device *dev,
-- 
2.53.0

[PATCH v2 5/9] power: use common sysfs routines

[Stephen Hemminger]

The power library and its backends open a sysfs file, read one value
from it, and close it again in many places. Where the file is only
read once, use the EAL routines instead.

Add power_sysfs_read_u32() as a thin wrapper so the backends keep
reading into a uint32_t. It rejects a value that does not fit rather
than truncating it, which the open coded fscanf("%u") could not.

The files that are held open for the lifetime of the device, the
"rw+" handles used to repeatedly read and write the scaling
frequency, are left alone: they are not a single read, and closing
and reopening them on every frequency change would be a behaviour
change on a hot path.

This also removes a few cases where the FILE pointer was left
uninitialised and then tested against NULL in the cleanup path.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/power/acpi/acpi_cpufreq.c             | 21 ++--
 drivers/power/amd_pstate/amd_pstate_cpufreq.c | 63 ++----------
 drivers/power/cppc/cppc_cpufreq.c             | 69 ++-----------
 .../power/intel_pstate/intel_pstate_cpufreq.c | 99 +++----------------
 drivers/power/intel_uncore/intel_uncore.c     | 32 ++----
 lib/power/power_common.c                      | 52 +++++-----
 lib/power/power_common.h                      |  6 ++
 lib/power/rte_power_qos.c                     | 29 +-----
 8 files changed, 87 insertions(+), 284 deletions(-)

diff --git a/drivers/power/acpi/acpi_cpufreq.c b/drivers/power/acpi/acpi_cpufreq.c
index af85a8cdec..84e33375d0 100644
--- a/drivers/power/acpi/acpi_cpufreq.c
+++ b/drivers/power/acpi/acpi_cpufreq.c
@@ -9,6 +9,7 @@
 #include <rte_memcpy.h>
 #include <rte_stdatomic.h>
 #include <rte_string_fns.h>
+#include <rte_sysfs.h>
 
 #include "acpi_cpufreq.h"
 #include "power_common.h"
@@ -111,21 +112,14 @@ power_set_governor_original(struct acpi_power_info *pi)
 static int
 power_get_available_freqs(struct acpi_power_info *pi)
 {
-	FILE *f;
+	char *freqs[RTE_MAX_LCORE_FREQS];
 	int ret = -1, i, count;
-	char *p;
 	char buf[BUFSIZ];
-	char *freqs[RTE_MAX_LCORE_FREQS];
-
-	open_core_sysfs_file(&f, "r", POWER_SYSFILE_AVAIL_FREQ, pi->lcore_id);
-	if (f == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_AVAIL_FREQ);
-		goto out;
-	}
+	char *p;
 
-	ret = read_core_sysfs_s(f, buf, sizeof(buf));
-	if ((ret) < 0) {
+	ret = rte_sysfs_parse_string(buf, sizeof(buf), POWER_SYSFILE_AVAIL_FREQ,
+			pi->lcore_id);
+	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_AVAIL_FREQ);
 		goto out;
@@ -169,9 +163,6 @@ power_get_available_freqs(struct acpi_power_info *pi)
 	POWER_DEBUG_LOG("%d frequency(s) of lcore %u are available",
 			count, pi->lcore_id);
 out:
-	if (f != NULL)
-		fclose(f);
-
 	return ret;
 }
 
diff --git a/drivers/power/amd_pstate/amd_pstate_cpufreq.c b/drivers/power/amd_pstate/amd_pstate_cpufreq.c
index af9c1309f3..1d9d4cb495 100644
--- a/drivers/power/amd_pstate/amd_pstate_cpufreq.c
+++ b/drivers/power/amd_pstate/amd_pstate_cpufreq.c
@@ -103,34 +103,19 @@ power_set_governor_userspace(struct amd_pstate_power_info *pi)
 static int
 power_check_turbo(struct amd_pstate_power_info *pi)
 {
-	FILE *f_nom = NULL, *f_max = NULL;
 	int ret = -1;
 	uint32_t nominal_perf = 0, highest_perf = 0;
 
-	open_core_sysfs_file(&f_max, "r", POWER_SYSFILE_HIGHEST_PERF,
+	ret = power_sysfs_read_u32(&highest_perf, POWER_SYSFILE_HIGHEST_PERF,
 			pi->lcore_id);
-	if (f_max == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_HIGHEST_PERF);
-		goto err;
-	}
-
-	open_core_sysfs_file(&f_nom, "r", POWER_SYSFILE_NOMINAL_PERF,
-			pi->lcore_id);
-	if (f_nom == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_NOMINAL_PERF);
-		goto err;
-	}
-
-	ret = read_core_sysfs_u32(f_max, &highest_perf);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_HIGHEST_PERF);
 		goto err;
 	}
 
-	ret = read_core_sysfs_u32(f_nom, &nominal_perf);
+	ret = power_sysfs_read_u32(&nominal_perf, POWER_SYSFILE_NOMINAL_PERF,
+			pi->lcore_id);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_NOMINAL_PERF);
@@ -156,10 +141,6 @@ power_check_turbo(struct amd_pstate_power_info *pi)
 	}
 
 err:
-	if (f_max != NULL)
-		fclose(f_max);
-	if (f_nom != NULL)
-		fclose(f_nom);
 
 	return ret;
 }
@@ -171,52 +152,30 @@ power_check_turbo(struct amd_pstate_power_info *pi)
 static int
 power_get_available_freqs(struct amd_pstate_power_info *pi)
 {
-	FILE *f_min = NULL, *f_max = NULL, *f_nom = NULL;
 	int ret = -1, nominal_idx = -1;
 	uint32_t scaling_min_freq = 0, scaling_max_freq = 0;
 	uint32_t i, num_freqs = RTE_MAX_LCORE_FREQS;
 	uint32_t nominal_freq = 0, scaling_freq = 0;
 	uint32_t freq_calc = 0;
 
-	open_core_sysfs_file(&f_max, "r", POWER_SYSFILE_SCALING_MAX_FREQ,
+	ret = power_sysfs_read_u32(&scaling_max_freq, POWER_SYSFILE_SCALING_MAX_FREQ,
 			pi->lcore_id);
-	if (f_max == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_SCALING_MAX_FREQ);
-		goto out;
-	}
-
-	open_core_sysfs_file(&f_min, "r", POWER_SYSFILE_SCALING_MIN_FREQ,
-			pi->lcore_id);
-	if (f_min == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_SCALING_MIN_FREQ);
-		goto out;
-	}
-
-	open_core_sysfs_file(&f_nom, "r", POWER_SYSFILE_NOMINAL_FREQ,
-			pi->lcore_id);
-	if (f_nom == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_NOMINAL_FREQ);
-		goto out;
-	}
-
-	ret = read_core_sysfs_u32(f_max, &scaling_max_freq);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_SCALING_MAX_FREQ);
 		goto out;
 	}
 
-	ret = read_core_sysfs_u32(f_min, &scaling_min_freq);
+	ret = power_sysfs_read_u32(&scaling_min_freq, POWER_SYSFILE_SCALING_MIN_FREQ,
+			pi->lcore_id);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_SCALING_MIN_FREQ);
 		goto out;
 	}
 
-	ret = read_core_sysfs_u32(f_nom, &nominal_freq);
+	ret = power_sysfs_read_u32(&nominal_freq, POWER_SYSFILE_NOMINAL_FREQ,
+			pi->lcore_id);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_NOMINAL_FREQ);
@@ -272,12 +231,6 @@ power_get_available_freqs(struct amd_pstate_power_info *pi)
 			num_freqs, pi->lcore_id);
 
 out:
-	if (f_min != NULL)
-		fclose(f_min);
-	if (f_max != NULL)
-		fclose(f_max);
-	if (f_nom != NULL)
-		fclose(f_nom);
 
 	return ret;
 }
diff --git a/drivers/power/cppc/cppc_cpufreq.c b/drivers/power/cppc/cppc_cpufreq.c
index aed44c1212..a25dc7eaf1 100644
--- a/drivers/power/cppc/cppc_cpufreq.c
+++ b/drivers/power/cppc/cppc_cpufreq.c
@@ -106,49 +106,27 @@ power_set_governor_userspace(struct cppc_power_info *pi)
 static int
 power_check_turbo(struct cppc_power_info *pi)
 {
-	FILE *f_nom = NULL, *f_max = NULL, *f_cmax = NULL;
-	int ret = -1;
 	uint32_t nominal_perf = 0, highest_perf = 0, cpuinfo_max_freq = 0;
+	int ret = -1;
 
-	open_core_sysfs_file(&f_max, "r", POWER_SYSFILE_HIGHEST_PERF,
-			pi->lcore_id);
-	if (f_max == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_HIGHEST_PERF);
-		goto err;
-	}
-
-	open_core_sysfs_file(&f_nom, "r", POWER_SYSFILE_NOMINAL_PERF,
-			pi->lcore_id);
-	if (f_nom == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_NOMINAL_PERF);
-		goto err;
-	}
-
-	open_core_sysfs_file(&f_cmax, "r", POWER_SYSFILE_SYS_MAX,
+	ret = power_sysfs_read_u32(&highest_perf, POWER_SYSFILE_HIGHEST_PERF,
 			pi->lcore_id);
-	if (f_cmax == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_SYS_MAX);
-		goto err;
-	}
-
-	ret = read_core_sysfs_u32(f_max, &highest_perf);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_HIGHEST_PERF);
 		goto err;
 	}
 
-	ret = read_core_sysfs_u32(f_nom, &nominal_perf);
+	ret = power_sysfs_read_u32(&nominal_perf, POWER_SYSFILE_NOMINAL_PERF,
+			pi->lcore_id);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_NOMINAL_PERF);
 		goto err;
 	}
 
-	ret = read_core_sysfs_u32(f_cmax, &cpuinfo_max_freq);
+	ret = power_sysfs_read_u32(&cpuinfo_max_freq, POWER_SYSFILE_SYS_MAX,
+			pi->lcore_id);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_SYS_MAX);
@@ -175,13 +153,6 @@ power_check_turbo(struct cppc_power_info *pi)
 	}
 
 err:
-	if (f_max != NULL)
-		fclose(f_max);
-	if (f_nom != NULL)
-		fclose(f_nom);
-	if (f_cmax != NULL)
-		fclose(f_cmax);
-
 	return ret;
 }
 
@@ -192,35 +163,20 @@ power_check_turbo(struct cppc_power_info *pi)
 static int
 power_get_available_freqs(struct cppc_power_info *pi)
 {
-	FILE *f_min = NULL, *f_max = NULL;
-	int ret = -1;
 	uint32_t scaling_min_freq = 0, scaling_max_freq = 0, nominal_perf = 0;
 	uint32_t i, num_freqs = 0;
+	int ret = -1;
 
-	open_core_sysfs_file(&f_max, "r", POWER_SYSFILE_SCALING_MAX_FREQ,
-			pi->lcore_id);
-	if (f_max == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_SCALING_MAX_FREQ);
-		goto out;
-	}
-
-	open_core_sysfs_file(&f_min, "r", POWER_SYSFILE_SCALING_MIN_FREQ,
+	ret = power_sysfs_read_u32(&scaling_max_freq, POWER_SYSFILE_SCALING_MAX_FREQ,
 			pi->lcore_id);
-	if (f_min == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_SCALING_MIN_FREQ);
-		goto out;
-	}
-
-	ret = read_core_sysfs_u32(f_max, &scaling_max_freq);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_SCALING_MAX_FREQ);
 		goto out;
 	}
 
-	ret = read_core_sysfs_u32(f_min, &scaling_min_freq);
+	ret = power_sysfs_read_u32(&scaling_min_freq, POWER_SYSFILE_SCALING_MIN_FREQ,
+			pi->lcore_id);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_SCALING_MIN_FREQ);
@@ -260,11 +216,6 @@ power_get_available_freqs(struct cppc_power_info *pi)
 			num_freqs, pi->lcore_id);
 
 out:
-	if (f_min != NULL)
-		fclose(f_min);
-	if (f_max != NULL)
-		fclose(f_max);
-
 	return ret;
 }
 
diff --git a/drivers/power/intel_pstate/intel_pstate_cpufreq.c b/drivers/power/intel_pstate/intel_pstate_cpufreq.c
index dfbb5635a1..c8d0ed2c21 100644
--- a/drivers/power/intel_pstate/intel_pstate_cpufreq.c
+++ b/drivers/power/intel_pstate/intel_pstate_cpufreq.c
@@ -108,29 +108,12 @@ out:	close(fd);
 static int
 power_init_for_setting_freq(struct pstate_power_info *pi)
 {
-	FILE *f_base = NULL, *f_base_min = NULL, *f_base_max = NULL,
-	     *f_min = NULL, *f_max = NULL;
+	FILE *f_min = NULL, *f_max = NULL;
 	uint32_t base_ratio, base_min_ratio, base_max_ratio;
 	uint64_t max_non_turbo = 0;
 	int ret;
 
-	/* open all files we expect to have open */
-	open_core_sysfs_file(&f_base_max, "r", POWER_SYSFILE_BASE_MAX_FREQ,
-			pi->lcore_id);
-	if (f_base_max == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_BASE_MAX_FREQ);
-		goto err;
-	}
-
-	open_core_sysfs_file(&f_base_min, "r", POWER_SYSFILE_BASE_MIN_FREQ,
-			pi->lcore_id);
-	if (f_base_min == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_BASE_MIN_FREQ);
-		goto err;
-	}
-
+	/* open the files that are kept open for setting the frequency */
 	open_core_sysfs_file(&f_min, "rw+", POWER_SYSFILE_MIN_FREQ,
 			pi->lcore_id);
 	if (f_min == NULL) {
@@ -147,12 +130,9 @@ power_init_for_setting_freq(struct pstate_power_info *pi)
 		goto err;
 	}
 
-	open_core_sysfs_file(&f_base, "r", POWER_SYSFILE_BASE_FREQ,
-			pi->lcore_id);
-	/* base ratio file may not exist in some kernels, so no error check */
-
 	/* read base max ratio */
-	ret = read_core_sysfs_u32(f_base_max, &base_max_ratio);
+	ret = power_sysfs_read_u32(&base_max_ratio, POWER_SYSFILE_BASE_MAX_FREQ,
+			pi->lcore_id);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_BASE_MAX_FREQ);
@@ -160,24 +140,18 @@ power_init_for_setting_freq(struct pstate_power_info *pi)
 	}
 
 	/* read base min ratio */
-	ret = read_core_sysfs_u32(f_base_min, &base_min_ratio);
+	ret = power_sysfs_read_u32(&base_min_ratio, POWER_SYSFILE_BASE_MIN_FREQ,
+			pi->lcore_id);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_BASE_MIN_FREQ);
 		goto err;
 	}
 
-	/* base ratio may not exist */
-	if (f_base != NULL) {
-		ret = read_core_sysfs_u32(f_base, &base_ratio);
-		if (ret < 0) {
-			POWER_LOG(ERR, "Failed to read %s",
-					POWER_SYSFILE_BASE_FREQ);
-			goto err;
-		}
-	} else {
+	/* base ratio file may not exist in some kernels, so no error check */
+	if (power_sysfs_read_u32(&base_ratio, POWER_SYSFILE_BASE_FREQ,
+			pi->lcore_id) < 0)
 		base_ratio = 0;
-	}
 
 	/* convert ratios to bins */
 	base_max_ratio /= BUS_FREQ;
@@ -222,20 +196,10 @@ power_init_for_setting_freq(struct pstate_power_info *pi)
 	pi->core_base_freq = base_ratio * BUS_FREQ;
 
 out:
-	if (f_base != NULL)
-		fclose(f_base);
-	fclose(f_base_max);
-	fclose(f_base_min);
 	/* f_min and f_max are stored, no need to close */
 	return 0;
 
 err:
-	if (f_base != NULL)
-		fclose(f_base);
-	if (f_base_min != NULL)
-		fclose(f_base_min);
-	if (f_base_max != NULL)
-		fclose(f_base_max);
 	if (f_min != NULL)
 		fclose(f_min);
 	if (f_max != NULL)
@@ -366,38 +330,22 @@ power_set_governor_original(struct pstate_power_info *pi)
 static int
 power_get_available_freqs(struct pstate_power_info *pi)
 {
-	FILE *f_min = NULL, *f_max = NULL;
-	int ret = -1;
 	uint32_t sys_min_freq = 0, sys_max_freq = 0, base_max_freq = 0;
 	int config_min_freq, config_max_freq;
 	uint32_t i, num_freqs = 0;
-
-	/* open all files */
-	open_core_sysfs_file(&f_max, "r", POWER_SYSFILE_BASE_MAX_FREQ,
-			pi->lcore_id);
-	if (f_max == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_BASE_MAX_FREQ);
-		goto out;
-	}
-
-	open_core_sysfs_file(&f_min, "r", POWER_SYSFILE_BASE_MIN_FREQ,
-			pi->lcore_id);
-	if (f_min == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_BASE_MIN_FREQ);
-		goto out;
-	}
+	int ret = -1;
 
 	/* read base ratios */
-	ret = read_core_sysfs_u32(f_max, &sys_max_freq);
+	ret = power_sysfs_read_u32(&sys_max_freq, POWER_SYSFILE_BASE_MAX_FREQ,
+			pi->lcore_id);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_BASE_MAX_FREQ);
 		goto out;
 	}
 
-	ret = read_core_sysfs_u32(f_min, &sys_min_freq);
+	ret = power_sysfs_read_u32(&sys_min_freq, POWER_SYSFILE_BASE_MIN_FREQ,
+			pi->lcore_id);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_BASE_MIN_FREQ);
@@ -467,31 +415,18 @@ power_get_available_freqs(struct pstate_power_info *pi)
 			num_freqs, pi->lcore_id);
 
 out:
-	if (f_min != NULL)
-		fclose(f_min);
-	if (f_max != NULL)
-		fclose(f_max);
-
 	return ret;
 }
 
 static int
 power_get_cur_idx(struct pstate_power_info *pi)
 {
-	FILE *f_cur;
-	int ret = -1;
 	uint32_t sys_cur_freq = 0;
 	unsigned int i;
+	int ret = -1;
 
-	open_core_sysfs_file(&f_cur, "r", POWER_SYSFILE_CUR_FREQ,
+	ret = power_sysfs_read_u32(&sys_cur_freq, POWER_SYSFILE_CUR_FREQ,
 			pi->lcore_id);
-	if (f_cur == NULL) {
-		POWER_LOG(ERR, "failed to open %s",
-				POWER_SYSFILE_CUR_FREQ);
-		goto fail;
-	}
-
-	ret = read_core_sysfs_u32(f_cur, &sys_cur_freq);
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
 				POWER_SYSFILE_CUR_FREQ);
@@ -517,8 +452,6 @@ power_get_cur_idx(struct pstate_power_info *pi)
 
 	ret = 0;
 fail:
-	if (f_cur != NULL)
-		fclose(f_cur);
 	return ret;
 }
 
diff --git a/drivers/power/intel_uncore/intel_uncore.c b/drivers/power/intel_uncore/intel_uncore.c
index 6759ea1445..0e758312cf 100644
--- a/drivers/power/intel_uncore/intel_uncore.c
+++ b/drivers/power/intel_uncore/intel_uncore.c
@@ -111,20 +111,14 @@ set_uncore_freq_internal(struct uncore_power_info *ui, uint32_t idx)
 static int
 power_init_for_setting_uncore_freq(struct uncore_power_info *ui)
 {
-	FILE *f_base_min = NULL, *f_base_max = NULL, *f_min = NULL, *f_max = NULL;
+	FILE *f_min = NULL, *f_max = NULL;
 	uint32_t base_min_freq = 0, base_max_freq = 0, min_freq = 0, max_freq = 0;
 	int ret;
 
 	/* open and read all uncore sys files */
 	/* Base max */
-	open_core_sysfs_file(&f_base_max, "r", POWER_INTEL_UNCORE_SYSFILE_BASE_MAX_FREQ,
+	ret = power_sysfs_read_u32(&base_max_freq, POWER_INTEL_UNCORE_SYSFILE_BASE_MAX_FREQ,
 			ui->pkg, ui->die);
-	if (f_base_max == NULL) {
-		POWER_LOG(DEBUG, "failed to open %s",
-				POWER_INTEL_UNCORE_SYSFILE_BASE_MAX_FREQ);
-		goto err;
-	}
-	ret = read_core_sysfs_u32(f_base_max, &base_max_freq);
 	if (ret < 0) {
 		POWER_LOG(DEBUG, "Failed to read %s",
 				POWER_INTEL_UNCORE_SYSFILE_BASE_MAX_FREQ);
@@ -132,21 +126,13 @@ power_init_for_setting_uncore_freq(struct uncore_power_info *ui)
 	}
 
 	/* Base min */
-	open_core_sysfs_file(&f_base_min, "r", POWER_INTEL_UNCORE_SYSFILE_BASE_MIN_FREQ,
-		ui->pkg, ui->die);
-	if (f_base_min == NULL) {
-		POWER_LOG(DEBUG, "failed to open %s",
+	ret = power_sysfs_read_u32(&base_min_freq, POWER_INTEL_UNCORE_SYSFILE_BASE_MIN_FREQ,
+			ui->pkg, ui->die);
+	if (ret < 0) {
+		POWER_LOG(DEBUG, "Failed to read %s",
 				POWER_INTEL_UNCORE_SYSFILE_BASE_MIN_FREQ);
 		goto err;
 	}
-	if (f_base_min != NULL) {
-		ret = read_core_sysfs_u32(f_base_min, &base_min_freq);
-		if (ret < 0) {
-			POWER_LOG(DEBUG, "Failed to read %s",
-					POWER_INTEL_UNCORE_SYSFILE_BASE_MIN_FREQ);
-			goto err;
-		}
-	}
 
 	/* Curr min */
 	open_core_sysfs_file(&f_min, "rw+", POWER_INTEL_UNCORE_SYSFILE_MIN_FREQ,
@@ -191,17 +177,11 @@ power_init_for_setting_uncore_freq(struct uncore_power_info *ui)
 	ui->init_max_freq = base_max_freq;
 	ui->init_min_freq = base_min_freq;
 
-	fclose(f_base_min);
-	fclose(f_base_max);
 	/* f_min and f_max are stored, no need to close */
 
 	return 0;
 
 err:
-	if (f_base_min != NULL)
-		fclose(f_base_min);
-	if (f_base_max != NULL)
-		fclose(f_base_max);
 	if (f_min != NULL)
 		fclose(f_min);
 	if (f_max != NULL)
diff --git a/lib/power/power_common.c b/lib/power/power_common.c
index da22a4d160..f583975152 100644
--- a/lib/power/power_common.c
+++ b/lib/power/power_common.c
@@ -3,14 +3,17 @@
  */
 
 #include <limits.h>
+#include <stdint.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
+#include <stdarg.h>
 
 #include <eal_export.h>
 #include <rte_log.h>
 #include <rte_string_fns.h>
 #include <rte_lcore.h>
+#include <rte_sysfs.h>
 
 #include "power_common.h"
 
@@ -28,34 +31,16 @@ cpufreq_check_scaling_driver(const char *driver_name)
 {
 	unsigned int lcore_id = 0; /* always check core 0 */
 	char readbuf[PATH_MAX];
-	size_t end_idx;
-	char *s;
-	FILE *f;
 
 	/*
 	 * Check if scaling driver matches what we expect.
+	 * If there is no driver at all, or it can't be read,
+	 * consider the system unsupported.
 	 */
-	open_core_sysfs_file(&f, "r", POWER_SYSFILE_SCALING_DRIVER,
-			lcore_id);
-	/* if there's no driver at all, bail out */
-	if (f == NULL)
-		return 0;
-
-	s = fgets(readbuf, sizeof(readbuf), f);
-	/* don't need it any more */
-	fclose(f);
-
-	/* if we can't read it, consider unsupported */
-	if (s == NULL)
+	if (rte_sysfs_parse_string(readbuf, sizeof(readbuf),
+			POWER_SYSFILE_SCALING_DRIVER, lcore_id) < 0)
 		return 0;
 
-	/* when read from sysfs, driver name has an extra newline at the end */
-	end_idx = strnlen(readbuf, sizeof(readbuf));
-	if (end_idx > 0 && readbuf[end_idx - 1] == '\n') {
-		end_idx--;
-		readbuf[end_idx] = '\0';
-	}
-
 	/* does the driver name match? */
 	if (strncmp(readbuf, driver_name, sizeof(readbuf)) != 0)
 		return 0;
@@ -87,6 +72,29 @@ open_core_sysfs_file(FILE **f, const char *mode, const char *format, ...)
 	return 0;
 }
 
+RTE_EXPORT_INTERNAL_SYMBOL(power_sysfs_read_u32)
+int
+power_sysfs_read_u32(uint32_t *val, const char *format, ...)
+{
+	unsigned long tmp;
+	va_list ap;
+	int ret;
+
+	va_start(ap, format);
+	ret = rte_sysfs_vparse_uint(&tmp, format, ap);
+	va_end(ap);
+	if (ret < 0)
+		return -1;
+
+	if (tmp > UINT32_MAX) {
+		POWER_LOG(ERR, "sysfs value does not fit in 32 bits");
+		return -1;
+	}
+
+	*val = tmp;
+	return 0;
+}
+
 RTE_EXPORT_INTERNAL_SYMBOL(read_core_sysfs_u32)
 int
 read_core_sysfs_u32(FILE *f, uint32_t *val)
diff --git a/lib/power/power_common.h b/lib/power/power_common.h
index 370c5246c6..6ca91658cd 100644
--- a/lib/power/power_common.h
+++ b/lib/power/power_common.h
@@ -8,6 +8,7 @@
 #include <rte_common.h>
 #include <rte_compat.h>
 #include <rte_log.h>
+#include <stdint.h>
 
 #define RTE_POWER_INVALID_FREQ_INDEX (~0)
 
@@ -48,6 +49,11 @@ int open_core_sysfs_file(FILE **f, const char *mode, const char *format, ...)
 __rte_internal
 int read_core_sysfs_u32(FILE *f, uint32_t *val);
 
+/* read a 32 bit value from a sysfs file given by a printf style path */
+__rte_internal
+int power_sysfs_read_u32(uint32_t *val, const char *format, ...)
+		__rte_format_printf(2, 3);
+
 __rte_internal
 int read_core_sysfs_s(FILE *f, char *buf, unsigned int len);
 
diff --git a/lib/power/rte_power_qos.c b/lib/power/rte_power_qos.c
index d8d8d36a76..dd1b8f7384 100644
--- a/lib/power/rte_power_qos.c
+++ b/lib/power/rte_power_qos.c
@@ -9,6 +9,7 @@
 #include <eal_export.h>
 #include <rte_lcore.h>
 #include <rte_log.h>
+#include <rte_sysfs.h>
 
 #include "power_common.h"
 #include "rte_power_qos.h"
@@ -24,7 +25,6 @@ rte_power_qos_set_cpu_resume_latency(uint16_t lcore_id, int latency)
 {
 	char buf[PM_QOS_CPU_RESUME_LATENCY_BUF_LEN];
 	uint32_t cpu_id;
-	FILE *f;
 	int ret;
 
 	RTE_POWER_VALID_LCOREID_OR_ERR_RET(lcore_id, -EINVAL);
@@ -37,13 +37,6 @@ rte_power_qos_set_cpu_resume_latency(uint16_t lcore_id, int latency)
 		return -EINVAL;
 	}
 
-	ret = open_core_sysfs_file(&f, "w", PM_QOS_SYSFILE_RESUME_LATENCY_US, cpu_id);
-	if (ret != 0) {
-		POWER_LOG(ERR, "Failed to open "PM_QOS_SYSFILE_RESUME_LATENCY_US" : %s",
-			  cpu_id, strerror(errno));
-		return ret;
-	}
-
 	/*
 	 * Based on the sysfs interface pm_qos_resume_latency_us under
 	 * @PM_QOS_SYSFILE_RESUME_LATENCY_US directory in kernel, their meaning
@@ -59,13 +52,11 @@ rte_power_qos_set_cpu_resume_latency(uint16_t lcore_id, int latency)
 	else
 		snprintf(buf, sizeof(buf), "%u", latency);
 
-	ret = write_core_sysfs_s(f, buf);
+	ret = rte_sysfs_write_string(buf, PM_QOS_SYSFILE_RESUME_LATENCY_US, cpu_id);
 	if (ret != 0)
 		POWER_LOG(ERR, "Failed to write "PM_QOS_SYSFILE_RESUME_LATENCY_US" : %s",
 			  cpu_id, strerror(errno));
 
-	fclose(f);
-
 	return ret;
 }
 
@@ -76,7 +67,6 @@ rte_power_qos_get_cpu_resume_latency(uint16_t lcore_id)
 	char buf[PM_QOS_CPU_RESUME_LATENCY_BUF_LEN];
 	int latency = -1;
 	uint32_t cpu_id;
-	FILE *f;
 	int ret;
 
 	RTE_POWER_VALID_LCOREID_OR_ERR_RET(lcore_id, -EINVAL);
@@ -84,18 +74,12 @@ rte_power_qos_get_cpu_resume_latency(uint16_t lcore_id)
 	if (ret != 0)
 		return ret;
 
-	ret = open_core_sysfs_file(&f, "r", PM_QOS_SYSFILE_RESUME_LATENCY_US, cpu_id);
-	if (ret != 0) {
-		POWER_LOG(ERR, "Failed to open "PM_QOS_SYSFILE_RESUME_LATENCY_US" : %s",
-			  cpu_id, strerror(errno));
-		return ret;
-	}
-
-	ret = read_core_sysfs_s(f, buf, sizeof(buf));
+	ret = rte_sysfs_parse_string(buf, sizeof(buf), PM_QOS_SYSFILE_RESUME_LATENCY_US,
+			cpu_id);
 	if (ret != 0) {
 		POWER_LOG(ERR, "Failed to read "PM_QOS_SYSFILE_RESUME_LATENCY_US" : %s",
 			  cpu_id, strerror(errno));
-		goto out;
+		return ret;
 	}
 
 	/*
@@ -113,8 +97,5 @@ rte_power_qos_get_cpu_resume_latency(uint16_t lcore_id)
 		latency = latency == 0 ? RTE_POWER_QOS_RESUME_LATENCY_NO_CONSTRAINT : latency;
 	}
 
-out:
-	fclose(f);
-
 	return latency != -1 ? latency : ret;
 }
-- 
2.53.0

[PATCH v2 6/9] drivers/bus: remove duplicate sysfs string helpers

[Stephen Hemminger]

get_sysfs_string() in the vmbus bus and read_sysfs_string() in the
platform bus are both open coded copies of what the EAL routine now
provides. Each had a single caller.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/bus/platform/platform.c     | 33 +++---------------------
 drivers/bus/vmbus/linux/vmbus_bus.c | 39 +++--------------------------
 2 files changed, 8 insertions(+), 64 deletions(-)

diff --git a/drivers/bus/platform/platform.c b/drivers/bus/platform/platform.c
index 9585fb79e9..c511893aab 100644
--- a/drivers/bus/platform/platform.c
+++ b/drivers/bus/platform/platform.c
@@ -187,40 +187,15 @@ device_unmap_resources(struct rte_platform_device *pdev)
 	pdev->num_resource = 0;
 }
 
-static int
-read_sysfs_string(const char *path, char *buf, size_t size)
-{
-	FILE *f;
-	char *p;
-
-	f = fopen(path, "r");
-	if (f == NULL)
-		return -errno;
-
-	if (fgets(buf, size, f) == NULL) {
-		fclose(f);
-		return -ENODATA;
-	}
-
-	fclose(f);
-
-	p = strrchr(buf, '\n');
-	if (p != NULL)
-		*p = '\0';
-
-	return 0;
-}
-
 static char *
 of_resource_name(const char *dev_name, int index)
 {
-	char path[PATH_MAX], buf[BUFSIZ] = { };
-	int num = 0, ret;
+	char buf[BUFSIZ] = { };
+	int num = 0;
 	char *name;
 
-	snprintf(path, sizeof(path), PLATFORM_BUS_DEVICES_PATH "/%s/of_node/reg-names", dev_name);
-	ret = read_sysfs_string(path, buf, sizeof(buf) - 1);
-	if (ret)
+	if (rte_sysfs_parse_string(buf, sizeof(buf) - 1,
+			PLATFORM_BUS_DEVICES_PATH "/%s/of_node/reg-names", dev_name) < 0)
 		return NULL;
 
 	for (name = buf; *name != 0; name += strlen(name) + 1) {
diff --git a/drivers/bus/vmbus/linux/vmbus_bus.c b/drivers/bus/vmbus/linux/vmbus_bus.c
index 9ee7983eb6..00f369120f 100644
--- a/drivers/bus/vmbus/linux/vmbus_bus.c
+++ b/drivers/bus/vmbus/linux/vmbus_bus.c
@@ -84,35 +84,6 @@ parse_sysfs_uuid(const char *filename, rte_uuid_t uu)
 	return 0;
 }
 
-static int
-get_sysfs_string(const char *filename, char *buf, size_t buflen)
-{
-	char *cp;
-	FILE *f;
-
-	f = fopen(filename, "r");
-	if (f == NULL) {
-		VMBUS_LOG(ERR, "cannot open sysfs value %s:%s",
-			  filename, strerror(errno));
-		return -1;
-	}
-
-	if (fgets(buf, buflen, f) == NULL) {
-		VMBUS_LOG(ERR, "cannot read sysfs value %s",
-				filename);
-		fclose(f);
-		return -1;
-	}
-	fclose(f);
-
-	/* remove trailing newline */
-	cp = memchr(buf, '\n', buflen);
-	if (cp)
-		*cp = '\0';
-
-	return 0;
-}
-
 static int
 vmbus_get_uio_dev(const struct rte_vmbus_device *dev,
 		  char *dstbuf, size_t buflen)
@@ -169,7 +140,7 @@ RTE_EXPORT_SYMBOL(rte_vmbus_map_device)
 int
 rte_vmbus_map_device(struct rte_vmbus_device *dev)
 {
-	char uioname[PATH_MAX], filename[PATH_MAX];
+	char uioname[PATH_MAX];
 	char dirname[PATH_MAX], mapname[64];
 	int i;
 
@@ -188,11 +159,9 @@ rte_vmbus_map_device(struct rte_vmbus_device *dev)
 		snprintf(dirname, sizeof(dirname),
 			 "%s/maps/map%d", uioname, i);
 
-		snprintf(filename, sizeof(filename),
-			 "%s/name", dirname);
-
-		if (get_sysfs_string(filename, mapname, sizeof(mapname)) < 0) {
-			VMBUS_LOG(ERR, "could not read %s", filename);
+		if (rte_sysfs_parse_string(mapname, sizeof(mapname),
+				"%s/name", dirname) < 0) {
+			VMBUS_LOG(ERR, "could not read name of %s", dirname);
 			return -1;
 		}
 
-- 
2.53.0

[PATCH v2 7/9] common/mlx5: use common sysfs routines

[Stephen Hemminger]

mlx5_sys_roce_disable() read the roce_enable attribute and then
reopened the same file to write it. Use the EAL read and write
routines instead.

The dev_port lookup is left alone: it selects the scanf format at
runtime and distinguishes ENOENT from other errors to fall back to
dev_id on older kernels.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/common/mlx5/linux/mlx5_common_os.c | 39 ++++++++--------------
 1 file changed, 14 insertions(+), 25 deletions(-)

diff --git a/drivers/common/mlx5/linux/mlx5_common_os.c b/drivers/common/mlx5/linux/mlx5_common_os.c
index 3e9cd86062..cf496b0b58 100644
--- a/drivers/common/mlx5/linux/mlx5_common_os.c
+++ b/drivers/common/mlx5/linux/mlx5_common_os.c
@@ -16,6 +16,7 @@
 #include <eal_export.h>
 #include <rte_errno.h>
 #include <rte_string_fns.h>
+#include <rte_sysfs.h>
 #include <bus_pci_driver.h>
 #include <bus_auxiliary_driver.h>
 
@@ -658,45 +659,33 @@ mlx5_nl_roce_disable(const char *addr)
 	return ret;
 }
 
+#define MLX5_ROCE_ENABLE_PATH "/sys/bus/pci/devices/%s/roce_enable"
+
 /* Try to disable ROCE by sysfs. */
 static int
 mlx5_sys_roce_disable(const char *addr)
 {
-	FILE *file_o;
-	int enable;
+	unsigned long enable;
 	int ret;
 
-	MKSTR(file_p, "/sys/bus/pci/devices/%s/roce_enable", addr);
-	file_o = fopen(file_p, "rb");
-	if (!file_o) {
+	if (rte_sysfs_parse_uint(&enable, MLX5_ROCE_ENABLE_PATH, addr) != 0) {
 		rte_errno = ENOTSUP;
 		return -ENOTSUP;
 	}
-	ret = fscanf(file_o, "%d", &enable);
-	if (ret != 1) {
-		rte_errno = EINVAL;
-		ret = EINVAL;
-		goto close;
-	} else if (!enable) {
-		ret = 0;
+	if (enable == 0) {
 		DRV_LOG(INFO, "ROCE has already disabled(sysfs).");
-		goto close;
+		return 0;
 	}
-	fclose(file_o);
-	file_o = fopen(file_p, "wb");
-	if (!file_o) {
+
+	ret = rte_sysfs_write_string("0\n", MLX5_ROCE_ENABLE_PATH, addr);
+	if (ret != 0) {
 		rte_errno = ENOTSUP;
+		DRV_LOG(DEBUG, "Failed to disable ROCE by sysfs.");
 		return -ENOTSUP;
 	}
-	fprintf(file_o, "0\n");
-	ret = 0;
-close:
-	if (ret)
-		DRV_LOG(DEBUG, "Failed to disable ROCE by sysfs: %d.", ret);
-	else
-		DRV_LOG(INFO, "ROCE is disabled by sysfs successfully.");
-	fclose(file_o);
-	return ret;
+
+	DRV_LOG(INFO, "ROCE is disabled by sysfs successfully.");
+	return 0;
 }
 
 static int
-- 
2.53.0

[PATCH v2 8/9] net/mlx5: use common sysfs routines

[Stephen Hemminger]

Read the bonding interface indexes with the EAL routine rather than
opening each file and parsing it with fscanf().

The index was read with fscanf("%u") and is now converted with base
0, so a value with a leading zero would parse as octal. The kernel
does not print ifindex with leading zeros.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/mlx5/linux/mlx5_ethdev_os.c | 17 +++++------------
 drivers/net/mlx5/linux/mlx5_os.c        | 13 +++++--------
 2 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/net/mlx5/linux/mlx5_ethdev_os.c b/drivers/net/mlx5/linux/mlx5_ethdev_os.c
index 4bbc590e91..d1c03d66af 100644
--- a/drivers/net/mlx5/linux/mlx5_ethdev_os.c
+++ b/drivers/net/mlx5/linux/mlx5_ethdev_os.c
@@ -36,6 +36,7 @@
 #include <rte_string_fns.h>
 #include <rte_rwlock.h>
 #include <rte_cycles.h>
+#include <rte_sysfs.h>
 
 #include <mlx5_glue.h>
 #include <mlx5_devx_cmds.h>
@@ -1192,27 +1193,19 @@ mlx5_sysfs_bond_info(unsigned int pf_ifindex, unsigned int *ifindex,
 		     char *ifname)
 {
 	char name[IF_NAMESIZE];
-	FILE *file;
 	unsigned int index;
-	int ret;
+	unsigned long val;
 
 	if (!if_indextoname(pf_ifindex, name) || !strlen(name)) {
 		rte_errno = errno;
 		return -rte_errno;
 	}
-	MKSTR(bond_if, "/sys/class/net/%s/master/ifindex", name);
 	/* read bond ifindex */
-	file = fopen(bond_if, "rb");
-	if (file == NULL) {
-		rte_errno = errno;
-		return -rte_errno;
-	}
-	ret = fscanf(file, "%u", &index);
-	fclose(file);
-	if (ret <= 0) {
-		rte_errno = errno;
+	if (rte_sysfs_parse_uint(&val, "/sys/class/net/%s/master/ifindex", name) != 0) {
+		rte_errno = ENODEV;
 		return -rte_errno;
 	}
+	index = val;
 	if (ifindex)
 		*ifindex = index;
 
diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index adc5878296..203e9339bb 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -29,6 +29,7 @@
 #include <rte_string_fns.h>
 #include <rte_alarm.h>
 #include <rte_eal_paging.h>
+#include <rte_sysfs.h>
 
 #include <mlx5_glue.h>
 #include <mlx5_devx_cmds.h>
@@ -2103,6 +2104,7 @@ mlx5_device_bond_pci_match(const struct ibv_device *ibdev,
 		char tmp_str[IF_NAMESIZE + 32];
 		struct rte_pci_addr pci_addr;
 		struct mlx5_switch_info	info;
+		unsigned long val;
 		int ret;
 
 		/* Process slave interface names in the loop. */
@@ -2140,15 +2142,10 @@ mlx5_device_bond_pci_match(const struct ibv_device *ibdev,
 			break;
 		}
 		/* Get ifindex. */
-		snprintf(tmp_str, sizeof(tmp_str),
-			 "/sys/class/net/%s/ifindex", ifname);
-		file = fopen(tmp_str, "rb");
-		if (!file)
-			break;
-		ret = fscanf(file, "%u", &ifindex);
-		fclose(file);
-		if (ret != 1)
+		if (rte_sysfs_parse_uint(&val, "/sys/class/net/%s/ifindex",
+				ifname) != 0)
 			break;
+		ifindex = val;
 		/* Save bonding info. */
 		snprintf(bond_info->ports[info.port_name].ifname,
 			 sizeof(bond_info->ports[0].ifname), "%s", ifname);
-- 
2.53.0

[PATCH v2 9/9] net/mana: use common sysfs routines

[Stephen Hemminger]

Read dev_port with the EAL routine rather than opening the file and
parsing it with fscanf().

The value was read with fscanf("%d") and is now converted with base
0, so a value with a leading zero would parse as octal. The kernel
does not print dev_port with leading zeros.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/mana/mana.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/net/mana/mana.c b/drivers/net/mana/mana.c
index 0b72f711a1..39bfb5b4b7 100644
--- a/drivers/net/mana/mana.c
+++ b/drivers/net/mana/mana.c
@@ -14,6 +14,7 @@
 #include <rte_kvargs.h>
 #include <rte_eal_paging.h>
 #include <rte_pci.h>
+#include <rte_sysfs.h>
 
 #include <infiniband/verbs.h>
 #include <infiniband/manadv.h>
@@ -1282,6 +1283,7 @@ get_port_mac(struct ibv_device *device, unsigned int port,
 	DIR *dir;
 	struct dirent *dent;
 	unsigned int dev_port;
+	unsigned long val;
 
 	MANA_MKSTR(path, "%s/device/net", device->ibdev_path);
 
@@ -1293,23 +1295,15 @@ get_port_mac(struct ibv_device *device, unsigned int port,
 		char *name = dent->d_name;
 		char *mac = NULL;
 
-		MANA_MKSTR(port_path, "%s/%s/dev_port", path, name);
-
 		/* Ignore . and .. */
 		if ((name[0] == '.') &&
 		    ((name[1] == '\0') ||
 		     ((name[1] == '.') && (name[2] == '\0'))))
 			continue;
 
-		file = fopen(port_path, "r");
-		if (!file)
-			continue;
-
-		ret = fscanf(file, "%u", &dev_port);
-		fclose(file);
-
-		if (ret != 1)
+		if (rte_sysfs_parse_uint(&val, "%s/%s/dev_port", path, name) != 0)
 			continue;
+		dev_port = val;
 
 		/* Ethernet ports start at 0, IB port start at 1 */
 		if (dev_port == port - 1) {
-- 
2.53.0