|WARNING| drivers: improve DPAA crypto fq resource handling

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/172199

_AI Code Review_

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

# DPDK Patch Review

## ERRORS

### 1. Error path resource leak (bus/dpaa/base/qbman/qman.c)

**Line 2991-2993:**
```c
if (!ret) {
	fq->state = qman_fq_state_oos;
	qman_destroy_fq(fq, 0);
}
```

**Issue:** When `qman_shutdown_fq()` fails (`ret != 0`), the function returns without calling `qman_destroy_fq()`. The FQ was created with `qman_create_fq()` earlier and remains allocated. The caller has no way to clean up since the FQ is in an unknown state - neither fully shut down nor properly tracked for retry. This leaks the `qman_fq` structure's internal resources.

**Fix:** Either always call `qman_destroy_fq(fq, 0)` regardless of `ret`, or document that the caller must call it on error. The former is safer:
```c
if (!ret)
	fq->state = qman_fq_state_oos;
qman_destroy_fq(fq, 0);
```

### 2. Missing error check (drivers/crypto/dpaa_sec/dpaa_sec.c)

**Line 3634:**
```c
ret = rte_dpaa_portal_init((void *)0);
if (ret) {
	DPAA_SEC_ERR("Failure in affining portal");
	return ret;
}
```

**Issue:** `dpaa_sec_uninit()` can fail during portal initialization but continues to call `qman_shutdown_fq()` anyway in the loop that follows (lines 3641-3649). When `DPAA_PER_LCORE_PORTAL` is NULL and portal init fails, `qman_shutdown_fq()` will call `get_affine_portal()` which may also fail or return NULL, leading to NULL pointer dereference in the QMan calls.

**Fix:** Return immediately on portal init failure since shutdown cannot proceed without an affine portal.

---

## WARNINGS

### 1. Pre-allocated FQID range not released on partial failure (drivers/crypto/dpaa_sec/dpaa_sec.c)

**Lines 3798-3807:**
```c
num = qman_alloc_fqid_range(fqids, internals->max_nb_queue_pairs, 0, 0);
if (num < (int)internals->max_nb_queue_pairs) {
	if (num < 0) {
		DPAA_SEC_ERR("%s: Failed(%d) to alloc %d TX fqids",
			cryptodev->data->name, num, internals->max_nb_queue_pairs);
	} else {
		DPAA_SEC_ERR("%s: Alloc %d fqids < %d",
			cryptodev->data->name, num, internals->max_nb_queue_pairs);
		qman_release_fqid_range(fqids[0], num);
	}
	goto init_error;
}
```

**Issue:** When `num == 0` (allocation returned zero FQIDs), the code jumps to `init_error` without releasing anything. The `else` branch only handles `num > 0 && num < max_nb_queue_pairs`. If `num == 0`, `fqids[]` is uninitialized and no cleanup occurs, but the allocator may have reserved internal state.

**Fix:** Handle `num == 0` explicitly, or fold it into the `num > 0` check:
```c
} else if (num > 0) {
	DPAA_SEC_ERR("%s: Alloc %d fqids < %d", ...);
	qman_release_fqid_range(fqids[0], num);
}
```

### 2. Inconsistent device name logging (drivers/crypto/dpaa_sec/dpaa_sec.c)

**Lines 3680, 3844, 3798, 3832:**

Mixed use of `dev->data->name` and `cryptodev->data->name` within the same function. Both refer to the same device but use different variable names.

**Fix:** Use consistent naming. Since the function parameter is `cryptodev`, prefer `cryptodev->data->name` throughout for consistency.

### 3. Error log precision (drivers/crypto/dpaa_sec/dpaa_sec.c)

**Line 3834:**
```c
DPAA_SEC_ERR("Failed(%d) to alloc %d fqids", num, RTE_DPAA_MAX_RX_QUEUE);
```

**Issue:** Missing device name in the error message, unlike the TX FQID allocation error at line 3798 which includes `cryptodev->data->name`. This makes it harder to identify which device failed in multi-device systems.

**Fix:** Add device name for consistency:
```c
DPAA_SEC_ERR("%s: Failed(%d) to alloc %d fqids",
	cryptodev->data->name, num, RTE_DPAA_MAX_RX_QUEUE);
```

---

## INFO

### 1. Variable initialization (drivers/crypto/dpaa_sec/dpaa_sec.c)

**Line 3731:**
```c
int ret, num = 0, map_fd = -1;
```

`num = 0` initialization is unnecessary - `num` is assigned from `qman_alloc_fqid_range()` return value before being read (line 3798). The initialization defeats the compiler's uninitialized variable warning, which would catch missing assignment on new code paths.

### 2. Hardcoded function argument (drivers/crypto/dpaa_sec/dpaa_sec.c)

**Lines 3798, 3830:**
```c
num = qman_alloc_fqid_range(fqids, internals->max_nb_queue_pairs, 0, 0);
num = qman_alloc_fqid_range(fqids, RTE_DPAA_MAX_RX_QUEUE, 0, 0);
```

The last two `0` arguments to `qman_alloc_fqid_range()` are hardcoded. If the function signature or expected arguments change, these magic numbers provide no context. Consider using named constants or at least a comment explaining what `0, 0` means (e.g., alignment and flags).

---

Review-Result: ERROR