Re: [Qemu-devel] [PATCH 0/9] introduce virtio net dataplane

[liu ping fan]

On Fri, Feb 22, 2013 at 12:51 AM, Michael S. Tsirkin <address@hidden> wrote:
> On Thu, Feb 21, 2013 at 06:45:54PM +0200, Michael S. Tsirkin wrote:
>> On Thu, Feb 21, 2013 at 10:15:58AM -0600, Anthony Liguori wrote:
>> > "Michael S. Tsirkin" <address@hidden> writes:
>> >
>> > > On Thu, Feb 21, 2013 at 08:54:44PM +0800, Liu Ping Fan wrote:
>> > >> This is a emulation to virtio-blk dataplane, which push the data
>> > >> handling out of biglock. And it is a try to implement this process
>> > >> in userspace, while vhost-net in kernel.
>> > >
>> > > What's the motivation for doing this?
>> >
>> > I haven't looked carefully at the patches, but I don't think we should
>> > "cheat" when it comes to virtio-net.  I think it's useful to have a
>> > baseline proof of concept but I think we should focus on making the
>> > network layer re-entrant.
>> >
>> > In terms of why even bother, if we can make virtio-net a viable
>> > alternative to vhost-net, it's a huge win.  Being in userspace is a huge
>> > advantage.
>> >
>> > I understand the challenges with zero-copy but we really need a proper
>> > userspace implementation to determine how much it really matters.
>> > Zero-copy tap is also not entirely outside the realm of possibility.
>>
>> This is exactly what we have, vhost-net is the interface we use
>> for asynchronous communication. If you want to add yet another
>> asynchronous interface in kernel, it's certainly doable but then what's
>> the point?
>>
>> > >> The iperf's result show it improving the perfermance of base line,
>> > >> but still slower than vhost-net (maybe the rx path need optimized).
>> > >>
>> > >> Todo:
>> > >> implement fine lock for net-subsystem
>> > >
>> > > vhost-net is currently the only way to get zero copy transmit
>> > > on linux (and zero copy receive in the future)
>> > > so at least in theory it'll always be faster.
>> >
>> > It might always be faster.  But that doesn't mean we should limit the
>> > performance of virtio-net in userspace.  Some people may be willing to
>> > take a small performance hit in order to obtain the security offered by
>> > being in userspace.
>> >
>> > Regards,
>> >
>> > Anthony Liguori
>>
>> By 'being in userspace' I presume you mean the ring processing code.
>> Ring processing can be done in userspace if you are ready
>> to use the synchronous read/write operations, skipping
>> this bunch of code might improve security. But it requires a data
>> copy almost by definition.
>>
>
> BTW I think we do have a problem at the moment,
> and that is that virtio net can process the same ring both in userspace
> (for level interrupts) and in kernel (for edge interrupts).

When using virtio net, the proc/interrupt shows "IO-APIC-edge" as
vhost-net, so does virtio net use level interrupt?
> And which one is selected is under guest control.
> This is twice the number of potential security issues.
> A solution, in my opinion, is to improve handling of level
> interrupts to the point where we can make vhost-net
> the default for level as well.
> The new support for level IRQFD in kvm should make
> this possible.
>
Seems qemu lacks of something like eoi-eventfd for level interrupt, right?

Thanks and regards,
Pingfan

> Unfortunately, this prototype moves in exactly the reverse
> direction.
>
>> > >
>> > >> Liu Ping Fan (9):
>> > >>   vring: split the modification and publish of used ring
>> > >>   vring: introduce vring_restore() to restore from img
>> > >>   event poll: make epoll work for normal fd
>> > >>   event poll: pass event type to event callback
>> > >>   event poll: enable event poll handle more than one event each time
>> > >>   virtio net: introduce dataplane for virtio net
>> > >>   tap: make tap peer work on dedicated data-plane thread
>> > >>   virtio net: enable dataplane for virtio net
>> > >>   configure: make virtio net dataplane configurable
>> > >>
>> > >>  configure                  |   12 ++
>> > >>  hw/dataplane/Makefile.objs |    4 +-
>> > >>  hw/dataplane/event-poll.c  |   69 +++++--
>> > >>  hw/dataplane/event-poll.h  |   14 ++-
>> > >>  hw/dataplane/virtio-blk.c  |    8 +-
>> > >>  hw/dataplane/virtio-net.c  |  444 
>> > >> ++++++++++++++++++++++++++++++++++++++++++++
>> > >>  hw/dataplane/virtio-net.h  |   32 ++++
>> > >>  hw/dataplane/vring.c       |   37 ++++
>> > >>  hw/dataplane/vring.h       |    3 +
>> > >>  hw/virtio-net.c            |   94 +++++-----
>> > >>  hw/virtio-net.h            |   64 +++++++
>> > >>  hw/virtio-pci.c            |    2 +-
>> > >>  include/net/tap.h          |    6 +
>> > >>  net/tap.c                  |   92 +++++++++-
>> > >>  14 files changed, 806 insertions(+), 75 deletions(-)
>> > >>  create mode 100644 hw/dataplane/virtio-net.c
>> > >>  create mode 100644 hw/dataplane/virtio-net.h
>> > >>
>> > >> --
>> > >> 1.7.4.4
>> > >>