|WARNING| [v7-S1, 6/6] mempool/dpaa2: look up ops index locally in secondary

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## Patch 1/6: bus/fslmc: defer bus initialization to probe

### Errors

None.

### Warnings

None.

### Info

The patch moves VFIO setup and DMA mapping from scan to probe, which is correct since these operations require hugepage memory that is not available during scan. The error handling in the new `rte_fslmc_probe()` function properly propagates error codes instead of returning 0 on failure (as the old scan code did). No issues found.

---

## Patch 2/6: bus/fslmc: reduce probe logging and skip ignored devices

### Errors

None.

### Warnings

None.

### Info

The logging level change (NOTICE to DEBUG) is appropriate for reducing noise. The addition of `rte_bus_device_is_ignored()` check ensures consistency with the main probe loop. No issues found.

---

## Patch 3/6: dma/dpaa2: use memcpy to fill completion index ring

### Errors

None.

### Warnings

None.

### Info

The refactoring from a loop to at most two memcpy calls is a straightforward optimization. The logic correctly handles the wrap-around case. No issues found.

---

## Patch 4/6: dma/dpaa2: release SG FLE on completion ring overflow

### Errors

None.

### Warnings

None.

### Info

The patch clarifies ownership of the FLE on error by releasing it immediately with `rte_mempool_put()` instead of relying on the bulk release path. This is a good practice change even though the original code was correct. No issues found.

---

## Patch 5/6: dma/dpaa2: validate FLE pool IOVA mapping at vchan setup

### Errors

**Error 1: Resource leak on validation failure**

In `dpaa2_qdma_vchan_setup()`, when the FLE pool IOVA validation fails after successful `rte_mempool_get_bulk()` calls for silent mode, the patch adds `goto err_pool` which releases the mempool but does NOT return the already-allocated FLE objects from the two `rte_mempool_get_bulk()` calls.

Lines 1425-1434 allocate objects from the pool:
```c
if (qdma_dev->is_silent) {
    ret = rte_mempool_get_bulk(qdma_dev->vqs[vchan].fle_pool,
            (void **)qdma_dev->vqs[vchan].cntx_sg,
            RTE_DPAA2_QDMA_JOB_SUBMIT_MAX);
    if (ret) {
        DPAA2_QDMA_ERR("sg cntx get from %s for silent mode",
                   pool_name);
        goto err_pool;
    }
    ret = rte_mempool_get_bulk(qdma_dev->vqs[vchan].fle_pool,
            (void **)qdma_dev->vqs[vchan].cntx_fle_sdd,
            RTE_DPAA2_QDMA_JOB_SUBMIT_MAX);
    if (ret) {
        DPAA2_QDMA_ERR("long cntx get from %s for silent mode",
                   pool_name);
        goto err_pool;
    }
}
```

But `err_pool` only does:
```c
err_pool:
    rte_mempool_free(qdma_dev->vqs[vchan].fle_pool);
    qdma_dev->vqs[vchan].fle_pool = NULL;
    return ret;
```

When the second `rte_mempool_get_bulk()` fails, the first batch (`cntx_sg`) is not returned. When `ring_cntx_idx` allocation fails after both get_bulk succeed, neither batch is returned. These are leaks within the mempool (the pool itself is freed, but its accounting is corrupted).

**Suggested fix:** Add `rte_mempool_put_bulk()` calls in the error path or on failure of the second allocation:

```c
if (qdma_dev->is_silent) {
    ret = rte_mempool_get_bulk(qdma_dev->vqs[vchan].fle_pool,
            (void **)qdma_dev->vqs[vchan].cntx_sg,
            RTE_DPAA2_QDMA_JOB_SUBMIT_MAX);
    if (ret) {
        DPAA2_QDMA_ERR("sg cntx get from %s for silent mode",
                   pool_name);
        goto err_pool;
    }
    ret = rte_mempool_get_bulk(qdma_dev->vqs[vchan].fle_pool,
            (void **)qdma_dev->vqs[vchan].cntx_fle_sdd,
            RTE_DPAA2_QDMA_JOB_SUBMIT_MAX);
    if (ret) {
        DPAA2_QDMA_ERR("long cntx get from %s for silent mode",
                   pool_name);
        rte_mempool_put_bulk(qdma_dev->vqs[vchan].fle_pool,
                (void **)qdma_dev->vqs[vchan].cntx_sg,
                RTE_DPAA2_QDMA_JOB_SUBMIT_MAX);
        goto err_pool;
    }
} else {
    qdma_dev->vqs[vchan].ring_cntx_idx = rte_malloc(NULL,
            sizeof(struct qdma_cntx_idx_ring),
            RTE_CACHE_LINE_SIZE);
    if (!qdma_dev->vqs[vchan].ring_cntx_idx) {
        DPAA2_QDMA_ERR("DQ response ring alloc failed.");
        ret = -ENOMEM;
        goto err_pool;
    }
    ...
}
...
return 0;

err_pool:
    if (qdma_dev->is_silent) {
        if (qdma_dev->vqs[vchan].cntx_fle_sdd[0]) {
            rte_mempool_put_bulk(qdma_dev->vqs[vchan].fle_pool,
                    (void **)qdma_dev->vqs[vchan].cntx_fle_sdd,
                    RTE_DPAA2_QDMA_JOB_SUBMIT_MAX);
        }
        if (qdma_dev->vqs[vchan].cntx_sg[0]) {
            rte_mempool_put_bulk(qdma_dev->vqs[vchan].fle_pool,
                    (void **)qdma_dev->vqs[vchan].cntx_sg,
                    RTE_DPAA2_QDMA_JOB_SUBMIT_MAX);
        }
    }
    rte_mempool_free(qdma_dev->vqs[vchan].fle_pool);
    qdma_dev->vqs[vchan].fle_pool = NULL;
    return ret;
```

Or more simply, add an `err_silent` label before `err_pool` that does the put_bulk calls, and use that from the silent-mode error paths.

---

## Patch 6/6: mempool/dpaa2: look up ops index locally in secondary

### Errors

None.

### Warnings

None.

### Info

The fix correctly addresses the secondary process issue by locally scanning the ops table. The logic is sound: mempool ops registration order is process-invariant because it is driven by constructors in the same link order. The error message improvement ("Only single ops index only" -> "Single ops index only") is good. No issues found.

---

## Summary

**Total issues: 1 Error**

**Patch 5/6 requires a fix** for the resource leak in the error path when silent mode FLE allocation partially succeeds. All other patches are correct.