RE: [PATCH v4 1/4] net/dpaa2: fix duplicate calling of dpaa2 dev close

[Hemant Agrawal <hemant.agrawal@nxp.com>]

Hi Maxime,

> -----Original Message-----
> From: Maxime Leroy <maxime@leroys.fr>
> Hi Hemant,
> 
> Le jeu. 13 nov. 2025 à 12:44, Hemant Agrawal <hemant.agrawal@nxp.com> a
> écrit :
> >
> > When rte_eth_dev_close() is called, it performs the following actions:
> >
> > Calls dev->dev_ops->dev_close(), which in this case is dpaa2_dev_close().
> > Then calls rte_eth_dev_release_port(), which releases all device data
> > and sets dev->data to NULL.
> >
> > Later, when rte_dev_remove() is called, the FSLMC bus invokes
> > dev->remove() — that is, rte_dpaa2_remove().
> > However, rte_dpaa2_remove() calls dpaa2_dev_close() again. Since
> > dev->data was already set to NULL by the previous call, this second
> > invocation causes a crash.
> >
> > Fixes: 5964d36a2904 ("net/dpaa2: release port upon close")
> > Cc: sachin.saxena@nxp.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> > ---
> >  drivers/net/dpaa2/dpaa2_ethdev.c | 15 ++++++++++-----
> >  1 file changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c
> > b/drivers/net/dpaa2/dpaa2_ethdev.c
> > index 7da32ce856..fc63cf4f09 100644
> > --- a/drivers/net/dpaa2/dpaa2_ethdev.c
> > +++ b/drivers/net/dpaa2/dpaa2_ethdev.c
> > @@ -3347,14 +3347,19 @@ static int
> >  rte_dpaa2_remove(struct rte_dpaa2_device *dpaa2_dev)  {
> >         struct rte_eth_dev *eth_dev;
> > -       int ret;
> > +       int ret = 0;
> > +
> > +       eth_dev = rte_eth_dev_allocated(dpaa2_dev->device.name);
> > +       if (eth_dev) {
> > +               dpaa2_dev_close(eth_dev);
> > +               ret = rte_eth_dev_release_port(eth_dev);
> > +       }
> 
> dpaa2_dev_close() returns a status code, but it isn’t being checked here.
> 
> For comparison, in rte_eth_dev_pci_generic_remove(), if dev_uninit (i.e. the
> device close callback) fails, rte_eth_dev_release_port() is not called.
> How should the dpaa2 driver handle this kind of error case?
> 
[Hemant] The current framework can only record the error. 

> By the way, if it makes sense for you, feel free to add a Tested-by tag from me,
> since I’ve validated the series on my side with Grout.
[Hemant] sure
> 
> Regards,
> 
> Maxime Leroy