29/09/2020 18:01, Ferruh Yigit: > On 9/29/2020 12:14 AM, Thomas Monjalon wrote: > > When closing a port, it is supposed to be already stopped, > > and marked as such with "dev_started" state zeroed. > > > > 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()". > > > > Signed-off-by: Thomas Monjalon <tho...@monjalon.net> > > --- > > lib/librte_ethdev/rte_ethdev.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c > > index d7668114ca..0b8e8e3e8d 100644 > > --- a/lib/librte_ethdev/rte_ethdev.c > > +++ b/lib/librte_ethdev/rte_ethdev.c > > @@ -1716,7 +1716,6 @@ rte_eth_dev_close(uint16_t port_id) > > dev = &rte_eth_devices[port_id]; > > > > RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_close); > > - dev->data->dev_started = 0; > > (*dev->dev_ops->dev_close)(dev); > > > > rte_ethdev_trace_close(port_id); > > > > The driver 'remove()' function may be calling the driver 'stop()' dev_ops > internally, so the device will be stopped properly but the 'dev_started' > status > won't be updated because ethdev API is not called.
If the driver is managing it internally, it should reset the state as well. > This API assumes device stopped and updates the state accordingly, it is not > good but removing it also won't be good for the case device already stopped. > > What do you think calling 'rte_eth_dev_stop()' from 'rte_eth_dev_close()'? I think it would be confusing. Better to let the application and the driver manage "stop" at their best. > Although not sure how to handle driver 'remove()' case. What are you referring to exactly?
