[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] vhost-user breaks after 96a3d98.
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] vhost-user breaks after 96a3d98. |
Date: |
Wed, 4 Jan 2017 15:10:34 +0200 |
On Wed, Jan 04, 2017 at 11:00:55AM -0200, Flavio Leitner wrote:
> On Wed, 4 Jan 2017 15:52:55 +0800
> Jason Wang <address@hidden> wrote:
>
> > On 2017年01月04日 11:26, Jason Wang wrote:
> > >
> > >
> > > On 2017年01月04日 00:27, Michael S. Tsirkin wrote:
> > >> On Tue, Jan 03, 2017 at 06:28:18PM +0800, Jason Wang wrote:
> > >>>
> > >>> On 2017年01月03日 11:09, Jason Wang wrote:
> > >>>>
> > >>>> On 2016年12月30日 20:41, Flavio Leitner wrote:
> > >>>>> Hi,
> > >>>>>
> > >>>>> While I was testing vhost-user using OVS 2.5 and DPDK 2.2.0 in the
> > >>>>> host and testpmd dpdk 2.2.0 in the guest, I found that the commit
> > >>>>> below breaks the environment and no packets gets into the guest.
> > >>>>>
> > >>>>> dpdk port --> OVS --> vhost-user --> guest --> testpmd
> > >>>>> ^--- drops here ^--- no packets
> > >>>>> here.
> > >>>>>
> > >>>>> commit 96a3d98d2cdbd897ff5ab33427aa4cfb94077665
> > >>>>> Author: Jason Wang <address@hidden>
> > >>>>> Date: Mon Aug 1 16:07:58 2016 +0800
> > >>>>>
> > >>>>> vhost: don't set vring call if no vector
> > >>>>> We used to set vring call fd unconditionally even if guest
> > >>>>> driver does
> > >>>>> not use MSIX for this vritqueue at all. This will cause lots of
> > >>>>> unnecessary userspace access and other checks for drivers does
> > >>>>> not use
> > >>>>> interrupt at all (e.g virtio-net pmd). So check and clean vring
> > >>>>> call
> > >>>>> fd if guest does not use any vector for this virtqueue at
> > >>>>> all.
> > >>>>> [...]
> > >>>>>
> > >>>>> Thanks,
> > >>>> Hi Flavio:
> > >>>>
> > >>>> Thanks for reporting this issue, could this be a bug of vhost-user? (I
> > >>>> believe virito-net pmd does not use interrupt for rx/tx at all)
> > >>>>
> > >>>> Anyway, will try to reproduce it.
> > >>>>
> > >>> Could not reproduce this issue on similar setups (the only
> > >>> difference is I
> > >>> don't create dpdk port) with dpdk 16.11 and ovs.git HEAD. Suspect an
> > >>> issue
> > >>> dpdk. Will try OVS 2.5 + DPDK 2.2.0.
> > >>>
> > >>> Thanks
> > >> Possibly dpdk assumed that call fd must be present unconditionally.
> > >> Limit this patch to when protocol is updated? add a new protocol flag?
> > >
> > > If this is a bug of dpdk, I tend to fix it (or just disable this patch
> > > for vhost-user). I'm not sure whether or not it's worthwhile to add a
> > > new protocol flag which was used to tell qemu that bug X was fixed.
> > >
> > > Thanks
> > >
> > >
> >
> > Haven't tried but looking at vq_is_ready() in v2.2.0:
> >
> > static int
> > vq_is_ready(struct vhost_virtqueue *vq)
> > {
> > return vq && vq->desc &&
> > vq->kickfd != -1 &&
> > vq->callfd != -1;
> > }
> >
> > Which assumes callfd must be set which seems wrong. And this has been
> > fixed by
> >
> > commit fb871d0a4dc1c038a381c524cdb86fe83d21d842
> > Author: Tetsuya Mukawa <address@hidden>
> > Date: Mon Mar 14 17:53:32 2016 +0900
> >
> > vhost: fix default value of kickfd and callfd
> >
> > Currently, default values of kickfd and callfd are -1.
> > If the values are -1, current code guesses kickfd and callfd haven't
> > been initialized yet. Then vhost library will guess the virtqueue isn't
> > ready for processing.
> >
> > But callfd and kickfd will be set as -1 when "--enable-kvm"
> > isn't specified in QEMU command line. It means we cannot treat -1 as
> > uninitialized state.
> >
> > The patch defines -1 and -2 as VIRTIO_INVALID_EVENTFD and
> > VIRTIO_UNINITIALIZED_EVENTFD, and uses VIRTIO_UNINITIALIZED_EVENTFD for
> > the default values of kickfd and callfd.
> >
> > Signed-off-by: Tetsuya Mukawa <address@hidden>
> > Acked-by: Yuanhan Liu <address@hidden>
> >
> > Flavio, you could try to backport this to 2.2.0 to see if it fixes your
> > issue.
>
> Yup, that patch does fix the problem in my environment, thanks a lot!
>
> Unfortunately this fix cannot be backported in upstream because DPDK 2.2.0
> doesn't have a LTS branch. Perhaps we don't care because 2.2.0 is too old
> and 16.04 is fixed? Not sure.
>
> --
> Flavio
I wonder how common is this bug though. What does e.g. snabbswitch do?
We can work around that in QEMU if we do setup some value
if VHOST_USER_F_PROTOCOL_FEATURES is not negotiated.
Not sure it's worth it.
--
MST
Re: [Qemu-devel] vhost-user breaks after 96a3d98., Flavio Leitner, 2017/01/03