|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. **Resource leak on error path in `rte_intr_callback_register()`**
   
   In `lib/eal/freebsd/eal_interrupts.c` at lines 138-149:
   ```c
   } else {
       src->intr_handle = intr_handle;
       TAILQ_INIT(&src->callbacks);
       TAILQ_INSERT_TAIL(&intr_sources, src, next);
   }
   ```
   
   If `TAILQ_INSERT_TAIL(&(src->callbacks), callback, next)` fails (or any subsequent operation), the newly allocated `src` is not freed, and `callback` is not freed either. The code removed the cleanup that was present in the original version where `rte_intr_instance_dup()` failure was handled.
   
   The same issue exists in `lib/eal/linux/eal_interrupts.c` at lines 534-539.

2. **Missing NULL check for `callback` allocation**
   
   In both FreeBSD and Linux versions, after `callback = rte_zmalloc("interrupt callback list", ...)`, there's a check `if (callback == NULL)` followed by error handling. However, when `src` is newly allocated, if `callback` allocation fails, the code goes to `fail:` label which doesn't free the newly allocated `src`.

**Warnings:**

1. **API signature change without version symbol**
   
   The patch changes `rte_intr_unregister_callback_fn` signature to take `const struct rte_intr_handle *` instead of `struct rte_intr_handle *`. While documented in release notes, this is an ABI break. The patch should use `RTE_VERSION_SYMBOL` and `RTE_DEFAULT_SYMBOL` to maintain ABI compatibility, or justify why this is acceptable.

---

## 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:**

1. **Resource leak in `memif_socket_init()` error path**
   
   At line 1118-1122, when `rte_hash_add_key_data()` fails:
   ```c
   if (socket->listener) {
       rte_intr_callback_unregister(socket->intr_handle,
           memif_listener_handler, socket);
       rte_intr_fd_close(socket->intr_handle);
       rte_intr_instance_free(socket->intr_handle);
   }
   rte_free(socket);
   ```
   
   The code only cleans up `socket->intr_handle` if `socket->listener` is true. However, the interrupt handle was allocated for both listener and non-listener sockets in `memif_socket_create()`. For non-listener sockets, `socket->intr_handle` is leaked.

---

## 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 primarily addresses FD leak warnings by properly resetting FD references before freeing interrupt handles. However, there are correctness bugs in patches 3 and 6 that must be fixed, and patch 3 introduces an ABI change that should be versioned.

Review-Result: ERROR