|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