[dpdk-dev] [PATCH v3 27/29] ethdev: remove forcing stopped state upon close

[Thomas Monjalon]

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 <thomas at 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?