Re: [PATCH v2 12/24] vhost-net: vhost-kernel: introduce vhost_net_virtqueue_restart()

[Kangjie Xu]

在 2022/8/24 17:01, Jason Wang 写道:
> 在 2022/8/24 10:53, Kangjie Xu 写道:
> > 在 2022/8/24 10:44, Jason Wang 写道:
> > > 在 2022/8/16 09:06, Kangjie Xu 写道:
> > > > Introduce vhost_net_virtqueue_restart(), which can restart the
> > > > virtqueue when the vhost net started running before. If it fails
> > > > to restart the virtqueue, the device will be stopped.
> > > >
> > > > This patch only considers the case for 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>
> > >
> > >
> > > I would explain why current 
> > > vhost_net_start_one()/vhost_net_stop_one() can't work. Is it because 
> > > it works at queue pair level? If yes can we restructure the code and 
> > > try to reuse ?
> > >
> > > Thanks
> > Because vhost_net_start_one()/vhost_net_stop_one() works at device 
> > level.
> >
> > The queue pair level start/stop are vhost_virtqueue_start() and 
> > vhost_virtqueue_stop().
> >
> > What we can reuse is the vhost_virtqueue_start(). 
> > vhost_virtqueue_stop() cannot be reused because it will destroy device.
>
>
> Let's add this in the changelog or a comment in the code.
>
> Thanks
Will fix.

Thanks.

> > I think we do not need to restructure because we've already had an 
> > abstraction vhost_virtqueue_start().
> >
> > Thanks.
> >
> > > > ---
> > > >   hw/net/vhost_net.c      | 48 
> > > > +++++++++++++++++++++++++++++++++++++++++
> > > >   include/net/vhost_net.h |  2 ++
> > > >   2 files changed, 50 insertions(+)
> > > >
> > > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > > > index aa60dd901c..2ab67e875e 100644
> > > > --- a/hw/net/vhost_net.c
> > > > +++ b/hw/net/vhost_net.c
> > > > @@ -535,3 +535,51 @@ void vhost_net_virtqueue_stop(VirtIODevice 
> > > > *vdev, NetClientState *nc,
> > > >         vhost_dev_virtqueue_stop(&net->dev, vdev, idx);
> > > >   }
> > > > +
> > > > +int vhost_net_virtqueue_restart(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 = { };
> > > > +    int idx, r;
> > > > +
> > > > +    if (!net->dev.started) {
> > > > +        return 0;
> > > > +    }
> > > > +
> > > > +    assert(vhost_ops);
> > > > +
> > > > +    idx = vhost_ops->vhost_get_vq_index(&net->dev, vq_index);
> > > > +
> > > > +    r = vhost_dev_virtqueue_restart(&net->dev, vdev, idx);
> > > > +    if (r < 0) {
> > > > +        goto err_start;
> > > > +    }
> > > > +
> > > > +    if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
> > > > +        file.index = idx;
> > > > +        file.fd = net->backend;
> > > > +        r = vhost_net_set_backend(&net->dev, &file);
> > > > +        if (r < 0) {
> > > > +            r = -errno;
> > > > +            goto err_start;
> > > > +        }
> > > > +    }
> > > > +
> > > > +    return 0;
> > > > +
> > > > +err_start:
> > > > +    error_report("Error when restarting the queue.");
> > > > +
> > > > +    if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
> > > > +        file.fd = -1;
> > > > +        file.index = idx;
> > > > +        int r = vhost_net_set_backend(&net->dev, &file);
> > > > +        assert(r >= 0);
> > > > +    }
> > > > +
> > > > +    vhost_dev_stop(&net->dev, vdev);
> > > > +
> > > > +    return r;
> > > > +}
> > > > diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
> > > > index 9b3aaf3814..e11a297380 100644
> > > > --- a/include/net/vhost_net.h
> > > > +++ b/include/net/vhost_net.h
> > > > @@ -50,4 +50,6 @@ int vhost_net_set_mtu(struct vhost_net *net, 
> > > > uint16_t mtu);
> > > >     void vhost_net_virtqueue_stop(VirtIODevice *vdev, 
> > > > NetClientState *nc,
> > > >                                 int vq_index);
> > > > +int vhost_net_virtqueue_restart(VirtIODevice *vdev, NetClientState 
> > > > *nc,
> > > > +                                int vq_index);
> > > >   #endif