Re: [dpdk-dev] [PATCH v5 1/3] ethdev: remove forcing stopped state upon close

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

On 10/13/2020 11:06 AM, Thomas Monjalon wrote:
> When closing a port, it is supposed to be already stopped,
> and marked as such with "dev_started" state zeroed by the stop API.
> 
> Resetting "dev_started" before calling the driver close operation
> was hiding the case of not properly stopped port being closed.
> The flag "dev_started" is not changed anymore in "rte_eth_dev_close()".
> 
> In case the "dev_stop" function is called from "dev_close",
> bypassing "rte_eth_dev_stop()" API,
> the "dev_started" state must be explicitly reset in the PMD
> in order to keep the same behaviour.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>

<...>

> @@ -1497,6 +1497,7 @@ eth_igb_stop(struct rte_eth_dev *dev)
>   	}
>   
>   	adapter->stopped = true;
> +	dev->data->dev_started = 0;
>   }

'igbvf_dev_stop()' may be missed.

<...>

> @@ -2917,6 +2917,7 @@ ixgbe_dev_stop(struct rte_eth_dev *dev)
>   	adapter->rss_reta_updated = 0;
>   
>   	hw->adapter_stopped = true;
> +	dev->data->dev_started = 0;
>   }

'ixgbevf_dev_stop()' may be missed.


Also many virtual PMDs doesn't call 'dev_stop()' from the 'dev_close()', for 
many cases they may not needed to, since there is no device to stop.
But for the sake of the correct status, should 'dev_started' set to '0' in 
'dev_close()' dev_ops, or do you think can we ignore this?