From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 79B0F48AF8; Thu, 13 Nov 2025 10:48:32 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3F59A4028B; Thu, 13 Nov 2025 10:48:32 +0100 (CET) Received: from mail-qt1-f180.google.com (mail-qt1-f180.google.com [209.85.160.180]) by mails.dpdk.org (Postfix) with ESMTP id DFA3040151 for ; Thu, 13 Nov 2025 10:48:31 +0100 (CET) Received: by mail-qt1-f180.google.com with SMTP id d75a77b69052e-4e89de04d62so5315941cf.0 for ; Thu, 13 Nov 2025 01:48:31 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1763027311; x=1763632111; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=zX0hrP9j3XFyJfD2iQWyXxNiiboowVa/thjagm+VCoI=; b=xDHjtgszKtK20VAeIFB1cwU8QuNtF5V/22KcJUQDmqD1vf6OyqedNp15W7M5lQB4mg aHg2wCn6VUcf1/e1ok1os7WeUu9eErvByogezIp3kkVOeLxCXkn6U1WIA/7/EPPaxrbc 2ctr1kzkWlO+c9YX/99No4sNiLNs9FCQ20puGLCEdTUohrlF4U3GaiDNThLK/OvKlUfL EpMFXQk07qBHuQ0K7FHlG+6H2JZhOr0cw5kdnRQsJChjVT62UjqCm1TUfHh1YyZiYL1G 1Ij67aXqmA+c2b669FD7kOuSncZclAVsLClbcyhXSsqxBQJ9w2GopEWv6Xl77QTLVsm7 Uqtw== X-Gm-Message-State: AOJu0YxHgf7dBKqO6Dt6HUUcN7/Z+q9K4Kkn1QPjOyABAWr1PNO8mxh2 invibzrPOPJwQ9/JF1HZRFuW3jrpcW1hbWnKC7Y0c+dsj/TSf4V2EzNbYrBLt9TRvHtK6rlmz3M EDW+cVl5CGydn6bqOqN54WoywpLughWg= X-Gm-Gg: ASbGncvZCEyfHxIaVRA7uVFEhuADpyOXJbhkGVa4B3zqjb6/7yt8rv9IhzilDOHZ+Am 6qctV/bftZxZtvZMAEDef676jbiisBGPf4vhP4Gn/mkZFZ4HWrsu1b0/n8PZ5lXIUQ/qNiUNHBX z2qe4uCYEm+yjmKiuQDvH2Qa3XdcydFKxzO+zZvFRqwU2QwRJZlsYvVGMdujiGL4GGQHG9HUY3k 1Yw8fl5NoaTwX7iqfOxq3K7GNLGYPoFmJnmApNg/r5ItzoiYCWi49UfcQ== X-Google-Smtp-Source: AGHT+IFx6IrWOZUPHY9GDzn9cBjifr+jMR8nAYxwAB+Ir79DSxIcmcrUjv5XqaOn5pJtFxAyeBOLJYyZyaFEa8eF39M= X-Received: by 2002:a05:622a:1308:b0:4e8:9f46:3ffd with SMTP id d75a77b69052e-4eddbddd3c5mr77915821cf.80.1763027310891; Thu, 13 Nov 2025 01:48:30 -0800 (PST) MIME-Version: 1.0 References: <20251106163807.201451-1-hemant.agrawal@nxp.com> <20251113052931.1784953-1-hemant.agrawal@nxp.com> <20251113052931.1784953-4-hemant.agrawal@nxp.com> In-Reply-To: <20251113052931.1784953-4-hemant.agrawal@nxp.com> From: Maxime Leroy Date: Thu, 13 Nov 2025 10:48:19 +0100 X-Gm-Features: AWmQ_bmFAyvy6YZZfzoImkS6hYgq6UGIASuBjwnlLd7VoHuglsIs2pHnAblaeN4 Message-ID: Subject: Re: [PATCH v2 4/5] net/dpaa2: bifurcate close into close and deinit To: Hemant Agrawal Cc: dev@dpdk.org, stephen@networkplumber.org, david.marchand@redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Hi Hemant, Le jeu. 13 nov. 2025 =C3=A0 06:30, Hemant Agrawal = a =C3=A9crit : > > 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 > --- > 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_e= thdev.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 =3D dev->data->dev_private; > - struct fsl_mc_io *dpni =3D dev->process_private; > - int i, ret; > - struct rte_eth_link link; > - > PMD_INIT_FUNC_TRACE(); > > if (rte_eal_process_type() !=3D 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 =3D dpni_reset(dpni, CMD_PRI_LOW, priv->token); > - if (ret) { > - DPAA2_PMD_ERR("Failure cleaning dpni device: err=3D%d", r= et); > - 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 =3D 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 =3D NULL; > - dev->process_private =3D NULL; > - rte_free(dpni); > > - for (i =3D 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, con= st char *key) > return 1; > } > > +static int > +dpaa2_dev_deinit(struct rte_eth_dev *dev) > +{ > + struct dpaa2_dev_priv *priv =3D dev->data->dev_private; > + struct fsl_mc_io *dpni =3D 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 =3D dpni_reset(dpni, CMD_PRI_LOW, priv->token); > + if (ret) { > + DPAA2_PMD_ERR("Failure cleaning dpni device: err=3D%d", r= et); > + 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 =3D 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 =3D NULL; > + dev->process_private =3D NULL; > + rte_free(dpni); > + > + for (i =3D 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 =3D rte_eth_dev_allocated(dpaa2_dev->device.name); > if (eth_dev) { > - dpaa2_dev_close(eth_dev); > + dpaa2_dev_deinit(eth_dev); > ret =3D 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=E2=80=99s remove callback. I don=E2=80=99t see any issue doing the = same here=E2=80=94calling 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=E2=80=99s 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=E2=80=99t dpaa2_tx_sg_pool_init() done directly in dpaa2_dev_init(), with its corresponding cleanup performed in dpaa2_dev_close()? Maxime Leroy