[RFC v4 5/5] interrupts: warn on leaked file descriptors

[David Marchand <david.marchand@redhat.com>]

Let's warn on interrupt handle that still reference what looks like a
valid file descriptor.

This will likely raise false positives, but it should help limiting FD
leaks on the long term.

Add missing reset to -1 in drivers, and document code.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since RFC v2:
- split code comments added by patch 3 in the RFC v2,
- added a check on leftover FD in a interrupt handle,

---
 app/test/test_interrupts.c                       | 5 ++++-
 drivers/bus/cdx/cdx_vfio.c                       | 4 ++++
 drivers/bus/pci/linux/pci_vfio.c                 | 2 ++
 drivers/common/mlx5/linux/mlx5_common_os.c       | 6 +++++-
 drivers/net/mana/mana.c                          | 1 +
 drivers/net/mlx4/mlx4_intr.c                     | 1 +
 drivers/net/sxe2/sxe2_irq.c                      | 5 ++++-
 drivers/net/tap/rte_eth_tap.c                    | 1 +
 drivers/net/virtio/virtio_user/virtio_user_dev.c | 2 ++
 drivers/raw/cnxk_gpio/cnxk_gpio.c                | 3 +++
 drivers/vdpa/mlx5/mlx5_vdpa_event.c              | 1 +
 drivers/vdpa/mlx5/mlx5_vdpa_virtq.c              | 1 +
 lib/eal/common/eal_common_interrupts.c           | 6 ++++++
 13 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/app/test/test_interrupts.c b/app/test/test_interrupts.c
index 67f1e6429a..a737a9cd51 100644
--- a/app/test/test_interrupts.c
+++ b/app/test/test_interrupts.c
@@ -130,8 +130,11 @@ test_interrupt_deinit(void)
 {
 	int i;
 
-	for (i = 0; i < TEST_INTERRUPT_HANDLE_MAX; i++)
+	for (i = 0; i < TEST_INTERRUPT_HANDLE_MAX; i++) {
+		rte_intr_fd_set(intr_handles[i], -1);
+		rte_intr_dev_fd_set(intr_handles[i], -1);
 		rte_intr_instance_free(intr_handles[i]);
+	}
 	close(pfds.pipefd[0]);
 	close(pfds.pipefd[1]);
 
diff --git a/drivers/bus/cdx/cdx_vfio.c b/drivers/bus/cdx/cdx_vfio.c
index 8d67058bfe..0c1b263a85 100644
--- a/drivers/bus/cdx/cdx_vfio.c
+++ b/drivers/bus/cdx/cdx_vfio.c
@@ -116,6 +116,8 @@ cdx_vfio_unmap_resource_primary(struct rte_cdx_device *dev)
 	vfio_dev_fd = rte_intr_dev_fd_get(dev->intr_handle);
 	if (vfio_dev_fd < 0)
 		return -1;
+	/* vfio_dev_fd is owned by VFIO, only clear reference here. */
+	rte_intr_dev_fd_set(dev->intr_handle, -1);
 
 	ret = rte_vfio_release_device(RTE_CDX_BUS_DEVICES_PATH, dev->device.name,
 				      vfio_dev_fd);
@@ -150,6 +152,8 @@ cdx_vfio_unmap_resource_secondary(struct rte_cdx_device *dev)
 	vfio_dev_fd = rte_intr_dev_fd_get(dev->intr_handle);
 	if (vfio_dev_fd < 0)
 		return -1;
+	/* vfio_dev_fd is owned by VFIO, only clear reference here. */
+	rte_intr_dev_fd_set(dev->intr_handle, -1);
 
 	ret = rte_vfio_release_device(RTE_CDX_BUS_DEVICES_PATH, dev->device.name,
 				      vfio_dev_fd);
diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
index 84c338c68e..b5f05e9f9f 100644
--- a/drivers/bus/pci/linux/pci_vfio.c
+++ b/drivers/bus/pci/linux/pci_vfio.c
@@ -356,6 +356,7 @@ pci_vfio_enable_notifier(struct rte_pci_device *dev, int vfio_dev_fd)
 error:
 	rte_intr_fd_close(dev->vfio_req_intr_handle);
 	rte_intr_type_set(dev->vfio_req_intr_handle, RTE_INTR_HANDLE_UNKNOWN);
+	/* vfio_dev_fd is managed by VFIO layer, only clear reference here. */
 	rte_intr_dev_fd_set(dev->vfio_req_intr_handle, -1);
 
 	return -1;
@@ -383,6 +384,7 @@ pci_vfio_disable_notifier(struct rte_pci_device *dev)
 
 	rte_intr_fd_close(dev->vfio_req_intr_handle);
 	rte_intr_type_set(dev->vfio_req_intr_handle, RTE_INTR_HANDLE_UNKNOWN);
+	/* vfio_dev_fd is managed by VFIO layer, only clear reference here. */
 	rte_intr_dev_fd_set(dev->vfio_req_intr_handle, -1);
 
 	return 0;
diff --git a/drivers/common/mlx5/linux/mlx5_common_os.c b/drivers/common/mlx5/linux/mlx5_common_os.c
index 3e9cd86062..09d37f32cb 100644
--- a/drivers/common/mlx5/linux/mlx5_common_os.c
+++ b/drivers/common/mlx5/linux/mlx5_common_os.c
@@ -1108,6 +1108,7 @@ mlx5_os_interrupt_handler_create(int mode, bool set_fd_nonblock, int fd,
 	}
 	return tmp_intr_handle;
 err:
+	rte_intr_fd_set(tmp_intr_handle, -1);
 	rte_intr_instance_free(tmp_intr_handle);
 	return NULL;
 }
@@ -1181,8 +1182,11 @@ void
 mlx5_os_interrupt_handler_destroy(struct rte_intr_handle *intr_handle,
 				  rte_intr_callback_fn cb, void *cb_arg)
 {
-	if (rte_intr_fd_get(intr_handle) >= 0)
+	if (rte_intr_fd_get(intr_handle) >= 0) {
 		mlx5_intr_callback_unregister(intr_handle, cb, cb_arg);
+		/* fd is not owned by the driver, only clear reference here. */
+		rte_intr_fd_set(intr_handle, -1);
+	}
 	rte_intr_instance_free(intr_handle);
 }
 
diff --git a/drivers/net/mana/mana.c b/drivers/net/mana/mana.c
index 1864ba2a2b..f51598ea68 100644
--- a/drivers/net/mana/mana.c
+++ b/drivers/net/mana/mana.c
@@ -1964,6 +1964,7 @@ mana_intr_install(struct rte_eth_dev *eth_dev, struct mana_priv *priv)
 					 mana_intr_handler, priv);
 	if (ret) {
 		DRV_LOG(ERR, "Failed to register intr callback");
+		/* fd is owned by ibverbs, only clear reference here. */
 		rte_intr_fd_set(priv->intr_handle, -1);
 		goto free_intr;
 	}
diff --git a/drivers/net/mlx4/mlx4_intr.c b/drivers/net/mlx4/mlx4_intr.c
index 01057482ec..c278dd5ecc 100644
--- a/drivers/net/mlx4/mlx4_intr.c
+++ b/drivers/net/mlx4/mlx4_intr.c
@@ -265,6 +265,7 @@ mlx4_intr_uninstall(struct mlx4_priv *priv)
 					     (void (*)(void *))
 					     mlx4_interrupt_handler,
 					     priv);
+		/* fd is owned by ibverbs, only clear reference here. */
 		if (rte_intr_fd_set(priv->intr_handle, -1))
 			return -rte_errno;
 	}
diff --git a/drivers/net/sxe2/sxe2_irq.c b/drivers/net/sxe2/sxe2_irq.c
index 3306504761..c30a26d94e 100644
--- a/drivers/net/sxe2/sxe2_irq.c
+++ b/drivers/net/sxe2/sxe2_irq.c
@@ -377,8 +377,11 @@ static void sxe2_intr_handler_destroy(struct rte_intr_handle *intr_handle,
 	if (!intr_handle)
 		return;
 
-	if (rte_intr_fd_get(intr_handle) >= 0)
+	if (rte_intr_fd_get(intr_handle) >= 0) {
 		(void)rte_intr_callback_unregister(intr_handle, cb, cb_arg);
+		/* fd is not owned by the driver, only clear reference here. */
+		rte_intr_fd_set(intr_handle, -1);
+	}
 	rte_intr_instance_free(intr_handle);
 }
 
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 13114edba5..693be653ba 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -1947,6 +1947,7 @@ tap_lsc_intr_handle_set(struct rte_eth_dev *dev, int set)
 
 	if (rte_intr_fd_get(pmd->intr_handle) >= 0) {
 		tap_nl_final(rte_intr_fd_get(pmd->intr_handle));
+		/* fd is not owned by the driver, only clear reference here. */
 		rte_intr_fd_set(pmd->intr_handle, -1);
 	}
 
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index f3df73c1f0..f89c871beb 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -858,6 +858,8 @@ virtio_user_dev_uninit(struct virtio_user_dev *dev)
 {
 	struct rte_eth_dev *eth_dev = &rte_eth_devices[dev->hw.port_id];
 
+	/* fd is not owned by the driver, only clear reference here. */
+	rte_intr_fd_set(eth_dev->intr_handle, -1);
 	rte_intr_instance_free(eth_dev->intr_handle);
 	eth_dev->intr_handle = NULL;
 
diff --git a/drivers/raw/cnxk_gpio/cnxk_gpio.c b/drivers/raw/cnxk_gpio/cnxk_gpio.c
index 0549e326f9..2b5ea0bf9f 100644
--- a/drivers/raw/cnxk_gpio/cnxk_gpio.c
+++ b/drivers/raw/cnxk_gpio/cnxk_gpio.c
@@ -442,6 +442,8 @@ cnxk_gpio_unregister_irq(struct cnxk_gpio *gpio)
 	if (ret)
 		return ret;
 
+	/* fd is owned by gpio, only clear reference here. */
+	rte_intr_fd_set(gpio->intr.intr_handle, -1);
 	rte_intr_instance_free(gpio->intr.intr_handle);
 	gpio->intr.intr_handle = NULL;
 
@@ -635,6 +637,7 @@ cnxk_gpio_register_irq_compat(struct cnxk_gpio *gpio, struct cnxk_gpio_irq *irq,
 
 	return 0;
 out:
+	rte_intr_fd_set(intr_handle, -1);
 	rte_intr_instance_free(intr_handle);
 
 	return ret;
diff --git a/drivers/vdpa/mlx5/mlx5_vdpa_event.c b/drivers/vdpa/mlx5/mlx5_vdpa_event.c
index dee2ccc23d..d18cf7d7e5 100644
--- a/drivers/vdpa/mlx5/mlx5_vdpa_event.c
+++ b/drivers/vdpa/mlx5/mlx5_vdpa_event.c
@@ -487,6 +487,7 @@ mlx5_vdpa_err_event_unset(struct mlx5_vdpa_priv *priv)
 			rte_pause();
 		}
 	}
+	/* fd is owned by err_chnl, only clear reference here. */
 	rte_intr_fd_set(priv->err_intr_handle, -1);
 	if (priv->err_chnl) {
 #ifdef HAVE_IBV_DEVX_EVENT
diff --git a/drivers/vdpa/mlx5/mlx5_vdpa_virtq.c b/drivers/vdpa/mlx5/mlx5_vdpa_virtq.c
index 093cdd08d2..9cad7e1df4 100644
--- a/drivers/vdpa/mlx5/mlx5_vdpa_virtq.c
+++ b/drivers/vdpa/mlx5/mlx5_vdpa_virtq.c
@@ -96,6 +96,7 @@ mlx5_vdpa_virtq_unregister_intr_handle(struct mlx5_vdpa_virtq *virtq)
 				pthread_mutex_lock(&virtq->virtq_lock);
 			}
 		}
+		/* fd is owned by vhost, only clear reference here. */
 		(void)rte_intr_fd_set(virtq->intr_handle, -1);
 	}
 	rte_intr_instance_free(virtq->intr_handle);
diff --git a/lib/eal/common/eal_common_interrupts.c b/lib/eal/common/eal_common_interrupts.c
index 8176eb089a..6368ef1771 100644
--- a/lib/eal/common/eal_common_interrupts.c
+++ b/lib/eal/common/eal_common_interrupts.c
@@ -187,6 +187,12 @@ void rte_intr_instance_free(struct rte_intr_handle *intr_handle)
 {
 	if (intr_handle == NULL)
 		return;
+	if (rte_intr_fd_get(intr_handle) >= 0)
+		EAL_LOG(NOTICE, "Some interrupt handle is leaking a FD: %d",
+			rte_intr_fd_get(intr_handle));
+	if (rte_intr_dev_fd_get(intr_handle) >= 0)
+		EAL_LOG(NOTICE, "Some interrupt handle is leaking a device FD: %d",
+			rte_intr_dev_fd_get(intr_handle));
 	if (RTE_INTR_INSTANCE_USES_RTE_MEMORY(intr_handle->alloc_flags)) {
 		rte_free(intr_handle->efds);
 		rte_free(intr_handle->elist);
-- 
2.54.0