[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/5] net: spread hub on AioContexts
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/5] net: spread hub on AioContexts |
Date: |
Tue, 12 Mar 2013 09:50:26 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130219 Thunderbird/17.0.3 |
Il 07/03/2013 03:53, Liu Ping Fan ha scritto:
> From: Liu Ping Fan <address@hidden>
>
> Forward packet to other hub ports by their AioContext.
>
> Signed-off-by: Liu Ping Fan <address@hidden>
> ---
> hw/qdev-properties-system.c | 1 +
> include/block/aio.h | 1 +
> include/net/net.h | 5 +++++
> include/net/queue.h | 14 ++++++++++++++
> main-loop.c | 5 +++++
> net/hub.c | 33 ++++++++++++++++++++++++++++++---
> net/net.c | 1 +
> net/queue.c | 4 ++--
> 8 files changed, 59 insertions(+), 5 deletions(-)
>
> diff --git a/hw/qdev-properties-system.c b/hw/qdev-properties-system.c
> index ce3af22..587a335 100644
> --- a/hw/qdev-properties-system.c
> +++ b/hw/qdev-properties-system.c
> @@ -307,6 +307,7 @@ static void set_vlan(Object *obj, Visitor *v, void
> *opaque,
> name, prop->info->name);
> return;
> }
> + hubport->info->bind_ctx(hubport, qemu_get_aio_context());
> *ptr = hubport;
> }
>
> diff --git a/include/block/aio.h b/include/block/aio.h
> index 5b54d38..bcb5126 100644
> --- a/include/block/aio.h
> +++ b/include/block/aio.h
> @@ -229,6 +229,7 @@ bool qemu_aio_wait(void);
> void qemu_aio_set_event_notifier(EventNotifier *notifier,
> EventNotifierHandler *io_read,
> AioFlushEventNotifierHandler *io_flush);
> +AioContext *qemu_get_aio_context(void);
>
> #ifdef CONFIG_POSIX
> void qemu_aio_set_fd_handler(int fd,
> diff --git a/include/net/net.h b/include/net/net.h
> index cb049a1..9c2b357 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -8,6 +8,9 @@
> #include "net/queue.h"
> #include "migration/vmstate.h"
> #include "qapi-types.h"
> +#include "qemu/thread.h"
> +#include "block/aio.h"
> +
>
> #define MAX_QUEUE_NUM 1024
>
> @@ -44,6 +47,7 @@ typedef ssize_t (NetReceiveIOV)(NetClientState *, const
> struct iovec *, int);
> typedef void (NetCleanup) (NetClientState *);
> typedef void (LinkStatusChanged)(NetClientState *);
> typedef void (NetClientDestructor)(NetClientState *);
> +typedef void (NetClientBindCtx)(NetClientState *, AioContext *);
>
> typedef struct NetClientInfo {
> NetClientOptionsKind type;
> @@ -55,6 +59,7 @@ typedef struct NetClientInfo {
> NetCleanup *cleanup;
> LinkStatusChanged *link_status_changed;
> NetPoll *poll;
> + NetClientBindCtx *bind_ctx;
> } NetClientInfo;
>
> struct NetClientState {
> diff --git a/include/net/queue.h b/include/net/queue.h
> index fc02b33..f60e57f 100644
> --- a/include/net/queue.h
> +++ b/include/net/queue.h
> @@ -38,6 +38,20 @@ NetQueue *qemu_new_net_queue(void *opaque);
>
> void qemu_del_net_queue(NetQueue *queue);
>
> +void qemu_net_queue_append(NetQueue *queue,
> + NetClientState *sender,
> + unsigned flags,
> + const uint8_t *buf,
> + size_t size,
> + NetPacketSent *sent_cb);
> +
> +void qemu_net_queue_append_iov(NetQueue *queue,
> + NetClientState *sender,
> + unsigned flags,
> + const struct iovec *iov,
> + int iovcnt,
> + NetPacketSent *sent_cb);
> +
> ssize_t qemu_net_queue_send(NetQueue *queue,
> NetClientState *sender,
> unsigned flags,
> diff --git a/main-loop.c b/main-loop.c
> index 8c9b58c..eb80ff3 100644
> --- a/main-loop.c
> +++ b/main-loop.c
> @@ -109,6 +109,11 @@ static int qemu_signal_init(void)
>
> static AioContext *qemu_aio_context;
>
> +AioContext *qemu_get_aio_context(void)
> +{
> + return qemu_aio_context;
> +}
> +
> void qemu_notify_event(void)
> {
> if (!qemu_aio_context) {
> diff --git a/net/hub.c b/net/hub.c
> index df32074..73c1f26 100644
> --- a/net/hub.c
> +++ b/net/hub.c
> @@ -31,6 +31,8 @@ typedef struct NetHubPort {
> QLIST_ENTRY(NetHubPort) next;
> NetHub *hub;
> int id;
> + EventNotifier e;
> + AioContext *ctx;
> } NetHubPort;
>
> struct NetHub {
> @@ -52,11 +54,20 @@ static ssize_t net_hub_receive(NetHub *hub, NetHubPort
> *source_port,
> continue;
> }
>
> - qemu_send_packet(&port->nc, buf, len);
> + qemu_net_queue_append(port->nc.peer->send_queue, &port->nc,
> + QEMU_NET_PACKET_FLAG_NONE, buf, len, NULL);
> + event_notifier_set(&port->e);
Why are the context and the EventNotifier a property of the port, rather
than applicable to the NetClientState?
Please explain the design and where you want to go. We do not have a
crystal ball :) and lack of explanation can only prevent people from
looking at your patches.
The absolute best would be if every tiny step along the way provides a
benefit. For example, the patches that introduced AioContext enabled
asynchronous I/O on Windows. Without such an incentive, it is very
difficult that patches progress beyond the RFC phase.
Paolo
> }
> return len;
> }
>
> +static void hub_port_deliver_packet(void *opaque)
> +{
> + NetHubPort *port = (NetHubPort *)opaque;
> +
> + qemu_net_queue_flush(port->nc.peer->send_queue);
> +}
> +
> static ssize_t net_hub_receive_iov(NetHub *hub, NetHubPort *source_port,
> const struct iovec *iov, int iovcnt)
> {
> @@ -68,7 +79,9 @@ static ssize_t net_hub_receive_iov(NetHub *hub, NetHubPort
> *source_port,
> continue;
> }
>
> - qemu_sendv_packet(&port->nc, iov, iovcnt);
> + qemu_net_queue_append_iov(port->nc.peer->send_queue, &port->nc,
> + QEMU_NET_PACKET_FLAG_NONE, iov, iovcnt, NULL);
> + event_notifier_set(&port->e);
> }
> return len;
> }
> @@ -126,9 +139,22 @@ static void net_hub_port_cleanup(NetClientState *nc)
> {
> NetHubPort *port = DO_UPCAST(NetHubPort, nc, nc);
>
> + if (port->ctx) {
> + aio_set_fd_handler(port->ctx, event_notifier_get_fd(&port->e),
> + NULL, NULL, NULL, NULL);
> + }
> QLIST_REMOVE(port, next);
> }
>
> +static void net_hub_port_bind_ctx(NetClientState *nc, AioContext *ctx)
> +{
> + NetHubPort *port = DO_UPCAST(NetHubPort, nc, nc);
> +
> + port->ctx = ctx;
> + aio_set_fd_handler(ctx, event_notifier_get_fd(&port->e),
> + hub_port_deliver_packet, NULL, NULL, port);
> +}
> +
> static NetClientInfo net_hub_port_info = {
> .type = NET_CLIENT_OPTIONS_KIND_HUBPORT,
> .size = sizeof(NetHubPort),
> @@ -136,6 +162,7 @@ static NetClientInfo net_hub_port_info = {
> .receive = net_hub_port_receive,
> .receive_iov = net_hub_port_receive_iov,
> .cleanup = net_hub_port_cleanup,
> + .bind_ctx = net_hub_port_bind_ctx,
> };
>
> static NetHubPort *net_hub_port_new(NetHub *hub, const char *name)
> @@ -155,7 +182,7 @@ static NetHubPort *net_hub_port_new(NetHub *hub, const
> char *name)
> port = DO_UPCAST(NetHubPort, nc, nc);
> port->id = id;
> port->hub = hub;
> -
> + event_notifier_init(&port->e, 0);
> QLIST_INSERT_HEAD(&hub->ports, port, next);
>
> return port;
> diff --git a/net/net.c b/net/net.c
> index f3d67f8..104c5b2 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -781,6 +781,7 @@ static int net_client_init1(const void *object, int
> is_netdev, Error **errp)
> (opts->kind != NET_CLIENT_OPTIONS_KIND_NIC ||
> !opts->nic->has_netdev)) {
> peer = net_hub_add_port(u.net->has_vlan ? u.net->vlan : 0, NULL);
> + peer->info->bind_ctx(peer, qemu_get_aio_context());
> }
>
> if (net_client_init_fun[opts->kind](opts, name, peer) < 0) {
> diff --git a/net/queue.c b/net/queue.c
> index 859d02a..67959f8 100644
> --- a/net/queue.c
> +++ b/net/queue.c
> @@ -87,7 +87,7 @@ void qemu_del_net_queue(NetQueue *queue)
> g_free(queue);
> }
>
> -static void qemu_net_queue_append(NetQueue *queue,
> +void qemu_net_queue_append(NetQueue *queue,
> NetClientState *sender,
> unsigned flags,
> const uint8_t *buf,
> @@ -110,7 +110,7 @@ static void qemu_net_queue_append(NetQueue *queue,
> QTAILQ_INSERT_TAIL(&queue->packets, packet, entry);
> }
>
> -static void qemu_net_queue_append_iov(NetQueue *queue,
> +void qemu_net_queue_append_iov(NetQueue *queue,
> NetClientState *sender,
> unsigned flags,
> const struct iovec *iov,
>