|WARNING| [2/2] common/cnxk: fix DPC double counting for legacy pools
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Fri Oct 30 11:36:06 CET 2026
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
None identified.
### Warnings
None identified.
### Info
None identified.
---
## PATCH 2/2: common/cnxk: fix DPC double counting for legacy pools
### Errors
**Error path resource leak in roc_npa_pool_dpc_enable()**
The error path at the `disable:` label has been removed, but the code after the first `mbox_process()` can still fail.
If `rsp->hdr.rc != 0` after the first AQ operation completes, the function returns with `rc = rsp->hdr.rc` (implicitly, via the goto exit with rc unset), leaving DPC enabled in the aura context with no way to clean up.
Before this patch, the `disable:` label called `roc_npa_pool_dpc_disable()` to roll back the partial configuration.
After this patch, that rollback is gone.
**Original code (simplified):**
```c
req = mbox_alloc_msg_npa_cn20k_aq_enq(mbox);
/* ... configure aura DPC ... */
rc = mbox_process(mbox);
if (rc < 0)
goto disable; /* rollback */
rsp = ...;
if (rsp->hdr.rc != 0)
goto disable; /* rollback */
rc = 0;
goto exit;
disable:
roc_npa_pool_dpc_disable(aura_handle, flags);
exit:
mbox_put(mbox);
return rc;
```
**New code:**
```c
req = mbox_alloc_msg_npa_cn20k_aq_enq(mbox);
/* ... configure aura DPC ... */
rc = mbox_process(mbox);
if (rc < 0)
goto exit; /* no rollback */
rsp = ...;
if (rsp->hdr.rc != 0)
goto exit; /* no rollback */
rc = 0;
exit:
mbox_put(mbox);
return rc;
```
If either failure occurs after the aura DPC was successfully written but before success is confirmed, the aura is left with DPC partially configured.
**Suggested fix:**
Restore the cleanup path:
```c
rc = 0;
goto exit;
disable:
roc_npa_pool_dpc_disable(aura_handle, flags);
exit:
mbox_put(mbox);
return rc;
```
Or add explicit rollback at each failure point:
```c
rc = mbox_process(mbox);
if (rc < 0)
goto exit;
off = mbox->rx_start +
PLT_ALIGN(sizeof(struct mbox_hdr), MBOX_MSG_ALIGN);
rsp = (struct npa_cn20k_aq_enq_rsp *)((uintptr_t)mdev->mbase + off);
if (rsp->hdr.rc != 0) {
roc_npa_pool_dpc_disable(aura_handle, flags);
goto exit;
}
rc = 0;
exit:
mbox_put(mbox);
return rc;
```
---
**Variable assigned without initialization and overwritten before use**
In `roc_npa_pool_dpc_enable()`, the `rc` variable is declared at function entry but not initialized.
If `mbox_alloc_msg_npa_cn20k_aq_enq()` returns NULL (allocation failure), the function goes to `exit:` and returns `rc` uninitialized.
**Problematic code:**
```c
int
roc_npa_pool_dpc_enable(...)
{
int rc;
...
req = mbox_alloc_msg_npa_cn20k_aq_enq(mbox);
if (req == NULL)
goto exit; /* rc is uninitialized */
...
exit:
mbox_put(mbox);
return rc;
}
```
**Suggested fix:**
```c
int rc = -ENOMEM; /* or -ENOSPC, matching mbox allocation failure semantics */
```
Or explicitly set it before the goto:
```c
req = mbox_alloc_msg_npa_cn20k_aq_enq(mbox);
if (req == NULL) {
rc = -ENOMEM;
goto exit;
}
```
Same issue exists in `roc_npa_pool_dpc_disable()`.
---
### Warnings
None identified.
### Info
None identified.
---
Review-Result: ERROR
More information about the test-report
mailing list