Re: [PATCH v21 00/27] NXP DPAA driver enhancements and fixes

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue,  6 Oct 2026 14:56:36 +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
> 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.
> 
> v21:

More detailed AI review shows some outstanding issues:

Applies cleanly to main (49bb9a5).  HEAD and each of the 27 commits
build with -Dwerror=true (gcc 13.3, x86) for bus/dpaa, common/dpaax,
mempool/dpaa, net/dpaa, crypto/dpaa_sec, dma/dpaa and event/dpaa.
Not built for arm64.  All Fixes tags resolve to the commits they
name; v20 02/27 had a nonexistent hash, now corrected.

The v20 items are addressed.  The CGR delete on a never-created CGR
in 11/27 is gone, the 04/27 Tx gotos are fixed, and the commit
messages of 02, 10, 12, 18 and 22 now match their diffs.  The 19/20
devargs are range checked before narrowing, the unused defines and
macros in 15 and 22 are dropped, and 25/27 saves errno and releases
its FQIDs.

Two new problems come with the fixes.  01/27's rewritten message
describes the wrong mechanism.  The FQID release in 25/27 runs even
when the FQ did not shut down; 26/27 has the same pattern.

Warning
-------

Patch 01/27: The new commit message says rte_eth_dev_release_port()
  calls the close op again through rte_eth_dev_destroy().  It does
  not.  rte_eth_dev_release_port() never calls dev_close, and
  rte_dpaa_remove() never calls rte_eth_dev_destroy().  The second
  close was the explicit second call in the old code:

	ret = dpaa_eth_dev_close(eth_dev);
	if (eth_dev->state !=  RTE_ETH_DEV_UNUSED) {
		dpaa_eth_dev_close(eth_dev);

  Describe that instead.

Patch 25/27: dpaa_oldev_queues_release() returns the FQID even when
  qman_shutdown_fq() failed:

	ret = qman_shutdown_fq(&dpaa_intf->tx_queues[i]);
	if (ret) {
		DPAA_PMD_WARN(...);
	}
	...
	qman_release_fqid(dpaa_intf->tx_queues[i].fqid);

  qman_shutdown_fq() returns -EBUSY when the retire does not
  complete.  It also returns -EBUSY when the FQ is scheduled on a
  DCP channel with frames still queued, which is where this Tx FQ
  points (ch_info.channel_id from the O/H port).  In that case the
  FQ is still live in QMan, and the next qman_alloc_fqid() can hand
  the same FQID to someone else.  Release the FQID only when the
  shutdown succeeded; on failure, leaking it is the safe choice.
  The Rx loop has the same issue.

Patch 26/27: Same problem in dpaa_sec_uninit().  Shutdown failures
  on outq[] and inq[] are logged, then the whole range is released:

  if (fqid)
	qman_release_fqid_range(fqid, internals->max_nb_queue_pairs);

  inq[] are QMAN_FQ_FLAG_TO_DCPORTAL queues towards CAAM, so the
  -EBUSY case applies here too.  Release per FQID, and only for the
  queues that shut down.

Info
----

Patch 11/27: If qman_create_cgr() fails, probe now fails, where it
  used to continue without tail drop.  That fixes the crash, but it
  is a behaviour change and belongs in the commit message.

Patch 17/27: The FINI now uses the literal 104, but the
  "#define RTE_PRIORITY_104 104" line is still there and has no
  user.  Drop it.

Patch 25/27: The FQIDs are now returned, but qman_create_fq() also
  takes an FQ lookup table entry on 64-bit builds.  Only
  qman_destroy_fq() clears it, and no DPAA driver calls that.  So
  each probe/close cycle still leaks one entry per FQ, out of a
  table of CONFIG_FSL_QMAN_FQ_LOOKUP_MAX (32K) entries.  net/dpaa
  and dpaa_sec have the same gap, so a bus/dpaa helper that releases
  both the FQID and the entry after a successful shutdown would fix
  all three.

Patch 27/27: The FMCLESS default Rx queue count change in 20/27
  (rte_lcore_count() to DPAA_MAX_NUM_PCD_QUEUES) is user visible and
  is still not in the release notes.

Pre-existing, not introduced here: in FMCLESS mode, dpaa_dev_init()
gets Rx FQIDs from qman_alloc_fqid_range(), but net/dpaa never
releases them, on close or on probe failure.