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

[Stefan Hajnoczi]

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?

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.

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.

Stefan