|WARNING| [v2, 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/167186

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-08-05

# DPDK Patch Review

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

### Errors

None.

### Warnings

None.

### Info

**Documentation and release notes:**
The patch correctly updates all required documentation:
- Removes the devarg from `doc/guides/nics/intel_vf.rst`
- Removes the deprecation notice from `doc/guides/rel_notes/deprecation.rst`
- Adds a removal notice in `doc/guides/rel_notes/release_26_11.rst`

The release notes clearly explain that promiscuous and all-multicast restoration is now unconditional.

**Code changes:**
The patch cleanly removes the `auto_reconfig` devarg and its associated logic:
- Removes the struct field from `iavf_devargs`
- Removes the devarg parsing
- Removes the conditional wrapper around `iavf_post_reset_reconfig()`
- Removes the `else` branch that was clearing promiscuous states when auto-reconfig was disabled

**Simplification in `iavf_post_reset_reconfig`:**
The removal of the `status` variable and direct return of `ret` is correct - the function only had one error path.

---

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

### Errors

None.

### Warnings

**Missing release notes entry:**
This patch changes functional behavior (when promiscuous/all-multicast settings are restored) but does not update `doc/guides/rel_notes/release_26_11.rst`. The change is not purely internal - it affects the timing and frequency of PF requests. A brief note in the release notes would document this optimization.

**Error handling in `iavf_dev_configure`:**
The patch adds a call to `iavf_post_reset_reconfig()` at the end of `iavf_dev_configure()` and checks the return value, but the function can return without releasing resources acquired earlier in `iavf_dev_configure()`. If `iavf_post_reset_reconfig()` fails after a queue reconfiguration, the device may be left in an inconsistent state. Consider whether cleanup is needed on this error path.

### Info

**Implementation approach:**
The patch implements `get_restore_flags()` to exclude `RTE_ETH_RESTORE_PROMISC` and `RTE_ETH_RESTORE_ALLMULTI` from the ethdev restore logic. This correctly tells ethdev that the driver will handle these settings itself.

**Double restoration prevention:**
The patch prevents double restoration when a queue reconfiguration triggers a reset during `dev_configure` by checking `vf->in_reset_recovery`. The logic appears sound: if the configure is part of reset recovery, skip the restoration here because the reset handler will do it once at the end.

**Code comment quality:**
The comments in both `iavf_get_restore_flags()` and the new block in `iavf_dev_configure()` clearly explain why the restoration is being skipped in each case.

---

## Overall Series Assessment

The patch series is well-structured and the rationale for removing the `auto_reconfig` devarg is sound. The code changes are clean and the documentation updates are thorough.

**Summary of findings:**
- **1 Warning** in patch 2/2: missing release notes entry for functional behavior change
- **1 Warning** in patch 2/2: potential incomplete error handling in `iavf_dev_configure`

Both patches compile cleanly and maintain internal consistency. The removal of the devarg simplifies the code and eliminates the inconsistency where only some settings were conditionally restored.