From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <yuanhan.liu@linux.intel.com>
Received: from mga14.intel.com (mga14.intel.com [192.55.52.115])
by dpdk.org (Postfix) with ESMTP id 46D9A559C
for <dev@dpdk.org>; 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 <yuanhan.liu@linux.intel.com>
To: Tetsuya Mukawa <mukawa@igel.co.jp>
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 <dev.dpdk.org>
List-Unsubscribe: <http://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: <http://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=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 <mukawa@igel.co.jp>
> ---
> 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