|WARNING| net/mlx5: fix HWS sample action table leak on quit
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Sun Aug 16 10:33:08 CEST 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/167831
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-08-16
# DPDK Patch Review
## Patch Summary
This patch fixes a resource leak in the mlx5 PMD's hardware steering (HWS) sample/mirror action cleanup path. The issue occurs when destroying mirror actions: releasing clone destinations before the composite dest_array action can leave firmware flow table objects leaked if releasing a JUMP clone drops the last table reference.
---
## Correctness Issues
### Errors
None found. The patch correctly addresses a destruction-order bug that could leak firmware resources.
---
## Style and Process Issues
### Warnings
None found.
### Info
**Comment style could be more concise:**
The added comment is 6 lines explaining the destruction order requirement. While the explanation is accurate and helpful, DPDK style prefers shorter comments when possible. Consider condensing to 2-3 lines:
```c
/*
* Destroy dest_array before clones so clone release does not drop
* the last table reference while dest_array still points at it.
*/
```
However, this is a minor style preference. The current comment is acceptable and clearly explains a subtle ordering constraint.
---
## Verification Notes
**Positive findings:**
1. **Correct fix for resource leak:** The reordering ensures the composite dest_array action is destroyed before any referenced destinations are freed, preventing dangling references to flow tables.
2. **Proper loop unchanged:** The `for` loop freeing clones remains correct; moving it after the dest_array destruction does not affect its logic.
3. **Cc: stable at dpdk.org present:** Appropriate for a resource leak fix targeting backport.
4. **Comment explains rationale:** The added comment documents why this order matters, which is valuable for future maintainers.
**No issues found with:**
- Variable naming (existing code style maintained)
- Memory allocation/free patterns (`mlx5_free(mirror)` still last)
- Error handling (none needed for this cleanup path)
- NULL checks (existing code already handles `mirror->mirror_action` NULL check)
---
## Conclusion
**No blocking issues.** The patch correctly fixes a firmware resource leak by reordering destruction of a composite action and its referenced destinations. The added comment is slightly verbose but acceptable. The fix is correct and ready for merge.
More information about the test-report
mailing list