[dpdk-dev] [PATCH 16/20] ethdev: remove old close behaviour

[Ferruh Yigit]

On 9/13/2020 11:07 PM, Thomas Monjalon wrote:
> The temporary flag RTE_ETH_DEV_CLOSE_REMOVE is removed.
> It was introduced in DPDK 18.11 in order to give time for PMDs to migrate.
> 
> The old behaviour was to free only queues when closing a port.
> The new behaviour is calling rte_eth_dev_release_port() which does
> three more tasks:
> 	- trigger event callback
> 	- reset state and few pointers
> 	- free all generic port resources
> 
> The private port resources must be released in the .dev_close callback.
> 
> The .remove callback should:
> 	- call .dev_close callback
> 	- call rte_eth_dev_release_port()
> 	- free multi-port device shared resources
> 
> Despite waiting two years, some drivers have not migrated,
> so they may hit issues with the incompatible new behaviour.
> After sending emails, adding logs, and announcing the deprecation,
> the only last solution is to declare these drivers as unmaintained:
> 	bnx2x, cxgbe, dpaa, dpaa2, enetc, ionic,
> 	ipn3ke,	liquidio, nfp, pfe, qede
> Below is a summary of what to implement in those drivers.
> 
> * The freeing of private port resources must be moved
> from the ".remove(device)" function to the ".dev_close(port)" function.
> 
> * If a generic resource (.mac_addrs or .hash_mac_addrs) cannot be freed,
> it must be set to NULL in ".dev_close" function to protect from
> subsequent rte_eth_dev_release_port() freeing.
> 
> * Note 1:
> The generic resources are freed in rte_eth_dev_release_port(),
> after ".dev_close" is called in rte_eth_dev_close(), but not when
> calling ".dev_close" directly from the ".remove" PMD function.
> That's why rte_eth_dev_release_port() must still be called explicitly
> from ".remove(device)" after calling the ".dev_close" PMD function.
> 
> * Note 2:
> If a device can have multiple ports, the common resources must be freed
> only in the ".remove(device)" function.
> 
> * Note 3:
> The port is supposed to be in a stopped state when it is closed.
> If it is not the case, it is free to the PMD implementation
> how to react when trying to close a non-stopped port:
> either try to stop it automatically or just return an error.

For this note, 'rte_eth_dev_close()' assumes port stopped and sets 
"dev->data->dev_started = 0;" blindly.

should we verify this, or perhaps should call 'rte_eth_dev_stop()' 
within the 'rte_eth_dev_close()' to be sure.


Also many PMDs doesn't check primary process check in the close, which 
should be fixed too, I wonder if it can be fixed in this series?


> -Cavium LiquidIO
> +Cavium LiquidIO - UNMAINTAINED
>   M: Shijith Thotton <sthotton at marvell.com>
>   M: Srisivasubramanian Srinivasan <srinivasan at marvell.com>
>   T: git://dpdk.org/next/dpdk-next-net-mrvl
> @@ -622,7 +622,7 @@ F: drivers/net/octeontx/
>   F: doc/guides/nics/octeontx.rst
>   F: doc/guides/nics/features/octeontx.ini
>   
> -Chelsio cxgbe
> +Chelsio cxgbe - UNMAINTAINED
>   M: Rahul Lakkireddy <rahul.lakkireddy at chelsio.com>
>   F: drivers/net/cxgbe/
>   F: doc/guides/nics/cxgbe.rst
> @@ -711,7 +711,7 @@ F: drivers/net/igc/
>   F: doc/guides/nics/igc.rst
>   F: doc/guides/nics/features/igc.ini
>   
> -Intel ipn3ke
> +Intel ipn3ke - UNMAINTAINED
>   M: Rosen Xu <rosen.xu at intel.com>
>   T: git://dpdk.org/next/dpdk-next-net-intel
>   F: drivers/net/ipn3ke/
> @@ -794,13 +794,13 @@ F: drivers/net/nfb/
>   F: doc/guides/nics/nfb.rst
>   F: doc/guides/nics/features/nfb.ini
>   
> -Netronome nfp
> +Netronome nfp - UNMAINTAINED
>   M: Heinrich Kuhn <heinrich.kuhn at netronome.com>
>   F: drivers/net/nfp/
>   F: doc/guides/nics/nfp.rst
>   F: doc/guides/nics/features/nfp*.ini
>   
> -NXP dpaa
> +NXP dpaa - UNMAINTAINED
>   M: Hemant Agrawal <hemant.agrawal at nxp.com>
>   M: Sachin Saxena <sachin.saxena at nxp.com>
>   F: drivers/mempool/dpaa/
> @@ -808,7 +808,7 @@ F: drivers/net/dpaa/
>   F: doc/guides/nics/dpaa.rst
>   F: doc/guides/nics/features/dpaa.ini
>   
> -NXP dpaa2
> +NXP dpaa2 - UNMAINTAINED
>   M: Hemant Agrawal <hemant.agrawal at nxp.com>
>   M: Sachin Saxena <sachin.saxena at nxp.com>
>   F: drivers/mempool/dpaa2/
> @@ -816,27 +816,27 @@ F: drivers/net/dpaa2/
>   F: doc/guides/nics/dpaa2.rst
>   F: doc/guides/nics/features/dpaa2.ini
>   
> -NXP enetc
> +NXP enetc - UNMAINTAINED
>   M: Gagandeep Singh <g.singh at nxp.com>
>   M: Sachin Saxena <sachin.saxena at nxp.com>
>   F: drivers/net/enetc/
>   F: doc/guides/nics/enetc.rst
>   F: doc/guides/nics/features/enetc.ini
>   
> -NXP pfe
> +NXP pfe - UNMAINTAINED
>   M: Gagandeep Singh <g.singh at nxp.com>
>   M: Akhil Goyal <akhil.goyal at nxp.com>
>   F: doc/guides/nics/pfe.rst
>   F: drivers/net/pfe/
>   F: doc/guides/nics/features/pfe.ini
>   
> -Pensando ionic
> +Pensando ionic - UNMAINTAINED
>   M: Alfredo Cardigliano <cardigliano at ntop.org>
>   F: drivers/net/ionic/
>   F: doc/guides/nics/ionic.rst
>   F: doc/guides/nics/features/ionic.ini
>   
> -QLogic bnx2x
> +QLogic bnx2x - UNMAINTAINED
>   M: Rasesh Mody <rmody at marvell.com>
>   M: Shahed Shaikh <shshaikh at marvell.com>
>   T: git://dpdk.org/next/dpdk-next-net-mrvl
> @@ -844,7 +844,7 @@ F: drivers/net/bnx2x/
>   F: doc/guides/nics/bnx2x.rst
>   F: doc/guides/nics/features/bnx2x*.ini
>   
> -QLogic qede PMD
> +QLogic qede PMD - UNMAINTAINED

I guess 'cxgbe' already send the patch, but for rest, is there a 
deadline for the patches, before mark them as "UNMAINTAINED"?