From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tetsuya Mukawa Subject: Re: [PATCH] vhost: Fix reset_owner message handling not to clear callfd Date: Thu, 19 Nov 2015 16:12:11 +0900 Message-ID: <564D764B.3070003@igel.co.jp> References: <1447914206-18369-1-git-send-email-mukawa@igel.co.jp> <20151119070320.GK2326@yliu-dev.sh.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org, ann.zhuangyanying@huawei.com To: Yuanhan Liu Return-path: Received: from mail-pa0-f54.google.com (mail-pa0-f54.google.com [209.85.220.54]) by dpdk.org (Postfix) with ESMTP id 39D7D2EDA for ; Thu, 19 Nov 2015 08:12:15 +0100 (CET) Received: by padhx2 with SMTP id hx2so71997960pad.1 for ; Wed, 18 Nov 2015 23:12:14 -0800 (PST) In-Reply-To: <20151119070320.GK2326@yliu-dev.sh.intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 2015/11/19 16:03, Yuanhan Liu wrote: > On Thu, Nov 19, 2015 at 03:23:26PM +0900, Tetsuya Mukawa wrote: >> The patch fixes reset_owner message handling not to clear callfd, >> because callfd will be valid while connection is establihed. >> >> Signed-off-by: Tetsuya Mukawa >> --- >> lib/librte_vhost/virtio-net.c | 21 ++++++++++++++++++++- >> 1 file changed, 20 insertions(+), 1 deletion(-) >> >> diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c >> index 886c104..ae1e4bd 100644 >> --- a/lib/librte_vhost/virtio-net.c >> +++ b/lib/librte_vhost/virtio-net.c >> @@ -322,6 +322,25 @@ init_vring_queue_pair(struct virtio_net *dev, uint32_t qp_idx) >> init_vring_queue(dev->virtqueue[base_idx + VIRTIO_TXQ], qp_idx); >> } >> >> +static void >> +reset_vring_queue(struct vhost_virtqueue *vq, int qp_idx) >> +{ >> + int callfd; >> + >> + callfd = vq->callfd; >> + init_vring_queue(vq, qp_idx); >> + vq->callfd = callfd; > It may fix your issue, but it's not a proper fix. As we actually closed > callfd at cleanup_vq(). Yes, you are correct. We also need to fix above. > > BTW, the name, cleanup_vq/device, is not well taken. For a name like > cleanup, those functions should be invoked at device remove stage (say, > shutting down a VM), but not at reset stage (which is actually treated > as STOP in current QEMU implementation). > > I have a plan to correct it, but I guess it's not the right time as > v2.2 is coming soon. However, for a fix, it could be simple: how about > adding another arg to cleanup_vq: > > cleanup_vq(struct vhost_virtqueue *vq, int destory) > > And we only close callfd when destroy flag is set. (And of course, this > patch is needed). Yes I agree with it! Let me have a few hours, I will send it by today. Thanks, Tetsuya > What do you think of that? > > --yliu >> +} >> + >> +static void >> +reset_vring_queue_pair(struct virtio_net *dev, uint32_t qp_idx) >> +{ >> + uint32_t base_idx = qp_idx * VIRTIO_QNUM; >> + >> + reset_vring_queue(dev->virtqueue[base_idx + VIRTIO_RXQ], qp_idx); >> + reset_vring_queue(dev->virtqueue[base_idx + VIRTIO_TXQ], qp_idx); >> +} >> + >> static int >> alloc_vring_queue_pair(struct virtio_net *dev, uint32_t qp_idx) >> { >> @@ -362,7 +381,7 @@ reset_device(struct virtio_net *dev) >> dev->flags = 0; >> >> for (i = 0; i < dev->virt_qp_nb; i++) >> - init_vring_queue_pair(dev, i); >> + reset_vring_queue_pair(dev, i); >> } >> >> /* >> -- >> 2.1.4