DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/mlx5: close tools socket with the last device
@ 2021-10-14  8:50 Dmitry Kozlyuk
  2021-10-14  8:55 ` [dpdk-dev] [PATCH v2] " Dmitry Kozlyuk
  0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Kozlyuk @ 2021-10-14  8:50 UTC (permalink / raw)
  To: dev
  Cc: David Marchand, Raslan Darawsheh, Harman Kalra, Thomas Monjalon,
	Matan Azrad, Viacheslav Ovsiienko

MLX5 PMD exposes a socket for external tools to dump port state.
Socket events are listened using an interrupt source of EXT type.
The socket was closed and the interrupt callback was unregistered
at program exit, which is incorrect because DPDK could be already
shut down at this point. Move actions performed at program exit
to the moment the last MLX5 port is closed. The socket will be opened
again if later a new MLX5 device is plugged in and probed.
Also fix comments that were deceisively talking
about secondary processes instead of external tools.

Reported-by: Harman Kalra <hkalra@marvell.com>
Signed-off-by: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
Acked-by: Thomas Monjalon <thomas@monjalon.net>
---
 drivers/net/mlx5/linux/mlx5_os.c     |  9 +++++++++
 drivers/net/mlx5/linux/mlx5_socket.c | 12 +++---------
 drivers/net/mlx5/mlx5.c              |  6 ++++--
 drivers/net/mlx5/mlx5.h              |  2 ++
 drivers/net/mlx5/windows/mlx5_os.c   |  8 ++++++++
 5 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index 3746057673..28db0827d5 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -2793,6 +2793,15 @@ mlx5_os_net_probe(struct rte_device *dev)
 		return mlx5_os_auxiliary_probe(dev);
 }
 
+/**
+ * Cleanup resources when the last device is closed.
+ */
+void
+mlx5_os_net_cleanup(void)
+{
+	mlx5_pmd_socket_uninit();
+}
+
 static int
 mlx5_config_doorbell_mapping_env(const struct mlx5_dev_config *config)
 {
diff --git a/drivers/net/mlx5/linux/mlx5_socket.c b/drivers/net/mlx5/linux/mlx5_socket.c
index 6356b66dc4..902b8ec934 100644
--- a/drivers/net/mlx5/linux/mlx5_socket.c
+++ b/drivers/net/mlx5/linux/mlx5_socket.c
@@ -167,10 +167,7 @@ mlx5_pmd_interrupt_handler_uninstall(void)
 }
 
 /**
- * Initialise the socket to communicate with the secondary process
- *
- * @param[in] dev
- *   Pointer to Ethernet device.
+ * Initialise the socket to communicate with external tools.
  *
  * @return
  *   0 on success, a negative value otherwise.
@@ -187,10 +184,6 @@ mlx5_pmd_socket_init(void)
 	MLX5_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY);
 	if (server_socket)
 		return 0;
-	/*
-	 * Initialize the socket to communicate with the secondary
-	 * process.
-	 */
 	ret = socket(AF_UNIX, SOCK_STREAM, 0);
 	if (ret < 0) {
 		DRV_LOG(WARNING, "Failed to open mlx5 socket: %s",
@@ -237,7 +230,8 @@ mlx5_pmd_socket_init(void)
 /**
  * Un-Initialize the pmd socket
  */
-RTE_FINI(mlx5_pmd_socket_uninit)
+void
+mlx5_pmd_socket_uninit(void)
 {
 	if (!server_socket)
 		return;
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 45ccfe2784..497cf52787 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -1315,9 +1315,11 @@ mlx5_free_shared_dev_ctx(struct mlx5_dev_ctx_shared *sh)
 	mlx5_mr_release_cache(&sh->share_cache);
 	/* Remove context from the global device list. */
 	LIST_REMOVE(sh, next);
-	/* Release flow workspaces objects on the last device. */
-	if (LIST_EMPTY(&mlx5_dev_ctx_list))
+	/* Release resources on the last device removal. */
+	if (LIST_EMPTY(&mlx5_dev_ctx_list)) {
+		mlx5_os_net_cleanup();
 		mlx5_flow_os_release_workspace();
+	}
 	pthread_mutex_unlock(&mlx5_dev_ctx_list_mutex);
 	/*
 	 *  Ensure there is no async event handler installed.
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 3581414b78..08eeddaddc 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -1739,6 +1739,7 @@ int mlx5_mp_os_req_queue_control(struct rte_eth_dev *dev, uint16_t queue_id,
 /* mlx5_socket.c */
 
 int mlx5_pmd_socket_init(void);
+void mlx5_pmd_socket_uninit(void);
 
 /* mlx5_flow_meter.c */
 
@@ -1787,6 +1788,7 @@ int mlx5_os_set_promisc(struct rte_eth_dev *dev, int enable);
 int mlx5_os_set_allmulti(struct rte_eth_dev *dev, int enable);
 int mlx5_os_set_nonblock_channel_fd(int fd);
 void mlx5_os_mac_addr_flush(struct rte_eth_dev *dev);
+void mlx5_os_net_cleanup(void);
 
 /* mlx5_txpp.c */
 
diff --git a/drivers/net/mlx5/windows/mlx5_os.c b/drivers/net/mlx5/windows/mlx5_os.c
index 26fa927039..bcfb26c57f 100644
--- a/drivers/net/mlx5/windows/mlx5_os.c
+++ b/drivers/net/mlx5/windows/mlx5_os.c
@@ -1185,4 +1185,12 @@ mlx5_os_get_pdn(void *pd, uint32_t *pdn)
 	return 0;
 }
 
+/**
+ * Cleanup resources when the last device is closed.
+ */
+void
+mlx5_os_net_cleanup(void)
+{
+}
+
 const struct mlx5_flow_driver_ops mlx5_flow_verbs_drv_ops = {0};
-- 
2.25.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [dpdk-dev] [PATCH v2] net/mlx5: close tools socket with the last device
  2021-10-14  8:50 [dpdk-dev] [PATCH] net/mlx5: close tools socket with the last device Dmitry Kozlyuk
@ 2021-10-14  8:55 ` Dmitry Kozlyuk
  2021-10-14 11:20   ` David Marchand
  2021-10-21 10:36   ` Raslan Darawsheh
  0 siblings, 2 replies; 4+ messages in thread
From: Dmitry Kozlyuk @ 2021-10-14  8:55 UTC (permalink / raw)
  To: dev
  Cc: David Marchand, Raslan Darawsheh, Xueming Li, stable,
	Harman Kalra, Thomas Monjalon, Matan Azrad, Viacheslav Ovsiienko

MLX5 PMD exposes a socket for external tools to dump port state.
Socket events are listened using an interrupt source of EXT type.
The socket was closed and the interrupt callback was unregistered
at program exit, which is incorrect because DPDK could be already
shut down at this point. Move actions performed at program exit
to the moment the last MLX5 port is closed. The socket will be opened
again if later a new MLX5 device is plugged in and probed.
Also fix comments that were deceisively talking
about secondary processes instead of external tools.

Fixes: e6cdc54cc0ef ("net/mlx5: add socket server for external tools")
Cc: Xueming Li <xuemingl@nvidia.com>
Cc: stable@dpdk.org

Reported-by: Harman Kalra <hkalra@marvell.com>
Signed-off-by: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
Acked-by: Thomas Monjalon <thomas@monjalon.net>
---
v2: add Fixes tag

 drivers/net/mlx5/linux/mlx5_os.c     |  9 +++++++++
 drivers/net/mlx5/linux/mlx5_socket.c | 12 +++---------
 drivers/net/mlx5/mlx5.c              |  6 ++++--
 drivers/net/mlx5/mlx5.h              |  2 ++
 drivers/net/mlx5/windows/mlx5_os.c   |  8 ++++++++
 5 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index 3746057673..28db0827d5 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -2793,6 +2793,15 @@ mlx5_os_net_probe(struct rte_device *dev)
 		return mlx5_os_auxiliary_probe(dev);
 }
 
+/**
+ * Cleanup resources when the last device is closed.
+ */
+void
+mlx5_os_net_cleanup(void)
+{
+	mlx5_pmd_socket_uninit();
+}
+
 static int
 mlx5_config_doorbell_mapping_env(const struct mlx5_dev_config *config)
 {
diff --git a/drivers/net/mlx5/linux/mlx5_socket.c b/drivers/net/mlx5/linux/mlx5_socket.c
index 6356b66dc4..902b8ec934 100644
--- a/drivers/net/mlx5/linux/mlx5_socket.c
+++ b/drivers/net/mlx5/linux/mlx5_socket.c
@@ -167,10 +167,7 @@ mlx5_pmd_interrupt_handler_uninstall(void)
 }
 
 /**
- * Initialise the socket to communicate with the secondary process
- *
- * @param[in] dev
- *   Pointer to Ethernet device.
+ * Initialise the socket to communicate with external tools.
  *
  * @return
  *   0 on success, a negative value otherwise.
@@ -187,10 +184,6 @@ mlx5_pmd_socket_init(void)
 	MLX5_ASSERT(rte_eal_process_type() == RTE_PROC_PRIMARY);
 	if (server_socket)
 		return 0;
-	/*
-	 * Initialize the socket to communicate with the secondary
-	 * process.
-	 */
 	ret = socket(AF_UNIX, SOCK_STREAM, 0);
 	if (ret < 0) {
 		DRV_LOG(WARNING, "Failed to open mlx5 socket: %s",
@@ -237,7 +230,8 @@ mlx5_pmd_socket_init(void)
 /**
  * Un-Initialize the pmd socket
  */
-RTE_FINI(mlx5_pmd_socket_uninit)
+void
+mlx5_pmd_socket_uninit(void)
 {
 	if (!server_socket)
 		return;
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 45ccfe2784..497cf52787 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -1315,9 +1315,11 @@ mlx5_free_shared_dev_ctx(struct mlx5_dev_ctx_shared *sh)
 	mlx5_mr_release_cache(&sh->share_cache);
 	/* Remove context from the global device list. */
 	LIST_REMOVE(sh, next);
-	/* Release flow workspaces objects on the last device. */
-	if (LIST_EMPTY(&mlx5_dev_ctx_list))
+	/* Release resources on the last device removal. */
+	if (LIST_EMPTY(&mlx5_dev_ctx_list)) {
+		mlx5_os_net_cleanup();
 		mlx5_flow_os_release_workspace();
+	}
 	pthread_mutex_unlock(&mlx5_dev_ctx_list_mutex);
 	/*
 	 *  Ensure there is no async event handler installed.
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 3581414b78..08eeddaddc 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -1739,6 +1739,7 @@ int mlx5_mp_os_req_queue_control(struct rte_eth_dev *dev, uint16_t queue_id,
 /* mlx5_socket.c */
 
 int mlx5_pmd_socket_init(void);
+void mlx5_pmd_socket_uninit(void);
 
 /* mlx5_flow_meter.c */
 
@@ -1787,6 +1788,7 @@ int mlx5_os_set_promisc(struct rte_eth_dev *dev, int enable);
 int mlx5_os_set_allmulti(struct rte_eth_dev *dev, int enable);
 int mlx5_os_set_nonblock_channel_fd(int fd);
 void mlx5_os_mac_addr_flush(struct rte_eth_dev *dev);
+void mlx5_os_net_cleanup(void);
 
 /* mlx5_txpp.c */
 
diff --git a/drivers/net/mlx5/windows/mlx5_os.c b/drivers/net/mlx5/windows/mlx5_os.c
index 26fa927039..bcfb26c57f 100644
--- a/drivers/net/mlx5/windows/mlx5_os.c
+++ b/drivers/net/mlx5/windows/mlx5_os.c
@@ -1185,4 +1185,12 @@ mlx5_os_get_pdn(void *pd, uint32_t *pdn)
 	return 0;
 }
 
+/**
+ * Cleanup resources when the last device is closed.
+ */
+void
+mlx5_os_net_cleanup(void)
+{
+}
+
 const struct mlx5_flow_driver_ops mlx5_flow_verbs_drv_ops = {0};
-- 
2.25.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH v2] net/mlx5: close tools socket with the last device
  2021-10-14  8:55 ` [dpdk-dev] [PATCH v2] " Dmitry Kozlyuk
@ 2021-10-14 11:20   ` David Marchand
  2021-10-21 10:36   ` Raslan Darawsheh
  1 sibling, 0 replies; 4+ messages in thread
From: David Marchand @ 2021-10-14 11:20 UTC (permalink / raw)
  To: Dmitry Kozlyuk
  Cc: dev, Raslan Darawsheh, Xueming Li, dpdk stable, Harman Kalra,
	Thomas Monjalon, Matan Azrad, Viacheslav Ovsiienko

On Thu, Oct 14, 2021 at 10:55 AM Dmitry Kozlyuk <dkozlyuk@nvidia.com> wrote:
>
> MLX5 PMD exposes a socket for external tools to dump port state.
> Socket events are listened using an interrupt source of EXT type.
> The socket was closed and the interrupt callback was unregistered
> at program exit, which is incorrect because DPDK could be already
> shut down at this point. Move actions performed at program exit
> to the moment the last MLX5 port is closed. The socket will be opened
> again if later a new MLX5 device is plugged in and probed.
> Also fix comments that were deceisively talking
> about secondary processes instead of external tools.

+1 for fixing those comments.

>
> Fixes: e6cdc54cc0ef ("net/mlx5: add socket server for external tools")
> Cc: Xueming Li <xuemingl@nvidia.com>
> Cc: stable@dpdk.org
>
> Reported-by: Harman Kalra <hkalra@marvell.com>
> Signed-off-by: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
> Acked-by: Thomas Monjalon <thomas@monjalon.net>

The fix lgtm, thanks.

There is a separate issue I spotted while reviewing.
I'll send a separate fix.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH v2] net/mlx5: close tools socket with the last device
  2021-10-14  8:55 ` [dpdk-dev] [PATCH v2] " Dmitry Kozlyuk
  2021-10-14 11:20   ` David Marchand
@ 2021-10-21 10:36   ` Raslan Darawsheh
  1 sibling, 0 replies; 4+ messages in thread
From: Raslan Darawsheh @ 2021-10-21 10:36 UTC (permalink / raw)
  To: Dmitry Kozlyuk, dev
  Cc: David Marchand, Raslan Darawsheh, Xueming Li, stable,
	Harman Kalra, NBU-Contact-Thomas Monjalon, Matan Azrad,
	Viacheslav Ovsiienko

Hi,

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Dmitry Kozlyuk
> Sent: Thursday, October 14, 2021 11:55 AM
> To: dev@dpdk.org
> Cc: David Marchand <david.marchand@redhat.com>; Raslan Darawsheh
> <rasland@oss.nvidia.com>; Xueming Li <xuemingl@oss.nvidia.com>;
> stable@dpdk.org; Harman Kalra <hkalra@marvell.com>; NBU-Contact-
> Thomas Monjalon <thomas@monjalon.net>; Matan Azrad
> <matan@oss.nvidia.com>; Viacheslav Ovsiienko
> <viacheslavo@oss.nvidia.com>
> Subject: [dpdk-dev] [PATCH v2] net/mlx5: close tools socket with the last
> device
> 
> MLX5 PMD exposes a socket for external tools to dump port state.
> Socket events are listened using an interrupt source of EXT type.
> The socket was closed and the interrupt callback was unregistered at
> program exit, which is incorrect because DPDK could be already shut down at
> this point. Move actions performed at program exit to the moment the last
> MLX5 port is closed. The socket will be opened again if later a new MLX5
> device is plugged in and probed.
> Also fix comments that were deceisively talking about secondary processes
> instead of external tools.
> 
> Fixes: e6cdc54cc0ef ("net/mlx5: add socket server for external tools")
> Cc: Xueming Li <xuemingl@nvidia.com>
> Cc: stable@dpdk.org
> 
> Reported-by: Harman Kalra <hkalra@marvell.com>
> Signed-off-by: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> v2: add Fixes tag
> 

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-10-21 10:36 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-14  8:50 [dpdk-dev] [PATCH] net/mlx5: close tools socket with the last device Dmitry Kozlyuk
2021-10-14  8:55 ` [dpdk-dev] [PATCH v2] " Dmitry Kozlyuk
2021-10-14 11:20   ` David Marchand
2021-10-21 10:36   ` Raslan Darawsheh

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git