|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:39:16 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
## Summary
This patch fixes a resource leak in the mlx5 PMD's HWS mirror action cleanup path. Overall the fix is correct, but there are minor process issues.
---
## Findings
### Errors
None.
### Warnings
**1. Missing release notes update**
This patch fixes a user-visible resource leak (firmware flow table objects leaked on port close). API changes and driver bug fixes of this severity require a release notes entry in `doc/guides/rel_notes/release_26_03.rst` (or the current release notes file).
Add a bullet under the "Fixed Issues" section for the mlx5 driver describing the fix.
---
### Info
**1. Comment could be more concise**
The block comment is accurate and helpful, but could be condensed slightly while retaining clarity:
```c
/*
* Destroy the mirror dest_array action before releasing clone
* destinations. Releasing a clone can drop the last group table
* reference; the dest_array must not reference that table when
* it is destroyed, or the firmware flow table will leak.
*/
```
This is a style suggestion, not a requirement. The existing comment is acceptable.
---
## Correctness Analysis
**Resource leak fix**: The patch correctly addresses the destroy-order bug. The original code destroyed clones first, which could free a group table while `mirror->mirror_action` still held a reference to it, leaking the firmware flow table object. Moving `mlx5dr_action_destroy(mirror->mirror_action)` before the clone loop ensures the composite dest_array releases its references before the underlying destinations are freed. This is correct.
**Error paths**: The function `mlx5_hw_mirror_destroy()` appears to be a cleanup-only path (called during port close or mirror destruction). There are no early returns after `mlx5_indirect_list_remove_entry()`, so no new error-path leaks are introduced.
**NULL safety**: `mlx5dr_action_destroy()` is called under an `if (mirror->mirror_action)` guard, which is correct -- it handles the case where mirror action creation may have failed.
---
## Conclusion
The patch is correct and fixes a real resource leak. The only required change is adding a release notes entry. The comment verbosity is a matter of style preference.
More information about the test-report
mailing list