[v3] examples/vhost: workaround qemu abort

  Current qemu vhost net ring start has a dependency on feature bit
VHOST_USER_F_PROTOCOL_FEATURES. Without this feature, vhost ring can't
enabled and ioevent fd won't be deleted after vhost device stop. That
will cause qemu abort when reloading driver. Work around qemu issues by
enabling feature bit in vhost user backend.

Signed-off-by: Marvin Liu <yong.liu@intel.com>
  

On Wed, Jul 25, 2018 at 06:25:56PM +0800, Marvin Liu wrote:
> Current qemu vhost net ring start has a dependency on feature bit
> VHOST_USER_F_PROTOCOL_FEATURES. Without this feature, vhost ring can't
> enabled and ioevent fd won't be deleted after vhost device stop. That
> will cause qemu abort when reloading driver. Work around qemu issues by
> enabling feature bit in vhost user backend.
> 
> Signed-off-by: Marvin Liu <yong.liu@intel.com>
> 
> diff --git a/examples/vhost/main.c b/examples/vhost/main.c
> index 2175c1186..8573004dd 100644
> --- a/examples/vhost/main.c
> +++ b/examples/vhost/main.c
> @@ -1520,7 +1520,10 @@ main(int argc, char *argv[])
>  		}
>  
>  		if (builtin_net_driver)
> -			rte_vhost_driver_set_features(file, VIRTIO_NET_FEATURES);
> +			/* Workaround for qemu vhost net device startup */
> +			rte_vhost_driver_set_features(file,
> +				VIRTIO_NET_FEATURES |
> +				1ULL << VHOST_USER_F_PROTOCOL_FEATURES);

I have taken a closer look. I think it's not really right
to just offer this bit like this. Because once this bit is
negotiated, QEMU will ask the builtin_net_driver for the
protocol features it supports. Currently, there is no API
to allow external backends like the builtin_net_driver in
this example to set the protocol features it supports, and
instead, the protocol features VHOST_USER_PROTOCOL_FEATURES
defined in librte_vhost will be returned to QEMU. This will
cause problems because the builtin_net_driver doesn't really
support those protocol features.


>  
>  		if (mergeable == 0) {
>  			rte_vhost_driver_disable_features(file,
> -- 
> 2.17.0
>
  

> -----Original Message-----
> From: Bie, Tiwei
> Sent: Wednesday, July 25, 2018 2:39 PM
> To: Liu, Yong <yong.liu@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v3] examples/vhost: workaround qemu abort
> 
> On Wed, Jul 25, 2018 at 06:25:56PM +0800, Marvin Liu wrote:
> > Current qemu vhost net ring start has a dependency on feature bit
> > VHOST_USER_F_PROTOCOL_FEATURES. Without this feature, vhost ring can't
> > enabled and ioevent fd won't be deleted after vhost device stop. That
> > will cause qemu abort when reloading driver. Work around qemu issues by
> > enabling feature bit in vhost user backend.
> >
> > Signed-off-by: Marvin Liu <yong.liu@intel.com>
> >
> > diff --git a/examples/vhost/main.c b/examples/vhost/main.c
> > index 2175c1186..8573004dd 100644
> > --- a/examples/vhost/main.c
> > +++ b/examples/vhost/main.c
> > @@ -1520,7 +1520,10 @@ main(int argc, char *argv[])
> >  		}
> >
> >  		if (builtin_net_driver)
> > -			rte_vhost_driver_set_features(file, VIRTIO_NET_FEATURES);
> > +			/* Workaround for qemu vhost net device startup */
> > +			rte_vhost_driver_set_features(file,
> > +				VIRTIO_NET_FEATURES |
> > +				1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
> 
> I have taken a closer look. I think it's not really right
> to just offer this bit like this. Because once this bit is
> negotiated, QEMU will ask the builtin_net_driver for the
> protocol features it supports. Currently, there is no API
> to allow external backends like the builtin_net_driver in
> this example to set the protocol features it supports, and
> instead, the protocol features VHOST_USER_PROTOCOL_FEATURES
> defined in librte_vhost will be returned to QEMU. This will
> cause problems because the builtin_net_driver doesn't really
> support those protocol features.
> 

Agreed, just announce protocol feature may have potential problem. Due to protocol features bits is fixed when no vdpa backend existing. IMHO, it has no benefit that adapt builtin_net_driver into vdpa framework. So just updated this sample's doc about builtin_net_driver compatible issue with QEMU.
Please kindly review that patch.

Regards,
Marvin

> 
> >
> >  		if (mergeable == 0) {
> >  			rte_vhost_driver_disable_features(file,
> > --
> > 2.17.0
> >