Re: [dpdk-dev] [PATCH v3 21/29] ethdev: remove old close behaviour

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 9/29/2020 12:14 AM, 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:
> 	ionic, liquidio, nfp
> 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.
> 
> Cc: Shijith Thotton <sthotton@marvell.com>
> Cc: Srisivasubramanian Srinivasan <srinivasan@marvell.com>
> Cc: Heinrich Kuhn <heinrich.kuhn@netronome.com>
> Cc: Alfredo Cardigliano <cardigliano@ntop.org>
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> Reviewed-by: Liron Himi <lironh@marvell.com>


Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 49a6dfa7a5..6aaf80d483 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -604,7 +604,7 @@ F: drivers/net/thunderx/
>   F: doc/guides/nics/thunderx.rst
>   F: doc/guides/nics/features/thunderx.ini
>   
> -Cavium LiquidIO
> +Cavium LiquidIO - UNMAINTAINED
>   M: Shijith Thotton <sthotton@marvell.com>
>   M: Srisivasubramanian Srinivasan <srinivasan@marvell.com>
>   T: git://dpdk.org/next/dpdk-next-net-mrvl
> @@ -793,7 +793,7 @@ 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@netronome.com>
>   F: drivers/net/nfp/
>   F: doc/guides/nics/nfp.rst
> @@ -829,7 +829,7 @@ 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@ntop.org>
>   F: drivers/net/ionic/
>   F: doc/guides/nics/ionic.rst

Just to highlight above drivers that are marked as 'UNMAINTAINED'.