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?
> > rte_pmd_mvneta_remove(struct rte_vdev_device *vdev)
> > {
> > uint16_t port_id;
> > + int ret = 0;
> >
> > RTE_ETH_FOREACH_DEV(port_id) {
> > if (rte_eth_devices[port_id].device != &vdev->device)
> > continue;
> > - rte_eth_dev_close(port_id);
> > + ret = rte_eth_dev_close(port_id);
>
> I guess |= should be used here as well (similar as above).
Yes
> > -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.
