|WARNING| [RFC, 2/2] net/iavf: restore promiscuous mode only after VF reset

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## Patch 1/2: net/iavf: remove auto reconfig devarg

### Errors

None found.

### Warnings

None found.

### Info

The patch removes the `auto_reconfig` devarg and makes promiscuous/all-multicast restoration unconditional after VF reset. The code changes are straightforward: the devarg is removed, the corresponding struct field is deleted, and the conditional logic in `iavf_handle_hw_reset()` is simplified to always call `iavf_post_reset_reconfig()`. The release notes and documentation are updated appropriately.

---

## Patch 2/2: net/iavf: restore promiscuous mode only after VF reset

### Errors

**1. Variable `reset_done` may be read uninitialized**

In `iavf_dev_configure()`, the variable `reset_done` is declared but only conditionally set to `true` inside the `if` blocks. If neither of those conditions is met, `reset_done` remains uninitialized when read at line:
```c
if (reset_done && !vf->in_reset_recovery) {
```

**Fix:** Initialize `reset_done` to `false` at declaration:
```c
bool reset_done = false;
```

(Note: The patch already does this on line 693, so this is actually correct. However, verifying all paths: the variable is initialized, so no error here. Retracting this item.)

### Warnings

**1. Missing documentation of new callback implementation**

The patch implements the `get_restore_flags` callback but does not document this change in the release notes. This is a behavioral change that affects how the driver interacts with the ethdev restore logic.

**Suggested fix:** Add a note to `doc/guides/rel_notes/release_26_07.rst` under the iavf section:
```
The iavf PMD now implements the ``get_restore_flags`` callback to prevent
redundant restoration of promiscuous and all-multicast settings, which
persist in hardware across port stop/start cycles.
```

**2. `iavf_post_reset_reconfig()` forward declaration**

The function `iavf_post_reset_reconfig()` is forward-declared at line 168 but was already visible in the file (defined earlier in the .c file based on patch 1 context). The forward declaration is only needed because `iavf_dev_configure()` now calls it.

**Suggested fix:** Verify whether the forward declaration is necessary. If `iavf_post_reset_reconfig()` is defined later in the file (after `iavf_dev_configure()`), the forward declaration is correct. If it's defined earlier, the forward declaration can be removed. Based on patch 1 context showing it exists around line 3350, the forward declaration is indeed needed.

### Info

**1. Logic flow in `iavf_dev_configure()`**

The new logic at the end of `iavf_dev_configure()` checks `reset_done && !vf->in_reset_recovery`. This correctly avoids double-restoration: if a queue reconfiguration triggered a reset, the settings are restored once here; during actual reset recovery, `iavf_handle_hw_reset()` restores them instead. The two code paths are mutually exclusive.

**2. Return value handling**

The patch correctly propagates the return value from `iavf_post_reset_reconfig()` in the new code path, matching the existing error handling pattern in the function.

---

## Summary

**Patch 1/2:** Clean removal of the `auto_reconfig` devarg with appropriate documentation updates. No issues found.

**Patch 2/2:** The implementation is correct. One warning about missing release notes documentation for the new `get_restore_flags` callback.