Re: [dpdk-dev] [PATCH] ethdev: stop the device before close

[Thomas Monjalon <thomas@monjalon.net>]

20/10/2021 12:47, Andrew Rybchenko:
> From: Ivan Ilchenko <ivan.ilchenko@oktetlabs.ru>
> 
> In drivers important cleanup could happen on the device stop.
> Do stop in the rte_eth_dev_close() function for robustness and
> to simplify drivers code.
> 
> Signed-off-by: Ivan Ilchenko <ivan.ilchenko@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> ---
> In fact the patch is required to fix segfault in the case of
> net/virtio on close without stop after Rx interrupts enabled.
> 
> I believe that the right way to address the problem is automated
> stop from close, but I guess it cannot not be backported and
> may be fix in a different way required in stable branches.

It is possible to do this addition.
But the right fix (not changing API behaviour) should be to return early
if the port is not stopped.

> @@ -1894,6 +1894,17 @@ rte_eth_dev_close(uint16_t port_id)
>  	dev = &rte_eth_devices[port_id];
>  
>  	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_close, -ENOTSUP);
> +
> +	if (dev->data->dev_started) {
> +		*lasterr = rte_eth_dev_stop(port_id);
> +		if (*lasterr != 0) {
> +			RTE_ETHDEV_LOG(ERR,
> +				"Failed to stop device (port %u) before close: %s - ignore\n",
> +				port_id, rte_strerror(-*lasterr));
> +			lasterr = &binerr;
> +		}
> +	}
> +
>  	*lasterr = (*dev->dev_ops->dev_close)(dev);
>  	if (*lasterr != 0)
>  		lasterr = &binerr;