|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