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

[Ferruh Yigit]

On 10/13/2020 1:51 PM, Thomas Monjalon wrote:
> 13/10/2020 14:45, Ferruh Yigit:
>> 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 at monjalon.net>
>>> Acked-by: Stephen Hemminger <stephen at networkplumber.org>
>>> Reviewed-by: Andrew Rybchenko <arybchenko at solarflare.com>
> [...]
>> Following non-virtual PMDs doesn't call 'dev_stop()' from 'dev_close()',
>> maintainers of the PMDs cc'ed.
>>
>> avp
>> axgbe
>> bnx2x
>> cxgbe
>> dpaa2
>> enic
>> ice_dcf
>> ionic
>> ipn3ke
>> mlx4
>> mlx5
>> mvpp2
>> nfp
>> octeontx
>> octeontx2
>> sfc
>>
>> Can you please double check for your driver:
>>
>> 1) Device stopped properly before it has been closed?
>>
>> 2) The device stopped state ('dev->data->dev_started') is correct if device
>> closed without explicitly stopped first.
> 
> The application is supposed to call stop before close,
> so I don't know what has to be checked.
> The automatic stop in close is an extra feature per PMD choice.
> 

A device can be hot-removed without stop, that is why I was thinking 'close()' 
should stop the device (if it is not already).

Even current 'rte_eth_dev_close()' API doesn't check if the port stopped or not.

If application does stop -> close/remove in order, that would work, but for the 
case it didn't, PMD ensuring the device stopped before close is safer.