From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 46D9A559C for ; Thu, 8 Oct 2015 08:16:07 +0200 (CEST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga103.fm.intel.com with ESMTP; 07 Oct 2015 23:16:06 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,653,1437462000"; d="scan'208";a="787336511" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.66.49]) by orsmga001.jf.intel.com with ESMTP; 07 Oct 2015 23:16:01 -0700 Date: Thu, 8 Oct 2015 14:19:47 +0800 From: Yuanhan Liu To: Tetsuya Mukawa Message-ID: <20151008061947.GC3115@yliu-dev.sh.intel.com> References: <1439977869-28091-1-git-send-email-mukawa@igel.co.jp> <1439977869-28091-2-git-send-email-mukawa@igel.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1439977869-28091-2-git-send-email-mukawa@igel.co.jp> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH 2/3] vhost: Fix RESET_OWNER handling not to close callfd 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, 08 Oct 2015 06:16:08 -0000 On Wed, Aug 19, 2015 at 06:51:08PM +0900, Tetsuya Mukawa wrote: > When RESET_OWNER message is issued, vhost backend shouldn't close > 'callfd', because it will be valid while vhost-user connection > is established. It should be closed when connection is closed. Doesn't it mean the end of connection when RESET_OWNER is received? If you check my latest patch set for enabling vhost mq, you will find free_device() will be invoked immediately once RESET_OWNER signal is received. http://dpdk.org/ml/archives/dev/2015-September/023752.html So, I doubt this patch is necessary. BTW, does these 3 patches fix a real issue, or just some potential issue in your mind? I saw that you mentioned your vhost pmd won't work without those 3 patches, but I never saw you mentioned what issue you fixed exactly. And it'd be good if you can post them in commit log. And, if we encounter same issue, will my above patch fix yours? --yliu > > Signed-off-by: Tetsuya Mukawa > --- > lib/librte_vhost/virtio-net.c | 35 ++++++++++++++++++++++++----------- > 1 file changed, 24 insertions(+), 11 deletions(-) > > diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c > index 144f301..7794e8b 100644 > --- a/lib/librte_vhost/virtio-net.c > +++ b/lib/librte_vhost/virtio-net.c > @@ -189,13 +189,9 @@ cleanup_device(struct virtio_net *dev) > free(dev->mem); > } > > - /* Close any event notifiers opened by device. */ > - if ((int)dev->virtqueue[VIRTIO_RXQ]->callfd >= 0) > - close((int)dev->virtqueue[VIRTIO_RXQ]->callfd); > + /* Close kickfd event notifiers opened by device. */ > if ((int)dev->virtqueue[VIRTIO_RXQ]->kickfd >= 0) > close((int)dev->virtqueue[VIRTIO_RXQ]->kickfd); > - if ((int)dev->virtqueue[VIRTIO_TXQ]->callfd >= 0) > - close((int)dev->virtqueue[VIRTIO_TXQ]->callfd); > if ((int)dev->virtqueue[VIRTIO_TXQ]->kickfd >= 0) > close((int)dev->virtqueue[VIRTIO_TXQ]->kickfd); > } > @@ -206,6 +202,12 @@ cleanup_device(struct virtio_net *dev) > static void > free_device(struct virtio_net_config_ll *ll_dev) > { > + /* close callfd when connection is closed */ > + if ((int)ll_dev->dev.virtqueue[VIRTIO_RXQ]->callfd >= 0) > + close((int)ll_dev->dev.virtqueue[VIRTIO_RXQ]->callfd); > + if ((int)ll_dev->dev.virtqueue[VIRTIO_TXQ]->callfd >= 0) > + close((int)ll_dev->dev.virtqueue[VIRTIO_TXQ]->callfd); > + > /* Free any malloc'd memory */ > rte_free(ll_dev->dev.virtqueue[VIRTIO_RXQ]); > rte_free(ll_dev->dev.virtqueue[VIRTIO_TXQ]); > @@ -241,6 +243,21 @@ rm_config_ll_entry(struct virtio_net_config_ll *ll_dev, > } > } > > + > +/* > + * Rset variables in device structure. > + */ > +static void > +reset_device(struct virtio_net *dev) > +{ > + dev->virtqueue[VIRTIO_RXQ]->kickfd = (eventfd_t)-1; > + dev->virtqueue[VIRTIO_TXQ]->kickfd = (eventfd_t)-1; > + > + /* Backends are set to -1 indicating an inactive device. */ > + dev->virtqueue[VIRTIO_RXQ]->backend = VIRTIO_DEV_STOPPED; > + dev->virtqueue[VIRTIO_TXQ]->backend = VIRTIO_DEV_STOPPED; > +} > + > /* > * Initialise all variables in device structure. > */ > @@ -261,14 +278,10 @@ init_device(struct virtio_net *dev) > memset(dev->virtqueue[VIRTIO_RXQ], 0, sizeof(struct vhost_virtqueue)); > memset(dev->virtqueue[VIRTIO_TXQ], 0, sizeof(struct vhost_virtqueue)); > > - dev->virtqueue[VIRTIO_RXQ]->kickfd = (eventfd_t)-1; > dev->virtqueue[VIRTIO_RXQ]->callfd = (eventfd_t)-1; > - dev->virtqueue[VIRTIO_TXQ]->kickfd = (eventfd_t)-1; > dev->virtqueue[VIRTIO_TXQ]->callfd = (eventfd_t)-1; > > - /* Backends are set to -1 indicating an inactive device. */ > - dev->virtqueue[VIRTIO_RXQ]->backend = VIRTIO_DEV_STOPPED; > - dev->virtqueue[VIRTIO_TXQ]->backend = VIRTIO_DEV_STOPPED; > + reset_device(dev); > } > > /* > @@ -403,7 +416,7 @@ reset_owner(struct vhost_device_ctx ctx) > ll_dev = get_config_ll_entry(ctx); > > cleanup_device(&ll_dev->dev); > - init_device(&ll_dev->dev); > + reset_device(&ll_dev->dev); > > return 0; > } > -- > 2.1.4