|SUCCESS| [v2,7/7] net/ice: fix ACL profile segment leak
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Mon Nov 2 09:59:43 CET 2026
Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/172341
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-11-02
# DPDK Patch Review
## Patch 1/7: net/ice: fix DDP package double free on init failure
**Errors:**
None.
**Warnings:**
None.
**Info:**
None.
---
## Patch 2/7: net/ice: fix resource leaks on port close
**Errors:**
1. **Missing NULL check after ice_release_vsi()**
- Line: `ice_release_vsi(pf->main_vsi);` followed by `ice_deinit_hw(hw);`
- `ice_deinit_hw()` will call `ice_free_seg(hw)`, which accesses `hw->seg` and `hw->pkg_copy`. If the main VSI release has already freed structures that `ice_deinit_hw()` tries to access, this could cause issues. However, examining the change, `ice_deinit_hw()` is the correct cleanup function that should handle all hardware state teardown.
- The patch removes individual cleanup calls (`ice_sched_cleanup_all()`, `ice_free_hw_tbls()`, manual port_info free, `ice_shutdown_all_ctrlq()`) and replaces them with the comprehensive `ice_deinit_hw()`, which is correct.
- Actually, upon closer inspection, the code flow is: release main VSI, then call `ice_deinit_hw()` which does the full hardware cleanup. The main VSI was already allocated and is being released here, then the hardware state is torn down. This ordering appears correct - release the VSI context first, then clean up the hardware state. No issue.
Correction: No errors identified.
**Warnings:**
None.
**Info:**
None.
---
## Patch 3/7: net/ice: fix DDP filename leak
**Errors:**
None.
**Warnings:**
None.
**Info:**
None.
---
## Patch 4/7: net/ice: fix main VSI leak on init failure
**Errors:**
None.
**Warnings:**
None.
**Info:**
None.
---
## Patch 5/7: net/ice: fix MAC and VLAN filter leak on VSI release
**Errors:**
None.
**Warnings:**
None.
**Info:**
None.
---
## Patch 6/7: net/ice: fix FDIR resource leak on setup failure
**Errors:**
None.
**Warnings:**
None.
**Info:**
None.
---
## Patch 7/7: net/ice: fix ACL profile segment leak
**Errors:**
None.
**Warnings:**
None.
**Info:**
None.
---
## Summary
All patches address legitimate resource leak issues with appropriate fixes. The changes correctly:
- Replace direct `ice_free()` with `ice_free_seg()` to ensure pointer nullification (patch 1)
- Use comprehensive `ice_deinit_hw()` instead of partial manual cleanup (patch 2)
- Free allocated devargs strings on error paths and prevent re-assignment leaks (patch 3)
- Add missing VSI cleanup on error paths (patch 4)
- Clean up filter lists that may remain after failed hardware operations (patch 5)
- Add comprehensive cleanup for all FDIR setup error paths (patch 6)
- Eliminate leak by using stack allocation instead of heap for temporary structure (patch 7)
All patches target correct error paths and use appropriate cleanup functions. Code quality and style are consistent with DPDK conventions.
Review-Result: CLEAN
More information about the test-report
mailing list