Re: [dpdk-dev] [PATCH] app/testpmd: fix failsafe PMD failure on exit

[Yuanhan Liu <yliu@fridaylinux.org>]

On Tue, May 22, 2018 at 07:35:08PM +0100, Ferruh Yigit wrote:
> vdevs detach on testpmd exit implemented as workaround to fix
> a virtio-user issue. The issue was virtio-user cleanup is not
> called and existing socket file not cleaned up which will fail
> next run.
> 
> The vdev cleanup causing problems in failsafe PMD.
> 
> Reduce the cleanup to only virtio-user and add a comment that this
> workaround should be converted to a proper cleanup, not something
> specific to virtio-user, and not something specific to vdev and
> testpmd.
> 
> Fixes: fe890955114d ("app/testpmd: fix exit for vdevs")
> 
...
>  pmd_test_exit(void)
>  {
> -	const struct rte_bus *bus;
>  	struct rte_device *device;
>  	portid_t pt_id;
>  	int ret;
> @@ -2025,13 +2024,21 @@ pmd_test_exit(void)
>  	if (ports != NULL) {
>  		no_link_check = 1;
>  		RTE_ETH_FOREACH_DEV(pt_id) {
> -			device = rte_eth_devices[pt_id].device;
> -			bus = rte_bus_find_by_device(device);
>  			printf("\nShutting down port %d...\n", pt_id);
>  			fflush(stdout);
>  			stop_port(pt_id);
>  			close_port(pt_id);
> -			if (bus && !strcmp(bus->name, "vdev"))
> +
> +			/*
> +			 * This is a workaround to fix a virtio-user issue that
> +			 * requires to call clean-up routine to remove existing
> +			 * socket.

I came across this patch while I was cherry-picking patches to 17.11.4
release. And this patch seems wrong to me.

Any particular reason why the socket removal can not be done in virtio-user
pmd, say at its close method?

	--yliu
> +			 * This workaround valid only for testpmd, needs a fix
> +			 * valid for all applications.
> +			 * TODO: Implement proper resource cleanup
> +			 */
> +			device = rte_eth_devices[pt_id].device;
> +			if (device && !strcmp(device->driver->name, "net_virtio_user"))
>  				detach_port(pt_id);
>  		}
>  	}
> -- 
> 2.14.3