Clean up EAL runtime data paths

---
 lib/librte_eal/common/eal_filesystem.h | 2 --
 1 file changed, 2 deletions(-)

lib/librte_eal/common/eal_filesystem.h +0 -2

index 0a82f89..37ac672 100644
@@ -104,8 +104,6 @@ eal_hugepage_file_path(void)
 
 /** String format for hugepage map files. */
 #define HUGEFILE_FMT "%s/%smap_%d"
-#define TEMP_HUGEFILE_FMT "%s/%smap_temp_%d"
-
 static inline const char *
 eal_get_hugefile_path(char *buffer, size_t buflen, const char *hugedir, int f_id)
 {

Signature · 1 line

2.7.4

David Marchand

Reviewed-by on v1 1/4

2018-04-26 07:12 UTC #2 lore raw

Re: [PATCH 18.05-RC2 1/4] eal: remove unused define

Quoted 7 lines · Anatoly Burakov

The define was a leftover from IVSHMEM library.
Fixes c711ccb30987 ("ivshmem: remove library and its EAL integration")
Cc [email protected]
Cc [email protected]
Signed-off-by Anatoly Burakov <[email protected]>

Indeed.

Reviewed-by David Marchand <[email protected]>

However, not sure this really needs to go to stable.

Signature · 1 line

David Marchand

Anatoly Burakov

author

2018-04-25 12:24 UTC #3 lore raw

PATCH 2/4 eal: rename function returning hugepage data path

The original name for this path was not too descriptive and confusing. Rename it to a more appropriate and descriptive name:
it stores data about hugepages, so name it eal_hugepage_data_path().

---
 lib/librte_eal/common/eal_filesystem.h   |  2 +-
 lib/librte_eal/linuxapp/eal/eal_memory.c | 10 ++++++----
 2 files changed, 7 insertions(+), 5 deletions(-)

lib/librte_eal/common/eal_filesystem.h +1 -1

index 37ac672..2ed857e 100644
@@ -89,7 +89,7 @@ eal_hugepage_info_path(void)
 #define HUGEPAGE_FILE_FMT "%s/.%s_hugepage_file"
 
 static inline const char *
-eal_hugepage_file_path(void)
+eal_hugepage_data_path(void)
 {
 	static char buffer[PATH_MAX]; /* static so auto-zeroed */
 	const char *directory = default_config_dir;

lib/librte_eal/linuxapp/eal/eal_memory.c +6 -4

index ad3add8..db401a9 100644
@@ -1499,7 +1499,7 @@ eal_legacy_hugepage_init(void)
 	}
 
 	/* create shared memory */
-	hugepage = create_shared_memory(eal_hugepage_file_path(),
+	hugepage = create_shared_memory(eal_hugepage_data_path(),
 			nr_hugefiles * sizeof(struct hugepage_file));
 
 	if (hugepage == NULL) {

@@ -1696,16 +1696,18 @@ eal_legacy_hugepage_attach(void)
 
 	test_phys_addrs_available();
 
-	fd_hugepage = open(eal_hugepage_file_path(), O_RDONLY);
+	fd_hugepage = open(eal_hugepage_data_path(), O_RDONLY);
 	if (fd_hugepage < 0) {
-		RTE_LOG(ERR, EAL, "Could not open %s\n", eal_hugepage_file_path());
+		RTE_LOG(ERR, EAL, "Could not open %s\n",
+				eal_hugepage_data_path());
 		goto error;
 	}
 
 	size = getFileSize(fd_hugepage);
 	hp = mmap(NULL, size, PROT_READ, MAP_PRIVATE, fd_hugepage, 0);
 	if (hp == MAP_FAILED) {
-		RTE_LOG(ERR, EAL, "Could not mmap %s\n", eal_hugepage_file_path());
+		RTE_LOG(ERR, EAL, "Could not mmap %s\n",
+				eal_hugepage_data_path());
 		goto error;
 	}

Signature · 1 line

2.7.4

Anatoly Burakov

author

2018-04-25 12:24 UTC #4 lore raw

PATCH 3/4 eal: add directory for DPDK runtime data

Currently, during runtime, DPDK will store a bunch of files here and there (in /var/run, /tmp or in $HOME). Fix it by creating a DPDK-specific runtime directory, under which all runtime data will be placed. The template for creating this runtime directory is the following:

<base path>/dpdk/<DPDK prefix>/

Where <base path> is set to either "/var/run" if run as root, or $XDG_RUNTIME_DIR if run as non-root, with a fallback to /tmp if $XDG_RUNTIME_DIR is not defined. So, for example, if run as root, by default all runtime data will be stored at /var/run/dpdk/rte/.

There is no equivalent of "mkdir -p", so we will be creating the path step by step.

Nothing uses this new path yet, changes for that will come in next commit.

---
 lib/librte_eal/bsdapp/eal/eal.c        | 68 ++++++++++++++++++++++++++++++++++
 lib/librte_eal/common/eal_filesystem.h |  8 ++++
 lib/librte_eal/linuxapp/eal/eal.c      | 67 +++++++++++++++++++++++++++++++++
 3 files changed, 143 insertions(+)

lib/librte_eal/bsdapp/eal/eal.c +68 -0

index d996190..400c273 100644
@@ -18,6 +18,7 @@
 #include <limits.h>
 #include <sys/mman.h>
 #include <sys/queue.h>
+#include <sys/stat.h>
 
 #include <rte_compat.h>
 #include <rte_common.h>

@@ -83,6 +84,66 @@ struct internal_config internal_config;
 /* used by rte_rdtsc() */
 int rte_cycles_vmware_tsc_map;
 
+/* platform-specific runtime dir */
+static char runtime_dir[PATH_MAX];
+
+int
+eal_create_runtime_dir(void)
+{
+	const char *directory = default_config_dir;
+	const char *xdg_runtime_dir = getenv("XDG_RUNTIME_DIR");
+	const char *fallback = "/tmp";
+	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(runtime_dir, sizeof(runtime_dir), "%s/%s",
+			tmp, internal_config.hugefile_prefix);
+	if (ret < 0 || ret == sizeof(runtime_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, 0600);
+	if (ret < 0 && errno != EEXIST) {
+		RTE_LOG(ERR, EAL, "Error creating '%s': %s\n",
+			tmp, strerror(errno));
+		return -1;
+	}
+
+	ret = mkdir(runtime_dir, 0600);
+	if (ret < 0 && errno != EEXIST) {
+		RTE_LOG(ERR, EAL, "Error creating '%s': %s\n",
+			runtime_dir, strerror(errno));
+		return -1;
+	}
+
+	return 0;
+}
+
+const char *
+eal_get_runtime_dir(void)
+{
+	return runtime_dir;
+}
+
 /* Return user provided mbuf pool ops name */
 const char * __rte_experimental
 rte_eal_mbuf_user_pool_ops(void)

@@ -522,6 +583,13 @@ rte_eal_init(int argc, char **argv)
 	/* set log level as early as possible */
 	eal_log_level_parse(argc, argv);
 
+	/* create runtime data directory */
+	if (eal_create_runtime_dir() < 0) {
+		rte_eal_init_alert("Cannot create runtime directory\n");
+		rte_errno = EACCES;
+		return -1;
+	}
+
 	if (rte_eal_cpu_init() < 0) {
 		rte_eal_init_alert("Cannot detect lcores.");
 		rte_errno = ENOTSUP;

lib/librte_eal/common/eal_filesystem.h +8 -0

index 2ed857e..22f9f21 100644
@@ -25,6 +25,14 @@
 
 static const char *default_config_dir = "/var/run";
 
+/* sets up platform-specific runtime data dir */
+int
+eal_create_runtime_dir(void);
+
+/* returns runtime dir */
+const char *
+eal_get_runtime_dir(void);
+
 static inline const char *
 eal_runtime_config_path(void)
 {

lib/librte_eal/linuxapp/eal/eal.c +67 -0

79 lines not on this page · raw message

Signature · 1 line

2.7.4

Anatoly Burakov

author

2018-04-25 12:24 UTC #5 lore raw

PATCH 4/4 eal: move all runtime data into DPDK runtime dir

Fix all calls to functions in eal_filesystem to produce paths residing inside dedicated DPDK runtime directory.

---
 lib/librte_eal/bsdapp/eal/eal.c        |  2 +
 lib/librte_eal/common/eal_filesystem.h | 71 +++++++++-------------------------
 lib/librte_eal/linuxapp/eal/eal.c      |  2 +
 3 files changed, 22 insertions(+), 53 deletions(-)

lib/librte_eal/bsdapp/eal/eal.c +2 -0

index 400c273..e21c1ed 100644
@@ -87,6 +87,8 @@ int rte_cycles_vmware_tsc_map;
 /* platform-specific runtime dir */
 static char runtime_dir[PATH_MAX];
 
+static const char *default_config_dir = "/var/run";
+
 int
 eal_create_runtime_dir(void)
 {

lib/librte_eal/common/eal_filesystem.h +18 ~~-53~~

134 lines not on this page · raw message

lib/librte_eal/linuxapp/eal/eal.c +2 -0

index 0e2672b..072d34c 100644
@@ -95,6 +95,8 @@ int rte_cycles_vmware_tsc_map;
 /* platform-specific runtime dir */
 static char runtime_dir[PATH_MAX];
 
+static const char *default_config_dir = "/var/run";
+
 int
 eal_create_runtime_dir(void)
 {

Signature · 1 line

2.7.4

Anatoly Burakov

author

2018-04-30 12:08 UTC #6 lore raw

v2 cover Clean up EAL runtime data paths

[1] http://dpdk.org/dev/patchwork/patch/38688/

As has been suggested [1], all DPDK runtime paths should be put into a single place. This patchset accomplishes exactly that.

v2:
- Rebase on rc1

Anatoly Burakov (4):
eal: remove unused define
eal: rename function returning hugepage data path
eal: add directory for DPDK runtime data
eal: move all runtime data into DPDK runtime dir

 lib/librte_eal/bsdapp/eal/eal.c          | 70 +++++++++++++++++++++++++++
 lib/librte_eal/common/eal_filesystem.h   | 81 ++++++++++----------------------
 lib/librte_eal/linuxapp/eal/eal.c        | 69 +++++++++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/eal_memory.c | 10 ++--
 4 files changed, 171 insertions(+), 59 deletions(-)

Signature · 1 line

2.7.4

Anatoly Burakov

author

2018-04-30 12:08 UTC #7 lore raw

PATCH v2 1/4 eal: remove unused define

The define was a leftover from IVSHMEM library.

Fixes c711ccb30987 ("ivshmem: remove library and its EAL integration")

Cc [email protected]

Reviewed-by David Marchand <[email protected]>

---
 lib/librte_eal/common/eal_filesystem.h | 2 --
 1 file changed, 2 deletions(-)

lib/librte_eal/common/eal_filesystem.h +0 -2

index 4db5c10..078e2eb 100644
@@ -104,8 +104,6 @@ eal_hugepage_file_path(void)
 
 /** String format for hugepage map files. */
 #define HUGEFILE_FMT "%s/%smap_%d"
-#define TEMP_HUGEFILE_FMT "%s/%smap_temp_%d"
-
 static inline const char *
 eal_get_hugefile_path(char *buffer, size_t buflen, const char *hugedir, int f_id)
 {

Signature · 1 line

2.7.4

Anatoly Burakov

author

2018-04-30 12:08 UTC #8 lore raw

PATCH v2 2/4 eal: rename function returning hugepage data path

The original name for this path was not too descriptive and confusing. Rename it to a more appropriate and descriptive name:
it stores data about hugepages, so name it eal_hugepage_data_path().

---
 lib/librte_eal/common/eal_filesystem.h   |  2 +-
 lib/librte_eal/linuxapp/eal/eal_memory.c | 10 ++++++----
 2 files changed, 7 insertions(+), 5 deletions(-)

lib/librte_eal/common/eal_filesystem.h +1 -1

index 078e2eb..060ac2b 100644
@@ -89,7 +89,7 @@ eal_hugepage_info_path(void)
 #define HUGEPAGE_FILE_FMT "%s/.%s_hugepage_file"
 
 static inline const char *
-eal_hugepage_file_path(void)
+eal_hugepage_data_path(void)
 {
 	static char buffer[PATH_MAX]; /* static so auto-zeroed */
 	const char *directory = default_config_dir;

lib/librte_eal/linuxapp/eal/eal_memory.c +6 -4

index e0baabb..c917de1 100644
@@ -1499,7 +1499,7 @@ eal_legacy_hugepage_init(void)
 	}
 
 	/* create shared memory */
-	hugepage = create_shared_memory(eal_hugepage_file_path(),
+	hugepage = create_shared_memory(eal_hugepage_data_path(),
 			nr_hugefiles * sizeof(struct hugepage_file));
 
 	if (hugepage == NULL) {

@@ -1727,16 +1727,18 @@ eal_legacy_hugepage_attach(void)
 
 	test_phys_addrs_available();
 
-	fd_hugepage = open(eal_hugepage_file_path(), O_RDONLY);
+	fd_hugepage = open(eal_hugepage_data_path(), O_RDONLY);
 	if (fd_hugepage < 0) {
-		RTE_LOG(ERR, EAL, "Could not open %s\n", eal_hugepage_file_path());
+		RTE_LOG(ERR, EAL, "Could not open %s\n",
+				eal_hugepage_data_path());
 		goto error;
 	}
 
 	size = getFileSize(fd_hugepage);
 	hp = mmap(NULL, size, PROT_READ, MAP_PRIVATE, fd_hugepage, 0);
 	if (hp == MAP_FAILED) {
-		RTE_LOG(ERR, EAL, "Could not mmap %s\n", eal_hugepage_file_path());
+		RTE_LOG(ERR, EAL, "Could not mmap %s\n",
+				eal_hugepage_data_path());
 		goto error;
 	}

Signature · 1 line

2.7.4

Pattan, Reshma

Reviewed-by on v2 2/4

2018-05-10 13:14 UTC #9 lore raw

Re: [PATCH v2 2/4] eal: rename function returning hugepage data path

Quoted 10 lines

-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Anatoly Burakov
Sent: Monday, April 30, 2018 1:09 PM
To: [email protected]
Cc: Richardson, Bruce <[email protected]>; [email protected]
Subject: [dpdk-dev] [PATCH v2 2/4] eal: rename function returning hugepage data path

Quoted 5 lines

The original name for this path was not too descriptive and confusing. Rename it to a more appropriate and descriptive name:
it stores data about hugepages, so name it eal_hugepage_data_path().
Signed-off-by Anatoly Burakov <[email protected]>

data_path sounds like packet processing data path. eal_hugepage_data_file?. I am ok anyway :-).

Anatoly Burakov

author

2018-04-30 12:08 UTC #10 lore raw

PATCH v2 3/4 eal: add directory for DPDK runtime data

<base path>/dpdk/<DPDK prefix>/

There is no equivalent of "mkdir -p", so we will be creating the path step by step.

Nothing uses this new path yet, changes for that will come in next commit.

---
 lib/librte_eal/bsdapp/eal/eal.c        | 68 ++++++++++++++++++++++++++++++++++
 lib/librte_eal/common/eal_filesystem.h |  8 ++++
 lib/librte_eal/linuxapp/eal/eal.c      | 67 +++++++++++++++++++++++++++++++++
 3 files changed, 143 insertions(+)

lib/librte_eal/bsdapp/eal/eal.c +68 -0

index a63f11f..256ab2d 100644
@@ -18,6 +18,7 @@
 #include <limits.h>
 #include <sys/mman.h>
 #include <sys/queue.h>
+#include <sys/stat.h>
 
 #include <rte_compat.h>
 #include <rte_common.h>

@@ -83,6 +84,66 @@ struct internal_config internal_config;
 /* used by rte_rdtsc() */
 int rte_cycles_vmware_tsc_map;
 
+/* platform-specific runtime dir */
+static char runtime_dir[PATH_MAX];
+
+int
+eal_create_runtime_dir(void)
+{
+	const char *directory = default_config_dir;
+	const char *xdg_runtime_dir = getenv("XDG_RUNTIME_DIR");
+	const char *fallback = "/tmp";
+	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(runtime_dir, sizeof(runtime_dir), "%s/%s",
+			tmp, internal_config.hugefile_prefix);
+	if (ret < 0 || ret == sizeof(runtime_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, 0600);
+	if (ret < 0 && errno != EEXIST) {
+		RTE_LOG(ERR, EAL, "Error creating '%s': %s\n",
+			tmp, strerror(errno));
+		return -1;
+	}
+
+	ret = mkdir(runtime_dir, 0600);
+	if (ret < 0 && errno != EEXIST) {
+		RTE_LOG(ERR, EAL, "Error creating '%s': %s\n",
+			runtime_dir, strerror(errno));
+		return -1;
+	}
+
+	return 0;
+}
+
+const char *
+eal_get_runtime_dir(void)
+{
+	return runtime_dir;
+}
+
 /* Return user provided mbuf pool ops name */
 const char * __rte_experimental
 rte_eal_mbuf_user_pool_ops(void)

@@ -522,6 +583,13 @@ rte_eal_init(int argc, char **argv)
 	/* set log level as early as possible */
 	eal_log_level_parse(argc, argv);
 
+	/* create runtime data directory */
+	if (eal_create_runtime_dir() < 0) {
+		rte_eal_init_alert("Cannot create runtime directory\n");
+		rte_errno = EACCES;
+		return -1;
+	}
+
 	if (rte_eal_cpu_init() < 0) {
 		rte_eal_init_alert("Cannot detect lcores.");
 		rte_errno = ENOTSUP;

lib/librte_eal/common/eal_filesystem.h +8 -0

index 060ac2b..67f5ca8 100644
@@ -25,6 +25,14 @@
 
 static const char *default_config_dir = "/var/run";
 
+/* sets up platform-specific runtime data dir */
+int
+eal_create_runtime_dir(void);
+
+/* returns runtime dir */
+const char *
+eal_get_runtime_dir(void);
+
 static inline const char *
 eal_runtime_config_path(void)
 {

lib/librte_eal/linuxapp/eal/eal.c +67 -0

79 lines not on this page · raw message

Signature · 1 line

2.7.4

Pattan, Reshma

Reviewed-by Reshma Pattan on v2 3/4

2018-05-10 14:05 UTC #11 lore raw

Re: [PATCH v2 3/4] eal: add directory for DPDK runtime data

Hi,

Quoted 24 lines

-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Anatoly Burakov
Sent: Monday, April 30, 2018 1:09 PM
To: [email protected]
Cc: Richardson, Bruce <[email protected]>; [email protected]
Subject: [dpdk-dev] [PATCH v2 3/4] eal: add directory for DPDK runtime data
Currently, during runtime, DPDK will store a bunch of files here and there (in /var/run, /tmp or in $HOME). Fix it by creating a DPDK-specific runtime directory, under which all runtime data will be placed. The template for creating this runtime directory is the following:
<base path>/dpdk/<DPDK prefix>/
Where <base path> is set to either "/var/run" if run as root, or $XDG_RUNTIME_DIR if run as non-root, with a fallback to /tmp if $XDG_RUNTIME_DIR is not defined. So, for example, if run as root, by default all runtime data will be stored at /var/run/dpdk/rte/.
There is no equivalent of "mkdir -p", so we will be creating the path step by step.
Nothing uses this new path yet, changes for that will come in next commit.

Quoted 9 lines

---
 lib/librte_eal/bsdapp/eal/eal.c        | 68
++++++++++++++++++++++++++++++++++
 lib/librte_eal/common/eal_filesystem.h |  8 ++++
 lib/librte_eal/linuxapp/eal/eal.c      | 67
+++++++++++++++++++++++++++++++++
 3 files changed, 143 insertions(+)

Looks good to me.

Anatoly Burakov

author

2018-04-30 12:08 UTC #12 lore raw

PATCH v2 4/4 eal: move all runtime data into DPDK runtime dir

Fix all calls to functions in eal_filesystem to produce paths residing inside dedicated DPDK runtime directory.

---
 lib/librte_eal/bsdapp/eal/eal.c        |  2 +
 lib/librte_eal/common/eal_filesystem.h | 71 +++++++++-------------------------
 lib/librte_eal/linuxapp/eal/eal.c      |  2 +
 3 files changed, 22 insertions(+), 53 deletions(-)

lib/librte_eal/bsdapp/eal/eal.c +2 -0

index 256ab2d..ebda2ef 100644
@@ -87,6 +87,8 @@ int rte_cycles_vmware_tsc_map;
 /* platform-specific runtime dir */
 static char runtime_dir[PATH_MAX];
 
+static const char *default_config_dir = "/var/run";
+
 int
 eal_create_runtime_dir(void)
 {

lib/librte_eal/common/eal_filesystem.h +18 ~~-53~~

134 lines not on this page · raw message

lib/librte_eal/linuxapp/eal/eal.c +2 -0

index 053b7e7..4ccb523 100644
@@ -95,6 +95,8 @@ int rte_cycles_vmware_tsc_map;
 /* platform-specific runtime dir */
 static char runtime_dir[PATH_MAX];
 
+static const char *default_config_dir = "/var/run";
+
 int
 eal_create_runtime_dir(void)
 {

Signature · 1 line

2.7.4

Van Haaren, Harry

2018-05-09 15:59 UTC #13 lore raw

[1] https://github.com/collectd/collectd/blob/master/src/utils_dpdk.c#L46

Quoted 16 lines

From: dev [mailto:[email protected]] On Behalf Of Anatoly Burakov
Sent: Monday, April 30, 2018 1:09 PM
To: [email protected]
Cc: Richardson, Bruce <[email protected]>; [email protected]
Subject: [dpdk-dev] [PATCH v2 0/4] Clean up EAL runtime data paths
As has been suggested [1], all DPDK runtime paths should be put into a single place. This patchset accomplishes exactly that.
If running as root, all files will be put under /var/run/dpdk/<prefix>, otherwise they will be put under $XDG_RUNTIME_PATH/dpdk/<prefix>, or, if that environment variable is not defined, all files will go under /tmp/dpdk/<prefix>.
[1] http://dpdk.org/dev/patchwork/patch/38688/

Quoted 8 lines

v2:
- Rebase on rc1
Anatoly Burakov (4):
eal: remove unused define
eal: rename function returning hugepage data path
eal: add directory for DPDK runtime data
eal: move all runtime data into DPDK runtime dir

<snip>

No full code review, high level comments:

We have to be careful in changing /var/run/.rte_config, which has always been the default DPDK primary application lockfile. This has been used to identify if a primary DPDK application is alive (see rte_eal_primary_proc_alive()) and possibly the write-lock on this file is checked by other tools/utilities directly without any DPDK function call.

Changing the filepath just before a release isn't a good idea - we should treat this as an ABI/API break, as the change will break functionality in other projects such as CollectD[1], which (by default ;) rely on the defaults. There is a config file for CollectD to manually override the location, but this will cause headaches from a usability POV.

I'm not opposed to the change - particularly as I gather the new memory subsystem causes a number of lockfiles to be created - but we must do our due diligence and give other projects fair-warning that this change is coming.

As such, I recommend this patchset in its current form (particularly patches 2,3,4) to be deferred past 18.05.

-Harry

Bruce Richardson

2018-05-09 16:11 UTC #14 lore raw

Quoted 42 lines · Harry

Quoted 16 lines
From: dev [mailto:[email protected]] On Behalf Of Anatoly Burakov
Sent: Monday, April 30, 2018 1:09 PM
To: [email protected]
Cc: Richardson, Bruce <[email protected]>; [email protected]
Subject: [dpdk-dev] [PATCH v2 0/4] Clean up EAL runtime data paths
As has been suggested [1], all DPDK runtime paths should be put into a single place. This patchset accomplishes exactly that.
If running as root, all files will be put under /var/run/dpdk/<prefix>, otherwise they will be put under $XDG_RUNTIME_PATH/dpdk/<prefix>, or, if that environment variable is not defined, all files will go under /tmp/dpdk/<prefix>.
[1] http://dpdk.org/dev/patchwork/patch/38688/
Quoted 8 lines
v2:
- Rebase on rc1
Anatoly Burakov (4):
eal: remove unused define
eal: rename function returning hugepage data path
eal: add directory for DPDK runtime data
eal: move all runtime data into DPDK runtime dir
<snip>
No full code review, high level comments:
We have to be careful in changing /var/run/.rte_config, which has always been the default DPDK primary application lockfile. This has been used to identify if a primary DPDK application is alive (see rte_eal_primary_proc_alive()) and possibly the write-lock on this file is checked by other tools/utilities directly without any DPDK function call.
Changing the filepath just before a release isn't a good idea - we should treat this as an ABI/API break, as the change will break functionality in other projects such as CollectD[1], which (by default ;) rely on the defaults. There is a config file for CollectD to manually override the location, but this will cause headaches from a usability POV.

Quoted 8 lines

I'm not opposed to the change - particularly as I gather the new memory subsystem causes a number of lockfiles to be created - but we must do our due diligence and give other projects fair-warning that this change is coming.
As such, I recommend this patchset in its current form (particularly patches 2,3,4) to be deferred past 18.05.

What about if we keep the .rte_config file in the same place but move the rest? The number of new files causes quite a bit of clutter. We can then have a deprecation notice for the move in 18.05 and finish the cleanup in 18.08.

/Bruce

Thomas Monjalon

2018-05-09 19:03 UTC #15 lore raw

09/05/2018 18:11, Bruce Richardson:

Quoted 47 lines · Harry

Quoted 46 lines
Quoted 16 lines
From: dev [mailto:[email protected]] On Behalf Of Anatoly Burakov
Sent: Monday, April 30, 2018 1:09 PM
To: [email protected]
Cc: Richardson, Bruce <[email protected]>; [email protected]
Subject: [dpdk-dev] [PATCH v2 0/4] Clean up EAL runtime data paths
As has been suggested [1], all DPDK runtime paths should be put into a single place. This patchset accomplishes exactly that.
If running as root, all files will be put under /var/run/dpdk/<prefix>, otherwise they will be put under $XDG_RUNTIME_PATH/dpdk/<prefix>, or, if that environment variable is not defined, all files will go under /tmp/dpdk/<prefix>.
[1] http://dpdk.org/dev/patchwork/patch/38688/
Quoted 8 lines
v2:
- Rebase on rc1
Anatoly Burakov (4):
eal: remove unused define
eal: rename function returning hugepage data path
eal: add directory for DPDK runtime data
eal: move all runtime data into DPDK runtime dir
<snip>
No full code review, high level comments:
We have to be careful in changing /var/run/.rte_config, which has always been the default DPDK primary application lockfile. This has been used to identify if a primary DPDK application is alive (see rte_eal_primary_proc_alive()) and possibly the write-lock on this file is checked by other tools/utilities directly without any DPDK function call.
Changing the filepath just before a release isn't a good idea - we should treat this as an ABI/API break, as the change will break functionality in other projects such as CollectD[1], which (by default ;) rely on the defaults. There is a config file for CollectD to manually override the location, but this will cause headaches from a usability POV.
I'm not opposed to the change - particularly as I gather the new memory subsystem causes a number of lockfiles to be created - but we must do our due diligence and give other projects fair-warning that this change is coming.

Quoted 8 lines

Quoted 4 lines
As such, I recommend this patchset in its current form (particularly patches 2,3,4) to be deferred past 18.05.
What about if we keep the .rte_config file in the same place but move the rest? The number of new files causes quite a bit of clutter. We can then have a deprecation notice for the move in 18.05 and finish the cleanup in 18.08.

I agree

Burakov, Anatoly

author

2018-05-14 08:26 UTC #16 lore raw

Quoted 52 lines · Thomas Monjalon

09/05/2018 18:11, Bruce Richardson:
Quoted 51 lines · Harry
Quoted 50 lines
Quoted 16 lines
From: dev [mailto:[email protected]] On Behalf Of Anatoly Burakov
Sent: Monday, April 30, 2018 1:09 PM
To: [email protected]
Cc: Richardson, Bruce <[email protected]>; [email protected]
Subject: [dpdk-dev] [PATCH v2 0/4] Clean up EAL runtime data paths
As has been suggested [1], all DPDK runtime paths should be put into a single place. This patchset accomplishes exactly that.
If running as root, all files will be put under /var/run/dpdk/<prefix>, otherwise they will be put under $XDG_RUNTIME_PATH/dpdk/<prefix>, or, if that environment variable is not defined, all files will go under /tmp/dpdk/<prefix>.
[1] http://dpdk.org/dev/patchwork/patch/38688/
Quoted 8 lines
v2:
- Rebase on rc1
Anatoly Burakov (4):
eal: remove unused define
eal: rename function returning hugepage data path
eal: add directory for DPDK runtime data
eal: move all runtime data into DPDK runtime dir
<snip>
No full code review, high level comments:
We have to be careful in changing /var/run/.rte_config, which has always been the default DPDK primary application lockfile. This has been used to identify if a primary DPDK application is alive (see rte_eal_primary_proc_alive()) and possibly the write-lock on this file is checked by other tools/utilities directly without any DPDK function call.
Changing the filepath just before a release isn't a good idea - we should treat this as an ABI/API break, as the change will break functionality in other projects such as CollectD[1], which (by default ;) rely on the defaults. There is a config file for CollectD to manually override the location, but this will cause headaches from a usability POV.
I'm not opposed to the change - particularly as I gather the new memory subsystem causes a number of lockfiles to be created - but we must do our due diligence and give other projects fair-warning that this change is coming.
As such, I recommend this patchset in its current form (particularly patches 2,3,4) to be deferred past 18.05.

Quoted 7 lines

Quoted 4 lines
What about if we keep the .rte_config file in the same place but move the rest? The number of new files causes quite a bit of clutter. We can then have a deprecation notice for the move in 18.05 and finish the cleanup in 18.08.
I agree

Thanks for catching this, Harry. I'll resubmit the patches then, along with a deprecation notice about /var/run/.<prefix>_config.

Signature · 2 lines

Thanks,
Anatoly

Anatoly Burakov

author

2018-05-14 16:27 UTC #17 lore raw

v3 cover Clean up EAL runtime data paths

[1] http://dpdk.org/dev/patchwork/patch/38688/

As has been suggested [1], all DPDK runtime paths should be put into a single place. This patchset accomplishes exactly that.

v3:
- Rebase on rc2
- Leave rte_config path in place, to be fixed for next release

v2:
- Rebase on rc1

Anatoly Burakov (4):
eal: remove unused define
eal: rename function returning hugepage data path
eal: add directory for DPDK runtime data
eal: move all runtime data into DPDK runtime dir

 lib/librte_eal/bsdapp/eal/eal.c          | 70 +++++++++++++++++++++++++++++++
 lib/librte_eal/common/eal_filesystem.h   | 71 +++++++++++---------------------
 lib/librte_eal/linuxapp/eal/eal.c        | 69 +++++++++++++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/eal_memory.c | 10 +++--
 4 files changed, 169 insertions(+), 51 deletions(-)

Signature · 1 line

2.7.4

Anatoly Burakov

author

2018-05-14 16:27 UTC #18 lore raw

PATCH v3 1/4 eal: remove unused define

The define was a leftover from IVSHMEM library.

Fixes c711ccb30987 ("ivshmem: remove library and its EAL integration")

Cc [email protected]

Reviewed-by David Marchand <[email protected]>

---
 lib/librte_eal/common/eal_filesystem.h | 2 --
 1 file changed, 2 deletions(-)

lib/librte_eal/common/eal_filesystem.h +0 -2

index 4db5c10..078e2eb 100644
@@ -104,8 +104,6 @@ eal_hugepage_file_path(void)
 
 /** String format for hugepage map files. */
 #define HUGEFILE_FMT "%s/%smap_%d"
-#define TEMP_HUGEFILE_FMT "%s/%smap_temp_%d"
-
 static inline const char *
 eal_get_hugefile_path(char *buffer, size_t buflen, const char *hugedir, int f_id)
 {

Signature · 1 line

2.7.4

Anatoly Burakov

author

2018-05-14 16:27 UTC #19 lore raw

PATCH v3 2/4 eal: rename function returning hugepage data path

The original name for this path was not too descriptive and confusing. Rename it to a more appropriate and descriptive name:
it stores data about hugepages, so name it eal_hugepage_data_path().

---
 lib/librte_eal/common/eal_filesystem.h   |  2 +-
 lib/librte_eal/linuxapp/eal/eal_memory.c | 10 ++++++----
 2 files changed, 7 insertions(+), 5 deletions(-)

lib/librte_eal/common/eal_filesystem.h +1 -1

index 078e2eb..060ac2b 100644
@@ -89,7 +89,7 @@ eal_hugepage_info_path(void)
 #define HUGEPAGE_FILE_FMT "%s/.%s_hugepage_file"
 
 static inline const char *
-eal_hugepage_file_path(void)
+eal_hugepage_data_path(void)
 {
 	static char buffer[PATH_MAX]; /* static so auto-zeroed */
 	const char *directory = default_config_dir;

lib/librte_eal/linuxapp/eal/eal_memory.c +6 -4

index e0baabb..c917de1 100644
@@ -1499,7 +1499,7 @@ eal_legacy_hugepage_init(void)
 	}
 
 	/* create shared memory */
-	hugepage = create_shared_memory(eal_hugepage_file_path(),
+	hugepage = create_shared_memory(eal_hugepage_data_path(),
 			nr_hugefiles * sizeof(struct hugepage_file));
 
 	if (hugepage == NULL) {

@@ -1727,16 +1727,18 @@ eal_legacy_hugepage_attach(void)
 
 	test_phys_addrs_available();
 
-	fd_hugepage = open(eal_hugepage_file_path(), O_RDONLY);
+	fd_hugepage = open(eal_hugepage_data_path(), O_RDONLY);
 	if (fd_hugepage < 0) {
-		RTE_LOG(ERR, EAL, "Could not open %s\n", eal_hugepage_file_path());
+		RTE_LOG(ERR, EAL, "Could not open %s\n",
+				eal_hugepage_data_path());
 		goto error;
 	}
 
 	size = getFileSize(fd_hugepage);
 	hp = mmap(NULL, size, PROT_READ, MAP_PRIVATE, fd_hugepage, 0);
 	if (hp == MAP_FAILED) {
-		RTE_LOG(ERR, EAL, "Could not mmap %s\n", eal_hugepage_file_path());
+		RTE_LOG(ERR, EAL, "Could not mmap %s\n",
+				eal_hugepage_data_path());
 		goto error;
 	}

Signature · 1 line

2.7.4

Anatoly Burakov

author

2018-05-14 16:27 UTC #20 lore raw

PATCH v3 3/4 eal: add directory for DPDK runtime data

<base path>/dpdk/<DPDK prefix>/

There is no equivalent of "mkdir -p", so we will be creating the path step by step.

Nothing uses this new path yet, changes for that will come in next commit.

---
 lib/librte_eal/bsdapp/eal/eal.c        | 68 ++++++++++++++++++++++++++++++++++
 lib/librte_eal/common/eal_filesystem.h |  8 ++++
 lib/librte_eal/linuxapp/eal/eal.c      | 67 +++++++++++++++++++++++++++++++++
 3 files changed, 143 insertions(+)

lib/librte_eal/bsdapp/eal/eal.c +68 -0

index 3ba328a..de88b6a 100644
@@ -18,6 +18,7 @@
 #include <limits.h>
 #include <sys/mman.h>
 #include <sys/queue.h>
+#include <sys/stat.h>
 
 #include <rte_compat.h>
 #include <rte_common.h>

@@ -83,6 +84,66 @@ struct internal_config internal_config;
 /* used by rte_rdtsc() */
 int rte_cycles_vmware_tsc_map;
 
+/* platform-specific runtime dir */
+static char runtime_dir[PATH_MAX];
+
+int
+eal_create_runtime_dir(void)
+{
+	const char *directory = default_config_dir;
+	const char *xdg_runtime_dir = getenv("XDG_RUNTIME_DIR");
+	const char *fallback = "/tmp";
+	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(runtime_dir, sizeof(runtime_dir), "%s/%s",
+			tmp, internal_config.hugefile_prefix);
+	if (ret < 0 || ret == sizeof(runtime_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, 0600);
+	if (ret < 0 && errno != EEXIST) {
+		RTE_LOG(ERR, EAL, "Error creating '%s': %s\n",
+			tmp, strerror(errno));
+		return -1;
+	}
+
+	ret = mkdir(runtime_dir, 0600);
+	if (ret < 0 && errno != EEXIST) {
+		RTE_LOG(ERR, EAL, "Error creating '%s': %s\n",
+			runtime_dir, strerror(errno));
+		return -1;
+	}
+
+	return 0;
+}
+
+const char *
+eal_get_runtime_dir(void)
+{
+	return runtime_dir;
+}
+
 /* Return user provided mbuf pool ops name */
 const char * __rte_experimental
 rte_eal_mbuf_user_pool_ops(void)

@@ -523,6 +584,13 @@ rte_eal_init(int argc, char **argv)
 	/* set log level as early as possible */
 	eal_log_level_parse(argc, argv);
 
+	/* create runtime data directory */
+	if (eal_create_runtime_dir() < 0) {
+		rte_eal_init_alert("Cannot create runtime directory\n");
+		rte_errno = EACCES;
+		return -1;
+	}
+
 	if (rte_eal_cpu_init() < 0) {
 		rte_eal_init_alert("Cannot detect lcores.");
 		rte_errno = ENOTSUP;

lib/librte_eal/common/eal_filesystem.h +8 -0

index 060ac2b..67f5ca8 100644
@@ -25,6 +25,14 @@
 
 static const char *default_config_dir = "/var/run";
 
+/* sets up platform-specific runtime data dir */
+int
+eal_create_runtime_dir(void);
+
+/* returns runtime dir */
+const char *
+eal_get_runtime_dir(void);
+
 static inline const char *
 eal_runtime_config_path(void)
 {

lib/librte_eal/linuxapp/eal/eal.c +67 -0

79 lines not on this page · raw message

Signature · 1 line

2.7.4

Anatoly Burakov

author

2018-05-14 16:27 UTC #21 lore raw

PATCH v3 4/4 eal: move all runtime data into DPDK runtime dir

Fix all calls to functions in eal_filesystem to produce paths residing inside dedicated DPDK runtime directory. Leaving DPDK runtime config in place as 3rd-party applications within the DPDK ecosystem might rely on this path to determine whether DPDK is running, so moving that will be postponed to the next release cycle.

---

Notes:
v3:
- Leave rte_config path as is

 lib/librte_eal/bsdapp/eal/eal.c        |  4 ++-
 lib/librte_eal/common/eal_filesystem.h | 61 +++++++++-------------------------
 lib/librte_eal/linuxapp/eal/eal.c      |  4 ++-
 3 files changed, 22 insertions(+), 47 deletions(-)

lib/librte_eal/bsdapp/eal/eal.c +3 -1

index de88b6a..c890bfe 100644
@@ -87,10 +87,12 @@ int rte_cycles_vmware_tsc_map;
 /* platform-specific runtime dir */
 static char runtime_dir[PATH_MAX];
 
+static const char *default_runtime_dir = "/var/run";
+
 int
 eal_create_runtime_dir(void)
 {
-	const char *directory = default_config_dir;
+	const char *directory = default_runtime_dir;
 	const char *xdg_runtime_dir = getenv("XDG_RUNTIME_DIR");
 	const char *fallback = "/tmp";
 	char tmp[PATH_MAX];

lib/librte_eal/common/eal_filesystem.h +16 ~~-45~~

121 lines not on this page · raw message

lib/librte_eal/linuxapp/eal/eal.c +3 -1

index 6048063..bed5823 100644
@@ -95,10 +95,12 @@ int rte_cycles_vmware_tsc_map;
 /* platform-specific runtime dir */
 static char runtime_dir[PATH_MAX];
 
+static const char *default_runtime_dir = "/var/run";
+
 int
 eal_create_runtime_dir(void)
 {
-	const char *directory = default_config_dir;
+	const char *directory = default_runtime_dir;
 	const char *xdg_runtime_dir = getenv("XDG_RUNTIME_DIR");
 	const char *fallback = "/tmp";
 	char tmp[PATH_MAX];

Signature · 1 line

2.7.4

Thomas Monjalon

Applied

2018-05-14 22:45 UTC #22 lore raw