Re: [PATCH v3 13/15] virtio-net: support queue reset

[Kangjie Xu <kangjie.xu@linux.alibaba.com>]

在 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;