[PATCH v2 2/3] net/mlx5: fix link status change detection

[Dmitry Kozlyuk <dkozlyuk@nvidia.com>]

Sometimes net/mlx5 devices did not detect link status change to "up".

Each shared device was monitoring IBV_EVENT_PORT_{ACTIVE,ERR}
and queried the link status upon receiving the event.
IBV_EVENT_PORT_ACTIVE is delivered when the logical link status
(UP flag) is set, but the physical link status (RUNNING flag)
may be down at that time, in which case the new link status
would be erroneously considered down.

IBV interface is insufficient for the task.
Monitor interface events using Netlink.

Fixes: 198a3c339a8f ("mlx5: handle link status interrupts")
Cc: stable@dpdk.org

Signed-off-by: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
Reviewed-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
---
Coverity complains about unchecked rte_intr_fd/type_set() return values.
That would be useless, because rte_intr_handle is in a known state.

 drivers/net/mlx5/linux/mlx5_ethdev_os.c | 63 ++++++++++++++++++++-----
 drivers/net/mlx5/linux/mlx5_os.c        | 55 +++++++++++++++++++++
 drivers/net/mlx5/mlx5.c                 |  1 +
 drivers/net/mlx5/mlx5.h                 |  3 ++
 drivers/net/mlx5/mlx5_trigger.c         | 12 ++++-
 5 files changed, 120 insertions(+), 14 deletions(-)

diff --git a/drivers/net/mlx5/linux/mlx5_ethdev_os.c b/drivers/net/mlx5/linux/mlx5_ethdev_os.c
index c19825ee52..8fe73f1adb 100644
--- a/drivers/net/mlx5/linux/mlx5_ethdev_os.c
+++ b/drivers/net/mlx5/linux/mlx5_ethdev_os.c
@@ -38,6 +38,7 @@
 #include <mlx5_devx_cmds.h>
 #include <mlx5_common.h>
 #include <mlx5_malloc.h>
+#include <mlx5_nl.h>
 
 #include "mlx5.h"
 #include "mlx5_rxtx.h"
@@ -760,6 +761,56 @@ mlx5_dev_interrupt_device_fatal(struct mlx5_dev_ctx_shared *sh)
 	}
 }
 
+static void
+mlx5_dev_interrupt_nl_cb(struct nlmsghdr *hdr, void *cb_arg)
+{
+	struct mlx5_dev_ctx_shared *sh = cb_arg;
+	uint32_t i;
+	uint32_t if_index;
+
+	if (mlx5_nl_parse_link_status_update(hdr, &if_index) < 0)
+		return;
+	for (i = 0; i < sh->max_port; i++) {
+		struct mlx5_dev_shared_port *port = &sh->port[i];
+		struct rte_eth_dev *dev;
+		struct mlx5_priv *priv;
+
+		if (port->nl_ih_port_id >= RTE_MAX_ETHPORTS)
+			continue;
+		dev = &rte_eth_devices[port->nl_ih_port_id];
+		/* Probing may initiate an LSC before configuration is done. */
+		if (dev->data->dev_configured &&
+		    !dev->data->dev_conf.intr_conf.lsc)
+			break;
+		priv = dev->data->dev_private;
+		if (priv->if_index == if_index) {
+			/* Block logical LSC events. */
+			uint16_t prev_status = dev->data->dev_link.link_status;
+
+			if (mlx5_link_update(dev, 0) < 0)
+				DRV_LOG(ERR, "Failed to update link status: %s",
+					rte_strerror(rte_errno));
+			else if (prev_status != dev->data->dev_link.link_status)
+				rte_eth_dev_callback_process
+					(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
+			break;
+		}
+	}
+}
+
+void
+mlx5_dev_interrupt_handler_nl(void *arg)
+{
+	struct mlx5_dev_ctx_shared *sh = arg;
+	int nlsk_fd = rte_intr_fd_get(sh->intr_handle_nl);
+
+	if (nlsk_fd < 0)
+		return;
+	if (mlx5_nl_read_events(nlsk_fd, mlx5_dev_interrupt_nl_cb, sh) < 0)
+		DRV_LOG(ERR, "Failed to process Netlink events: %s",
+			rte_strerror(rte_errno));
+}
+
 /**
  * Handle shared asynchronous events the NIC (removal event
  * and link status change). Supports multiport IB device.
@@ -823,18 +874,6 @@ mlx5_dev_interrupt_handler(void *cb_arg)
 		tmp = sh->port[tmp - 1].ih_port_id;
 		dev = &rte_eth_devices[tmp];
 		MLX5_ASSERT(dev);
-		if ((event.event_type == IBV_EVENT_PORT_ACTIVE ||
-		     event.event_type == IBV_EVENT_PORT_ERR) &&
-			dev->data->dev_conf.intr_conf.lsc) {
-			mlx5_glue->ack_async_event(&event);
-			if (mlx5_link_update(dev, 0) == -EAGAIN) {
-				usleep(0);
-				continue;
-			}
-			rte_eth_dev_callback_process
-				(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
-			continue;
-		}
 		DRV_LOG(DEBUG,
 			"port %u cannot handle an unknown event (type %d)",
 			dev->data->port_id, event.event_type);
diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index 17e7144cc9..29a4890d14 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -2495,6 +2495,40 @@ mlx5_os_net_cleanup(void)
 	mlx5_pmd_socket_uninit();
 }
 
+static int
+mlx5_os_dev_shared_handler_install_lsc(struct mlx5_dev_ctx_shared *sh)
+{
+	int nlsk_fd, flags, ret;
+
+	nlsk_fd = mlx5_nl_init(NETLINK_ROUTE, RTMGRP_LINK);
+	if (nlsk_fd < 0) {
+		DRV_LOG(ERR, "Failed to create a socket for Netlink events: %s",
+			rte_strerror(rte_errno));
+		return -1;
+	}
+	flags = fcntl(nlsk_fd, F_GETFL);
+	ret = fcntl(nlsk_fd, F_SETFL, flags | O_NONBLOCK);
+	if (ret != 0) {
+		DRV_LOG(ERR, "Failed to make Netlink event socket non-blocking: %s",
+			strerror(errno));
+		rte_errno = errno;
+		goto error;
+	}
+	rte_intr_type_set(sh->intr_handle_nl, RTE_INTR_HANDLE_EXT);
+	rte_intr_fd_set(sh->intr_handle_nl, nlsk_fd);
+	if (rte_intr_callback_register(sh->intr_handle_nl,
+				       mlx5_dev_interrupt_handler_nl,
+				       sh) != 0) {
+		DRV_LOG(ERR, "Failed to register Netlink events interrupt");
+		rte_intr_fd_set(sh->intr_handle_nl, -1);
+		goto error;
+	}
+	return 0;
+error:
+	close(nlsk_fd);
+	return -1;
+}
+
 /**
  * Install shared asynchronous device events handler.
  * This function is implemented to support event sharing
@@ -2532,6 +2566,18 @@ mlx5_os_dev_shared_handler_install(struct mlx5_dev_ctx_shared *sh)
 			rte_intr_fd_set(sh->intr_handle, -1);
 		}
 	}
+	sh->intr_handle_nl = rte_intr_instance_alloc
+						(RTE_INTR_INSTANCE_F_SHARED);
+	if (sh->intr_handle_nl == NULL) {
+		DRV_LOG(ERR, "Fail to allocate intr_handle");
+		rte_errno = ENOMEM;
+		return;
+	}
+	rte_intr_fd_set(sh->intr_handle_nl, -1);
+	if (mlx5_os_dev_shared_handler_install_lsc(sh) < 0) {
+		DRV_LOG(INFO, "Fail to install the shared Netlink event handler.");
+		rte_intr_fd_set(sh->intr_handle_nl, -1);
+	}
 	if (sh->cdev->config.devx) {
 #ifdef HAVE_IBV_DEVX_ASYNC
 		sh->intr_handle_devx =
@@ -2579,10 +2625,19 @@ mlx5_os_dev_shared_handler_install(struct mlx5_dev_ctx_shared *sh)
 void
 mlx5_os_dev_shared_handler_uninstall(struct mlx5_dev_ctx_shared *sh)
 {
+	int nlsk_fd;
+
 	if (rte_intr_fd_get(sh->intr_handle) >= 0)
 		mlx5_intr_callback_unregister(sh->intr_handle,
 					      mlx5_dev_interrupt_handler, sh);
 	rte_intr_instance_free(sh->intr_handle);
+	nlsk_fd = rte_intr_fd_get(sh->intr_handle_nl);
+	if (nlsk_fd >= 0) {
+		mlx5_intr_callback_unregister
+			(sh->intr_handle_nl, mlx5_dev_interrupt_handler_nl, sh);
+		close(nlsk_fd);
+	}
+	rte_intr_instance_free(sh->intr_handle_nl);
 #ifdef HAVE_IBV_DEVX_ASYNC
 	if (rte_intr_fd_get(sh->intr_handle_devx) >= 0)
 		rte_intr_callback_unregister(sh->intr_handle_devx,
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 74841caaf9..09cd1367db 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -1457,6 +1457,7 @@ mlx5_alloc_shared_dev_ctx(const struct mlx5_dev_spawn_data *spawn,
 	for (i = 0; i < sh->max_port; i++) {
 		sh->port[i].ih_port_id = RTE_MAX_ETHPORTS;
 		sh->port[i].devx_ih_port_id = RTE_MAX_ETHPORTS;
+		sh->port[i].nl_ih_port_id = RTE_MAX_ETHPORTS;
 	}
 	if (sh->cdev->config.devx) {
 		sh->td = mlx5_devx_cmd_create_td(sh->cdev->ctx);
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 9a4aff97cb..0f0045a2b5 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -637,6 +637,7 @@ struct mlx5_age_info {
 struct mlx5_dev_shared_port {
 	uint32_t ih_port_id;
 	uint32_t devx_ih_port_id;
+	uint32_t nl_ih_port_id;
 	/*
 	 * Interrupt handler port_id. Used by shared interrupt
 	 * handler to find the corresponding rte_eth device
@@ -1239,6 +1240,7 @@ struct mlx5_dev_ctx_shared {
 	/* Shared interrupt handler section. */
 	struct rte_intr_handle *intr_handle; /* Interrupt handler for device. */
 	struct rte_intr_handle *intr_handle_devx; /* DEVX interrupt handler. */
+	struct rte_intr_handle *intr_handle_nl; /* Netlink interrupt handler. */
 	void *devx_comp; /* DEVX async comp obj. */
 	struct mlx5_devx_obj *tis[16]; /* TIS object. */
 	struct mlx5_devx_obj *td; /* Transport domain. */
@@ -1666,6 +1668,7 @@ int mlx5_dev_set_flow_ctrl(struct rte_eth_dev *dev,
 			   struct rte_eth_fc_conf *fc_conf);
 void mlx5_dev_interrupt_handler(void *arg);
 void mlx5_dev_interrupt_handler_devx(void *arg);
+void mlx5_dev_interrupt_handler_nl(void *arg);
 int mlx5_set_link_down(struct rte_eth_dev *dev);
 int mlx5_set_link_up(struct rte_eth_dev *dev);
 int mlx5_is_removed(struct rte_eth_dev *dev);
diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c
index fe8b42c414..c68b32cf14 100644
--- a/drivers/net/mlx5/mlx5_trigger.c
+++ b/drivers/net/mlx5/mlx5_trigger.c
@@ -1207,11 +1207,18 @@ mlx5_dev_start(struct rte_eth_dev *dev)
 		priv->sh->port[priv->dev_port - 1].ih_port_id =
 					(uint32_t)dev->data->port_id;
 	} else {
-		DRV_LOG(INFO, "port %u starts without LSC and RMV interrupts.",
+		DRV_LOG(INFO, "port %u starts without RMV interrupts.",
 			dev->data->port_id);
-		dev->data->dev_conf.intr_conf.lsc = 0;
 		dev->data->dev_conf.intr_conf.rmv = 0;
 	}
+	if (rte_intr_fd_get(priv->sh->intr_handle_nl) >= 0) {
+		priv->sh->port[priv->dev_port - 1].nl_ih_port_id =
+					(uint32_t)dev->data->port_id;
+	} else {
+		DRV_LOG(INFO, "port %u starts without LSC interrupts.",
+			dev->data->port_id);
+		dev->data->dev_conf.intr_conf.lsc = 0;
+	}
 	if (rte_intr_fd_get(priv->sh->intr_handle_devx) >= 0)
 		priv->sh->port[priv->dev_port - 1].devx_ih_port_id =
 					(uint32_t)dev->data->port_id;
@@ -1263,6 +1270,7 @@ mlx5_dev_stop(struct rte_eth_dev *dev)
 	mlx5_rx_intr_vec_disable(dev);
 	priv->sh->port[priv->dev_port - 1].ih_port_id = RTE_MAX_ETHPORTS;
 	priv->sh->port[priv->dev_port - 1].devx_ih_port_id = RTE_MAX_ETHPORTS;
+	priv->sh->port[priv->dev_port - 1].nl_ih_port_id = RTE_MAX_ETHPORTS;
 	mlx5_txq_stop(dev);
 	mlx5_rxq_stop(dev);
 	if (priv->obj_ops.lb_dummy_queue_release)
-- 
2.25.1