[1/2] net/mlx5: improve socket file path

Message ID 20241213092444.2987-1-ming.1.yang@nokia-sbell.com (mailing list archive)
State New
Delegated to: Raslan Darawsheh
Headers
Series [1/2] net/mlx5: improve socket file path |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Yang Ming Dec. 13, 2024, 9:24 a.m. UTC
1. /var/tmp is hard code which is not a good style
2. /var/tmp may be not allowed to be written via container's
read only mode.

Signed-off-by: Yang Ming <ming.1.yang@nokia-sbell.com>
---
 drivers/net/mlx5/linux/mlx5_socket.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
  

Comments

Stephen Hemminger Dec. 13, 2024, 5:12 p.m. UTC | #1
On Fri, 13 Dec 2024 17:24:42 +0800
Yang Ming <ming.1.yang@nokia-sbell.com> wrote:

> 1. /var/tmp is hard code which is not a good style
> 2. /var/tmp may be not allowed to be written via container's
> read only mode.
> 
> Signed-off-by: Yang Ming <ming.1.yang@nokia-sbell.com>

Since this is a unix domain socket, why not use abstract socket
that doesn't have to be associated with filesystem?
  
Bruce Richardson Dec. 13, 2024, 5:16 p.m. UTC | #2
On Fri, Dec 13, 2024 at 09:12:39AM -0800, Stephen Hemminger wrote:
> On Fri, 13 Dec 2024 17:24:42 +0800
> Yang Ming <ming.1.yang@nokia-sbell.com> wrote:
> 
> > 1. /var/tmp is hard code which is not a good style
> > 2. /var/tmp may be not allowed to be written via container's
> > read only mode.
> > 
> > Signed-off-by: Yang Ming <ming.1.yang@nokia-sbell.com>
> 
> Since this is a unix domain socket, why not use abstract socket
> that doesn't have to be associated with filesystem?

In general, I think we should avoid abstract sockets in DPDK. Primary
reason is that they are linux-specific. Last time I checked other unixes,
like BSD, don't support them. A secondary concern is that having a
filesystem path allows permission checks, so for e.g. telemetry sockets,
only users with appropriate permissions can connect. With an abstract socket
we'd have to open up the area of user authentication.

/Bruce
  
Yang Ming Jan. 3, 2025, 2:51 a.m. UTC | #3
On 2024/12/14 01:16, Bruce Richardson wrote:
> On Fri, Dec 13, 2024 at 09:12:39AM -0800, Stephen Hemminger wrote:
>> On Fri, 13 Dec 2024 17:24:42 +0800
>> Yang Ming<ming.1.yang@nokia-sbell.com>  wrote:
>>
>>> 1. /var/tmp is hard code which is not a good style
>>> 2. /var/tmp may be not allowed to be written via container's
>>> read only mode.
>>>
>>> Signed-off-by: Yang Ming<ming.1.yang@nokia-sbell.com>
>> Since this is a unix domain socket, why not use abstract socket
>> that doesn't have to be associated with filesystem?
> In general, I think we should avoid abstract sockets in DPDK. Primary
> reason is that they are linux-specific. Last time I checked other unixes,
> like BSD, don't support them. A secondary concern is that having a
> filesystem path allows permission checks, so for e.g. telemetry sockets,
> only users with appropriate permissions can connect. With an abstract socket
> we'd have to open up the area of user authentication.
>
> /Bruce
>
Hi Stephen & Bruce,

I'm not sure whether abstract socket is a good idea. Maybe it can be improved further or step by step. But we don't need to discuss it for this commit.
We do this improvement because "/var/tmp" and "/var/log" can't be write in Readonly mode of container except that we add /var/ specfic for DPDK application in container's setting. But nearly all DPDK modules have already used common runtime path returned from `rte_eal_get_runtime_dir()`. Why not we apply this common path for Mellanox NIC?
  

Patch

diff --git a/drivers/net/mlx5/linux/mlx5_socket.c b/drivers/net/mlx5/linux/mlx5_socket.c
index 6ce0e59643..7fa6e5345c 100644
--- a/drivers/net/mlx5/linux/mlx5_socket.c
+++ b/drivers/net/mlx5/linux/mlx5_socket.c
@@ -20,7 +20,7 @@ 
 
 /* PMD socket service for tools. */
 
-#define MLX5_SOCKET_PATH "/var/tmp/dpdk_net_mlx5_%d"
+#define MLX5_SOCKET_FNAME "dpdk_net_mlx5"
 #define MLX5_ALL_PORT_IDS 0xffff
 
 int server_socket = -1; /* Unix socket for primary process. */
@@ -177,8 +177,8 @@  mlx5_pmd_socket_init(void)
 	ret = fcntl(server_socket, F_SETFL, flags | O_NONBLOCK);
 	if (ret < 0)
 		goto close;
-	snprintf(sun.sun_path, sizeof(sun.sun_path), MLX5_SOCKET_PATH,
-		 getpid());
+	snprintf(sun.sun_path, sizeof(sun.sun_path), "%s/%s_%d",
+		 rte_eal_get_runtime_dir(), MLX5_SOCKET_FNAME, getpid());
 	remove(sun.sun_path);
 	ret = bind(server_socket, (const struct sockaddr *)&sun, sizeof(sun));
 	if (ret < 0) {
@@ -223,6 +223,7 @@  mlx5_pmd_socket_uninit(void)
 					  mlx5_pmd_socket_handle, NULL);
 	claim_zero(close(server_socket));
 	server_socket = -1;
-	MKSTR(path, MLX5_SOCKET_PATH, getpid());
+	MKSTR(path, "%s/%s_%d", rte_eal_get_runtime_dir(), MLX5_SOCKET_FNAME,
+	      getpid());
 	claim_zero(remove(path));
 }