[dpdk-dev] [PATCH v2] vhost: prevent disabled rings to be processed with zero-copy

[dpdk-dev] [PATCH v2] vhost: prevent disabled rings to be processed with zero-copy

[Maxime Coquelin]

The vhost-user spec says that once the vring is disabled, the
client has to stop processing it. But it can happen when
dequeue zero-copy is enabled if outstanding descriptors buffers
are still being processed by an external NIC or another guest.

The fix consists in draining the zmbufs list to ensure no more
descriptors buffers are in the wild.

Note that this fix is only working in the case REPLY_ACK
protocol feature is enabled, which is not the case by default
for now (it is only enabled when IOMMU feature is enabled in
the vhost library).

Fixes: b0a985d1f340 ("vhost: add dequeue zero copy")
Cc: stable@dpdk.org

Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost_user.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 36c0c676d..555d09ad9 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -1345,6 +1345,10 @@ vhost_user_set_vring_enable(struct virtio_net **pdev,
 		dev->notify_ops->vring_state_changed(dev->vid,
 				index, enable);
 
+	/* On disable, rings have to be stopped being processed. */
+	if (!enable && dev->dequeue_zero_copy)
+		drain_zmbuf_list(dev->virtqueue[index]);
+
 	dev->virtqueue[index]->enabled = enable;
 
 	return RTE_VHOST_MSG_RESULT_OK;
-- 
2.20.1

Re: [dpdk-dev] [PATCH v2] vhost: prevent disabled rings to be processed with zero-copy

[Maxime Coquelin]

On 2/28/19 6:57 PM, Maxime Coquelin wrote:
> The vhost-user spec says that once the vring is disabled, the
> client has to stop processing it. But it can happen when
> dequeue zero-copy is enabled if outstanding descriptors buffers
> are still being processed by an external NIC or another guest.
> 
> The fix consists in draining the zmbufs list to ensure no more
> descriptors buffers are in the wild.
> 
> Note that this fix is only working in the case REPLY_ACK
> protocol feature is enabled, which is not the case by default
> for now (it is only enabled when IOMMU feature is enabled in
> the vhost library).
> 
> Fixes: b0a985d1f340 ("vhost: add dequeue zero copy")
> Cc: stable@dpdk.org
> 
> Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>   lib/librte_vhost/vhost_user.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> index 36c0c676d..555d09ad9 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -1345,6 +1345,10 @@ vhost_user_set_vring_enable(struct virtio_net **pdev,
>   		dev->notify_ops->vring_state_changed(dev->vid,
>   				index, enable);
>   
> +	/* On disable, rings have to be stopped being processed. */
> +	if (!enable && dev->dequeue_zero_copy)
> +		drain_zmbuf_list(dev->virtqueue[index]);
> +
>   	dev->virtqueue[index]->enabled = enable;
>   
>   	return RTE_VHOST_MSG_RESULT_OK;
> 

Applied to dpdk-next-virtio/master.

Maxime

Re: [dpdk-dev] [PATCH v2] vhost: prevent disabled rings to be processed with zero-copy

[Maxime Coquelin]

On 2/28/19 6:57 PM, Maxime Coquelin wrote:
> The vhost-user spec says that once the vring is disabled, the
> client has to stop processing it. But it can happen when
> dequeue zero-copy is enabled if outstanding descriptors buffers
> are still being processed by an external NIC or another guest.
> 
> The fix consists in draining the zmbufs list to ensure no more
> descriptors buffers are in the wild.
> 
> Note that this fix is only working in the case REPLY_ACK
> protocol feature is enabled, which is not the case by default
> for now (it is only enabled when IOMMU feature is enabled in
> the vhost library).
> 
> Fixes: b0a985d1f340 ("vhost: add dequeue zero copy")
> Cc: stable@dpdk.org
> 
> Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>   lib/librte_vhost/vhost_user.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> index 36c0c676d..555d09ad9 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -1345,6 +1345,10 @@ vhost_user_set_vring_enable(struct virtio_net **pdev,
>   		dev->notify_ops->vring_state_changed(dev->vid,
>   				index, enable);
>   
> +	/* On disable, rings have to be stopped being processed. */
> +	if (!enable && dev->dequeue_zero_copy)
> +		drain_zmbuf_list(dev->virtqueue[index]);
> +
>   	dev->virtqueue[index]->enabled = enable;
>   
>   	return RTE_VHOST_MSG_RESULT_OK;
> 

Applied to dpdk-next-virtio/master.

Maxime