|SUCCESS| [v10-S2,11/11] net/dpaa2: rename Rx queue flags

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## Patch 01/11: net/dpaa2: fix integer overflow in CCSR region mapping

**Errors:**

None.

**Warnings:**

None.

**Info:**

The patch correctly fixes the Coverity integer overflow issue by:
- Checking `sysconf(_SC_PAGESIZE)` return value before use
- Computing `page_mask` from the verified positive value
- Using `RTE_ALIGN_CEIL(offset + len, page_size)` for map length so the full requested region is covered
- Early return on error before attempting `mmap`

The fix is sound. The original code's `len & PAGE_MASK` would zero out `len` on large-page kernels; the new computation ensures the mapping always spans the full region.

---

## Patch 02/11: net/dpaa2: set Tx confirmation on device init

**Errors:**

None.

**Warnings:**

None.

**Info:**

Refactor moves the Tx confirmation mode setting from per-queue setup to device init. The loop over `priv->num_channels` matches the original loop over `ceetm_ch_idx`, and the `dpni_set_tx_confirmation_mode()` calls are unchanged. Logic is preserved; only the call site moves.

---

## Patch 03/11: net/dpaa2: support larger burst size

**Errors:**

None.

**Warnings:**

None.

**Info:**

The patch widens `max_burst_size` in the API struct from `uint16_t` to `uint32_t` and splits it into two 16-bit MC command fields using the new `DPNI_BURST_LO/HI` macros. The `dpaa2_get_burst_max()` helper returns the SoC-specific limit and is used in the range checks. The command struct adds the `_hi` fields and the `dpni_set_tx_shaping()` function populates them. Implementation is correct.

---

## Patch 04/11: net/dpaa2: support MPLS and PPPoE flow distribution

**Errors:**

1. **NH_FLD_MPLS_MPLSL_1 extracts 4 bytes but original code passed 3**:
   The commit message states the fix: MC extracts the full 4-byte MPLS LSE (label + TC + S + TTL) when `NH_FLD_MPLS_MPLSL_1` is used with `DPKG_FULL_FIELD`, so passing `sizeof(label_tc_s) = 3` caused the key profile to record 3 bytes while MC placed 4, corrupting the layout. The patch now passes `sizeof(*mask) = 4` (the full `struct rte_flow_item_mpls`). However, the code in `dpaa2_configure_flow_mpls()` at the final `dpaa2_flow_add_hdr_extract_rule()` calls passes:

   ```c
   ret = dpaa2_flow_add_hdr_extract_rule(flow, NET_PROT_MPLS,
           NH_FLD_MPLS_MPLSL_1, spec,
           mask, sizeof(*mask),
           ...);
   ```

   Here `spec` and `mask` are `const struct rte_flow_item_mpls *`, and `sizeof(*mask)` is `sizeof(struct rte_flow_item_mpls)` which is 4 bytes (3-byte `label_tc_s` array plus 1-byte `ttl`). This is correct and matches the MC extraction size. The fix is as described.

**Warnings:**

None.

**Info:**

- The MPLS and PPPoE feature support is added: patterns with no spec use the FAF identify path, patterns with spec add a header extract rule for the MPLS label or PPPoE session ID.
- The default MPLS mask uses `{ 0xff, 0xff, 0xff }` for `label_tc_s`, leaving `ttl` unmasked (0x00), so only the label/TC/S are matched.
- PPPoE uses `session_id` as a `rte_be16_t`.
- Both features are documented in the feature matrix.

---

## Patch 05/11: net/dpaa2: support flow drop action

**Errors:**

None.

**Warnings:**

None.

**Info:**

The drop action is added to the supported list, verified in `dpaa2_flow_verify_action()`, configured in `dpaa2_configure_flow_fs_action()` with `DPNI_FS_OPT_DISCARD`, and handled in the generic flow-set and destroy paths. Implementation is straightforward and correct.

---

## Patch 06/11: net/dpaa2: set default flow miss action per device

**Errors:**

None.

**Warnings:**

None.

**Info:**

- The global `dpaa2_flow_miss_flow_id` variable is removed. The miss flow ID is now a per-device field `priv->default_flow`, initialized to `RTE_MAX(RTE_MIN(priv->fs_entries, priv->dist_queues), 1) - 1` at probe time (lowest-priority flow ID). This ensures each device has independent miss-flow configuration.
- The `DPAA2_FLOW_CONTROL_MISS_FLOW` environment variable is removed. Users who relied on it must now use the rte_flow API to configure miss-flow behavior. This is documented in the release notes.
- The refactor is a behavior change (removal of a tunable) but is handled correctly with documentation and a clear migration path.

---

## Patch 07/11: net/dpaa2: identify Rx mbuf hash information by FLC

**Errors:**

1. **Rx queue index validation missing upper bound check in `dpaa2_flow_verify_action()`**:
   The validation added in `dpaa2_flow_verify_action()`:
   ```c
   if (dest_queue->index >= MAX_RX_QUEUES ||
       !priv->rx_vq[dest_queue->index]) {
       DPAA2_PMD_ERR("Invalid FSQ index(%d)",
           dest_queue->index);
       return -EINVAL;
   }
   ```
   is correct and prevents out-of-bounds access to `priv->rx_vq[]`. The same check appears in `dpaa2_configure_flow_fs_action()` as well. Both are necessary and correct.

**Warnings:**

None.

**Info:**

- The patch configures the FS action with FLC enabled (`DPNI_FS_OPT_SET_FLC | DPNI_FS_OPT_SET_STASH_CONTROL`) and sets the stashing fields based on the queue's `data_stashing_off` flag. The FLC low word is packed with a marker bit, TC, and flow ID so that FS-delivered packets can be distinguished from RSS-delivered packets.
- The Rx parse path checks the FS_MARK bit in FLC low word: if set, the mbuf is marked `RTE_MBUF_F_RX_FDIR | RTE_MBUF_F_RX_FDIR_ID` and the TC/flow are stored in `m->hash.fdir.hi`; otherwise it is an RSS frame and the hash is in `fd->simple.flc_hi`.
- The DPAA2_PMD_DP_DEBUG log lines are added for datapath debugging.
- The TC and flow ID validation in `dpaa2_configure_flow_fs_action()` ensures the destination queue is in the correct TC before programming the FS action.

---

## Patch 08/11: net/dpaa2: add minimum key size support

**Errors:**

None.

**Warnings:**

None.

**Info:**

- The default (minimum) QoS key size is defined as `DPNI_DEFAULT_KEY_SIZE` (24 bytes). For DPNI version 8.7+, `dpaa2_flow_entry_size()` returns the minimum size when the key fits; for older versions, it returns the maximum (56 bytes) for compatibility.
- The helper function is called from QoS and FS table setup, and from the flow-set path to determine the rule key size.
- The refactor is correct and does not change behavior for DPNI < 8.7.

---

## Patch 09/11: net/dpaa2: parse tunnel and fragmented packet types

**Errors:**

None.

**Warnings:**

None.

**Info:**

- New packet type constants are defined for IPv4/IPv6 fragments, ESP, and GTP-U/GTP-C. The fast parse path (`dpaa2_dev_rx_parse_new()`) switches on the FRC value and sets `m->packet_type` accordingly.
- GTP-U and GTP-C packets are marked with `RTE_PTYPE_L4_UDP` in addition to the tunnel type, reflecting the UDP transport layer of the outer header.
- The new types are advertised in `dpaa2_supported_ptypes_get()`.
- Implementation is correct and the additional packet type identification improves application visibility.

---

## Patch 10/11: net/dpaa2: remove unused soft parser driver

**Errors:**

None.

**Warnings:**

None.

**Info:**

- The soft parser byte code must be installed by boot firmware before Linux starts; the WRIOP parser cannot be reprogrammed at run time once MC is up. The loader code in `dpaa2_sparser.c` and its call site in `dpaa2_ethdev.c` were therefore dead code and are removed.
- The VXLAN and eCPRI rte_flow pattern items were only accepted when `dpaa2_sp_loaded` was believed to be positive. With the SP gone, `dpaa2_dev_verify_patterns()` rejects them before the configure switch is reached, so the handlers, switch cases, and mask descriptors are removed.
- Both items are dropped from the dpaa2 feature matrix so documented capabilities match the code.
- The removal is a feature deprecation, documented in the release notes with a clear explanation of why the feature never worked and how users should proceed (load the SP via boot firmware if needed).
- The `dpaa2_sp_loaded` global, `dpaa2_soft_parser_loaded()`, and `WRIOP_SS_INITIALIZER()` are removed.
- The code removal is clean and does not leave stray references.

---

## Patch 11/11: net/dpaa2: rename Rx queue flags

**Errors:**

None.

**Warnings:**

None.

**Info:**

Mechanical rename of Rx-side flags to a consistent `DPAA2_RX_*_FLAG` naming:
- `DPAAX_RX_ERROR_QUEUE_FLAG` - `DPAA2_RX_ERROR_QUEUE_FLAG`
- `DPAA2_DATA_STASHING_OFF` - `DPAA2_RX_DATA_STASHING_OFF_FLAG`
- A new flag `DPAA2_RX_PRINT_PSR_RESULT_FLAG` is defined but not used in this patch (likely for future use).

All uses are updated consistently. No functional change.

---

## Summary

The series includes one correctness fix (patch 01, integer overflow / wrong mmap size), multiple feature additions (MPLS/PPPoE flow items, drop action, larger burst sizes, packet type parsing, FLC-based hash identification, minimum key size), one behavior change (per-device miss-flow default, removing an environment variable), and refactors (Tx conf setup, flag rename, dead SP code removal).

All patches are well-structured and the code changes are correct. No resource leaks, use-after-free, race conditions, or style violations were identified.

Review-Result: CLEAN