Re: [Qemu-devel] vhost-user breaks after 96a3d98.

[Flavio Leitner <fbl@redhat.com>]

On Wed, 4 Jan 2017 15:52:55 +0800
Jason Wang <jasowang@redhat.com> 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 <jasowang@redhat.com>
> >>>>> 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 <mukawa@igel.co.jp>
> 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 <mukawa@igel.co.jp>
>      Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> 
> 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