[PATCH v16 00/23] NXP DPAA driver enhancements and fixes

[Hemant Agrawal]

NXP Confidential
> -----Original Message-----
> From: Stephen Hemminger <stephen at networkplumber.org>
> Sent: 28 September 2026 23:54
> To: Hemant Agrawal <hemant.agrawal at nxp.com>
> Cc: thomas at monjalon.net; dev at dpdk.org
> Subject: Re: [PATCH v16 00/23] NXP DPAA driver enhancements and fixes
> Importance: High
>
> On Mon, 28 Sep 2026 12:28:26 +0530
> Hemant Agrawal <hemant.agrawal at nxp.com> wrote:
>
> > This series collects a set of fixes and enhancements for the NXP DPAA
> > bus, mempool, dma, crypto and net drivers targeting 26.11.
> >
> > It includes memory-leak and resource-cleanup fixes on the device
> > remove/close paths, more robust frame queue and congestion-group
> > shutdown, secondary-process safety guards, BPID and cgrid lifecycle
> >
> > It includes memory-leak and resource-cleanup fixes on the device
> > remove/close paths, more robust frame queue and congestion-group
> > shutdown, secondary-process safety guards, BPID and cgrid lifecycle
> > handling, and several new features: offline (O/H) port device support,
> > enhanced virtual storage profile (VSP) port support, fmcless Rx queue
> > configuration via devargs, Rx/Tx taildrop threshold devargs, non
> > fmX-macY shared Ethernet naming, and DMA scatter-gather and
> > errata-workaround devargs. Documentation and release notes are updated
> > accordingly.
> >
> > v16:
> > * rebased on main
>
> More AI review feedback.
>
> Subject: Re: [PATCH v16 00/23] NXP DPAA driver updates
>
> v16 applies cleanly to main (edd57b2).  Every commit builds with
> -Dwerror=true: gcc 13.3 on x86, covering bus/dpaa, common/dpaax,
> mempool/dpaa, net/dpaa, crypto/dpaa_sec, dma/dpaa and event/dpaa.
> dma/dpaa was also built with RTE_DMA_DPAA_ERRATA_ERR050757 defined.
> Not built for arm64.
>
> The v14 errors are fixed: bman_release_fast() no longer publishes stack bytes,
> and dpaa_ol_remove() no longer double frees dev_private.
> Also resolved: fmbm_tfrc is exposed and the BMI static_assert is exact, the
> unused VSP fields are gone, dma/dpaa rejects secondary probe, and
> check_fd() is race free.  The taildrop getenv() fallback is now justified in the
> commit message; dropping that item.
>
> What remains blocking: the O/H port in patch 21 cannot be enabled or
> configured as submitted, and the new mempool destructor in patch 13 frees
> EAL memory at process exit.
>
> Errors
> ------
>
> Patch 13/23: drivers: release DPAA bpid on driver destructor
>
>   dpaa_mpool_finish() is an RTE_FINI destructor that does
>
>       rte_free(rte_dpaa_bpid_info);
>
>   The commit message says the destructor must not touch bman_pool
>   because EAL memory may already be gone, and then frees EAL memory.
>   rte_eal_cleanup() calls rte_eal_memory_detach() before returning
>   ("after this point, any DPDK pointers will become dangling").  Any
>   application that calls rte_eal_cleanup() before exit, testpmd
>   included, will rte_free() unmapped memory in this destructor.
>
>   It is also wrong in a secondary.  There rte_dpaa_bpid_info is not a
>   local allocation; the Rx path installs the primary's shared array:
>
>       rte_dpaa_bpid_info = fq->bp_array;   /* dpaa_rxtx.c:924, :1449 */
>
>   A secondary that exits frees the primary's live BPID table.
>
>   Releasing the BPIDs from the static flag table is fine.  Drop the
>   rte_free() from the destructor.
>
dropped `rte_free(rte_dpaa_bpid_info)` from the RTE_FINI destructor. Right on both counts - `rte_eal_cleanup()` detaches EAL memory before destructors run, and in a secondary the array is the primary's (installed from `fq->bp_array`). BPID release from the static flag table is kept.

> Patch 21/23: drivers: add offline (O/H) port device support
>
>   drv_oldev can never be enabled.  dpaa_bus_parse_bus_args() does
>
>       RTE_EAL_DEVARGS_FOREACH("dpaa", devargs)
>
>   but the bus is registered as RTE_REGISTER_BUS(dpaa_bus, ...), so its
>   name is "dpaa_bus".  rte_devargs_next() compares against
>   da->bus->name, never matches, and oldev_enabled stays 0.  The dpaa.rst
>   example "bus=dpaa,drv_oldev=1" is worse: rte_devargs_layers_parse()
>   fails with "Could not find bus" and EAL init aborts.
>
>   Even with that fixed, drv_bh_port cannot reach the device.  The OL
>   block in dpaa_create_device_list() never sets dev->device.devargs,
>   unlike the ETH, SEC and QDMA blocks, which use
>   rte_bus_find_devargs().  A lookup would not match anyway:
>   rte_dpaa_bus_parse() only accepts fmX-macY, fmX-ohY, fmX-onicY and
>   dpaa_sec-N, and the device is named "oldev1".  So
>   dpaa_ol_get_bh_port_name() always returns 0, and
>   dpaa_ol_tx_queue_setup() fails with -EINVAL once Rx is set up.
>
>   This needs to be tested end to end.

`drv_oldev` is now usable - bus devargs matched against `rte_dpaa_bus.name` ("dpaa_bus"), OL device gets devargs via `rte_bus_find_devargs()`, `rte_dpaa_bus_parse()` accepts `oldevN`, dpaa.rst examples corrected.

>
> Warnings
> --------
>
> Patch 05/23: bus/dpaa: scan max BPID from DTS
>
>   bman_pool_max = start + count is taken from fsl,bpid-range without a
>   bound.  bman_create_portal() does
>
>       u8 bpid = 0;
>       while (bpid < bman_pool_max) {
>               bm_isr_bscn_mask(p, bpid, 0);
>               bpid++;
>       }
>
>   A value above 255 never terminates.  A value above 64 writes past
>   BM_REG_SCN(1); bm_isr_bscn_mask() only covers bpid 0..63, and struct
>   bman_depletion is 64 bits.  Clamp to 64 and reject count == 0.
>
Fixed

> Patch 08/23: drivers: add DPAA cgrid cleanup support
>
>   The CGR created for a queue whose FQ init fails is still leaked.
>   dpaa_rx_queue_init() and dpaa_tx_queue_init() call qman_create_cgr()
>   and then qman_init_fq().  If qman_init_fq() fails they return an
>   error, and nb_rx_cgr / nb_tx_cgr is only incremented on success.
>   The error path then skips qman_delete_cgr() for that entry, and
>   rte_free(cgr_rx) leaves it linked on cgr_cbs.  That is the same
>   dangling pointer this patch sets out to fix.  Count the CGR when
>   qman_create_cgr() succeeds, or delete it in the helper on failure.
>
>   qman_pending_fq_by_cgrid() returns -ERANGE ("none left") whenever the
>   CGR byte count is zero.  A scheduled FQ with CGE set and a matching
>   cgid is a member whether or not it holds frames.  So an empty stale
>   FQ survives, and its CGRID is deleted and released while still
>   referenced, contrary to the rule stated in the commit message.
>   Close is not a fast path; scan unconditionally there.
>
fixed

> Patch 09/23: bus/dpaa: improve FQ shutdown with channel validation
>
>   The commit message describes things not in the diff.  The
>   pool-channel-range parsing is added here, not "in an earlier patch".
>   Nothing here cleans up CGRID or other queue parameters on shutdown.
>   The base code has no portal affinity check for pool channels to
>   remove.
>
>   The FQRN drain loop is unbounded:
>
>       do {
>               qm_dqrr_drain_nomatch(&p->p);
>               found_fqrn = qm_mr_drain(&p->p, FQRN);
>               cpu_relax();
>       } while (!found_fqrn);
>
>   With QM_SDQCR_CHANNELS_DEDICATED the portal only dequeues its own
>   channel.  If the FQ is scheduled on another portal's dedicated
>   channel, FQRN never arrives and this spins forever.  That is
>   pre-existing, but patch 08 now calls qman_shutdown_fq_by_fqid()
>   (qp == NULL, so the caller's affine portal) on stale FQs left by a
>   previous run.  Push-mode Rx queues are exactly FQs on another
>   portal's dedicated channel.  Return -EBUSY when a dedicated channel
>   is not p->config->channel, or bound the loop.
>
fixed
> Patch 14/23: dma/dpaa: add devargs for SG and errata workaround
>
>   RTE_PMD_REGISTER_PARAM_STRING() is invoked with an #ifdef inside its
>   argument list.  Directives inside macro arguments are undefined
>   behaviour (C11 6.10.3p11).  GCC accepts it; other compilers need
>   not.  Build the string with a separate macro defined under the
>   #ifdef.
>
Fixed

> Patch 20/23: net/dpaa: enhance VSP port support
>
>   dpaa_eth_rx_queue_setup() now writes
>
>       dpaa_intf->vsp[vsp_id].vsp_bp[0] =
> DPAA_MEMPOOL_TO_POOL_INFO(mp);
>
>   before any bound check.  dpaa_port_vsp_update() then reads
>   vsp[vsp_id].vsp_handle; only dpaa_port_vsp_configure() checks
>   vsp_id < DPAA_VSP_PROFILE_MAX_NUM.  vsp_id comes from the FMC
>   direct_relative_profile_id.  The check in update was also loosened
>   from vsp_id >= num_profiles to vsp_id >= base_profile_id +
>   num_profiles, both of which come from the DTS vsp-window.  Check
>   against DPAA_VSP_PROFILE_MAX_NUM before first use.
>
>   The commit message covers the ONIC port type and the cleanup
>   signature.  It does not mention several other changes:
>
>   - struct dpaa_if_vsp replaces the per-profile arrays.
>   - dpaa_eth_dev_stop() changes for ONIC.
>   - dpaa_port_vsp_update() now returns -EINVAL where it returned 0.
>   - The same-bpid shortcut is removed, so every Rx queue setup frees
>     and reconfigures the VSP.

fixed
>
> Patch 21/23: drivers: add offline (O/H) port device support
>
>   dpaa_ol_tx_queue_setup() silently skips ASK_CTRL_SET_DPDK_INFO when
>   Rx has not been set up yet:
>
>       if (dpaa_intf->bp_info == NULL || dpaa_intf->rx_queues == NULL)
>               return 0;
>
>   testpmd start_port() sets up Tx queues before Rx, so on first start
>   the kernel is never told the FQs.  ethdev does not order queue
>   setup.  Issue the ioctl from dev_start, where both are known.
>
>   dpaa_ol_probe() has no process-type handling.  A secondary runs
>   dpaa_oldev_init(), creating dynamic FQs and issuing ioctls against
>   state the primary owns.  Either attach with
>   rte_eth_dev_attach_secondary() or reject the secondary as patch 06
>   does for dma/dpaa.  As written, the secondary branch in
>   dpaa_ol_remove() is unreachable.
>
fixed
> Info
> ----
>
> Patch 05/23: "if (!bman_ip_rev)" is dead now that the else branch
>   sets BMAN_REV21.
>
Fixed

> Patch 07/23: The stated motivation is push-mode Rx queue shutdown, but
>   nothing in the series shuts down a net/dpaa Rx queue by descriptor.
>   The only callers of qman_shutdown_fq(fq) are dpaa_sec and oldev.
>
> Patch 08/23: __rte_internal appears twice on
>   qman_pending_fq_by_cgrid().  RTE_EXPORT_INTERNAL_SYMBOL(
>   qman_release_fqid_range) is added here, but its first user outside
>   the bus is patch 22.
>
> Patch 12/23: dpaa_port_fmc_port_parse() parses the name before the
>   OH check, which does not use the result.  It logs DPAA_PMD_ERR for
>   any port name without "MAC/" or "OFFLINE/", although the caller
>   treats that as "not this port".  It also logs DPAA_PMD_INFO for
>   every port on every probe.  Both should be DEBUG.
>
> Patch 13/23: RTE_PRIORITY_104 is a driver-local define in the RTE_
>   namespace.
>
> Patch 21/23:
>   - rte_pmd_dpaa_ol_*() and ask_set_fq_info() return the raw ioctl()
>     result (-1) on failure, mixed with -ENODEV from check_fd().
>     Return -errno.
>   - rte_dpaa_bus_oldev_enabled() is exported and has no users.
>   - dpaa_ol_devops can be const.
>   - The meson.build copyright changes to 2018-2021 in a 2026 patch.
>   - rte_pmd_dpaa_oldev.h is not in doc/api/doxy-api-index.md and has
>     no extern "C" guard.
>
> Patch 23/23:
>   - The FMCLESS default Rx queue count change in patch 16
>     (rte_lcore_count() to DPAA_MAX_NUM_PCD_QUEUES) is user visible and
>     not in the release notes.
>   - The new rte_pmd_dpaa_ol_* API is not listed.
>   - Five top-level entries for one driver family; fold them into one
>     "Updated NXP DPAA drivers" entry with sub-bullets.
>
> Pre-existing, not introduced here: in dpaa_dev_init() the "FMC initializes
> failed" path does goto free_rx with ret still 0, so probe succeeds with no Rx
> queues.