Re: [PATCH v1 00/17] net/dpaa: bug fixes for bus, net and fmlib drivers

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu, 18 Jun 2026 19:41:34 +0530
Hemant Agrawal <hemant.agrawal@nxp.com> wrote:

> This series contains bug fixes for the DPAA PMD (bus/dpaa, net/dpaa,
> net/dpaa/fmlib and dma/dpaa):
> 
> - Fix error handling in qman_create_fq and qman_query
> - Fix fqid endianness in qman_fq_flow_control
> - Fix CGR index usage in dpaa_modify_cgr
> - Add null check in fmlib scheme delete
> - Fix BMI RX stats register offset
> - Fix file descriptor leak after CCSR mmap
> - Fix device probe regression on LS1043A
> - Fix double-close in device remove path
> - Fix incorrect condition in interrupt unregister
> - Fix Coverity-reported issues in dpaa_flow and dpaa_qdma
> - Fix xstat name for tx undersized counter
> - Fix xstat string typos in BMI stats table
> - Remove duplicate ptype entries
> - Fix wrong buffer in xstats get by id
> - Fix null l3_len check in checksum offload
> - Fix mbuf leak in SG fd creation
> 
> All patches are bug fixes tagged with Fixes: and Cc: stable@dpdk.org.
> 
> Gagandeep Singh (3):
>   bus/dpaa: fix device probe issue
>   net/dpaa: fix device remove
>   net/dpaa: fix invalid check on interrupt unregister
> 
> Hemant Agrawal (11):
>   bus/dpaa: fix error handling of qman_create_fq
>   bus/dpaa: fix fqid endianness
>   bus/dpaa: fix error handling in qman_query
>   net/dpaa: fix modify cgr to use index
>   bus/dpaa: fix fd leak for ccsr mmap
>   net/dpaa: fix xstat name for tx undersized counter
>   net/dpaa: fix xstat string typos in BMI stats table
>   net/dpaa: remove duplicate ptype entries
>   net/dpaa: fix wrong buffer in xstats get by id
>   net/dpaa: fix null l3_len check in checksum offload
>   net/dpaa: fix mbuf leak in SG fd creation
> 
> Jun Yang (1):
>   bus/dpaa: fix BMI RX stats register offset
> 
> Prashant Gupta (1):
>   net/dpaa/fmlib: add null check in scheme delete
> 
> Vanshika Shukla (1):
>   net/dpaa: fix coverity reported issues
> 
>  drivers/bus/dpaa/base/qbman/bman_driver.c |  3 ++-
>  drivers/bus/dpaa/base/qbman/qman.c        | 11 +++++----
>  drivers/bus/dpaa/base/qbman/qman_driver.c |  6 ++---
>  drivers/bus/dpaa/dpaa_bus.c               |  6 ++---
>  drivers/bus/dpaa/include/fman.h           |  6 ++---
>  drivers/dma/dpaa/dpaa_qdma.c              |  7 +++++-
>  drivers/net/dpaa/dpaa_ethdev.c            | 27 +++++++++++------------
>  drivers/net/dpaa/dpaa_flow.c              |  4 ++++
>  drivers/net/dpaa/dpaa_rxtx.c              |  3 +++
>  drivers/net/dpaa/fmlib/fm_lib.c           |  3 +++
>  10 files changed, 46 insertions(+), 30 deletions(-)
> 

Looks good but there are some warnings from more detailed AI review that
need addressing.

Review of [PATCH v1 00/17] bus/dpaa, net/dpaa fixes

Reviewed against current main. No errors found; the series is a
solid set of bug fixes. A few warnings and notes below, mostly
about undocumented side effects and Fixes: accuracy. Patches not
listed (01, 02, 03, 04, 06, 07, 12, 14, 15, 16, 17) look correct.


[PATCH v1 05/17] net/dpaa/fmlib: add null check in scheme delete

Info: the commit body refers to FM_PCD_MatchTableSchemeDelete(),
but the function actually changed is fm_pcd_kg_scheme_delete().
The NULL guard and E_NO_DEVICE return are correct and match the
sibling functions in fm_lib.c; only the message names the wrong
API.


[PATCH v1 08/17] bus/dpaa: fix device probe issue

The early "return 0" was indeed skipping device-list creation, so
removing it fixes probe on LS1043A. Good.

Warning: that early return did two things -- it forced
max_push_rxq_num = 0 AND skipped the DPAA_PUSH_QUEUES_NUMBER
override that follows. With the return gone, execution now falls
through to:

	penv = getenv("DPAA_PUSH_QUEUES_NUMBER");
	if (penv)
		dpaa_bus.max_push_rxq_num = atoi(penv);

so on LS1043A the env var can now raise the push-queue count back
above zero, which the original code deliberately prevented (the
comment is "Disabling the default push mode for LS1043A", and the
SoC has the FMAN push-mode errata handled in dpaa_rxtx.c). If
LS1043A must keep push mode disabled regardless of the env var,
guard the override, e.g.:

	if (dpaa_bus.svr_ver == SVR_LS1043A_FAMILY) {
		dpaa_bus.max_push_rxq_num = 0;
	} else {
		penv = getenv("DPAA_PUSH_QUEUES_NUMBER");
		...
	}

If the override is intended to apply to LS1043A, please say so in
the commit message.


[PATCH v1 09/17] net/dpaa: fix device remove

The RTE_ETH_DEV_UNUSED guard against double close/release is
correct, and "int ret = 0" is now required because the assignment
is conditional.

Warning: the patch also drops the dpaa_finish() call, which the
commit message does not mention. dpaa_finish() is registered as a
destructor (RTE_FINI_PRIO(dpaa_finish, 103)), so it still runs at
process exit, but the previous explicit call ran at last-device
remove (!dpaa_valid_dev). Removing it moves the global teardown
(dpaa_fm_term, per-queue portal close, is_global_init = 0) from
last-remove time to exit time. For run-then-exit this is
equivalent, but for remove-all-then-continue (e.g. re-probe in a
running process) is_global_init now stays 1 and portals stay open
until exit. Please call this change out in the commit message and
confirm re-probe still behaves.


[PATCH v1 10/17] net/dpaa: fix invalid check on interrupt unregister

The fix is correct: rte_intr_callback_unregister() returns the
number of callbacks removed (>=1) on success and a negative value
on failure, so "if (ret)" logged a spurious warning on every
successful unregister; "if (ret < 0)" is right.

Warning: the Fixes: tag points at

	9c99878aa1 ("log: introduce logtype register macro")

which is unrelated to interrupt unregistration and looks like a
copy/paste error. An incorrect Fixes: will misdirect the stable
backport -- please point it at the commit that introduced the
"if (ret)" check.


[PATCH v1 11/17] net/dpaa: fix coverity reported issues

Both fixes are correct. The dpaa_qdma.c bound (num == 0 ||
num > FSL_QDMA_SG_MAX_ENTRY) prevents the desc_ssge[num - 1]
underflow when pending_num is 0. The dpaa_flow.c port_handle
close fixes the continue-path leak without introducing a
double-close: the success path nulls port_handle inside
dpaa_fm_deconfig(), and the error/continue paths legitimately
leave it set for the new guard to close.

Info: this bundles two unrelated Coverity fixes across
drivers/dma/dpaa and drivers/net/dpaa under a single net/dpaa
Fixes: tag (e7665de896). The qdma OOB almost certainly has a
different origin commit. Splitting into two patches (or at least
carrying the correct Fixes: for the qdma change) would make the
stable backport cleaner.


[PATCH v1 13/17] net/dpaa: fix xstat string typos in BMI stats table

Both typo fixes are correct.

Info: the very next entry on the same table is also misspelled --
"rx_buf_diallocate" (fmbm_rbdc, "Rx Buffers Deallocate Counter")
should be "rx_buf_deallocate". Worth fixing in the same pass since
this patch is specifically cleaning up these names.