|WARNING| [v14, 23/23] 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/168146

_AI Code Review_

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

## Review of DPDK Patch Series (dpaa net/bus/crypto/mempool)

This review covers patches 01/23 through 23/23 of the DPDK DPAA patch series. Following the guidelines in AGENTS.md, focusing on correctness bugs first, then style/process issues where they are clearly wrong.

---

### **Patch 01/23: net/dpaa: fix device remove**

**Errors:**

1. **Use-after-free on close failure path (line ~2686):**
   ```c
   if (eth_dev && eth_dev->state != RTE_ETH_DEV_UNUSED) {
       int close_ret = dpaa_eth_dev_close(eth_dev);

       if (close_ret)
           DPAA_PMD_WARN("%s: close failed (%d), releasing port anyway",
               dpaa_dev->device.name, close_ret);
       ret = rte_eth_dev_release_port(eth_dev);
   }
   ```
   If `dpaa_eth_dev_close()` fails, the code still calls `rte_eth_dev_release_port(eth_dev)` which frees the `eth_dev` structure. The subsequent access to `eth_dev->device.name` in the warning message is use-after-free if `rte_eth_dev_release_port()` has already freed the device structure or invalidated the pointer.

   **Suggested fix:** Store `dpaa_dev->device.name` in a local buffer before the release call, or restructure so the warning is printed before release.

---

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

**Warnings:**

1. **NULL pointer assignment after rte_free (line ~627):**
   ```c
   rte_free(dpaa_intf->tx_conf_queues);
   dpaa_intf->tx_conf_queues = NULL;
   ```
   This is correct practice (setting freed pointer to NULL to avoid dangling references). No issue here.

2. **Error path cleanup on probe failure (lines ~2503-2511):**
   The patch correctly adds cleanup of `tx_conf_queues` on the error path (`free_tx` label). Trace the allocation:
   - Allocated unconditionally at `dpaa_dev_init()` line ~2362.
   - Freed on `free_tx` error label (new code).
   - Freed on close path (new code).
   - Freed on `clean_1` label for offline/ONIC ports (new code).

   All paths that allocate or have allocated the resource release it on error. Correctness verified.

---

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

**Errors:**

None. The patch correctly fixes the bitfield extraction logic: `dest_wq` bits [15:3] are channel, bits [2:0] are work queue. The old code inverted these.

**Warnings:**

1. **Macro `GENMASK` redefinition (line ~10):**
   The patch adds `#define GENMASK(h, l)` to `qman.h`, but this macro is commonly defined in Linux kernel headers and may conflict if the header is included alongside kernel headers. Suggest namespacing it as `DPAA_GENMASK` or use existing DPDK bitfield macros. This is a **Warning** (potential future conflict, not a current bug).

---

### **Patch 04/23: bus/dpaa: refine fman naming**

**Info:**

Renaming `ccsr_map` to `memac_map` and `bmi_map` to `rx_bmi_map` improves clarity. No correctness or style issues.

---

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

**Errors:**

None. The patch correctly reads the BPID range from the device tree and validates it has two items.

**Warnings:**

1. **Default BMAN version comment (line ~217):**
   ```c
   pr_warn("unknown bman version in portal node, default to rev2.1.0");
   bman_ip_rev = BMAN_REV21;
   ```
   The warning message says "rev2.1.0" but the code sets `BMAN_REV21`. Ensure the constant matches the documented version number. This is cosmetic but could confuse a reader. **Info** level.

---

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

**Errors:**

None. The patch correctly guards hardware initialization with `rte_eal_process_type() != RTE_PROC_PRIMARY` checks.

---

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

**Errors:**

None. Passing the full `struct qman_fq` to `qman_shutdown_fq()` allows accessing the portal pointer `fq->qp`, which is needed for channel-affine portals.

---

### **Patch 08/23: drivers: add DPAA cgrid cleanup support**

**Errors:**

1. **Potential integer overflow in start_fqid increment (dpaa_cgr_stale_fq_cleanup, line ~519):**
   ```c
   start_fqid = fqid + 1;
   ```
   `fqid` is `uint32_t`, so `fqid + 1` could wrap to 0 if `fqid == UINT32_MAX`. The loop condition is `while (qman_pending_fq_by_cgrid(..., start_fqid, &fqid) == 0)`, which expects `start_fqid` to advance. If it wraps, the scan restarts from FQID 0.

   This is unlikely in practice (FQIDs are 24-bit, max `QMAN_MAX_FQID` is 0x00FFFFFF), but the patch does not document or enforce this assumption. If the FQID space is ever expanded, this could loop indefinitely.

   **Suggested fix:** Cap `start_fqid` at `QMAN_MAX_FQID` or break if `fqid >= QMAN_MAX_FQID`.

2. **Error path resource leak in dpaa_dev_init (lines ~2500-2580):**
   - Rx CGRs are allocated and created in a loop (line ~2383-2413).
   - `nb_rx_cgr` tracks how many were successfully created.
   - On error (`goto free_tx`), the patch releases only `nb_rx_cgr` CGRs (line ~2573-2575) and the CGRID range.
   - Tx CGRs are allocated and created in a loop (line ~2455-2473).
   - `nb_tx_cgr` tracks how many were successfully created.
   - On error, the patch releases only `nb_tx_cgr` CGRs (line ~2560-2562) and the CGRID range.

   Trace the allocations:
   - `cgr_rx` is allocated via `rte_malloc` (line ~2378).
   - `cgr_tx` is allocated via `rte_malloc` (line ~2450).
   - Both are freed on `free_rx` label (lines ~2574, 2577).
   - CGRID ranges are released on `free_tx` and `free_rx` labels (lines ~2561, 2576).

   The patch correctly cleans up CGRs and CGRIDs on error. Verified.

---

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

**Errors:**

1. **Pool channel range check uses hardcoded constants (qman_shutdown_fq, lines ~2860-2866):**
   ```c
   const u16 pool_ch_start = dpaa_get_qm_channel_pool();
   const u16 pool_ch_end = pool_ch_start +
           dpaa_get_qm_channel_pool_num();
   if (channel >= pool_ch_start && channel < pool_ch_end) {
   ```
   This correctly replaces the hardcoded `qm_channel_pool1` with DTS-derived values. No issue.

2. **Portal SDQCR restore for both dedicated and pool channels (line ~2897):**
   ```c
   /* Restore portal SDQCR for both dedicated and pool channels */
   qm_dqrr_sdqcr_set(&p->p, p->sdqcr);
   ```
   The old code only restored SDQCR for pool channels (the `if (channel < qm_channel_pool1)` branch did not restore it). The new code unconditionally restores `p->sdqcr` after draining, which is correct for both channel types. No issue.

---

### **Patch 10/23: drivers: add BMI Tx statistics**

**Errors:**

None. The patch correctly adds Tx BMI statistics by reading from `fmbm_tfdc`, `fmbm_tfledc`, `fmbm_tfufdc`, `fmbm_tbdc`.

**Warnings:**

1. **`static_assert` on `DPAA_BMI_XSTATS_COUNT` (dpaa_ethdev.c, line ~159-162):**
   ```c
   static_assert(sizeof(struct dpaa_if_rx_bmi_stats) / sizeof(uint32_t)
           + sizeof(struct dpaa_if_tx_bmi_stats) / sizeof(uint32_t) - 1
           == DPAA_BMI_XSTATS_COUNT,
           "DPAA_BMI_XSTATS_COUNT out of sync with BMI stats structs");
   ```
   The assertion checks that the number of BMI xstats entries equals the sum of Rx BMI fields and Tx BMI fields minus 1. The `-1` accounts for `fmbm_tfrc` (Tx Frame Counter), which the comment in patch 10/23 says is part of Tx BMI but the structure starts with it (offset 0x204 in `struct tx_bmi_regs`, added in this patch).

   The math: `rx_bmi_stats` has 8 fields (from patch 10/23 comment), `tx_bmi_stats` has 4 fields (newly added). `8 + 4 - 1 = 11`. But the patch defines `DPAA_BMI_XSTATS_COUNT` as `RTE_DIM(dpaa_xstats_strings) - DPAA_MAC_XSTATS_COUNT` where `DPAA_MAC_XSTATS_COUNT = 13` (line ~158). If `dpaa_xstats_strings[]` has `13 + 11 = 24` entries, this assertion will pass. Need to verify the array size matches.

   Counting the entries in `dpaa_xstats_strings[]` (lines ~130-152):
   - 13 MAC stats (ending at "broadcast_packets")
   - 8 Rx BMI stats (starting "rx_frames_count")
   - 4 Tx BMI stats (starting "tx_bad_frames_count")
   Total: 13 + 8 + 4 = 25 entries.

   The assertion expects `8 + 4 - 1 = 11`, but the array has 8 + 4 = 12 BMI entries. The `-1` in the assertion is incorrect unless one field is intentionally excluded. Review the original `dpaa_if_rx_bmi_stats` structure before patch 10/23: it has `fmbm_rstc` (a control register, not a counter) as the first field, which was included in the structure but not in the xstats array.

   After patch 10/23, the structure is:
   ```c
   struct dpaa_if_rx_bmi_stats {
       uint32_t fmbm_rfrc;		/**< Rx Frame Counter*/
       uint32_t fmbm_rfbc;		/**< Rx Bad Frames Counter*/
       uint32_t fmbm_rlfc;		/**< Rx Large Frames Counter*/
       uint32_t fmbm_rffc;		/**< Rx Filter Frames Counter*/
       uint32_t fmbm_rfdc;		/**< Rx Frame Discard Counter*/
       uint32_t fmbm_rfldec;		/**< Rx Frames List DMA Error Counter*/
       uint32_t fmbm_rodc;		/**< Rx Out of Buffers Discard nntr*/
       uint32_t fmbm_rbdc;		/**< Rx Buffers Deallocate Counter*/
   };
   ```
   8 fields, all counters. The `-1` in the assertion is now incorrect. The assertion should be:
   ```c
   static_assert(sizeof(struct dpaa_if_rx_bmi_stats) / sizeof(uint32_t)
           + sizeof(struct dpaa_if_tx_bmi_stats) / sizeof(uint32_t)
           == DPAA_BMI_XSTATS_COUNT, ...);
   ```
   without the `- 1`.

   **This is an Error** because the assertion will fail if `sizeof(dpaa_if_rx_bmi_stats)/4 + sizeof(dpaa_if_tx_bmi_stats)/4 - 1 != 12`. With 8 + 4 - 1 = 11, and the actual count is 12, the assertion is wrong. The code will compile but the static assertion is incorrect logic.

   **Suggested fix:** Remove the `- 1` from the assertion.

---

### **Patch 11/23: net/dpaa: optimize FM deconfig**

**Errors:**

None. The patch consolidates FM deconfig to a single call and correctly moves VSP cleanup before FQ release.

---

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

**Errors:**

1. **Potential buffer overflow in strtol (dpaa_port_fmc_get_idx_from_name, line ~231):**
   ```c
   idx = (int)strtol(&found[idx_str_start], &endptr, 10);
   if (endptr == &found[idx_str_start] || (*endptr != '\0' && *endptr != '/')) {
       DPAA_PMD_ERR("Invalid index in fmc port name: %s", name);
       return -EINVAL;
   }
   ```
   `strtol` will parse digits until it hits a non-digit. The check `*endptr != '\0' && *endptr != '/'` allows either NUL-terminator or `/` after the digits. If the string is `"MAC/10/extra"`, `strtol` stops at the second `/`, `endptr` points to `/extra`, and `*endptr == '/'` so the check passes. The index is 10.

   If the string is `"MAC/999999999999999999"`, `strtol` overflows to `LONG_MAX` or `LONG_MIN` and sets `errno` to `ERANGE`. The code does not check `errno`, so an overflow is silently accepted as a valid index.

   **Suggested fix:** Check `errno == ERANGE` after `strtol` and return `-EINVAL` on overflow. Or validate the parsed `idx` is within a reasonable range (0..9 for current hardware).

---

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

**Errors:**

None. The patch correctly tracks allocated BPIDs in a per-BPID flag table and releases them in a destructor registered with `RTE_FINI_PRIO`. The destructor also frees `rte_dpaa_bpid_info` which is shared memory.

**Info:**

1. **Mempool cache flush threshold tuning (comment line ~91):**
   The commit message says "tune the per-lcore mempool cache flush threshold to match DPAA_MBUF_MAX_ACQ_REL". This tuning is not visible in the patch code. If it refers to a default cache size in the mempool code, that is outside this patch and does not require review here.

---

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

**Errors:**

None. The patch adds three devargs to control existing driver behavior (SG mode, data validation, ERR050757 workaround).

**Warnings:**

1. **Devarg parsing logic (dpaa_qdma_init, lines ~