From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 7FB93A0563; Wed, 15 Apr 2020 09:28:26 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id DDAB41D163; Wed, 15 Apr 2020 09:28:25 +0200 (CEST) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 0683A2AB for ; Wed, 15 Apr 2020 09:28:23 +0200 (CEST) IronPort-SDR: 5LayMhuvGd3IL7PHbmyM9R4kxa37TcE3IfjgBfnPFBngac+l8TG4b1J33AC6fyBvIRaWHLb4Yd N86zV4clV4Gg== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Apr 2020 00:28:22 -0700 IronPort-SDR: p6bDVKJHjdUFKy6eOdqEH92Z6CZDP6C4DwPbfeucrsMqtMbWNMBiSn6SntJPYz5iDsvpoB5ybh TrmSewpTmj5Q== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,386,1580803200"; d="scan'208";a="277530388" Received: from yexl-server.sh.intel.com (HELO localhost) ([10.67.117.17]) by fmsmga004.fm.intel.com with ESMTP; 15 Apr 2020 00:28:20 -0700 Date: Wed, 15 Apr 2020 15:24:17 +0800 From: Ye Xiaolong To: Marvin Liu Cc: maxime.coquelin@redhat.com, zhihong.wang@intel.com, dev@dpdk.org, xuan.ding@intel.com Message-ID: <20200415072417.GB33551@intel.com> References: <20200414125555.86601-1-yong.liu@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200414125555.86601-1-yong.liu@intel.com> User-Agent: Mutt/1.9.4 (2018-02-28) Subject: Re: [dpdk-dev] [PATCH] net/virtio: fix crash when device reconnecting X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi, Marvin On 04/14, Marvin Liu wrote: >When doing virtio device initialization, virtqueues will be reset in >server mode if ring type is packed. This will cause issue because queues >have been freed in the beginning of device initialization. > >Fix this issue by splitting device initial process and device reinit >process. Virt queues won't be freed or realloc in reinit process. Also >moved virtio device initialization from configuration to start stage, >which can reduce number of reinitialization times. > >Fixes: 6ebbf4109f35 ("net/virtio-user: fix packed ring server mode") I think it also needs to cc stable. > >Signed-off-by: Marvin Liu > >diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c >index 21570e5cf..8c84bfe91 100644 >--- a/drivers/net/virtio/virtio_ethdev.c >+++ b/drivers/net/virtio/virtio_ethdev.c >@@ -1670,7 +1670,9 @@ virtio_configure_intr(struct rte_eth_dev *dev) > > /* reset device and renegotiate features if needed */ > static int >-virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features) >+virtio_init_device(struct rte_eth_dev *eth_dev, >+ uint64_t req_features, >+ bool reinit) > { > struct virtio_hw *hw = eth_dev->data->dev_private; > struct virtio_net_config *config; >@@ -1681,7 +1683,7 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features) > /* Reset the device although not necessary at startup */ > vtpci_reset(hw); > >- if (hw->vqs) { >+ if (hw->vqs && !reinit) { > virtio_dev_free_mbufs(eth_dev); > virtio_free_queues(hw); > } >@@ -1794,9 +1796,11 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features) > VLAN_TAG_LEN - hw->vtnet_hdr_size; > } > >- ret = virtio_alloc_queues(eth_dev); >- if (ret < 0) >- return ret; >+ if (!reinit) { >+ ret = virtio_alloc_queues(eth_dev); >+ if (ret < 0) >+ return ret; >+ } > > if (eth_dev->data->dev_conf.intr_conf.rxq) { > if (virtio_configure_intr(eth_dev) < 0) { >@@ -1925,7 +1929,8 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev) > rte_spinlock_init(&hw->state_lock); > > /* reset device and negotiate default features */ >- ret = virtio_init_device(eth_dev, VIRTIO_PMD_DEFAULT_GUEST_FEATURES); >+ ret = virtio_init_device(eth_dev, VIRTIO_PMD_DEFAULT_GUEST_FEATURES, >+ false); > if (ret < 0) > goto err_virtio_init; > >@@ -2091,12 +2096,6 @@ virtio_dev_configure(struct rte_eth_dev *dev) > return -EINVAL; > } > >- if (dev->data->dev_conf.intr_conf.rxq) { >- ret = virtio_init_device(dev, hw->req_guest_features); >- if (ret < 0) >- return ret; >- } >- > if (rxmode->max_rx_pkt_len > hw->max_mtu + ether_hdr_len) > req_features &= ~(1ULL << VIRTIO_NET_F_MTU); > >@@ -2120,7 +2119,7 @@ virtio_dev_configure(struct rte_eth_dev *dev) > > /* if request features changed, reinit the device */ > if (req_features != hw->req_guest_features) { >- ret = virtio_init_device(dev, req_features); >+ ret = virtio_negotiate_features(hw, req_features); Why do we need to change virtio_init_device to virtio_negotiate_features here? Thanks, Xiaolong > if (ret < 0) > return ret; > } >@@ -2235,6 +2234,11 @@ virtio_dev_start(struct rte_eth_dev *dev) > struct virtio_hw *hw = dev->data->dev_private; > int ret; > >+ /* reinit the device */ >+ ret = virtio_init_device(dev, hw->req_guest_features, true); >+ if (ret < 0) >+ return ret; >+ > /* Finish the initialization of the queues */ > for (i = 0; i < dev->data->nb_rx_queues; i++) { > ret = virtio_dev_rx_queue_setup_finish(dev, i); >-- >2.17.1 >