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

[Stephen Hemminger <stephen@networkplumber.org>]

On Mon, 28 Sep 2026 12:28:26 +0530
Hemant Agrawal <hemant.agrawal@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.

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.

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.

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.

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.

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.

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.

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.

Info
----

Patch 05/23: "if (!bman_ip_rev)" is dead now that the else branch
  sets BMAN_REV21.

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.