diff mbox

[dpdk-dev,RFC,23/24] WORKAROUND revert virtio-net mq vring deletion

Message ID 20180119134444.24927-24-stefanha@redhat.com (mailing list archive)
State RFC, archived
Delegated to: Maxime Coquelin
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply patch file failure

Commit Message

Stefan Hajnoczi Jan. 19, 2018, 1:44 p.m. UTC 
  The virtio-net mq vring deletion code should be in virtio_net.c, not in
the generic vhost_user.c code where it breaks non-virtio-net devices.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 drivers/librte_vhost/vhost_user.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Maxime Coquelin Jan. 30, 2018, 5:52 p.m. UTC | #1 
Hi Stefan,

On 01/19/2018 02:44 PM, Stefan Hajnoczi wrote:
> The virtio-net mq vring deletion code should be in virtio_net.c, not in
> the generic vhost_user.c code where it breaks non-virtio-net devices.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   drivers/librte_vhost/vhost_user.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/librte_vhost/vhost_user.c b/drivers/librte_vhost/vhost_user.c
> index a819684b4..08fab933b 100644
> --- a/drivers/librte_vhost/vhost_user.c
> +++ b/drivers/librte_vhost/vhost_user.c
> @@ -163,6 +163,7 @@ vhost_user_set_features(struct virtio_net *dev, uint64_t features)
>   		(dev->features & (1 << VIRTIO_NET_F_MRG_RXBUF)) ? "on" : "off",
>   		(dev->features & (1ULL << VIRTIO_F_VERSION_1)) ? "on" : "off");
>   
> +#if 0
>   	if (!(dev->features & (1ULL << VIRTIO_NET_F_MQ))) {
>   		/*
>   		 * Remove all but first queue pair if MQ hasn't been
> @@ -181,6 +182,7 @@ vhost_user_set_features(struct virtio_net *dev, uint64_t features)
>   			free_vq(vq);
>   		}
>   	}
> +#endif
>   
>   	return 0;
>   }
> 

Thanks for reporting the issue.
It seems difficult to move this check in virtio-net.c without a deep
rework.

But I think we can workaround by ensuring the backend supports
VIRTIO_NET_F_MQ, but it has not been negotiated.
Something like:

if ((vhost_features & (1ULL << VIRTIO_NET_F_MQ)) &&
	!(dev->features & (1ULL << VIRTIO_NET_F_MQ)) {
...
}

Any thoughts?

Thanks,
Maxime
diff mbox

Patch

diff --git a/drivers/librte_vhost/vhost_user.c b/drivers/librte_vhost/vhost_user.c
index a819684b4..08fab933b 100644
--- a/drivers/librte_vhost/vhost_user.c
+++ b/drivers/librte_vhost/vhost_user.c
@@ -163,6 +163,7 @@  vhost_user_set_features(struct virtio_net *dev, uint64_t features)
 		(dev->features & (1 << VIRTIO_NET_F_MRG_RXBUF)) ? "on" : "off",
 		(dev->features & (1ULL << VIRTIO_F_VERSION_1)) ? "on" : "off");
 
+#if 0
 	if (!(dev->features & (1ULL << VIRTIO_NET_F_MQ))) {
 		/*
 		 * Remove all but first queue pair if MQ hasn't been
@@ -181,6 +182,7 @@  vhost_user_set_features(struct virtio_net *dev, uint64_t features)
 			free_vq(vq);
 		}
 	}
+#endif
 
 	return 0;
 }