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

[Thomas Monjalon <thomas@monjalon.net>]

13/10/2020 14:36, 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@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.

Will check

> <...>
> 
> > @@ -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.

Will check

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

I think we can ignore for this patch,
but maybe these PMDs could be updated separately
to comply with the stop/close sequence.