Hi Hemant, On Fri, Jul 3, 2026 at 7:21 PM Hemant Agrawal <[email protected]> wrote: > > > On 30-06-2026 20:13, Maxime Leroy wrote: > > rte_dpaa2_close_dpcon_device() called dpcon_close() but not > dpcon_disable(). close only releases the MC control session; it does not > disable the channel. Cosmetic: an idle DPCON generates nothing and every > consumer reprograms on setup, so disable before close only to return the > channel clean, as the symmetric counterpart of the dpcon_enable() done on > setup. > > Signed-off-by: Maxime Leroy <[email protected]> > --- > drivers/event/dpaa2/dpaa2_hw_dpcon.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/event/dpaa2/dpaa2_hw_dpcon.c > b/drivers/event/dpaa2/dpaa2_hw_dpcon.c > index ea5b0d4b85..f65a63a786 100644 > --- a/drivers/event/dpaa2/dpaa2_hw_dpcon.c > +++ b/drivers/event/dpaa2/dpaa2_hw_dpcon.c > @@ -128,6 +128,7 @@ rte_dpaa2_close_dpcon_device(int object_id) > > if (dpcon_dev) { > rte_dpaa2_free_dpcon_dev(dpcon_dev); > + dpcon_disable(&dpcon_dev->dpcon, CMD_PRI_LOW, dpcon_dev->token); > dpcon_close(&dpcon_dev->dpcon, CMD_PRI_LOW, dpcon_dev->token); > TAILQ_REMOVE(&dpcon_dev_list, dpcon_dev, next); > rte_free(dpcon_dev); > > The ordering here is wrong. rte_dpaa2_free_dpcon_dev(dpcon_dev) is called > before dpcon_disable(). Once the device is returned to the shared pool via > rte_dpaa2_free_dpcon_dev(), another thread can immediately re-allocate the > same dpcon_dev pointer. The subsequent dpcon_disable() call would then > quiesce a channel already in active use by the new owner — a use-after-free > on the pool object. > > The correct teardown sequence must be: > > dpcon_disable() — quiesce the hardware channel > dpcon_close() — release the MC control session > rte_dpaa2_free_dpcon_dev() — return to pool > > Please reorder accordingly: > > dpcon_disable(&dpcon_dev->dpcon, CMD_PRI_LOW, dpcon_dev->token); > > dpcon_close(&dpcon_dev->dpcon, CMD_PRI_LOW, dpcon_dev->token); > > TAILQ_REMOVE(&dpcon_dev_list, dpcon_dev, next); > > rte_dpaa2_free_dpcon_dev(dpcon_dev); > > rte_free(dpcon_dev); > > > On the ordering: there's no real race. This is the .close handler, run once at teardown from fslmc_vfio_close_group() on a single thread, not concurrent with the datapath, so nothing can re-acquire the node between free and close.
But the patch shouldn't exist at all. The pooled DPAA2 objects use a reprogram-on-borrow model: the free helpers (rte_dpaa2_free_dpcon_dev, rte_dpaa2_free_dpci_dev, dpaa2_free_dpbp_dev) only drop in_use, and nothing un-programs an object on return, the next borrower reprograms. The sibling DPCI close handler confirms it: it does dpci_close() with no dpci_disable(). Adding dpcon_disable() here makes DPCON the lone outlier for no benefit, an enabled channel returned to the pool is inert (FQ parked at unbind, enable is re-applied idempotently on the next borrow). So there's no reason to treat dpcon/CDAN differently. Dropping the patch. Thanks, Maxime
