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

Ferruh Yigit ferruh.yigit at intel.com
Tue Sep 29 18:39:32 CEST 2020


On 9/29/2020 5:06 PM, Thomas Monjalon wrote:
> 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.
> 

I think many PMD don't update the 'dev_started' right now.

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

OK

>> Although not sure how to handle driver 'remove()' case.
> 
> What are you referring to exactly?
> 

This was questioning how to manage 'rte_eth_dev_stop()' call on driver 
'remove()' path. Not valid after above comment.



More information about the dev mailing list