Re: [PATCH v2 3/3] vduse: Temporarily disable control queue features

[Maxime Coquelin]

On 7/4/23 18:43, Michael S. Tsirkin wrote:
> On Tue, Jul 04, 2023 at 06:40:45PM +0200, Maxime Coquelin wrote:
> > Virtio-net driver control queue implementation is not safe
> > when used with VDUSE. If the VDUSE application does not
> > reply to control queue messages, it currently ends up
> > hanging the kernel thread sending this command.
> >
> > Some work is on-going to make the control queue
> > implementation robust with VDUSE. Until it is completed,
> > let's disable control virtqueue and features that depend on
> > it.
> >
> > Signed-off-by: Maxime Coquelin <maxime.coquelin@xxxxxxxxxx>
> > ---
> >   drivers/vdpa/vdpa_user/vduse_dev.c | 21 +++++++++++++++++++++
> >   1 file changed, 21 insertions(+)
> >
> > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > index 1271c9796517..04367a53802b 100644
> > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > @@ -1778,6 +1778,25 @@ static struct attribute *vduse_dev_attrs[] = {
> >   
> >   ATTRIBUTE_GROUPS(vduse_dev);
> >   
> > +static void vduse_dev_features_fixup(struct vduse_dev_config *config)
> > +{
> > +	if (config->device_id == VIRTIO_ID_NET) {
> > +		/*
> > +		 * Temporarily disable control virtqueue and features that
> > +		 * depend on it while CVQ is being made more robust for VDUSE.
> > +		 */
> > +		config->features &= ~((1ULL << VIRTIO_NET_F_CTRL_VQ) |
> > +				(1ULL << VIRTIO_NET_F_CTRL_RX) |
> > +				(1ULL << VIRTIO_NET_F_CTRL_VLAN) |
> > +				(1ULL << VIRTIO_NET_F_GUEST_ANNOUNCE) |
> > +				(1ULL << VIRTIO_NET_F_MQ) |
> > +				(1ULL << VIRTIO_NET_F_CTRL_MAC_ADDR) |
> > +				(1ULL << VIRTIO_NET_F_RSS) |
> > +				(1ULL << VIRTIO_NET_F_HASH_REPORT) |
> > +				(1ULL << VIRTIO_NET_F_NOTF_COAL));
> > +	}
> > +}
> > +
>
>
> This will never be exhaustive, we are adding new features.
> Please add an allowlist with just legal ones instead.

Ok, got it!
I'll post a new revision.

Thanks,
Maxime

> >   static int vduse_create_dev(struct vduse_dev_config *config,
> >   			    void *config_buf, u64 api_version)
> >   {
> > @@ -1793,6 +1812,8 @@ static int vduse_create_dev(struct vduse_dev_config *config,
> >   	if (!dev)
> >   		goto err;
> >   
> > +	vduse_dev_features_fixup(config);
> > +
> >   	dev->api_version = api_version;
> >   	dev->device_features = config->features;
> >   	dev->device_id = config->device_id;
> > --
> > 2.41.0