|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