|WARNING| [2/2] common/cnxk: fix DPC double counting for legacy pools

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## Patch 1/2: common/cnxk: fix DPC alloc config width

### ERRORS

**Integer truncation on mailbox parameter**

The patch correctly identifies and fixes an integer truncation bug where a 64-bit configuration value was truncated to 16 bits before being sent to the AF (Admin Function). This would cause the upper 48 bits of `dpc_conf` to be silently lost.

```c
/* Before - WRONG: truncates 64-bit value to 16 bits */
uint16_t __io dpc_conf;

/* After - CORRECT: preserves full 64-bit value */
uint64_t __io dpc_conf;
```

The fix is correct and necessary. The patch updates all three locations consistently: the mailbox structure (`roc_mbox.h`), the function signature in the implementation (`roc_npa.c`), and the function declaration in the header (`roc_npa.h`).

**Missing release notes entry**

This is a bug fix that changes the API signature of `roc_npa_dpc_alloc()`. According to AGENTS.md, "Changes to existing API require release notes." The patch should update `doc/guides/rel_notes/release_XX_YY.rst` (where XX_YY is the current release) to document this fix.

---

## Patch 2/2: common/cnxk: fix DPC double counting for legacy pools

### ERRORS

None.

### WARNINGS

**Missing release notes entry**

This patch fixes a behavioral bug (double counting) in the DPC (Diagnostic Performance Counter) logic. While it does not change API signatures, it is a significant functional fix that users should be aware of. The release notes should document that DPC counts for legacy pools are now correct (counted once instead of twice).

### Code Review

**Logic change verification:**

The patch removes duplicate DPC enablement/disablement on pool contexts, keeping only the aura context operations. The removal is clean:

- `roc_npa_pool_dpc_enable()`: Originally enabled DPC on both pool and aura; now only on aura (lines removed: pool enable + second aura enable + error handling)
- `roc_npa_pool_dpc_disable()`: Originally disabled DPC on both pool and aura; now only on aura (lines removed: second aura disable operation)

The halo path (`flags & ROC_NPA_HALO_F`) is unchanged and still correctly uses `NPA_AQ_CTYPE_HALO`.

**Resource cleanup:**

The original code had a `goto disable` path that would call `roc_npa_pool_dpc_disable()` on failure. This error path is removed. Reviewing the function:

- `mbox_alloc_msg_npa_cn20k_aq_enq()` failure: immediately goes to `exit` (correct - no cleanup needed)
- `mbox_process()` failure: goes to `exit` (correct - mailbox handles its own cleanup)
- `rsp->hdr.rc != 0`: goes to `exit` (correct - AF rejected the request, no local state to clean)

The removal of the disable-on-error path is acceptable because:
1. The function now performs only one operation instead of two
2. If that operation fails, no DPC state was successfully modified, so no rollback is needed
3. The caller can determine whether to retry or free the counter_id

**Code reduction:**

The patch removes ~50 lines of duplicated logic. The simplification makes the code easier to maintain and eliminates the possibility of the pool and aura settings diverging.

---

## Summary

Both patches address real correctness bugs:
1. Integer truncation that would cause configuration loss
2. Double counting that would produce incorrect diagnostic data

However, both patches are missing release notes updates documenting these fixes.

---

Review-Result: ERROR