| Message ID | 20200927234249.3198780-19-thomas@monjalon.net (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Thomas Monjalon |
| Headers | show |
| Series | cleanup ethdev close operation | expand |
| Context | Check | Description |
|---|---|---|
| ci/checkpatch | success | coding style OK |
Hi, > -----Original Message----- > From: Thomas Monjalon <thomas@monjalon.net> > Sent: Monday, September 28, 2020 7:43 > To: dev@dpdk.org > Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; arybchenko@solarflare.com; Xu, > Rosen <rosen.xu@intel.com>; Stephen Hemminger > <sthemmin@microsoft.com>; K. Y. Srinivasan <kys@microsoft.com>; > Haiyang Zhang <haiyangz@microsoft.com>; Long Li <longli@microsoft.com>; > Heinrich Kuhn <heinrich.kuhn@netronome.com>; Gagandeep Singh > <g.singh@nxp.com>; Akhil Goyal <akhil.goyal@nxp.com>; Martin Spinler > <spinler@cesnet.cz>; Burakov, Anatoly <anatoly.burakov@intel.com> > Subject: [PATCH v2 18/25] drivers/net: accept removing device without any > port > > The ports can be closed (i.e. completely released) before removing the > whole device. > Such case was wrongly considered an error by some drivers. > > If the device supports only one port, there is nothing much to free after the > port is closed. > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net> > Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com> > Reviewed-by: Rosen Xu <rosen.xu@intel.com> > --- > drivers/net/ipn3ke/ipn3ke_ethdev.c | 6 ++---- > drivers/net/kni/rte_eth_kni.c | 16 +++++++--------- > drivers/net/netvsc/hn_ethdev.c | 2 +- > drivers/net/nfp/nfp_net.c | 2 ++ > drivers/net/pfe/pfe_ethdev.c | 6 ++---- > drivers/net/szedata2/rte_eth_szedata2.c | 6 ++---- > 6 files changed, 16 insertions(+), 22 deletions(-) > > diff --git a/drivers/net/ipn3ke/ipn3ke_ethdev.c > b/drivers/net/ipn3ke/ipn3ke_ethdev.c > index 027be29bd8..4446d2af9e 100644 > --- a/drivers/net/ipn3ke/ipn3ke_ethdev.c > +++ b/drivers/net/ipn3ke/ipn3ke_ethdev.c > @@ -562,10 +562,8 @@ static int ipn3ke_vswitch_remove(struct > rte_afu_device *afu_dev) > afu_dev->device.name, i); > > ethdev = rte_eth_dev_allocated(afu_dev->device.name); > - if (!ethdev) > - return -ENODEV; > - > - rte_eth_dev_destroy(ethdev, ipn3ke_rpst_uninit); > + if (ethdev != NULL) > + rte_eth_dev_destroy(ethdev, ipn3ke_rpst_uninit); > } > > ret = rte_eth_switch_domain_free(hw->switch_domain_id); > diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c > index 45ab1b17a8..2a4058f7b0 100644 > --- a/drivers/net/kni/rte_eth_kni.c > +++ b/drivers/net/kni/rte_eth_kni.c > @@ -488,17 +488,15 @@ eth_kni_remove(struct rte_vdev_device *vdev) > > /* find the ethdev entry */ > eth_dev = rte_eth_dev_allocated(name); > - if (eth_dev == NULL) > - return -1; > - > - if (rte_eal_process_type() != RTE_PROC_PRIMARY) { > - eth_kni_dev_stop(eth_dev); > - return rte_eth_dev_release_port(eth_dev); > + if (eth_dev != NULL) { > + if (rte_eal_process_type() != RTE_PROC_PRIMARY) { > + eth_kni_dev_stop(eth_dev); > + return rte_eth_dev_release_port(eth_dev); > + } > + eth_kni_close(eth_dev); > + rte_eth_dev_release_port(eth_dev); > } > > - eth_kni_close(eth_dev); > - rte_eth_dev_release_port(eth_dev); > - > is_kni_initialized--; > if (is_kni_initialized == 0) > rte_kni_close(); > diff --git a/drivers/net/netvsc/hn_ethdev.c > b/drivers/net/netvsc/hn_ethdev.c index 15d6e9762d..19a9eb6bc2 100644 > --- a/drivers/net/netvsc/hn_ethdev.c > +++ b/drivers/net/netvsc/hn_ethdev.c > @@ -1092,7 +1092,7 @@ static int eth_hn_remove(struct rte_vmbus_device > *dev) > > eth_dev = rte_eth_dev_allocated(dev->device.name); > if (!eth_dev) > - return -ENODEV; > + return 0; /* port already released */ > > ret = eth_hn_dev_uninit(eth_dev); > if (ret) > diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c index > 9509dc8bd6..ce25cf1ed4 100644 > --- a/drivers/net/nfp/nfp_net.c > +++ b/drivers/net/nfp/nfp_net.c > @@ -3739,6 +3739,8 @@ static int eth_nfp_pci_remove(struct > rte_pci_device *pci_dev) > int port = 0; > > eth_dev = rte_eth_dev_allocated(pci_dev->device.name); > + if (eth_dev == NULL) > + return 0; /* port already released */ > if ((pci_dev->id.device_id == PCI_DEVICE_ID_NFP4000_PF_NIC) || > (pci_dev->id.device_id == PCI_DEVICE_ID_NFP6000_PF_NIC)) { > port = get_pf_port_number(eth_dev->data->name); > diff --git a/drivers/net/pfe/pfe_ethdev.c b/drivers/net/pfe/pfe_ethdev.c > index 187a0019ff..9d5415d9b1 100644 > --- a/drivers/net/pfe/pfe_ethdev.c > +++ b/drivers/net/pfe/pfe_ethdev.c > @@ -1155,10 +1155,8 @@ pmd_pfe_remove(struct rte_vdev_device *vdev) > return 0; > > eth_dev = rte_eth_dev_allocated(name); > - if (eth_dev == NULL) > - return -ENODEV; > - > - pfe_eth_exit(eth_dev, g_pfe); > + if (eth_dev != NULL) > + pfe_eth_exit(eth_dev, g_pfe); > munmap(g_pfe->cbus_baseaddr, g_pfe->cbus_size); > > if (g_pfe->nb_devs == 0) { > diff --git a/drivers/net/szedata2/rte_eth_szedata2.c > b/drivers/net/szedata2/rte_eth_szedata2.c > index 4325b9a30d..5f589dfa4c 100644 > --- a/drivers/net/szedata2/rte_eth_szedata2.c > +++ b/drivers/net/szedata2/rte_eth_szedata2.c > @@ -1910,10 +1910,8 @@ static int szedata2_eth_pci_remove(struct > rte_pci_device *pci_dev) > pci_dev->device.name, i); > PMD_DRV_LOG(DEBUG, "Removing eth_dev %s", name); > eth_dev = rte_eth_dev_allocated(name); > - if (!eth_dev) { > - PMD_DRV_LOG(ERR, "eth_dev %s not found", > name); > - retval = retval ? retval : -ENODEV; > - } > + if (eth_dev == NULL) > + continue; /* port already released */ > > ret = rte_szedata2_eth_dev_uninit(eth_dev); > if (ret != 0) { > -- > 2.28.0 For net/ipn3ke Reviewed-by: Rosen Xu <rosen.xu@intel.com>
For "net/pfe" Reviewed-by: Sachin Saxena<sachin.saxena@oss.nxp.com> On 28-Sep-20 5:12 AM, Thomas Monjalon wrote: > The ports can be closed (i.e. completely released) > before removing the whole device. > Such case was wrongly considered an error by some drivers. > > If the device supports only one port, there is nothing much > to free after the port is closed. > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net> > Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com> > Reviewed-by: Rosen Xu <rosen.xu@intel.com> > --- > drivers/net/ipn3ke/ipn3ke_ethdev.c | 6 ++---- > drivers/net/kni/rte_eth_kni.c | 16 +++++++--------- > drivers/net/netvsc/hn_ethdev.c | 2 +- > drivers/net/nfp/nfp_net.c | 2 ++ > drivers/net/pfe/pfe_ethdev.c | 6 ++---- > drivers/net/szedata2/rte_eth_szedata2.c | 6 ++---- > 6 files changed, 16 insertions(+), 22 deletions(-) > > diff --git a/drivers/net/ipn3ke/ipn3ke_ethdev.c b/drivers/net/ipn3ke/ipn3ke_ethdev.c > index 027be29bd8..4446d2af9e 100644 > --- a/drivers/net/ipn3ke/ipn3ke_ethdev.c > +++ b/drivers/net/ipn3ke/ipn3ke_ethdev.c > @@ -562,10 +562,8 @@ static int ipn3ke_vswitch_remove(struct rte_afu_device *afu_dev) > afu_dev->device.name, i); > > ethdev = rte_eth_dev_allocated(afu_dev->device.name); > - if (!ethdev) > - return -ENODEV; > - > - rte_eth_dev_destroy(ethdev, ipn3ke_rpst_uninit); > + if (ethdev != NULL) > + rte_eth_dev_destroy(ethdev, ipn3ke_rpst_uninit); > } > > ret = rte_eth_switch_domain_free(hw->switch_domain_id); > diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c > index 45ab1b17a8..2a4058f7b0 100644 > --- a/drivers/net/kni/rte_eth_kni.c > +++ b/drivers/net/kni/rte_eth_kni.c > @@ -488,17 +488,15 @@ eth_kni_remove(struct rte_vdev_device *vdev) > > /* find the ethdev entry */ > eth_dev = rte_eth_dev_allocated(name); > - if (eth_dev == NULL) > - return -1; > - > - if (rte_eal_process_type() != RTE_PROC_PRIMARY) { > - eth_kni_dev_stop(eth_dev); > - return rte_eth_dev_release_port(eth_dev); > + if (eth_dev != NULL) { > + if (rte_eal_process_type() != RTE_PROC_PRIMARY) { > + eth_kni_dev_stop(eth_dev); > + return rte_eth_dev_release_port(eth_dev); > + } > + eth_kni_close(eth_dev); > + rte_eth_dev_release_port(eth_dev); > } > > - eth_kni_close(eth_dev); > - rte_eth_dev_release_port(eth_dev); > - > is_kni_initialized--; > if (is_kni_initialized == 0) > rte_kni_close(); > diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c > index 15d6e9762d..19a9eb6bc2 100644 > --- a/drivers/net/netvsc/hn_ethdev.c > +++ b/drivers/net/netvsc/hn_ethdev.c > @@ -1092,7 +1092,7 @@ static int eth_hn_remove(struct rte_vmbus_device *dev) > > eth_dev = rte_eth_dev_allocated(dev->device.name); > if (!eth_dev) > - return -ENODEV; > + return 0; /* port already released */ > > ret = eth_hn_dev_uninit(eth_dev); > if (ret) > diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c > index 9509dc8bd6..ce25cf1ed4 100644 > --- a/drivers/net/nfp/nfp_net.c > +++ b/drivers/net/nfp/nfp_net.c > @@ -3739,6 +3739,8 @@ static int eth_nfp_pci_remove(struct rte_pci_device *pci_dev) > int port = 0; > > eth_dev = rte_eth_dev_allocated(pci_dev->device.name); > + if (eth_dev == NULL) > + return 0; /* port already released */ > if ((pci_dev->id.device_id == PCI_DEVICE_ID_NFP4000_PF_NIC) || > (pci_dev->id.device_id == PCI_DEVICE_ID_NFP6000_PF_NIC)) { > port = get_pf_port_number(eth_dev->data->name); > diff --git a/drivers/net/pfe/pfe_ethdev.c b/drivers/net/pfe/pfe_ethdev.c > index 187a0019ff..9d5415d9b1 100644 > --- a/drivers/net/pfe/pfe_ethdev.c > +++ b/drivers/net/pfe/pfe_ethdev.c > @@ -1155,10 +1155,8 @@ pmd_pfe_remove(struct rte_vdev_device *vdev) > return 0; > > eth_dev = rte_eth_dev_allocated(name); > - if (eth_dev == NULL) > - return -ENODEV; > - > - pfe_eth_exit(eth_dev, g_pfe); > + if (eth_dev != NULL) > + pfe_eth_exit(eth_dev, g_pfe); > munmap(g_pfe->cbus_baseaddr, g_pfe->cbus_size); > > if (g_pfe->nb_devs == 0) { > diff --git a/drivers/net/szedata2/rte_eth_szedata2.c b/drivers/net/szedata2/rte_eth_szedata2.c > index 4325b9a30d..5f589dfa4c 100644 > --- a/drivers/net/szedata2/rte_eth_szedata2.c > +++ b/drivers/net/szedata2/rte_eth_szedata2.c > @@ -1910,10 +1910,8 @@ static int szedata2_eth_pci_remove(struct rte_pci_device *pci_dev) > pci_dev->device.name, i); > PMD_DRV_LOG(DEBUG, "Removing eth_dev %s", name); > eth_dev = rte_eth_dev_allocated(name); > - if (!eth_dev) { > - PMD_DRV_LOG(ERR, "eth_dev %s not found", name); > - retval = retval ? retval : -ENODEV; > - } > + if (eth_dev == NULL) > + continue; /* port already released */ > > ret = rte_szedata2_eth_dev_uninit(eth_dev); > if (ret != 0) {
diff --git a/drivers/net/ipn3ke/ipn3ke_ethdev.c b/drivers/net/ipn3ke/ipn3ke_ethdev.c index 027be29bd8..4446d2af9e 100644 --- a/drivers/net/ipn3ke/ipn3ke_ethdev.c +++ b/drivers/net/ipn3ke/ipn3ke_ethdev.c @@ -562,10 +562,8 @@ static int ipn3ke_vswitch_remove(struct rte_afu_device *afu_dev) afu_dev->device.name, i); ethdev = rte_eth_dev_allocated(afu_dev->device.name); - if (!ethdev) - return -ENODEV; - - rte_eth_dev_destroy(ethdev, ipn3ke_rpst_uninit); + if (ethdev != NULL) + rte_eth_dev_destroy(ethdev, ipn3ke_rpst_uninit); } ret = rte_eth_switch_domain_free(hw->switch_domain_id); diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c index 45ab1b17a8..2a4058f7b0 100644 --- a/drivers/net/kni/rte_eth_kni.c +++ b/drivers/net/kni/rte_eth_kni.c @@ -488,17 +488,15 @@ eth_kni_remove(struct rte_vdev_device *vdev) /* find the ethdev entry */ eth_dev = rte_eth_dev_allocated(name); - if (eth_dev == NULL) - return -1; - - if (rte_eal_process_type() != RTE_PROC_PRIMARY) { - eth_kni_dev_stop(eth_dev); - return rte_eth_dev_release_port(eth_dev); + if (eth_dev != NULL) { + if (rte_eal_process_type() != RTE_PROC_PRIMARY) { + eth_kni_dev_stop(eth_dev); + return rte_eth_dev_release_port(eth_dev); + } + eth_kni_close(eth_dev); + rte_eth_dev_release_port(eth_dev); } - eth_kni_close(eth_dev); - rte_eth_dev_release_port(eth_dev); - is_kni_initialized--; if (is_kni_initialized == 0) rte_kni_close(); diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c index 15d6e9762d..19a9eb6bc2 100644 --- a/drivers/net/netvsc/hn_ethdev.c +++ b/drivers/net/netvsc/hn_ethdev.c @@ -1092,7 +1092,7 @@ static int eth_hn_remove(struct rte_vmbus_device *dev) eth_dev = rte_eth_dev_allocated(dev->device.name); if (!eth_dev) - return -ENODEV; + return 0; /* port already released */ ret = eth_hn_dev_uninit(eth_dev); if (ret) diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c index 9509dc8bd6..ce25cf1ed4 100644 --- a/drivers/net/nfp/nfp_net.c +++ b/drivers/net/nfp/nfp_net.c @@ -3739,6 +3739,8 @@ static int eth_nfp_pci_remove(struct rte_pci_device *pci_dev) int port = 0; eth_dev = rte_eth_dev_allocated(pci_dev->device.name); + if (eth_dev == NULL) + return 0; /* port already released */ if ((pci_dev->id.device_id == PCI_DEVICE_ID_NFP4000_PF_NIC) || (pci_dev->id.device_id == PCI_DEVICE_ID_NFP6000_PF_NIC)) { port = get_pf_port_number(eth_dev->data->name); diff --git a/drivers/net/pfe/pfe_ethdev.c b/drivers/net/pfe/pfe_ethdev.c index 187a0019ff..9d5415d9b1 100644 --- a/drivers/net/pfe/pfe_ethdev.c +++ b/drivers/net/pfe/pfe_ethdev.c @@ -1155,10 +1155,8 @@ pmd_pfe_remove(struct rte_vdev_device *vdev) return 0; eth_dev = rte_eth_dev_allocated(name); - if (eth_dev == NULL) - return -ENODEV; - - pfe_eth_exit(eth_dev, g_pfe); + if (eth_dev != NULL) + pfe_eth_exit(eth_dev, g_pfe); munmap(g_pfe->cbus_baseaddr, g_pfe->cbus_size); if (g_pfe->nb_devs == 0) { diff --git a/drivers/net/szedata2/rte_eth_szedata2.c b/drivers/net/szedata2/rte_eth_szedata2.c index 4325b9a30d..5f589dfa4c 100644 --- a/drivers/net/szedata2/rte_eth_szedata2.c +++ b/drivers/net/szedata2/rte_eth_szedata2.c @@ -1910,10 +1910,8 @@ static int szedata2_eth_pci_remove(struct rte_pci_device *pci_dev) pci_dev->device.name, i); PMD_DRV_LOG(DEBUG, "Removing eth_dev %s", name); eth_dev = rte_eth_dev_allocated(name); - if (!eth_dev) { - PMD_DRV_LOG(ERR, "eth_dev %s not found", name); - retval = retval ? retval : -ENODEV; - } + if (eth_dev == NULL) + continue; /* port already released */ ret = rte_szedata2_eth_dev_uninit(eth_dev); if (ret != 0) {