netdev - Re: [External] Re: [PATCH 0/7] Introduce vdpa management tool

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 1 Dec 2020 22:18:35 +0800
From:   Yongji Xie <xieyongji@...edance.com>
To:     Parav Pandit <parav@...dia.com>
Cc:     Jason Wang <jasowang@...hat.com>,
        "virtualization@...ts.linux-foundation.org" 
        <virtualization@...ts.linux-foundation.org>,
        "Michael S. Tsirkin" <mst@...hat.com>, Eli Cohen <elic@...dia.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [External] Re: [PATCH 0/7] Introduce vdpa management tool

On Tue, Dec 1, 2020 at 7:32 PM Parav Pandit <parav@...dia.com> wrote:
>
>
>
> > From: Yongji Xie <xieyongji@...edance.com>
> > Sent: Tuesday, December 1, 2020 3:26 PM
> >
> > On Tue, Dec 1, 2020 at 2:25 PM Jason Wang <jasowang@...hat.com> wrote:
> > >
> > >
> > > On 2020/11/30 下午3:07, Yongji Xie wrote:
> > > >>> Thanks for adding me, Jason!
> > > >>>
> > > >>> Now I'm working on a v2 patchset for VDUSE (vDPA Device in
> > > >>> Userspace) [1]. This tool is very useful for the vduse device. So
> > > >>> I'm considering integrating this into my v2 patchset. But there is
> > > >>> one problem：
> > > >>>
> > > >>> In this tool, vdpa device config action and enable action are
> > > >>> combined into one netlink msg: VDPA_CMD_DEV_NEW. But in vduse
> > > >>> case, it needs to be splitted because a chardev should be created
> > > >>> and opened by a userspace process before we enable the vdpa device
> > > >>> (call vdpa_register_device()).
> > > >>>
> > > >>> So I'd like to know whether it's possible (or have some plans) to
> > > >>> add two new netlink msgs something like: VDPA_CMD_DEV_ENABLE
> > and
> > > >>> VDPA_CMD_DEV_DISABLE to make the config path more flexible.
> > > >>>
> > > >> Actually, we've discussed such intermediate step in some early
> > > >> discussion. It looks to me VDUSE could be one of the users of this.
> > > >>
> > > >> Or I wonder whether we can switch to use anonymous inode(fd) for
> > > >> VDUSE then fetching it via an VDUSE_GET_DEVICE_FD ioctl?
> > > >>
> > > > Yes, we can. Actually the current implementation in VDUSE is like
> > > > this.  But seems like this is still a intermediate step. The fd
> > > > should be binded to a name or something else which need to be
> > > > configured before.
> > >
> > >
> > > The name could be specified via the netlink. It looks to me the real
> > > issue is that until the device is connected with a userspace, it can't
> > > be used. So we also need to fail the enabling if it doesn't opened.
> > >
> >
> > Yes, that's true. So you mean we can firstly try to fetch the fd binded to a
> > name/vduse_id via an VDUSE_GET_DEVICE_FD, then use the
> > name/vduse_id as a attribute to create vdpa device? It looks fine to me.
>
> I probably do not well understand. I tried reading patch [1] and few things do not look correct as below.
> Creating the vdpa device on the bus device and destroying the device from the workqueue seems unnecessary and racy.
>
> It seems vduse driver needs
> This is something should be done as part of the vdpa dev add command, instead of connecting two sides separately and ensuring race free access to it.
>
> So VDUSE_DEV_START and VDUSE_DEV_STOP should possibly be avoided.
>

Yes, we can avoid these two ioctls with the help of the management tool.

> $ vdpa dev add parentdev vduse_mgmtdev type net name foo2
>
> When above command is executed it creates necessary vdpa device foo2 on the bus.
> When user binds foo2 device with the vduse driver, in the probe(), it creates respective char device to access it from user space.

But vduse driver is not a vdpa bus driver. It works like vdpasim
driver, but offloads the data plane and control plane to a user space
process.

Thanks,
Yongji