|WARNING| eal: do not dispatch newly registered callbacks

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-11-03

# DPDK Patch Review

## ERRORS

**Missing error handling in loop termination logic**

The loop termination condition `if (cb == last)` executes after calling the callback and re-acquiring the lock. If the callback unregistered itself (which is explicitly mentioned as a use case in the commit message), `cb` becomes invalid after the unlock, and comparing it to `last` is use-after-free.

```c
/* Current code - WRONG */
for (cb = TAILQ_FIRST(&src->callbacks); cb != NULL; cb = next) {
    next = TAILQ_NEXT(cb, next);
    active_cb = *cb;
    rte_spinlock_unlock(&intr_lock);
    /* cb may be freed here if callback unregisters itself */
    active_cb.cb_fn(active_cb.cb_arg, active_events);
    rte_spinlock_lock(&intr_lock);
    
    if (cb == last)  /* BUG: cb may have been freed */
        break;
}
```

**Suggested fix**: Check the termination condition before unlocking:

```c
for (cb = TAILQ_FIRST(&src->callbacks); cb != NULL; cb = next) {
    next = TAILQ_NEXT(cb, next);
    bool is_last = (cb == last);
    
    active_cb = *cb;
    rte_spinlock_unlock(&intr_lock);
    active_cb.cb_fn(active_cb.cb_arg, active_events);
    rte_spinlock_lock(&intr_lock);
    
    if (is_last)
        break;
}
```

## WARNINGS

**Missing release notes update**

This patch fixes a significant correctness bug (infinite blocking in net/tap driver) that could affect users. The fix changes interrupt callback dispatch behavior, which is important enough to document in release notes.

Add an entry to `doc/guides/rel_notes/release_XX_XX.rst` under "Fixed Issues":

```rst
* **Fixed interrupt callback re-registration during dispatch.**

  Fixed an issue where registering a new interrupt callback from within
  a callback (e.g., net/tap LSC on netns change) could cause the new
  callback to be invoked with a stale event, leading to blocking I/O.
```

**Inconsistent variable declaration style**

The `last` pointer is declared inside the `if (call)` block, while `next` is declared at loop initialization. For consistency and to make the loop logic clearer, declare both at the start of the block.

```c
/* Current - mixed declaration locations */
if (call) {
    struct rte_intr_callback *last;
    last = TAILQ_LAST(&src->callbacks, rte_intr_cb_list);
    for (cb = TAILQ_FIRST(&src->callbacks); cb != NULL; cb = next) {
        next = TAILQ_NEXT(cb, next);

/* Suggested - declare together */
if (call) {
    struct rte_intr_callback *last, *next;
    last = TAILQ_LAST(&src->callbacks, rte_intr_cb_list);
    for (cb = TAILQ_FIRST(&src->callbacks); cb != NULL; cb = next) {
```

---

Review-Result: ERROR