From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id A498A29CF; Thu, 3 Nov 2016 17:09:09 +0100 (CET) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga104.fm.intel.com with ESMTP; 03 Nov 2016 09:09:09 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,587,1473145200"; d="scan'208";a="1080138931" Received: from yliu-dev.sh.intel.com ([10.239.67.162]) by fmsmga002.fm.intel.com with ESMTP; 03 Nov 2016 09:09:09 -0700 From: Yuanhan Liu To: dev@dpdk.org Date: Fri, 4 Nov 2016 00:09:53 +0800 Message-Id: <1478189400-14606-2-git-send-email-yuanhan.liu@linux.intel.com> X-Mailer: git-send-email 1.9.0 In-Reply-To: <1478189400-14606-1-git-send-email-yuanhan.liu@linux.intel.com> References: <1478189400-14606-1-git-send-email-yuanhan.liu@linux.intel.com> Cc: stable@dpdk.org, Ilya Maximets Subject: [dpdk-dev] [PATCH 1/8] net/virtio: revert "virtio: fix restart" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 03 Nov 2016 16:09:10 -0000 This reverts commit 9a0615af7746 ("virtio: fix restart"); conflict is manually addressed. Kyle reported an issue with above commit qemu-kvm: Guest moved used index from 5 to 1 with following steps, 1) Start my virtio interfaces 2) Send some traffic into/out of the interfaces 3) Stop the interfaces 4) Start the interfaces 5) Send some more traffic And here are some quotes from Kyle's analysis, Prior to the patch, if an interface were stopped then started, without restarting the application, the queues would be left as-is, because hw->started would be set to 1. Now, calling stop sets hw->started to 0, which means the next call to start will "touch the queues". This is the unintended side-effect that causes the problem. Fixes: 9a0615af7746 ("virtio: fix restart") Cc: Jianfeng Tan Cc: Reported-by: Kyle Larose Signed-off-by: Yuanhan Liu --- drivers/net/virtio/virtio_ethdev.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c index b388134..5815875 100644 --- a/drivers/net/virtio/virtio_ethdev.c +++ b/drivers/net/virtio/virtio_ethdev.c @@ -550,9 +550,6 @@ virtio_dev_close(struct rte_eth_dev *dev) PMD_INIT_LOG(DEBUG, "virtio_dev_close"); - if (hw->started == 1) - virtio_dev_stop(dev); - if (hw->cvq) virtio_dev_queue_release(hw->cvq->vq); @@ -560,6 +557,7 @@ virtio_dev_close(struct rte_eth_dev *dev) if (dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC) vtpci_irq_config(hw, VIRTIO_MSI_NO_VECTOR); vtpci_reset(hw); + hw->started = 0; virtio_dev_free_mbufs(dev); virtio_free_queues(dev); } @@ -1296,15 +1294,17 @@ static int eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev) { struct rte_pci_device *pci_dev; + struct virtio_hw *hw = eth_dev->data->dev_private; PMD_INIT_FUNC_TRACE(); if (rte_eal_process_type() == RTE_PROC_SECONDARY) return -EPERM; - /* Close it anyway since there's no way to know if closed */ - virtio_dev_close(eth_dev); - + if (hw->started == 1) { + virtio_dev_stop(eth_dev); + virtio_dev_close(eth_dev); + } pci_dev = eth_dev->pci_dev; eth_dev->dev_ops = NULL; @@ -1543,12 +1543,9 @@ static void virtio_dev_stop(struct rte_eth_dev *dev) { struct rte_eth_link link; - struct virtio_hw *hw = dev->data->dev_private; PMD_INIT_LOG(DEBUG, "stop"); - hw->started = 0; - if (dev->data->dev_conf.intr_conf.lsc) rte_intr_disable(&dev->pci_dev->intr_handle); -- 1.9.0