Hi
NXP Confidential > -----Original Message----- > From: Stephen Hemminger <[email protected]> > Sent: Wednesday, November 4, 2026 4:59 PM > To: Gagandeep Singh <[email protected]> > Cc: [email protected]; Hemant Agrawal <[email protected]> > Subject: Re: [PATCH v3 0/4] DPAA driver changes > > On Tue, 3 Nov 2026 15:05:57 +0530 > Gagandeep Singh <[email protected]> wrote: > > > V3-Changes: > > - Updated rte_pmd_dpaa_oldev.h APIs as per DPDK compliance. > > - Added missing release not for OH port support. > > - Fixed dump_netcfg issue. > > - Added an API to release FQID. > > - moved the fman.h changes to first patch. > > > > V2-changes: > > - Fixed most of the AI comments. > > - added new patch: > > fix out-of-bounds read of mac_idx > > - removed drv_oldev, updated comments and notes. > > - fixes FQ shutdown > > - removed unwanted checks, updated open, updated snprintf. > > - updated commit messages. > > > > V1: > > - Adding OH port support. > > - Updates in shared ethernet code. > > - Congestion related code cleanup. > > > > Gagandeep Singh (2): > > bus/dpaa: fix out-of-bounds read of mac_idx array > > drivers: add offline (O/H) port device support > > > > Hemant Agrawal (1): > > net/dpaa: support non fmX-macY type of shared Ethernet name > > > > Jun Yang (1): > > drivers: add DPAA cgrid cleanup support > > > > doc/api/doxy-api-index.md | 1 + > > doc/guides/nics/dpaa.rst | 36 + > > doc/guides/rel_notes/release_26_11.rst | 10 + > > drivers/bus/dpaa/base/fman/fman.c | 19 + > > drivers/bus/dpaa/base/qbman/qman.c | 61 ++ > > drivers/bus/dpaa/bus_dpaa_driver.h | 4 +- > > drivers/bus/dpaa/dpaa_bus.c | 68 +- > > drivers/bus/dpaa/dpaa_bus_base_symbols.c | 2 + > > drivers/bus/dpaa/include/fman.h | 6 +- > > drivers/bus/dpaa/include/fsl_qman.h | 28 + > > drivers/net/dpaa/dpaa_ethdev.c | 235 +++++- > > drivers/net/dpaa/dpaa_ethdev.h | 12 + > > drivers/net/dpaa/dpaa_flow.c | 5 +- > > drivers/net/dpaa/dpaa_oldev.c | 938 +++++++++++++++++++++++ > > drivers/net/dpaa/meson.build | 11 +- > > drivers/net/dpaa/rte_pmd_dpaa_oldev.h | 198 +++++ > > 16 files changed, 1603 insertions(+), 31 deletions(-) create mode > > 100644 drivers/net/dpaa/dpaa_oldev.c create mode 100644 > > drivers/net/dpaa/rte_pmd_dpaa_oldev.h > > > > Still see AI reported errors > > Review of [PATCH v3 0/4] DPAA offline port, shared MAC, cgrid cleanup > > Applied to next-net for-main (3b51e2e2f0). Each commit builds with - > Dwerror=true. > > Series > ------ > > The rte_pmd_dpaa_ol_* functions now take a port_id, but it is only used to > look > up which fd the ioctl goes to. The classification and LGW payloads carry > nothing > from the port, and the kernel module consumes them. My earlier objection > stands: this is configuration API for the out-of-tree cdx driver and does not > belong > in DPDK. > you're right that the new APIs don't interpret or act on their payloads in DPDK — the structs are serialized and handed to the ASK/CDX kernel driver via ioctl, and all the real logic (uplink classification, LGW subnet programming) lives in that out-of-tree module. No argument there. The reason we think this still belongs with the offline-port driver rather than being left entirely to the application is the "fd" and lifetime binding, not the payload semantics: - The ioctl must go out on `dpaa_if::ol_fd`, which is private port state opened on first use and closed in `dev_close()`. There is no public DPDK handle to that fd today, and we don't want to add one — that would mean exposing a raw file descriptor as public API, which is worse than what we have. - If this configuration path is pushed out of DPDK, the application needs its own separate way to resolve "which `/dev/cdx_ctrl` instance backhaul port corresponds to this `oldev1` port", duplicating the resolution `dpaa_ol_get_bh_port_name()` already does at probe time. That logic only exists on the DPDK side. > > Patch 2/4 drivers: add offline (O/H) port device support > -------------------------------------------------------- > > Error > > dpaa_oldev_init() opens /dev/cdx_ctrl through check_fd() in > dpaa_ol_tx_queue_init(). If a later step fails, for example the mac_addrs > allocation, the free_rx path does not close the fd: > > free_rx: > dpaa_oldev_queues_release(dpaa_intf); > return ret; > > dpaa_ol_probe() then calls rte_eth_dev_release_port(), which frees dev_private > and ol_fd with it. Close it on the error path: > > free_rx: > dpaa_oldev_queues_release(dpaa_intf); > if (dpaa_intf->ol_fd >= 0) { > close(dpaa_intf->ol_fd); > dpaa_intf->ol_fd = -1; > } > return ret; > > Warning > > dpaa_oldev_queues_release() sends ASK_CTRL_RESET_DPDK_INFO and releases > the FQIDs only if that succeeds. It does this even when > ASK_CTRL_SET_DPDK_INFO was never issued: on the probe error path, and on > close of a port that was never started. In those cases the kernel never had > the > FQIDs, so there is nothing to reset. If /dev/cdx_ctrl is missing, probe fails > in > dpaa_ol_tx_queue_init(), the reset fails with -ENODEV, and the Rx FQID > allocated > with QMAN_FQ_FLAG_DYNAMIC_FQID leaks on every attempt. Track whether > the kernel holds the FQIDs: > > /* dpaa_ol_set_fq_info(), after ask_set_fq_info() succeeds */ > dpaa_intf->ol_fq_info_set = true; > > /* dpaa_oldev_queues_release() */ > release_fqids = true; > if (dpaa_intf->ol_fq_info_set) { > if (ask_reset_fq_info(dpaa_intf, &fq_info) != 0) > release_fqids = false; > else > dpaa_intf->ol_fq_info_set = false; > } > > Info > > v3 replaced the atomic ask_fd with a plain dpaa_intf->ol_fd, but kept the > double- > checked lock in check_fd(). The unlocked read of ol_fd now races with the > locked > write if control calls can be concurrent. If they cannot, the mutex is not > needed. > The fd is always opened during probe by dpaa_ol_tx_queue_init(), so open it > once > in dpaa_oldev_init() and drop fd_init_lock and the lazy open. > > dpaa_ol_set_fq_info() copies bh_port_name with rte_memcpy(); this is control > path, use memcpy(). > > The blank line added after "/* Rx Statistics Counter */" in struct > dpaa_if_stats is > unrelated.
