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.
