Re: [PATCH 07/16] virtio-net: support queue reset

[Jason Wang]

On Tue, Jul 26, 2022 at 3:02 PM Kangjie Xu <kangjie.xu@linux.alibaba.com> wrote:
>
>
> 在 2022/7/26 11:43, Jason Wang 写道:
> >
> > 在 2022/7/18 19:17, Kangjie Xu 写道:
> >> From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> >>
> >> virtio-net implements queue reset. Queued packets in the corresponding
> >> queue pair are flushed or purged.
> >>
> >> Queue reset is currently only implemented for non-vhosts.
> >>
> >> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> >> ---
> >>   hw/net/virtio-net.c | 15 +++++++++++++++
> >>   1 file changed, 15 insertions(+)
> >>
> >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> >> index 7ad948ee7c..8396e21a67 100644
> >> --- a/hw/net/virtio-net.c
> >> +++ b/hw/net/virtio-net.c
> >> @@ -531,6 +531,19 @@ 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;
> >> +    }
> >> +
> >> +    qemu_flush_or_purge_queued_packets(nc->peer, true);
> >> +    assert(!virtio_net_get_subqueue(nc)->async_tx.elem);
> >
> >
> > Let's try to reuse this function in virtio_net_reset().
> >
> Yeah, I'll fix it.
>
> Thanks.
>
> >
> >> +}
> >> +
> >>   static void virtio_net_reset(VirtIODevice *vdev)
> >>   {
> >>       VirtIONet *n = VIRTIO_NET(vdev);
> >> @@ -741,6 +754,7 @@ static uint64_t
> >> virtio_net_get_features(VirtIODevice *vdev, uint64_t features,
> >>       }
> >>         if (!get_vhost_net(nc->peer)) {
> >> +        virtio_add_feature(&features, VIRTIO_F_RING_RESET);
> >
> >
> > This breaks migration compatibility.
> >
> > We probably need:
> >
> > 1) a new command line parameter
> > 2) make it disabled for pre-7.2 machine
> >
> > Thanks
> >
> >
> Sorry, I don't get what is the meaning of "pre-7.2 machine". Could you
> please explain it?

I meant for pre 7.2 machine type, we should make reset fault off by default.

Otherwise we break migration compatibility.

Thanks

>
> Thanks
>
> >>           return features;
> >>       }
> >>   @@ -3766,6 +3780,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;
>