[PATCH 00/45] net/dpaa2: features and fixes for NXP DPAA2 drivers
Stephen Hemminger
stephen at networkplumber.org
Mon Sep 7 23:03:27 CEST 2026
On Thu, 3 Sep 2026 19:23:08 +0530
Prashant Gupta <prashant.gupta_3 at nxp.com> wrote:
> This series brings the NXP DPAA2 driver stack up to date with the
> functionality carried in the NXP internal tree, together with a number of
> bug fixes. It covers the crypto (dpaa2_sec), net, dma, mempool, event and
> bus/fslmc drivers.
>
> Fixes first, then features, so the bug fixes can be picked independently of
> the larger reworks.
And the errors caught by Fable continue in Patch 21-32
DPAA2 series review (bundle 2096), patches 9-20 of 45
Base: upstream main d55ccd4; series applied with git am --3way.
Patch 9: dma/dpaa2: fix array-bounds warning in dequeue path
Warning: The warning this patch fixes does not exist at this point
in the series. At patch 9, qdma_cntx_idx_ring_eq() is still the
per-element loop:
for (i = 0; i < nb; i++) {
ring->cntx_idx_ring[ring->tail] = elem[i];
and passing &idx with nb == 1 indexes elem[0] only. The
-Warray-bounds diagnostic comes from the rte_memcpy rewrite in
patch 12, which forms &elem[DPAA2_QDMA_MAX_DESC - ring->tail] on a
one-element object in the wrap-around branch. So this is not a fix
for 388e888dc082 and should not carry Fixes:/Cc: stable; it
belongs with patch 12.
The fix itself is heavy: DPAA2_QDMA_MAX_DESC is
((1 << 13) / 2) = 4096, so
uint16_t idxs[DPAA2_QDMA_MAX_DESC];
adds 8 KB to every struct qdma_virt_queue and only idxs[0] is ever
written. Handle the single-index case in ring_eq instead, e.g.
compute first = RTE_MIN(nb, DPAA2_QDMA_MAX_DESC - ring->tail),
copy first elements, and only touch elem + first when nb > first,
which also gives the compiler a bound it can prove.
Patch 11: dma/dpaa2: validate IOVA in pre-populate helpers
Error: The new early returns leak the fle_pool element in
non-silent mode. In dpaa2_qdma_copy_sg():
ret = rte_mempool_get(qdma_vq->fle_pool,
(void **)&cntx_sg);
...
ret = fle_sdd_sg_pre_populate(cntx_sg, qdma_vq);
if (ret)
return ret;
and in dpaa2_qdma_long_copy():
ret = rte_mempool_get(qdma_vq->fle_pool,
(void **)&fle_sdd);
...
ret = fle_sdd_pre_populate(fle_sdd,
&qdma_vq->rbp,
0, 0, QBMAN_FLE_WORD4_FMT_SBF);
if (ret)
return ret;
Neither path returns the context to the pool. Each failed enqueue
permanently consumes one fle_pool element. Add rte_mempool_put()
on the error path when !is_silent (the silent path uses the
pre-allocated cntx_sg[]/cntx_fle_sdd[] arrays and needs no free).
Patch 13: drivers: add dpaa2 DMA bypass memory translation option
Error: RTE_DPAAX_QDMA_BMT_FLAG is only partially honoured.
(a) In the pre-populate SG path the flag is applied once, when the
context is first initialised:
if (unlikely(!fle[DPAA2_QDMA_SRC_FLE].length)) {
ret = fle_sdd_sg_pre_populate(cntx_sg, qdma_vq,
(flags & RTE_DPAAX_QDMA_BMT_FLAG) ?
QDMA_SG_BMT_ENABLE : QDMA_SG_BMT_DISABLE);
sg_entry_post_populate() never writes ctrl.bmt, so every later job
that picks this context out of fle_pool inherits whatever the first
job asked for, regardless of its own flags.
(b) The long-FD path ignores the flag entirely. Neither
fle_sdd_pre_populate() nor fle_populate() takes a bmt argument and
nothing sets fle->word4.bmt, so on a vchan with
using_short_fd == 0 (dpaa2_qdma_long_copy) the flag has no effect.
The commit message says the flag is propagated to all FD populate
helpers; it is not.
Warning: RTE_DPAAX_QDMA_BMT_FLAG is a new public API in an
installed header (drivers/common/dpaax/meson.build lists
rte_pmd_dpaax_qdma.h in headers) with no Doxygen comment, no
mention in doc/guides/dmadevs/dpaa2.rst, and no release note. It
also occupies bit 7, immediately below the RTE_DPAAX_QDMA index
bits at 8+, and immediately above the generic
RTE_DMA_OP_FLAG_* bits (0-3), so a future generic flag at bit 4-7
collides with it.
Patch 14: mempool/dpaa2: support ops index from primary in
secondary
Error: rte_dpaa2_mpool_get_ops_idx() returns the wrong value on
the first successful IPC round trip in a secondary:
if (rsp_msg->msg_type == DPAA2_POOL_OPS_IDX_RSP) {
rte_memcpy(&s_dpaa2_pool_ops_idx, rsp_msg->msg_data,
sizeof(s_dpaa2_pool_ops_idx));
ret = 0;
} ...
free(mp_reply.msgs);
return ret;
It returns 0 instead of s_dpaa2_pool_ops_idx. Every caller compares
the result against mb_pool->ops_index (dpaa2_sec enqueue, net Tx),
so the first packet burst in a secondary process gets a false
mismatch and takes the MAX_BPID path. Return
s_dpaa2_pool_ops_idx on success.
Error: mp_req is an uninitialised stack struct:
struct rte_mp_msg mp_req;
...
strlcpy(mp_req.name, DPAA2_POOL_MP_SYNC, sizeof(mp_req.name));
req_msg->msg_type = DPAA2_POOL_OPS_IDX_REQ;
...
ret = rte_mp_request_sync(&mp_req, &mp_reply, &ts);
len_param and num_fds are never set. check_input() in
eal_common_proc.c rejects negative or oversized values, and
send_msg() attaches num_fds entries of fds[] as SCM_RIGHTS, so with
stack garbage the request either fails outright or sends random
descriptors. memset the request to zero and set
mp_req.len_param = sizeof(struct dpaa2_pool_mp_msg) +
sizeof(s_dpaa2_pool_ops_idx). The primary-side reply has the
matching problem: reply is zeroed but reply.len_param stays 0
while param carries the ops index.
Error: Resource leak on the new failure path in
rte_hw_mbuf_create_pool():
ret = rte_mp_action_register(DPAA2_POOL_MP_SYNC,
dpaa2_mbuf_pool_mp_primary);
if (ret && rte_errno != ENOTSUP)
return ret;
At this point bp_list and bp_info are allocated, the dpbp is
enabled and avail_dpbp is taken; every other failure in this
function uses goto err4. Replace the return with goto err4.
rte_mp_action_register() also returns -1 with the reason in
rte_errno, so the caller sees a bare -1; return -rte_errno.
Warning: rte_memcpy() for a 2-byte copy on the IPC control path
(both in the handler and in the requester); use memcpy().
Patch 16: drivers: optimize dpaa2 Tx queue and channel mapping
Error: The new capping logic is defeated by uint8_t truncation.
priv->nb_rx_queues and priv->nb_tx_queues are uint8_t, and
attr.num_rx_tcs, attr.num_tx_tcs and attr.num_queues are uint8_t:
priv->nb_rx_queues = attr.num_rx_tcs * attr.num_queues;
if (priv->nb_rx_queues > MAX_RX_QUEUES) {
...
priv->nb_tx_queues = attr.num_tx_tcs * attr.num_queues;
if (priv->nb_tx_queues > MAX_TX_QUEUES) {
The product is computed as int and truncated to 8 bits on
assignment before the comparison, so a DPNI with 8 TCs and
32 queues per TC (256) yields nb_rx_queues = nb_tx_queues = 0 and
the "Too many" branch never fires. Compute into a uint32_t local,
cap, then assign. The TX side is new in this patch (the old value
was num_tx_tcs * num_channels, which cannot overflow); the RX side
overflowed before too but is rewritten here.
Related: capping the total to 128 rather than the per-TC count
makes tc_index = i / (128 / num_tx_tc) exceed num_tx_tc when
num_tx_tc does not divide 128 (e.g. 3 TCs -> per_tc 42,
i = 126, 127 map to TC 3). Cap num_queue_per_tc instead and derive
the totals from that.
Error: The new early return in dpaa2_dev_rx_queue_setup() breaks
Rx queue reconfiguration:
if (dpaa2_q->fqid != DPAA2_INVALID_FQ_ID) {
DPAA2_PMD_WARN("%s: RXQ[%d] has been setup",
dev->data->name, rx_queue_id);
dev->data->rx_queues[rx_queue_id] = dpaa2_q;
return 0;
}
dpaa2_dev_rx_queue_release() does not reset fqid to
DPAA2_INVALID_FQ_ID, so the stop / reconfigure / rx_queue_setup
sequence that rte_eth_rx_queue_setup() performs (release, then
setup) hits this return and silently keeps the old mb_pool, nb_desc
and offloads. Worse, release cleared the CGID
(priv->cgid_in_use[cgid] = 0; dpaa2_q->cgid = DPAA2_INVALID_CGID)
and the early return skips the block that allocates a new one and
programs taildrop / congestion notification, so the queue runs
without its drop configuration after any reconfigure. Either reset
fqid in rx_queue_release, or drop the early return on the Rx side
(the Tx side had the same pattern before this patch via
DPAA2_INVALID_FLOW_ID, and tx_queue_release should be checked for
the same reason).
Patch 17: net/dpaa2: support larger burst size
Warning: The version gate excludes newer major versions:
if (priv->dpni_ver_major == 8 && priv->dpni_ver_minor >= 7)
A DPNI API 9.x would fall back to 64 KB. dpaa2_ethdev.h already has
dpaa2_dev_cmp_dpni_ver(priv, major, minor); use
dpaa2_dev_cmp_dpni_ver(priv, 8, 7) >= 0.
Warning: The new header comment and the code disagree on the
non-LX2160A limit. fsl_dpni.h says:
* @max_burst_size: Burst size in bytes. Limits depend on the SoC
* (0x37FFF for LX2160A, 0xF7FF for all others)
but dpaa2_get_burst_max() returns (64 * 1024) = 0x10000 for
"all others", which is above 0xF7FF. One of the two is wrong.
Patch 18: net/dpaa2: support MPLS and PPPoE flow distribution
Warning: doc/guides/nics/features/dpaa2.ini is not updated; the
[rte_flow items] section needs mpls and pppoes entries for the new
pattern support.
Patch 19: net/dpaa2: support meter and policing
Error: dpaa2_mtr_ops_get() re-initialises the lock on every call:
int
dpaa2_mtr_ops_get(struct rte_eth_dev *dev, void *ops)
{
struct dpaa2_dev_priv *priv = dev->data->dev_private;
rte_spinlock_init(&priv->meter_lock);
rte_mtr_ops_get() in lib/ethdev/rte_mtr.c calls
dev->dev_ops->mtr_ops_get() at the start of every rte_mtr_*() API
call, so any concurrent rte_mtr call resets a spinlock that another
thread may currently hold, and the list manipulation it protects
is then unprotected. Initialise the lock once in dpaa2_dev_init()
next to the LIST_HEAD fields.
Warning: dpaa2_mtr_profile_delete() and dpaa2_mtr_policy_delete()
walk priv->meters and silently free every meter that references
the profile/policy:
if (meter->profile_id == profile_id) {
...
LIST_REMOVE(tmp, next);
rte_free(tmp);
rte_mtr.h documents both operations as failing when at least one
MTR object still uses the profile/policy. Return -EBUSY instead;
otherwise the application keeps a meter id that no longer exists
and any flow created with it (patch 37 wires meters into
dpni_set_rx_tc_policing) is left pointing at a freed object.
Warning: No documentation for the new feature: nothing in
doc/guides/nics/features/dpaa2.ini, doc/guides/nics/dpaa2.rst, or
the 26.11 release notes. (The series as a whole adds nothing under
doc/.)
Info: In dpaa2_mtr_profile_add():
} else if (profile->packet_mode > DPNI_POLICER_UNIT_FRAMES) {
dpaa2_profile->policer_unit =
DPNI_POLICER_UNIT_BYTES_L2_WITHOUT_FCS;
rte_mtr_meter_profile.packet_mode is a 0/1 flag and
DPNI_POLICER_UNIT_FRAMES is 1, so this branch is unreachable.
Info: s_err_msg is a single static buffer shared by every port and
thread, and its address is handed to the caller via
rte_mtr_error_set(); concurrent callers can see each other's
message. dpaa2_mtr_policy_add() returns ENOMEM for "action not
supported" (should be ENOTSUP), and dpaa2_mtr_profile_add() returns
-ENOTSUP without filling in *error. struct dpaa2_dev_meter_policy's
red_drop is an int holding a bool.
Review-Result: ERROR
More information about the dev
mailing list