|WARNING| [v4,4/4] net/iavf: fix watchdog enable racing disable

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## PATCH 1/4: net/iavf: fix crash in Tx drain on PF reset

### Errors

None.

### Warnings

None.

### Info

None.

---

## PATCH 2/4: net/iavf: fix event handler leak on init failure

### Errors

None.

### Warnings

None.

### Info

None.

---

## PATCH 3/4: net/iavf: fix watchdog clearing reset state during recovery

### Errors

**Error 1: Missing error propagation in `iavf_dev_event_post()` callers**

Line in `iavf_vchnl.c`, added line ~177:
```c
if (iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET, NULL, 0) != 0) {
    PMD_DRV_LOG(ERR, "VF \"%s\" failed to post reset event", dev->data->name);
    rte_atomic_fetch_sub_explicit(&vf->reset_events_pending, 1,
            rte_memory_order_release);
}
```

The error path in `iavf_dev_enter_reset()` backs out the `reset_events_pending` count but does not return an error to the caller. The function returns `true` (reset started) even when the event post fails. Callers in `iavf_read_msg_from_pf()` and `iavf_handle_pf_event_msg()` then conditionally enable the watchdog based on this return value, leaving the watchdog armed when the reset event was not actually posted.

The `reset_events_pending` count is now out of sync with the actual event queue. The watchdog expects the count to reach zero when all events are handled, but if the post failed the count will never decrement back to zero (since the event handler never runs). This leaves `vf_reset` stuck forever.

**Suggested fix:**
Return `false` from `iavf_dev_enter_reset()` when event posting fails, so callers do not arm the watchdog:

```c
if (iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET, NULL, 0) != 0) {
    PMD_DRV_LOG(ERR, "VF \"%s\" failed to post reset event", dev->data->name);
    rte_atomic_fetch_sub_explicit(&vf->reset_events_pending, 1,
            rte_memory_order_release);
    return false;
}
```

**Error 2: Race between `reset_events_pending` increment and compare-exchange**

Lines in `iavf_vchnl.c`, added lines ~163-166:
```c
rte_atomic_fetch_add_explicit(&vf->reset_events_pending, 1, rte_memory_order_relaxed);
if (!rte_atomic_compare_exchange_strong_explicit(&vf->vf_reset, &expected, true,
        rte_memory_order_release, rte_memory_order_acquire)) {
    rte_atomic_fetch_sub_explicit(&vf->reset_events_pending, 1,
            rte_memory_order_relaxed);
```

A race exists between incrementing `reset_events_pending` and the compare-exchange on `vf_reset`. If the watchdog runs between these two operations and observes `reset_events_pending != 0` with `vf_reset` still false, it will not clear the flag but the flag is not yet set. If the compare-exchange then fails because another thread won, this thread backs out the count. However, the watchdog has already made decisions based on the transient non-zero count.

Worse: if two threads enter simultaneously, both increment `reset_events_pending` to 2, both attempt the compare-exchange, one wins and one loses. The loser backs the count out to 1, and only one event is posted. The watchdog will wait for `reset_events_pending` to reach zero, but it never will because the second event was never posted.

**Suggested fix:**
Move the `reset_events_pending` increment to after the successful compare-exchange:

```c
if (!rte_atomic_compare_exchange_strong_explicit(&vf->vf_reset, &expected, true,
        rte_memory_order_release, rte_memory_order_acquire)) {
    return false;
}

rte_atomic_fetch_add_explicit(&vf->reset_events_pending, 1, rte_memory_order_relaxed);
```

Then in the event post failure path, back it out before returning false. This ensures the count only increments when a reset is actually started by this thread.

---

## PATCH 4/4: net/iavf: fix watchdog enable racing disable

### Errors

None.

### Warnings

None.

### Info

None.

---

Review-Result: ERROR