[RFC] eal: support systemd service convention for runtime directory
Checks
Commit Message
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(-)
Comments
> 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().
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>
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?
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
@@ -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)) {