Re: [PATCH v6 00/19] DPAA bus/net/mempool/DMA driver fixes and improvements

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu,  2 Jul 2026 11:03:40 +0530
Hemant Agrawal <hemant.agrawal@nxp.com> wrote:

> This series collects a set of correctness fixes, cleanups and feature
> additions across the NXP DPAA bus, net, mempool and DMA drivers.
> 
> 1. Bus/fman infrastructure cleanups (patches 01, 02, 13)
>    - bus/dpaa: refine fman naming and fix global scope
>    - bus/dpaa: scan max BPID from DTS
>    - bus/dpaa: improve log macro and fix bus detection
> 
> 2. Statistics (patch 03)
>    - net/dpaa: add BMI Tx statistics
> 
> 3. Process-type guards (patch 04)
>    - dpaa: add process-type guards to prevent segfaults in secondary
> 
> 4. FQ shutdown hardening (patches 05-11)
>    - bus/dpaa: define helpers for qman channel and wq
>    - drivers: shutdown DPAA FQ by fq descriptor
>    - bus/dpaa: improve FQ shutdown with channel validation
>    - bus/dpaa: enhance DPAA FQ shutdown
>    - drivers: add DPAA cgrid cleanup support
>    - net/dpaa: clean Tx confirmation FQ on device stop
>    - net/dpaa: remove redundant FQ shutdown from Rx queue setup
> 
> 5. net/dpaa improvements (patches 12, 14, 15)
>    - net/dpaa: optimize FM deconfig
>    - net/dpaa: optimize FMC MAC type parsing
>    - net/dpaa: report error on using deferred start
> 
> 6. mempool/dpaa (patches 16-17)
>    - drivers: optimize DPAA multi-entry buffer pool operations
>    - drivers: release DPAA bpid on driver destructor
> 
> 7. dma/dpaa (patch 18)
>    - dma/dpaa: add SG data validation and ERR050757 fix
> 
> 8. net/dpaa ONIC port support (patch 19)
>    - net/dpaa: add ONIC port checks
> 
> v6 changes:
> - Fix bman_release_fast() in patch 16: replace rte_memcpy() on the
>   small bm_bufs[] stack array with memcpy(); the SIMD implementation
>   of rte_memcpy reads in 32/64-byte chunks and triggers
>   -Warray-bounds with GCC 15 / ASAN builds on the 64-byte array.
>   memcpy() is correct here as this is a local stack buffer with no
>   DMA or multi-process constraints.
> 
> v5 changes:
> - Rebased onto current upstream main; resolved conflict in
>   drivers/crypto/dpaa_sec/dpaa_sec.c where upstream removed the
>   cryptodev_name[] local variable and snprintf() call in
>   cryptodev_dpaa_sec_probe() -- patch 04 now uses dpaa_dev->name
>   directly in rte_cryptodev_pmd_create().
> - Resolved conflict in drivers/net/dpaa/dpaa_ethdev.c where upstream
>   removed rte_dpaa_device.eth_dev -- patch 04 now uses
>   rte_eth_dev_allocated() to look up the Ethernet device by name.
> 
> v4 changes:
> - Fix dpaa_bus_dev_compare() to return the strncmp result (previously
>   always returned 0, breaking device matching).
> - Remove the dead get_tx_port_type() function that triggered a clang
>   -Wunused-function CI failure.
> - Guard all dpaa_fm_deconfig() call sites against NULL port_handle to
>   prevent a NULL dereference on partially initialised interfaces.
> - Move the penv variable declaration in dpaa_qdma_init() to the point of
>   use (C99 inline), fixing a spurious -Wunused-variable warning during
>   bisect of earlier patches in the series.
> 
> Gagandeep Singh (2):
>   bus/dpaa: enhance DPAA FQ shutdown
>   dma/dpaa: add SG data validation and ERR050757 fix
> 
> Hemant Agrawal (5):
>   net/dpaa: clean Tx confirmation FQ on device stop
>   net/dpaa: remove redundant FQ shutdown from Rx queue setup
>   net/dpaa: optimize FM deconfig
>   bus/dpaa: improve log macro and fix bus detection
>   net/dpaa: report error on using deferred start
> 
> Jun Yang (10):
>   bus/dpaa: refine fman naming and fix global scope
>   bus/dpaa: scan max BPID from DTS
>   drivers: add BMI Tx statistics
>   bus/dpaa: define helpers for qman channel and wq
>   drivers: shutdown DPAA FQ by fq descriptor
>   bus/dpaa: improve FQ shutdown with channel validation
>   drivers: add DPAA cgrid cleanup support
>   net/dpaa: optimize FMC MAC type parsing
>   drivers: optimize DPAA multi-entry buffer pool operations
>   drivers: release DPAA bpid on driver destructor
> 
> Prashant Gupta (1):
>   drivers: add process-type guards for secondary process
> 
> Vanshika Shukla (1):
>   net/dpaa: add ONIC port checks
> 
>  drivers/bus/dpaa/base/fman/fman.c         |  23 ++--
>  drivers/bus/dpaa/base/fman/fman_hw.c      | 108 +++++++++----------
>  drivers/bus/dpaa/base/qbman/bman.c        |  59 ++++------
>  drivers/bus/dpaa/base/qbman/bman_driver.c |  48 ++++++---
>  drivers/bus/dpaa/base/qbman/qman.c        | 115 +++++++++++---------
>  drivers/bus/dpaa/base/qbman/qman.h        |  23 +++-
>  drivers/bus/dpaa/base/qbman/qman_driver.c |  29 ++++-
>  drivers/bus/dpaa/dpaa_bus.c               |  33 ++++--
>  drivers/bus/dpaa/dpaa_bus_base_symbols.c  |   4 +
>  drivers/bus/dpaa/include/fman.h           |  30 +++++-
>  drivers/bus/dpaa/include/fsl_bman.h       |  49 +++++++--
>  drivers/bus/dpaa/include/fsl_qman.h       |  22 +++-
>  drivers/crypto/dpaa_sec/dpaa_sec.c        |   3 -
>  drivers/dma/dpaa/dpaa_qdma.c              | 102 +++++++++++++-----
>  drivers/mempool/dpaa/dpaa_mempool.c       |  75 +++++++++++--
>  drivers/mempool/dpaa/dpaa_mempool.h       |   3 +-
>  drivers/net/dpaa/dpaa_ethdev.c            | 122 ++++++++++++++++++----
>  drivers/net/dpaa/dpaa_ethdev.h            |  22 +++-
>  drivers/net/dpaa/dpaa_flow.c              | 120 +++++++++++----------
>  drivers/net/dpaa/dpaa_flow.h              |   7 +-
>  drivers/net/dpaa/dpaa_fmc.c               |  73 ++++++++-----
>  21 files changed, 725 insertions(+), 345 deletions(-)
> 

Still see lots of things that need fixing in detailed AI review:

Reviewed the v6 DPAA series (19 patches) against current main: applied
cleanly with git am and traced the affected paths in
drivers/{bus,net,mempool,dma}/dpaa.  Three of the errors from the v3
review are still open, and the v3 fix for patch 04 introduced a new
bisect break.  Details below, by patch.  No Reviewed-by this round.

Resolved since v3: the deconfig NULL deref (old 12/19 interaction).
dpaa_fm_deconfig() now NULLs dpaa_intf->port_handle at the end, and all
three close-path callers plus the two dpaa_flow.c sites guard on
port_handle first.  The double-deconfig/NULL-deref can no longer happen.

Patch 03: drivers: add BMI Tx statistics
  Error: the Tx BMI counters are still not read from Tx registers, and
  the xstats loop miscounts.  fman_if_bmi_stats_get_all() and _reset()
  only touch rx_bmi_map; the new tx_bmi_regs fields and
  FMAN_IF_BMI_TX_STAT_OFFSET_* macros are never read by anything.
  In dpaa_dev_xstats_get() the string table is now 13 standard + 8 Rx
  BMI + 4 Tx BMI = 25, but bmi_count is still
  sizeof(dpaa_if_rx_bmi_stats)/4 == 8, so the split point
  (num - (bmi_count - 1) == 18) treats the first 5 Rx-BMI strings as
  standard stats (read with the wrong offsets into the dpaa_if_stats
  array), and the second loop fills the 4 Tx entries from Rx register
  values.  The counters reported for the tx_* names are Rx data.
  Either wire get_all/reset to also read the tx_bmi_map range
  (FMAN_IF_BMI_TX_STAT_OFFSET_START..END) and fix the standard/Rx/Tx
  split, or drop the Tx entries from this patch.

Patch 04: drivers: add process-type guards for secondary process
  Error: this now fails to build standalone, breaking bisect.  The v3
  fix moved the getenv() block up into dpaa_qdma_init(), but the tunable
  declarations were left in patch 18.  After this patch,
  s_data_validation, s_sg_enable and s_pci_read are used but never
  declared (they are added as static int in patch 18/f4f9e13), so the
  commit does not compile.  The getenv/SG-validation tunables belong in
  patch 18, not here; move the whole block (and s_hw_err_check = 1) into
  18 and leave this patch with only the RTE_PROC_PRIMARY guards.  That
  fixes the build and keeps the commit matching its subject.
  Info: the dpaa_sec hunk removes the RTE_PROC_PRIMARY early-return in
  cryptodev_dpaa_sec_probe().  That is correct (dpaa_sec_dev_init() has
  its own primary-only guard), but the commit message should say so.

Patch 13: bus/dpaa: improve log macro and fix bus detection
  Error: dpaa_bus_dev_compare() still carries probe side effects and
  breaks devargs matching.  It is the bus dev_compare callback; the EAL
  (rte_bus_find_devargs) calls it per devargs entry and treats a 0
  return as a match, returning the first entry that matches.  This
  version returns 0 (match) when the sysfs path is absent, and returns
  0 for every call once dpaa_bus.detected is set - so after the first
  device is detected, every name compares equal and the wrong devargs
  is bound.  It also creates the pthread key and, on failure, calls
  dpaa_clean_device_list() and returns the errno as if it were a
  comparison result, all from inside a comparator.
  On top of that, the detection block (sysfs check + detected guard +
  pthread_key_create) is now duplicated: it is still present in
  rte_dpaa_bus_scan() (unchanged) and copied into dev_compare, so this
  is not the "move" the commit message describes.  Keep dev_compare a
  pure comparator (parse + strncmp, no side effects) and leave detection
  where it already is in scan().  The DPAA_DEV_PATH1/2 #defines should
  also not live inside the function body.
  The DPAA_BUS_LOG -> DPAA_BUS_{INFO,ERR,WARN,DEBUG} conversion is fine.

Patch 18: dma/dpaa: add SG data validation and ERR050757 fix
  Warning: the five getenv() tunables (DPAA_QDMA_DATA_VALIDATION,
  DPAA_QDMA_HW_ERR_CHECK, DPAA_QDMA_SG_ENABLE, DPAA_QDMA_SG_MAX_ENTRY_SIZE,
  DPAA_QDMA_PCI_READ) should be devargs; the driver already uses devargs
  (DPAA_DMA_ERROR_CHECK).  checkpatch will also flag the getenv additions.
  (This is the block that must move back here from patch 04.)
  Info: s_hw_err_check changes from bool to int; it is a pure flag, keep
  bool.

Patch 08: bus/dpaa: enhance DPAA FQ shutdown
  Info: qman_find_fq_by_cgrid() still ends with a "do {...} while (1);"
  whose only exits are returns, followed by an unreachable
  "return -ENODEV;".  Drop the dead return.

Patch 17: drivers: release DPAA bpid on driver destructor
  Info: "#define RTE_PRIORITY_104 104" is still unused;
  RTE_FINI_PRIO(dpaa_mpool_finish, 104) uses the literal.  Use the macro
  or drop it.

Patch 19: net/dpaa: add ONIC port checks
  Info: the subject undersells the change.  Besides the fman_onic
  guards, this reworks the VSP path from the flat vsp_bpid[] array to a
  vsp[].vsp_bp[]/bp_num structure and changes the dpaa_port_vsp_cleanup()
  and dpaa_port_vsp_update() signatures.  Consider splitting the VSP
  refactor out, or at least describe it in the commit message.

No Reviewed-by: patches 03, 04 and 13 are functional/build errors that
need a respin.  04 and 18 should be fixed together (move the tunables);
08, 17 and 19 can ride along.