On 13-11-2025 15:19, Maxime Leroy wrote:
[Some people who received this message don't often get email from
[email protected]. Learn why this is important at
https://aka.ms/LearnAboutSenderIdentification ]
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.
ok, it looks like best approach is to drop this patch.
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()?
dpaa2_tx_sg_pool is a global resource across devices; which is being
tracked with dpaa2_valid_dev
we will be revamping this logic in next major release.
Maxime Leroy
