1. Patchew
  2. QEMU
  3. [PATCH v3 00/15] Support VIRTIO_F_RING_RESET for virtio-net, vhost-net kernel in virtio pci-modern
  4. View patch

[PATCH v3 10/15] vhost-net: vhost-kernel: introduce vhost_net_virtqueue_reset()

Kangjie Xu posted 15 patches 3 years, 5 months ago
Maintainers: Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Yanan Wang <wangyanan55@huawei.com>, "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com>
There is a newer version of this series
[PATCH v3 10/15] vhost-net: vhost-kernel: introduce vhost_net_virtqueue_reset()
Posted by Kangjie Xu 3 years, 5 months ago 
Introduce vhost_virtqueue_reset(), which can reset the specific
virtqueue in the device. Then it will unmap vrings and the desc
of the virtqueue.

Here we do not reuse the vhost_net_stop_one() or vhost_dev_stop(),
because they work at queue pair level. We do not use
vhost_virtqueue_stop() because it may stop the device in the
backend.

This patch only considers the case of vhost-kernel, when
NetClientDriver is NET_CLIENT_DRIVER_TAP.

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

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index ccac5b7a64..be51be98b3 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -514,3 +514,25 @@ int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu)
 
     return vhost_ops->vhost_net_set_mtu(&net->dev, mtu);
 }
+
+void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState *nc,
+                               int vq_index)
+{
+    VHostNetState *net = get_vhost_net(nc->peer);
+    const VhostOps *vhost_ops = net->dev.vhost_ops;
+    struct vhost_vring_file file = { .fd = -1 };
+    int idx;
+
+    /* should only be called after backend is connected */
+    assert(vhost_ops);
+
+    idx = vhost_ops->vhost_get_vq_index(&net->dev, vq_index);
+
+    if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
+        file.index = idx;
+        int r = vhost_net_set_backend(&net->dev, &file);
+        assert(r >= 0);
+    }
+
+    vhost_virtqueue_unmap(&net->dev, vdev, net->dev.vqs + idx, idx);
+}
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index 387e913e4e..85d85a4957 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -48,4 +48,6 @@ uint64_t vhost_net_get_acked_features(VHostNetState *net);
 
 int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu);
 
+void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState *nc,
+                               int vq_index);
 #endif
-- 
2.32.0
Re: [PATCH v3 10/15] vhost-net: vhost-kernel: introduce vhost_net_virtqueue_reset()
Posted by Jason Wang 3 years, 5 months ago 
在 2022/8/25 16:08, Kangjie Xu 写道:
> Introduce vhost_virtqueue_reset(), which can reset the specific
> virtqueue in the device. Then it will unmap vrings and the desc
> of the virtqueue.
>
> Here we do not reuse the vhost_net_stop_one() or vhost_dev_stop(),
> because they work at queue pair level. We do not use
> vhost_virtqueue_stop() because it may stop the device in the
> backend.


So I think this is not true at least for vhost-net kernel baceknd.


>
> This patch only considers the case of vhost-kernel, when
> NetClientDriver is NET_CLIENT_DRIVER_TAP.
>
> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
>   hw/net/vhost_net.c      | 22 ++++++++++++++++++++++
>   include/net/vhost_net.h |  2 ++
>   2 files changed, 24 insertions(+)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index ccac5b7a64..be51be98b3 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -514,3 +514,25 @@ int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu)
>   
>       return vhost_ops->vhost_net_set_mtu(&net->dev, mtu);
>   }
> +
> +void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState *nc,
> +                               int vq_index)
> +{
> +    VHostNetState *net = get_vhost_net(nc->peer);
> +    const VhostOps *vhost_ops = net->dev.vhost_ops;
> +    struct vhost_vring_file file = { .fd = -1 };
> +    int idx;
> +
> +    /* should only be called after backend is connected */
> +    assert(vhost_ops);
> +
> +    idx = vhost_ops->vhost_get_vq_index(&net->dev, vq_index);
> +
> +    if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
> +        file.index = idx;
> +        int r = vhost_net_set_backend(&net->dev, &file);
> +        assert(r >= 0);
> +    }


Do we need to reset e.g last_avail_idx here?

Thanks


> +
> +    vhost_virtqueue_unmap(&net->dev, vdev, net->dev.vqs + idx, idx);
> +}
> diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
> index 387e913e4e..85d85a4957 100644
> --- a/include/net/vhost_net.h
> +++ b/include/net/vhost_net.h
> @@ -48,4 +48,6 @@ uint64_t vhost_net_get_acked_features(VHostNetState *net);
>   
>   int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu);
>   
> +void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState *nc,
> +                               int vq_index);
>   #endif
Re: [PATCH v3 10/15] vhost-net: vhost-kernel: introduce vhost_net_virtqueue_reset()
Posted by Kangjie Xu 3 years, 5 months ago 
在 2022/9/5 16:03, Jason Wang 写道:
>
> 在 2022/8/25 16:08, Kangjie Xu 写道:
>> Introduce vhost_virtqueue_reset(), which can reset the specific
>> virtqueue in the device. Then it will unmap vrings and the desc
>> of the virtqueue.
>>
>> Here we do not reuse the vhost_net_stop_one() or vhost_dev_stop(),
>> because they work at queue pair level. We do not use
>> vhost_virtqueue_stop() because it may stop the device in the
>> backend.
>
>
> So I think this is not true at least for vhost-net kernel baceknd.
>
>
But vhost-user(OVS-DPDK) will stop the device.

When DPDK vhost received VHOST_USER_GET_VRING_BASE message, it will call 
vhost_destroy_device_notify() to destroy the device.

It seems like it is a inconsistency error in DPDK. Maybe I should submit 
a patch to DPDK. We can stop the device only when all the virtqueues in 
one device are destroyed.

>>
>> This patch only considers the case of vhost-kernel, when
>> NetClientDriver is NET_CLIENT_DRIVER_TAP.
>>
>> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
>> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>> ---
>>   hw/net/vhost_net.c      | 22 ++++++++++++++++++++++
>>   include/net/vhost_net.h |  2 ++
>>   2 files changed, 24 insertions(+)
>>
>> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
>> index ccac5b7a64..be51be98b3 100644
>> --- a/hw/net/vhost_net.c
>> +++ b/hw/net/vhost_net.c
>> @@ -514,3 +514,25 @@ int vhost_net_set_mtu(struct vhost_net *net, 
>> uint16_t mtu)
>>         return vhost_ops->vhost_net_set_mtu(&net->dev, mtu);
>>   }
>> +
>> +void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState *nc,
>> +                               int vq_index)
>> +{
>> +    VHostNetState *net = get_vhost_net(nc->peer);
>> +    const VhostOps *vhost_ops = net->dev.vhost_ops;
>> +    struct vhost_vring_file file = { .fd = -1 };
>> +    int idx;
>> +
>> +    /* should only be called after backend is connected */
>> +    assert(vhost_ops);
>> +
>> +    idx = vhost_ops->vhost_get_vq_index(&net->dev, vq_index);
>> +
>> +    if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
>> +        file.index = idx;
>> +        int r = vhost_net_set_backend(&net->dev, &file);
>> +        assert(r >= 0);
>> +    }
>
>
> Do we need to reset e.g last_avail_idx here?
>
> Thanks
>
I did not reset it because we will re-configure them when we restart 
virtqueue.

Thanks

>
>> +
>> +    vhost_virtqueue_unmap(&net->dev, vdev, net->dev.vqs + idx, idx);
>> +}
>> diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
>> index 387e913e4e..85d85a4957 100644
>> --- a/include/net/vhost_net.h
>> +++ b/include/net/vhost_net.h
>> @@ -48,4 +48,6 @@ uint64_t vhost_net_get_acked_features(VHostNetState 
>> *net);
>>     int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu);
>>   +void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState 
>> *nc,
>> +                               int vq_index);
>>   #endif
Re: [PATCH v3 10/15] vhost-net: vhost-kernel: introduce vhost_net_virtqueue_reset()
Posted by Jason Wang 3 years, 5 months ago 
On Mon, Sep 5, 2022 at 6:15 PM Kangjie Xu <kangjie.xu@linux.alibaba.com> wrote:
>
>
> 在 2022/9/5 16:03, Jason Wang 写道:
> >
> > 在 2022/8/25 16:08, Kangjie Xu 写道:
> >> Introduce vhost_virtqueue_reset(), which can reset the specific
> >> virtqueue in the device. Then it will unmap vrings and the desc
> >> of the virtqueue.
> >>
> >> Here we do not reuse the vhost_net_stop_one() or vhost_dev_stop(),
> >> because they work at queue pair level. We do not use
> >> vhost_virtqueue_stop() because it may stop the device in the
> >> backend.
> >
> >
> > So I think this is not true at least for vhost-net kernel baceknd.
> >
> >
> But vhost-user(OVS-DPDK) will stop the device.

Yes, what I meant is that, considering this series only deal with
vhost-net. We can leave any "fix" or "workaround" until e.g the
vhost-user support is posted?

>
> When DPDK vhost received VHOST_USER_GET_VRING_BASE message, it will call
> vhost_destroy_device_notify() to destroy the device.

Right, actually this trick is used even in some hardware.

>
> It seems like it is a inconsistency error in DPDK. Maybe I should submit
> a patch to DPDK. We can stop the device only when all the virtqueues in
> one device are destroyed.

That would be fine.

>
> >>
> >> This patch only considers the case of vhost-kernel, when
> >> NetClientDriver is NET_CLIENT_DRIVER_TAP.
> >>
> >> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> >> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> >> ---
> >>   hw/net/vhost_net.c      | 22 ++++++++++++++++++++++
> >>   include/net/vhost_net.h |  2 ++
> >>   2 files changed, 24 insertions(+)
> >>
> >> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> >> index ccac5b7a64..be51be98b3 100644
> >> --- a/hw/net/vhost_net.c
> >> +++ b/hw/net/vhost_net.c
> >> @@ -514,3 +514,25 @@ int vhost_net_set_mtu(struct vhost_net *net,
> >> uint16_t mtu)
> >>         return vhost_ops->vhost_net_set_mtu(&net->dev, mtu);
> >>   }
> >> +
> >> +void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState *nc,
> >> +                               int vq_index)
> >> +{
> >> +    VHostNetState *net = get_vhost_net(nc->peer);
> >> +    const VhostOps *vhost_ops = net->dev.vhost_ops;
> >> +    struct vhost_vring_file file = { .fd = -1 };
> >> +    int idx;
> >> +
> >> +    /* should only be called after backend is connected */
> >> +    assert(vhost_ops);
> >> +
> >> +    idx = vhost_ops->vhost_get_vq_index(&net->dev, vq_index);
> >> +
> >> +    if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
> >> +        file.index = idx;
> >> +        int r = vhost_net_set_backend(&net->dev, &file);
> >> +        assert(r >= 0);
> >> +    }
> >
> >
> > Do we need to reset e.g last_avail_idx here?
> >
> > Thanks
> >
> I did not reset it because we will re-configure them when we restart
> virtqueue.

Is last_avail_idx reset to 0 in this case?

Thanks

>
> Thanks
>
> >
> >> +
> >> +    vhost_virtqueue_unmap(&net->dev, vdev, net->dev.vqs + idx, idx);
> >> +}
> >> diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
> >> index 387e913e4e..85d85a4957 100644
> >> --- a/include/net/vhost_net.h
> >> +++ b/include/net/vhost_net.h
> >> @@ -48,4 +48,6 @@ uint64_t vhost_net_get_acked_features(VHostNetState
> >> *net);
> >>     int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu);
> >>   +void vhost_net_virtqueue_reset(VirtIODevice *vdev, NetClientState
> >> *nc,
> >> +                               int vq_index);
> >>   #endif
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.