[RFC 0/4] move unix domain sockets from /tmp to /run/dpdk

[RFC 0/4] move unix domain sockets from /tmp to /run/dpdk

[Stephen Hemminger]

Current best practice is not to use /tmp for Unix domain sockets.
Move the socket communication to DPDK runtime directory.

To quote systemd:

    /run (traditionally /var/run) where privileged daemons can store
    runtime data, such as communication primitives. This is where your
    daemon should place its sockets.

See:

    http://0pointer.net/blog/projects/tmp.html
    https://en.wikipedia.org/wiki/Filesystem_Hierarchy_Standard
    https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html*

Stephen Hemminger (4):
  net/af_xdp: replace /tmp with /run/dpdk
  examples/vm_power_manager: replace /tmp with /run/dpdk
  net/nfp: replace /tmp with /run/dpdk
  net/cnxk: move socket from /tmp to /run/dpdk

 doc/guides/sample_app_ug/vm_power_management.rst | 13 ++-----------
 drivers/net/af_xdp/rte_eth_af_xdp.c              |  2 +-
 drivers/net/cnxk/cnxk_eswitch.h                  |  2 +-
 drivers/net/nfp/nfp_cpp_bridge.c                 |  4 ++--
 examples/vm_power_manager/channel_manager.h      |  2 +-
 5 files changed, 7 insertions(+), 16 deletions(-)

-- 
2.51.0

[RFC 1/4] net/af_xdp: replace /tmp with /run/dpdk

[Stephen Hemminger]

Applications should not use /tmp to store unix domain sockets.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/af_xdp/rte_eth_af_xdp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
index f2a8fb95d1..74ffb3ba57 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -83,7 +83,7 @@ RTE_LOG_REGISTER_DEFAULT(af_xdp_logtype, NOTICE);
 
 #define ETH_AF_XDP_MP_KEY "afxdp_mp_send_fds"
 
-#define DP_BASE_PATH			"/tmp/afxdp_dp"
+#define DP_BASE_PATH			"/run/dpdk/afxdp"
 #define DP_UDS_SOCK             "afxdp.sock"
 #define DP_XSK_MAP				"xsks_map"
 #define MAX_LONG_OPT_SZ			64
-- 
2.51.0

[RFC 2/4] examples/vm_power_manager: replace /tmp with /run/dpdk

[Stephen Hemminger]

Applications should not use /tmp to store unix domain sockets.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 doc/guides/sample_app_ug/vm_power_management.rst | 13 ++-----------
 examples/vm_power_manager/channel_manager.h      |  2 +-
 2 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/doc/guides/sample_app_ug/vm_power_management.rst b/doc/guides/sample_app_ug/vm_power_management.rst
index 1955140bb3..eb9228f0eb 100644
--- a/doc/guides/sample_app_ug/vm_power_management.rst
+++ b/doc/guides/sample_app_ug/vm_power_management.rst
@@ -195,7 +195,7 @@ The XML structure is as follows:
       <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
    </controller>
    <channel type='unix'>
-      <source mode='bind' path='/tmp/powermonitor/{vm_name}.{channel_num}'/>
+      <source mode='bind' path='/run/dpdk/powermonitor/{vm_name}.{channel_num}'/>
       <target type='virtio' name='virtio.serial.port.poweragent.{vm_channel_num}'/>
       <address type='virtio-serial' controller='0' bus='0' port='{N}'/>
    </channel>
@@ -208,18 +208,9 @@ channel. Likewise, the port value ``{N}`` must be incremented for each
 channel.
 
 On the host, for each channel to appear in the path, ensure the creation
-of the ``/tmp/powermonitor/`` directory and the assignment of ``qemu``
+of the ``/run/dpdk/powermonitor/`` directory and the assignment of ``qemu``
 permissions:
 
-.. code-block:: console
-
-   mkdir /tmp/powermonitor/
-   chown qemu:qemu /tmp/powermonitor
-
-Note that files and directories in ``/tmp`` are generally removed when
-rebooting the host and you may need to perform the previous steps after
-each reboot.
-
 The serial device as it appears on a VM is configured with the target
 element attribute name and must be in the form:
 ``virtio.serial.port.poweragent.{vm_channel_num}``, where
diff --git a/examples/vm_power_manager/channel_manager.h b/examples/vm_power_manager/channel_manager.h
index 6f70539815..5fc93ae0be 100644
--- a/examples/vm_power_manager/channel_manager.h
+++ b/examples/vm_power_manager/channel_manager.h
@@ -22,7 +22,7 @@ extern "C" {
 #define CHANNEL_MGR_DEFAULT_HV_PATH "qemu:///system"
 
 /* File socket directory */
-#define CHANNEL_MGR_SOCKET_PATH     "/tmp/powermonitor/"
+#define CHANNEL_MGR_SOCKET_PATH     "/run/dpdk/powermonitor/"
 
 /* FIFO file name template */
 #define CHANNEL_MGR_FIFO_PATTERN_NAME   "fifo"
-- 
2.51.0

[RFC 3/4] net/nfp: replace /tmp with /run/dpdk

[Stephen Hemminger]

Replace /tmp/0000:00:00 with /run/dpdk/nfp/0000:00:00 for the
unix domain service name.

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

diff --git a/drivers/net/nfp/nfp_cpp_bridge.c b/drivers/net/nfp/nfp_cpp_bridge.c
index da7ea35d62..b8ef60f22c 100644
--- a/drivers/net/nfp/nfp_cpp_bridge.c
+++ b/drivers/net/nfp/nfp_cpp_bridge.c
@@ -334,7 +334,7 @@ nfp_cpp_bridge_service_func(void *args)
 	int datafd;
 	struct nfp_cpp *cpp;
 	const char *pci_name;
-	char socket_handle[14];
+	char socket_handle[108];
 	struct sockaddr address;
 	struct nfp_pf_dev *pf_dev;
 	struct timeval timeout = {1, 0};
@@ -342,7 +342,7 @@ nfp_cpp_bridge_service_func(void *args)
 	pf_dev = args;
 
 	pci_name = strchr(pf_dev->pci_dev->name, ':') + 1;
-	snprintf(socket_handle, sizeof(socket_handle), "/tmp/%s", pci_name);
+	snprintf(socket_handle, sizeof(socket_handle), "/run/dpdk/nfp/%s", pci_name);
 
 	unlink(socket_handle);
 	sockfd = socket(AF_UNIX, SOCK_STREAM, 0);
-- 
2.51.0

[RFC 4/4] net/cnxk: move socket from /tmp to /run/dpdk

[Stephen Hemminger]

Replace /tmp/cxk_rep_ctrl_msg_sock with /run/dpdk/cnxk_ctrl_msg_sock.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/cnxk/cnxk_eswitch.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/cnxk/cnxk_eswitch.h b/drivers/net/cnxk/cnxk_eswitch.h
index 0275e760fb..bef78a95ba 100644
--- a/drivers/net/cnxk/cnxk_eswitch.h
+++ b/drivers/net/cnxk/cnxk_eswitch.h
@@ -12,7 +12,7 @@
 
 #include "cn10k_tx.h"
 
-#define CNXK_ESWITCH_CTRL_MSG_SOCK_PATH "/tmp/cxk_rep_ctrl_msg_sock"
+#define CNXK_ESWITCH_CTRL_MSG_SOCK_PATH "/run/dpdk/cnxk_ctrl_msg_sock"
 #define CNXK_ESWITCH_VLAN_TPID		ROC_ESWITCH_VLAN_TPID
 #define CNXK_REP_ESWITCH_DEV_MZ		"cnxk_eswitch_dev"
 #define CNXK_ESWITCH_MAX_TXQ		256
-- 
2.51.0

[PATCH v2 0/2] drivers: remove use of /tmp

[Stephen Hemminger]

Putting Unix domain sockets in /tmp is not current best practice,
instead use eal_runtime_dir(). This also fixes use of multiple
instances of DPDK, and containers.

Stephen Hemminger (2):
  net/cnxk: move socket from /tmp to runtime dir
  net/nfp: move socket from /tmp to runtime dir

 drivers/net/cnxk/cnxk_eswitch.c  | 29 +++++++++++++++++++++++++----
 drivers/net/cnxk/cnxk_eswitch.h  |  3 ++-
 drivers/net/cnxk/cnxk_rep_msg.c  | 26 ++++++++++++--------------
 drivers/net/nfp/nfp_cpp_bridge.c | 24 +++++++++++++-----------
 4 files changed, 52 insertions(+), 30 deletions(-)

-- 
2.53.0

[PATCH v2 1/2] net/cnxk: move socket from /tmp to runtime dir

[Stephen Hemminger]

The eswitch control message socket was created at a fixed path in
/tmp, which is world-writable and shared across DPDK instances.

Build the path under the EAL runtime directory (e.g. /run/dpdk/<prefix>)
instead, so it follows the configured prefix and is isolated per
instance.

Add cnxk_eswitch_ctrl_msg_sock_addr() to fill the sockaddr_un from the
runtime dir and use it on both the server (bind) and client (connect)
paths.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/cnxk/cnxk_eswitch.c | 29 +++++++++++++++++++++++++----
 drivers/net/cnxk/cnxk_eswitch.h |  3 ++-
 drivers/net/cnxk/cnxk_rep_msg.c | 26 ++++++++++++--------------
 3 files changed, 39 insertions(+), 19 deletions(-)

diff --git a/drivers/net/cnxk/cnxk_eswitch.c b/drivers/net/cnxk/cnxk_eswitch.c
index 7e717a2fbf..0a6482462c 100644
--- a/drivers/net/cnxk/cnxk_eswitch.c
+++ b/drivers/net/cnxk/cnxk_eswitch.c
@@ -9,6 +9,24 @@
 
 #define CNXK_NIX_DEF_SQ_COUNT 512
 
+/* Build the control message unix socket address under the EAL runtime
+ * directory (e.g. /run/dpdk/<prefix>) rather than a fixed path in /tmp.
+ */
+int
+cnxk_eswitch_ctrl_msg_sock_addr(struct sockaddr_un *un)
+{
+	int ret;
+
+	memset(un, 0, sizeof(*un));
+	un->sun_family = AF_UNIX;
+	ret = snprintf(un->sun_path, sizeof(un->sun_path), "%s/%s",
+		       rte_eal_get_runtime_dir(), CNXK_ESWITCH_CTRL_MSG_SOCK_NAME);
+	if (ret < 0 || (size_t)ret >= sizeof(un->sun_path))
+		return -ENAMETOOLONG;
+
+	return 0;
+}
+
 int
 cnxk_eswitch_representor_id(struct cnxk_eswitch_dev *eswitch_dev, uint16_t hw_func,
 			    uint16_t *rep_id)
@@ -122,14 +140,17 @@ cnxk_eswitch_dev_remove(struct rte_pci_device *pci_dev)
 					plt_err("Failed to open socket. err %d", -errno);
 					return -errno;
 				}
-				sun.sun_family = AF_UNIX;
+				rc = cnxk_eswitch_ctrl_msg_sock_addr(&sun);
+				if (rc) {
+					plt_err("Control message socket path too long");
+					close(sock_fd);
+					return rc;
+				}
 				sunlen = sizeof(struct sockaddr_un);
-				strncpy(sun.sun_path, CNXK_ESWITCH_CTRL_MSG_SOCK_PATH,
-					sizeof(sun.sun_path) - 1);
 
 				if (connect(sock_fd, (struct sockaddr *)&sun, sunlen) < 0) {
 					plt_err("Failed to connect socket: %s, err %d",
-						CNXK_ESWITCH_CTRL_MSG_SOCK_PATH, errno);
+						sun.sun_path, errno);
 					close(sock_fd);
 					return -errno;
 				}
diff --git a/drivers/net/cnxk/cnxk_eswitch.h b/drivers/net/cnxk/cnxk_eswitch.h
index 0275e760fb..b35a615edd 100644
--- a/drivers/net/cnxk/cnxk_eswitch.h
+++ b/drivers/net/cnxk/cnxk_eswitch.h
@@ -12,7 +12,7 @@
 
 #include "cn10k_tx.h"
 
-#define CNXK_ESWITCH_CTRL_MSG_SOCK_PATH "/tmp/cxk_rep_ctrl_msg_sock"
+#define CNXK_ESWITCH_CTRL_MSG_SOCK_NAME "cnxk_ctrl_msg_sock"
 #define CNXK_ESWITCH_VLAN_TPID		ROC_ESWITCH_VLAN_TPID
 #define CNXK_REP_ESWITCH_DEV_MZ		"cnxk_eswitch_dev"
 #define CNXK_ESWITCH_MAX_TXQ		256
@@ -179,6 +179,7 @@ cnxk_eswitch_pmd_priv(void)
 }
 
 /* HW Resources */
+int cnxk_eswitch_ctrl_msg_sock_addr(struct sockaddr_un *un);
 int cnxk_eswitch_nix_rsrc_start(struct cnxk_eswitch_dev *eswitch_dev);
 int cnxk_eswitch_representor_id(struct cnxk_eswitch_dev *eswitch_dev, uint16_t hw_func,
 				uint16_t *rep_id);
diff --git a/drivers/net/cnxk/cnxk_rep_msg.c b/drivers/net/cnxk/cnxk_rep_msg.c
index a222e2b5cd..9662213139 100644
--- a/drivers/net/cnxk/cnxk_rep_msg.c
+++ b/drivers/net/cnxk/cnxk_rep_msg.c
@@ -14,8 +14,11 @@
 static void
 close_socket(int fd)
 {
+	struct sockaddr_un un;
+
 	close(fd);
-	unlink(CNXK_ESWITCH_CTRL_MSG_SOCK_PATH);
+	if (cnxk_eswitch_ctrl_msg_sock_addr(&un) == 0)
+		unlink(un.sun_path);
 }
 
 static int
@@ -113,6 +116,7 @@ open_socket_ctrl_channel(void)
 {
 	struct sockaddr_un un;
 	int sock_fd;
+	int ret;
 
 	sock_fd = socket(AF_UNIX, SOCK_STREAM, 0);
 	if (sock_fd < 0) {
@@ -120,26 +124,20 @@ open_socket_ctrl_channel(void)
 		return -1;
 	}
 
-	/* Set unix socket path and bind */
-	memset(&un, 0, sizeof(un));
-	un.sun_family = AF_UNIX;
-
-	if (strlen(CNXK_ESWITCH_CTRL_MSG_SOCK_PATH) > sizeof(un.sun_path) - 1) {
-		plt_err("Server socket path too long: %s", CNXK_ESWITCH_CTRL_MSG_SOCK_PATH);
+	/* Set unix socket path under the runtime dir and bind */
+	ret = cnxk_eswitch_ctrl_msg_sock_addr(&un);
+	if (ret) {
+		plt_err("Server socket path too long");
 		close(sock_fd);
-		return -E2BIG;
+		return ret;
 	}
 
-	if (remove(CNXK_ESWITCH_CTRL_MSG_SOCK_PATH) == -1 && errno != ENOENT) {
-		plt_err("remove-%s", CNXK_ESWITCH_CTRL_MSG_SOCK_PATH);
+	if (remove(un.sun_path) == -1 && errno != ENOENT) {
+		plt_err("remove-%s", un.sun_path);
 		close(sock_fd);
 		return -errno;
 	}
 
-	memset(&un, 0, sizeof(struct sockaddr_un));
-	un.sun_family = AF_UNIX;
-	strncpy(un.sun_path, CNXK_ESWITCH_CTRL_MSG_SOCK_PATH, sizeof(un.sun_path) - 1);
-
 	if (bind(sock_fd, (struct sockaddr *)&un, sizeof(un)) < 0) {
 		plt_err("Failed to bind %s: %s", un.sun_path, strerror(errno));
 		close(sock_fd);
-- 
2.53.0

[PATCH v2 2/2] net/nfp: move socket from /tmp to runtime dir

[Stephen Hemminger]

The CPP bridge service created its unix socket at a fixed /tmp/<pci>
path, which is world-writable and shared across DPDK instances. Build
the path under the EAL runtime directory (e.g. /run/dpdk/<prefix>)
instead, so it follows the configured prefix and is isolated per
instance.

The old code copied the path into a struct sockaddr, whose sa_data is
only 14 bytes, and bound sizeof(struct sockaddr); a runtime-dir path
does not fit there. Use struct sockaddr_un so the full path fits, and
bail out if it is still truncated.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/nfp/nfp_cpp_bridge.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/net/nfp/nfp_cpp_bridge.c b/drivers/net/nfp/nfp_cpp_bridge.c
index da7ea35d62..c307eca730 100644
--- a/drivers/net/nfp/nfp_cpp_bridge.c
+++ b/drivers/net/nfp/nfp_cpp_bridge.c
@@ -7,6 +7,9 @@
 
 #include <unistd.h>
 #include <sys/ioctl.h>
+#include <sys/un.h>
+
+#include <rte_eal.h>
 
 #include "nfpcore/nfp_cpp.h"
 #include "nfp_logs.h"
@@ -334,17 +337,22 @@ nfp_cpp_bridge_service_func(void *args)
 	int datafd;
 	struct nfp_cpp *cpp;
 	const char *pci_name;
-	char socket_handle[14];
-	struct sockaddr address;
+	struct sockaddr_un address = { .sun_family = AF_UNIX };
 	struct nfp_pf_dev *pf_dev;
 	struct timeval timeout = {1, 0};
 
 	pf_dev = args;
 
 	pci_name = strchr(pf_dev->pci_dev->name, ':') + 1;
-	snprintf(socket_handle, sizeof(socket_handle), "/tmp/%s", pci_name);
 
-	unlink(socket_handle);
+	ret = snprintf(address.sun_path, sizeof(address.sun_path), "%s/nfp_%s",
+			rte_eal_get_runtime_dir(), pci_name);
+	if (ret < 0 || (size_t)ret >= sizeof(address.sun_path)) {
+		PMD_CPP_LOG(ERR, "Socket path too long. Service failed.");
+		return -EINVAL;
+	}
+
+	unlink(address.sun_path);
 	sockfd = socket(AF_UNIX, SOCK_STREAM, 0);
 	if (sockfd < 0) {
 		PMD_CPP_LOG(ERR, "Socket creation error. Service failed.");
@@ -353,13 +361,7 @@ nfp_cpp_bridge_service_func(void *args)
 
 	setsockopt(sockfd, SOL_SOCKET, SO_RCVTIMEO, (char *)&timeout, sizeof(timeout));
 
-	memset(&address, 0, sizeof(struct sockaddr));
-
-	address.sa_family = AF_UNIX;
-	strcpy(address.sa_data, socket_handle);
-
-	ret = bind(sockfd, (const struct sockaddr *)&address,
-			sizeof(struct sockaddr));
+	ret = bind(sockfd, (const struct sockaddr *)&address, sizeof(address));
 	if (ret < 0) {
 		PMD_CPP_LOG(ERR, "Bind error (%d). Service failed.", errno);
 		close(sockfd);
-- 
2.53.0