|WARNING| [v2,8/8] net/ice: fix DCF port leak on probe failure

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## PATCH 1/8: net/ice: fix DCF hardware init error path

### Errors

**Resource leak on error paths (Lines 856, 872, 882):**
```c
if ((hw->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_VLAN_V2) &&
    dcf_get_vlan_offload_caps_v2(hw))
	goto err_intr;
```
The patch adds `err_intr` cleanup but the new `err_intr` label itself has an error path leak.
At lines 859-865, the patch adds cleanup for interrupt and QoS config, then falls through to `err_rss:`.
However, if `dcf_get_vlan_offload_caps_v2()` succeeds (lines 854-855) but the function continues and later hits line 872 `goto err_alloc`, the VSI map is still not freed.
The `err_alloc:` label (line 870) does not have the VSI map free that was added at lines 871-872.
Similarly, if execution reaches line 882 `goto err_api`, the VSI map leak remains because `err_api:` is before the new VSI map cleanup.

The VSI map free must be added to ALL error labels that can be reached after the map is allocated, not just at the new `err_alloc` position.
Move the VSI map cleanup to occur in every error path, either by duplicating it at each label or by ensuring proper fall-through.

**Suggested fix:**
```c
err_intr:
	ice_dcf_disable_irq0(hw);
	rte_intr_disable(pci_dev->intr_handle);
	rte_intr_callback_unregister_sync(pci_dev->intr_handle,
				     ice_dcf_dev_interrupt_handler, hw);
	rte_free(hw->qos_bw_cfg);
	hw->qos_bw_cfg = NULL;
err_rss:
	rte_free(hw->rss_key);
	rte_free(hw->rss_lut);
	rte_free(hw->vf_vsi_map);
	hw->vf_vsi_map = NULL;
err_alloc:
	rte_free(hw->vf_res);
err_api:
	iavf_shutdown_adminq(&hw->avf);
```
The VSI map free should be moved to occur before `err_alloc` so all error paths after allocation reach it.

---

## PATCH 2/8: net/ice: fix DCF parent adapter init error path

### No Issues Found

The patch correctly adds the missing VSI free on all error paths and adds the `uninit_flow` label to ensure `ice_flow_uninit()` is called when `ice_flow_init()` fails.
Resource cleanup order is correct (flow uninit before hardware uninit).

---

## PATCH 3/8: net/ice: fix DCF queue config buffer leak

### No Issues Found

The patch correctly adds `rte_free(vc_config)` on both error paths where the function returns -EINVAL after allocating the message.

---

## PATCH 4/8: net/ice: fix DCF QoS buffer leak on commit

### Errors

**Use of `free()` instead of `rte_free()` (Lines 852-853):**
```c
	free(vf_bw);
	free(tc_bw);
```
The buffers `vf_bw` and `tc_bw` are allocated with `rte_zmalloc()` earlier in the function (not shown in the patch but referenced by the context).
DPDK code must use `rte_free()` for memory allocated with `rte_malloc()`/`rte_zmalloc()`.
Using standard `free()` on hugepage-backed memory is incorrect.

**Suggested fix:**
```c
	rte_free(vf_bw);
	rte_free(tc_bw);
```

---

## PATCH 5/8: net/ice: fix DCF QoS bandwidth replay config leak

### No Issues Found

The patch correctly:
- Allocates the new bandwidth config copy first
- Fills it with the updated data
- Swaps it into the shared array under the lock that the VF reset thread holds while replaying
- Frees the old copy after the swap
- Adds cleanup of all copies in `ice_dcf_uninit_hw()`

This properly handles the race condition with the VF reset thread.

---

## PATCH 6/8: net/ice: fix DCF TM config leak on close

### No Issues Found

The patch correctly restructures the cleanup logic so that `ice_dcf_tm_conf_uninit()` is called whenever QoS is supported, not only when the hierarchy was committed.
The fix properly handles uncommitted TM nodes and profiles that would otherwise leak.

---

## PATCH 7/8: net/ice: fix DCF representor info leak on close

### No Issues Found

The patch correctly:
- Extracts the teardown logic into `ice_dcf_dev_teardown()` so it can be called from both `ice_dcf_dev_close()` and `ice_dcf_dev_reset()`
- Frees the representor info only in `ice_dcf_dev_close()` since reset needs it to survive
- Properly separates concerns between close (which releases everything) and reset (which preserves representor info for re-initialization)

---

## PATCH 8/8: net/ice: fix DCF port leak on probe failure

### No Issues Found

The patch correctly:
- Checks the representor type before creating the DCF port (fail-fast)
- On representor creation failures, destroys all representors created so far in reverse order
- Removes the DCF port itself via `rte_eth_dev_pci_generic_remove()`
- Ensures no ports are leaked when probe fails partway through representor creation

---

## Summary

Only two patches have issues requiring fixes:

**Patch 1:** Incomplete error path cleanup - VSI map is freed in one error path but not all reachable paths after allocation.

**Patch 4:** Incorrect use of standard `free()` on `rte_malloc`'d memory.

Review-Result: ERROR