* [dpdk-dev] [PATCH] eal: close MP socket during cleanup
@ 2018-12-19 7:41 Qi Zhang
2018-12-19 8:46 ` Thomas Monjalon
2018-12-20 12:51 ` [dpdk-dev] [PATCH v2] " Qi Zhang
0 siblings, 2 replies; 5+ messages in thread
From: Qi Zhang @ 2018-12-19 7:41 UTC (permalink / raw)
To: anatoly.burakov; +Cc: thomas, dev, ferruh.yigit, Qi Zhang, stable
When secondary process quit, the mp_socket* file still exist, that
cause rte_mp_request_sync fail when try to send message on a floating
socket.
The patch fix the issue by introduce a function rte_mp_channel_fini.
This function will be called by rte_eal_cleanup and it will close the
mp socket and delete the mp_socket* file.
Fixes: bacaa2754017 ("eal: add channel for multi-process communication")
Cc: stable@dpdk.org
Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
lib/librte_eal/common/eal_common_proc.c | 22 +++++++++++++++++++++-
lib/librte_eal/common/eal_private.h | 6 ++++++
lib/librte_eal/linuxapp/eal/eal.c | 1 +
3 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c
index 6b876590a..ff19bea15 100644
--- a/lib/librte_eal/common/eal_common_proc.c
+++ b/lib/librte_eal/common/eal_common_proc.c
@@ -37,6 +37,7 @@ static int mp_fd = -1;
static char mp_filter[PATH_MAX]; /* Filter for secondary process sockets */
static char mp_dir_path[PATH_MAX]; /* The directory path for all mp sockets */
static pthread_mutex_t mp_mutex_action = PTHREAD_MUTEX_INITIALIZER;
+static char peer_name[PATH_MAX];
struct action_entry {
TAILQ_ENTRY(action_entry) next;
@@ -511,9 +512,9 @@ async_reply_handle(void *arg)
static int
open_socket_fd(void)
{
- char peer_name[PATH_MAX] = {0};
struct sockaddr_un un;
+ peer_name[0] = '\0';
if (rte_eal_process_type() == RTE_PROC_SECONDARY)
snprintf(peer_name, sizeof(peer_name),
"%d_%"PRIx64, getpid(), rte_rdtsc());
@@ -542,6 +543,19 @@ open_socket_fd(void)
return mp_fd;
}
+static void
+close_socket_fd(void)
+{
+ char path[PATH_MAX];
+
+ if (mp_fd < 0)
+ return;
+
+ close(mp_fd);
+ create_socket_path(peer_name, path, sizeof(path));
+ unlink(path);
+}
+
int
rte_mp_channel_init(void)
{
@@ -602,6 +616,12 @@ rte_mp_channel_init(void)
return 0;
}
+void
+rte_mp_channel_fini(void)
+{
+ close_socket_fd();
+}
+
/**
* Return -1, as fail to send message and it's caused by the local side.
* Return 0, as fail to send message and it's caused by the remote side.
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index 442c6dc48..10b6857f6 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -259,6 +259,12 @@ struct rte_bus *rte_bus_find_by_device_name(const char *str);
int rte_mp_channel_init(void);
/**
+ * Primary/secondary communication cleanup.
+ */
+
+void rte_mp_channel_fini(void);
+
+/**
* @internal
* Parse a device string and store its information in an
* rte_devargs structure.
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index d252c8591..52df05e8d 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -1229,6 +1229,7 @@ rte_eal_cleanup(void)
if (rte_eal_process_type() == RTE_PROC_PRIMARY)
rte_memseg_walk(mark_freeable, NULL);
rte_service_finalize();
+ rte_mp_channel_fini();
return 0;
}
--
2.13.6
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: close MP socket during cleanup
2018-12-19 7:41 [dpdk-dev] [PATCH] eal: close MP socket during cleanup Qi Zhang
@ 2018-12-19 8:46 ` Thomas Monjalon
2018-12-20 12:30 ` Zhang, Qi Z
2018-12-20 12:51 ` [dpdk-dev] [PATCH v2] " Qi Zhang
1 sibling, 1 reply; 5+ messages in thread
From: Thomas Monjalon @ 2018-12-19 8:46 UTC (permalink / raw)
To: Qi Zhang; +Cc: anatoly.burakov, dev, ferruh.yigit, stable
19/12/2018 08:41, Qi Zhang:
> When secondary process quit, the mp_socket* file still exist, that
> cause rte_mp_request_sync fail when try to send message on a floating
> socket.
>
> The patch fix the issue by introduce a function rte_mp_channel_fini.
> This function will be called by rte_eal_cleanup and it will close the
> mp socket and delete the mp_socket* file.
Why not call it rte_mp_channel_cleanup, keeping the same wording?
[...]
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -1229,6 +1229,7 @@ rte_eal_cleanup(void)
> if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> rte_memseg_walk(mark_freeable, NULL);
> rte_service_finalize();
> + rte_mp_channel_fini();
> return 0;
> }
Should be called in bsdapp/eal?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: close MP socket during cleanup
2018-12-19 8:46 ` Thomas Monjalon
@ 2018-12-20 12:30 ` Zhang, Qi Z
0 siblings, 0 replies; 5+ messages in thread
From: Zhang, Qi Z @ 2018-12-20 12:30 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: Burakov, Anatoly, dev, Yigit, Ferruh, stable
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Wednesday, December 19, 2018 4:46 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: Burakov, Anatoly <anatoly.burakov@intel.com>; dev@dpdk.org; Yigit, Ferruh
> <ferruh.yigit@intel.com>; stable@dpdk.org
> Subject: Re: [PATCH] eal: close MP socket during cleanup
>
> 19/12/2018 08:41, Qi Zhang:
> > When secondary process quit, the mp_socket* file still exist, that
> > cause rte_mp_request_sync fail when try to send message on a floating
> > socket.
> >
> > The patch fix the issue by introduce a function rte_mp_channel_fini.
> > This function will be called by rte_eal_cleanup and it will close the
> > mp socket and delete the mp_socket* file.
>
> Why not call it rte_mp_channel_cleanup, keeping the same wording?
Just some memory in my mind that "fini" is in fashion recently, but seems I'm wrong :)
>
> [...]
> > --- a/lib/librte_eal/linuxapp/eal/eal.c
> > +++ b/lib/librte_eal/linuxapp/eal/eal.c
> > @@ -1229,6 +1229,7 @@ rte_eal_cleanup(void)
> > if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> > rte_memseg_walk(mark_freeable, NULL);
> > rte_service_finalize();
> > + rte_mp_channel_fini();
> > return 0;
> > }
>
> Should be called in bsdapp/eal?
Yes, it should, will add in v2.
Thanks
Qi
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [dpdk-dev] [PATCH v2] eal: close MP socket during cleanup
2018-12-19 7:41 [dpdk-dev] [PATCH] eal: close MP socket during cleanup Qi Zhang
2018-12-19 8:46 ` Thomas Monjalon
@ 2018-12-20 12:51 ` Qi Zhang
2018-12-21 0:18 ` Thomas Monjalon
1 sibling, 1 reply; 5+ messages in thread
From: Qi Zhang @ 2018-12-20 12:51 UTC (permalink / raw)
To: anatoly.burakov; +Cc: thomas, dev, ferruh.yigit, Qi Zhang, stable
When secondary process quit, the mp_socket* file still exist, that
cause rte_mp_request_sync fail when try to send message on a floating
socket.
The patch fix the issue by introduce a function rte_mp_channel_cleanup.
This function will be called by rte_eal_cleanup and it will close the
mp socket and delete the mp_socket* file.
Fixes: bacaa2754017 ("eal: add channel for multi-process communication")
Cc: stable@dpdk.org
Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
v2:
- rename rte_mp_channel_fini to rte_mp_channel_cleanup.
- add missing bsd implementation.
lib/librte_eal/bsdapp/eal/eal.c | 1 +
lib/librte_eal/common/eal_common_proc.c | 22 +++++++++++++++++++++-
lib/librte_eal/common/eal_private.h | 6 ++++++
lib/librte_eal/linuxapp/eal/eal.c | 1 +
4 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 41ddb5a22..a6d02e7dc 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -919,6 +919,7 @@ int __rte_experimental
rte_eal_cleanup(void)
{
rte_service_finalize();
+ rte_mp_channel_cleanup();
return 0;
}
diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c
index 6b876590a..b46d644b3 100644
--- a/lib/librte_eal/common/eal_common_proc.c
+++ b/lib/librte_eal/common/eal_common_proc.c
@@ -37,6 +37,7 @@ static int mp_fd = -1;
static char mp_filter[PATH_MAX]; /* Filter for secondary process sockets */
static char mp_dir_path[PATH_MAX]; /* The directory path for all mp sockets */
static pthread_mutex_t mp_mutex_action = PTHREAD_MUTEX_INITIALIZER;
+static char peer_name[PATH_MAX];
struct action_entry {
TAILQ_ENTRY(action_entry) next;
@@ -511,9 +512,9 @@ async_reply_handle(void *arg)
static int
open_socket_fd(void)
{
- char peer_name[PATH_MAX] = {0};
struct sockaddr_un un;
+ peer_name[0] = '\0';
if (rte_eal_process_type() == RTE_PROC_SECONDARY)
snprintf(peer_name, sizeof(peer_name),
"%d_%"PRIx64, getpid(), rte_rdtsc());
@@ -542,6 +543,19 @@ open_socket_fd(void)
return mp_fd;
}
+static void
+close_socket_fd(void)
+{
+ char path[PATH_MAX];
+
+ if (mp_fd < 0)
+ return;
+
+ close(mp_fd);
+ create_socket_path(peer_name, path, sizeof(path));
+ unlink(path);
+}
+
int
rte_mp_channel_init(void)
{
@@ -602,6 +616,12 @@ rte_mp_channel_init(void)
return 0;
}
+void
+rte_mp_channel_cleanup(void)
+{
+ close_socket_fd();
+}
+
/**
* Return -1, as fail to send message and it's caused by the local side.
* Return 0, as fail to send message and it's caused by the remote side.
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index 442c6dc48..2eb2d5fa6 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -259,6 +259,12 @@ struct rte_bus *rte_bus_find_by_device_name(const char *str);
int rte_mp_channel_init(void);
/**
+ * Primary/secondary communication cleanup.
+ */
+
+void rte_mp_channel_cleanup(void);
+
+/**
* @internal
* Parse a device string and store its information in an
* rte_devargs structure.
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index dadba3385..c3fde536f 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -1234,6 +1234,7 @@ rte_eal_cleanup(void)
if (rte_eal_process_type() == RTE_PROC_PRIMARY)
rte_memseg_walk(mark_freeable, NULL);
rte_service_finalize();
+ rte_mp_channel_cleanup();
return 0;
}
--
2.13.6
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH v2] eal: close MP socket during cleanup
2018-12-20 12:51 ` [dpdk-dev] [PATCH v2] " Qi Zhang
@ 2018-12-21 0:18 ` Thomas Monjalon
0 siblings, 0 replies; 5+ messages in thread
From: Thomas Monjalon @ 2018-12-21 0:18 UTC (permalink / raw)
To: Qi Zhang; +Cc: dev, anatoly.burakov, ferruh.yigit, stable
20/12/2018 13:51, Qi Zhang:
> When secondary process quit, the mp_socket* file still exist, that
> cause rte_mp_request_sync fail when try to send message on a floating
> socket.
>
> The patch fix the issue by introduce a function rte_mp_channel_cleanup.
> This function will be called by rte_eal_cleanup and it will close the
> mp socket and delete the mp_socket* file.
>
> Fixes: bacaa2754017 ("eal: add channel for multi-process communication")
> Cc: stable@dpdk.org
>
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
[...]
> --- a/lib/librte_eal/common/eal_private.h
> +++ b/lib/librte_eal/common/eal_private.h
> /**
> + * Primary/secondary communication cleanup.
> + */
> +
> +void rte_mp_channel_cleanup(void);
extra blank line above
Applied with minor change, thanks
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-12-21 0:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-19 7:41 [dpdk-dev] [PATCH] eal: close MP socket during cleanup Qi Zhang
2018-12-19 8:46 ` Thomas Monjalon
2018-12-20 12:30 ` Zhang, Qi Z
2018-12-20 12:51 ` [dpdk-dev] [PATCH v2] " Qi Zhang
2018-12-21 0:18 ` Thomas Monjalon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).