From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by smtp.lore.kernel.org (Postfix) with ESMTP id 45882C79F99 for ; Mon, 7 Sep 2026 20:47:57 +0000 (UTC) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 70A9C40655; Mon, 7 Sep 2026 22:47:56 +0200 (CEST) Received: from mail-pj1-f49.google.com (mail-pj1-f49.google.com [209.85.216.49]) by mails.dpdk.org (Postfix) with ESMTP id 00927402E8 for ; Mon, 7 Sep 2026 22:47:54 +0200 (CEST) Received: by mail-pj1-f49.google.com with SMTP id 98e67ed59e1d1-398d2b28acfso2825153a91.1 for ; Mon, 07 Sep 2026 13:47:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20251104.gappssmtp.com; s=20251104; t=1788814074; x=1789418874; darn=dpdk.org; h=content-transfer-encoding:content-type:mime-version:references :in-reply-to:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=oVN2EWVCHfEoxY6VcDATyWrHC64pwpfrcHzRYNRX1Jg=; b=cGM2KGk9Uk8tA5ZGzNSS4norlo6+hopdFCWcrWlmwhq1r3OUbZZGOt3c66xCLC+yHj ue6PktmpZDoQ4Qdcv6xas7MUqezCYcMUgbxYG0ly7GZ64MtsjZZFe7NIFZv71ievILkk mLRC7FZ/5NoGJ14hTQPbd9cT4dVFN4PjyqMowXusxBN9b+miLT+POhoCbWN2ZXFeUDat 2iisBZmLtwWBEAdyMyoYujttDbefPiUfdS94Nx2OCdTfZso2MmL9+BT0Mqkq8puijYg/ Syk1lzztjIX393tWeimOgSUt3PE2Vw7mFKYcLgTPJ+GtaQBLibbjw6ARwHm4WU2Poabl iTTA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1788814074; x=1789418874; h=content-transfer-encoding:content-type:mime-version:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=oVN2EWVCHfEoxY6VcDATyWrHC64pwpfrcHzRYNRX1Jg=; b=gjPahx0zd9GqQbOwcspgbZBJHbSRnHr1WnxwAkd3kv8EVnn3fq1H/kfC6TQBUUlSrP Z96LoSYv5sxg4gDtu2b0zuQcxCJ57C+io5EkDBV46wLrQ1VWqScftH03HFJQTtcYbuWG LWSS1AQfoYVg90HnvSXolkWEJg+c8Stl9mmL0bzPBM5SyGrg1CGPN2DtAz6PX9EzuB5J ta5OHJz5cZd9qdjwFlPkxNlN/QBl2mnqraf/ykyXSUrIHxDTe2/f4cQZM7syyOIjKecN pkt+g9iSs6M4eyUbgpanp5G4EFxPhrz1edHdfj/dQLiUZDfjQzs++mBjMPdjN/0fxbxB DLmg== X-Gm-Message-State: AFuF++nZopEWR881lvqE8y7TrkG22QgChjhMslWrQux/Rpz2nczIeikq ZAEQwzTpQMIkFFFXzK7lAet/iGyuquPDLtuRQJLyTYBlevjmgAIgvHJm8rI9az5cG8g= X-Gm-Gg: AYBFou0wzPBMYEy89ijtDF8SlaAVriBC7bKQ/sLXszlUsfY6ZJ3qTe9cy9FV9j4TK0E EaCCL6prx0LkCkP2L/PBqdMzbkzHH4l7ZZY6sUkuNaDdr0sp6eb4TgyD6uhHgnLVD8w4Roa4hks 2kt1jxsVb+NNdkXj9yFhCbNnYiOEWVLddscNi/B0mI0Zt/aEeaW5td1KgdkcZHGjj5nbDy45Weg 0vGVK1NNSMQ35P6S2o2w3iDDUbOS7sUwPcFAT3+MnZxyl4ex6xOJF426wmH2mIxvfr8srCNKYVY nt3GiN/AzVT2+VZZKaUa0v5PVlszH96b4q/AbNMsRHCzu9HD8buZb95iJPCTmQFmefUVvCeQCrz QMlq8N3GUOxzJSS8Rm8BoVuViuaKYv05p69r0I3m9cEKiA8YgHDY56Ocsua6zC2r8wDDG45iiRJ ycwugEM0RAqW7WIUykhSFQErpKYpe90T0wTW12OVAPEBRlKSuZ4dvkINrdneBs7WamGMtv5UOFy UxEhFuOi77mLV9qwa7curNo99fFWA== X-Received: by 2002:a17:90b:33c9:b0:398:c16e:69d3 with SMTP id 98e67ed59e1d1-39b26122fbamr34915673a91.1.1788814073872; Mon, 07 Sep 2026 13:47:53 -0700 (PDT) Received: from phoenix.local (204-195-96-226.wavecable.com. [204.195.96.226]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-39b08c397b1sm28175504a91.8.2026.09.07.13.47.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 07 Sep 2026 13:47:53 -0700 (PDT) Date: Mon, 7 Sep 2026 13:47:44 -0700 From: Stephen Hemminger To: Prashant Gupta Cc: dev@dpdk.org Subject: Re: [PATCH 00/45] net/dpaa2: features and fixes for NXP DPAA2 drivers Message-ID: <20260907134744.177399c8@phoenix.local> In-Reply-To: <20260903135353.3358303-1-prashant.gupta_3@nxp.com> References: <20260903135353.3358303-1-prashant.gupta_3@nxp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Thu, 3 Sep 2026 19:23:08 +0530 Prashant Gupta 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. Another round of errors detected by Fable against 9-20. 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