Re: [Qemu-devel] [RFC PATCH v2 0/4] port network layer onto glib

[liu ping fan]

On Thu, Mar 28, 2013 at 10:55 PM, Stefan Hajnoczi <address@hidden> wrote:
>
> On Thu, Mar 28, 2013 at 03:55:51PM +0800, Liu Ping Fan wrote:
> > From: Liu Ping Fan <address@hidden>
> >
> > These series aim to make the whole network re-entrant, here only apply 
> > backend and frontend,
> > and for the netcore, separated patches have been sent out.  All of these 
> > will prepare us for
> > moving towards making network layer mutlit-thread.
> > Finally it would be omething like
> >    qemu -object io-thread,id=thread0 \
> >      -device virtio-net,rx[0]=thread0,tx[0]=thread0
> >
> > The brief of the whole aim and plan is documented on
> >   http://wiki.qemu.org/Features/network_reentrant
> >
> > The main issue is about GSource or AioContext,
> >   http://marc.info/?t=136315453300002&r=1&w=3
> > And I sumary the main points:
> >   disadvantage for current AioContext
> >    1st. need to define and expand interface for other fd events, while glib 
> > open this interface for user *
> >    2nd. need to add support for IOCanReadHandler, while gsource provide 
> > prepare, check method to allow more flexible control
> >    3rd. block layer's AioContext will block other AioContexts on the same 
> > thread.
> >    4th. need more document
> >  disadvantage for glib
> >    1st. if more than one fds on the same GSource, need re-implement 
> > something like aio_set_file_handler
> >
> > Since I have successed to port frontend on glib, there is no obstale to use 
> > glib.
> >
> >
> > v1->v2:
> >   1.NetClientState can associate with up to 2 GSource, for virtio net, one 
> > for tx, one for rx,
> >     so vq can run on different threads.
> >   2.make network front-end onto glib, currently virtio net dataplane
> >
> >
> > Liu Ping Fan (4):
> >   net: port tap onto glib
> >   net: resolve race of tap backend and its peer
> >   net: port hub onto glib
> >   net: port virtio net onto glib
> >
> >  hw/qdev-properties-system.c |    1 +
> >  hw/virtio-net.c             |  165 
> > +++++++++++++++++++++++++++++++++++++++++++
> >  hw/virtio.c                 |    6 ++
> >  hw/virtio.h                 |    2 +
> >  include/net/net.h           |   27 +++++++
> >  include/net/queue.h         |   14 ++++
> >  net/hub.c                   |   34 ++++++++-
> >  net/net.c                   |   97 +++++++++++++++++++++++++
> >  net/queue.c                 |    4 +-
> >  net/tap.c                   |   62 +++++++++++++---
> >  10 files changed, 397 insertions(+), 15 deletions(-)
>
> It seems the AioContext vs glib issue hasn't been settled yet.  My take
> is that glib is preferrable *if* we don't need to write too many
> helpers/wrappers on top (then we're basically back to rolling our own,
> AioContext).
>
> I was surprised by the amount of code required to listen on a file
> descriptor.  Are you sure there isn't a glib way of doing this that
> avoids rolling our own GSource?
>
Will diving more into the code, as mdroth's suggestion. Currently the
main issue is that glib seems no support for readalbe or writable.

> In the next series, please drop the hub re-entrancy stuff and virtio-net
> data plane.  Instead just focus on systematically moving existing net
> clients onto the event loop (net/*.c and NICs).  The only controversial
> issue there is AioContext vs glib, and once that's settled we can merge
> the patches.
>
What about the core (queue.c, net.c)?  Need I send them out at the
same time or after the backend(clients) convert finished?

> Please avoid layering violations - for example a comment about
> virtio-net in net.h, a comment about vhost in tap, or putting
> net_source_funcs in net.h.  I think converting all existing net clients
> will help make the code changes appropriate and eliminate these kinds of
> hacks which are because you're focussing just on virtio, tap, and hub
> here.

Ok, will convert all.

Regards,
Ping Fan
>
> Stefan