On 10/23/2026 1:42 PM, David Marchand wrote:
Hello,
On Fri, 23 Oct 2026 at 11:37, Anatoly Burakov <[email protected]> wrote:
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.
On failure, close all eventfds created by the call, clear their entries
and reset the eventfd count while preserving the original error.
Fixes: b9e0662a931d ("eal/linux: add interrupt API for drivers")
Cc: [email protected]
Signed-off-by: Anatoly Burakov <[email protected]>
Good catch.
How did you find this issue?
I didn't :D Bruce's AI review [1] caught the (somewhat confusingly
formulated) bug, and I went on investigating.
[1]
https://patches.dpdk.org/project/dpdk/patch/595c771fac2dda2d602c01ba171d56f3419db917.1792592724.git.anatoly.bura...@intel.com/#186792
I did not put warnings on the efd side, but maybe worth extending
d770ee9caed0 ("interrupts: warn on leaked FDs").
Probably, yes.
One comment below.
---
lib/eal/linux/eal_interrupts.c | 39 +++++++++++++++++++++++++++-------
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/lib/eal/linux/eal_interrupts.c b/lib/eal/linux/eal_interrupts.c
index 4cfaeba7fe0..ff05d649607 100644
--- a/lib/eal/linux/eal_interrupts.c
+++ b/lib/eal/linux/eal_interrupts.c
@@ -1570,30 +1570,43 @@ int
rte_intr_efd_enable(struct rte_intr_handle *intr_handle, uint32_t nb_efd)
{
uint32_t i;
- int fd;
+ int fd, ret;
uint32_t n = RTE_MIN(nb_efd, (uint32_t)RTE_MAX_RXTX_INTR_VEC_ID);
assert(nb_efd != 0);
if (rte_intr_type_get(intr_handle) == RTE_INTR_HANDLE_VFIO_MSIX) {
+ if (rte_intr_nb_efd_get(intr_handle) != 0) {
+ EAL_LOG(ERR, "Interrupt eventfds are already enabled");
+ return -EBUSY;
+ }
+
for (i = 0; i < n; i++) {
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;
+ if (rte_intr_efds_index_set(intr_handle, i, fd)) {
+ ret = -rte_errno;
+ close(fd);
+ goto error;
+ }
}
- if (rte_intr_nb_efd_set(intr_handle, n))
- return -rte_errno;
+ if (rte_intr_nb_efd_set(intr_handle, n)) {
+ ret = -rte_errno;
+ goto error;
+ }
- 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) >
@@ -1611,6 +1624,16 @@ rte_intr_efd_enable(struct rte_intr_handle *intr_handle,
uint32_t nb_efd)
}
return 0;
+
+error:
+ while (i > 0) {
The i variable should have been declared only in the scope of VFIO
type, in the first place.
Using i here could blow up later when this error label is used for
other interrupt types.
As mentionned in the commitlog, could we accumulate the number of
valid efds in the vfio setup loop?
The cleanup code here could then loop on rte_intr_nb_efd_get.
I'll see if this can be improved.
Thanks for the review!
+ 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.52.0
--
Thanks,
Anatoly
