|WARNING| [RFC,3/3] eal/linux: close alarm timerfd on cleanup

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/171590

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-17

# DPDK Patch Review

## PATCH 1/3: eal/linux: do not warn when freeing interrupt source copy

### ERRORS

None identified.

### WARNINGS

None identified.

### INFO

The patch correctly addresses a false-positive FD leak warning. The interrupt source holds a copy of the caller's interrupt handle (created via `rte_intr_instance_dup()`), and the file descriptors in that copy are owned by the caller, not the copy. The solution of clearing the fds before freeing the copy is appropriate.

The new helper function `eal_intr_source_free_handle()` correctly:
- Sets `fd` to -1 via `rte_intr_fd_set()`
- Sets `dev_fd` to -1 via `rte_intr_dev_fd_set()`
- Frees the instance via `rte_intr_instance_free()`

All call sites that previously called `rte_intr_instance_free()` on `src->intr_handle` are updated to use the new helper.

---

## PATCH 2/3: bus/pci: reset VFIO device FD after release

### ERRORS

**Resource leak on error path:**

In `pci_vfio_unmap_resource_primary()`, if `dev_vfio_release_device()` returns an error, the function returns early without closing the device fd or marking it invalid:

```c
ret = dev_vfio_release_device(dev);
if (ret < 0) {
    PCI_LOG(ERR, "Cannot release VFIO device");
    return ret;
}
rte_intr_dev_fd_set(dev->intr_handle, -1);
```

If `dev_vfio_release_device()` fails, the fd remains open and the function returns with the error, leaving the fd in the interrupt handle. On subsequent cleanup (when the interrupt handle is eventually freed), this will trigger the FD leak warning the patch series is trying to fix.

The same issue exists in `pci_vfio_unmap_resource_secondary()`:

```c
ret = dev_vfio_release_device(dev);
if (ret < 0) {
    PCI_LOG(ERR, "Cannot release VFIO device");
    return ret;
}
rte_intr_dev_fd_set(dev->intr_handle, -1);
rte_intr_dev_fd_set(dev->vfio_req_intr_handle, -1);
```

**Suggested fix:**

Set the fd to -1 *before* checking the error, or set it unconditionally after `dev_vfio_release_device()` regardless of success/failure:

```c
ret = dev_vfio_release_device(dev);
rte_intr_dev_fd_set(dev->intr_handle, -1);
if (ret < 0) {
    PCI_LOG(ERR, "Cannot release VFIO device");
    return ret;
}
```

Or handle it in the error path:

```c
ret = dev_vfio_release_device(dev);
if (ret < 0) {
    PCI_LOG(ERR, "Cannot release VFIO device");
    rte_intr_dev_fd_set(dev->intr_handle, -1);
    return ret;
}
rte_intr_dev_fd_set(dev->intr_handle, -1);
```

The first approach (unconditional after release) is cleaner since `dev_vfio_release_device()` closes the fd regardless of whether it succeeds or fails in other cleanup.

### WARNINGS

None identified beyond the error above.

### INFO

The rationale is correct: `dev_vfio_release_device()` closes the device fd, but leaves it in the interrupt handle, causing a false-positive FD leak warning when the handle is freed later. Marking the fd as -1 after closing is the correct pattern.

---

## PATCH 3/3: eal/linux: close alarm timerfd on cleanup

### ERRORS

None identified.

### WARNINGS

None identified.

### INFO

The patch correctly closes the timerfd used for alarms during EAL cleanup. The timerfd is obtained via `rte_intr_fd_get()` in `eal_alarm_set()` and should be closed before freeing the interrupt handle.

The call to `rte_intr_fd_close()` is placed correctly between callback unregister and handle free:
1. Unregister the callback (`rte_intr_callback_unregister_sync()`)
2. Close the fd (`rte_intr_fd_close()`) - **added by this patch**
3. Free the handle (`rte_intr_instance_free()`)

This matches the FreeBSD pattern and prevents the FD leak warning.

---

## SUMMARY

Patch 1/3: Clean.

Patch 2/3: Has a resource leak on the error path in both `pci_vfio_unmap_resource_primary()` and `pci_vfio_unmap_resource_secondary()` where `rte_intr_dev_fd_set(..., -1)` is not reached if `dev_vfio_release_device()` fails.

Patch 3/3: Clean.

Review-Result: ERROR