[PATCH 12/16] vhost-net: introduce restart and stop for vhost_net's vqs

Kangjie Xu posted 16 patches 3 years, 6 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com>
[PATCH 12/16] vhost-net: introduce restart and stop for vhost_net's vqs
Posted by Kangjie Xu 3 years, 6 months ago
Introduce vhost_virtqueue_restart(), which can restart the
virtqueue when the vhost net started running before.

Introduce vhost_virtqueue_stop(), which can disable the vq
and unmap vrings and the desc of the vq. When disabling the
vq, the function is blocked and waits for a reply.

Combining the two functions, we can reset a virtqueue with a
started vhost net.

Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 hw/net/vhost_net.c      | 55 +++++++++++++++++++++++++++++++++++++++++
 include/net/vhost_net.h |  5 ++++
 2 files changed, 60 insertions(+)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index ccac5b7a64..4f5f034c11 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -514,3 +514,58 @@ int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu)
 
     return vhost_ops->vhost_net_set_mtu(&net->dev, mtu);
 }
+
+void vhost_virtqueue_stop(VirtIODevice *vdev, NetClientState *nc,
+                          int vq_index)
+{
+    VHostNetState *net = get_vhost_net(nc->peer);
+    const VhostOps *vhost_ops = net->dev.vhost_ops;
+    int r;
+
+    assert(vhost_ops);
+
+    r = vhost_ops->vhost_set_single_vring_enable(&net->dev, vq_index, 0, true);
+    if (r < 0) {
+        goto err_queue_disable;
+    }
+
+    vhost_dev_virtqueue_release(&net->dev, vdev, vq_index);
+
+    return;
+
+err_queue_disable:
+    error_report("Error when releasing the qeuue.");
+}
+
+int vhost_virtqueue_restart(VirtIODevice *vdev, NetClientState *nc,
+                            int vq_index)
+{
+    VHostNetState *net = get_vhost_net(nc->peer);
+    const VhostOps *vhost_ops = net->dev.vhost_ops;
+    int r;
+
+    if (!net->dev.started) {
+        return 0;
+    }
+
+    assert(vhost_ops);
+
+    r = vhost_dev_virtqueue_restart(&net->dev, vdev, vq_index);
+    if (r < 0) {
+        goto err_start;
+    }
+
+    r = vhost_ops->vhost_set_single_vring_enable(&net->dev, vq_index, 1,
+                                                 false);
+    if (r < 0) {
+        goto err_start;
+    }
+
+    return 0;
+
+err_start:
+    error_report("Error when restarting the queue.");
+    vhost_dev_stop(&net->dev, vdev);
+
+    return r;
+}
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index 387e913e4e..fcb09e36ef 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -48,4 +48,9 @@ uint64_t vhost_net_get_acked_features(VHostNetState *net);
 
 int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu);
 
+void vhost_virtqueue_stop(VirtIODevice *vdev, NetClientState *nc,
+                          int vq_index);
+int vhost_virtqueue_restart(VirtIODevice *vdev, NetClientState *nc,
+                            int vq_index);
+
 #endif
-- 
2.32.0
Re: [PATCH 12/16] vhost-net: introduce restart and stop for vhost_net's vqs
Posted by Jason Wang 3 years, 6 months ago
在 2022/7/18 19:17, Kangjie Xu 写道:
> Introduce vhost_virtqueue_restart(), which can restart the
> virtqueue when the vhost net started running before.
>
> Introduce vhost_virtqueue_stop(), which can disable the vq
> and unmap vrings and the desc of the vq. When disabling the
> vq, the function is blocked and waits for a reply.
>
> Combining the two functions, we can reset a virtqueue with a
> started vhost net.
>
> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
>   hw/net/vhost_net.c      | 55 +++++++++++++++++++++++++++++++++++++++++
>   include/net/vhost_net.h |  5 ++++
>   2 files changed, 60 insertions(+)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index ccac5b7a64..4f5f034c11 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -514,3 +514,58 @@ int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu)
>   
>       return vhost_ops->vhost_net_set_mtu(&net->dev, mtu);
>   }
> +
> +void vhost_virtqueue_stop(VirtIODevice *vdev, NetClientState *nc,
> +                          int vq_index)


Let's rename this as vhost_net_virtqueue_stop()


> +{
> +    VHostNetState *net = get_vhost_net(nc->peer);
> +    const VhostOps *vhost_ops = net->dev.vhost_ops;
> +    int r;
> +
> +    assert(vhost_ops);
> +
> +    r = vhost_ops->vhost_set_single_vring_enable(&net->dev, vq_index, 0, true);
> +    if (r < 0) {
> +        goto err_queue_disable;
> +    }
> +
> +    vhost_dev_virtqueue_release(&net->dev, vdev, vq_index);
> +
> +    return;
> +
> +err_queue_disable:
> +    error_report("Error when releasing the qeuue.");
> +}
> +
> +int vhost_virtqueue_restart(VirtIODevice *vdev, NetClientState *nc,
> +                            int vq_index)
> +{
> +    VHostNetState *net = get_vhost_net(nc->peer);
> +    const VhostOps *vhost_ops = net->dev.vhost_ops;
> +    int r;
> +
> +    if (!net->dev.started) {
> +        return 0;
> +    }
> +
> +    assert(vhost_ops);
> +
> +    r = vhost_dev_virtqueue_restart(&net->dev, vdev, vq_index);
> +    if (r < 0) {
> +        goto err_start;
> +    }
> +
> +    r = vhost_ops->vhost_set_single_vring_enable(&net->dev, vq_index, 1,
> +                                                 false);


So it looks nothing vhost_net specific but vhost. And why not do 
set_single_vring_enable in vhost_dev_virtqueue_restart() (btw, the name 
of this function looks confusing).

Thanks


> +    if (r < 0) {
> +        goto err_start;
> +    }
> +
> +    return 0;
> +
> +err_start:
> +    error_report("Error when restarting the queue.");
> +    vhost_dev_stop(&net->dev, vdev);
> +
> +    return r;
> +}
> diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
> index 387e913e4e..fcb09e36ef 100644
> --- a/include/net/vhost_net.h
> +++ b/include/net/vhost_net.h
> @@ -48,4 +48,9 @@ uint64_t vhost_net_get_acked_features(VHostNetState *net);
>   
>   int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu);
>   
> +void vhost_virtqueue_stop(VirtIODevice *vdev, NetClientState *nc,
> +                          int vq_index);
> +int vhost_virtqueue_restart(VirtIODevice *vdev, NetClientState *nc,
> +                            int vq_index);
> +
>   #endif


Re: [PATCH 12/16] vhost-net: introduce restart and stop for vhost_net's vqs
Posted by Kangjie Xu 3 years, 6 months ago
在 2022/7/26 12:16, Jason Wang 写道:
>
> 在 2022/7/18 19:17, Kangjie Xu 写道:
>> Introduce vhost_virtqueue_restart(), which can restart the
>> virtqueue when the vhost net started running before.
>>
>> Introduce vhost_virtqueue_stop(), which can disable the vq
>> and unmap vrings and the desc of the vq. When disabling the
>> vq, the function is blocked and waits for a reply.
>>
>> Combining the two functions, we can reset a virtqueue with a
>> started vhost net.
>>
>> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
>> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>> ---
>>   hw/net/vhost_net.c      | 55 +++++++++++++++++++++++++++++++++++++++++
>>   include/net/vhost_net.h |  5 ++++
>>   2 files changed, 60 insertions(+)
>>
>> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
>> index ccac5b7a64..4f5f034c11 100644
>> --- a/hw/net/vhost_net.c
>> +++ b/hw/net/vhost_net.c
>> @@ -514,3 +514,58 @@ int vhost_net_set_mtu(struct vhost_net *net, 
>> uint16_t mtu)
>>         return vhost_ops->vhost_net_set_mtu(&net->dev, mtu);
>>   }
>> +
>> +void vhost_virtqueue_stop(VirtIODevice *vdev, NetClientState *nc,
>> +                          int vq_index)
>
>
> Let's rename this as vhost_net_virtqueue_stop()
>
Yeah, I agree.

Thanks

>
>> +{
>> +    VHostNetState *net = get_vhost_net(nc->peer);
>> +    const VhostOps *vhost_ops = net->dev.vhost_ops;
>> +    int r;
>> +
>> +    assert(vhost_ops);
>> +
>> +    r = vhost_ops->vhost_set_single_vring_enable(&net->dev, 
>> vq_index, 0, true);
>> +    if (r < 0) {
>> +        goto err_queue_disable;
>> +    }
>> +
>> +    vhost_dev_virtqueue_release(&net->dev, vdev, vq_index);
>> +
>> +    return;
>> +
>> +err_queue_disable:
>> +    error_report("Error when releasing the qeuue.");
>> +}
>> +
>> +int vhost_virtqueue_restart(VirtIODevice *vdev, NetClientState *nc,
>> +                            int vq_index)
>> +{
>> +    VHostNetState *net = get_vhost_net(nc->peer);
>> +    const VhostOps *vhost_ops = net->dev.vhost_ops;
>> +    int r;
>> +
>> +    if (!net->dev.started) {
>> +        return 0;
>> +    }
>> +
>> +    assert(vhost_ops);
>> +
>> +    r = vhost_dev_virtqueue_restart(&net->dev, vdev, vq_index);
>> +    if (r < 0) {
>> +        goto err_start;
>> +    }
>> +
>> +    r = vhost_ops->vhost_set_single_vring_enable(&net->dev, 
>> vq_index, 1,
>> +                                                 false);
>
>
> So it looks nothing vhost_net specific but vhost. And why not do 
> set_single_vring_enable in vhost_dev_virtqueue_restart() (btw, the 
> name of this function looks confusing).
>
> Thanks
>
>
I thought it was a convention that virtio-net will call vhost_net first, 
and cannot call vhost directly. So I implement it in this way.

It's no probem to move it to vhost. I'll fix it.

Thanks

>> +    if (r < 0) {
>> +        goto err_start;
>> +    }
>> +
>> +    return 0;
>> +
>> +err_start:
>> +    error_report("Error when restarting the queue.");
>> +    vhost_dev_stop(&net->dev, vdev);
>> +
>> +    return r;
>> +}
>> diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
>> index 387e913e4e..fcb09e36ef 100644
>> --- a/include/net/vhost_net.h
>> +++ b/include/net/vhost_net.h
>> @@ -48,4 +48,9 @@ uint64_t vhost_net_get_acked_features(VHostNetState 
>> *net);
>>     int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu);
>>   +void vhost_virtqueue_stop(VirtIODevice *vdev, NetClientState *nc,
>> +                          int vq_index);
>> +int vhost_virtqueue_restart(VirtIODevice *vdev, NetClientState *nc,
>> +                            int vq_index);
>> +
>>   #endif