|SUCCESS| [v14, 15/15] net/enetc4: add WRR Tx scheduler devarg for VF rings

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review: net/enetc Series (v14, 15 patches)

This review covers patches 01/15 through 15/15 of the ENETC4 PMD patch series.
The series adds several features to the ENETC4 PF and VF drivers for i.MX95 NETC IP.

---

## Patch 01/15: Keep-CRC Rx Offload

### Errors

None.

### Warnings

None.

### Info

- The patch correctly advertises `RTE_ETH_RX_OFFLOAD_KEEP_CRC`, configures `RBaMR[CRC]`, and handles the scatter-segment FCS boundary case.
- The `crc_len` field is set before ring enable and checked in the datapath, which is the correct pattern.
- Release notes and feature matrix updated appropriately.

---

## Patch 02/15: TSO Support for VF

### Errors

None.

### Warnings

None.

### Info

- LSO-enabled rings are allocated with `nb_desc * 2` to account for the extension BD per frame. The patch correctly validates `ring_desc > MAX_BD_COUNT` and documents the halving of effective frame capacity.
- The patch correctly sets `dev->tx_pkt_burst` in `enetc4_tx_queue_setup()` so all queues use the same burst function (LSO or non-LSO).
- The LSO burst handles non-TSO packets (falls through to standard encoding), so the function is safe for mixed traffic.
- The check `hdr_len >= rte_pktmbuf_pkt_len(seg)` correctly prevents underflow of `first_payload = seg_len - hdr_len`.
- The LSO compatibility check rejects `KEEP_CRC` (LSO needs HW FCS insertion), which is correct.

---

## Patch 03/15: RSC (Hardware LRO) Support

### Errors

None.

### Warnings

None.

### Info

- RSC preconditions are enforced: `BDS=1` (32B descriptors), `CRC=0` (FCS stripped), `ICEN=1` (interrupt coalescing enabled), and scatter Rx.
- The patch commits `RBMR[BDS]` to HW before setting `RBaRSCR[EN]` to satisfy the hardware precondition that BDS must be active before RSC is enabled.
- The `enetc_refill_rx_ring_rsc()` refill path walks the ring in 2-slot strides (32B descriptors), and the clean path atomically snapshots the 16B writeback half with `__uint128_t` or `rte_memcpy()`.
- `SIRXIDR` W1C is cleared every poll to keep the RSC coalescing timer armed.
- Release notes and documentation correctly note the nc=1 incompatibility.

---

## Patch 04/15: Extended PF-VF Link Speed Field

### Errors

None.

### Warnings

None.

### Info

- The speed code is extended from 4 bits to 8 bits. Speeds > 5Gbps follow the formula `SPEED = (link_speed - 5000) / 1000 + ENETC_SPEED_5000`, removing the need for explicit 10G/25G/50G/100G enumerations.
- The `vf_link_legacy` devarg allows backward compatibility with older kernel PFs (before 6.18.37).
- The patch correctly handles the reply message parsing for both legacy (4-bit code + 4-bit cookie) and current (8-bit code) layouts.
- Release notes document the compatibility requirement.

---

## Patch 05/15: VF Supported Features File

### Errors

None.

### Warnings

None.

### Info

- Adds `enetc4_vf.ini` and updates MAINTAINERS. No code changes.

---

## Patch 06/15: Firmware Version Get for VF

### Errors

None.

### Warnings

None.

### Info

- The PCI revision ID provides the IP major revision, and the VSI-PSI `Get IP version` command (class 0xF0, cmd 0x1) provides the minor revision.
- The fallback to "major.unknown" when `IP_MN` is unavailable is reasonable.
- Release notes updated.

---

## Patch 07/15: Registers Dump

### Errors

None.

### Warnings

None.

### Info

- The PF dumps SI, port, and per-ring BDR registers; the VF dumps SI and per-ring BDR registers (port registers are not accessible).
- The register arrays `enetc4_txbdr_regs` and `enetc4_rxbdr_regs` are declared `const` and have explicit dimensions matching the initializers.
- Release notes updated.

---

## Patch 08/15: Ethtool Ring Parameters

### Errors

None.

### Warnings

None.

### Info

- The patch makes `enetc4_rxq_info_get()` and `enetc4_txq_info_get()` non-static and registers them in both VF ops tables.
- RSC/LSO ring doubling is handled: `qinfo->nb_desc` reports `bd_count / 2` when `rsc_enable` or `lso_enable` is set, so ethtool sees the requested count rather than the doubled HW count.
- Release notes updated.

---

## Patch 09/15: Refresh Link Speed on VF Link-Up Interrupt

### Errors

None.

### Warnings

None.

### Info

- The patch introduces `enetc4_decode_link_speed()` to map status codes to `RTE_ETH_SPEED_NUM_*` / `RTE_ETH_LINK_*_DUPLEX` values, handling both legacy and current layouts.
- `enetc4_process_psi_msg()` now re-queries the link speed from the PF when the link-up notification arrives, before firing the LSC callback.
- The mailbox wait is bounded by `vsi_timeout * vsi_delay` (default 100 * 2000 us = 200 ms) and runs on the EAL interrupt thread with the mailbox interrupt masked.
- The `vsi_lock` mutex is initialized with `PTHREAD_PROCESS_SHARED` via `pthread_mutexattr_setpshared()` so it works in shared memory (primary/secondary process setup).
- Release notes updated.

---

## Patch 10/15: Stats Reset for VF

### Errors

None.

### Warnings

None.

### Info

- The SI-level counters (`SIROCT0`, `SIRFRM0`, etc.) are read-only for a VF and cannot be directly zeroed. The patch implements a software snapshot/delta approach: `stats_reset` captures the current HW values as a baseline, and `stats_get` reports `current_hw_value - saved_baseline`.
- The `enetc4_rd64()` helper reads 64-bit counters with a carry-detection loop (read high, read low, re-read high; retry if high changed).
- Per-ring Rx error accumulators (`ierrors`) are zeroed directly on reset.
- Release notes updated.

---

## Patch 11/15: Per-Queue Rx Interrupt Support for VF

### Errors

None.

### Warnings

None.

### Info

- MSI-X vector 0 is reserved for the PSI-to-VSI mailbox. Rx queue `i` is mapped to vector `i + 1` via `ENETC_SIMSIRRV`.
- The patch calls `rte_intr_efd_enable()` and `rte_intr_vec_list_alloc()` before `rte_intr_enable()` so `vfio-pci` can wire each MSI-X vector to its eventfd when programming the MSI-X table.
- Per-ring coalescing is enabled with `ICPT=1` so the first arriving packet fires the interrupt immediately.
- `SIRXIDR` W1C detect bit is cleared before re-arming `ENETC_RBIER` to prevent spurious interrupts after traffic stops.
- The non-cacheable Rx path (`nc=1`) returns `-ENOTSUP` from `rx_queue_intr_enable`, which is correct.
- The `enetc4_dev_configure()` interrupt setup is triggered when `intr_conf.rxq` is set (not only when `intr_conf.lsc` is set).
- Release notes and documentation updated with usage example and kernel setup instructions.

---

## Patch 12/15: SI-Based Port VLAN Insertion and Removal

### Errors

None.

### Warnings

None.

### Info

- The PF writes directly to `PSIaVLANR(0)` and `PSIaCFGR0(0)`. The VF forwards the request to the kernel PF via VSI-PSI mailbox (class 0x24, cmd 0x1).
- The patch adds `ENETC_CLASS_ID_SI_VLAN_ISO` to the pass-through list in `enetc4_msg_vsi_send()` so the reply is handled correctly.
- The PSI reports command completion in `reply_msg->class_id`: `ENETC_MSG_CLASS_ID_CMD_SUCCESS` (0x1) on success, other values on failure.
- Release notes and documentation updated.

---

## Patch 13/15: Update VF Link Status to Bitmask Encoding

### Errors

None.

### Warnings

None.

### Info

- The PF-to-VF link status code is changed from a two-value enum (`UP=0x0`, `DOWN=0x1`) to a bitmask (`ENETC_LINK_DOWN = BIT(0)`).
- Link up is now encoded as the DOWN bit being clear, which keeps the wire value for link-down identical (0x1) and ensures backward compatibility with older kernel PFs.
- The patch replaces `switch/case` on `ENETC_LINK_UP`/`DOWN` with bitmask decode (`if (msg->status & ENETC_LINK_DOWN)`).
- Release notes updated.

---

## Patch 14/15: Enable Tx PAUSE via VF Rx Congestion Mode

### Errors

None.

### Warnings

None.

### Info

- When the PF negotiates TX PAUSE on the port, it signals this to the VF via `BIT(1)` of the PF-to-VF link status mailbox message.
- The VF PMD responds by setting `RBMR_CM` (`BIT(4)`) on all active RX rings so the MAC emits PAUSE frames on ingress pressure.
- The `enetc4_vf_set_congestion_mode()` helper updates all active RX rings and persists the state in `hw->tx_pause_active` so rings started later inherit the correct state.
- The function is hooked into both the interrupt and poll link-update paths, and the saved state is applied in `rx_queue_setup()` and `rx_queue_start()`.
- The `vsi_lock` mutex serializes `RBMR` read-modify-write sequences so `set_congestion_mode()` and `rx_queue_start/stop()` cannot race on the same register.
- RX PAUSE (honoring received PAUSE frames) is handled at the MAC level by the PF and requires no VF PMD changes.
- Release notes updated.

---

## Patch 15/15: Add WRR Tx Scheduler Devarg for VF Rings

### Errors

None.

### Warnings

None.

### Info

- The `enetc4_txq_wrr` devarg configures per-ring WRR weights in the NETC LEAF-level Tx scheduler (`TBaMR` register, bits [6:4]).
- The patch validates WRR weights (1..8) and stores them pre-shifted for direct OR into `TBMR`.
- `TBaMR` is in the VF's own SI space, so no Linux PF involvement is needed.
- The WRR bits are cleared before applying fresh devarg values in `tx_queue_setup()` and `tx_queue_start()`.
- The `parse_txq_wrr()` parser explicitly rejects leading `-` to prevent `strtoul()` wrap-around.
- Release notes and documentation updated with usage examples.

---

## Series-Wide Observations

### Errors

None.

### Warnings

None.

### Info

1. **Secondary process support**: Patches 01 and 09 document that secondary processes cannot run the Rx/Tx datapath when LRO or TSO is enabled (doubled ring layout is a primary-only setup-time decision). The secondary-process initialization in `enetc4_dev_init()` and `enetc4_vf_dev_init()` sets the base burst pointers (`nc` or `cacheable`) and returns early, which is the correct pattern.

2. **Process-shared mutex**: Patch 09 initializes `vsi_lock` with `pthread_mutexattr_setpshared(&attr, PTHREAD_PROCESS_SHARED)` so it works correctly in shared memory (primary/secondary process setup). This is the correct pattern.

3. **64-bit counter reads**: Patch 10 introduces `enetc4_rd64()` with a carry-detection loop to read 64-bit statistics counters atomically on 32-bit accesses. The retry logic is correct.

4. **Documentation**: All patches update release notes and documentation appropriately. The RST documentation uses definition lists where appropriate (per the guidelines).

5. **Code quality**: The patches follow DPDK C coding style: 8-character tabs, `rte_` prefixes, lowercase function names, explicit comparisons (`!= NULL`, `!= 0` except on `bool`), and no forbidden tokens. The `enetc4_txbdr_regs` and `enetc4_rxbdr_regs` arrays are declared `const` with explicit dimensions.

6. **Error handling**: All VSI-PSI mailbox transactions check return values and propagate errors. The `vsi_lock` mutex is always released on error paths.

7. **Release notes**: All patches update `release_26_11.rst` with appropriate feature descriptions and compatibility notes (e.g., `vf_link_legacy` for older kernel PFs).

---

## Summary

**All 15 patches are correct.** No errors or warnings identified. The series implements Keep-CRC, TSO, RSC (LRO), extended link speed encoding, firmware version reporting, register dump, ring parameters, link speed refresh on interrupt, stats reset, per-queue Rx interrupts, SI-based port VLAN, bitmask link status, Tx PAUSE, and WRR Tx scheduler for the ENETC4 PMD. Code quality is high, error handling is complete, and documentation is thorough.