[PATCH 0/9] Interrupt fixes for leaks and warnings

[PATCH 0/9] Interrupt fixes for leaks and warnings

[David Marchand]

Following commit d770ee9caed0 ("interrupts: warn on leaked FDs"), a
number of leaks, but also false positive warnings have been identified.

The most notable change in this series is the removal of the interrupt
handle duplication. I concluded the principle is wrong (see patch 2
commitlog).
This will need some careful reviews.


-- 
David Marchand

David Marchand (7):
  eal/linux: reset FD in interrupt handle for uevent
  interrupts: do not duplicate interrupt handles
  net/mana: reset FD in interrupt handle
  net/memif: fix some interrupt FD leaks
  net/memif: reset FD in interrupt handle
  net/sxe2: reset FD in interrupt handle
  raw/ifpga: reset FD in interrupt handle

Stephen Hemminger (2):
  eal/linux: fix FD leak in alarm subsystem
  bus/pci: reset VFIO device FD after release

 doc/guides/rel_notes/release_26_11.rst |  1 +
 drivers/bus/pci/linux/pci_vfio.c       |  4 ++--
 drivers/common/cnxk/roc_platform.h     |  1 -
 drivers/net/mana/mana.c                |  4 ++++
 drivers/net/memif/memif_socket.c       | 28 ++++++++++++++++++----
 drivers/net/sxe2/sxe2_irq.c            |  2 ++
 drivers/raw/ifpga/ifpga_rawdev.c       |  2 ++
 lib/eal/common/eal_common_interrupts.c | 26 --------------------
 lib/eal/freebsd/eal_interrupts.c       | 17 ++++---------
 lib/eal/include/rte_interrupts.h       | 17 +------------
 lib/eal/linux/eal_alarm.c              |  1 +
 lib/eal/linux/eal_dev.c                |  4 ++--
 lib/eal/linux/eal_interrupts.c         | 33 ++++++++------------------
 13 files changed, 53 insertions(+), 87 deletions(-)

-- 
2.54.0

[PATCH 1/9] eal/linux: fix FD leak in alarm subsystem

[David Marchand]

From: Stephen Hemminger <stephen@networkplumber.org>

The timerfd used for alarms is never closed on EAL cleanup.
FreeBSD was fixed to close it, do the same on Linux.

Fixes: 90b13ab8d4f7 ("alarm: remove direct access to interrupt handle")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: David Marchand <david.marchand@redhat.com>
---
 lib/eal/linux/eal_alarm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/eal/linux/eal_alarm.c b/lib/eal/linux/eal_alarm.c
index a1433eb867..5f8655ea11 100644
--- a/lib/eal/linux/eal_alarm.c
+++ b/lib/eal/linux/eal_alarm.c
@@ -61,6 +61,7 @@ rte_eal_alarm_cleanup(void)
 	int ret = rte_intr_callback_unregister_sync(intr_handle,
 						eal_alarm_callback, (void *)-1);
 	if (ret >= 0) {
+		rte_intr_fd_close(intr_handle);
 		rte_intr_instance_free(intr_handle);
 		intr_handle = NULL;
 		handler_registered = 0;
-- 
2.54.0

[PATCH 2/9] eal/linux: reset FD in interrupt handle for uevent

[David Marchand]

Clear the warning when freeing the interrupt handle used for the uevent
handler.

Fixes: d770ee9caed0 ("interrupts: warn on leaked FDs")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/eal/linux/eal_dev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/eal/linux/eal_dev.c b/lib/eal/linux/eal_dev.c
index 35223b6b26..1c8ae1d60b 100644
--- a/lib/eal/linux/eal_dev.c
+++ b/lib/eal/linux/eal_dev.c
@@ -366,7 +366,7 @@ rte_dev_event_monitor_start(void)
 	ret = rte_intr_callback_register(intr_handle, dev_uev_handler, NULL);
 
 	if (ret) {
-		close(rte_intr_fd_get(intr_handle));
+		rte_intr_fd_close(intr_handle);
 		goto exit;
 	}
 
@@ -406,7 +406,7 @@ rte_dev_event_monitor_stop(void)
 		goto exit;
 	}
 
-	close(rte_intr_fd_get(intr_handle));
+	rte_intr_fd_close(intr_handle);
 	rte_intr_instance_free(intr_handle);
 	intr_handle = NULL;
 	ret = 0;
-- 
2.54.0

[PATCH 3/9] interrupts: do not duplicate interrupt handles

[David Marchand]

The FD leak warnings recently added have revealed an interesting issue.

An interrupt handle is allocated and living usually in a bus device object
(or some standalone code, like the alarm code).
All the logic around the referenced FDs in this object are specific to
the callers, and those FDs must then be closed in the callers context.

On the other hand, the interrupt subsystem works with copies of interrupt
handle objects passed to it via rte_intr_callback_register/unregister.

When releasing this clone of the interrupt handle, the FDs are reported
as leaked, though there is no way for the interrupt subsystem itself to
know what to do with those FDs.

Instead of creating a clone, reference the interrupt handle directly.
The callers are responsible for unregistering those handles,
closing the FDs and freeing the handles.

This change also switches the interrupt source lookup from FD based
matching to pointer based matching.
The original FD based matching allowed different intr_handle objects
with the same FD to share an interrupt source.
However, this was problematic:
* the cloned handle stored in the source came from the first registrant,
* the ucb_fn callback would receive this clone, not the (const)
  caller's handle,
* with multiple handles for the same FD, later registrants would get
  the wrong handle in their callback,

In practice, DPDK components use one intr_handle per FD, so pointer
based matching simply enforces this existing invariant.
The interrupt dispatch code continues to use FD based matching since
epoll returns the FD, not the handle pointer.

The unregister callback (ucb_fn) signature is updated to take a const
pointer, reflecting that the interrupt subsystem no longer owns the
handle. Note that with this change, the intr_handle parameter passed
to ucb_fn is the same pointer the caller registered with, callers
can simply use their own handle from cb_arg instead.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 doc/guides/rel_notes/release_26_11.rst |  1 +
 drivers/common/cnxk/roc_platform.h     |  1 -
 drivers/net/memif/memif_socket.c       |  5 ++--
 lib/eal/common/eal_common_interrupts.c | 26 --------------------
 lib/eal/freebsd/eal_interrupts.c       | 17 ++++---------
 lib/eal/include/rte_interrupts.h       | 17 +------------
 lib/eal/linux/eal_interrupts.c         | 33 ++++++++------------------
 7 files changed, 20 insertions(+), 80 deletions(-)

diff --git a/doc/guides/rel_notes/release_26_11.rst b/doc/guides/rel_notes/release_26_11.rst
index be758a0c02..78f9bb54aa 100644
--- a/doc/guides/rel_notes/release_26_11.rst
+++ b/doc/guides/rel_notes/release_26_11.rst
@@ -404,6 +404,7 @@ API Changes
     to invalid values instead of zero.
   * Added ``rte_intr_fd_close()`` and ``rte_intr_dev_fd_close()``
     helpers to close file descriptors and reset them to -1.
+  * ``rte_intr_unregister_callback_fn`` now receives a ``const`` interrupt handle.
 
 * mempool: When creating a mempool, the cache size can be freely specified
   (although still not exceed the number of elements),
diff --git a/drivers/common/cnxk/roc_platform.h b/drivers/common/cnxk/roc_platform.h
index aba9782023..149522608e 100644
--- a/drivers/common/cnxk/roc_platform.h
+++ b/drivers/common/cnxk/roc_platform.h
@@ -178,7 +178,6 @@ plt_thread_is_valid(plt_thread_t thr)
 #define plt_intr_type_get		rte_intr_type_get
 #define plt_intr_type_set		rte_intr_type_set
 #define plt_intr_instance_alloc		rte_intr_instance_alloc
-#define plt_intr_instance_dup		rte_intr_instance_dup
 #define plt_intr_instance_free		rte_intr_instance_free
 #define plt_intr_event_list_update	rte_intr_event_list_update
 #define plt_intr_max_intr_get		rte_intr_max_intr_get
diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index 87b23aa8b4..2d16decbef 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -572,13 +572,14 @@ memif_msg_enq_connected(struct rte_eth_dev *dev)
 }
 
 static void
-memif_intr_unregister_handler(struct rte_intr_handle *intr_handle, void *arg)
+memif_intr_unregister_handler(const struct rte_intr_handle *intr_handle, void *arg)
 {
 	struct memif_msg_queue_elt *elt;
 	struct memif_control_channel *cc = arg;
 
+	RTE_SET_USED(intr_handle);
 	/* close control channel fd */
-	rte_intr_fd_close(intr_handle);
+	rte_intr_fd_close(cc->intr_handle);
 	/* clear message queue */
 	while ((elt = TAILQ_FIRST(&cc->msg_queue)) != NULL) {
 		TAILQ_REMOVE(&cc->msg_queue, elt, next);
diff --git a/lib/eal/common/eal_common_interrupts.c b/lib/eal/common/eal_common_interrupts.c
index 6368ef1771..1208a2597d 100644
--- a/lib/eal/common/eal_common_interrupts.c
+++ b/lib/eal/common/eal_common_interrupts.c
@@ -103,32 +103,6 @@ struct rte_intr_handle *rte_intr_instance_alloc(uint32_t flags)
 	return NULL;
 }
 
-RTE_EXPORT_INTERNAL_SYMBOL(rte_intr_instance_dup)
-struct rte_intr_handle *rte_intr_instance_dup(const struct rte_intr_handle *src)
-{
-	struct rte_intr_handle *intr_handle;
-
-	if (src == NULL) {
-		EAL_LOG(DEBUG, "Source interrupt instance unallocated");
-		rte_errno = EINVAL;
-		return NULL;
-	}
-
-	intr_handle = rte_intr_instance_alloc(src->alloc_flags);
-	if (intr_handle != NULL) {
-		intr_handle->fd = src->fd;
-		intr_handle->dev_fd = src->dev_fd;
-		intr_handle->type = src->type;
-		intr_handle->max_intr = src->max_intr;
-		intr_handle->nb_efd = src->nb_efd;
-		intr_handle->efd_counter_size = src->efd_counter_size;
-		memcpy(intr_handle->efds, src->efds, src->nb_intr);
-		memcpy(intr_handle->elist, src->elist, src->nb_intr);
-	}
-
-	return intr_handle;
-}
-
 RTE_EXPORT_INTERNAL_SYMBOL(rte_intr_event_list_update)
 int rte_intr_event_list_update(struct rte_intr_handle *intr_handle, int size)
 {
diff --git a/lib/eal/freebsd/eal_interrupts.c b/lib/eal/freebsd/eal_interrupts.c
index bb2bc529f2..b29a2b174e 100644
--- a/lib/eal/freebsd/eal_interrupts.c
+++ b/lib/eal/freebsd/eal_interrupts.c
@@ -41,7 +41,7 @@ struct rte_intr_callback {
 
 struct rte_intr_source {
 	TAILQ_ENTRY(rte_intr_source) next;
-	struct rte_intr_handle *intr_handle; /**< interrupt handle */
+	const struct rte_intr_handle *intr_handle; /**< interrupt handle */
 	struct rte_intr_cb_list callbacks;  /**< user callbacks */
 	uint32_t active;
 };
@@ -105,7 +105,7 @@ rte_intr_callback_register(const struct rte_intr_handle *intr_handle,
 
 	/* find the source for this intr_handle */
 	TAILQ_FOREACH(src, &intr_sources, next) {
-		if (rte_intr_fd_get(src->intr_handle) == rte_intr_fd_get(intr_handle))
+		if (src->intr_handle == intr_handle)
 			break;
 	}
 
@@ -138,14 +138,7 @@ rte_intr_callback_register(const struct rte_intr_handle *intr_handle,
 				ret = -ENOMEM;
 				goto fail;
 			} else {
-				src->intr_handle = rte_intr_instance_dup(intr_handle);
-				if (src->intr_handle == NULL) {
-					EAL_LOG(ERR, "Can not create intr instance");
-					ret = -ENOMEM;
-					free(src);
-					src = NULL;
-					goto fail;
-				}
+				src->intr_handle = intr_handle;
 				TAILQ_INIT(&src->callbacks);
 				TAILQ_INSERT_TAIL(&intr_sources, src, next);
 			}
@@ -239,7 +232,7 @@ rte_intr_callback_unregister_pending(const struct rte_intr_handle *intr_handle,
 
 	/* check if the interrupt source for the fd is existent */
 	TAILQ_FOREACH(src, &intr_sources, next)
-		if (rte_intr_fd_get(src->intr_handle) == rte_intr_fd_get(intr_handle))
+		if (src->intr_handle == intr_handle)
 			break;
 
 	/* No interrupt source registered for the fd */
@@ -294,7 +287,7 @@ rte_intr_callback_unregister(const struct rte_intr_handle *intr_handle,
 
 	/* check if the interrupt source for the fd is existent */
 	TAILQ_FOREACH(src, &intr_sources, next)
-		if (rte_intr_fd_get(src->intr_handle) == rte_intr_fd_get(intr_handle))
+		if (src->intr_handle == intr_handle)
 			break;
 
 	/* No interrupt source registered for the fd */
diff --git a/lib/eal/include/rte_interrupts.h b/lib/eal/include/rte_interrupts.h
index d04c0c7b15..bb06180bea 100644
--- a/lib/eal/include/rte_interrupts.h
+++ b/lib/eal/include/rte_interrupts.h
@@ -70,7 +70,7 @@ typedef void (*rte_intr_callback_fn)(void *cb_arg);
  * Function to call after a callback is unregistered.
  * Can be used to close fd and free cb_arg.
  */
-typedef void (*rte_intr_unregister_callback_fn)(struct rte_intr_handle *intr_handle,
+typedef void (*rte_intr_unregister_callback_fn)(const struct rte_intr_handle *intr_handle,
 						void *cb_arg);
 
 /**
@@ -432,21 +432,6 @@ __rte_internal
 int
 rte_intr_cap_multiple(struct rte_intr_handle *intr_handle);
 
-/**
- * @internal
- * Creates a clone of src by allocating a new handle and copying src content.
- *
- * @param src
- *  Source interrupt handle to be cloned.
- *
- * @return
- *  - On success, address of interrupt handle.
- *  - On failure, NULL.
- */
-__rte_internal
-struct rte_intr_handle *
-rte_intr_instance_dup(const struct rte_intr_handle *src);
-
 /**
  * @internal
  * Set the device fd field of interrupt handle with user
diff --git a/lib/eal/linux/eal_interrupts.c b/lib/eal/linux/eal_interrupts.c
index 4cfaeba7fe..dcc2c0efdb 100644
--- a/lib/eal/linux/eal_interrupts.c
+++ b/lib/eal/linux/eal_interrupts.c
@@ -78,7 +78,7 @@ struct rte_intr_callback {
 
 struct rte_intr_source {
 	TAILQ_ENTRY(rte_intr_source) next;
-	struct rte_intr_handle *intr_handle; /**< interrupt handle */
+	const struct rte_intr_handle *intr_handle; /**< interrupt handle */
 	struct rte_intr_cb_list callbacks;  /**< user callbacks */
 	uint32_t active;
 };
@@ -511,7 +511,7 @@ rte_intr_callback_register(const struct rte_intr_handle *intr_handle,
 
 	/* check if there is at least one callback registered for the fd */
 	TAILQ_FOREACH(src, &intr_sources, next) {
-		if (rte_intr_fd_get(src->intr_handle) == rte_intr_fd_get(intr_handle)) {
+		if (src->intr_handle == intr_handle) {
 			/* we had no interrupts for this */
 			if (TAILQ_EMPTY(&src->callbacks))
 				wake_thread = 1;
@@ -531,22 +531,12 @@ rte_intr_callback_register(const struct rte_intr_handle *intr_handle,
 			free(callback);
 			callback = NULL;
 		} else {
-			src->intr_handle = rte_intr_instance_dup(intr_handle);
-			if (src->intr_handle == NULL) {
-				EAL_LOG(ERR, "Can not create intr instance");
-				ret = -ENOMEM;
-				free(callback);
-				callback = NULL;
-				free(src);
-				src = NULL;
-			} else {
-				TAILQ_INIT(&src->callbacks);
-				TAILQ_INSERT_TAIL(&(src->callbacks), callback,
-						  next);
-				TAILQ_INSERT_TAIL(&intr_sources, src, next);
-				wake_thread = 1;
-				ret = 0;
-			}
+			src->intr_handle = intr_handle;
+			TAILQ_INIT(&src->callbacks);
+			TAILQ_INSERT_TAIL(&(src->callbacks), callback, next);
+			TAILQ_INSERT_TAIL(&intr_sources, src, next);
+			wake_thread = 1;
+			ret = 0;
 		}
 	}
 
@@ -584,7 +574,7 @@ rte_intr_callback_unregister_pending(const struct rte_intr_handle *intr_handle,
 
 	/* check if the interrupt source for the fd is existent */
 	TAILQ_FOREACH(src, &intr_sources, next) {
-		if (rte_intr_fd_get(src->intr_handle) == rte_intr_fd_get(intr_handle))
+		if (src->intr_handle == intr_handle)
 			break;
 	}
 
@@ -635,7 +625,7 @@ rte_intr_callback_unregister(const struct rte_intr_handle *intr_handle,
 
 	/* check if the interrupt source for the fd is existent */
 	TAILQ_FOREACH(src, &intr_sources, next)
-		if (rte_intr_fd_get(src->intr_handle) == rte_intr_fd_get(intr_handle))
+		if (src->intr_handle == intr_handle)
 			break;
 
 	/* No interrupt source registered for the fd */
@@ -666,7 +656,6 @@ rte_intr_callback_unregister(const struct rte_intr_handle *intr_handle,
 		/* all callbacks for that source are removed. */
 		if (TAILQ_EMPTY(&src->callbacks)) {
 			TAILQ_REMOVE(&intr_sources, src, next);
-			rte_intr_instance_free(src->intr_handle);
 			free(src);
 		}
 	}
@@ -922,7 +911,6 @@ eal_intr_source_remove_and_free(struct rte_intr_source *src)
 	}
 
 	/* Free the interrupt source */
-	rte_intr_instance_free(src->intr_handle);
 	free(src);
 }
 
@@ -1067,7 +1055,6 @@ eal_intr_process_interrupts(struct epoll_event *events, int nfds)
 		/* all callbacks for that source are removed. */
 		if (TAILQ_EMPTY(&src->callbacks)) {
 			TAILQ_REMOVE(&intr_sources, src, next);
-			rte_intr_instance_free(src->intr_handle);
 			free(src);
 		}
 
-- 
2.54.0

[PATCH 4/9] bus/pci: reset VFIO device FD after release

[David Marchand]

From: Stephen Hemminger <stephen@networkplumber.org>

The VFIO device fd is closed by dev_vfio_release_device() but is left
in the interrupt handle, so freeing the handle on cleanup logs a bogus
FD leak warning.

Mark dev_fd invalid after release.

In secondary process the request handle is unused and does not have to
be set in the first place.

Fixes: d770ee9caed0 ("interrupts: warn on leaked FDs")

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/pci/linux/pci_vfio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
index fb35bc1d49..c81d20200d 100644
--- a/drivers/bus/pci/linux/pci_vfio.c
+++ b/drivers/bus/pci/linux/pci_vfio.c
@@ -998,8 +998,6 @@ pci_vfio_map_resource_secondary(struct rte_pci_device *dev)
 	/* we need save vfio_dev_fd, so it can be used during release */
 	if (rte_intr_dev_fd_set(dev->intr_handle, vfio_dev_fd))
 		goto err_map;
-	if (rte_intr_dev_fd_set(dev->vfio_req_intr_handle, vfio_dev_fd))
-		goto err_map;
 
 	return 0;
 err_map:
@@ -1103,6 +1101,7 @@ pci_vfio_unmap_resource_primary(struct rte_pci_device *dev)
 		PCI_LOG(ERR, "Cannot release VFIO device");
 		return ret;
 	}
+	rte_intr_dev_fd_set(dev->intr_handle, -1);
 
 	vfio_res_list =
 		RTE_TAILQ_CAST(dev_vfio_tailq.head, mapped_pci_res_list);
@@ -1142,6 +1141,7 @@ pci_vfio_unmap_resource_secondary(struct rte_pci_device *dev)
 		PCI_LOG(ERR, "Cannot release VFIO device");
 		return ret;
 	}
+	rte_intr_dev_fd_set(dev->intr_handle, -1);
 
 	vfio_res_list =
 		RTE_TAILQ_CAST(dev_vfio_tailq.head, mapped_pci_res_list);
-- 
2.54.0

[PATCH 5/9] net/mana: reset FD in interrupt handle

[David Marchand]

The interrupt handle references the ibverbs async_fd which is owned
by the ibverbs context, not by this driver. Clear the FD reference
before freeing the interrupt handle to avoid a spurious leak warning.

Fixes: d770ee9caed0 ("interrupts: warn on leaked FDs")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/mana/mana.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/mana/mana.c b/drivers/net/mana/mana.c
index 30b0ce90d6..0ea6d9a54a 100644
--- a/drivers/net/mana/mana.c
+++ b/drivers/net/mana/mana.c
@@ -1796,6 +1796,8 @@ mana_reset_exit(struct mana_priv *priv)
 		else
 			DRV_LOG(DEBUG, "%d intr callback(s) removed", ret);
 
+		/* fd is owned by ibverbs, only clear reference here. */
+		rte_intr_fd_set(priv->intr_handle, -1);
 		rte_intr_instance_free(priv->intr_handle);
 		priv->intr_handle = NULL;
 	}
@@ -1915,6 +1917,8 @@ mana_intr_uninstall(struct mana_priv *priv)
 		return ret;
 	}
 
+	/* fd is owned by ibverbs, only clear reference here. */
+	rte_intr_fd_set(priv->intr_handle, -1);
 	rte_intr_instance_free(priv->intr_handle);
 	priv->intr_handle = NULL;
 
-- 
2.54.0

[PATCH 6/9] net/memif: fix some interrupt FD leaks

[David Marchand]

Fix several resource leaks in error handling and cleanup paths:

In memif_socket_create(), when rte_intr_callback_register() fails,
return via error path instead of returning directly, ensuring sockfd,
sock and intr_handle are properly cleaned up.

In memif_socket_init(), when rte_hash_add_key_data() fails, clean up
the newly created socket including unregistering callback, closing fd
and freeing intr_handle for listener sockets.

In memif_socket_remove_device(), unregister the listener callback and
close the socket fd before freeing the interrupt handle.

Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/memif/memif_socket.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index 2d16decbef..bd52a95473 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -1051,7 +1051,7 @@ memif_socket_create(char *key, uint8_t listener, bool is_abstract, uid_t owner_u
 		if (ret < 0) {
 			MIF_LOG(ERR, "Failed to register interrupt "
 				"callback for listener socket");
-			return NULL;
+			goto error;
 		}
 	}
 
@@ -1115,6 +1115,14 @@ memif_socket_init(struct rte_eth_dev *dev, const char *socket_filename)
 		ret = rte_hash_add_key_data(hash, key, socket);
 		if (ret < 0) {
 			MIF_LOG(ERR, "Failed to add socket to socket hash.");
+			if (socket->listener) {
+				rte_intr_callback_unregister(socket->intr_handle,
+					memif_listener_handler, socket);
+				close(rte_intr_fd_get(socket->intr_handle));
+				rte_intr_fd_set(socket->intr_handle, -1);
+				rte_intr_instance_free(socket->intr_handle);
+			}
+			rte_free(socket);
 			return ret;
 		}
 	}
@@ -1180,8 +1188,13 @@ memif_socket_remove_device(struct rte_eth_dev *dev)
 				MIF_LOG(ERR, "Failed to remove socket file: %s",
 					socket->filename);
 		}
-		if (pmd->role != MEMIF_ROLE_CLIENT)
+		if (pmd->role != MEMIF_ROLE_CLIENT) {
+			rte_intr_callback_unregister(socket->intr_handle, memif_listener_handler,
+				socket);
+			close(rte_intr_fd_get(socket->intr_handle));
+			rte_intr_fd_set(socket->intr_handle, -1);
 			rte_intr_instance_free(socket->intr_handle);
+		}
 		rte_free(socket);
 	}
 }
-- 
2.54.0

[PATCH 7/9] net/memif: reset FD in interrupt handle

[David Marchand]

Clear FD references before freeing interrupt handles to avoid
spurious leak warnings. In error paths where the socket fd is
closed separately, clear the reference. In cleanup paths, use
rte_intr_fd_close() to close and clear in one call.

Fixes: d770ee9caed0 ("interrupts: warn on leaked FDs")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/memif/memif_socket.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index bd52a95473..b47c7a3b1b 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -638,7 +638,7 @@ memif_disconnect(struct rte_eth_dev *dev)
 							pmd->cc,
 							memif_intr_unregister_handler);
 			} else if (ret > 0) {
-				close(rte_intr_fd_get(ih));
+				rte_intr_fd_close(ih);
 				rte_intr_instance_free(ih);
 				rte_free(pmd->cc);
 			}
@@ -962,6 +962,8 @@ memif_listener_handler(void *arg)
 	if (cc != NULL) {
 		rte_intr_callback_unregister(cc->intr_handle, memif_intr_handler,
 					 cc);
+		/* fd is closed below, clear reference here. */
+		rte_intr_fd_set(cc->intr_handle, -1);
 		rte_intr_instance_free(cc->intr_handle);
 		rte_free(cc);
 	}
@@ -1060,6 +1062,8 @@ memif_socket_create(char *key, uint8_t listener, bool is_abstract, uid_t owner_u
  error:
 	MIF_LOG(ERR, "Failed to setup socket %s: %s", key, strerror(errno));
 	if (sock != NULL) {
+		/* fd is closed below, clear reference here. */
+		rte_intr_fd_set(sock->intr_handle, -1);
 		rte_intr_instance_free(sock->intr_handle);
 		rte_free(sock);
 	}
@@ -1118,8 +1122,7 @@ memif_socket_init(struct rte_eth_dev *dev, const char *socket_filename)
 			if (socket->listener) {
 				rte_intr_callback_unregister(socket->intr_handle,
 					memif_listener_handler, socket);
-				close(rte_intr_fd_get(socket->intr_handle));
-				rte_intr_fd_set(socket->intr_handle, -1);
+				rte_intr_fd_close(socket->intr_handle);
 				rte_intr_instance_free(socket->intr_handle);
 			}
 			rte_free(socket);
@@ -1191,8 +1194,7 @@ memif_socket_remove_device(struct rte_eth_dev *dev)
 		if (pmd->role != MEMIF_ROLE_CLIENT) {
 			rte_intr_callback_unregister(socket->intr_handle, memif_listener_handler,
 				socket);
-			close(rte_intr_fd_get(socket->intr_handle));
-			rte_intr_fd_set(socket->intr_handle, -1);
+			rte_intr_fd_close(socket->intr_handle);
 			rte_intr_instance_free(socket->intr_handle);
 		}
 		rte_free(socket);
@@ -1290,6 +1292,8 @@ memif_connect_client(struct rte_eth_dev *dev)
 		sockfd = -1;
 	}
 	if (pmd->cc != NULL) {
+		/* fd is closed above, clear reference here. */
+		rte_intr_fd_set(pmd->cc->intr_handle, -1);
 		rte_intr_instance_free(pmd->cc->intr_handle);
 		rte_free(pmd->cc);
 		pmd->cc = NULL;
-- 
2.54.0

[PATCH 8/9] net/sxe2: reset FD in interrupt handle

[David Marchand]

The fd passed to sxe2_intr_handler_create() is not owned by this
function. Clear the FD reference before freeing the interrupt handle
on error to avoid a spurious leak warning.

Fixes: d770ee9caed0 ("interrupts: warn on leaked FDs")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/sxe2/sxe2_irq.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/sxe2/sxe2_irq.c b/drivers/net/sxe2/sxe2_irq.c
index 256949e74e..0a16c263c1 100644
--- a/drivers/net/sxe2/sxe2_irq.c
+++ b/drivers/net/sxe2/sxe2_irq.c
@@ -387,6 +387,8 @@ sxe2_intr_handler_create(int32_t fd, rte_intr_callback_fn cb, void *cb_arg)
 
 	return tmp_intr_handle;
 err:
+	/* fd is not owned by this function, only clear reference here. */
+	rte_intr_fd_set(tmp_intr_handle, -1);
 	rte_intr_instance_free(tmp_intr_handle);
 	return NULL;
 }
-- 
2.54.0

[PATCH 9/9] raw/ifpga: reset FD in interrupt handle

[David Marchand]

The interrupt handle fd points to efd[0] which is already closed
by rte_intr_efd_disable(). Clear the FD reference before freeing
the interrupt handle to avoid a spurious leak warning.

Fixes: d770ee9caed0 ("interrupts: warn on leaked FDs")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/raw/ifpga/ifpga_rawdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c
index c4143bfb3e..d44141824f 100644
--- a/drivers/raw/ifpga/ifpga_rawdev.c
+++ b/drivers/raw/ifpga/ifpga_rawdev.c
@@ -1421,6 +1421,8 @@ ifpga_unregister_msix_irq(struct ifpga_rawdev *dev, enum ifpga_irq_type type,
 			type == IFPGA_FME_IRQ ? "FME" : "AFU",
 			type == IFPGA_FME_IRQ ? 0 : vec_start);
 	} else {
+		/* fd was pointing to efd[0] which is closed by rte_intr_efd_disable(). */
+		rte_intr_fd_set(*intr_handle, -1);
 		rte_intr_instance_free(*intr_handle);
 		*intr_handle = NULL;
 	}
-- 
2.54.0

[PATCH v2 00/12] Interrupt fixes for leaks and warnings

[David Marchand]

Following commit d770ee9caed0 ("interrupts: warn on leaked FDs"), a
number of leaks, but also false positive warnings have been identified.

The most notable change in this series is the removal of the interrupt
handle duplication. I concluded the principle is wrong (see patch 2
commitlog).
This will need some careful reviews.


-- 
David Marchand

Changes since v1:
- added Anatoly fixes which also extend the interrupt check to efds
  https://patchwork.dpdk.org/project/dpdk/list/?series=39788&state=*


Anatoly Burakov (3):
  eal/linux: fix closing of event FDs not owned by EAL
  eal/linux: fix eventfd leaks when enabling interrupts
  interrupts: warn on leaked event FDs

David Marchand (7):
  eal/linux: reset FD in interrupt handle for uevent
  interrupts: do not duplicate interrupt handles
  net/mana: reset FD in interrupt handle
  net/memif: fix some interrupt FD leaks
  net/memif: reset FD in interrupt handle
  net/sxe2: reset FD in interrupt handle
  raw/ifpga: reset FD in interrupt handle

Stephen Hemminger (2):
  eal/linux: fix FD leak in alarm subsystem
  bus/pci: reset VFIO device FD after release

 doc/guides/rel_notes/release_26_11.rst |  1 +
 drivers/bus/pci/linux/pci_vfio.c       |  4 +-
 drivers/common/cnxk/roc_platform.h     |  1 -
 drivers/net/mana/mana.c                |  4 ++
 drivers/net/memif/memif_socket.c       | 28 ++++++--
 drivers/net/sxe2/sxe2_irq.c            |  2 +
 drivers/raw/ifpga/ifpga_rawdev.c       |  2 +
 lib/eal/common/eal_common_interrupts.c | 31 ++-------
 lib/eal/freebsd/eal_interrupts.c       | 17 ++---
 lib/eal/include/rte_interrupts.h       | 24 ++-----
 lib/eal/linux/eal_alarm.c              |  1 +
 lib/eal/linux/eal_dev.c                |  4 +-
 lib/eal/linux/eal_interrupts.c         | 94 +++++++++++++++-----------
 13 files changed, 109 insertions(+), 104 deletions(-)

-- 
2.54.0

[PATCH v2 01/12] eal/linux: fix FD leak in alarm subsystem

[David Marchand]

From: Stephen Hemminger <stephen@networkplumber.org>

The timerfd used for alarms is never closed on EAL cleanup.
FreeBSD was fixed to close it, do the same on Linux.

Fixes: 90b13ab8d4f7 ("alarm: remove direct access to interrupt handle")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: David Marchand <david.marchand@redhat.com>
---
 lib/eal/linux/eal_alarm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/eal/linux/eal_alarm.c b/lib/eal/linux/eal_alarm.c
index a1433eb867..5f8655ea11 100644
--- a/lib/eal/linux/eal_alarm.c
+++ b/lib/eal/linux/eal_alarm.c
@@ -61,6 +61,7 @@ rte_eal_alarm_cleanup(void)
 	int ret = rte_intr_callback_unregister_sync(intr_handle,
 						eal_alarm_callback, (void *)-1);
 	if (ret >= 0) {
+		rte_intr_fd_close(intr_handle);
 		rte_intr_instance_free(intr_handle);
 		intr_handle = NULL;
 		handler_registered = 0;
-- 
2.54.0

[PATCH v2 02/12] eal/linux: reset FD in interrupt handle for uevent

[David Marchand]

Clear the warning when freeing the interrupt handle used for the uevent
handler.

Fixes: d770ee9caed0 ("interrupts: warn on leaked FDs")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/eal/linux/eal_dev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/eal/linux/eal_dev.c b/lib/eal/linux/eal_dev.c
index 35223b6b26..1c8ae1d60b 100644
--- a/lib/eal/linux/eal_dev.c
+++ b/lib/eal/linux/eal_dev.c
@@ -366,7 +366,7 @@ rte_dev_event_monitor_start(void)
 	ret = rte_intr_callback_register(intr_handle, dev_uev_handler, NULL);
 
 	if (ret) {
-		close(rte_intr_fd_get(intr_handle));
+		rte_intr_fd_close(intr_handle);
 		goto exit;
 	}
 
@@ -406,7 +406,7 @@ rte_dev_event_monitor_stop(void)
 		goto exit;
 	}
 
-	close(rte_intr_fd_get(intr_handle));
+	rte_intr_fd_close(intr_handle);
 	rte_intr_instance_free(intr_handle);
 	intr_handle = NULL;
 	ret = 0;
-- 
2.54.0

[PATCH v2 03/12] interrupts: do not duplicate interrupt handles

[David Marchand]

The FD leak warnings recently added have revealed an interesting issue.

An interrupt handle is allocated and living usually in a bus device object
(or some standalone code, like the alarm code).
All the logic around the referenced FDs in this object are specific to
the callers, and those FDs must then be closed in the callers context.

On the other hand, the interrupt subsystem works with copies of interrupt
handle objects passed to it via rte_intr_callback_register/unregister.

When releasing this clone of the interrupt handle, the FDs are reported
as leaked, though there is no way for the interrupt subsystem itself to
know what to do with those FDs.

Instead of creating a clone, reference the interrupt handle directly.
The callers are responsible for unregistering those handles,
closing the FDs and freeing the handles.

This change also switches the interrupt source lookup from FD based
matching to pointer based matching.
The original FD based matching allowed different intr_handle objects
with the same FD to share an interrupt source.
However, this was problematic:
* the cloned handle stored in the source came from the first registrant,
* the ucb_fn callback would receive this clone, not the (const)
  caller's handle,
* with multiple handles for the same FD, later registrants would get
  the wrong handle in their callback,

In practice, DPDK components use one intr_handle per FD, so pointer
based matching simply enforces this existing invariant.
The interrupt dispatch code continues to use FD based matching since
epoll returns the FD, not the handle pointer.

The unregister callback (ucb_fn) signature is updated to take a const
pointer, reflecting that the interrupt subsystem no longer owns the
handle. Note that with this change, the intr_handle parameter passed
to ucb_fn is the same pointer the caller registered with, callers
can simply use their own handle from cb_arg instead.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 doc/guides/rel_notes/release_26_11.rst |  1 +
 drivers/common/cnxk/roc_platform.h     |  1 -
 drivers/net/memif/memif_socket.c       |  5 ++--
 lib/eal/common/eal_common_interrupts.c | 26 --------------------
 lib/eal/freebsd/eal_interrupts.c       | 17 ++++---------
 lib/eal/include/rte_interrupts.h       | 17 +------------
 lib/eal/linux/eal_interrupts.c         | 33 ++++++++------------------
 7 files changed, 20 insertions(+), 80 deletions(-)

diff --git a/doc/guides/rel_notes/release_26_11.rst b/doc/guides/rel_notes/release_26_11.rst
index be758a0c02..78f9bb54aa 100644
--- a/doc/guides/rel_notes/release_26_11.rst
+++ b/doc/guides/rel_notes/release_26_11.rst
@@ -404,6 +404,7 @@ API Changes
     to invalid values instead of zero.
   * Added ``rte_intr_fd_close()`` and ``rte_intr_dev_fd_close()``
     helpers to close file descriptors and reset them to -1.
+  * ``rte_intr_unregister_callback_fn`` now receives a ``const`` interrupt handle.
 
 * mempool: When creating a mempool, the cache size can be freely specified
   (although still not exceed the number of elements),
diff --git a/drivers/common/cnxk/roc_platform.h b/drivers/common/cnxk/roc_platform.h
index aba9782023..149522608e 100644
--- a/drivers/common/cnxk/roc_platform.h
+++ b/drivers/common/cnxk/roc_platform.h
@@ -178,7 +178,6 @@ plt_thread_is_valid(plt_thread_t thr)
 #define plt_intr_type_get		rte_intr_type_get
 #define plt_intr_type_set		rte_intr_type_set
 #define plt_intr_instance_alloc		rte_intr_instance_alloc
-#define plt_intr_instance_dup		rte_intr_instance_dup
 #define plt_intr_instance_free		rte_intr_instance_free
 #define plt_intr_event_list_update	rte_intr_event_list_update
 #define plt_intr_max_intr_get		rte_intr_max_intr_get
diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index 87b23aa8b4..2d16decbef 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -572,13 +572,14 @@ memif_msg_enq_connected(struct rte_eth_dev *dev)
 }
 
 static void
-memif_intr_unregister_handler(struct rte_intr_handle *intr_handle, void *arg)
+memif_intr_unregister_handler(const struct rte_intr_handle *intr_handle, void *arg)
 {
 	struct memif_msg_queue_elt *elt;
 	struct memif_control_channel *cc = arg;
 
+	RTE_SET_USED(intr_handle);
 	/* close control channel fd */
-	rte_intr_fd_close(intr_handle);
+	rte_intr_fd_close(cc->intr_handle);
 	/* clear message queue */
 	while ((elt = TAILQ_FIRST(&cc->msg_queue)) != NULL) {
 		TAILQ_REMOVE(&cc->msg_queue, elt, next);
diff --git a/lib/eal/common/eal_common_interrupts.c b/lib/eal/common/eal_common_interrupts.c
index 6368ef1771..1208a2597d 100644
--- a/lib/eal/common/eal_common_interrupts.c
+++ b/lib/eal/common/eal_common_interrupts.c
@@ -103,32 +103,6 @@ struct rte_intr_handle *rte_intr_instance_alloc(uint32_t flags)
 	return NULL;
 }
 
-RTE_EXPORT_INTERNAL_SYMBOL(rte_intr_instance_dup)
-struct rte_intr_handle *rte_intr_instance_dup(const struct rte_intr_handle *src)
-{
-	struct rte_intr_handle *intr_handle;
-
-	if (src == NULL) {
-		EAL_LOG(DEBUG, "Source interrupt instance unallocated");
-		rte_errno = EINVAL;
-		return NULL;
-	}
-
-	intr_handle = rte_intr_instance_alloc(src->alloc_flags);
-	if (intr_handle != NULL) {
-		intr_handle->fd = src->fd;
-		intr_handle->dev_fd = src->dev_fd;
-		intr_handle->type = src->type;
-		intr_handle->max_intr = src->max_intr;
-		intr_handle->nb_efd = src->nb_efd;
-		intr_handle->efd_counter_size = src->efd_counter_size;
-		memcpy(intr_handle->efds, src->efds, src->nb_intr);
-		memcpy(intr_handle->elist, src->elist, src->nb_intr);
-	}
-
-	return intr_handle;
-}
-
 RTE_EXPORT_INTERNAL_SYMBOL(rte_intr_event_list_update)
 int rte_intr_event_list_update(struct rte_intr_handle *intr_handle, int size)
 {
diff --git a/lib/eal/freebsd/eal_interrupts.c b/lib/eal/freebsd/eal_interrupts.c
index bb2bc529f2..b29a2b174e 100644
--- a/lib/eal/freebsd/eal_interrupts.c
+++ b/lib/eal/freebsd/eal_interrupts.c
@@ -41,7 +41,7 @@ struct rte_intr_callback {
 
 struct rte_intr_source {
 	TAILQ_ENTRY(rte_intr_source) next;
-	struct rte_intr_handle *intr_handle; /**< interrupt handle */
+	const struct rte_intr_handle *intr_handle; /**< interrupt handle */
 	struct rte_intr_cb_list callbacks;  /**< user callbacks */
 	uint32_t active;
 };
@@ -105,7 +105,7 @@ rte_intr_callback_register(const struct rte_intr_handle *intr_handle,
 
 	/* find the source for this intr_handle */
 	TAILQ_FOREACH(src, &intr_sources, next) {
-		if (rte_intr_fd_get(src->intr_handle) == rte_intr_fd_get(intr_handle))
+		if (src->intr_handle == intr_handle)
 			break;
 	}
 
@@ -138,14 +138,7 @@ rte_intr_callback_register(const struct rte_intr_handle *intr_handle,
 				ret = -ENOMEM;
 				goto fail;
 			} else {
-				src->intr_handle = rte_intr_instance_dup(intr_handle);
-				if (src->intr_handle == NULL) {
-					EAL_LOG(ERR, "Can not create intr instance");
-					ret = -ENOMEM;
-					free(src);
-					src = NULL;
-					goto fail;
-				}
+				src->intr_handle = intr_handle;
 				TAILQ_INIT(&src->callbacks);
 				TAILQ_INSERT_TAIL(&intr_sources, src, next);
 			}
@@ -239,7 +232,7 @@ rte_intr_callback_unregister_pending(const struct rte_intr_handle *intr_handle,
 
 	/* check if the interrupt source for the fd is existent */
 	TAILQ_FOREACH(src, &intr_sources, next)
-		if (rte_intr_fd_get(src->intr_handle) == rte_intr_fd_get(intr_handle))
+		if (src->intr_handle == intr_handle)
 			break;
 
 	/* No interrupt source registered for the fd */
@@ -294,7 +287,7 @@ rte_intr_callback_unregister(const struct rte_intr_handle *intr_handle,
 
 	/* check if the interrupt source for the fd is existent */
 	TAILQ_FOREACH(src, &intr_sources, next)
-		if (rte_intr_fd_get(src->intr_handle) == rte_intr_fd_get(intr_handle))
+		if (src->intr_handle == intr_handle)
 			break;
 
 	/* No interrupt source registered for the fd */
diff --git a/lib/eal/include/rte_interrupts.h b/lib/eal/include/rte_interrupts.h
index d04c0c7b15..bb06180bea 100644
--- a/lib/eal/include/rte_interrupts.h
+++ b/lib/eal/include/rte_interrupts.h
@@ -70,7 +70,7 @@ typedef void (*rte_intr_callback_fn)(void *cb_arg);
  * Function to call after a callback is unregistered.
  * Can be used to close fd and free cb_arg.
  */
-typedef void (*rte_intr_unregister_callback_fn)(struct rte_intr_handle *intr_handle,
+typedef void (*rte_intr_unregister_callback_fn)(const struct rte_intr_handle *intr_handle,
 						void *cb_arg);
 
 /**
@@ -432,21 +432,6 @@ __rte_internal
 int
 rte_intr_cap_multiple(struct rte_intr_handle *intr_handle);
 
-/**
- * @internal
- * Creates a clone of src by allocating a new handle and copying src content.
- *
- * @param src
- *  Source interrupt handle to be cloned.
- *
- * @return
- *  - On success, address of interrupt handle.
- *  - On failure, NULL.
- */
-__rte_internal
-struct rte_intr_handle *
-rte_intr_instance_dup(const struct rte_intr_handle *src);
-
 /**
  * @internal
  * Set the device fd field of interrupt handle with user
diff --git a/lib/eal/linux/eal_interrupts.c b/lib/eal/linux/eal_interrupts.c
index 4cfaeba7fe..dcc2c0efdb 100644
--- a/lib/eal/linux/eal_interrupts.c
+++ b/lib/eal/linux/eal_interrupts.c
@@ -78,7 +78,7 @@ struct rte_intr_callback {
 
 struct rte_intr_source {
 	TAILQ_ENTRY(rte_intr_source) next;
-	struct rte_intr_handle *intr_handle; /**< interrupt handle */
+	const struct rte_intr_handle *intr_handle; /**< interrupt handle */
 	struct rte_intr_cb_list callbacks;  /**< user callbacks */
 	uint32_t active;
 };
@@ -511,7 +511,7 @@ rte_intr_callback_register(const struct rte_intr_handle *intr_handle,
 
 	/* check if there is at least one callback registered for the fd */
 	TAILQ_FOREACH(src, &intr_sources, next) {
-		if (rte_intr_fd_get(src->intr_handle) == rte_intr_fd_get(intr_handle)) {
+		if (src->intr_handle == intr_handle) {
 			/* we had no interrupts for this */
 			if (TAILQ_EMPTY(&src->callbacks))
 				wake_thread = 1;
@@ -531,22 +531,12 @@ rte_intr_callback_register(const struct rte_intr_handle *intr_handle,
 			free(callback);
 			callback = NULL;
 		} else {
-			src->intr_handle = rte_intr_instance_dup(intr_handle);
-			if (src->intr_handle == NULL) {
-				EAL_LOG(ERR, "Can not create intr instance");
-				ret = -ENOMEM;
-				free(callback);
-				callback = NULL;
-				free(src);
-				src = NULL;
-			} else {
-				TAILQ_INIT(&src->callbacks);
-				TAILQ_INSERT_TAIL(&(src->callbacks), callback,
-						  next);
-				TAILQ_INSERT_TAIL(&intr_sources, src, next);
-				wake_thread = 1;
-				ret = 0;
-			}
+			src->intr_handle = intr_handle;
+			TAILQ_INIT(&src->callbacks);
+			TAILQ_INSERT_TAIL(&(src->callbacks), callback, next);
+			TAILQ_INSERT_TAIL(&intr_sources, src, next);
+			wake_thread = 1;
+			ret = 0;
 		}
 	}
 
@@ -584,7 +574,7 @@ rte_intr_callback_unregister_pending(const struct rte_intr_handle *intr_handle,
 
 	/* check if the interrupt source for the fd is existent */
 	TAILQ_FOREACH(src, &intr_sources, next) {
-		if (rte_intr_fd_get(src->intr_handle) == rte_intr_fd_get(intr_handle))
+		if (src->intr_handle == intr_handle)
 			break;
 	}
 
@@ -635,7 +625,7 @@ rte_intr_callback_unregister(const struct rte_intr_handle *intr_handle,
 
 	/* check if the interrupt source for the fd is existent */
 	TAILQ_FOREACH(src, &intr_sources, next)
-		if (rte_intr_fd_get(src->intr_handle) == rte_intr_fd_get(intr_handle))
+		if (src->intr_handle == intr_handle)
 			break;
 
 	/* No interrupt source registered for the fd */
@@ -666,7 +656,6 @@ rte_intr_callback_unregister(const struct rte_intr_handle *intr_handle,
 		/* all callbacks for that source are removed. */
 		if (TAILQ_EMPTY(&src->callbacks)) {
 			TAILQ_REMOVE(&intr_sources, src, next);
-			rte_intr_instance_free(src->intr_handle);
 			free(src);
 		}
 	}
@@ -922,7 +911,6 @@ eal_intr_source_remove_and_free(struct rte_intr_source *src)
 	}
 
 	/* Free the interrupt source */
-	rte_intr_instance_free(src->intr_handle);
 	free(src);
 }
 
@@ -1067,7 +1055,6 @@ eal_intr_process_interrupts(struct epoll_event *events, int nfds)
 		/* all callbacks for that source are removed. */
 		if (TAILQ_EMPTY(&src->callbacks)) {
 			TAILQ_REMOVE(&intr_sources, src, next);
-			rte_intr_instance_free(src->intr_handle);
 			free(src);
 		}
 
-- 
2.54.0

[PATCH v2 04/12] bus/pci: reset VFIO device FD after release

[David Marchand]

From: Stephen Hemminger <stephen@networkplumber.org>

The VFIO device fd is closed by dev_vfio_release_device() but is left
in the interrupt handle, so freeing the handle on cleanup logs a bogus
FD leak warning.

Mark dev_fd invalid after release.

In secondary process the request handle is unused and does not have to
be set in the first place.

Fixes: d770ee9caed0 ("interrupts: warn on leaked FDs")

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/pci/linux/pci_vfio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
index fb35bc1d49..c81d20200d 100644
--- a/drivers/bus/pci/linux/pci_vfio.c
+++ b/drivers/bus/pci/linux/pci_vfio.c
@@ -998,8 +998,6 @@ pci_vfio_map_resource_secondary(struct rte_pci_device *dev)
 	/* we need save vfio_dev_fd, so it can be used during release */
 	if (rte_intr_dev_fd_set(dev->intr_handle, vfio_dev_fd))
 		goto err_map;
-	if (rte_intr_dev_fd_set(dev->vfio_req_intr_handle, vfio_dev_fd))
-		goto err_map;
 
 	return 0;
 err_map:
@@ -1103,6 +1101,7 @@ pci_vfio_unmap_resource_primary(struct rte_pci_device *dev)
 		PCI_LOG(ERR, "Cannot release VFIO device");
 		return ret;
 	}
+	rte_intr_dev_fd_set(dev->intr_handle, -1);
 
 	vfio_res_list =
 		RTE_TAILQ_CAST(dev_vfio_tailq.head, mapped_pci_res_list);
@@ -1142,6 +1141,7 @@ pci_vfio_unmap_resource_secondary(struct rte_pci_device *dev)
 		PCI_LOG(ERR, "Cannot release VFIO device");
 		return ret;
 	}
+	rte_intr_dev_fd_set(dev->intr_handle, -1);
 
 	vfio_res_list =
 		RTE_TAILQ_CAST(dev_vfio_tailq.head, mapped_pci_res_list);
-- 
2.54.0

[PATCH v2 05/12] net/mana: reset FD in interrupt handle

[David Marchand]

The interrupt handle references the ibverbs async_fd which is owned
by the ibverbs context, not by this driver. Clear the FD reference
before freeing the interrupt handle to avoid a spurious leak warning.

Fixes: d770ee9caed0 ("interrupts: warn on leaked FDs")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/mana/mana.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/mana/mana.c b/drivers/net/mana/mana.c
index 30b0ce90d6..0ea6d9a54a 100644
--- a/drivers/net/mana/mana.c
+++ b/drivers/net/mana/mana.c
@@ -1796,6 +1796,8 @@ mana_reset_exit(struct mana_priv *priv)
 		else
 			DRV_LOG(DEBUG, "%d intr callback(s) removed", ret);
 
+		/* fd is owned by ibverbs, only clear reference here. */
+		rte_intr_fd_set(priv->intr_handle, -1);
 		rte_intr_instance_free(priv->intr_handle);
 		priv->intr_handle = NULL;
 	}
@@ -1915,6 +1917,8 @@ mana_intr_uninstall(struct mana_priv *priv)
 		return ret;
 	}
 
+	/* fd is owned by ibverbs, only clear reference here. */
+	rte_intr_fd_set(priv->intr_handle, -1);
 	rte_intr_instance_free(priv->intr_handle);
 	priv->intr_handle = NULL;
 
-- 
2.54.0

[PATCH v2 06/12] net/memif: fix some interrupt FD leaks

[David Marchand]

Fix several resource leaks in error handling and cleanup paths:

In memif_socket_create(), when rte_intr_callback_register() fails,
return via error path instead of returning directly, ensuring sockfd,
sock and intr_handle are properly cleaned up.

In memif_socket_init(), when rte_hash_add_key_data() fails, clean up
the newly created socket including unregistering callback, closing fd
and freeing intr_handle for listener sockets.

In memif_socket_remove_device(), unregister the listener callback and
close the socket fd before freeing the interrupt handle.

Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/memif/memif_socket.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index 2d16decbef..bd52a95473 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -1051,7 +1051,7 @@ memif_socket_create(char *key, uint8_t listener, bool is_abstract, uid_t owner_u
 		if (ret < 0) {
 			MIF_LOG(ERR, "Failed to register interrupt "
 				"callback for listener socket");
-			return NULL;
+			goto error;
 		}
 	}
 
@@ -1115,6 +1115,14 @@ memif_socket_init(struct rte_eth_dev *dev, const char *socket_filename)
 		ret = rte_hash_add_key_data(hash, key, socket);
 		if (ret < 0) {
 			MIF_LOG(ERR, "Failed to add socket to socket hash.");
+			if (socket->listener) {
+				rte_intr_callback_unregister(socket->intr_handle,
+					memif_listener_handler, socket);
+				close(rte_intr_fd_get(socket->intr_handle));
+				rte_intr_fd_set(socket->intr_handle, -1);
+				rte_intr_instance_free(socket->intr_handle);
+			}
+			rte_free(socket);
 			return ret;
 		}
 	}
@@ -1180,8 +1188,13 @@ memif_socket_remove_device(struct rte_eth_dev *dev)
 				MIF_LOG(ERR, "Failed to remove socket file: %s",
 					socket->filename);
 		}
-		if (pmd->role != MEMIF_ROLE_CLIENT)
+		if (pmd->role != MEMIF_ROLE_CLIENT) {
+			rte_intr_callback_unregister(socket->intr_handle, memif_listener_handler,
+				socket);
+			close(rte_intr_fd_get(socket->intr_handle));
+			rte_intr_fd_set(socket->intr_handle, -1);
 			rte_intr_instance_free(socket->intr_handle);
+		}
 		rte_free(socket);
 	}
 }
-- 
2.54.0

[PATCH v2 07/12] net/memif: reset FD in interrupt handle

[David Marchand]

Clear FD references before freeing interrupt handles to avoid
spurious leak warnings. In error paths where the socket fd is
closed separately, clear the reference. In cleanup paths, use
rte_intr_fd_close() to close and clear in one call.

Fixes: d770ee9caed0 ("interrupts: warn on leaked FDs")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/memif/memif_socket.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index bd52a95473..b47c7a3b1b 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -638,7 +638,7 @@ memif_disconnect(struct rte_eth_dev *dev)
 							pmd->cc,
 							memif_intr_unregister_handler);
 			} else if (ret > 0) {
-				close(rte_intr_fd_get(ih));
+				rte_intr_fd_close(ih);
 				rte_intr_instance_free(ih);
 				rte_free(pmd->cc);
 			}
@@ -962,6 +962,8 @@ memif_listener_handler(void *arg)
 	if (cc != NULL) {
 		rte_intr_callback_unregister(cc->intr_handle, memif_intr_handler,
 					 cc);
+		/* fd is closed below, clear reference here. */
+		rte_intr_fd_set(cc->intr_handle, -1);
 		rte_intr_instance_free(cc->intr_handle);
 		rte_free(cc);
 	}
@@ -1060,6 +1062,8 @@ memif_socket_create(char *key, uint8_t listener, bool is_abstract, uid_t owner_u
  error:
 	MIF_LOG(ERR, "Failed to setup socket %s: %s", key, strerror(errno));
 	if (sock != NULL) {
+		/* fd is closed below, clear reference here. */
+		rte_intr_fd_set(sock->intr_handle, -1);
 		rte_intr_instance_free(sock->intr_handle);
 		rte_free(sock);
 	}
@@ -1118,8 +1122,7 @@ memif_socket_init(struct rte_eth_dev *dev, const char *socket_filename)
 			if (socket->listener) {
 				rte_intr_callback_unregister(socket->intr_handle,
 					memif_listener_handler, socket);
-				close(rte_intr_fd_get(socket->intr_handle));
-				rte_intr_fd_set(socket->intr_handle, -1);
+				rte_intr_fd_close(socket->intr_handle);
 				rte_intr_instance_free(socket->intr_handle);
 			}
 			rte_free(socket);
@@ -1191,8 +1194,7 @@ memif_socket_remove_device(struct rte_eth_dev *dev)
 		if (pmd->role != MEMIF_ROLE_CLIENT) {
 			rte_intr_callback_unregister(socket->intr_handle, memif_listener_handler,
 				socket);
-			close(rte_intr_fd_get(socket->intr_handle));
-			rte_intr_fd_set(socket->intr_handle, -1);
+			rte_intr_fd_close(socket->intr_handle);
 			rte_intr_instance_free(socket->intr_handle);
 		}
 		rte_free(socket);
@@ -1290,6 +1292,8 @@ memif_connect_client(struct rte_eth_dev *dev)
 		sockfd = -1;
 	}
 	if (pmd->cc != NULL) {
+		/* fd is closed above, clear reference here. */
+		rte_intr_fd_set(pmd->cc->intr_handle, -1);
 		rte_intr_instance_free(pmd->cc->intr_handle);
 		rte_free(pmd->cc);
 		pmd->cc = NULL;
-- 
2.54.0

[PATCH v2 08/12] net/sxe2: reset FD in interrupt handle

[David Marchand]

The fd passed to sxe2_intr_handler_create() is not owned by this
function. Clear the FD reference before freeing the interrupt handle
on error to avoid a spurious leak warning.

Fixes: d770ee9caed0 ("interrupts: warn on leaked FDs")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/sxe2/sxe2_irq.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/sxe2/sxe2_irq.c b/drivers/net/sxe2/sxe2_irq.c
index 256949e74e..0a16c263c1 100644
--- a/drivers/net/sxe2/sxe2_irq.c
+++ b/drivers/net/sxe2/sxe2_irq.c
@@ -387,6 +387,8 @@ sxe2_intr_handler_create(int32_t fd, rte_intr_callback_fn cb, void *cb_arg)
 
 	return tmp_intr_handle;
 err:
+	/* fd is not owned by this function, only clear reference here. */
+	rte_intr_fd_set(tmp_intr_handle, -1);
 	rte_intr_instance_free(tmp_intr_handle);
 	return NULL;
 }
-- 
2.54.0

[PATCH v2 09/12] raw/ifpga: reset FD in interrupt handle

[David Marchand]

The interrupt handle fd points to efd[0] which is already closed
by rte_intr_efd_disable(). Clear the FD reference before freeing
the interrupt handle to avoid a spurious leak warning.

Fixes: d770ee9caed0 ("interrupts: warn on leaked FDs")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/raw/ifpga/ifpga_rawdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c
index c4143bfb3e..d44141824f 100644
--- a/drivers/raw/ifpga/ifpga_rawdev.c
+++ b/drivers/raw/ifpga/ifpga_rawdev.c
@@ -1421,6 +1421,8 @@ ifpga_unregister_msix_irq(struct ifpga_rawdev *dev, enum ifpga_irq_type type,
 			type == IFPGA_FME_IRQ ? "FME" : "AFU",
 			type == IFPGA_FME_IRQ ? 0 : vec_start);
 	} else {
+		/* fd was pointing to efd[0] which is closed by rte_intr_efd_disable(). */
+		rte_intr_fd_set(*intr_handle, -1);
 		rte_intr_instance_free(*intr_handle);
 		*intr_handle = NULL;
 	}
-- 
2.54.0

[PATCH v2 10/12] eal/linux: fix closing of event FDs not owned by EAL

[David Marchand]

From: Anatoly Burakov <anatoly.burakov@intel.com>

rte_intr_efd_disable() decides whether to close the event FDs based on
max_intr being greater than nb_efd. This is a side effect of how
rte_intr_efd_enable() fills the handle for VFIO MSI-X, where an extra
slot is reserved for the misc interrupt, and only indirectly identifies
the eventfds that EAL created itself.

Any driver filling the event FD array for a VDEV or EXT handle and
setting max_intr the same way gets its descriptors closed by EAL even
though it owns them. The virtio-user driver does exactly this, so its
callfds are closed by rte_intr_efd_disable() and then closed again by
the driver on uninit.

Check the interrupt handle type instead, so that only the eventfds
created by rte_intr_efd_enable() for VFIO MSI-X are closed.

Fixes: b9e0662a931d ("eal/linux: add interrupt API for drivers")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: David Marchand <david.marchand@redhat.com>
---
 lib/eal/linux/eal_interrupts.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/eal/linux/eal_interrupts.c b/lib/eal/linux/eal_interrupts.c
index dcc2c0efdb..fa658f5a3c 100644
--- a/lib/eal/linux/eal_interrupts.c
+++ b/lib/eal/linux/eal_interrupts.c
@@ -1607,7 +1607,11 @@ rte_intr_efd_disable(struct rte_intr_handle *intr_handle)
 	uint32_t i;
 
 	rte_intr_free_epoll_fd(intr_handle);
-	if (rte_intr_max_intr_get(intr_handle) > rte_intr_nb_efd_get(intr_handle)) {
+	/* Only eventfds created by rte_intr_efd_enable() for MSI-X are owned
+	 * by the handle. In other modes the entries reference fds owned by
+	 * the device or the driver and must not be closed here.
+	 */
+	if (rte_intr_type_get(intr_handle) == RTE_INTR_HANDLE_VFIO_MSIX) {
 		for (i = 0; i < (uint32_t)rte_intr_nb_efd_get(intr_handle); i++)
 			close(rte_intr_efds_index_get(intr_handle, i));
 	}
-- 
2.54.0

[PATCH v2 11/12] eal/linux: fix eventfd leaks when enabling interrupts

[David Marchand]

From: Anatoly Burakov <anatoly.burakov@intel.com>

When enabling MSI-X eventfds, a failure after creating some descriptors
leaves them open. The eventfd count is recorded only after all allocations
succeed, so disabling eventfds cannot clean up a partial allocation.

Calling enable again without disabling first also leaks descriptors by
overwriting the existing eventfd entries.

Reject repeated MSI-X enable calls with EBUSY before modifying the handle.
Update the eventfd count as each descriptor is created, so that on failure
every eventfd created by the call can be closed, its entry cleared, and the
count reset.

Fixes: b9e0662a931d ("eal/linux: add interrupt API for drivers")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: David Marchand <david.marchand@redhat.com>
---
 lib/eal/linux/eal_interrupts.c | 46 +++++++++++++++++++++++++---------
 1 file changed, 34 insertions(+), 12 deletions(-)

diff --git a/lib/eal/linux/eal_interrupts.c b/lib/eal/linux/eal_interrupts.c
index fa658f5a3c..73f23c314b 100644
--- a/lib/eal/linux/eal_interrupts.c
+++ b/lib/eal/linux/eal_interrupts.c
@@ -1556,31 +1556,44 @@ RTE_EXPORT_INTERNAL_SYMBOL(rte_intr_efd_enable)
 int
 rte_intr_efd_enable(struct rte_intr_handle *intr_handle, uint32_t nb_efd)
 {
-	uint32_t i;
-	int fd;
 	uint32_t n = RTE_MIN(nb_efd, (uint32_t)RTE_MAX_RXTX_INTR_VEC_ID);
+	int ret;
 
 	assert(nb_efd != 0);
 
 	if (rte_intr_type_get(intr_handle) == RTE_INTR_HANDLE_VFIO_MSIX) {
-		for (i = 0; i < n; i++) {
-			fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
+		if (rte_intr_nb_efd_get(intr_handle) != 0) {
+			EAL_LOG(ERR, "Interrupt eventfds are already enabled");
+			return -EBUSY;
+		}
+
+		for (uint32_t i = 0; i < n; i++) {
+			int fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
+
 			if (fd < 0) {
+				ret = -errno;
 				EAL_LOG(ERR,
 					"can't setup eventfd, error %i (%s)",
 					errno, strerror(errno));
-				return -errno;
+				goto error;
 			}
 
-			if (rte_intr_efds_index_set(intr_handle, i, fd))
-				return -rte_errno;
+			/* Record each eventfd as it is created so that the
+			 * error path can close exactly those that exist.
+			 */
+			if (rte_intr_efds_index_set(intr_handle, i, fd) ||
+					rte_intr_nb_efd_set(intr_handle, i + 1)) {
+				ret = -rte_errno;
+				close(fd);
+				rte_intr_efds_index_set(intr_handle, i, -1);
+				goto error;
+			}
 		}
 
-		if (rte_intr_nb_efd_set(intr_handle, n))
-			return -rte_errno;
-
-		if (rte_intr_max_intr_set(intr_handle, NB_OTHER_INTR + n))
-			return -rte_errno;
+		if (rte_intr_max_intr_set(intr_handle, NB_OTHER_INTR + n)) {
+			ret = -rte_errno;
+			goto error;
+		}
 	} else if (rte_intr_type_get(intr_handle) == RTE_INTR_HANDLE_VDEV) {
 		/* only check, initialization would be done in vdev driver.*/
 		if ((uint64_t)rte_intr_efd_counter_size_get(intr_handle) >
@@ -1598,6 +1611,15 @@ rte_intr_efd_enable(struct rte_intr_handle *intr_handle, uint32_t nb_efd)
 	}
 
 	return 0;
+
+error:
+	for (int i = 0; i < rte_intr_nb_efd_get(intr_handle); i++) {
+		close(rte_intr_efds_index_get(intr_handle, i));
+		rte_intr_efds_index_set(intr_handle, i, -1);
+	}
+	rte_intr_nb_efd_set(intr_handle, 0);
+
+	return ret;
 }
 
 RTE_EXPORT_INTERNAL_SYMBOL(rte_intr_efd_disable)
-- 
2.54.0

[PATCH v2 12/12] interrupts: warn on leaked event FDs

[David Marchand]

From: Anatoly Burakov <anatoly.burakov@intel.com>

Extend the leak warning in rte_intr_instance_free() to the event FD
array, in the same way as the handle FD and device FD.

rte_intr_efd_disable() used to close the eventfds it owns but left
the stale descriptors in the array, and never cleared the entry that
merely references the handle FD in the non-MSI-X case.
For all types but VDEV, reset all entries in use to -1 so a properly
disabled handle is not reported.

Drivers filling the event FD array themselves are expected to reset
the entries once the descriptors are closed or no longer referenced.

Suggested-by: David Marchand <david.marchand@redhat.com>
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since Anatoly v4:
- added check on type != VDEV when disabling,

---
 lib/eal/common/eal_common_interrupts.c | 5 +++++
 lib/eal/include/rte_interrupts.h       | 7 ++++++-
 lib/eal/linux/eal_interrupts.c         | 9 ++++++---
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/lib/eal/common/eal_common_interrupts.c b/lib/eal/common/eal_common_interrupts.c
index 1208a2597d..f3eee636df 100644
--- a/lib/eal/common/eal_common_interrupts.c
+++ b/lib/eal/common/eal_common_interrupts.c
@@ -167,6 +167,11 @@ void rte_intr_instance_free(struct rte_intr_handle *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));
+	for (int i = 0; i < intr_handle->nb_intr; i++) {
+		if (intr_handle->efds[i] >= 0)
+			EAL_LOG(NOTICE, "Some interrupt handle is leaking an event FD: %d (index %d)",
+				intr_handle->efds[i], i);
+	}
 	if (RTE_INTR_INSTANCE_USES_RTE_MEMORY(intr_handle->alloc_flags)) {
 		rte_free(intr_handle->efds);
 		rte_free(intr_handle->elist);
diff --git a/lib/eal/include/rte_interrupts.h b/lib/eal/include/rte_interrupts.h
index bb06180bea..85b5546dee 100644
--- a/lib/eal/include/rte_interrupts.h
+++ b/lib/eal/include/rte_interrupts.h
@@ -389,6 +389,7 @@ rte_intr_efd_enable(struct rte_intr_handle *intr_handle, uint32_t nb_efd);
  * @internal
  * It disables the packet I/O interrupt event.
  * It deletes registered eventfds and closes the open fds.
+ * All event fd entries in use are reset to -1.
  *
  * @param intr_handle
  *   Pointer to the interrupt handle.
@@ -605,12 +606,16 @@ rte_intr_efd_counter_size_get(const struct rte_intr_handle *intr_handle);
  * @internal
  * Set the event fd array index with the given fd.
  *
+ * Entries still holding a valid fd when the instance is freed are reported
+ * as leaks, so callers managing the array themselves must reset entries
+ * to -1 once the fd is closed or no longer referenced.
+ *
  * @param intr_handle
  *  pointer to the interrupt handle.
  * @param index
  *  efds array index to be set
  * @param fd
- *  event fd
+ *  event fd, or -1 to clear the entry
  *
  * @return
  *  - On success, zero.
diff --git a/lib/eal/linux/eal_interrupts.c b/lib/eal/linux/eal_interrupts.c
index 73f23c314b..dfe1660dd0 100644
--- a/lib/eal/linux/eal_interrupts.c
+++ b/lib/eal/linux/eal_interrupts.c
@@ -1626,17 +1626,20 @@ RTE_EXPORT_INTERNAL_SYMBOL(rte_intr_efd_disable)
 void
 rte_intr_efd_disable(struct rte_intr_handle *intr_handle)
 {
-	uint32_t i;
-
 	rte_intr_free_epoll_fd(intr_handle);
 	/* Only eventfds created by rte_intr_efd_enable() for MSI-X are owned
 	 * by the handle. In other modes the entries reference fds owned by
 	 * the device or the driver and must not be closed here.
 	 */
 	if (rte_intr_type_get(intr_handle) == RTE_INTR_HANDLE_VFIO_MSIX) {
-		for (i = 0; i < (uint32_t)rte_intr_nb_efd_get(intr_handle); i++)
+		for (int i = 0; i < rte_intr_nb_efd_get(intr_handle); i++)
 			close(rte_intr_efds_index_get(intr_handle, i));
 	}
+	/* VDEV is responsible for efds, see rte_intr_efd_enable(). */
+	if (rte_intr_type_get(intr_handle) != RTE_INTR_HANDLE_VDEV) {
+		for (int i = 0; i < rte_intr_nb_efd_get(intr_handle); i++)
+			rte_intr_efds_index_set(intr_handle, i, -1);
+	}
 	rte_intr_nb_efd_set(intr_handle, 0);
 	rte_intr_max_intr_set(intr_handle, 0);
 }
-- 
2.54.0

Re: [PATCH v2 02/12] eal/linux: reset FD in interrupt handle for uevent

[Burakov, Anatoly]

On 10/24/2026 3:21 PM, David Marchand wrote:
> Clear the warning when freeing the interrupt handle used for the uevent
> handler.
> 
> Fixes: d770ee9caed0 ("interrupts: warn on leaked FDs")
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

Re: [PATCH v2 03/12] interrupts: do not duplicate interrupt handles

[Burakov, Anatoly]

On 10/24/2026 3:21 PM, David Marchand wrote:
> The FD leak warnings recently added have revealed an interesting issue.
> 
> An interrupt handle is allocated and living usually in a bus device object
> (or some standalone code, like the alarm code).
> All the logic around the referenced FDs in this object are specific to
> the callers, and those FDs must then be closed in the callers context.
> 
> On the other hand, the interrupt subsystem works with copies of interrupt
> handle objects passed to it via rte_intr_callback_register/unregister.
> 
> When releasing this clone of the interrupt handle, the FDs are reported
> as leaked, though there is no way for the interrupt subsystem itself to
> know what to do with those FDs.
> 
> Instead of creating a clone, reference the interrupt handle directly.
> The callers are responsible for unregistering those handles,
> closing the FDs and freeing the handles.
> 
> This change also switches the interrupt source lookup from FD based
> matching to pointer based matching.
> The original FD based matching allowed different intr_handle objects
> with the same FD to share an interrupt source.
> However, this was problematic:
> * the cloned handle stored in the source came from the first registrant,
> * the ucb_fn callback would receive this clone, not the (const)
>    caller's handle,
> * with multiple handles for the same FD, later registrants would get
>    the wrong handle in their callback,
> 
> In practice, DPDK components use one intr_handle per FD, so pointer
> based matching simply enforces this existing invariant.
> The interrupt dispatch code continues to use FD based matching since
> epoll returns the FD, not the handle pointer.
> 
> The unregister callback (ucb_fn) signature is updated to take a const
> pointer, reflecting that the interrupt subsystem no longer owns the
> handle. Note that with this change, the intr_handle parameter passed
> to ucb_fn is the same pointer the caller registered with, callers
> can simply use their own handle from cb_arg instead.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

However, my patches fixed the issue in instance_dup without removing the 
API. Those fixes should probably be backported to stable in some form.

-- 
Thanks,
Anatoly

Re: [PATCH v2 04/12] bus/pci: reset VFIO device FD after release

[Burakov, Anatoly]

On 10/24/2026 3:21 PM, David Marchand wrote:
> From: Stephen Hemminger <stephen@networkplumber.org>
> 
> The VFIO device fd is closed by dev_vfio_release_device() but is left
> in the interrupt handle, so freeing the handle on cleanup logs a bogus
> FD leak warning.
> 
> Mark dev_fd invalid after release.
> 
> In secondary process the request handle is unused and does not have to
> be set in the first place.
> 
> Fixes: d770ee9caed0 ("interrupts: warn on leaked FDs")
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

Re: [PATCH v2 03/12] interrupts: do not duplicate interrupt handles

[David Marchand]

On Sat, 24 Oct 2026 at 16:11, Burakov, Anatoly
<anatoly.burakov@intel.com> wrote:
>
> On 10/24/2026 3:21 PM, David Marchand wrote:
> > The FD leak warnings recently added have revealed an interesting issue.
> >
> > An interrupt handle is allocated and living usually in a bus device object
> > (or some standalone code, like the alarm code).
> > All the logic around the referenced FDs in this object are specific to
> > the callers, and those FDs must then be closed in the callers context.
> >
> > On the other hand, the interrupt subsystem works with copies of interrupt
> > handle objects passed to it via rte_intr_callback_register/unregister.
> >
> > When releasing this clone of the interrupt handle, the FDs are reported
> > as leaked, though there is no way for the interrupt subsystem itself to
> > know what to do with those FDs.
> >
> > Instead of creating a clone, reference the interrupt handle directly.
> > The callers are responsible for unregistering those handles,
> > closing the FDs and freeing the handles.
> >
> > This change also switches the interrupt source lookup from FD based
> > matching to pointer based matching.
> > The original FD based matching allowed different intr_handle objects
> > with the same FD to share an interrupt source.
> > However, this was problematic:
> > * the cloned handle stored in the source came from the first registrant,
> > * the ucb_fn callback would receive this clone, not the (const)
> >    caller's handle,
> > * with multiple handles for the same FD, later registrants would get
> >    the wrong handle in their callback,
> >
> > In practice, DPDK components use one intr_handle per FD, so pointer
> > based matching simply enforces this existing invariant.
> > The interrupt dispatch code continues to use FD based matching since
> > epoll returns the FD, not the handle pointer.
> >
> > The unregister callback (ucb_fn) signature is updated to take a const
> > pointer, reflecting that the interrupt subsystem no longer owns the
> > handle. Note that with this change, the intr_handle parameter passed
> > to ucb_fn is the same pointer the caller registered with, callers
> > can simply use their own handle from cb_arg instead.
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
>
> However, my patches fixed the issue in instance_dup without removing the
> API. Those fixes should probably be backported to stable in some form.

Indeed.
I can take your first two patches right after the 2 first fixes in
this series, right before removing the duplicate API.
That's just a little rebase for me (I'll double check compilation is
not broken between patches).

WDYT?


-- 
David Marchand

Re: [PATCH v2 00/12] Interrupt fixes for leaks and warnings

[David Marchand]

On Sat, 24 Oct 2026 at 15:22, David Marchand <david.marchand@redhat.com> wrote:
>
> Following commit d770ee9caed0 ("interrupts: warn on leaked FDs"), a
> number of leaks, but also false positive warnings have been identified.
>
> The most notable change in this series is the removal of the interrupt
> handle duplication. I concluded the principle is wrong (see patch 2
> commitlog).
> This will need some careful reviews.
>

As discussed, I slipped Anatoly fixes in this series when merging.
Thanks for the reviews in EAL.

Series applied.


-- 
David Marchand