|WARNING| common/cnxk: fix duplicate mbox alloc in NPA aura pool init

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## Summary
This patch fixes a critical double-allocation bug in `npa_aura_pool_init()` where `mbox_alloc_msg_npa_aq_enq()` is called twice, orphaning the first allocation and potentially sending uninitialized data to hardware.

---

## Errors

### 1. Resource Leak - Orphaned Mbox Message
**Issue:** The first allocated mbox message is leaked when `pool_init_req` is overwritten by the second allocation.

**Why it matters:** The orphaned message remains in the mbox queue and will be sent with uninitialized fields (`aura_id`, `ctype`, `op`), which can cause undefined hardware behavior.

**Verification:** The patch correctly removes the duplicate allocation at line 112, eliminating both the leak and the uninitialized-message hazard.

**Status:**  Fixed by this patch

---

## Warnings

### 1. NULL Check After Potential Use
**Issue:** The NULL check at line 113 (`if (pool_init_req == NULL)`) comes after the pointer has already been dereferenced in the commit message's scenario. However, reviewing the actual code flow:

```c
if (roc_model_is_cn20k()) {
    pool_init_req = mbox_alloc_msg_npa_lf_alloc_aura_pool(mbox);
} else {
    pool_init_req = mbox_alloc_msg_npa_aq_enq(mbox);
}
/* Duplicate line removed by patch was here */
if (pool_init_req == NULL)
    goto exit;
```

The NULL check is correctly positioned after the allocation(s). The bug was that the second allocation could fail and return NULL, overwriting a valid pointer from the first allocation, which would then leak.

**Status:** No issue with NULL check placement; the patch resolves the underlying bug.

---

## Info

### 1. Error Path Analysis
The `goto exit` at line 114 should be verified to properly clean up any resources allocated before this point. Reviewing typical DPDK mbox patterns:

```c
exit:
    return -ENOMEM; // or similar
```

The mbox messages are typically cleaned up by the mbox subsystem when the transaction completes or is aborted. No additional cleanup flag needed here, but verify that `mbox_process()` (or equivalent) handles allocation failures correctly in the caller's context.

**Recommendation:** Ensure the function calling `npa_aura_pool_init()` handles the error return and does not attempt to use `pool_init_req` or proceed with incomplete initialization.

---

## Conclusion

**This patch is CORRECT and should be merged.** It fixes a critical bug where:
1. A mbox message is leaked
2. An uninitialized message with garbage data could be sent to hardware
3. The intended message may be lost if the second allocation fails

The fix is minimal, surgical, and eliminates the duplicate allocation as intended.

### Pre-merge Checklist
-  Correctness: Fixes resource leak and uninitialized data issue
-  Style: Deletion-only patch, no style concerns
-  Commit message: Clearly describes bug, impact, and fix
-  Fixes tag: Correct SHA and Cc stable
-  Testing: Verify on both CN10K and CN20K platforms to confirm no unintended side effects

**Reviewed-by recommendation: APPROVE**