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

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

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

**Errors:** None

**Warnings:** None

**Info:** None

---

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

**Errors:** None

**Warnings:** None

**Info:** None

---

## PATCH 3/9: interrupts: do not duplicate interrupt handles

**Errors:**

1. **Missing release notes for API change**
   ```c
   typedef void (*rte_intr_unregister_callback_fn)(const struct rte_intr_handle *intr_handle,
   						void *cb_arg);
   ```
   The patch adds a note in `release_26_11.rst` that the callback signature changed to take a `const` pointer, which is good. However, the note does not explain the behavioral change: the interrupt subsystem no longer clones the handle, and the callback now receives the exact pointer the caller registered with. The note should clarify that `ucb_fn` is always called with the same `intr_handle` the caller passed to `rte_intr_callback_register()`, so callers can safely use their own handle from `cb_arg` instead of the `intr_handle` parameter.

2. **Removal of internal symbol without deprecation notice**
   ```c
   -RTE_EXPORT_INTERNAL_SYMBOL(rte_intr_instance_dup)
   -struct rte_intr_handle *rte_intr_instance_dup(const struct rte_intr_handle *src)
   ```
   The patch removes `rte_intr_instance_dup()`, which is an `__rte_internal` function listed in `rte_interrupts.h` and used in `drivers/common/cnxk/roc_platform.h` via `plt_intr_instance_dup`. Removing it breaks out-of-tree code that may have called `plt_intr_instance_dup`. While this is internal API and not subject to ABI stability, the patch should either note this removal in the release notes or demonstrate that no in-tree code relies on it after the patch series is applied.

**Warnings:**

1. **Incomplete release notes entry**
   The release notes mention the callback signature change but do not explain the matching algorithm change (FD-based to pointer-based for registration/unregistration) or that the interrupt subsystem no longer owns a clone of the handle. A more complete note would help users understand the impact.

**Info:** None

---

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

**Errors:** None

**Warnings:** None

**Info:** None

---

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

**Errors:** None

**Warnings:** None

**Info:** None

---

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

**Errors:** None

**Warnings:** None

**Info:** None

---

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

**Errors:** None

**Warnings:** None

**Info:** None

---

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

**Errors:** None

**Warnings:** None

**Info:** None

---

## PATCH 9/9: raw/ifpga: reset FD in interrupt handle

**Errors:** None

**Warnings:** None

**Info:** None

---

## Summary

The series systematically fixes FD leak warnings introduced by commit d770ee9caed0. Most patches correctly reset FD references before freeing interrupt handles. Patch 3/9 makes a significant design change to the interrupt subsystem by eliminating handle duplication and changing from FD-based to pointer-based matching.

**Critical issues in Patch 3/9:**

1. The release notes should clarify the behavioral change: `ucb_fn` now receives the caller's original handle pointer, and the subsystem no longer owns a clone.
2. The removal of `rte_intr_instance_dup()` needs verification that no in-tree code depends on it after the series is applied (the `plt_intr_instance_dup` reference in `roc_platform.h` is removed in the patch, which is good, but this should be noted).

Review-Result: ERROR