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

[Thomas Monjalon]

13/10/2020 19:54, 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.

Yes that's probably a case which not well tested.