|SUCCESS| net/mlx5: fix HWS meter ASO SQ leak on multi-port init

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Sun Oct 18 11:19:17 CEST 2026


Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/171607

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-18

# DPDK Patch Review

## Patch Summary
The patch fixes a resource leak in the mlx5 PMD where Hardware Steering (HWS) meter ASO send queues are recreated without destroying the previous instance when multiple ports on the same device initialize meters.

---

## ERRORS

None identified.

---

## WARNINGS

### Missing Error Check on mlx5_aso_sq_create()

**Location:** `drivers/net/mlx5/mlx5_flow_aso.c:316-322`

```c
if (pool_mng != NULL && pool_mng->sq.sq_obj.sq == NULL) {
	if (mlx5_aso_sq_create(cdev, &pool_mng->sq,
			       sh->tx_uar.obj,
			       MLX5_ASO_QUEUE_LOG_DESC))
		return -1;
	pool_mng->sqn = pool_mng->sq.sqn;
}
```

The return value of `mlx5_aso_sq_create()` is checked via an `if` statement but the body only returns `-1` on failure. If `mlx5_aso_sq_create()` succeeds (returns 0), execution falls through to assign `pool_mng->sqn` and continue. However, if it fails (returns non-zero), the function returns `-1` immediately, which is correct.

Upon re-examination, this pattern is actually correct: the `if` checks the return value, and only on failure (non-zero) does it execute the `return -1;` statement. This is acceptable error handling for this context.

**Correction:** This is actually correct - no issue here.

---

## INFO

### Comment Accuracy

**Location:** `drivers/net/mlx5/mlx5_flow_meter.c:63-66`

The comment states that `pools_mng->sq` "is released with the shared context." This accurately describes the cleanup strategy where the shared ASO queue is freed in `mlx5_aso_flow_mtrs_mng_close()` rather than per-port in `mlx5_flow_meter_uninit()`. The comment correctly explains why the `mlx5_aso_queue_uninit()` call was removed.

### Null Pointer Check Style

**Location:** `drivers/net/mlx5/mlx5_flow_aso.c:323`

```c
if (pool_mng != NULL && pool_mng->sq.sq_obj.sq == NULL) {
```

The explicit `!= NULL` comparison follows DPDK coding standards for pointer checks. This is correct style.

---

Review-Result: CLEAN


More information about the test-report mailing list