From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
virtio-net and vhost-kernel implement queue reset.
Queued packets in the corresponding queue pair are flushed
or purged.
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
---
hw/net/virtio-net.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 27b59c0ad6..d774a3e652 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -540,6 +540,23 @@ static RxFilterInfo *virtio_net_query_rxfilter(NetClientState *nc)
return info;
}
+static void virtio_net_queue_reset(VirtIODevice *vdev, uint32_t queue_index)
+{
+ VirtIONet *n = VIRTIO_NET(vdev);
+ NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(queue_index));
+
+ if (!nc->peer) {
+ return;
+ }
+
+ if (get_vhost_net(nc->peer) &&
+ nc->peer->info->type == NET_CLIENT_DRIVER_TAP) {
+ vhost_net_virtqueue_reset(vdev, nc, queue_index);
+ }
+
+ flush_or_purge_queued_packets(nc);
+}
+
static void virtio_net_reset(VirtIODevice *vdev)
{
VirtIONet *n = VIRTIO_NET(vdev);
@@ -3784,6 +3801,7 @@ static void virtio_net_class_init(ObjectClass *klass, void *data)
vdc->set_features = virtio_net_set_features;
vdc->bad_features = virtio_net_bad_features;
vdc->reset = virtio_net_reset;
+ vdc->queue_reset = virtio_net_queue_reset;
vdc->set_status = virtio_net_set_status;
vdc->guest_notifier_mask = virtio_net_guest_notifier_mask;
vdc->guest_notifier_pending = virtio_net_guest_notifier_pending;
--
2.32.0
在 2022/8/25 16:08, Kangjie Xu 写道:
> From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>
> virtio-net and vhost-kernel implement queue reset.
> Queued packets in the corresponding queue pair are flushed
> or purged.
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> ---
> hw/net/virtio-net.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 27b59c0ad6..d774a3e652 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -540,6 +540,23 @@ static RxFilterInfo *virtio_net_query_rxfilter(NetClientState *nc)
> return info;
> }
>
> +static void virtio_net_queue_reset(VirtIODevice *vdev, uint32_t queue_index)
> +{
> + VirtIONet *n = VIRTIO_NET(vdev);
> + NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(queue_index));
> +
> + if (!nc->peer) {
> + return;
> + }
> +
> + if (get_vhost_net(nc->peer) &&
> + nc->peer->info->type == NET_CLIENT_DRIVER_TAP) {
> + vhost_net_virtqueue_reset(vdev, nc, queue_index);
> + }
> +
> + flush_or_purge_queued_packets(nc);
But the codes doesn't prevent the usersapce datapath from being used?
(e.g vhost=off)
E.g vhost_net_start_one() had:
if (net->nc->info->poll) {
net->nc->info->poll(net->nc, false);
}
And I will wonder if it's better to consider to:
1) factor out the per virtqueue start/stop from vhost_net_start/stop_one()
2) simply use the helper factored out via step 1)
Thanks
> +}
> +
> static void virtio_net_reset(VirtIODevice *vdev)
> {
> VirtIONet *n = VIRTIO_NET(vdev);
> @@ -3784,6 +3801,7 @@ static void virtio_net_class_init(ObjectClass *klass, void *data)
> vdc->set_features = virtio_net_set_features;
> vdc->bad_features = virtio_net_bad_features;
> vdc->reset = virtio_net_reset;
> + vdc->queue_reset = virtio_net_queue_reset;
> vdc->set_status = virtio_net_set_status;
> vdc->guest_notifier_mask = virtio_net_guest_notifier_mask;
> vdc->guest_notifier_pending = virtio_net_guest_notifier_pending;
在 2022/9/5 16:30, Jason Wang 写道:
>
> 在 2022/8/25 16:08, Kangjie Xu 写道:
>> From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>>
>> virtio-net and vhost-kernel implement queue reset.
>> Queued packets in the corresponding queue pair are flushed
>> or purged.
>>
>> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
>> ---
>> hw/net/virtio-net.c | 18 ++++++++++++++++++
>> 1 file changed, 18 insertions(+)
>>
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index 27b59c0ad6..d774a3e652 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -540,6 +540,23 @@ static RxFilterInfo
>> *virtio_net_query_rxfilter(NetClientState *nc)
>> return info;
>> }
>> +static void virtio_net_queue_reset(VirtIODevice *vdev, uint32_t
>> queue_index)
>> +{
>> + VirtIONet *n = VIRTIO_NET(vdev);
>> + NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(queue_index));
>> +
>> + if (!nc->peer) {
>> + return;
>> + }
>> +
>> + if (get_vhost_net(nc->peer) &&
>> + nc->peer->info->type == NET_CLIENT_DRIVER_TAP) {
>> + vhost_net_virtqueue_reset(vdev, nc, queue_index);
>> + }
>> +
>> + flush_or_purge_queued_packets(nc);
>
>
> But the codes doesn't prevent the usersapce datapath from being used?
> (e.g vhost=off)
I think we do not need to prevent it for vhost=off, because the
virtio-net device is in control of the tap device.
After we reset the vq, the virtio-net send and recv will not use the
userspace datapath. (virtio_net_flush_tx() and virtio_net_receive() will
early returns because vq->vring.avail == 0)
So even if we don't prevent it using net->nc->info->poll, virtio-net
device will prevent it. And the logic here is similar to virtio_reset(),
I think it will not cause problems.
Thanks.
>
> E.g vhost_net_start_one() had:
>
> if (net->nc->info->poll) {
> net->nc->info->poll(net->nc, false);
> }
>
> And I will wonder if it's better to consider to:
>
> 1) factor out the per virtqueue start/stop from
> vhost_net_start/stop_one()
>
> 2) simply use the helper factored out via step 1)
>
> Thanks
>
>
>> +}
>> +
>> static void virtio_net_reset(VirtIODevice *vdev)
>> {
>> VirtIONet *n = VIRTIO_NET(vdev);
>> @@ -3784,6 +3801,7 @@ static void virtio_net_class_init(ObjectClass
>> *klass, void *data)
>> vdc->set_features = virtio_net_set_features;
>> vdc->bad_features = virtio_net_bad_features;
>> vdc->reset = virtio_net_reset;
>> + vdc->queue_reset = virtio_net_queue_reset;
>> vdc->set_status = virtio_net_set_status;
>> vdc->guest_notifier_mask = virtio_net_guest_notifier_mask;
>> vdc->guest_notifier_pending = virtio_net_guest_notifier_pending;
在 2022/9/5 16:30, Jason Wang 写道:
>
> 在 2022/8/25 16:08, Kangjie Xu 写道:
>> From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>>
>> virtio-net and vhost-kernel implement queue reset.
>> Queued packets in the corresponding queue pair are flushed
>> or purged.
>>
>> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
>> ---
>> hw/net/virtio-net.c | 18 ++++++++++++++++++
>> 1 file changed, 18 insertions(+)
>>
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index 27b59c0ad6..d774a3e652 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -540,6 +540,23 @@ static RxFilterInfo
>> *virtio_net_query_rxfilter(NetClientState *nc)
>> return info;
>> }
>> +static void virtio_net_queue_reset(VirtIODevice *vdev, uint32_t
>> queue_index)
>> +{
>> + VirtIONet *n = VIRTIO_NET(vdev);
>> + NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(queue_index));
>> +
>> + if (!nc->peer) {
>> + return;
>> + }
>> +
>> + if (get_vhost_net(nc->peer) &&
>> + nc->peer->info->type == NET_CLIENT_DRIVER_TAP) {
>> + vhost_net_virtqueue_reset(vdev, nc, queue_index);
>> + }
>> +
>> + flush_or_purge_queued_packets(nc);
>
>
> But the codes doesn't prevent the usersapce datapath from being used?
> (e.g vhost=off)
>
> E.g vhost_net_start_one() had:
>
> if (net->nc->info->poll) {
> net->nc->info->poll(net->nc, false);
> }
I review this part in vhost_net_start/stop_one().
I didn't take the usersapce datapath into consideration. If we don't
prevent it, the userspace datapath may access it and causes some
problems. From this point, we should disable it.
But if we add net->nc->info->poll in vq reset, it can only operate at
queue-pair level, which obeys "per-virtqueue".
What's your opinion on this point? I think it's ok to add it in vq reset.
>
> And I will wonder if it's better to consider to:
>
> 1) factor out the per virtqueue start/stop from
> vhost_net_start/stop_one()
>
> 2) simply use the helper factored out via step 1)
>
> Thanks
>
I have a different idea about it, vhost_virtqueue_start/stop()(in
hw/virtio/vhost.c) are already good abstractions of per virtqueue
start/stop.
These two functions are used in vhost_dev_start. It's just because
vhost-net needs some configuration, so we add net->nc->info->poll and
set_backend for it. But for other devices using vhost_dev_start,
set_backend and net->nc->info->poll may be not necessary.
I think your apporach to abstract per virtqueue start/stop from
vhost_net_start/stop_one() will break the generality of
vhost_dev_start(), which is a common interface for different devices.
To conclude my opinions
1. I think we need to add net->nc->info->poll in our
vhost_net_virtqueue_reset() and vhost_net_virtqueue_restart()
2. We still need vhost_net_virtqueue_reset() and
vhost_net_virtqueue_restart().
a. vhost_net_virtqueue_reset() is a wrapper for vhost_virtqueue_stop().
b. vhost_net_virtqueue_restart() is a wrapper for
vhost_virtqueue_start().
Thanks
>
>> +}
>> +
>> static void virtio_net_reset(VirtIODevice *vdev)
>> {
>> VirtIONet *n = VIRTIO_NET(vdev);
>> @@ -3784,6 +3801,7 @@ static void virtio_net_class_init(ObjectClass
>> *klass, void *data)
>> vdc->set_features = virtio_net_set_features;
>> vdc->bad_features = virtio_net_bad_features;
>> vdc->reset = virtio_net_reset;
>> + vdc->queue_reset = virtio_net_queue_reset;
>> vdc->set_status = virtio_net_set_status;
>> vdc->guest_notifier_mask = virtio_net_guest_notifier_mask;
>> vdc->guest_notifier_pending = virtio_net_guest_notifier_pending;
On Tue, Sep 6, 2022 at 12:25 AM Kangjie Xu <kangjie.xu@linux.alibaba.com> wrote:
>
>
> 在 2022/9/5 16:30, Jason Wang 写道:
> >
> > 在 2022/8/25 16:08, Kangjie Xu 写道:
> >> From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> >>
> >> virtio-net and vhost-kernel implement queue reset.
> >> Queued packets in the corresponding queue pair are flushed
> >> or purged.
> >>
> >> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> >> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
> >> ---
> >> hw/net/virtio-net.c | 18 ++++++++++++++++++
> >> 1 file changed, 18 insertions(+)
> >>
> >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> >> index 27b59c0ad6..d774a3e652 100644
> >> --- a/hw/net/virtio-net.c
> >> +++ b/hw/net/virtio-net.c
> >> @@ -540,6 +540,23 @@ static RxFilterInfo
> >> *virtio_net_query_rxfilter(NetClientState *nc)
> >> return info;
> >> }
> >> +static void virtio_net_queue_reset(VirtIODevice *vdev, uint32_t
> >> queue_index)
> >> +{
> >> + VirtIONet *n = VIRTIO_NET(vdev);
> >> + NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(queue_index));
> >> +
> >> + if (!nc->peer) {
> >> + return;
> >> + }
> >> +
> >> + if (get_vhost_net(nc->peer) &&
> >> + nc->peer->info->type == NET_CLIENT_DRIVER_TAP) {
> >> + vhost_net_virtqueue_reset(vdev, nc, queue_index);
> >> + }
> >> +
> >> + flush_or_purge_queued_packets(nc);
> >
> >
> > But the codes doesn't prevent the usersapce datapath from being used?
> > (e.g vhost=off)
> >
> > E.g vhost_net_start_one() had:
> >
> > if (net->nc->info->poll) {
> > net->nc->info->poll(net->nc, false);
> > }
> I review this part in vhost_net_start/stop_one().
>
> I didn't take the usersapce datapath into consideration.
Because it's used for vhost-net. So I think maybe we should first
implement the rest in the userspace datapath first then try to
implement them in the vhost?
> If we don't
> prevent it, the userspace datapath may access it and causes some
> problems. From this point, we should disable it.
>
> But if we add net->nc->info->poll in vq reset, it can only operate at
> queue-pair level, which obeys "per-virtqueue".
>
> What's your opinion on this point? I think it's ok to add it in vq reset.
See above, I'd suggest to implement the vq reset in qemu usersapce
datapath first. Then we can do vhost part on top.
>
> >
> > And I will wonder if it's better to consider to:
> >
> > 1) factor out the per virtqueue start/stop from
> > vhost_net_start/stop_one()
> >
> > 2) simply use the helper factored out via step 1)
> >
> > Thanks
> >
> I have a different idea about it, vhost_virtqueue_start/stop()(in
> hw/virtio/vhost.c) are already good abstractions of per virtqueue
> start/stop.
>
> These two functions are used in vhost_dev_start. It's just because
> vhost-net needs some configuration, so we add net->nc->info->poll and
> set_backend for it. But for other devices using vhost_dev_start,
> set_backend and net->nc->info->poll may be not necessary.
>
> I think your apporach to abstract per virtqueue start/stop from
> vhost_net_start/stop_one() will break the generality of
> vhost_dev_start(), which is a common interface for different devices.
>
> To conclude my opinions
>
> 1. I think we need to add net->nc->info->poll in our
> vhost_net_virtqueue_reset() and vhost_net_virtqueue_restart()
>
> 2. We still need vhost_net_virtqueue_reset() and
> vhost_net_virtqueue_restart().
>
> a. vhost_net_virtqueue_reset() is a wrapper for vhost_virtqueue_stop().
>
> b. vhost_net_virtqueue_restart() is a wrapper for
> vhost_virtqueue_start().
Sounds good, let's do and see.
Thanks
>
> Thanks
>
> >
> >> +}
> >> +
> >> static void virtio_net_reset(VirtIODevice *vdev)
> >> {
> >> VirtIONet *n = VIRTIO_NET(vdev);
> >> @@ -3784,6 +3801,7 @@ static void virtio_net_class_init(ObjectClass
> >> *klass, void *data)
> >> vdc->set_features = virtio_net_set_features;
> >> vdc->bad_features = virtio_net_bad_features;
> >> vdc->reset = virtio_net_reset;
> >> + vdc->queue_reset = virtio_net_queue_reset;
> >> vdc->set_status = virtio_net_set_status;
> >> vdc->guest_notifier_mask = virtio_net_guest_notifier_mask;
> >> vdc->guest_notifier_pending = virtio_net_guest_notifier_pending;
>
在 2022/9/5 16:30, Jason Wang 写道:
>
> 在 2022/8/25 16:08, Kangjie Xu 写道:
>> From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>>
>> virtio-net and vhost-kernel implement queue reset.
>> Queued packets in the corresponding queue pair are flushed
>> or purged.
>>
>> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com>
>> ---
>> hw/net/virtio-net.c | 18 ++++++++++++++++++
>> 1 file changed, 18 insertions(+)
>>
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index 27b59c0ad6..d774a3e652 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -540,6 +540,23 @@ static RxFilterInfo
>> *virtio_net_query_rxfilter(NetClientState *nc)
>> return info;
>> }
>> +static void virtio_net_queue_reset(VirtIODevice *vdev, uint32_t
>> queue_index)
>> +{
>> + VirtIONet *n = VIRTIO_NET(vdev);
>> + NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(queue_index));
>> +
>> + if (!nc->peer) {
>> + return;
>> + }
>> +
>> + if (get_vhost_net(nc->peer) &&
>> + nc->peer->info->type == NET_CLIENT_DRIVER_TAP) {
>> + vhost_net_virtqueue_reset(vdev, nc, queue_index);
>> + }
>> +
>> + flush_or_purge_queued_packets(nc);
>
>
> But the codes doesn't prevent the usersapce datapath from being used?
> (e.g vhost=off)
>
> E.g vhost_net_start_one() had:
>
> if (net->nc->info->poll) {
> net->nc->info->poll(net->nc, false);
> }
>
> And I will wonder if it's better to consider to:
>
> 1) factor out the per virtqueue start/stop from
> vhost_net_start/stop_one()
>
> 2) simply use the helper factored out via step 1)
>
> Thanks
>
Will update it based on your suggestions.
Thanks
>
>> +}
>> +
>> static void virtio_net_reset(VirtIODevice *vdev)
>> {
>> VirtIONet *n = VIRTIO_NET(vdev);
>> @@ -3784,6 +3801,7 @@ static void virtio_net_class_init(ObjectClass
>> *klass, void *data)
>> vdc->set_features = virtio_net_set_features;
>> vdc->bad_features = virtio_net_bad_features;
>> vdc->reset = virtio_net_reset;
>> + vdc->queue_reset = virtio_net_queue_reset;
>> vdc->set_status = virtio_net_set_status;
>> vdc->guest_notifier_mask = virtio_net_guest_notifier_mask;
>> vdc->guest_notifier_pending = virtio_net_guest_notifier_pending;
© 2016 - 2026 Red Hat, Inc.