From mboxrd@z Thu Jan  1 00:00:00 1970
From: Yuanhan Liu <yliu@fridaylinux.org>
Subject: Re: [PATCH] app/testpmd: fix failsafe PMD failure on exit
Date: Sun, 27 May 2018 12:06:30 +0800
Message-ID: <20180527040630.x3wrjsblyqbxk3bz@yuanhanliu-NB0.tencent.com>
References: <20180522183509.66644-1-ferruh.yigit@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Wenzhuo Lu <wenzhuo.lu@intel.com>, Jingjing Wu <jingjing.wu@intel.com>,
 dev@dpdk.org, Zhiyong Yang <zhiyong.yang@intel.com>,
 Bernard Iremonger <bernard.iremonger@intel.com>,
 Thomas Monjalon <thomas@monjalon.net>,
 Maxime Coquelin <maxime.coquelin@redhat.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com
 [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id CCDEF23A
 for <dev@dpdk.org>; Sun, 27 May 2018 06:06:55 +0200 (CEST)
Content-Disposition: inline
In-Reply-To: <20180522183509.66644-1-ferruh.yigit@intel.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.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