Hi Hemant, Le jeu. 13 nov. 2025 à 06:30, Hemant Agrawal <[email protected]> a écrit : > > The close function was overloaded to be also used as deinit function. > This can cause issues when using functions like hotplug. > > This patch cleans up the code and implement separate close and deinit > > Signed-off-by: Hemant Agrawal <[email protected]> > --- > drivers/net/dpaa2/dpaa2_ethdev.c | 92 ++++++++++++++++++-------------- > 1 file changed, 51 insertions(+), 41 deletions(-) > > diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c > b/drivers/net/dpaa2/dpaa2_ethdev.c > index f82c50341d..0cd875d715 100644 > --- a/drivers/net/dpaa2/dpaa2_ethdev.c > +++ b/drivers/net/dpaa2/dpaa2_ethdev.c > @@ -1524,53 +1524,14 @@ dpaa2_dev_stop(struct rte_eth_dev *dev) > static int > dpaa2_dev_close(struct rte_eth_dev *dev) > { > - struct dpaa2_dev_priv *priv = dev->data->dev_private; > - struct fsl_mc_io *dpni = dev->process_private; > - int i, ret; > - struct rte_eth_link link; > - > PMD_INIT_FUNC_TRACE(); > > if (rte_eal_process_type() != RTE_PROC_PRIMARY) > return 0; > > - if (!dpni) { > - DPAA2_PMD_WARN("Already closed or not started"); > - return -EINVAL; > - } > - > dpaa2_tm_deinit(dev); > dpaa2_flow_clean(dev); > - /* Clean the device first */ > - ret = dpni_reset(dpni, CMD_PRI_LOW, priv->token); > - if (ret) { > - DPAA2_PMD_ERR("Failure cleaning dpni device: err=%d", ret); > - return ret; > - } > - > - memset(&link, 0, sizeof(link)); > - rte_eth_linkstatus_set(dev, &link); > - > - /* Free private queues memory */ > - dpaa2_free_rx_tx_queues(dev); > - /* Close the device at underlying layer*/ > - ret = dpni_close(dpni, CMD_PRI_LOW, priv->token); > - if (ret) { > - DPAA2_PMD_ERR("Failure closing dpni device with err code %d", > - ret); > - } > - > - /* Free the allocated memory for ethernet private data and dpni*/ > - priv->hw = NULL; > - dev->process_private = NULL; > - rte_free(dpni); > > - for (i = 0; i < MAX_TCS; i++) > - rte_free(priv->extract.tc_extract_param[i]); > - > - rte_free(priv->extract.qos_extract_param); > - > - DPAA2_PMD_INFO("%s: netdev deleted", dev->data->name); > return 0; > } > > @@ -2891,6 +2852,55 @@ dpaa2_get_devargs(struct rte_devargs *devargs, const > char *key) > return 1; > } > > +static int > +dpaa2_dev_deinit(struct rte_eth_dev *dev) > +{ > + struct dpaa2_dev_priv *priv = dev->data->dev_private; > + struct fsl_mc_io *dpni = dev->process_private; > + int i, ret; > + struct rte_eth_link link; > + > + PMD_INIT_FUNC_TRACE(); > + > + if (!dpni) { > + DPAA2_PMD_WARN("Already closed or not started"); > + return -EINVAL; > + } > + > + /* Clean the device first */ > + ret = dpni_reset(dpni, CMD_PRI_LOW, priv->token); > + if (ret) { > + DPAA2_PMD_ERR("Failure cleaning dpni device: err=%d", ret); > + return ret; > + } > + > + memset(&link, 0, sizeof(link)); > + rte_eth_linkstatus_set(dev, &link); > + > + /* Free private queues memory */ > + dpaa2_free_rx_tx_queues(dev); > + /* Close the device at underlying layer*/ > + ret = dpni_close(dpni, CMD_PRI_LOW, priv->token); > + if (ret) { > + DPAA2_PMD_ERR("Failure closing dpni device with err code %d", > + ret); > + } > + > + /* Free the allocated memory for ethernet private data and dpni*/ > + priv->hw = NULL; > + dev->process_private = NULL; > + rte_free(dpni); > + > + for (i = 0; i < MAX_TCS; i++) > + rte_free(priv->extract.tc_extract_param[i]); > + > + rte_free(priv->extract.qos_extract_param); > + > + DPAA2_PMD_INFO("%s: netdev deleted", dev->data->name); > + return 0; > +} > + > + > static int > dpaa2_dev_init(struct rte_eth_dev *eth_dev) > { > @@ -3177,7 +3187,7 @@ dpaa2_dev_init(struct rte_eth_dev *eth_dev) > > return 0; > init_err: > - dpaa2_dev_close(eth_dev); > + dpaa2_dev_deinit(eth_dev); > > return ret; > } > @@ -3381,7 +3391,7 @@ rte_dpaa2_remove(struct rte_dpaa2_device *dpaa2_dev) > > eth_dev = rte_eth_dev_allocated(dpaa2_dev->device.name); > if (eth_dev) { > - dpaa2_dev_close(eth_dev); > + dpaa2_dev_deinit(eth_dev); > ret = rte_eth_dev_release_port(eth_dev); > } >
When a DPDK application stops, dpaa2_dev_close is no longer called. This happens because eal_cleanup() invokes rte_fslmc_close(), which then calls the remove function (rte_dpaa2_remove) for each DPAA2 Ethernet driver. As a result, dpaa2_tm_deinit() and dpaa2_flow_clean() are no longer executed. Another issue is that dpaa2_dev_deinit is now called in secondary process, which was not the case before this commit. For comparison, other drivers like ixgbe call their close method from the bus’s remove callback. I don’t see any issue doing the same here—calling close during remove if the Ethernet device is still allocated. As far as I know, there is no counterpart to rte_eth_dev_close() in the DPDK API (there could have been an rte_eth_dev_open()). The open path is always triggered by the bus’s probe callback. Because of this, it is expected that dpaa2_dev_init is called from the plug/probe path, while the corresponding deinit logic is handled in the close callback. One more point: why isn’t dpaa2_tx_sg_pool_init() done directly in dpaa2_dev_init(), with its corresponding cleanup performed in dpaa2_dev_close()? Maxime Leroy
