|WARNING| [v13, 25/25] doc: update release notes with NXP DPAA changes

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-08-19

# DPAA Patch Series Review

## Summary

This is a 25-patch series for NXP DPAA (Data Path Acceleration Architecture) drivers. I've reviewed all patches for correctness bugs, coding style, and compliance with DPDK guidelines.

---

## Patch 01/25: net/dpaa: fix device remove

**No issues found.**

The patch adds a NULL check before calling `dpaa_eth_dev_close()` and properly handles the close return value. The error is logged but the release continues, which is correct--a device should be removed from DPDK even if the close operation fails.

---

## Patch 02/25: net/dpaa: fix free port resources on close

**Error: Resource leak on error path**

**Location:** `drivers/net/dpaa/dpaa_ethdev.c:2503-2504`

```c
free_tx:
	rte_free(dpaa_intf->tx_conf_queues);
	dpaa_intf->tx_conf_queues = NULL;
	rte_free(dpaa_intf->tx_queues);
	dpaa_intf->tx_queues = NULL;
	dpaa_intf->nb_tx_queues = 0;

free_rx:
	rte_free(dpaa_intf->cgr_rx);
	rte_free(dpaa_intf->cgr_tx);
	rte_free(dpaa_intf->rx_queues);
	dpaa_intf->rx_queues = NULL;
	dpaa_intf->nb_rx_queues = 0;
```

The `free_tx` label frees `tx_conf_queues` and `tx_queues` but then falls through to `free_rx`, which frees `cgr_rx`, `cgr_tx`, and `rx_queues`. However, the patch adds two new `for` loops at lines 2511-2530 that allocate `cgr_rx` and `cgr_tx` and create CGRs. If CGR creation fails partway through, the error path jumps to `free_tx`, which does NOT free the already-created CGRs or the `cgr_rx`/`cgr_tx` arrays allocated so far. The CGR array allocation happens *before* the loop, so a failure in the middle of the loop leaves `cgr_rx`/`cgr_tx` allocated but not freed on the `free_tx` path.

**Suggested fix:**

Move the `rte_free(dpaa_intf->cgr_rx)` and `rte_free(dpaa_intf->cgr_tx)` calls from `free_rx` to immediately after the `free_tx` label, so that all error paths that jump to `free_tx` will also free the CGR arrays. Additionally, ensure that any CGRs successfully created before the error are deleted in the error path.

```c
free_tx:
	/* Delete any CGRs created before the error */
	for (loop = 0; loop < nb_tx_cgr; loop++)
		qman_delete_cgr(&dpaa_intf->cgr_tx[loop]);
	rte_free(dpaa_intf->cgr_tx);
	dpaa_intf->cgr_tx = NULL;

	rte_free(dpaa_intf->tx_conf_queues);
	dpaa_intf->tx_conf_queues = NULL;
	rte_free(dpaa_intf->tx_queues);
	dpaa_intf->tx_queues = NULL;
	dpaa_intf->nb_tx_queues = 0;

free_rx:
	for (loop = 0; loop < nb_rx_cgr; loop++)
		qman_delete_cgr(&dpaa_intf->cgr_rx[loop]);
	rte_free(dpaa_intf->cgr_rx);
	dpaa_intf->cgr_rx = NULL;
	rte_free(dpaa_intf->rx_queues);
	dpaa_intf->rx_queues = NULL;
	dpaa_intf->nb_rx_queues = 0;
```

Track `nb_rx_cgr` and `nb_tx_cgr` counters to know how many CGRs were successfully created, as shown in patch 08/25.

---

## Patch 03/25: bus/dpaa: fix FQD dest wq channel decoding

**No issues found.**

The fix correctly swaps the bit extraction to match the hardware descriptor layout (channel is bits 15:3, wq is bits 2:0). The new helpers `qm_fqd_get_chan()` and `qm_fqd_get_wq()` correctly extract the fields.

---

## Patch 04/25: bus/dpaa: refine fman naming

**No issues found.**

The renames (`ccsr_map` - `memac_map`, `bmi_map` - `rx_bmi_map`) improve clarity. No functional change.

---

## Patch 05/25: bus/dpaa: scan max BPID from DTS

**Warning: Missing input validation**

**Location:** `drivers/bus/dpaa/base/qbman/bman_driver.c:227-232`

```c
for_each_compatible_node(dt_node, NULL, "fsl,bpid-range") {
	range = of_get_property(dt_node, "fsl,bpid-range", &len);
	if (!range || len < (int)(2 * sizeof(rte_be32_t)))
		continue;
	start = rte_be_to_cpu_32(range[BPID_RANGE_START_INDEX]);
	count = rte_be_to_cpu_32(range[BPID_RANGE_COUNT_INDEX]);
	bman_pool_max = start + count;
```

The code sets `bman_pool_max = start + count` without checking for integer overflow. On a malicious or corrupt device tree, if `start + count > UINT16_MAX` (the range of `bman_pool_max`), the addition silently wraps, producing an incorrect pool limit.

**Suggested fix:**

```c
if (start > UINT16_MAX || count > UINT16_MAX || start + count > UINT16_MAX) {
	pr_warn("BPID range [%u, %u) exceeds hardware limit, capping to %u\n",
		start, start + count, UINT16_MAX);
	bman_pool_max = UINT16_MAX;
} else {
	bman_pool_max = start + count;
}
```

---

## Patch 06/25: drivers: add process-type guards for secondary process

**No issues found.**

The guards prevent secondary processes from initializing hardware, which is correct.

---

## Patch 07/25: drivers: shutdown DPAA FQ by fq descriptor

**No issues found.**

Passing the full `struct qman_fq *` instead of just the fqid allows the shutdown function to access the channel-affine portal correctly.

---

## Patch 08/25: drivers: add DPAA cgrid cleanup support

**No issues found.**

The `qman_pending_fq_by_cgrid()` function and the stale FQ cleanup logic are correctly implemented.

---

## Patch 09/25: bus/dpaa: improve FQ shutdown with channel validation

**No issues found.**

The pool-channel range is now derived from the device tree instead of being hardcoded. The SDQCR programming is correct for both dedicated and pool channels.

---

## Patch 10/25: drivers: add BMI Tx statistics

**No issues found.**

The Tx BMI statistics are correctly read from the hardware registers and exposed through xstats.

---

## Patch 11/25: net/dpaa: optimize FM deconfig

**No issues found.**

Consolidates FM deconfig to a single location. No functional issues.

---

## Patch 12/25: net/dpaa: optimize FMC MAC type parsing

**No issues found.**

The new `dpaa_port_fmc_get_idx_from_name()` function correctly extracts the MAC index from the port name, handling both MAC and OFFLINE port types.

---

## Patch 13/25: drivers: release DPAA bpid on driver destructor

**Error: Use-after-free of shared memory**

**Location:** `drivers/mempool/dpaa/dpaa_mempool.c:168-175`

```c
static void
dpaa_mbuf_free_pool(struct rte_mempool *mp)
{
	struct dpaa_bp_info *bp_info = DPAA_MEMPOOL_TO_POOL_INFO(mp);

	if (bp_info && bp_info->bp) {
		bman_free_pool(bp_info->bp);
		DPAA_MEMPOOL_INFO("BMAN pool freed for bpid =%d",
				  bp_info->bpid);
		if (rte_dpaa_bpid_info != NULL) {
			rte_dpaa_bpid_info[bp_info->bpid].mp = NULL;
			rte_dpaa_bpid_info[bp_info->bpid].bp = NULL;
		}
		s_dpaa_bpid_allocated_flag[bp_info->bpid].used = false;
		rte_free(bp_info);
		mp->pool_data = NULL;
	}
	/* rte_dpaa_bpid_info is shared (hugepage) memory referenced by every
	 * Rx queue via fq->bp_array, including in secondary processes. Freeing
	 * it here when the last local mempool is released would leave those
	 * references (and any secondary that reinstalls from fq->bp_array)
	 * pointing at freed memory. It is therefore released only from the
	 * driver destructor, once, at process teardown.
	 */
}
```

The comment says `rte_dpaa_bpid_info` is NOT freed here because it's shared memory referenced by Rx queues, but the code at lines 169-171 writes to `rte_dpaa_bpid_info[bp_info->bpid]` *after* `rte_free(bp_info)` at line 173. If `bp_info` is freed, `bp_info->bpid` is a use-after-free. The bpid value should be read before freeing `bp_info`.

**Suggested fix:**

```c
if (bp_info && bp_info->bp) {
	uint32_t bpid = bp_info->bpid; /* Read before free */
	bman_free_pool(bp_info->bp);
	DPAA_MEMPOOL_INFO("BMAN pool freed for bpid =%d", bpid);
	if (rte_dpaa_bpid_info != NULL) {
		rte_dpaa_bpid_info[bpid].mp = NULL;
		rte_dpaa_bpid_info[bpid].bp = NULL;
	}
	s_dpaa_bpid_allocated_flag[bpid].used = false;
	rte_free(bp_info);
	mp->pool_data = NULL;
}
```

---

## Patch 14/25: dma/dpaa: add devargs for SG and errata workaround

**No issues found.**

The new device arguments (`dpaa_dma_sg_disable`, `dpaa_dma_data_validation`, `dpaa_dma_pci_read_disable`) are correctly implemented and documented.

---

## Patch 15/25: net/dpaa: support Rx/Tx taildrop threshold devarg

**No issues found.**

The `drv_rx_taildrop` and `drv_tx_taildrop` device arguments are correctly parsed and applied. The `parse_int_devarg_handler()` function correctly uses `strtol()` and checks for parse errors.

---

## Patch 16/25: bus/dpaa: orp queue create and burst enqueue

**No issues found.**

The ORP burst enqueue API is correctly implemented, batching up to 8 entries before writing the verb bytes.

---

## Patch 17/25: net/dpaa: support fmcless rxq number as devargs

**No issues found.**

The `drv_fmcless_rxq` device argument is correctly parsed and validated.

---

## Patch 18/25: net/dpaa: support non fmX-macY type of shared Ethernet name

**No issues found.**

The `drv_sh_if_name` device argument allows the application to provide a custom kernel interface name for shared MAC interfaces. The `dpaa_get_devargs_str()` function correctly parses string-valued device arguments.

---

## Patch 19/25: bus/dpaa: optimize DPAA multi-entry buffer pool operations

**No issues found.**

The buffer pool acquire/release operations are correctly optimized. The `bm_buffer_set64_to_be()` and `bm_buffer_get64_from_be()` helpers correctly handle endianness.

---

## Patch 20/25: bus/dpaa: improve log macro usages

**No issues found.**

The log macro replacements (`DPAA_BUS_LOG(LEVEL, ...)` - `DPAA_BUS_INFO`, `DPAA_BUS_ERR`, etc.) improve readability. No functional change.

---

## Patch 21/25: net/dpaa: enhance VSP port support

**No issues found.**

The VSP port configuration correctly handles ONIC and offline-internal ports. The `dpaa_port_vsp_cleanup()` signature change removes the unused `fif` parameter.

---

## Patch 22/25: drivers: add offline (O/H) port device support

**Error: Missing NULL check on devargs**

**Location:** `drivers/net/dpaa/dpaa_oldev.c:403`

```c
static int
dpaa_ol_get_bh_port_name(struct rte_devargs *devargs, char *val, size_t size)
{
	struct rte_kvargs *kvlist;
	const char *str;
	int ret;

	if (!devargs)
		return 0;

	kvlist = rte_kvargs_parse(devargs->args, NULL);
	if (!kvlist)
		return 0;
```

The function correctly checks `if (!devargs)` at line 408, but the caller at `dpaa_ol_tx_queue_setup()` (line 472) does not check the device structure before passing `dev->device->devargs`. If `dev->device` is NULL, this is a NULL pointer dereference.

**Suggested fix:**

In `dpaa_ol_tx_queue_setup()`, check that `dev->device` is not NULL before calling `dpaa_ol_get_bh_port_name()`:

```c
if (!dev->device || !dev->device->devargs) {
	DPAA_PMD_ERR("Device or devargs not available");
	return -EINVAL;
}

ret = dpaa_ol_get_bh_port_name(dev->device->devargs,
			       (char *)fq_info.bh_port_name,
			       MAX_BH_PORT_NAME_LEN);
```

---

**Warning: Global static file descriptor not reset on close**

**Location:** `drivers/net/dpaa/dpaa_oldev.c:370`

```c
static int dpaa_ol_dev_close(struct rte_eth_dev *dev)
{
	/* ... */
	if (fd >= 0) {
		close(fd);
		fd = -1;
	}
	return 0;
}
```

The `fd` variable is a static global at file scope (line 50). If `dpaa_ol_dev_close()` is called by a secondary process, it will close the fd and set it to -1. When the primary process later tries to use the fd (e.g., in `ask_set_fq_info()` or `rte_pmd_dpaa_ol_set_classif_info()`), the global `fd` will be -1, and the functions will fail. This is a cross-process race condition.

**Suggested fix:**

Only close the fd in the primary process:

```c
if (rte_eal_process_type() == RTE_PROC_PRIMARY && fd >= 0) {
	close(fd);
	fd = -1;
}
```

Or better, make `fd` per-device instead of a global static.

---

##