From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx6-phx2.redhat.com (mx6-phx2.redhat.com [209.132.183.39]) by dpdk.org (Postfix) with ESMTP id 59E2C47D2 for ; Mon, 9 May 2016 12:45:03 +0200 (CEST) Received: from zmail26.collab.prod.int.phx2.redhat.com (zmail26.collab.prod.int.phx2.redhat.com [10.5.83.33]) by mx6-phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u49Aj2D5016435; Mon, 9 May 2016 06:45:02 -0400 Date: Mon, 9 May 2016 06:45:02 -0400 (EDT) From: Victor Kaplansky To: Yuanhan Liu Cc: dev@dpdk.org, huawei xie , "Michael S. Tsirkin" Message-ID: <1245742132.28022376.1462790702229.JavaMail.zimbra@redhat.com> In-Reply-To: <1462603224-29510-5-git-send-email-yuanhan.liu@linux.intel.com> References: <1462603224-29510-1-git-send-email-yuanhan.liu@linux.intel.com> <1462603224-29510-5-git-send-email-yuanhan.liu@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [10.35.203.67] X-Mailer: Zimbra 8.0.6_GA_5922 (ZimbraWebClient - GC43 (Linux)/8.0.6_GA_5922) Thread-Topic: vhost: workaround stale vring base Thread-Index: /LTKcBL5bNGuufo1D2wplqH0hwEjCQ== Subject: Re: [dpdk-dev] [PATCH 4/6] vhost: workaround stale vring base 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: Mon, 09 May 2016 10:45:03 -0000 ----- Original Message ----- > From: "Yuanhan Liu" > To: dev@dpdk.org > Cc: "huawei xie" , "Yuanhan Liu" , "Michael S. Tsirkin" > > Sent: Saturday, May 7, 2016 9:40:22 AM > Subject: [dpdk-dev] [PATCH 4/6] vhost: workaround stale vring base > > When DPDK app crashes (or quits, or gets killed), and when QEMU supports > reconnecting (patches have been sent, not merged yet), a restart of DPDK > app would get stale vring base from QEMU. That would break the kernel > virtio net completely, making it non-work any more, unless a driver reset > is done. > > So, instead of getting the stale vring base from QEMU, Huawei suggested > we could get a proper one from used->idx. > > Cc: "Michael S. Tsirkin" > Suggested-by: "Xie, Huawei" > Signed-off-by: Yuanhan Liu > --- > > Note that a right way to handle reconnect from abnormal quit would be > let the guest driver to initiate a reset when reconnect is detected > from QEMU. As a reset from the virtio net driver would resets all virtio > related meta datas, and trigger the vhost-user re-initiation again, > therefore, it would make the reconnect work as expected. > > What's unforunate is that driver reset on reconnect, as the term "driver" > implies, need driver hackings, which could be a linux kernel virtio net > driver, DPDK pmd driver, or even, the windows driver. Apparently, it will > not work so far, or even for a long while, until we are adapted to the > new kernel. > > The fix (or more precisely, the workaround) from this patch would make > reconnect work in most case, including the following two hypothesis that > might corrupt virtio memory: > > - vring_used_elem might be in stale state when we are in the middle of > updating used vring. Say, we might have updated the "id" field, but > leaving "len" untouched. > > - vring desc buff might also be in stale state, when we are copying > data into it. > > The reason it still works is that we haven't updated used->idx yet, > which means guest driver will not start processing those buggy used > entries. Therefore, no issues. > > However, Michael claims some concerns: he made a good point: a crash > is happening means some memory is corrupted, and it could be the virtio > memory being corrupted. In such case, nothing will work without the > reset. > > But I would say, that an app in product use would rare crash, and even > if it crashes, the chance that virtio memory being corrupted would be > relatively smaller then. Besides that, it would work, say when the app > is killed by ctrl-c or kill command. So, it works in the most cases. > But still, I would say it's just a workaround so far. > > On the other hand, without this patch, it would be 100% not working > from abnormal quit. But with this patch, it works in most cases, just > don't work in a rare case that when virtio memory is corrupted. Therefore, > I'd say, it is still good to have, until we have a perfect solution. > --- > lib/librte_vhost/virtio-net.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c > index c88aaa3..df103aa 100644 > --- a/lib/librte_vhost/virtio-net.c > +++ b/lib/librte_vhost/virtio-net.c > @@ -560,6 +560,14 @@ vhost_set_vring_addr(int vid, struct vhost_vring_addr > *addr) > return -1; > } > > + if (vq->last_used_idx != vq->used->idx) { > + RTE_LOG(WARNING, VHOST_CONFIG, > + "last_used_idx (%u) and vq->used->idx (%u) mismatch\n", I agree with this approach. I just would add to the log message that last_user_idx have overrode by used_idx and some packets may be dropped. > + vq->last_used_idx, vq->used->idx); > + vq->last_used_idx = vq->used->idx; > + vq->last_used_idx_res = vq->used->idx; > + } > + > vq->log_guest_addr = addr->log_guest_addr; > > LOG_DEBUG(VHOST_CONFIG, "(%d) mapped address desc: %p\n", > -- > 1.9.0 > >