Re: [PATCH 4/5] net/dpaa2: remove ethdev device in bus device

[Maxime Leroy <maxime@leroys.fr>]

Hi David,

Le sam. 8 nov. 2025 à 16:32, David Marchand
<david.marchand@redhat.com> a écrit :
>
> Calling rte_eth_dev_allocated() is enough to retrieve the
> ethdev device object.
>
> This leaves no user of the ethdev field in the fslmc device object.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  drivers/bus/fslmc/bus_fslmc_driver.h | 1 -
>  drivers/net/dpaa2/dpaa2_ethdev.c     | 6 ++++--
>  2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bus/fslmc/bus_fslmc_driver.h b/drivers/bus/fslmc/bus_fslmc_driver.h
> index efa88754a7..094f885f59 100644
> --- a/drivers/bus/fslmc/bus_fslmc_driver.h
> +++ b/drivers/bus/fslmc/bus_fslmc_driver.h
> @@ -98,7 +98,6 @@ struct rte_dpaa2_device {
>         TAILQ_ENTRY(rte_dpaa2_device) next; /**< Next probed DPAA2 device. */
>         struct rte_device device;           /**< Inherit core device */
>         union {
> -               struct rte_eth_dev *eth_dev;        /**< ethernet device */
>                 struct rte_rawdev *rawdev;          /**< Raw Device */
>         };
>         enum rte_dpaa2_dev_type dev_type;   /**< Device Type */
> diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
> index 7da32ce856..2a26a02cb4 100644
> --- a/drivers/net/dpaa2/dpaa2_ethdev.c
> +++ b/drivers/net/dpaa2/dpaa2_ethdev.c
> @@ -3320,7 +3320,6 @@ rte_dpaa2_probe(struct rte_dpaa2_driver *dpaa2_drv,
>
>         eth_dev->device = &dpaa2_dev->device;
>
> -       dpaa2_dev->eth_dev = eth_dev;
>         eth_dev->data->rx_mbuf_alloc_failed = 0;
>
>         if (dpaa2_drv->drv_flags & RTE_DPAA2_DRV_INTR_LSC)
> @@ -3349,7 +3348,10 @@ rte_dpaa2_remove(struct rte_dpaa2_device *dpaa2_dev)
>         struct rte_eth_dev *eth_dev;
>         int ret;
>
> -       eth_dev = dpaa2_dev->eth_dev;
> +       eth_dev = rte_eth_dev_allocated(dpaa2_dev->device.name);
> +       if (!eth_dev)
> +               return 0;
> +
>         dpaa2_dev_close(eth_dev);
>         dpaa2_valid_dev--;
>         if (!dpaa2_valid_dev)


dpaa2_valid_dev is currently incremented in rte_dpaa2_probe. With your
modification, dpaa2_valid_dev will never be decremented in
rte_dpaa2_remove if rte_eth_dev_close has already been called.
As a consequence, dpaa2_tx_sg_pool will never be freed.

I don’t think dpaa2_tx_sg_pool should be freed in rte_dpaa2_remove; it
should be freed in dpaa2_dev_close instead.
For symmetry, dpaa2_tx_sg_pool should also be allocated directly in
dpaa2_dev_init.

Regards,

Maxime Leroy