|SUCCESS| [v2, 3/3] net/iavf: fix watchdog clearing reset state during recovery

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/172621

_AI Code Review_

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

# DPDK Patch Review

## Patch 1/3: net/iavf: fix crash in Tx drain on PF reset

### Errors

None identified.

### Warnings

None identified.

### Info

None.

---

## Patch 2/3: net/iavf: fix event handler leak on init failure

### Errors

None identified.

### Warnings

None identified.

### Info

None.

---

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

### Errors

**Error 1: Variable overwritten before read (dead store)**
```c
iavf_dev_event_post(struct rte_eth_dev *dev,
		enum rte_eth_event_type event,
		void *param, size_t param_alloc_size)
{
	struct iavf_dev_event_handler *handler = &event_handler;
	char notify_byte = 0;
	struct iavf_event_element *elem = rte_malloc(NULL, sizeof(*elem) + param_alloc_size, 0);
	if (!elem)
		return -ENOMEM;

	elem->dev = dev;
	elem->event = event;
	elem->param = NULL;
	if (param) {
		elem->param = (void *)(elem + 1);
		rte_memcpy(elem->param, param, param_alloc_size);
	}

	pthread_mutex_lock(&handler->mutex);
	TAILQ_INSERT_TAIL(&handler->pending, elem, next);
	pthread_mutex_unlock(&handler->mutex);

	ssize_t nw = write(handler->fd[1], &notify_byte, 1);
	RTE_SET_USED(nw);

	return 0;
}
```
In the new `iavf_dev_reset_event_post()` wrapper at line +128:
```c
void
iavf_dev_reset_event_post(struct rte_eth_dev *dev)
{
	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);

	if (iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET, NULL, 0) != 0)
		rte_atomic_fetch_sub_explicit(&vf->reset_events_pending, 1,
				rte_memory_order_release);
}
```
This function increments `reset_events_pending` **before** calling `iavf_dev_reset_event_post()`, which may fail to post the event (if `rte_malloc()` returns NULL). When the post fails, the wrapper correctly decrements the counter. However, at three call sites (iavf_dev_watchdog line +367, iavf_read_msg_from_pf line +358, iavf_handle_pf_event_msg line +601), the code does:
```c
rte_atomic_fetch_add_explicit(&adapter->vf.reset_events_pending, 1,
		rte_memory_order_relaxed);
/* publish the count before vf_reset is visible */
rte_atomic_thread_fence(rte_memory_order_release);
adapter->vf.vf_reset = true;
iavf_set_no_poll(adapter, false);
...
iavf_dev_reset_event_post(dev);
```
If `iavf_dev_reset_event_post()` fails (malloc failure in `iavf_dev_event_post()`), it decrements the counter, but `vf_reset` remains set to `true`. The watchdog at line +343 will not clear `vf_reset` until `reset_events_pending` reaches zero:
```c
if (!vfr_inprogress &&
    rte_atomic_load_explicit(&adapter->vf.reset_events_pending,
			rte_memory_order_acquire) == 0) {
	PMD_DRV_LOG(INFO, "VF \"%s\" reset has completed", dev->data->name);
	adapter->vf.vf_reset = false;
	...
}
```
So the counter correctly drops back to zero, the watchdog clears `vf_reset`, and the device can resume. This is not a resource leak or logic error -- the malloc failure path is handled. No error here after all; removing this item.

Actually, re-reading: the increment happens, the post is called, and if the post's internal malloc fails, the wrapper decrements. The counter returns to its prior value, and `vf_reset` is already set. On the next watchdog tick, if the VFR is complete and the counter is zero, `vf_reset` is cleared. The state is correct. There is no leak or stuck state. This is fine.

### Warnings

None identified.

### Info

None.

---

## Summary

All three patches address correctness bugs (crash on PF reset, event handler leak on init failure, and watchdog race with reset events). The fixes are appropriate and the code changes are correct. No style, API, or documentation violations were found.

Review-Result: CLEAN