[dpdk-dev] [PATCH v3 29/29] ethdev: allow close function to return an error

[Andrew Rybchenko]

On 9/29/20 2:47 PM, Thomas Monjalon wrote:
> 29/09/2020 13:05, Andrew Rybchenko:
>> On 9/29/20 2:14 AM, Thomas Monjalon wrote:
>>> The API function rte_eth_dev_close() was returning void.
>>> The return type is changed to int for notifying of errors.
>>>
>>> If an error happens during a close operation,
>>> the status of the port is undefined,
>>> a maximum of resources having been freed.
> [...]
>>> @@ -135,7 +135,6 @@ Deprecation Notices
>>>    invalid port ID, unsupported operation, failed operation):
>>>  
>>>    - ``rte_eth_dev_stop``
>>> -  - ``rte_eth_dev_close``
>>
>> Many thanks for continuing my unfinished job.
> 
> It is not finished.
> Would you take rte_eth_dev_stop?

Sorry, I can't promise that I find time. I'll try.
If anyone else is ready, welcome.

>>> -void
>>> +int
>>>  rte_eth_dev_close(uint16_t port_id)
>>>  {
>>>  	struct rte_eth_dev *dev;
>>> +	int ret;
>>>  
>>> -	RTE_ETH_VALID_PORTID_OR_RET(port_id);
>>> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>>>  	dev = &rte_eth_devices[port_id];
>>>  
>>> -	RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_close);
>>> -	(*dev->dev_ops->dev_close)(dev);
>>> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_close, -ENOTSUP);
>>> +	ret = (*dev->dev_ops->dev_close)(dev);
>>>  
>>>  	rte_ethdev_trace_close(port_id);
>>> -	rte_eth_dev_release_port(dev);
>>> +	ret = rte_eth_dev_release_port(dev);
>>
>> It does not look good that it overwrites close return status.
> 
> Indeed, it seems I did not finish this patch,
> or maybe I was drunk ;)
> 
> What do you think is the best strategy?
> I think we must call rte_eth_dev_release_port()
> even if dev_close returns an error.
> Then which error to return? I would return the first non-zero one.

Yes, exactly. Return the first non-zero error code.