[RFC] eal: support systemd service convention for runtime directory

[RFC] eal: support systemd service convention for runtime directory

[Stephen Hemminger]

Systemd.exec supports configuring the runtime directory of a service
via RuntimeDirectory=. This creates the directory with the necessary
permissions which actual service may not have if running in container.

The change to DPDK is to look for the environment RUNTIME_DIRECTORY
first and use that in preference to the fallback alternatives.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/linux/eal.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 60b49248388e..e729c713b393 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -86,25 +86,26 @@ struct lcore_config lcore_config[RTE_MAX_LCORE];
 /* used by rte_rdtsc() */
 int rte_cycles_vmware_tsc_map;
 
-static const char *default_runtime_dir = "/var/run";
-
 int
 eal_create_runtime_dir(void)
 {
-	const char *directory = default_runtime_dir;
-	const char *xdg_runtime_dir = getenv("XDG_RUNTIME_DIR");
-	const char *fallback = "/tmp";
+	const char *directory;
 	char run_dir[PATH_MAX];
 	char tmp[PATH_MAX];
 	int ret;
 
-	if (getuid() != 0) {
-		/* try XDG path first, fall back to /tmp */
-		if (xdg_runtime_dir != NULL)
-			directory = xdg_runtime_dir;
-		else
-			directory = fallback;
+	/* from RuntimeDirectory= see systemd.exec */
+	directory = getenv("RUNTIME_DIRECTORY");
+	if (directory == NULL) {
+		if (getuid() == 0)
+			directory = "/var/run";
+		else {
+			directory = getenv("XDG_RUNTIME_DIR");
+			if (directory == NULL)
+				directory = "/tmp";
+		}
 	}
+
 	/* create DPDK subdirectory under runtime dir */
 	ret = snprintf(tmp, sizeof(tmp), "%s/dpdk", directory);
 	if (ret < 0 || ret == sizeof(tmp)) {
-- 
2.30.2

RE: [RFC] eal: support systemd service convention for runtime directory

[Morten Brørup]

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Friday, 24 December 2021 00.39
> 
> Systemd.exec supports configuring the runtime directory of a service
> via RuntimeDirectory=. This creates the directory with the necessary
> permissions which actual service may not have if running in container.
> 
> The change to DPDK is to look for the environment RUNTIME_DIRECTORY
> first and use that in preference to the fallback alternatives.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/eal/linux/eal.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
> index 60b49248388e..e729c713b393 100644
> --- a/lib/eal/linux/eal.c
> +++ b/lib/eal/linux/eal.c
> @@ -86,25 +86,26 @@ struct lcore_config lcore_config[RTE_MAX_LCORE];
>  /* used by rte_rdtsc() */
>  int rte_cycles_vmware_tsc_map;
> 
> -static const char *default_runtime_dir = "/var/run";
> -
>  int
>  eal_create_runtime_dir(void)
>  {
> -	const char *directory = default_runtime_dir;
> -	const char *xdg_runtime_dir = getenv("XDG_RUNTIME_DIR");
> -	const char *fallback = "/tmp";
> +	const char *directory;
>  	char run_dir[PATH_MAX];
>  	char tmp[PATH_MAX];
>  	int ret;
> 
> -	if (getuid() != 0) {
> -		/* try XDG path first, fall back to /tmp */
> -		if (xdg_runtime_dir != NULL)
> -			directory = xdg_runtime_dir;
> -		else
> -			directory = fallback;
> +	/* from RuntimeDirectory= see systemd.exec */
> +	directory = getenv("RUNTIME_DIRECTORY");
> +	if (directory == NULL) {
> +		if (getuid() == 0)
> +			directory = "/var/run";
> +		else {
> +			directory = getenv("XDG_RUNTIME_DIR");
> +			if (directory == NULL)
> +				directory = "/tmp";
> +		}
>  	}
> +
>  	/* create DPDK subdirectory under runtime dir */
>  	ret = snprintf(tmp, sizeof(tmp), "%s/dpdk", directory);
>  	if (ret < 0 || ret == sizeof(tmp)) {
> --
> 2.30.2
> 

Reviewed-by: Morten Brørup <mb@smartsharesystems.com>

Also, while reviewing this, I stumbled across eal_set_runtime_dir() in eal_common_config.c, which I think should be fixed:

int
eal_set_runtime_dir(char *run_dir, size_t size)
{
	size_t str_size;

-	str_size = strlcpy(runtime_dir, run_dir, size);
-	if (str_size >= size) {
+	str_size = strlcpy(runtime_dir, run_dir, sizeof(runtime_dir));
+	if (str_size >= sizeof(runtime_dir)) {
		RTE_LOG(ERR, EAL, "Runtime directory string too long\n");
		return -1;
	}

	return 0;
}

And I don't understand why size is passed as a parameter to eal_set_runtime_dir().

[RFC] eal: remove size for eal_set_runtime_dir

[Stephen Hemminger]

The size argument to eal_set_runtime_dir is useless and was
being used incorrectly in strlcpy. It worked only because
all callers passed PATH_MAX which is same as sizeof the destination
runtime_dir.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Suggested-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/eal/common/eal_common_config.c | 7 ++-----
 lib/eal/common/eal_private.h       | 4 +---
 lib/eal/freebsd/eal.c              | 2 +-
 lib/eal/linux/eal.c                | 2 +-
 4 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/lib/eal/common/eal_common_config.c b/lib/eal/common/eal_common_config.c
index 1c4c4dd585d0..62a9d7a198db 100644
--- a/lib/eal/common/eal_common_config.c
+++ b/lib/eal/common/eal_common_config.c
@@ -29,12 +29,9 @@ rte_eal_get_runtime_dir(void)
 }
 
 int
-eal_set_runtime_dir(char *run_dir, size_t size)
+eal_set_runtime_dir(const char *run_dir)
 {
-	size_t str_size;
-
-	str_size = strlcpy(runtime_dir, run_dir, size);
-	if (str_size >= size) {
+	if (strlcpy(runtime_dir, run_dir, PATH_MAX) >= PATH_MAX) {
 		RTE_LOG(ERR, EAL, "Runtime directory string too long\n");
 		return -1;
 	}
diff --git a/lib/eal/common/eal_private.h b/lib/eal/common/eal_private.h
index 36bcc0b5a492..734f1f334b69 100644
--- a/lib/eal/common/eal_private.h
+++ b/lib/eal/common/eal_private.h
@@ -681,13 +681,11 @@ eal_mem_set_dump(void *virt, size_t size, bool dump);
  *
  * @param run_dir
  *   The new runtime directory path of DPDK
- * @param size
- *   The size of the new runtime directory path in bytes.
  * @return
  *   0 on success, (-1) on failure.
  */
 int
-eal_set_runtime_dir(char *run_dir, size_t size);
+eal_set_runtime_dir(const char *run_dir);
 
 /**
  * Get the internal configuration structure.
diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index a1cd2462db1b..503e276dc27f 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -123,7 +123,7 @@ eal_create_runtime_dir(void)
 		return -1;
 	}
 
-	if (eal_set_runtime_dir(run_dir, sizeof(run_dir)))
+	if (eal_set_runtime_dir(run_dir))
 		return -1;
 
 	return 0;
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 80ffe1c7f9d8..a73ff75f65c9 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -135,7 +135,7 @@ eal_create_runtime_dir(void)
 		return -1;
 	}
 
-	if (eal_set_runtime_dir(run_dir, sizeof(run_dir)))
+	if (eal_set_runtime_dir(run_dir))
 		return -1;
 
 	return 0;
-- 
2.30.2

RE: [RFC] eal: remove size for eal_set_runtime_dir

[Morten Brørup]

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Wednesday, 5 January 2022 19.02
> 
> The size argument to eal_set_runtime_dir is useless and was
> being used incorrectly in strlcpy. It worked only because
> all callers passed PATH_MAX which is same as sizeof the destination
> runtime_dir.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Suggested-by: Morten Brørup <mb@smartsharesystems.com>
> ---
>  lib/eal/common/eal_common_config.c | 7 ++-----
>  lib/eal/common/eal_private.h       | 4 +---
>  lib/eal/freebsd/eal.c              | 2 +-
>  lib/eal/linux/eal.c                | 2 +-
>  4 files changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/eal/common/eal_common_config.c
> b/lib/eal/common/eal_common_config.c
> index 1c4c4dd585d0..62a9d7a198db 100644
> --- a/lib/eal/common/eal_common_config.c
> +++ b/lib/eal/common/eal_common_config.c
> @@ -29,12 +29,9 @@ rte_eal_get_runtime_dir(void)
>  }
> 
>  int
> -eal_set_runtime_dir(char *run_dir, size_t size)
> +eal_set_runtime_dir(const char *run_dir)
>  {
> -	size_t str_size;
> -
> -	str_size = strlcpy(runtime_dir, run_dir, size);
> -	if (str_size >= size) {
> +	if (strlcpy(runtime_dir, run_dir, PATH_MAX) >= PATH_MAX) {
>  		RTE_LOG(ERR, EAL, "Runtime directory string too long\n");
>  		return -1;
>  	}
> diff --git a/lib/eal/common/eal_private.h
> b/lib/eal/common/eal_private.h
> index 36bcc0b5a492..734f1f334b69 100644
> --- a/lib/eal/common/eal_private.h
> +++ b/lib/eal/common/eal_private.h
> @@ -681,13 +681,11 @@ eal_mem_set_dump(void *virt, size_t size, bool
> dump);
>   *
>   * @param run_dir
>   *   The new runtime directory path of DPDK
> - * @param size
> - *   The size of the new runtime directory path in bytes.
>   * @return
>   *   0 on success, (-1) on failure.
>   */
>  int
> -eal_set_runtime_dir(char *run_dir, size_t size);
> +eal_set_runtime_dir(const char *run_dir);
> 
>  /**
>   * Get the internal configuration structure.
> diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
> index a1cd2462db1b..503e276dc27f 100644
> --- a/lib/eal/freebsd/eal.c
> +++ b/lib/eal/freebsd/eal.c
> @@ -123,7 +123,7 @@ eal_create_runtime_dir(void)
>  		return -1;
>  	}
> 
> -	if (eal_set_runtime_dir(run_dir, sizeof(run_dir)))
> +	if (eal_set_runtime_dir(run_dir))
>  		return -1;
> 
>  	return 0;
> diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
> index 80ffe1c7f9d8..a73ff75f65c9 100644
> --- a/lib/eal/linux/eal.c
> +++ b/lib/eal/linux/eal.c
> @@ -135,7 +135,7 @@ eal_create_runtime_dir(void)
>  		return -1;
>  	}
> 
> -	if (eal_set_runtime_dir(run_dir, sizeof(run_dir)))
> +	if (eal_set_runtime_dir(run_dir))
>  		return -1;
> 
>  	return 0;
> --
> 2.30.2
> 

Reviewed-by: Morten Brørup<mb@smartsharesystems.com>

Re: [RFC] eal: support systemd service convention for runtime directory

[Bruce Richardson]

On Thu, Dec 23, 2021 at 03:39:07PM -0800, Stephen Hemminger wrote:
> Systemd.exec supports configuring the runtime directory of a service
> via RuntimeDirectory=. This creates the directory with the necessary
> permissions which actual service may not have if running in container.
> 
> The change to DPDK is to look for the environment RUNTIME_DIRECTORY
> first and use that in preference to the fallback alternatives.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/eal/linux/eal.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
Seems reasonable. The path lookup for telemetry in the dpdk-telemetry.py
script will need updating to match this.

Otherwise:

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [RFC] eal: support systemd service convention for runtime directory

[Thomas Monjalon]

07/01/2022 13:07, Bruce Richardson:
> On Thu, Dec 23, 2021 at 03:39:07PM -0800, Stephen Hemminger wrote:
> > Systemd.exec supports configuring the runtime directory of a service
> > via RuntimeDirectory=. This creates the directory with the necessary
> > permissions which actual service may not have if running in container.
> > 
> > The change to DPDK is to look for the environment RUNTIME_DIRECTORY
> > first and use that in preference to the fallback alternatives.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> >  lib/eal/linux/eal.c | 23 ++++++++++++-----------
> >  1 file changed, 12 insertions(+), 11 deletions(-)
> > 
> Seems reasonable. The path lookup for telemetry in the dpdk-telemetry.py
> script will need updating to match this.

Any followup?
Do you plan a v2 to address comments from Morten and Bruce?

Re: [RFC] eal: support systemd service convention for runtime directory

[Stephen Hemminger]

On Wed, 02 Feb 2022 22:03:44 +0100
Thomas Monjalon <thomas@monjalon.net> wrote:

> 07/01/2022 13:07, Bruce Richardson:
> > On Thu, Dec 23, 2021 at 03:39:07PM -0800, Stephen Hemminger wrote:  
> > > Systemd.exec supports configuring the runtime directory of a service
> > > via RuntimeDirectory=. This creates the directory with the necessary
> > > permissions which actual service may not have if running in container.
> > > 
> > > The change to DPDK is to look for the environment RUNTIME_DIRECTORY
> > > first and use that in preference to the fallback alternatives.
> > > 
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > ---
> > >  lib/eal/linux/eal.c | 23 ++++++++++++-----------
> > >  1 file changed, 12 insertions(+), 11 deletions(-)
> > >   
> > Seems reasonable. The path lookup for telemetry in the dpdk-telemetry.py
> > script will need updating to match this.  
> 
> Any followup?
> Do you plan a v2 to address comments from Morten and Bruce?

Was waiting for any more responses, not urgent

[PATCH 0/2] better support of configuring runtime directory

[Stephen Hemminger]

Found this while exploring running a DPDK service with systemd
container environment. It helps if the place DPDK puts its runtime
directory is configurable.

This version supersedes earlier RFC and includes the feedback.

Stephen Hemminger (2):
  eal: remove size for eal_set_runtime_dir
  eal: support systemd service convention for runtime directory

 lib/eal/common/eal_common_config.c |  7 ++-----
 lib/eal/common/eal_private.h       |  4 +---
 lib/eal/freebsd/eal.c              |  2 +-
 lib/eal/linux/eal.c                | 25 ++++++++++++++-----------
 usertools/dpdk-telemetry.py        |  9 +++++++--
 5 files changed, 25 insertions(+), 22 deletions(-)

-- 
2.34.1

[PATCH 1/2] eal: remove size for eal_set_runtime_dir

[Stephen Hemminger]

The size argument to eal_set_runtime_dir is useless and was
being used incorrectly in strlcpy. It worked only because
all callers passed PATH_MAX which is same as sizeof the destination
runtime_dir.

Note: this is an internal API so no user exposed change.

Fixes: 57a2efb30477 ("eal: move OS common config objects")
Cc: talshn@mellanox.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Reviewed-by: Morten Brørup<mb@smartsharesystems.com>
---
 lib/eal/common/eal_common_config.c | 7 ++-----
 lib/eal/common/eal_private.h       | 4 +---
 lib/eal/freebsd/eal.c              | 2 +-
 lib/eal/linux/eal.c                | 2 +-
 4 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/lib/eal/common/eal_common_config.c b/lib/eal/common/eal_common_config.c
index 1c4c4dd585d0..62a9d7a198db 100644
--- a/lib/eal/common/eal_common_config.c
+++ b/lib/eal/common/eal_common_config.c
@@ -29,12 +29,9 @@ rte_eal_get_runtime_dir(void)
 }
 
 int
-eal_set_runtime_dir(char *run_dir, size_t size)
+eal_set_runtime_dir(const char *run_dir)
 {
-	size_t str_size;
-
-	str_size = strlcpy(runtime_dir, run_dir, size);
-	if (str_size >= size) {
+	if (strlcpy(runtime_dir, run_dir, PATH_MAX) >= PATH_MAX) {
 		RTE_LOG(ERR, EAL, "Runtime directory string too long\n");
 		return -1;
 	}
diff --git a/lib/eal/common/eal_private.h b/lib/eal/common/eal_private.h
index 36bcc0b5a492..734f1f334b69 100644
--- a/lib/eal/common/eal_private.h
+++ b/lib/eal/common/eal_private.h
@@ -681,13 +681,11 @@ eal_mem_set_dump(void *virt, size_t size, bool dump);
  *
  * @param run_dir
  *   The new runtime directory path of DPDK
- * @param size
- *   The size of the new runtime directory path in bytes.
  * @return
  *   0 on success, (-1) on failure.
  */
 int
-eal_set_runtime_dir(char *run_dir, size_t size);
+eal_set_runtime_dir(const char *run_dir);
 
 /**
  * Get the internal configuration structure.
diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index a1cd2462db1b..503e276dc27f 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -123,7 +123,7 @@ eal_create_runtime_dir(void)
 		return -1;
 	}
 
-	if (eal_set_runtime_dir(run_dir, sizeof(run_dir)))
+	if (eal_set_runtime_dir(run_dir))
 		return -1;
 
 	return 0;
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 60b49248388e..f2551c64b10c 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -137,7 +137,7 @@ eal_create_runtime_dir(void)
 		return -1;
 	}
 
-	if (eal_set_runtime_dir(run_dir, sizeof(run_dir)))
+	if (eal_set_runtime_dir(run_dir))
 		return -1;
 
 	return 0;
-- 
2.34.1

[PATCH 2/2] eal: support systemd service convention for runtime directory

[Stephen Hemminger]

Systemd.exec supports configuring the runtime directory of a service
via RuntimeDirectory=. This creates the directory with the necessary
permissions which actual service may not have if running in container.

The change to DPDK is to look for the environment RUNTIME_DIRECTORY
first and use that in preference to the fallback alternatives.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Reviewed-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/eal/linux/eal.c         | 23 +++++++++++++----------
 usertools/dpdk-telemetry.py |  9 +++++++--
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index f2551c64b10c..8a5723f3b3a7 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -86,25 +86,28 @@ struct lcore_config lcore_config[RTE_MAX_LCORE];
 /* used by rte_rdtsc() */
 int rte_cycles_vmware_tsc_map;
 
-static const char *default_runtime_dir = "/var/run";
-
 int
 eal_create_runtime_dir(void)
 {
-	const char *directory = default_runtime_dir;
-	const char *xdg_runtime_dir = getenv("XDG_RUNTIME_DIR");
-	const char *fallback = "/tmp";
+	const char *directory;
 	char run_dir[PATH_MAX];
 	char tmp[PATH_MAX];
 	int ret;
 
-	if (getuid() != 0) {
-		/* try XDG path first, fall back to /tmp */
-		if (xdg_runtime_dir != NULL)
-			directory = xdg_runtime_dir;
+	/* from RuntimeDirectory= see systemd.exec */
+	directory = getenv("RUNTIME_DIRECTORY");
+	if (directory == NULL) {
+		/*
+		 * Used standard convention defined in
+		 * XDG Base Directory Specification and
+		 * Filesystem Hierachy Standard.
+		 */
+		if (getuid() == 0)
+			directory = "/var/run";
 		else
-			directory = fallback;
+			directory = getenv("XDG_RUNTIME_DIR") ? : "/tmp";
 	}
+
 	/* create DPDK subdirectory under runtime dir */
 	ret = snprintf(tmp, sizeof(tmp), "%s/dpdk", directory);
 	if (ret < 0 || ret == sizeof(tmp)) {
diff --git a/usertools/dpdk-telemetry.py b/usertools/dpdk-telemetry.py
index 5b3bf83356c3..a49f0e76d07b 100755
--- a/usertools/dpdk-telemetry.py
+++ b/usertools/dpdk-telemetry.py
@@ -75,9 +75,14 @@ def print_socket_options(prefix, paths):
 def get_dpdk_runtime_dir(fp):
     """ Using the same logic as in DPDK's EAL, get the DPDK runtime directory
     based on the file-prefix and user """
+    run_dir = os.environ.get('RUNTIME_DIRECTORY')
+    if run_dir:
+        return run_dir
     if (os.getuid() == 0):
-        return os.path.join('/var/run/dpdk', fp)
-    return os.path.join(os.environ.get('XDG_RUNTIME_DIR', '/tmp'), 'dpdk', fp)
+        run_dir = '/var/run'
+    else:
+        run_dir = os.environ.get('XDG_RUNTIME_DIR', '/tmp'))
+    return os.path.join(run_dir, 'dpdk', fp)
 
 
 def list_fp():
-- 
2.34.1

Re: [PATCH 2/2] eal: support systemd service convention for runtime directory

[Bruce Richardson]

On Wed, Feb 02, 2022 at 10:00:25PM -0800, Stephen Hemminger wrote:
> Systemd.exec supports configuring the runtime directory of a service
> via RuntimeDirectory=. This creates the directory with the necessary
> permissions which actual service may not have if running in container.
> 
> The change to DPDK is to look for the environment RUNTIME_DIRECTORY
> first and use that in preference to the fallback alternatives.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> Reviewed-by: Morten Brørup <mb@smartsharesystems.com>
> ---
>  lib/eal/linux/eal.c         | 23 +++++++++++++----------
>  usertools/dpdk-telemetry.py |  9 +++++++--
>  2 files changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
> index f2551c64b10c..8a5723f3b3a7 100644
> --- a/lib/eal/linux/eal.c
> +++ b/lib/eal/linux/eal.c
> @@ -86,25 +86,28 @@ struct lcore_config lcore_config[RTE_MAX_LCORE];
>  /* used by rte_rdtsc() */
>  int rte_cycles_vmware_tsc_map;
>  
> -static const char *default_runtime_dir = "/var/run";
> -
>  int
>  eal_create_runtime_dir(void)
>  {
> -	const char *directory = default_runtime_dir;
> -	const char *xdg_runtime_dir = getenv("XDG_RUNTIME_DIR");
> -	const char *fallback = "/tmp";
> +	const char *directory;
>  	char run_dir[PATH_MAX];
>  	char tmp[PATH_MAX];
>  	int ret;
>  
> -	if (getuid() != 0) {
> -		/* try XDG path first, fall back to /tmp */
> -		if (xdg_runtime_dir != NULL)
> -			directory = xdg_runtime_dir;
> +	/* from RuntimeDirectory= see systemd.exec */
> +	directory = getenv("RUNTIME_DIRECTORY");
> +	if (directory == NULL) {
> +		/*
> +		 * Used standard convention defined in
> +		 * XDG Base Directory Specification and
> +		 * Filesystem Hierachy Standard.
> +		 */
> +		if (getuid() == 0)
> +			directory = "/var/run";
>  		else
> -			directory = fallback;
> +			directory = getenv("XDG_RUNTIME_DIR") ? : "/tmp";
>  	}
> +
>  	/* create DPDK subdirectory under runtime dir */
>  	ret = snprintf(tmp, sizeof(tmp), "%s/dpdk", directory);
>  	if (ret < 0 || ret == sizeof(tmp)) {
> diff --git a/usertools/dpdk-telemetry.py b/usertools/dpdk-telemetry.py
> index 5b3bf83356c3..a49f0e76d07b 100755
> --- a/usertools/dpdk-telemetry.py
> +++ b/usertools/dpdk-telemetry.py
> @@ -75,9 +75,14 @@ def print_socket_options(prefix, paths):
>  def get_dpdk_runtime_dir(fp):
>      """ Using the same logic as in DPDK's EAL, get the DPDK runtime directory
>      based on the file-prefix and user """
> +    run_dir = os.environ.get('RUNTIME_DIRECTORY')
> +    if run_dir:
> +        return run_dir

This bit doesn't seem to match the EAL changes above, since in the EAL code
you are still appending the "dpdk" suffixes to the value returned from
RUNTIME_DIRECTORY.

>      if (os.getuid() == 0):
> -        return os.path.join('/var/run/dpdk', fp)
> -    return os.path.join(os.environ.get('XDG_RUNTIME_DIR', '/tmp'), 'dpdk', fp)
> +        run_dir = '/var/run'
> +    else:
> +        run_dir = os.environ.get('XDG_RUNTIME_DIR', '/tmp'))
> +    return os.path.join(run_dir, 'dpdk', fp)
>  
>  
>  def list_fp():
> -- 
> 2.34.1
> 

[PATCH v2 0/2] eal: support configuring runtime directory

[Stephen Hemminger]

Found this while exploring running a DPDK service with systemd
container environment. It helps if the place DPDK puts its runtime
directory is configurable.

v2 - fix dpdk_telemetry.py to use append dpdk

Stephen Hemminger (2):
  eal: remove size for eal_set_runtime_dir
  eal: support systemd service convention for runtime directory

 lib/eal/common/eal_common_config.c |  7 ++-----
 lib/eal/common/eal_private.h       |  4 +---
 lib/eal/freebsd/eal.c              |  2 +-
 lib/eal/linux/eal.c                | 25 ++++++++++++++-----------
 usertools/dpdk-telemetry.py        | 10 +++++++---
 5 files changed, 25 insertions(+), 23 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] eal: remove size for eal_set_runtime_dir

[Stephen Hemminger]

The size argument to eal_set_runtime_dir is useless and was
being used incorrectly in strlcpy. It worked only because
all callers passed PATH_MAX which is same as sizeof the destination
runtime_dir.

Note: this is an internal API so no user exposed change.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Reviewed-by: Morten Brørup<mb@smartsharesystems.com>
---
 lib/eal/common/eal_common_config.c | 7 ++-----
 lib/eal/common/eal_private.h       | 4 +---
 lib/eal/freebsd/eal.c              | 2 +-
 lib/eal/linux/eal.c                | 2 +-
 4 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/lib/eal/common/eal_common_config.c b/lib/eal/common/eal_common_config.c
index 1c4c4dd585d0..62a9d7a198db 100644
--- a/lib/eal/common/eal_common_config.c
+++ b/lib/eal/common/eal_common_config.c
@@ -29,12 +29,9 @@ rte_eal_get_runtime_dir(void)
 }
 
 int
-eal_set_runtime_dir(char *run_dir, size_t size)
+eal_set_runtime_dir(const char *run_dir)
 {
-	size_t str_size;
-
-	str_size = strlcpy(runtime_dir, run_dir, size);
-	if (str_size >= size) {
+	if (strlcpy(runtime_dir, run_dir, PATH_MAX) >= PATH_MAX) {
 		RTE_LOG(ERR, EAL, "Runtime directory string too long\n");
 		return -1;
 	}
diff --git a/lib/eal/common/eal_private.h b/lib/eal/common/eal_private.h
index 36bcc0b5a492..734f1f334b69 100644
--- a/lib/eal/common/eal_private.h
+++ b/lib/eal/common/eal_private.h
@@ -681,13 +681,11 @@ eal_mem_set_dump(void *virt, size_t size, bool dump);
  *
  * @param run_dir
  *   The new runtime directory path of DPDK
- * @param size
- *   The size of the new runtime directory path in bytes.
  * @return
  *   0 on success, (-1) on failure.
  */
 int
-eal_set_runtime_dir(char *run_dir, size_t size);
+eal_set_runtime_dir(const char *run_dir);
 
 /**
  * Get the internal configuration structure.
diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index a1cd2462db1b..503e276dc27f 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -123,7 +123,7 @@ eal_create_runtime_dir(void)
 		return -1;
 	}
 
-	if (eal_set_runtime_dir(run_dir, sizeof(run_dir)))
+	if (eal_set_runtime_dir(run_dir))
 		return -1;
 
 	return 0;
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 60b49248388e..f2551c64b10c 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -137,7 +137,7 @@ eal_create_runtime_dir(void)
 		return -1;
 	}
 
-	if (eal_set_runtime_dir(run_dir, sizeof(run_dir)))
+	if (eal_set_runtime_dir(run_dir))
 		return -1;
 
 	return 0;
-- 
2.34.1

[PATCH v2 2/2] eal: support systemd service convention for runtime directory

[Stephen Hemminger]

Systemd.exec supports configuring the runtime directory of a service
via RuntimeDirectory=. This creates the directory with the necessary
permissions which actual service may not have if running in container.

The change to DPDK is to look for the environment RUNTIME_DIRECTORY
first and use that in preference to the fallback alternatives.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Reviewed-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/eal/linux/eal.c         | 23 +++++++++++++----------
 usertools/dpdk-telemetry.py | 10 +++++++---
 2 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index f2551c64b10c..8a5723f3b3a7 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -86,25 +86,28 @@ struct lcore_config lcore_config[RTE_MAX_LCORE];
 /* used by rte_rdtsc() */
 int rte_cycles_vmware_tsc_map;
 
-static const char *default_runtime_dir = "/var/run";
-
 int
 eal_create_runtime_dir(void)
 {
-	const char *directory = default_runtime_dir;
-	const char *xdg_runtime_dir = getenv("XDG_RUNTIME_DIR");
-	const char *fallback = "/tmp";
+	const char *directory;
 	char run_dir[PATH_MAX];
 	char tmp[PATH_MAX];
 	int ret;
 
-	if (getuid() != 0) {
-		/* try XDG path first, fall back to /tmp */
-		if (xdg_runtime_dir != NULL)
-			directory = xdg_runtime_dir;
+	/* from RuntimeDirectory= see systemd.exec */
+	directory = getenv("RUNTIME_DIRECTORY");
+	if (directory == NULL) {
+		/*
+		 * Used standard convention defined in
+		 * XDG Base Directory Specification and
+		 * Filesystem Hierachy Standard.
+		 */
+		if (getuid() == 0)
+			directory = "/var/run";
 		else
-			directory = fallback;
+			directory = getenv("XDG_RUNTIME_DIR") ? : "/tmp";
 	}
+
 	/* create DPDK subdirectory under runtime dir */
 	ret = snprintf(tmp, sizeof(tmp), "%s/dpdk", directory);
 	if (ret < 0 || ret == sizeof(tmp)) {
diff --git a/usertools/dpdk-telemetry.py b/usertools/dpdk-telemetry.py
index 5b3bf83356c3..a81868a54789 100755
--- a/usertools/dpdk-telemetry.py
+++ b/usertools/dpdk-telemetry.py
@@ -75,9 +75,13 @@ def print_socket_options(prefix, paths):
 def get_dpdk_runtime_dir(fp):
     """ Using the same logic as in DPDK's EAL, get the DPDK runtime directory
     based on the file-prefix and user """
-    if (os.getuid() == 0):
-        return os.path.join('/var/run/dpdk', fp)
-    return os.path.join(os.environ.get('XDG_RUNTIME_DIR', '/tmp'), 'dpdk', fp)
+    run_dir = os.environ.get('RUNTIME_DIRECTORY')
+    if not run_dir:
+        if (os.getuid() == 0):
+            run_dir = '/var/run'
+        else:
+            run_dir = os.environ.get('XDG_RUNTIME_DIR', '/tmp')
+    return os.path.join(run_dir, 'dpdk', fp)
 
 
 def list_fp():
-- 
2.34.1

Re: [PATCH v2 0/2] eal: support configuring runtime directory

[Stephen Hemminger]

On Sat,  5 Feb 2022 09:10:59 -0800
Stephen Hemminger <stephen@networkplumber.org> wrote:

> Found this while exploring running a DPDK service with systemd
> container environment. It helps if the place DPDK puts its runtime
> directory is configurable.
> 
> v2 - fix dpdk_telemetry.py to use append dpdk
> 
> Stephen Hemminger (2):
>   eal: remove size for eal_set_runtime_dir
>   eal: support systemd service convention for runtime directory
> 
>  lib/eal/common/eal_common_config.c |  7 ++-----
>  lib/eal/common/eal_private.h       |  4 +---
>  lib/eal/freebsd/eal.c              |  2 +-
>  lib/eal/linux/eal.c                | 25 ++++++++++++++-----------
>  usertools/dpdk-telemetry.py        | 10 +++++++---
>  5 files changed, 25 insertions(+), 23 deletions(-)
> 

Bruce is it worth moving this (and the sysfs logic) from
Linux only into unix/ directory and have common code with FreeBSD?

Re: [PATCH v2 0/2] eal: support configuring runtime directory

[Bruce Richardson]

On Mon, Feb 07, 2022 at 08:43:48PM -0800, Stephen Hemminger wrote:
> On Sat,  5 Feb 2022 09:10:59 -0800
> Stephen Hemminger <stephen@networkplumber.org> wrote:
> 
> > Found this while exploring running a DPDK service with systemd
> > container environment. It helps if the place DPDK puts its runtime
> > directory is configurable.
> > 
> > v2 - fix dpdk_telemetry.py to use append dpdk
> > 
> > Stephen Hemminger (2):
> >   eal: remove size for eal_set_runtime_dir
> >   eal: support systemd service convention for runtime directory
> > 
> >  lib/eal/common/eal_common_config.c |  7 ++-----
> >  lib/eal/common/eal_private.h       |  4 +---
> >  lib/eal/freebsd/eal.c              |  2 +-
> >  lib/eal/linux/eal.c                | 25 ++++++++++++++-----------
> >  usertools/dpdk-telemetry.py        | 10 +++++++---
> >  5 files changed, 25 insertions(+), 23 deletions(-)
> > 
> 
> Bruce is it worth moving this (and the sysfs logic) from
> Linux only into unix/ directory and have common code with FreeBSD?
> 
The "create_runtime_dir()" function is indeed a copy-paste between the two
OS's so looks like it should be moved into a common unix implementation.

[PATCH v3 0/3] eal: support configuring runtime directory

[Stephen Hemminger]

Systemd defines a standard way for setting the runtime directory
for services by setting RUNTIME_DIRECTORY in the environment,
and this is a useful way to interact with DPDK internals.
DPDK was already following the relevant standards
but there can be time when running as a non-root user that
the defaults don't work well.

v3 - move to common code between Linux and FreeBSD
     there is lots more code that could be combined here in future.

v2 - fix dpdk_telemetry.py to use append dpdk

Stephen Hemminger (3):
  eal: remove size for eal_set_runtime_dir
  eal: support systemd service convention for runtime directory
  eal: move common filesystem setup code into one file

 lib/eal/common/eal_common_config.c |   7 +-
 lib/eal/common/eal_private.h       |   4 +-
 lib/eal/freebsd/eal.c              |  88 -----------------------
 lib/eal/linux/eal.c                |  87 -----------------------
 lib/eal/unix/eal_filesystem.c      | 110 +++++++++++++++++++++++++++++
 lib/eal/unix/meson.build           |   1 +
 usertools/dpdk-telemetry.py        |  10 ++-
 7 files changed, 121 insertions(+), 186 deletions(-)
 create mode 100644 lib/eal/unix/eal_filesystem.c

-- 
2.34.1

[PATCH v3 1/3] eal: remove size for eal_set_runtime_dir

[Stephen Hemminger]

The size argument to eal_set_runtime_dir is useless and was
being used incorrectly in strlcpy. It worked only because
all callers passed PATH_MAX which is same as sizeof the destination
runtime_dir.

Note: this is an internal API so no user exposed change.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Reviewed-by: Morten Brørup<mb@smartsharesystems.com>
---
 lib/eal/common/eal_common_config.c | 7 ++-----
 lib/eal/common/eal_private.h       | 4 +---
 lib/eal/freebsd/eal.c              | 2 +-
 lib/eal/linux/eal.c                | 2 +-
 4 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/lib/eal/common/eal_common_config.c b/lib/eal/common/eal_common_config.c
index 6d19aadb2057..3cef43a4f706 100644
--- a/lib/eal/common/eal_common_config.c
+++ b/lib/eal/common/eal_common_config.c
@@ -29,12 +29,9 @@ rte_eal_get_runtime_dir(void)
 }
 
 int
-eal_set_runtime_dir(char *run_dir, size_t size)
+eal_set_runtime_dir(const char *run_dir)
 {
-	size_t str_size;
-
-	str_size = strlcpy(runtime_dir, run_dir, size);
-	if (str_size >= size) {
+	if (strlcpy(runtime_dir, run_dir, PATH_MAX) >= PATH_MAX) {
 		RTE_LOG(ERR, EAL, "Runtime directory string too long\n");
 		return -1;
 	}
diff --git a/lib/eal/common/eal_private.h b/lib/eal/common/eal_private.h
index 36bcc0b5a492..734f1f334b69 100644
--- a/lib/eal/common/eal_private.h
+++ b/lib/eal/common/eal_private.h
@@ -681,13 +681,11 @@ eal_mem_set_dump(void *virt, size_t size, bool dump);
  *
  * @param run_dir
  *   The new runtime directory path of DPDK
- * @param size
- *   The size of the new runtime directory path in bytes.
  * @return
  *   0 on success, (-1) on failure.
  */
 int
-eal_set_runtime_dir(char *run_dir, size_t size);
+eal_set_runtime_dir(const char *run_dir);
 
 /**
  * Get the internal configuration structure.
diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index a1cd2462db1b..503e276dc27f 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -123,7 +123,7 @@ eal_create_runtime_dir(void)
 		return -1;
 	}
 
-	if (eal_set_runtime_dir(run_dir, sizeof(run_dir)))
+	if (eal_set_runtime_dir(run_dir))
 		return -1;
 
 	return 0;
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 9c8395ab14d0..e37372ac7305 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -137,7 +137,7 @@ eal_create_runtime_dir(void)
 		return -1;
 	}
 
-	if (eal_set_runtime_dir(run_dir, sizeof(run_dir)))
+	if (eal_set_runtime_dir(run_dir))
 		return -1;
 
 	return 0;
-- 
2.34.1

[PATCH v3 2/3] eal: support systemd service convention for runtime directory

[Stephen Hemminger]

Systemd.exec supports configuring the runtime directory of a service
via RuntimeDirectory=. This creates the directory with the necessary
permissions which actual service may not have if running in container.

The change to DPDK is to look for the environment RUNTIME_DIRECTORY
first and use that in preference to the fallback alternatives.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Reviewed-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/eal/linux/eal.c         | 23 +++++++++++++----------
 usertools/dpdk-telemetry.py | 10 +++++++---
 2 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index e37372ac7305..e9764f6c9f37 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -86,25 +86,28 @@ struct lcore_config lcore_config[RTE_MAX_LCORE];
 /* used by rte_rdtsc() */
 int rte_cycles_vmware_tsc_map;
 
-static const char *default_runtime_dir = "/var/run";
-
 int
 eal_create_runtime_dir(void)
 {
-	const char *directory = default_runtime_dir;
-	const char *xdg_runtime_dir = getenv("XDG_RUNTIME_DIR");
-	const char *fallback = "/tmp";
+	const char *directory;
 	char run_dir[PATH_MAX];
 	char tmp[PATH_MAX];
 	int ret;
 
-	if (getuid() != 0) {
-		/* try XDG path first, fall back to /tmp */
-		if (xdg_runtime_dir != NULL)
-			directory = xdg_runtime_dir;
+	/* from RuntimeDirectory= see systemd.exec */
+	directory = getenv("RUNTIME_DIRECTORY");
+	if (directory == NULL) {
+		/*
+		 * Used standard convention defined in
+		 * XDG Base Directory Specification and
+		 * Filesystem Hierarchy Standard.
+		 */
+		if (getuid() == 0)
+			directory = "/var/run";
 		else
-			directory = fallback;
+			directory = getenv("XDG_RUNTIME_DIR") ? : "/tmp";
 	}
+
 	/* create DPDK subdirectory under runtime dir */
 	ret = snprintf(tmp, sizeof(tmp), "%s/dpdk", directory);
 	if (ret < 0 || ret == sizeof(tmp)) {
diff --git a/usertools/dpdk-telemetry.py b/usertools/dpdk-telemetry.py
index 5b3bf83356c3..a81868a54789 100755
--- a/usertools/dpdk-telemetry.py
+++ b/usertools/dpdk-telemetry.py
@@ -75,9 +75,13 @@ def print_socket_options(prefix, paths):
 def get_dpdk_runtime_dir(fp):
     """ Using the same logic as in DPDK's EAL, get the DPDK runtime directory
     based on the file-prefix and user """
-    if (os.getuid() == 0):
-        return os.path.join('/var/run/dpdk', fp)
-    return os.path.join(os.environ.get('XDG_RUNTIME_DIR', '/tmp'), 'dpdk', fp)
+    run_dir = os.environ.get('RUNTIME_DIRECTORY')
+    if not run_dir:
+        if (os.getuid() == 0):
+            run_dir = '/var/run'
+        else:
+            run_dir = os.environ.get('XDG_RUNTIME_DIR', '/tmp')
+    return os.path.join(run_dir, 'dpdk', fp)
 
 
 def list_fp():
-- 
2.34.1

[PATCH v3 3/3] eal: move common filesystem setup code into one file

[Stephen Hemminger]

Both Linux and FreeBSD have same code for creating runtime
directory and reading sysfs files. Put them in the new lib/eal/unix
subdirectory.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/freebsd/eal.c         |  88 ---------------------------
 lib/eal/linux/eal.c           |  90 ----------------------------
 lib/eal/unix/eal_filesystem.c | 110 ++++++++++++++++++++++++++++++++++
 lib/eal/unix/meson.build      |   1 +
 4 files changed, 111 insertions(+), 178 deletions(-)
 create mode 100644 lib/eal/unix/eal_filesystem.c

diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index 503e276dc27f..eacd432ab621 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -72,62 +72,6 @@ struct lcore_config lcore_config[RTE_MAX_LCORE];
 /* used by rte_rdtsc() */
 int rte_cycles_vmware_tsc_map;
 
-static const char *default_runtime_dir = "/var/run";
-
-int
-eal_create_runtime_dir(void)
-{
-	const char *directory = default_runtime_dir;
-	const char *xdg_runtime_dir = getenv("XDG_RUNTIME_DIR");
-	const char *fallback = "/tmp";
-	char run_dir[PATH_MAX];
-	char tmp[PATH_MAX];
-	int ret;
-
-	if (getuid() != 0) {
-		/* try XDG path first, fall back to /tmp */
-		if (xdg_runtime_dir != NULL)
-			directory = xdg_runtime_dir;
-		else
-			directory = fallback;
-	}
-	/* create DPDK subdirectory under runtime dir */
-	ret = snprintf(tmp, sizeof(tmp), "%s/dpdk", directory);
-	if (ret < 0 || ret == sizeof(tmp)) {
-		RTE_LOG(ERR, EAL, "Error creating DPDK runtime path name\n");
-		return -1;
-	}
-
-	/* create prefix-specific subdirectory under DPDK runtime dir */
-	ret = snprintf(run_dir, sizeof(run_dir), "%s/%s",
-			tmp, eal_get_hugefile_prefix());
-	if (ret < 0 || ret == sizeof(run_dir)) {
-		RTE_LOG(ERR, EAL, "Error creating prefix-specific runtime path name\n");
-		return -1;
-	}
-
-	/* create the path if it doesn't exist. no "mkdir -p" here, so do it
-	 * step by step.
-	 */
-	ret = mkdir(tmp, 0700);
-	if (ret < 0 && errno != EEXIST) {
-		RTE_LOG(ERR, EAL, "Error creating '%s': %s\n",
-			tmp, strerror(errno));
-		return -1;
-	}
-
-	ret = mkdir(run_dir, 0700);
-	if (ret < 0 && errno != EEXIST) {
-		RTE_LOG(ERR, EAL, "Error creating '%s': %s\n",
-			run_dir, strerror(errno));
-		return -1;
-	}
-
-	if (eal_set_runtime_dir(run_dir))
-		return -1;
-
-	return 0;
-}
 
 int
 eal_clean_runtime_dir(void)
@@ -138,38 +82,6 @@ eal_clean_runtime_dir(void)
 	return 0;
 }
 
-/* parse a sysfs (or other) file containing one integer 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) {
-		RTE_LOG(ERR, EAL, "%s(): cannot open sysfs value %s\n",
-			__func__, filename);
-		return -1;
-	}
-
-	if (fgets(buf, sizeof(buf), f) == NULL) {
-		RTE_LOG(ERR, EAL, "%s(): cannot read sysfs value %s\n",
-			__func__, filename);
-		fclose(f);
-		return -1;
-	}
-	*val = strtoul(buf, &end, 0);
-	if ((buf[0] == '\0') || (end == NULL) || (*end != '\n')) {
-		RTE_LOG(ERR, EAL, "%s(): cannot parse sysfs value %s\n",
-				__func__, filename);
-		fclose(f);
-		return -1;
-	}
-	fclose(f);
-	return 0;
-}
-
-
 /* create memory configuration in shared/mmap memory. Take out
  * a write lock on the memsegs, so we can auto-detect primary/secondary.
  * This means we never close the file while running (auto-close on exit).
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index e9764f6c9f37..b99cab25ce48 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -86,65 +86,6 @@ struct lcore_config lcore_config[RTE_MAX_LCORE];
 /* used by rte_rdtsc() */
 int rte_cycles_vmware_tsc_map;
 
-int
-eal_create_runtime_dir(void)
-{
-	const char *directory;
-	char run_dir[PATH_MAX];
-	char tmp[PATH_MAX];
-	int ret;
-
-	/* from RuntimeDirectory= see systemd.exec */
-	directory = getenv("RUNTIME_DIRECTORY");
-	if (directory == NULL) {
-		/*
-		 * Used standard convention defined in
-		 * XDG Base Directory Specification and
-		 * Filesystem Hierarchy Standard.
-		 */
-		if (getuid() == 0)
-			directory = "/var/run";
-		else
-			directory = getenv("XDG_RUNTIME_DIR") ? : "/tmp";
-	}
-
-	/* create DPDK subdirectory under runtime dir */
-	ret = snprintf(tmp, sizeof(tmp), "%s/dpdk", directory);
-	if (ret < 0 || ret == sizeof(tmp)) {
-		RTE_LOG(ERR, EAL, "Error creating DPDK runtime path name\n");
-		return -1;
-	}
-
-	/* create prefix-specific subdirectory under DPDK runtime dir */
-	ret = snprintf(run_dir, sizeof(run_dir), "%s/%s",
-			tmp, eal_get_hugefile_prefix());
-	if (ret < 0 || ret == sizeof(run_dir)) {
-		RTE_LOG(ERR, EAL, "Error creating prefix-specific runtime path name\n");
-		return -1;
-	}
-
-	/* create the path if it doesn't exist. no "mkdir -p" here, so do it
-	 * step by step.
-	 */
-	ret = mkdir(tmp, 0700);
-	if (ret < 0 && errno != EEXIST) {
-		RTE_LOG(ERR, EAL, "Error creating '%s': %s\n",
-			tmp, strerror(errno));
-		return -1;
-	}
-
-	ret = mkdir(run_dir, 0700);
-	if (ret < 0 && errno != EEXIST) {
-		RTE_LOG(ERR, EAL, "Error creating '%s': %s\n",
-			run_dir, strerror(errno));
-		return -1;
-	}
-
-	if (eal_set_runtime_dir(run_dir))
-		return -1;
-
-	return 0;
-}
 
 int
 eal_clean_runtime_dir(void)
@@ -232,37 +173,6 @@ eal_clean_runtime_dir(void)
 	return -1;
 }
 
-/* parse a sysfs (or other) file containing one integer 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) {
-		RTE_LOG(ERR, EAL, "%s(): cannot open sysfs value %s\n",
-			__func__, filename);
-		return -1;
-	}
-
-	if (fgets(buf, sizeof(buf), f) == NULL) {
-		RTE_LOG(ERR, EAL, "%s(): cannot read sysfs value %s\n",
-			__func__, filename);
-		fclose(f);
-		return -1;
-	}
-	*val = strtoul(buf, &end, 0);
-	if ((buf[0] == '\0') || (end == NULL) || (*end != '\n')) {
-		RTE_LOG(ERR, EAL, "%s(): cannot parse sysfs value %s\n",
-				__func__, filename);
-		fclose(f);
-		return -1;
-	}
-	fclose(f);
-	return 0;
-}
-
 
 /* create memory configuration in shared/mmap memory. Take out
  * a write lock on the memsegs, so we can auto-detect primary/secondary.
diff --git a/lib/eal/unix/eal_filesystem.c b/lib/eal/unix/eal_filesystem.c
new file mode 100644
index 000000000000..4759651a9581
--- /dev/null
+++ b/lib/eal/unix/eal_filesystem.c
@@ -0,0 +1,110 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2010-2018 Intel Corporation.
+ * Copyright(c) 2012-2014 6WIND S.A.
+ */
+
+#include <errno.h>
+#include <limits.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <rte_common.h>
+#include <rte_log.h>
+
+#include "eal_private.h"
+#include "eal_filesystem.h"
+
+int
+eal_create_runtime_dir(void)
+{
+	const char *directory;
+	char run_dir[PATH_MAX];
+	char tmp[PATH_MAX];
+	int ret;
+
+	/* from RuntimeDirectory= see systemd.exec */
+	directory = getenv("RUNTIME_DIRECTORY");
+	if (directory == NULL) {
+		/*
+		 * Used standard convention defined in
+		 * XDG Base Directory Specification and
+		 * Filesystem Hierarchy Standard.
+		 */
+		if (getuid() == 0)
+			directory = "/var/run";
+		else
+			directory = getenv("XDG_RUNTIME_DIR") ? : "/tmp";
+	}
+
+	/* create DPDK subdirectory under runtime dir */
+	ret = snprintf(tmp, sizeof(tmp), "%s/dpdk", directory);
+	if (ret < 0 || ret == sizeof(tmp)) {
+		RTE_LOG(ERR, EAL, "Error creating DPDK runtime path name\n");
+		return -1;
+	}
+
+	/* create prefix-specific subdirectory under DPDK runtime dir */
+	ret = snprintf(run_dir, sizeof(run_dir), "%s/%s",
+			tmp, eal_get_hugefile_prefix());
+	if (ret < 0 || ret == sizeof(run_dir)) {
+		RTE_LOG(ERR, EAL, "Error creating prefix-specific runtime path name\n");
+		return -1;
+	}
+
+	/* create the path if it doesn't exist. no "mkdir -p" here, so do it
+	 * step by step.
+	 */
+	ret = mkdir(tmp, 0700);
+	if (ret < 0 && errno != EEXIST) {
+		RTE_LOG(ERR, EAL, "Error creating '%s': %s\n",
+			tmp, strerror(errno));
+		return -1;
+	}
+
+	ret = mkdir(run_dir, 0700);
+	if (ret < 0 && errno != EEXIST) {
+		RTE_LOG(ERR, EAL, "Error creating '%s': %s\n",
+			run_dir, strerror(errno));
+		return -1;
+	}
+
+	if (eal_set_runtime_dir(run_dir))
+		return -1;
+
+	return 0;
+}
+/* parse a sysfs (or other) file containing one integer 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) {
+		RTE_LOG(ERR, EAL, "%s(): cannot open sysfs value %s\n",
+			__func__, filename);
+		return -1;
+	}
+
+	if (fgets(buf, sizeof(buf), f) == NULL) {
+		RTE_LOG(ERR, EAL, "%s(): cannot read sysfs value %s\n",
+			__func__, filename);
+		fclose(f);
+		return -1;
+	}
+	*val = strtoul(buf, &end, 0);
+	if ((buf[0] == '\0') || (end == NULL) || (*end != '\n')) {
+		RTE_LOG(ERR, EAL, "%s(): cannot parse sysfs value %s\n",
+				__func__, filename);
+		fclose(f);
+		return -1;
+	}
+	fclose(f);
+	return 0;
+}
diff --git a/lib/eal/unix/meson.build b/lib/eal/unix/meson.build
index e3ecd3e956b2..a22ea7cabc46 100644
--- a/lib/eal/unix/meson.build
+++ b/lib/eal/unix/meson.build
@@ -6,5 +6,6 @@ sources += files(
         'eal_unix_memory.c',
         'eal_unix_timer.c',
         'eal_firmware.c',
+        'eal_filesystem.c',
         'rte_thread.c',
 )
-- 
2.34.1

Re: [PATCH v3 3/3] eal: move common filesystem setup code into one file

[Bruce Richardson]

On Tue, Feb 08, 2022 at 10:54:03PM -0800, Stephen Hemminger wrote:
> Both Linux and FreeBSD have same code for creating runtime
> directory and reading sysfs files. Put them in the new lib/eal/unix
> subdirectory.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
LGTM, apart from one minor nit inline below.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  lib/eal/freebsd/eal.c         |  88 ---------------------------
>  lib/eal/linux/eal.c           |  90 ----------------------------
>  lib/eal/unix/eal_filesystem.c | 110 ++++++++++++++++++++++++++++++++++
>  lib/eal/unix/meson.build      |   1 +
>  4 files changed, 111 insertions(+), 178 deletions(-)
>  create mode 100644 lib/eal/unix/eal_filesystem.c
> 
<snip>
> +
> +	/* create the path if it doesn't exist. no "mkdir -p" here, so do it
> +	 * step by step.
> +	 */
> +	ret = mkdir(tmp, 0700);
> +	if (ret < 0 && errno != EEXIST) {
> +		RTE_LOG(ERR, EAL, "Error creating '%s': %s\n",
> +			tmp, strerror(errno));
> +		return -1;
> +	}
> +
> +	ret = mkdir(run_dir, 0700);
> +	if (ret < 0 && errno != EEXIST) {
> +		RTE_LOG(ERR, EAL, "Error creating '%s': %s\n",
> +			run_dir, strerror(errno));
> +		return -1;
> +	}
> +
> +	if (eal_set_runtime_dir(run_dir))
> +		return -1;
> +
> +	return 0;
> +}

Missing a blank line here between the two functions.

> +/* parse a sysfs (or other) file containing one integer 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) {
> +		RTE_LOG(ERR, EAL, "%s(): cannot open sysfs value %s\n",
> +			__func__, filename);
> +		return -1;
> +	}
> +
> +	if (fgets(buf, sizeof(buf), f) == NULL) {
> +		RTE_LOG(ERR, EAL, "%s(): cannot read sysfs value %s\n",
> +			__func__, filename);
> +		fclose(f);
> +		return -1;
> +	}
> +	*val = strtoul(buf, &end, 0);
> +	if ((buf[0] == '\0') || (end == NULL) || (*end != '\n')) {
> +		RTE_LOG(ERR, EAL, "%s(): cannot parse sysfs value %s\n",
> +				__func__, filename);
> +		fclose(f);
> +		return -1;
> +	}
> +	fclose(f);
> +	return 0;
> +}
> diff --git a/lib/eal/unix/meson.build b/lib/eal/unix/meson.build
> index e3ecd3e956b2..a22ea7cabc46 100644
> --- a/lib/eal/unix/meson.build
> +++ b/lib/eal/unix/meson.build
> @@ -6,5 +6,6 @@ sources += files(
>          'eal_unix_memory.c',
>          'eal_unix_timer.c',
>          'eal_firmware.c',
> +        'eal_filesystem.c',
>          'rte_thread.c',
>  )
> -- 
> 2.34.1
> 

Re: [PATCH v3 0/3] eal: support configuring runtime directory

[Thomas Monjalon]

09/02/2022 07:54, Stephen Hemminger:
> Systemd defines a standard way for setting the runtime directory
> for services by setting RUNTIME_DIRECTORY in the environment,
> and this is a useful way to interact with DPDK internals.
> DPDK was already following the relevant standards
> but there can be time when running as a non-root user that
> the defaults don't work well.

Applied with suggested minor fix, thanks.