* [Qemu-devel] [RFC PATCH v5 01/14] util: introduce gsource event abstraction
2013-04-26 2:47 [Qemu-devel] [RFC PATCH v5 00/14] port network layer onto glib Liu Ping Fan
@ 2013-04-26 2:47 ` Liu Ping Fan
2013-04-26 9:19 ` Stefan Hajnoczi
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 02/14] net: introduce bind_ctx to NetClientInfo Liu Ping Fan
` (12 subsequent siblings)
13 siblings, 1 reply; 22+ messages in thread
From: Liu Ping Fan @ 2013-04-26 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: mdroth, Jan Kiszka, Stefan Hajnoczi, Anthony Liguori,
Paolo Bonzini
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
Introduce two structs EventGSource, EventsGSource
EventGSource is used to abstract the event with single backend file.
EventsGSource is used to abstract the event with dynamically changed
backend file, ex, slirp.
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
util/Makefile.objs | 1 +
util/event_gsource.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++++++
util/event_gsource.h | 49 +++++++++++++++
3 files changed, 208 insertions(+), 0 deletions(-)
create mode 100644 util/event_gsource.c
create mode 100644 util/event_gsource.h
diff --git a/util/Makefile.objs b/util/Makefile.objs
index 495a178..a676d7d 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -8,3 +8,4 @@ util-obj-y += error.o qemu-error.o
util-obj-$(CONFIG_POSIX) += compatfd.o
util-obj-y += iov.o aes.o qemu-config.o qemu-sockets.o uri.o notify.o
util-obj-y += qemu-option.o qemu-progress.o
+util-obj-y += event_gsource.o
diff --git a/util/event_gsource.c b/util/event_gsource.c
new file mode 100644
index 0000000..9cfdb4a
--- /dev/null
+++ b/util/event_gsource.c
@@ -0,0 +1,158 @@
+/*
+ * Copyright (C) 2013 IBM
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; under version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "event_gsource.h"
+#include "qemu/bitops.h"
+
+static gboolean prepare(GSource *src, gint *time)
+{
+ EventGSource *nsrc = (EventGSource *)src;
+ int events = 0;
+
+ if (!nsrc->readable && !nsrc->writable) {
+ return false;
+ }
+ if (nsrc->readable) {
+ events = nsrc->readable(nsrc->opaque);
+ }
+ if ((nsrc->writable)) {
+ events |= nsrc->writable(nsrc->opaque);
+ }
+ nsrc->gfd.events = events;
+
+ return false;
+}
+
+static gboolean check(GSource *src)
+{
+ EventGSource *nsrc = (EventGSource *)src;
+
+ if (nsrc->gfd.revents & nsrc->gfd.events) {
+ return true;
+ }
+ return false;
+}
+
+static gboolean dispatch(GSource *src, GSourceFunc cb, gpointer data)
+{
+ gboolean ret = false;
+
+ if (cb) {
+ ret = cb(data);
+ }
+ return ret;
+}
+
+static GSourceFuncs net_gsource_funcs = {
+ prepare,
+ check,
+ dispatch,
+ NULL
+};
+
+EventGSource *event_source_new(int fd, GSourceFunc dispatch_cb, void *opaque)
+{
+ EventGSource *nsrc = (EventGSource *)g_source_new(&net_gsource_funcs,
+ sizeof(EventGSource));
+ nsrc->gfd.fd = fd;
+ nsrc->opaque = opaque;
+ g_source_set_callback(&nsrc->source, dispatch_cb, nsrc, NULL);
+ g_source_add_poll(&nsrc->source, &nsrc->gfd);
+
+ return nsrc;
+}
+
+void event_source_release(EventGSource *src)
+{
+ g_source_destroy(&src->source);
+}
+
+GPollFD *events_source_add_gfd(EventsGSource *src, int fd)
+{
+ GPollFD *retfd;
+
+ retfd = g_slice_alloc(sizeof(GPollFD));
+ retfd->events = 0;
+ retfd->fd = fd;
+ src->pollfds_list = g_list_append(src->pollfds_list, retfd);
+ if (fd > 0) {
+ g_source_add_poll(&src->source, retfd);
+ }
+
+ return retfd;
+}
+
+void events_source_remove_gfd(EventsGSource *src, GPollFD *pollfd)
+{
+ g_source_remove_poll(&src->source, pollfd);
+ src->pollfds_list = g_list_remove(src->pollfds_list, pollfd);
+ g_slice_free(GPollFD, pollfd);
+}
+
+static gboolean events_source_check(GSource *src)
+{
+ EventsGSource *nsrc = (EventsGSource *)src;
+ GList *cur;
+ GPollFD *gfd;
+
+ cur = nsrc->pollfds_list;
+ while (cur) {
+ gfd = cur->data;
+ if (gfd->fd > 0 && (gfd->revents & gfd->events)) {
+ return true;
+ }
+ cur = g_list_next(cur);
+ }
+
+ return false;
+}
+
+static gboolean events_source_dispatch(GSource *src, GSourceFunc cb,
+ gpointer data)
+{
+ gboolean ret = false;
+
+ if (cb) {
+ ret = cb(data);
+ }
+ return ret;
+}
+
+EventsGSource *events_source_new(GPrepare prepare, GSourceFunc dispatch_cb,
+ void *opaque)
+{
+ EventsGSource *src;
+ GSourceFuncs *gfuncs = g_new0(GSourceFuncs, 1);
+ gfuncs->prepare = prepare;
+ gfuncs->check = events_source_check,
+ gfuncs->dispatch = events_source_dispatch,
+
+ src = (EventsGSource *)g_source_new(gfuncs, sizeof(EventsGSource));
+ src->gfuncs = gfuncs;
+ src->pollfds_list = NULL;
+ src->opaque = opaque;
+ g_source_set_callback(&src->source, dispatch_cb, src, NULL);
+
+ return src;
+}
+
+void events_source_release(EventsGSource *src)
+{
+ g_list_free(src->pollfds_list);
+ g_free(src->gfuncs);
+ g_source_destroy(&src->source);
+}
+
diff --git a/util/event_gsource.h b/util/event_gsource.h
new file mode 100644
index 0000000..25b15d7
--- /dev/null
+++ b/util/event_gsource.h
@@ -0,0 +1,49 @@
+/*
+ * Copyright (C) 2013 IBM
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; under version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef EVENT_GSOURCE_H
+#define EVENT_GSOURCE_H
+#include "qemu-common.h"
+
+typedef gushort (*Pollable)(void *opaque);
+typedef gboolean (*GPrepare)(GSource *source, gint *timeout_);
+
+/* single fd drive gsource */
+typedef struct EventGSource {
+ GSource source;
+ GPollFD gfd;
+ Pollable readable;
+ Pollable writable;
+ void *opaque;
+} EventGSource;
+
+EventGSource *event_source_new(int fd, GSourceFunc dispatch_cb, void *opaque);
+void event_source_release(EventGSource *src);
+
+/* multi fd drive gsource*/
+typedef struct EventsGSource {
+ GSource source;
+ GList *pollfds_list;
+ GSourceFuncs *gfuncs;
+ void *opaque;
+} EventsGSource;
+
+EventsGSource *events_source_new(GPrepare prepare, GSourceFunc dispatch_cb,
+ void *opaque);
+void events_source_release(EventsGSource *src);
+GPollFD *events_source_add_gfd(EventsGSource *src, int fd);
+void events_source_remove_gfd(EventsGSource *src, GPollFD *pollfd);
+#endif
--
1.7.4.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [RFC PATCH v5 01/14] util: introduce gsource event abstraction
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 01/14] util: introduce gsource event abstraction Liu Ping Fan
@ 2013-04-26 9:19 ` Stefan Hajnoczi
2013-04-27 2:11 ` liu ping fan
0 siblings, 1 reply; 22+ messages in thread
From: Stefan Hajnoczi @ 2013-04-26 9:19 UTC (permalink / raw)
To: Liu Ping Fan
Cc: mdroth, Jan Kiszka, qemu-devel, Anthony Liguori, Paolo Bonzini
On Fri, Apr 26, 2013 at 10:47:22AM +0800, Liu Ping Fan wrote:
> +GPollFD *events_source_add_gfd(EventsGSource *src, int fd)
> +{
> + GPollFD *retfd;
> +
> + retfd = g_slice_alloc(sizeof(GPollFD));
> + retfd->events = 0;
> + retfd->fd = fd;
> + src->pollfds_list = g_list_append(src->pollfds_list, retfd);
> + if (fd > 0) {
0 (stdin) is a valid fd number. Maybe just assert(fd >= 0)?
> +static gboolean events_source_check(GSource *src)
> +{
> + EventsGSource *nsrc = (EventsGSource *)src;
> + GList *cur;
> + GPollFD *gfd;
> +
> + cur = nsrc->pollfds_list;
> + while (cur) {
> + gfd = cur->data;
> + if (gfd->fd > 0 && (gfd->revents & gfd->events)) {
revents will always be 0 if fd is invalid, since we didn't call
g_source_add_poll(). Is there a reason to perform the fd > 0 check
again?
> +void events_source_release(EventsGSource *src)
> +{
assert that pollfds_list is empty? We don't g_slice_free() GPollFDs so
it must be empty here.
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [RFC PATCH v5 01/14] util: introduce gsource event abstraction
2013-04-26 9:19 ` Stefan Hajnoczi
@ 2013-04-27 2:11 ` liu ping fan
2013-04-29 8:00 ` Stefan Hajnoczi
0 siblings, 1 reply; 22+ messages in thread
From: liu ping fan @ 2013-04-27 2:11 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: mdroth, Jan Kiszka, qemu-devel, Anthony Liguori, Paolo Bonzini
On Fri, Apr 26, 2013 at 5:19 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> On Fri, Apr 26, 2013 at 10:47:22AM +0800, Liu Ping Fan wrote:
>> +GPollFD *events_source_add_gfd(EventsGSource *src, int fd)
>> +{
>> + GPollFD *retfd;
>> +
>> + retfd = g_slice_alloc(sizeof(GPollFD));
>> + retfd->events = 0;
>> + retfd->fd = fd;
>> + src->pollfds_list = g_list_append(src->pollfds_list, retfd);
>> + if (fd > 0) {
>
> 0 (stdin) is a valid fd number. Maybe just assert(fd >= 0)?
>
Yes, 0 should be allowed. Here, the reason to use check instead of
assert , is that socreate() in slirp is a good place to call
_add_gfd, but unfortunately, at that time, its socket handler so->s =
-1; So we create the GPollFD ahead, and delay to call
g_source_add_poll()
>> +static gboolean events_source_check(GSource *src)
>> +{
>> + EventsGSource *nsrc = (EventsGSource *)src;
>> + GList *cur;
>> + GPollFD *gfd;
>> +
>> + cur = nsrc->pollfds_list;
>> + while (cur) {
>> + gfd = cur->data;
>> + if (gfd->fd > 0 && (gfd->revents & gfd->events)) {
>
> revents will always be 0 if fd is invalid, since we didn't call
> g_source_add_poll(). Is there a reason to perform the fd > 0 check
> again?
>
As explained above, we should skip the case gfd->fd=-1, which may
occur in pollfds_list.
>> +void events_source_release(EventsGSource *src)
>> +{
>
> assert that pollfds_list is empty? We don't g_slice_free() GPollFDs so
> it must be empty here.
Do you mean that events_source_add_gfd/events_source_remove_gfd are
paired, so here, we ensure that pollfds_list==NULL ?
Thanks
Pingfan
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [RFC PATCH v5 01/14] util: introduce gsource event abstraction
2013-04-27 2:11 ` liu ping fan
@ 2013-04-29 8:00 ` Stefan Hajnoczi
0 siblings, 0 replies; 22+ messages in thread
From: Stefan Hajnoczi @ 2013-04-29 8:00 UTC (permalink / raw)
To: liu ping fan
Cc: mdroth, Jan Kiszka, qemu-devel, Anthony Liguori, Paolo Bonzini
On Sat, Apr 27, 2013 at 10:11:40AM +0800, liu ping fan wrote:
> On Fri, Apr 26, 2013 at 5:19 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > On Fri, Apr 26, 2013 at 10:47:22AM +0800, Liu Ping Fan wrote:
> >> +GPollFD *events_source_add_gfd(EventsGSource *src, int fd)
> >> +{
> >> + GPollFD *retfd;
> >> +
> >> + retfd = g_slice_alloc(sizeof(GPollFD));
> >> + retfd->events = 0;
> >> + retfd->fd = fd;
> >> + src->pollfds_list = g_list_append(src->pollfds_list, retfd);
> >> + if (fd > 0) {
> >
> > 0 (stdin) is a valid fd number. Maybe just assert(fd >= 0)?
> >
> Yes, 0 should be allowed. Here, the reason to use check instead of
> assert , is that socreate() in slirp is a good place to call
> _add_gfd, but unfortunately, at that time, its socket handler so->s =
> -1; So we create the GPollFD ahead, and delay to call
> g_source_add_poll()
ok
> >> +static gboolean events_source_check(GSource *src)
> >> +{
> >> + EventsGSource *nsrc = (EventsGSource *)src;
> >> + GList *cur;
> >> + GPollFD *gfd;
> >> +
> >> + cur = nsrc->pollfds_list;
> >> + while (cur) {
> >> + gfd = cur->data;
> >> + if (gfd->fd > 0 && (gfd->revents & gfd->events)) {
> >
> > revents will always be 0 if fd is invalid, since we didn't call
> > g_source_add_poll(). Is there a reason to perform the fd > 0 check
> > again?
> >
> As explained above, we should skip the case gfd->fd=-1, which may
> occur in pollfds_list.
ok
> >> +void events_source_release(EventsGSource *src)
> >> +{
> >
> > assert that pollfds_list is empty? We don't g_slice_free() GPollFDs so
> > it must be empty here.
>
> Do you mean that events_source_add_gfd/events_source_remove_gfd are
> paired, so here, we ensure that pollfds_list==NULL ?
Yes. It is an error to hold a GPollFD across events_source_release() so
you could place an assert() here to check.
Stefan
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [RFC PATCH v5 02/14] net: introduce bind_ctx to NetClientInfo
2013-04-26 2:47 [Qemu-devel] [RFC PATCH v5 00/14] port network layer onto glib Liu Ping Fan
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 01/14] util: introduce gsource event abstraction Liu Ping Fan
@ 2013-04-26 2:47 ` Liu Ping Fan
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 03/14] net: port tap onto GSource Liu Ping Fan
` (11 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Liu Ping Fan @ 2013-04-26 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: mdroth, Jan Kiszka, Stefan Hajnoczi, Anthony Liguori,
Paolo Bonzini
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
Introduce bind_ctx interface for NetClientState. It will help to
bind NetClientState with a GSource. Currently, these GSource attached
with default context, but in future, after resolving all the race
condition in network layer, NetClientStates can run on different
threads
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
include/net/net.h | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/include/net/net.h b/include/net/net.h
index cb049a1..88332d2 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -44,6 +44,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 *, GMainContext *);
typedef struct NetClientInfo {
NetClientOptionsKind type;
@@ -55,6 +56,7 @@ typedef struct NetClientInfo {
NetCleanup *cleanup;
LinkStatusChanged *link_status_changed;
NetPoll *poll;
+ NetClientBindCtx *bind_ctx;
} NetClientInfo;
struct NetClientState {
--
1.7.4.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [RFC PATCH v5 03/14] net: port tap onto GSource
2013-04-26 2:47 [Qemu-devel] [RFC PATCH v5 00/14] port network layer onto glib Liu Ping Fan
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 01/14] util: introduce gsource event abstraction Liu Ping Fan
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 02/14] net: introduce bind_ctx to NetClientInfo Liu Ping Fan
@ 2013-04-26 2:47 ` Liu Ping Fan
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 04/14] net: port vde " Liu Ping Fan
` (10 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Liu Ping Fan @ 2013-04-26 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: mdroth, Jan Kiszka, Stefan Hajnoczi, Anthony Liguori,
Paolo Bonzini
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
net/tap.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++---------
1 files changed, 54 insertions(+), 10 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index daab350..5f4d59f 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -41,6 +41,7 @@
#include "qemu/error-report.h"
#include "net/tap.h"
+#include "util/event_gsource.h"
#include "hw/vhost_net.h"
@@ -62,6 +63,7 @@ typedef struct TAPState {
bool enabled;
VHostNetState *vhost_net;
unsigned host_vnet_hdr_len;
+ EventGSource *nsrc;
} TAPState;
static int launch_script(const char *setup_script, const char *ifname, int fd);
@@ -70,25 +72,48 @@ static int tap_can_send(void *opaque);
static void tap_send(void *opaque);
static void tap_writable(void *opaque);
-static void tap_update_fd_handler(TAPState *s)
+static gushort readable(void *opaque)
{
- qemu_set_fd_handler2(s->fd,
- s->read_poll && s->enabled ? tap_can_send : NULL,
- s->read_poll && s->enabled ? tap_send : NULL,
- s->write_poll && s->enabled ? tap_writable : NULL,
- s);
+ TAPState *s = opaque;
+
+ if (s->enabled && s->read_poll &&
+ tap_can_send(s)) {
+ return G_IO_IN;
+ }
+ return 0;
+}
+
+static gushort writable(void *opaque)
+{
+ TAPState *s = opaque;
+
+ if (s->enabled && s->write_poll) {
+ return G_IO_OUT;
+ }
+ return 0;
+}
+
+static gboolean tap_handler(gpointer data)
+{
+ EventGSource *nsrc = data;
+
+ if (nsrc->gfd.revents & G_IO_IN) {
+ tap_send(nsrc->opaque);
+ }
+ if (nsrc->gfd.revents & G_IO_OUT) {
+ tap_writable(nsrc->opaque);
+ }
+ return true;
}
static void tap_read_poll(TAPState *s, bool enable)
{
s->read_poll = enable;
- tap_update_fd_handler(s);
}
static void tap_write_poll(TAPState *s, bool enable)
{
s->write_poll = enable;
- tap_update_fd_handler(s);
}
static void tap_writable(void *opaque)
@@ -291,6 +316,7 @@ static void tap_cleanup(NetClientState *nc)
tap_read_poll(s, false);
tap_write_poll(s, false);
+ event_source_release(s->nsrc);
close(s->fd);
s->fd = -1;
}
@@ -300,6 +326,12 @@ static void tap_poll(NetClientState *nc, bool enable)
TAPState *s = DO_UPCAST(TAPState, nc, nc);
tap_read_poll(s, enable);
tap_write_poll(s, enable);
+
+ if (!enable) {
+ g_source_remove_poll(&s->nsrc->source, &s->nsrc->gfd);
+ } else {
+ g_source_add_poll(&s->nsrc->source, &s->nsrc->gfd);
+ }
}
int tap_get_fd(NetClientState *nc)
@@ -309,6 +341,13 @@ int tap_get_fd(NetClientState *nc)
return s->fd;
}
+static void tap_bind_ctx(NetClientState *nc, GMainContext *ctx)
+{
+ TAPState *s = DO_UPCAST(TAPState, nc, nc);
+
+ g_source_attach(&s->nsrc->source, ctx);
+}
+
/* fd support */
static NetClientInfo net_tap_info = {
@@ -319,6 +358,7 @@ static NetClientInfo net_tap_info = {
.receive_iov = tap_receive_iov,
.poll = tap_poll,
.cleanup = tap_cleanup,
+ .bind_ctx = tap_bind_ctx,
};
static TAPState *net_tap_fd_init(NetClientState *peer,
@@ -596,6 +636,7 @@ static int net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
int vnet_hdr, int fd)
{
TAPState *s;
+ EventGSource *nsrc;
s = net_tap_fd_init(peer, model, name, fd, vnet_hdr);
if (!s) {
@@ -606,6 +647,11 @@ static int net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
if (tap_set_sndbuf(s->fd, tap) < 0) {
return -1;
}
+ nsrc = event_source_new(s->fd, tap_handler, s);
+ nsrc->readable = readable;
+ nsrc->writable = writable;
+ s->nsrc = nsrc;
+ s->nc.info->bind_ctx(&s->nc, NULL);
if (tap->has_fd || tap->has_fds) {
snprintf(s->nc.info_str, sizeof(s->nc.info_str), "fd=%d", fd);
@@ -844,7 +890,6 @@ int tap_enable(NetClientState *nc)
ret = tap_fd_enable(s->fd);
if (ret == 0) {
s->enabled = true;
- tap_update_fd_handler(s);
}
return ret;
}
@@ -862,7 +907,6 @@ int tap_disable(NetClientState *nc)
if (ret == 0) {
qemu_purge_queued_packets(nc);
s->enabled = false;
- tap_update_fd_handler(s);
}
return ret;
}
--
1.7.4.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [RFC PATCH v5 04/14] net: port vde onto GSource
2013-04-26 2:47 [Qemu-devel] [RFC PATCH v5 00/14] port network layer onto glib Liu Ping Fan
` (2 preceding siblings ...)
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 03/14] net: port tap onto GSource Liu Ping Fan
@ 2013-04-26 2:47 ` Liu Ping Fan
2013-04-26 9:25 ` Stefan Hajnoczi
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 05/14] net: port socket to GSource Liu Ping Fan
` (9 subsequent siblings)
13 siblings, 1 reply; 22+ messages in thread
From: Liu Ping Fan @ 2013-04-26 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: mdroth, Jan Kiszka, Stefan Hajnoczi, Anthony Liguori,
Paolo Bonzini
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
net/vde.c | 31 +++++++++++++++++++++++++++++--
1 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/net/vde.c b/net/vde.c
index 4dea32d..6dbde04 100644
--- a/net/vde.c
+++ b/net/vde.c
@@ -30,10 +30,12 @@
#include "qemu-common.h"
#include "qemu/option.h"
#include "qemu/main-loop.h"
+#include "util/event_gsource.h"
typedef struct VDEState {
NetClientState nc;
VDECONN *vde;
+ EventGSource *nsrc;
} VDEState;
static void vde_to_qemu(void *opaque)
@@ -60,20 +62,43 @@ static ssize_t vde_receive(NetClientState *nc, const uint8_t *buf, size_t size)
return ret;
}
+static gboolean vde_handler(gpointer data)
+{
+ EventGSource *nsrc = (EventGSource *)data;
+
+ if (nsrc->gfd.revents & G_IO_IN) {
+ vde_to_qemu(nsrc->opaque);
+ }
+ return true;
+}
+
static void vde_cleanup(NetClientState *nc)
{
VDEState *s = DO_UPCAST(VDEState, nc, nc);
- qemu_set_fd_handler(vde_datafd(s->vde), NULL, NULL, NULL);
+ event_source_release(s->nsrc);
vde_close(s->vde);
}
+static void vde_bind_ctx(NetClientState *nc, GMainContext *ctx)
+{
+ VDEState *s = DO_UPCAST(VDEState, nc, nc);
+
+ g_source_attach(&s->nsrc->source, ctx);
+}
+
static NetClientInfo net_vde_info = {
.type = NET_CLIENT_OPTIONS_KIND_VDE,
.size = sizeof(VDEState),
.receive = vde_receive,
.cleanup = vde_cleanup,
+ .bind_ctx = vde_bind_ctx,
};
+static gushort readable(void *opaque)
+{
+ return G_IO_IN;
+}
+
static int net_vde_init(NetClientState *peer, const char *model,
const char *name, const char *sock,
int port, const char *group, int mode)
@@ -104,7 +129,9 @@ static int net_vde_init(NetClientState *peer, const char *model,
s->vde = vde;
- qemu_set_fd_handler(vde_datafd(s->vde), vde_to_qemu, NULL, s);
+ s->nsrc = event_source_new(vde_datafd(vde), vde_handler, s);
+ s->nsrc->readable = readable;
+ nc->info->bind_ctx(nc, NULL);
return 0;
}
--
1.7.4.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [RFC PATCH v5 05/14] net: port socket to GSource
2013-04-26 2:47 [Qemu-devel] [RFC PATCH v5 00/14] port network layer onto glib Liu Ping Fan
` (3 preceding siblings ...)
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 04/14] net: port vde " Liu Ping Fan
@ 2013-04-26 2:47 ` Liu Ping Fan
2013-04-26 9:48 ` Stefan Hajnoczi
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 06/14] net: port tap-win32 onto GSource Liu Ping Fan
` (8 subsequent siblings)
13 siblings, 1 reply; 22+ messages in thread
From: Liu Ping Fan @ 2013-04-26 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: mdroth, Jan Kiszka, Stefan Hajnoczi, Anthony Liguori,
Paolo Bonzini
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
Port NetSocketState onto NetClientSource. The only thing specail is that
owning to the socket's state machine changes, we need to change the handler.
We implement that by destroy the old NetClientSource and attach a new one
with NetSocketState.
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
net/socket.c | 196 +++++++++++++++++++++++++++++++++++++++++++++++-----------
1 files changed, 159 insertions(+), 37 deletions(-)
diff --git a/net/socket.c b/net/socket.c
index 396dc8c..abdb809 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -31,6 +31,8 @@
#include "qemu/option.h"
#include "qemu/sockets.h"
#include "qemu/iov.h"
+#include "util/event_gsource.h"
+
typedef struct NetSocketState {
NetClientState nc;
@@ -42,13 +44,15 @@ typedef struct NetSocketState {
unsigned int send_index; /* number of bytes sent (only SOCK_STREAM) */
uint8_t buf[4096];
struct sockaddr_in dgram_dst; /* contains inet host and port destination iff connectionless (SOCK_DGRAM) */
- IOHandler *send_fn; /* differs between SOCK_STREAM/SOCK_DGRAM */
bool read_poll; /* waiting to receive data? */
bool write_poll; /* waiting to transmit data? */
+ EventGSource *nsrc;
} NetSocketState;
-static void net_socket_accept(void *opaque);
static void net_socket_writable(void *opaque);
+static gboolean net_socket_listen_handler(gpointer data);
+static gboolean net_socket_establish_handler(gpointer data);
+
/* Only read packets from socket when peer can receive them */
static int net_socket_can_send(void *opaque)
@@ -58,25 +62,14 @@ static int net_socket_can_send(void *opaque)
return qemu_can_send_packet(&s->nc);
}
-static void net_socket_update_fd_handler(NetSocketState *s)
-{
- qemu_set_fd_handler2(s->fd,
- s->read_poll ? net_socket_can_send : NULL,
- s->read_poll ? s->send_fn : NULL,
- s->write_poll ? net_socket_writable : NULL,
- s);
-}
-
static void net_socket_read_poll(NetSocketState *s, bool enable)
{
s->read_poll = enable;
- net_socket_update_fd_handler(s);
}
static void net_socket_write_poll(NetSocketState *s, bool enable)
{
s->write_poll = enable;
- net_socket_update_fd_handler(s);
}
static void net_socket_writable(void *opaque)
@@ -141,6 +134,59 @@ static ssize_t net_socket_receive_dgram(NetClientState *nc, const uint8_t *buf,
return ret;
}
+static gushort socket_connecting_readable(void *opaque)
+{
+ return G_IO_IN;
+}
+
+static gushort socket_listen_readable(void *opaque)
+{
+ /* listen only handle in-req, no err */
+ return G_IO_IN;
+}
+
+static gushort socket_establish_readable(void *opaque)
+{
+ NetSocketState *s = opaque;
+
+ /* rely on net_socket_send to handle err */
+ if (s->read_poll && net_socket_can_send(s)) {
+ return G_IO_IN|G_IO_HUP|G_IO_ERR;
+ }
+ return G_IO_HUP|G_IO_ERR;
+}
+
+static gushort socket_establish_writable(void *opaque)
+{
+ NetSocketState *s = opaque;
+
+ if (s->write_poll) {
+ return G_IO_OUT;
+ }
+ return 0;
+}
+
+static gushort socket_dgram_readable(void *opaque)
+{
+ NetSocketState *s = opaque;
+
+ /* rely on net_socket_send_dgram to handle err */
+ if (s->read_poll && net_socket_can_send(s)) {
+ return G_IO_IN|G_IO_ERR;
+ }
+ return G_IO_ERR;
+}
+
+static gushort socket_dgram_writable(void *opaque)
+{
+ NetSocketState *s = opaque;
+
+ if (s->write_poll) {
+ return G_IO_OUT;
+ }
+ return 0;
+}
+
static void net_socket_send(void *opaque)
{
NetSocketState *s = opaque;
@@ -160,7 +206,11 @@ static void net_socket_send(void *opaque)
net_socket_read_poll(s, false);
net_socket_write_poll(s, false);
if (s->listen_fd != -1) {
- qemu_set_fd_handler(s->listen_fd, net_socket_accept, NULL, s);
+ event_source_release(s->nsrc);
+ s->nsrc = event_source_new(s->listen_fd, net_socket_listen_handler,
+ s);
+ s->nsrc->readable = socket_listen_readable;
+ s->nc.info->bind_ctx(&s->nc, NULL);
}
closesocket(s->fd);
@@ -231,6 +281,8 @@ static void net_socket_send_dgram(void *opaque)
/* end of connection */
net_socket_read_poll(s, false);
net_socket_write_poll(s, false);
+ /* for dgram err, removing it */
+ g_source_remove_poll(&s->nsrc->source, &s->nsrc->gfd);
return;
}
qemu_send_packet(&s->nc, s->buf, size);
@@ -331,6 +383,14 @@ static void net_socket_cleanup(NetClientState *nc)
closesocket(s->listen_fd);
s->listen_fd = -1;
}
+ event_source_release(s->nsrc);
+}
+
+static void net_socket_bind_ctx(NetClientState *nc, GMainContext *ctx)
+{
+ NetSocketState *s = DO_UPCAST(NetSocketState, nc, nc);
+
+ g_source_attach(&s->nsrc->source, ctx);
}
static NetClientInfo net_dgram_socket_info = {
@@ -338,8 +398,23 @@ static NetClientInfo net_dgram_socket_info = {
.size = sizeof(NetSocketState),
.receive = net_socket_receive_dgram,
.cleanup = net_socket_cleanup,
+ .bind_ctx = net_socket_bind_ctx,
};
+static gboolean net_socket_dgram_handler(gpointer data)
+{
+ EventGSource *nsrc = (EventGSource *)data;
+ NetSocketState *s = nsrc->opaque;
+
+ /* for err, unregister the handler */
+ if (nsrc->gfd.revents & (G_IO_IN|G_IO_ERR)) {
+ net_socket_send_dgram(s);
+ } else {
+ net_socket_writable(s);
+ }
+ return true;
+}
+
static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer,
const char *model,
const char *name,
@@ -393,8 +468,12 @@ static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer,
s->fd = fd;
s->listen_fd = -1;
- s->send_fn = net_socket_send_dgram;
+ s->nsrc = event_source_new(fd, net_socket_dgram_handler, s);
+ s->nsrc->readable = socket_dgram_readable;
+ s->nsrc->writable = socket_dgram_writable;
+ nc->info->bind_ctx(nc, NULL);
net_socket_read_poll(s, true);
+ net_socket_write_poll(s, true);
/* mcast: save bound address as dst */
if (is_connected) {
@@ -408,20 +487,30 @@ err:
return NULL;
}
-static void net_socket_connect(void *opaque)
-{
- NetSocketState *s = opaque;
- s->send_fn = net_socket_send;
- net_socket_read_poll(s, true);
-}
-
static NetClientInfo net_socket_info = {
.type = NET_CLIENT_OPTIONS_KIND_SOCKET,
.size = sizeof(NetSocketState),
.receive = net_socket_receive,
.cleanup = net_socket_cleanup,
+ .bind_ctx = net_socket_bind_ctx,
};
+static gboolean net_socket_connect_handler(gpointer data)
+{
+ EventGSource *nsrc = data;
+ NetSocketState *s = nsrc->opaque;
+
+ event_source_release(s->nsrc);
+ s->nsrc = event_source_new(s->fd, net_socket_establish_handler, s);
+ s->nsrc->readable = socket_establish_readable;
+ s->nsrc->writable = socket_establish_writable;
+ s->nc.info->bind_ctx(&s->nc, NULL);
+ net_socket_read_poll(s, true);
+ net_socket_write_poll(s, true);
+
+ return true;
+}
+
static NetSocketState *net_socket_fd_init_stream(NetClientState *peer,
const char *model,
const char *name,
@@ -440,9 +529,20 @@ static NetSocketState *net_socket_fd_init_stream(NetClientState *peer,
s->listen_fd = -1;
if (is_connected) {
- net_socket_connect(s);
+ assert(!s->nsrc);
+ s->nsrc = event_source_new(fd, net_socket_establish_handler, s);
+ s->nsrc->readable = socket_establish_readable;
+ s->nsrc->writable = socket_establish_writable;
+ nc->info->bind_ctx(nc, NULL);
+ net_socket_read_poll(s, true);
+ net_socket_write_poll(s, true);
} else {
- qemu_set_fd_handler(s->fd, NULL, net_socket_connect, s);
+ assert(!s->nsrc);
+ s->nsrc = event_source_new(fd, net_socket_connect_handler, s);
+ s->nsrc->readable = socket_connecting_readable;
+ nc->info->bind_ctx(nc, NULL);
+ net_socket_read_poll(s, true);
+ net_socket_write_poll(s, false);
}
return s;
}
@@ -473,30 +573,49 @@ static NetSocketState *net_socket_fd_init(NetClientState *peer,
return NULL;
}
-static void net_socket_accept(void *opaque)
+static gboolean net_socket_establish_handler(gpointer data)
{
- NetSocketState *s = opaque;
+ EventGSource *nsrc = (EventGSource *)data;
+ NetSocketState *s = nsrc->opaque;
+
+ /* for err case, resort to the logic in net_socket_send to recover */
+ if (nsrc->gfd.revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) {
+ net_socket_send(s);
+ }
+ if ((nsrc->gfd.revents & G_IO_OUT)) {
+ net_socket_writable(s);
+ }
+ return true;
+}
+
+static gboolean net_socket_listen_handler(gpointer data)
+{
+ EventGSource *nsrc = data;
+ NetSocketState *s = nsrc->opaque;
struct sockaddr_in saddr;
socklen_t len;
int fd;
- for(;;) {
- len = sizeof(saddr);
- fd = qemu_accept(s->listen_fd, (struct sockaddr *)&saddr, &len);
- if (fd < 0 && errno != EINTR) {
- return;
- } else if (fd >= 0) {
- qemu_set_fd_handler(s->listen_fd, NULL, NULL, NULL);
- break;
- }
+ len = sizeof(saddr);
+ fd = qemu_accept(s->listen_fd, (struct sockaddr *)&saddr, &len);
+ if (fd < 0 && errno != EINTR) {
+ return false;
}
s->fd = fd;
s->nc.link_down = false;
- net_socket_connect(s);
+ /* prevent more than one connect req */
+ event_source_release(s->nsrc);
+ s->nsrc = event_source_new(fd, net_socket_establish_handler, s);
+ s->nsrc->readable = socket_establish_readable;
+ s->nsrc->writable = socket_establish_writable;
+ s->nc.info->bind_ctx(&s->nc, NULL);
+ net_socket_read_poll(s, true);
snprintf(s->nc.info_str, sizeof(s->nc.info_str),
"socket: connection from %s:%d",
inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
+
+ return true;
}
static int net_socket_listen_init(NetClientState *peer,
@@ -542,7 +661,10 @@ static int net_socket_listen_init(NetClientState *peer,
s->listen_fd = fd;
s->nc.link_down = true;
- qemu_set_fd_handler(s->listen_fd, net_socket_accept, NULL, s);
+ s->nsrc = event_source_new(fd, net_socket_listen_handler, s);
+ s->nsrc->readable = socket_listen_readable;
+ nc->info->bind_ctx(nc, NULL);
+
return 0;
}
--
1.7.4.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [RFC PATCH v5 05/14] net: port socket to GSource
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 05/14] net: port socket to GSource Liu Ping Fan
@ 2013-04-26 9:48 ` Stefan Hajnoczi
2013-04-27 7:09 ` liu ping fan
0 siblings, 1 reply; 22+ messages in thread
From: Stefan Hajnoczi @ 2013-04-26 9:48 UTC (permalink / raw)
To: Liu Ping Fan
Cc: mdroth, Jan Kiszka, qemu-devel, Anthony Liguori, Paolo Bonzini
On Fri, Apr 26, 2013 at 10:47:26AM +0800, Liu Ping Fan wrote:
> @@ -141,6 +134,59 @@ static ssize_t net_socket_receive_dgram(NetClientState *nc, const uint8_t *buf,
> return ret;
> }
>
> +static gushort socket_connecting_readable(void *opaque)
> +{
> + return G_IO_IN;
> +}
> +
> +static gushort socket_listen_readable(void *opaque)
> +{
> + /* listen only handle in-req, no err */
> + return G_IO_IN;
>From the accept(2) man page:
"Linux accept() (and accept4()) passes already-pending network errors on
the new socket as an error code from accept()."
So we must handle errors from accept(2), please use G_IO_IN | G_IO_HUP |
G_IO_ERR.
> +static gushort socket_establish_readable(void *opaque)
> +{
> + NetSocketState *s = opaque;
> +
> + /* rely on net_socket_send to handle err */
> + if (s->read_poll && net_socket_can_send(s)) {
> + return G_IO_IN|G_IO_HUP|G_IO_ERR;
> + }
> + return G_IO_HUP|G_IO_ERR;
> +}
This new function always monitors G_IO_HUP | G_IO_ERR. The old code
only monitored it when read_poll == true and net_socket_can_send() ==
true.
Please preserve semantics.
> +static gushort socket_establish_writable(void *opaque)
> +{
> + NetSocketState *s = opaque;
> +
> + if (s->write_poll) {
> + return G_IO_OUT;
Errors/hang up?
> @@ -440,9 +529,20 @@ static NetSocketState *net_socket_fd_init_stream(NetClientState *peer,
> s->listen_fd = -1;
>
> if (is_connected) {
> - net_socket_connect(s);
> + assert(!s->nsrc);
> + s->nsrc = event_source_new(fd, net_socket_establish_handler, s);
> + s->nsrc->readable = socket_establish_readable;
> + s->nsrc->writable = socket_establish_writable;
> + nc->info->bind_ctx(nc, NULL);
> + net_socket_read_poll(s, true);
> + net_socket_write_poll(s, true);
> } else {
> - qemu_set_fd_handler(s->fd, NULL, net_socket_connect, s);
> + assert(!s->nsrc);
> + s->nsrc = event_source_new(fd, net_socket_connect_handler, s);
> + s->nsrc->readable = socket_connecting_readable;
The original code wants writeable, not readable.
> +static gboolean net_socket_listen_handler(gpointer data)
> +{
> + EventGSource *nsrc = data;
> + NetSocketState *s = nsrc->opaque;
> struct sockaddr_in saddr;
> socklen_t len;
> int fd;
>
> - for(;;) {
> - len = sizeof(saddr);
> - fd = qemu_accept(s->listen_fd, (struct sockaddr *)&saddr, &len);
> - if (fd < 0 && errno != EINTR) {
> - return;
> - } else if (fd >= 0) {
> - qemu_set_fd_handler(s->listen_fd, NULL, NULL, NULL);
> - break;
> - }
> + len = sizeof(saddr);
> + fd = qemu_accept(s->listen_fd, (struct sockaddr *)&saddr, &len);
> + if (fd < 0 && errno != EINTR) {
> + return false;
> }
This breaks the code when accept(2) is interrupted by a signal and we
get -1, errno == EINTR. Why did you remove the loop?
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [RFC PATCH v5 05/14] net: port socket to GSource
2013-04-26 9:48 ` Stefan Hajnoczi
@ 2013-04-27 7:09 ` liu ping fan
2013-04-29 8:21 ` Stefan Hajnoczi
0 siblings, 1 reply; 22+ messages in thread
From: liu ping fan @ 2013-04-27 7:09 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: mdroth, Jan Kiszka, qemu-devel, Anthony Liguori, Paolo Bonzini
On Fri, Apr 26, 2013 at 5:48 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> On Fri, Apr 26, 2013 at 10:47:26AM +0800, Liu Ping Fan wrote:
>> @@ -141,6 +134,59 @@ static ssize_t net_socket_receive_dgram(NetClientState *nc, const uint8_t *buf,
>> return ret;
>> }
>>
>> +static gushort socket_connecting_readable(void *opaque)
>> +{
>> + return G_IO_IN;
>> +}
>> +
>> +static gushort socket_listen_readable(void *opaque)
>> +{
>> + /* listen only handle in-req, no err */
>> + return G_IO_IN;
>
> From the accept(2) man page:
>
> "Linux accept() (and accept4()) passes already-pending network errors on
> the new socket as an error code from accept()."
>
> So we must handle errors from accept(2), please use G_IO_IN | G_IO_HUP |
> G_IO_ERR.
>
Here, we handle listen(2), not accept(2)
>> +static gushort socket_establish_readable(void *opaque)
>> +{
>> + NetSocketState *s = opaque;
>> +
>> + /* rely on net_socket_send to handle err */
>> + if (s->read_poll && net_socket_can_send(s)) {
>> + return G_IO_IN|G_IO_HUP|G_IO_ERR;
>> + }
>> + return G_IO_HUP|G_IO_ERR;
>> +}
>
> This new function always monitors G_IO_HUP | G_IO_ERR. The old code
> only monitored it when read_poll == true and net_socket_can_send() ==
> true.
>
> Please preserve semantics.
>
But the only the code in net_socket_send() will handle the err
condition. See the code behind "/* end of connection */". And I think
it is safely to handle err, even when the peer is not ready to
receive.
>> +static gushort socket_establish_writable(void *opaque)
>> +{
>> + NetSocketState *s = opaque;
>> +
>> + if (s->write_poll) {
>> + return G_IO_OUT;
>
> Errors/hang up?
>
As explained above, net_socket_writable() does not handle the err
condition. But maybe we need the qemu_flush_queued_packets() in it?
>> @@ -440,9 +529,20 @@ static NetSocketState *net_socket_fd_init_stream(NetClientState *peer,
>> s->listen_fd = -1;
>>
>> if (is_connected) {
>> - net_socket_connect(s);
>> + assert(!s->nsrc);
>> + s->nsrc = event_source_new(fd, net_socket_establish_handler, s);
>> + s->nsrc->readable = socket_establish_readable;
>> + s->nsrc->writable = socket_establish_writable;
>> + nc->info->bind_ctx(nc, NULL);
>> + net_socket_read_poll(s, true);
>> + net_socket_write_poll(s, true);
>> } else {
>> - qemu_set_fd_handler(s->fd, NULL, net_socket_connect, s);
>> + assert(!s->nsrc);
>> + s->nsrc = event_source_new(fd, net_socket_connect_handler, s);
>> + s->nsrc->readable = socket_connecting_readable;
>
> The original code wants writeable, not readable.
>
Will fix it.
>> +static gboolean net_socket_listen_handler(gpointer data)
>> +{
>> + EventGSource *nsrc = data;
>> + NetSocketState *s = nsrc->opaque;
>> struct sockaddr_in saddr;
>> socklen_t len;
>> int fd;
>>
>> - for(;;) {
>> - len = sizeof(saddr);
>> - fd = qemu_accept(s->listen_fd, (struct sockaddr *)&saddr, &len);
>> - if (fd < 0 && errno != EINTR) {
>> - return;
>> - } else if (fd >= 0) {
>> - qemu_set_fd_handler(s->listen_fd, NULL, NULL, NULL);
>> - break;
>> - }
>> + len = sizeof(saddr);
>> + fd = qemu_accept(s->listen_fd, (struct sockaddr *)&saddr, &len);
>> + if (fd < 0 && errno != EINTR) {
>> + return false;
>> }
>
> This breaks the code when accept(2) is interrupted by a signal and we
> get -1, errno == EINTR. Why did you remove the loop?
Oh, will fix it.
Thanks,
Pingfan
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [RFC PATCH v5 05/14] net: port socket to GSource
2013-04-27 7:09 ` liu ping fan
@ 2013-04-29 8:21 ` Stefan Hajnoczi
0 siblings, 0 replies; 22+ messages in thread
From: Stefan Hajnoczi @ 2013-04-29 8:21 UTC (permalink / raw)
To: liu ping fan
Cc: mdroth, Jan Kiszka, qemu-devel, Anthony Liguori, Paolo Bonzini
On Sat, Apr 27, 2013 at 03:09:10PM +0800, liu ping fan wrote:
> On Fri, Apr 26, 2013 at 5:48 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > On Fri, Apr 26, 2013 at 10:47:26AM +0800, Liu Ping Fan wrote:
> >> @@ -141,6 +134,59 @@ static ssize_t net_socket_receive_dgram(NetClientState *nc, const uint8_t *buf,
> >> return ret;
> >> }
> >>
> >> +static gushort socket_connecting_readable(void *opaque)
> >> +{
> >> + return G_IO_IN;
> >> +}
> >> +
> >> +static gushort socket_listen_readable(void *opaque)
> >> +{
> >> + /* listen only handle in-req, no err */
> >> + return G_IO_IN;
> >
> > From the accept(2) man page:
> >
> > "Linux accept() (and accept4()) passes already-pending network errors on
> > the new socket as an error code from accept()."
> >
> > So we must handle errors from accept(2), please use G_IO_IN | G_IO_HUP |
> > G_IO_ERR.
> >
> Here, we handle listen(2), not accept(2)
Look again, the handler invokes accept(2). listen(2) was called to put
the socket into the listening state but now we are monitoring for
accept.
> >> +static gushort socket_establish_readable(void *opaque)
> >> +{
> >> + NetSocketState *s = opaque;
> >> +
> >> + /* rely on net_socket_send to handle err */
> >> + if (s->read_poll && net_socket_can_send(s)) {
> >> + return G_IO_IN|G_IO_HUP|G_IO_ERR;
> >> + }
> >> + return G_IO_HUP|G_IO_ERR;
> >> +}
> >
> > This new function always monitors G_IO_HUP | G_IO_ERR. The old code
> > only monitored it when read_poll == true and net_socket_can_send() ==
> > true.
> >
> > Please preserve semantics.
> >
> But the only the code in net_socket_send() will handle the err
> condition. See the code behind "/* end of connection */". And I think
> it is safely to handle err, even when the peer is not ready to
> receive.
>
> >> +static gushort socket_establish_writable(void *opaque)
> >> +{
> >> + NetSocketState *s = opaque;
> >> +
> >> + if (s->write_poll) {
> >> + return G_IO_OUT;
> >
> > Errors/hang up?
> >
> As explained above, net_socket_writable() does not handle the err
> condition. But maybe we need the qemu_flush_queued_packets() in it?
net_socket_receive() does handle send(2) errors but it does so
differently from net_socket_send(). It fails the packet and resets
->send_index.
The change you made doesn't really solve this because an error could
still happen right when QEMU calls send(2) and therefore not be
processed by net_socket_send().
Changing the send/receive error handling is something that could be done
carefully in a separate patch. But please preserve semantics in
conversion patches - it makes them easy to review and merge.
As explained above, I'm not convinced that the change you made is
useful.
Stefan
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [RFC PATCH v5 06/14] net: port tap-win32 onto GSource
2013-04-26 2:47 [Qemu-devel] [RFC PATCH v5 00/14] port network layer onto glib Liu Ping Fan
` (4 preceding siblings ...)
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 05/14] net: port socket to GSource Liu Ping Fan
@ 2013-04-26 2:47 ` Liu Ping Fan
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 07/14] net: hub use lock to protect ports list Liu Ping Fan
` (7 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Liu Ping Fan @ 2013-04-26 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: mdroth, Jan Kiszka, Stefan Hajnoczi, Anthony Liguori,
Paolo Bonzini
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
net/tap-win32.c | 31 +++++++++++++++++++++++++++++--
1 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/net/tap-win32.c b/net/tap-win32.c
index 91e9e84..7a84195 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -635,13 +635,14 @@ static int tap_win32_open(tap_win32_overlapped_t **phandle,
typedef struct TAPState {
NetClientState nc;
tap_win32_overlapped_t *handle;
+ EventGSource *nsrc;
} TAPState;
static void tap_cleanup(NetClientState *nc)
{
TAPState *s = DO_UPCAST(TAPState, nc, nc);
- qemu_del_wait_object(s->handle->tap_semaphore, NULL, NULL);
+ event_source_release(s->nsrc);
/* FIXME: need to kill thread and close file handle:
tap_win32_close(s);
@@ -669,13 +670,37 @@ static void tap_win32_send(void *opaque)
}
}
+static void tap_bind_ctx(NetClientState *nc, GMainContext *ctx)
+{
+ TAPState *s = DO_UPCAST(TAPState, nc, nc);
+
+ g_source_attach(&s->nsrc->source, ctx);
+}
+
static NetClientInfo net_tap_win32_info = {
.type = NET_CLIENT_OPTIONS_KIND_TAP,
.size = sizeof(TAPState),
.receive = tap_receive,
.cleanup = tap_cleanup,
+ .bind_ctx = tap_bind_ctx,
};
+static gboolean tap_win32_handler(gpointer data)
+{
+ EventGSource *nsrc = data;
+ TAPState *s = nsrc->opaque;
+
+ if (nsrc->gfd.revents & G_IO_IN) {
+ tap_win32_send(s);
+ }
+ return true;
+}
+
+static gushort readable(void *opaque)
+{
+ return G_IO_IN;
+}
+
static int tap_win32_init(NetClientState *peer, const char *model,
const char *name, const char *ifname)
{
@@ -697,7 +722,9 @@ static int tap_win32_init(NetClientState *peer, const char *model,
s->handle = handle;
- qemu_add_wait_object(s->handle->tap_semaphore, tap_win32_send, s);
+ s->nsrc = event_source_new(s->handle->tap_semaphore, tap_win32_handler, s);
+ s->nsrc->readable = readable;
+ nc->info->bind_ctx(&s->nc, NULL);
return 0;
}
--
1.7.4.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [RFC PATCH v5 07/14] net: hub use lock to protect ports list
2013-04-26 2:47 [Qemu-devel] [RFC PATCH v5 00/14] port network layer onto glib Liu Ping Fan
` (5 preceding siblings ...)
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 06/14] net: port tap-win32 onto GSource Liu Ping Fan
@ 2013-04-26 2:47 ` Liu Ping Fan
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 08/14] net: introduce lock to protect NetQueue Liu Ping Fan
` (6 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Liu Ping Fan @ 2013-04-26 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: mdroth, Jan Kiszka, Stefan Hajnoczi, Anthony Liguori,
Paolo Bonzini
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
Hub ports will run on multi-threads, so use lock to protect them.
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
net/hub.c | 25 ++++++++++++++++++++++++-
1 files changed, 24 insertions(+), 1 deletions(-)
diff --git a/net/hub.c b/net/hub.c
index df32074..812a6dc 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -37,6 +37,7 @@ struct NetHub {
int id;
QLIST_ENTRY(NetHub) next;
int num_ports;
+ QemuMutex ports_lock;
QLIST_HEAD(, NetHubPort) ports;
};
@@ -47,6 +48,7 @@ static ssize_t net_hub_receive(NetHub *hub, NetHubPort *source_port,
{
NetHubPort *port;
+ qemu_mutex_lock(&hub->ports_lock);
QLIST_FOREACH(port, &hub->ports, next) {
if (port == source_port) {
continue;
@@ -54,6 +56,7 @@ static ssize_t net_hub_receive(NetHub *hub, NetHubPort *source_port,
qemu_send_packet(&port->nc, buf, len);
}
+ qemu_mutex_unlock(&hub->ports_lock);
return len;
}
@@ -63,6 +66,7 @@ static ssize_t net_hub_receive_iov(NetHub *hub, NetHubPort *source_port,
NetHubPort *port;
ssize_t len = iov_size(iov, iovcnt);
+ qemu_mutex_lock(&hub->ports_lock);
QLIST_FOREACH(port, &hub->ports, next) {
if (port == source_port) {
continue;
@@ -70,6 +74,7 @@ static ssize_t net_hub_receive_iov(NetHub *hub, NetHubPort *source_port,
qemu_sendv_packet(&port->nc, iov, iovcnt);
}
+ qemu_mutex_unlock(&hub->ports_lock);
return len;
}
@@ -80,6 +85,7 @@ static NetHub *net_hub_new(int id)
hub = g_malloc(sizeof(*hub));
hub->id = id;
hub->num_ports = 0;
+ qemu_mutex_init(&hub->ports_lock);
QLIST_INIT(&hub->ports);
QLIST_INSERT_HEAD(&hubs, hub, next);
@@ -93,16 +99,19 @@ static int net_hub_port_can_receive(NetClientState *nc)
NetHubPort *src_port = DO_UPCAST(NetHubPort, nc, nc);
NetHub *hub = src_port->hub;
+ qemu_mutex_lock(&hub->ports_lock);
QLIST_FOREACH(port, &hub->ports, next) {
if (port == src_port) {
continue;
}
if (qemu_can_send_packet(&port->nc)) {
+ qemu_mutex_unlock(&hub->ports_lock);
return 1;
}
}
+ qemu_mutex_unlock(&hub->ports_lock);
return 0;
}
@@ -155,8 +164,9 @@ static NetHubPort *net_hub_port_new(NetHub *hub, const char *name)
port = DO_UPCAST(NetHubPort, nc, nc);
port->id = id;
port->hub = hub;
-
+ qemu_mutex_lock(&hub->ports_lock);
QLIST_INSERT_HEAD(&hub->ports, port, next);
+ qemu_mutex_unlock(&hub->ports_lock);
return port;
}
@@ -197,13 +207,16 @@ NetClientState *net_hub_find_client_by_name(int hub_id, const char *name)
QLIST_FOREACH(hub, &hubs, next) {
if (hub->id == hub_id) {
+ qemu_mutex_lock(&hub->ports_lock);
QLIST_FOREACH(port, &hub->ports, next) {
peer = port->nc.peer;
if (peer && strcmp(peer->name, name) == 0) {
+ qemu_mutex_unlock(&hub->ports_lock);
return peer;
}
}
+ qemu_mutex_unlock(&hub->ports_lock);
}
}
return NULL;
@@ -220,12 +233,15 @@ NetClientState *net_hub_port_find(int hub_id)
QLIST_FOREACH(hub, &hubs, next) {
if (hub->id == hub_id) {
+ qemu_mutex_lock(&hub->ports_lock);
QLIST_FOREACH(port, &hub->ports, next) {
nc = port->nc.peer;
if (!nc) {
+ qemu_mutex_unlock(&hub->ports_lock);
return &(port->nc);
}
}
+ qemu_mutex_unlock(&hub->ports_lock);
break;
}
}
@@ -244,12 +260,14 @@ void net_hub_info(Monitor *mon)
QLIST_FOREACH(hub, &hubs, next) {
monitor_printf(mon, "hub %d\n", hub->id);
+ qemu_mutex_lock(&hub->ports_lock);
QLIST_FOREACH(port, &hub->ports, next) {
if (port->nc.peer) {
monitor_printf(mon, " \\ ");
print_net_client(mon, port->nc.peer);
}
}
+ qemu_mutex_unlock(&hub->ports_lock);
}
}
@@ -306,6 +324,7 @@ void net_hub_check_clients(void)
QLIST_FOREACH(hub, &hubs, next) {
int has_nic = 0, has_host_dev = 0;
+ qemu_mutex_lock(&hub->ports_lock);
QLIST_FOREACH(port, &hub->ports, next) {
peer = port->nc.peer;
if (!peer) {
@@ -328,6 +347,7 @@ void net_hub_check_clients(void)
break;
}
}
+ qemu_mutex_unlock(&hub->ports_lock);
if (has_host_dev && !has_nic) {
fprintf(stderr, "Warning: vlan %d with no nics\n", hub->id);
}
@@ -343,12 +363,15 @@ bool net_hub_flush(NetClientState *nc)
{
NetHubPort *port;
NetHubPort *source_port = DO_UPCAST(NetHubPort, nc, nc);
+ NetHub *hub = source_port->hub;
int ret = 0;
+ qemu_mutex_lock(&hub->ports_lock);
QLIST_FOREACH(port, &source_port->hub->ports, next) {
if (port != source_port) {
ret += qemu_net_queue_flush(port->nc.send_queue);
}
}
+ qemu_mutex_unlock(&hub->ports_lock);
return ret ? true : false;
}
--
1.7.4.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [RFC PATCH v5 08/14] net: introduce lock to protect NetQueue
2013-04-26 2:47 [Qemu-devel] [RFC PATCH v5 00/14] port network layer onto glib Liu Ping Fan
` (6 preceding siblings ...)
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 07/14] net: hub use lock to protect ports list Liu Ping Fan
@ 2013-04-26 2:47 ` Liu Ping Fan
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 09/14] net: introduce lock to protect NetClientState's peer's access Liu Ping Fan
` (5 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Liu Ping Fan @ 2013-04-26 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: mdroth, Jan Kiszka, Stefan Hajnoczi, Anthony Liguori,
Paolo Bonzini
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
NetQueue will be accessed by nc and its peers at the same time,
need lock to protect it.
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
net/queue.c | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/net/queue.c b/net/queue.c
index 859d02a..2856c1d 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -53,6 +53,7 @@ struct NetQueue {
uint32_t nq_maxlen;
uint32_t nq_count;
+ QemuMutex lock;
QTAILQ_HEAD(packets, NetPacket) packets;
unsigned delivering : 1;
@@ -68,6 +69,7 @@ NetQueue *qemu_new_net_queue(void *opaque)
queue->nq_maxlen = 10000;
queue->nq_count = 0;
+ qemu_mutex_init(&queue->lock);
QTAILQ_INIT(&queue->packets);
queue->delivering = 0;
@@ -107,7 +109,9 @@ static void qemu_net_queue_append(NetQueue *queue,
memcpy(packet->data, buf, size);
queue->nq_count++;
+ qemu_mutex_lock(&queue->lock);
QTAILQ_INSERT_TAIL(&queue->packets, packet, entry);
+ qemu_mutex_unlock(&queue->lock);
}
static void qemu_net_queue_append_iov(NetQueue *queue,
@@ -142,7 +146,9 @@ static void qemu_net_queue_append_iov(NetQueue *queue,
}
queue->nq_count++;
+ qemu_mutex_lock(&queue->lock);
QTAILQ_INSERT_TAIL(&queue->packets, packet, entry);
+ qemu_mutex_unlock(&queue->lock);
}
static ssize_t qemu_net_queue_deliver(NetQueue *queue,
@@ -229,6 +235,7 @@ void qemu_net_queue_purge(NetQueue *queue, NetClientState *from)
{
NetPacket *packet, *next;
+ qemu_mutex_lock(&queue->lock);
QTAILQ_FOREACH_SAFE(packet, &queue->packets, entry, next) {
if (packet->sender == from) {
QTAILQ_REMOVE(&queue->packets, packet, entry);
@@ -236,10 +243,12 @@ void qemu_net_queue_purge(NetQueue *queue, NetClientState *from)
g_free(packet);
}
}
+ qemu_mutex_unlock(&queue->lock);
}
bool qemu_net_queue_flush(NetQueue *queue)
{
+ qemu_mutex_lock(&queue->lock);
while (!QTAILQ_EMPTY(&queue->packets)) {
NetPacket *packet;
int ret;
@@ -256,6 +265,7 @@ bool qemu_net_queue_flush(NetQueue *queue)
if (ret == 0) {
queue->nq_count++;
QTAILQ_INSERT_HEAD(&queue->packets, packet, entry);
+ qemu_mutex_unlock(&queue->lock);
return false;
}
@@ -265,5 +275,6 @@ bool qemu_net_queue_flush(NetQueue *queue)
g_free(packet);
}
+ qemu_mutex_unlock(&queue->lock);
return true;
}
--
1.7.4.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [RFC PATCH v5 09/14] net: introduce lock to protect NetClientState's peer's access
2013-04-26 2:47 [Qemu-devel] [RFC PATCH v5 00/14] port network layer onto glib Liu Ping Fan
` (7 preceding siblings ...)
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 08/14] net: introduce lock to protect NetQueue Liu Ping Fan
@ 2013-04-26 2:47 ` Liu Ping Fan
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 10/14] net: make netclient re-entrant with refcnt Liu Ping Fan
` (4 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Liu Ping Fan @ 2013-04-26 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: mdroth, Jan Kiszka, Stefan Hajnoczi, Anthony Liguori,
Paolo Bonzini
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
Introduce nc->peer_lock to shield off the race of nc->peer's reader and
deleter. With it, after deleter finish, no new qemu_send_packet_xx()
will append packet to peer->send_queue, therefore no new reference from
packet->sender to nc will exist in nc->peer->send_queue.
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
include/net/net.h | 7 +++++
net/net.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++---
net/queue.c | 4 +-
3 files changed, 84 insertions(+), 6 deletions(-)
diff --git a/include/net/net.h b/include/net/net.h
index 88332d2..54f91ea 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -5,6 +5,7 @@
#include "qemu-common.h"
#include "qapi/qmp/qdict.h"
#include "qemu/option.h"
+#include "qemu/thread.h"
#include "net/queue.h"
#include "migration/vmstate.h"
#include "qapi-types.h"
@@ -63,6 +64,10 @@ struct NetClientState {
NetClientInfo *info;
int link_down;
QTAILQ_ENTRY(NetClientState) next;
+ /* protect the race access of peer only between reader and writer.
+ * to resolve the writer's race condition, resort on biglock.
+ */
+ QemuMutex peer_lock;
NetClientState *peer;
NetQueue *send_queue;
char *model;
@@ -75,6 +80,7 @@ struct NetClientState {
typedef struct NICState {
NetClientState *ncs;
+ NetClientState **pending_peer;
NICConf *conf;
void *opaque;
bool peer_deleted;
@@ -102,6 +108,7 @@ NetClientState *qemu_find_vlan_client_by_name(Monitor *mon, int vlan_id,
const char *client_str);
typedef void (*qemu_nic_foreach)(NICState *nic, void *opaque);
void qemu_foreach_nic(qemu_nic_foreach func, void *opaque);
+int qemu_can_send_packet_nolock(NetClientState *sender);
int qemu_can_send_packet(NetClientState *nc);
ssize_t qemu_sendv_packet(NetClientState *nc, const struct iovec *iov,
int iovcnt);
diff --git a/net/net.c b/net/net.c
index f3d67f8..7619762 100644
--- a/net/net.c
+++ b/net/net.c
@@ -207,6 +207,7 @@ static void qemu_net_client_setup(NetClientState *nc,
nc->peer = peer;
peer->peer = nc;
}
+ qemu_mutex_init(&nc->peer_lock);
QTAILQ_INSERT_TAIL(&net_clients, nc, next);
nc->send_queue = qemu_new_net_queue(nc);
@@ -246,6 +247,7 @@ NICState *qemu_new_nic(NetClientInfo *info,
nic->ncs = (void *)nic + info->size;
nic->conf = conf;
nic->opaque = opaque;
+ nic->pending_peer = g_malloc0(sizeof(NetClientState *) * queues);
for (i = 0; i < queues; i++) {
qemu_net_client_setup(&nic->ncs[i], info, peers[i], model, name,
@@ -304,6 +306,38 @@ static void qemu_free_net_client(NetClientState *nc)
}
}
+/* elimate the reference and sync with exit of rx/tx action.
+ * And flush out peer's queue.
+ */
+static void qemu_net_client_detach_flush(NetClientState *nc)
+{
+ NetClientState *peer;
+
+ /* reader of self's peer field , fixme? the deleters are not concurrent,
+ * so this pair lock can save.
+ */
+ qemu_mutex_lock(&nc->peer_lock);
+ peer = nc->peer;
+ qemu_mutex_unlock(&nc->peer_lock);
+
+ /* writer of peer's peer field*/
+ if (peer) {
+ /* exclude the race with tx to @nc */
+ qemu_mutex_lock(&peer->peer_lock);
+ peer->peer = NULL;
+ qemu_mutex_unlock(&peer->peer_lock);
+ }
+
+ /* writer of self's peer field*/
+ /* exclude the race with tx from @nc */
+ qemu_mutex_lock(&nc->peer_lock);
+ nc->peer = NULL;
+ if (peer) {
+ qemu_net_queue_purge(peer->send_queue, nc);
+ }
+ qemu_mutex_unlock(&nc->peer_lock);
+}
+
void qemu_del_net_client(NetClientState *nc)
{
NetClientState *ncs[MAX_QUEUE_NUM];
@@ -334,7 +368,9 @@ void qemu_del_net_client(NetClientState *nc)
}
for (i = 0; i < queues; i++) {
+ qemu_net_client_detach_flush(ncs[i]);
qemu_cleanup_net_client(ncs[i]);
+ nic->pending_peer[i] = ncs[i];
}
return;
@@ -343,6 +379,7 @@ void qemu_del_net_client(NetClientState *nc)
assert(nc->info->type != NET_CLIENT_OPTIONS_KIND_NIC);
for (i = 0; i < queues; i++) {
+ qemu_net_client_detach_flush(ncs[i]);
qemu_cleanup_net_client(ncs[i]);
qemu_free_net_client(ncs[i]);
}
@@ -355,17 +392,19 @@ void qemu_del_nic(NICState *nic)
/* If this is a peer NIC and peer has already been deleted, free it now. */
if (nic->peer_deleted) {
for (i = 0; i < queues; i++) {
- qemu_free_net_client(qemu_get_subqueue(nic, i)->peer);
+ qemu_free_net_client(nic->pending_peer[i]);
}
}
for (i = queues - 1; i >= 0; i--) {
NetClientState *nc = qemu_get_subqueue(nic, i);
+ qemu_net_client_detach_flush(nc);
qemu_cleanup_net_client(nc);
qemu_free_net_client(nc);
}
+ g_free(nic->pending_peer);
g_free(nic);
}
@@ -382,7 +421,7 @@ void qemu_foreach_nic(qemu_nic_foreach func, void *opaque)
}
}
-int qemu_can_send_packet(NetClientState *sender)
+int qemu_can_send_packet_nolock(NetClientState *sender)
{
if (!sender->peer) {
return 1;
@@ -397,6 +436,28 @@ int qemu_can_send_packet(NetClientState *sender)
return 1;
}
+int qemu_can_send_packet(NetClientState *sender)
+{
+ int ret = 1;
+
+ qemu_mutex_lock(&sender->peer_lock);
+ if (!sender->peer) {
+ goto unlock;
+ }
+
+ if (sender->peer->receive_disabled) {
+ ret = 0;
+ goto unlock;
+ } else if (sender->peer->info->can_receive &&
+ !sender->peer->info->can_receive(sender->peer)) {
+ ret = 0;
+ goto unlock;
+ }
+unlock:
+ qemu_mutex_unlock(&sender->peer_lock);
+ return ret;
+}
+
ssize_t qemu_deliver_packet(NetClientState *sender,
unsigned flags,
const uint8_t *data,
@@ -460,19 +521,24 @@ static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender,
NetPacketSent *sent_cb)
{
NetQueue *queue;
+ ssize_t sz;
#ifdef DEBUG_NET
printf("qemu_send_packet_async:\n");
hex_dump(stdout, buf, size);
#endif
+ qemu_mutex_lock(&sender->peer_lock);
if (sender->link_down || !sender->peer) {
+ qemu_mutex_unlock(&sender->peer_lock);
return size;
}
queue = sender->peer->send_queue;
- return qemu_net_queue_send(queue, sender, flags, buf, size, sent_cb);
+ sz = qemu_net_queue_send(queue, sender, flags, buf, size, sent_cb);
+ qemu_mutex_unlock(&sender->peer_lock);
+ return sz;
}
ssize_t qemu_send_packet_async(NetClientState *sender,
@@ -540,16 +606,21 @@ ssize_t qemu_sendv_packet_async(NetClientState *sender,
NetPacketSent *sent_cb)
{
NetQueue *queue;
+ ssize_t sz;
+ qemu_mutex_lock(&sender->peer_lock);
if (sender->link_down || !sender->peer) {
+ qemu_mutex_unlock(&sender->peer_lock);
return iov_size(iov, iovcnt);
}
queue = sender->peer->send_queue;
- return qemu_net_queue_send_iov(queue, sender,
+ sz = qemu_net_queue_send_iov(queue, sender,
QEMU_NET_PACKET_FLAG_NONE,
iov, iovcnt, sent_cb);
+ qemu_mutex_unlock(&sender->peer_lock);
+ return sz;
}
ssize_t
diff --git a/net/queue.c b/net/queue.c
index 2856c1d..123c338 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -190,7 +190,7 @@ ssize_t qemu_net_queue_send(NetQueue *queue,
{
ssize_t ret;
- if (queue->delivering || !qemu_can_send_packet(sender)) {
+ if (queue->delivering || !qemu_can_send_packet_nolock(sender)) {
qemu_net_queue_append(queue, sender, flags, data, size, sent_cb);
return 0;
}
@@ -215,7 +215,7 @@ ssize_t qemu_net_queue_send_iov(NetQueue *queue,
{
ssize_t ret;
- if (queue->delivering || !qemu_can_send_packet(sender)) {
+ if (queue->delivering || !qemu_can_send_packet_nolock(sender)) {
qemu_net_queue_append_iov(queue, sender, flags, iov, iovcnt, sent_cb);
return 0;
}
--
1.7.4.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [RFC PATCH v5 10/14] net: make netclient re-entrant with refcnt
2013-04-26 2:47 [Qemu-devel] [RFC PATCH v5 00/14] port network layer onto glib Liu Ping Fan
` (8 preceding siblings ...)
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 09/14] net: introduce lock to protect NetClientState's peer's access Liu Ping Fan
@ 2013-04-26 2:47 ` Liu Ping Fan
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 11/14] slirp: make timeout local Liu Ping Fan
` (3 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Liu Ping Fan @ 2013-04-26 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: mdroth, Jan Kiszka, Stefan Hajnoczi, Anthony Liguori,
Paolo Bonzini
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
With refcnt, NetClientState's user can run agaist deleter.
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
hw/qdev-properties-system.c | 14 +++++++++++++
include/net/net.h | 3 ++
net/hub.c | 3 ++
net/net.c | 46 ++++++++++++++++++++++++++++++++++++++++--
net/slirp.c | 3 +-
5 files changed, 65 insertions(+), 4 deletions(-)
diff --git a/hw/qdev-properties-system.c b/hw/qdev-properties-system.c
index ce3af22..14c6d49 100644
--- a/hw/qdev-properties-system.c
+++ b/hw/qdev-properties-system.c
@@ -301,6 +301,7 @@ static void set_vlan(Object *obj, Visitor *v, void *opaque,
return;
}
+ /* inc ref, released when unset property */
hubport = net_hub_port_find(id);
if (!hubport) {
error_set(errp, QERR_INVALID_PARAMETER_VALUE,
@@ -310,11 +311,24 @@ static void set_vlan(Object *obj, Visitor *v, void *opaque,
*ptr = hubport;
}
+static void release_vlan(Object *obj, const char *name, void *opaque)
+{
+ DeviceState *dev = DEVICE(obj);
+ Property *prop = opaque;
+ NICPeers *peers_ptr = qdev_get_prop_ptr(dev, prop);
+ NetClientState **ptr = &peers_ptr->ncs[0];
+
+ if (*ptr) {
+ netclient_unref(*ptr);
+ }
+}
+
PropertyInfo qdev_prop_vlan = {
.name = "vlan",
.print = print_vlan,
.get = get_vlan,
.set = set_vlan,
+ .release = release_vlan,
};
int qdev_prop_set_drive(DeviceState *dev, const char *name,
diff --git a/include/net/net.h b/include/net/net.h
index 54f91ea..ef4137d 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -61,6 +61,7 @@ typedef struct NetClientInfo {
} NetClientInfo;
struct NetClientState {
+ int ref;
NetClientInfo *info;
int link_down;
QTAILQ_ENTRY(NetClientState) next;
@@ -89,6 +90,8 @@ typedef struct NICState {
NetClientState *qemu_find_netdev(const char *id);
int qemu_find_net_clients_except(const char *id, NetClientState **ncs,
NetClientOptionsKind type, int max);
+void netclient_ref(NetClientState *nc);
+void netclient_unref(NetClientState *nc);
NetClientState *qemu_new_net_client(NetClientInfo *info,
NetClientState *peer,
const char *model,
diff --git a/net/hub.c b/net/hub.c
index 812a6dc..2970f8e 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -212,6 +212,7 @@ NetClientState *net_hub_find_client_by_name(int hub_id, const char *name)
peer = port->nc.peer;
if (peer && strcmp(peer->name, name) == 0) {
+ netclient_ref(peer);
qemu_mutex_unlock(&hub->ports_lock);
return peer;
}
@@ -237,6 +238,7 @@ NetClientState *net_hub_port_find(int hub_id)
QLIST_FOREACH(port, &hub->ports, next) {
nc = port->nc.peer;
if (!nc) {
+ netclient_ref(&port->nc);
qemu_mutex_unlock(&hub->ports_lock);
return &(port->nc);
}
@@ -247,6 +249,7 @@ NetClientState *net_hub_port_find(int hub_id)
}
nc = net_hub_add_port(hub_id, NULL);
+ netclient_ref(nc);
return nc;
}
diff --git a/net/net.c b/net/net.c
index 7619762..ac859ff 100644
--- a/net/net.c
+++ b/net/net.c
@@ -45,6 +45,7 @@
# define CONFIG_NET_BRIDGE
#endif
+static QemuMutex net_clients_lock;
static QTAILQ_HEAD(, NetClientState) net_clients;
int default_net = 1;
@@ -166,6 +167,7 @@ static char *assign_name(NetClientState *nc1, const char *model)
char buf[256];
int id = 0;
+ qemu_mutex_lock(&net_clients_lock);
QTAILQ_FOREACH(nc, &net_clients, next) {
if (nc == nc1) {
continue;
@@ -176,6 +178,7 @@ static char *assign_name(NetClientState *nc1, const char *model)
id++;
}
}
+ qemu_mutex_unlock(&net_clients_lock);
snprintf(buf, sizeof(buf), "%s.%d", model, id);
@@ -206,9 +209,13 @@ static void qemu_net_client_setup(NetClientState *nc,
assert(!peer->peer);
nc->peer = peer;
peer->peer = nc;
+ netclient_ref(peer);
+ netclient_ref(nc);
}
qemu_mutex_init(&nc->peer_lock);
+ qemu_mutex_lock(&net_clients_lock);
QTAILQ_INSERT_TAIL(&net_clients, nc, next);
+ qemu_mutex_unlock(&net_clients_lock);
nc->send_queue = qemu_new_net_queue(nc);
nc->destructor = destructor;
@@ -224,6 +231,7 @@ NetClientState *qemu_new_net_client(NetClientInfo *info,
assert(info->size >= sizeof(NetClientState));
nc = g_malloc0(info->size);
+ netclient_ref(nc);
qemu_net_client_setup(nc, info, peer, model, name,
qemu_net_client_destructor);
@@ -284,7 +292,9 @@ void *qemu_get_nic_opaque(NetClientState *nc)
static void qemu_cleanup_net_client(NetClientState *nc)
{
+ qemu_mutex_lock(&net_clients_lock);
QTAILQ_REMOVE(&net_clients, nc, next);
+ qemu_mutex_unlock(&net_clients_lock);
if (nc->info->cleanup) {
nc->info->cleanup(nc);
@@ -306,6 +316,18 @@ static void qemu_free_net_client(NetClientState *nc)
}
}
+void netclient_ref(NetClientState *nc)
+{
+ __sync_add_and_fetch(&nc->ref, 1);
+}
+
+void netclient_unref(NetClientState *nc)
+{
+ if (__sync_sub_and_fetch(&nc->ref, 1) == 0) {
+ qemu_free_net_client(nc);
+ }
+}
+
/* elimate the reference and sync with exit of rx/tx action.
* And flush out peer's queue.
*/
@@ -334,8 +356,10 @@ static void qemu_net_client_detach_flush(NetClientState *nc)
nc->peer = NULL;
if (peer) {
qemu_net_queue_purge(peer->send_queue, nc);
+ netclient_unref(peer);
}
qemu_mutex_unlock(&nc->peer_lock);
+ netclient_unref(nc);
}
void qemu_del_net_client(NetClientState *nc)
@@ -381,7 +405,7 @@ void qemu_del_net_client(NetClientState *nc)
for (i = 0; i < queues; i++) {
qemu_net_client_detach_flush(ncs[i]);
qemu_cleanup_net_client(ncs[i]);
- qemu_free_net_client(ncs[i]);
+ netclient_unref(ncs[i]);
}
}
@@ -392,7 +416,7 @@ void qemu_del_nic(NICState *nic)
/* If this is a peer NIC and peer has already been deleted, free it now. */
if (nic->peer_deleted) {
for (i = 0; i < queues; i++) {
- qemu_free_net_client(nic->pending_peer[i]);
+ netclient_unref(nic->pending_peer[i]);
}
}
@@ -401,7 +425,7 @@ void qemu_del_nic(NICState *nic)
qemu_net_client_detach_flush(nc);
qemu_cleanup_net_client(nc);
- qemu_free_net_client(nc);
+ netclient_unref(nc);
}
g_free(nic->pending_peer);
@@ -412,6 +436,7 @@ void qemu_foreach_nic(qemu_nic_foreach func, void *opaque)
{
NetClientState *nc;
+ qemu_mutex_lock(&net_clients_lock);
QTAILQ_FOREACH(nc, &net_clients, next) {
if (nc->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
if (nc->queue_index == 0) {
@@ -419,6 +444,7 @@ void qemu_foreach_nic(qemu_nic_foreach func, void *opaque)
}
}
}
+ qemu_mutex_unlock(&net_clients_lock);
}
int qemu_can_send_packet_nolock(NetClientState *sender)
@@ -633,13 +659,17 @@ NetClientState *qemu_find_netdev(const char *id)
{
NetClientState *nc;
+ qemu_mutex_lock(&net_clients_lock);
QTAILQ_FOREACH(nc, &net_clients, next) {
if (nc->info->type == NET_CLIENT_OPTIONS_KIND_NIC)
continue;
if (!strcmp(nc->name, id)) {
+ netclient_ref(nc);
+ qemu_mutex_unlock(&net_clients_lock);
return nc;
}
}
+ qemu_mutex_unlock(&net_clients_lock);
return NULL;
}
@@ -650,6 +680,7 @@ int qemu_find_net_clients_except(const char *id, NetClientState **ncs,
NetClientState *nc;
int ret = 0;
+ qemu_mutex_lock(&net_clients_lock);
QTAILQ_FOREACH(nc, &net_clients, next) {
if (nc->info->type == type) {
continue;
@@ -661,6 +692,7 @@ int qemu_find_net_clients_except(const char *id, NetClientState **ncs,
ret++;
}
}
+ qemu_mutex_unlock(&net_clients_lock);
return ret;
}
@@ -969,6 +1001,7 @@ void net_host_device_remove(Monitor *mon, const QDict *qdict)
return;
}
qemu_del_net_client(nc);
+ netclient_unref(nc);
}
void netdev_add(QemuOpts *opts, Error **errp)
@@ -1024,6 +1057,7 @@ void qmp_netdev_del(const char *id, Error **errp)
}
qemu_del_net_client(nc);
+ netclient_unref(nc);
qemu_opts_del(opts);
}
@@ -1042,6 +1076,7 @@ void do_info_network(Monitor *mon, const QDict *qdict)
net_hub_info(mon);
+ qemu_mutex_lock(&net_clients_lock);
QTAILQ_FOREACH(nc, &net_clients, next) {
peer = nc->peer;
type = nc->info->type;
@@ -1059,6 +1094,7 @@ void do_info_network(Monitor *mon, const QDict *qdict)
print_net_client(mon, peer);
}
}
+ qemu_mutex_unlock(&net_clients_lock);
}
void qmp_set_link(const char *name, bool up, Error **errp)
@@ -1112,6 +1148,7 @@ void net_cleanup(void)
qemu_del_net_client(nc);
}
}
+ qemu_mutex_destroy(&net_clients_lock);
}
void net_check_clients(void)
@@ -1133,6 +1170,7 @@ void net_check_clients(void)
net_hub_check_clients();
+ qemu_mutex_lock(&net_clients_lock);
QTAILQ_FOREACH(nc, &net_clients, next) {
if (!nc->peer) {
fprintf(stderr, "Warning: %s %s has no peer\n",
@@ -1140,6 +1178,7 @@ void net_check_clients(void)
"nic" : "netdev", nc->name);
}
}
+ qemu_mutex_unlock(&net_clients_lock);
/* Check that all NICs requested via -net nic actually got created.
* NICs created via -device don't need to be checked here because
@@ -1197,6 +1236,7 @@ int net_init_clients(void)
#endif
}
+ qemu_mutex_init(&net_clients_lock);
QTAILQ_INIT(&net_clients);
if (qemu_opts_foreach(qemu_find_opts("netdev"), net_init_netdev, NULL, 1) == -1)
diff --git a/net/slirp.c b/net/slirp.c
index 4df550f..a6116d5 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -346,7 +346,7 @@ void net_slirp_hostfwd_remove(Monitor *mon, const QDict *qdict)
err = slirp_remove_hostfwd(QTAILQ_FIRST(&slirp_stacks)->slirp, is_udp,
host_addr, host_port);
-
+ netclient_unref(&s->nc);
monitor_printf(mon, "host forwarding rule for %s %s\n", src_str,
err ? "not found" : "removed");
return;
@@ -437,6 +437,7 @@ void net_slirp_hostfwd_add(Monitor *mon, const QDict *qdict)
}
if (s) {
slirp_hostfwd(s, redir_str, 0);
+ netclient_unref(&s->nc);
}
}
--
1.7.4.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [RFC PATCH v5 11/14] slirp: make timeout local
2013-04-26 2:47 [Qemu-devel] [RFC PATCH v5 00/14] port network layer onto glib Liu Ping Fan
` (9 preceding siblings ...)
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 10/14] net: make netclient re-entrant with refcnt Liu Ping Fan
@ 2013-04-26 2:47 ` Liu Ping Fan
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 12/14] slirp: make slirp event dispatch based on slirp instance, not global Liu Ping Fan
` (2 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Liu Ping Fan @ 2013-04-26 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: mdroth, Jan Kiszka, Stefan Hajnoczi, Anthony Liguori,
Paolo Bonzini
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
Each slirp has its own time to caculate timeout.
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
slirp/slirp.c | 22 ++++++++++------------
slirp/slirp.h | 3 +++
2 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/slirp/slirp.c b/slirp/slirp.c
index bd9b7cb..08c6b26 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -40,8 +40,6 @@ static const uint8_t special_ethaddr[ETH_ALEN] = {
static const uint8_t zero_ethaddr[ETH_ALEN] = { 0, 0, 0, 0, 0, 0 };
u_int curtime;
-static u_int time_fasttimo, last_slowtimo;
-static int do_slowtimo;
static QTAILQ_HEAD(slirp_instances, Slirp) slirp_instances =
QTAILQ_HEAD_INITIALIZER(slirp_instances);
@@ -278,14 +276,13 @@ void slirp_pollfds_fill(GArray *pollfds)
/*
* First, TCP sockets
*/
- do_slowtimo = 0;
QTAILQ_FOREACH(slirp, &slirp_instances, entry) {
/*
* *_slowtimo needs calling if there are IP fragments
* in the fragment queue, or there are TCP connections active
*/
- do_slowtimo |= ((slirp->tcb.so_next != &slirp->tcb) ||
+ slirp->do_slowtimo = ((slirp->tcb.so_next != &slirp->tcb) ||
(&slirp->ipq.ip_link != slirp->ipq.ip_link.next));
for (so = slirp->tcb.so_next; so != &slirp->tcb;
@@ -299,8 +296,9 @@ void slirp_pollfds_fill(GArray *pollfds)
/*
* See if we need a tcp_fasttimo
*/
- if (time_fasttimo == 0 && so->so_tcpcb->t_flags & TF_DELACK) {
- time_fasttimo = curtime; /* Flag when we want a fasttimo */
+ if (slirp->time_fasttimo == 0 &&
+ so->so_tcpcb->t_flags & TF_DELACK) {
+ slirp->time_fasttimo = curtime; /* Flag when want a fasttimo */
}
/*
@@ -381,7 +379,7 @@ void slirp_pollfds_fill(GArray *pollfds)
udp_detach(so);
continue;
} else {
- do_slowtimo = 1; /* Let socket expire */
+ slirp->do_slowtimo = 1; /* Let socket expire */
}
}
@@ -422,7 +420,7 @@ void slirp_pollfds_fill(GArray *pollfds)
icmp_detach(so);
continue;
} else {
- do_slowtimo = 1; /* Let socket expire */
+ slirp->do_slowtimo = 1; /* Let socket expire */
}
}
@@ -454,14 +452,14 @@ void slirp_pollfds_poll(GArray *pollfds, int select_error)
/*
* See if anything has timed out
*/
- if (time_fasttimo && ((curtime - time_fasttimo) >= 2)) {
+ if (slirp->time_fasttimo && ((curtime - slirp->time_fasttimo) >= 2)) {
tcp_fasttimo(slirp);
- time_fasttimo = 0;
+ slirp->time_fasttimo = 0;
}
- if (do_slowtimo && ((curtime - last_slowtimo) >= 499)) {
+ if (slirp->do_slowtimo && ((curtime - slirp->last_slowtimo) >= 499)) {
ip_slowtimo(slirp);
tcp_slowtimo(slirp);
- last_slowtimo = curtime;
+ slirp->last_slowtimo = curtime;
}
/*
diff --git a/slirp/slirp.h b/slirp/slirp.h
index fe0e65d..008360e 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -203,6 +203,9 @@ bool arp_table_search(Slirp *slirp, uint32_t ip_addr,
struct Slirp {
QTAILQ_ENTRY(Slirp) entry;
+ u_int time_fasttimo;
+ u_int last_slowtimo;
+ int do_slowtimo;
/* virtual network configuration */
struct in_addr vnetwork_addr;
--
1.7.4.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [RFC PATCH v5 12/14] slirp: make slirp event dispatch based on slirp instance, not global
2013-04-26 2:47 [Qemu-devel] [RFC PATCH v5 00/14] port network layer onto glib Liu Ping Fan
` (10 preceding siblings ...)
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 11/14] slirp: make timeout local Liu Ping Fan
@ 2013-04-26 2:47 ` Liu Ping Fan
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 13/14] slirp: handle race condition Liu Ping Fan
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 14/14] slirp: use lock to protect the slirp_instances Liu Ping Fan
13 siblings, 0 replies; 22+ messages in thread
From: Liu Ping Fan @ 2013-04-26 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: mdroth, Jan Kiszka, Stefan Hajnoczi, Anthony Liguori,
Paolo Bonzini
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
Split slirp_pollfds_fill/_poll actions into each slirp, so that SlirpState
can run on dedicated context. Each slirp socket will corresponds to a GPollFD,
and its SlirpState stands for a GSource(EventsGSource). Finally different
SlirpState can run on different context.
The logic in slirp_pollfds_fill/_poll is not changed, but due to drop of
the functions, rearrange the code to obey the coding style. For other minor
changes, they accord to the nearby style.
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
main-loop.c | 4 -
net/slirp.c | 32 +++
slirp/libslirp.h | 7 +-
slirp/slirp.c | 567 +++++++++++++++++++++++++-----------------------------
slirp/socket.c | 2 +
slirp/socket.h | 1 +
stubs/slirp.c | 8 -
7 files changed, 299 insertions(+), 322 deletions(-)
diff --git a/main-loop.c b/main-loop.c
index 8c9b58c..970f25d 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -432,14 +432,10 @@ int main_loop_wait(int nonblocking)
/* XXX: separate device handlers from system ones */
#ifdef CONFIG_SLIRP
slirp_update_timeout(&timeout);
- slirp_pollfds_fill(gpollfds);
#endif
qemu_iohandler_fill(gpollfds);
ret = os_host_main_loop_wait(timeout);
qemu_iohandler_poll(gpollfds, ret);
-#ifdef CONFIG_SLIRP
- slirp_pollfds_poll(gpollfds, (ret < 0));
-#endif
qemu_run_all_timers();
diff --git a/net/slirp.c b/net/slirp.c
index a6116d5..6ff5ca8 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -36,6 +36,7 @@
#include "qemu/sockets.h"
#include "slirp/libslirp.h"
#include "char/char.h"
+#include "util/event_gsource.h"
static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
{
@@ -76,6 +77,7 @@ typedef struct SlirpState {
#ifndef _WIN32
char smb_dir[128];
#endif
+ EventsGSource *slirp_src;
} SlirpState;
static struct slirp_config_str *slirp_configs;
@@ -120,17 +122,44 @@ static void net_slirp_cleanup(NetClientState *nc)
SlirpState *s = DO_UPCAST(SlirpState, nc, nc);
slirp_cleanup(s->slirp);
+ events_source_release(s->slirp_src);
slirp_smb_cleanup(s);
QTAILQ_REMOVE(&slirp_stacks, s, entry);
}
+static void net_slirp_bind_ctx(NetClientState *nc, GMainContext *ctx)
+{
+ SlirpState *s = DO_UPCAST(SlirpState, nc, nc);
+
+ g_source_attach(&s->slirp_src->source, ctx);
+}
+
static NetClientInfo net_slirp_info = {
.type = NET_CLIENT_OPTIONS_KIND_USER,
.size = sizeof(SlirpState),
.receive = net_slirp_receive,
.cleanup = net_slirp_cleanup,
+ .bind_ctx = net_slirp_bind_ctx,
};
+GPollFD *slirp_gsource_get_gfd(void *opaque, int fd)
+{
+ GPollFD *retfd;
+ SlirpState *s = opaque;
+ EventsGSource *src = s->slirp_src;
+ retfd = events_source_add_gfd(src, fd);
+
+ return retfd;
+}
+
+void slirp_gsource_close_gfd(void *opaque, GPollFD *pollfd)
+{
+ SlirpState *s = opaque;
+ EventsGSource *src = s->slirp_src;
+
+ events_source_remove_gfd(src, pollfd);
+}
+
static int net_slirp_init(NetClientState *peer, const char *model,
const char *name, int restricted,
const char *vnetwork, const char *vhost,
@@ -244,6 +273,8 @@ static int net_slirp_init(NetClientState *peer, const char *model,
s->slirp = slirp_init(restricted, net, mask, host, vhostname,
tftp_export, bootfile, dhcp, dns, dnssearch, s);
+ s->slirp_src = events_source_new(slirp_prepare, slirp_handler, s->slirp);
+
QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
for (config = slirp_configs; config; config = config->next) {
@@ -266,6 +297,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
goto error;
}
#endif
+ s->nc.info->bind_ctx(&s->nc, NULL);
return 0;
diff --git a/slirp/libslirp.h b/slirp/libslirp.h
index ceabff8..1aad5a4 100644
--- a/slirp/libslirp.h
+++ b/slirp/libslirp.h
@@ -17,11 +17,10 @@ Slirp *slirp_init(int restricted, struct in_addr vnetwork,
void slirp_cleanup(Slirp *slirp);
void slirp_update_timeout(uint32_t *timeout);
-void slirp_pollfds_fill(GArray *pollfds);
-
-void slirp_pollfds_poll(GArray *pollfds, int select_error);
void slirp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len);
+gboolean slirp_prepare(GSource *source, gint *time);
+gboolean slirp_handler(gpointer data);
/* you must provide the following functions: */
void slirp_output(void *opaque, const uint8_t *pkt, int pkt_len);
@@ -40,5 +39,7 @@ void slirp_socket_recv(Slirp *slirp, struct in_addr guest_addr,
int guest_port, const uint8_t *buf, int size);
size_t slirp_socket_can_recv(Slirp *slirp, struct in_addr guest_addr,
int guest_port);
+GPollFD *slirp_gsource_get_gfd(void *opaque, int fd);
+void slirp_gsource_close_gfd(void *opaque, GPollFD *pollfd);
#endif
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 08c6b26..691f82f 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -26,6 +26,7 @@
#include "char/char.h"
#include "slirp.h"
#include "hw/hw.h"
+#include "util/event_gsource.h"
/* host loopback address */
struct in_addr loopback_addr;
@@ -262,386 +263,338 @@ void slirp_update_timeout(uint32_t *timeout)
if (!QTAILQ_EMPTY(&slirp_instances)) {
*timeout = MIN(1000, *timeout);
}
+ curtime = qemu_get_clock_ms(rt_clock);
}
-void slirp_pollfds_fill(GArray *pollfds)
+gboolean slirp_prepare(GSource *source, gint *time)
{
- Slirp *slirp;
+ EventsGSource *slirp_src = (EventsGSource *)source;
+ Slirp *slirp = slirp_src->opaque;
struct socket *so, *so_next;
-
- if (QTAILQ_EMPTY(&slirp_instances)) {
- return;
- }
+ int events = 0;
/*
- * First, TCP sockets
+ * *_slowtimo needs calling if there are IP fragments
+ * in the fragment queue, or there are TCP connections active
*/
+ slirp->do_slowtimo = ((slirp->tcb.so_next != &slirp->tcb) ||
+ (&slirp->ipq.ip_link != slirp->ipq.ip_link.next));
+
+ for (so = slirp->tcb.so_next; so != &slirp->tcb;
+ so = so_next) {
- QTAILQ_FOREACH(slirp, &slirp_instances, entry) {
+ so_next = so->so_next;
+ if (so->pollfd->fd == -1 && so->s != -1) {
+ so->pollfd->fd = so->s;
+ g_source_add_poll(source, so->pollfd);
+ }
/*
- * *_slowtimo needs calling if there are IP fragments
- * in the fragment queue, or there are TCP connections active
+ * See if we need a tcp_fasttimo
*/
- slirp->do_slowtimo = ((slirp->tcb.so_next != &slirp->tcb) ||
- (&slirp->ipq.ip_link != slirp->ipq.ip_link.next));
-
- for (so = slirp->tcb.so_next; so != &slirp->tcb;
- so = so_next) {
- int events = 0;
-
- so_next = so->so_next;
-
- so->pollfds_idx = -1;
-
- /*
- * See if we need a tcp_fasttimo
- */
- if (slirp->time_fasttimo == 0 &&
- so->so_tcpcb->t_flags & TF_DELACK) {
- slirp->time_fasttimo = curtime; /* Flag when want a fasttimo */
- }
-
- /*
- * NOFDREF can include still connecting to local-host,
- * newly socreated() sockets etc. Don't want to select these.
- */
- if (so->so_state & SS_NOFDREF || so->s == -1) {
- continue;
- }
-
- /*
- * Set for reading sockets which are accepting
- */
- if (so->so_state & SS_FACCEPTCONN) {
- GPollFD pfd = {
- .fd = so->s,
- .events = G_IO_IN | G_IO_HUP | G_IO_ERR,
- };
- so->pollfds_idx = pollfds->len;
- g_array_append_val(pollfds, pfd);
- continue;
- }
+ if (slirp->time_fasttimo == 0 &&
+ so->so_tcpcb->t_flags & TF_DELACK) {
+ slirp->time_fasttimo = curtime; /* Flag when want a fasttimo */
+ }
- /*
- * Set for writing sockets which are connecting
- */
- if (so->so_state & SS_ISFCONNECTING) {
- GPollFD pfd = {
- .fd = so->s,
- .events = G_IO_OUT | G_IO_ERR,
- };
- so->pollfds_idx = pollfds->len;
- g_array_append_val(pollfds, pfd);
- continue;
- }
+ /*
+ * NOFDREF can include still connecting to local-host,
+ * newly socreated() sockets etc. Don't want to select these.
+ */
+ if (so->so_state & SS_NOFDREF || so->s == -1) {
+ continue;
+ }
- /*
- * Set for writing if we are connected, can send more, and
- * we have something to send
- */
- if (CONN_CANFSEND(so) && so->so_rcv.sb_cc) {
- events |= G_IO_OUT | G_IO_ERR;
- }
+ /*
+ * Set for reading sockets which are accepting
+ */
+ if (so->so_state & SS_FACCEPTCONN) {
+ so->pollfd->events = G_IO_IN | G_IO_HUP | G_IO_ERR;
+ continue;
+ }
- /*
- * Set for reading (and urgent data) if we are connected, can
- * receive more, and we have room for it XXX /2 ?
- */
- if (CONN_CANFRCV(so) &&
- (so->so_snd.sb_cc < (so->so_snd.sb_datalen/2))) {
- events |= G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_PRI;
- }
+ /*
+ * Set for writing sockets which are connecting
+ */
+ if (so->so_state & SS_ISFCONNECTING) {
+ so->pollfd->events = G_IO_OUT | G_IO_ERR;
+ continue;
+ }
- if (events) {
- GPollFD pfd = {
- .fd = so->s,
- .events = events,
- };
- so->pollfds_idx = pollfds->len;
- g_array_append_val(pollfds, pfd);
- }
+ /*
+ * Set for writing if we are connected, can send more, and
+ * we have something to send
+ */
+ if (CONN_CANFSEND(so) && so->so_rcv.sb_cc) {
+ events |= G_IO_OUT | G_IO_ERR;
}
/*
- * UDP sockets
+ * Set for reading (and urgent data) if we are connected, can
+ * receive more, and we have room for it XXX /2 ?
*/
- for (so = slirp->udb.so_next; so != &slirp->udb;
- so = so_next) {
- so_next = so->so_next;
+ if (CONN_CANFRCV(so) &&
+ (so->so_snd.sb_cc < (so->so_snd.sb_datalen/2))) {
+ events |= G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_PRI;
+ }
- so->pollfds_idx = -1;
+ if (events) {
+ so->pollfd->events = events;
+ }
+ }
- /*
- * See if it's timed out
- */
- if (so->so_expire) {
- if (so->so_expire <= curtime) {
- udp_detach(so);
- continue;
- } else {
- slirp->do_slowtimo = 1; /* Let socket expire */
- }
- }
+ /*
+ * UDP sockets
+ */
+ for (so = slirp->udb.so_next; so != &slirp->udb;
+ so = so_next) {
+ so_next = so->so_next;
- /*
- * When UDP packets are received from over the
- * link, they're sendto()'d straight away, so
- * no need for setting for writing
- * Limit the number of packets queued by this session
- * to 4. Note that even though we try and limit this
- * to 4 packets, the session could have more queued
- * if the packets needed to be fragmented
- * (XXX <= 4 ?)
- */
- if ((so->so_state & SS_ISFCONNECTED) && so->so_queued <= 4) {
- GPollFD pfd = {
- .fd = so->s,
- .events = G_IO_IN | G_IO_HUP | G_IO_ERR,
- };
- so->pollfds_idx = pollfds->len;
- g_array_append_val(pollfds, pfd);
+ /*
+ * See if it's timed out
+ */
+ if (so->so_expire) {
+ if (so->so_expire <= curtime) {
+ udp_detach(so);
+ continue;
+ } else {
+ slirp->do_slowtimo = 1; /* Let socket expire */
}
}
/*
- * ICMP sockets
+ * When UDP packets are received from over the
+ * link, they're sendto()'d straight away, so
+ * no need for setting for writing
+ * Limit the number of packets queued by this session
+ * to 4. Note that even though we try and limit this
+ * to 4 packets, the session could have more queued
+ * if the packets needed to be fragmented
+ * (XXX <= 4 ?)
*/
- for (so = slirp->icmp.so_next; so != &slirp->icmp;
- so = so_next) {
- so_next = so->so_next;
+ if ((so->so_state & SS_ISFCONNECTED) && so->so_queued <= 4) {
+ so->pollfd->events = G_IO_IN | G_IO_HUP | G_IO_ERR;
+ }
+ }
- so->pollfds_idx = -1;
+ /*
+ * ICMP sockets
+ */
+ for (so = slirp->icmp.so_next; so != &slirp->icmp;
+ so = so_next) {
+ so_next = so->so_next;
- /*
- * See if it's timed out
- */
- if (so->so_expire) {
- if (so->so_expire <= curtime) {
- icmp_detach(so);
- continue;
- } else {
- slirp->do_slowtimo = 1; /* Let socket expire */
- }
+ /*
+ * See if it's timed out
+ */
+ if (so->so_expire) {
+ if (so->so_expire <= curtime) {
+ icmp_detach(so);
+ continue;
+ } else {
+ slirp->do_slowtimo = 1; /* Let socket expire */
}
+ }
- if (so->so_state & SS_ISFCONNECTED) {
- GPollFD pfd = {
- .fd = so->s,
- .events = G_IO_IN | G_IO_HUP | G_IO_ERR,
- };
- so->pollfds_idx = pollfds->len;
- g_array_append_val(pollfds, pfd);
- }
+ if (so->so_state & SS_ISFCONNECTED) {
+ so->pollfd->events = G_IO_IN | G_IO_HUP | G_IO_ERR;
}
}
+
+ return false;
}
-void slirp_pollfds_poll(GArray *pollfds, int select_error)
+gboolean slirp_handler(gpointer data)
{
- Slirp *slirp;
+ EventsGSource *src = data;
+ Slirp *slirp = src->opaque;
struct socket *so, *so_next;
int ret;
- if (QTAILQ_EMPTY(&slirp_instances)) {
- return;
+ /*
+ * See if anything has timed out
+ */
+ if (slirp->time_fasttimo && ((curtime - slirp->time_fasttimo) >= 2)) {
+ tcp_fasttimo(slirp);
+ slirp->time_fasttimo = 0;
+ }
+ if (slirp->do_slowtimo && ((curtime - slirp->last_slowtimo) >= 499)) {
+ ip_slowtimo(slirp);
+ tcp_slowtimo(slirp);
+ slirp->last_slowtimo = curtime;
}
- curtime = qemu_get_clock_ms(rt_clock);
+ /*
+ * Check TCP sockets
+ */
+ for (so = slirp->tcb.so_next; so != &slirp->tcb;
+ so = so_next) {
+ int revents;
- QTAILQ_FOREACH(slirp, &slirp_instances, entry) {
- /*
- * See if anything has timed out
- */
- if (slirp->time_fasttimo && ((curtime - slirp->time_fasttimo) >= 2)) {
- tcp_fasttimo(slirp);
- slirp->time_fasttimo = 0;
+ so_next = so->so_next;
+
+ revents = 0;
+ if (so->pollfd) {
+ revents = so->pollfd->revents;
}
- if (slirp->do_slowtimo && ((curtime - slirp->last_slowtimo) >= 499)) {
- ip_slowtimo(slirp);
- tcp_slowtimo(slirp);
- slirp->last_slowtimo = curtime;
+ if (so->so_state & SS_NOFDREF || so->s == -1) {
+ continue;
}
/*
- * Check sockets
+ * Check for URG data
+ * This will soread as well, so no need to
+ * test for G_IO_IN below if this succeeds
*/
- if (!select_error) {
+ if (revents & G_IO_PRI) {
+ sorecvoob(so);
+ }
+ /*
+ * Check sockets for reading
+ */
+ else if (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) {
/*
- * Check TCP sockets
+ * Check for incoming connections
*/
- for (so = slirp->tcb.so_next; so != &slirp->tcb;
- so = so_next) {
- int revents;
-
- so_next = so->so_next;
-
- revents = 0;
- if (so->pollfds_idx != -1) {
- revents = g_array_index(pollfds, GPollFD,
- so->pollfds_idx).revents;
- }
+ if (so->so_state & SS_FACCEPTCONN) {
+ tcp_connect(so);
+ continue;
+ } /* else */
+ ret = soread(so);
- if (so->so_state & SS_NOFDREF || so->s == -1) {
- continue;
- }
+ /* Output it if we read something */
+ if (ret > 0) {
+ tcp_output(sototcpcb(so));
+ }
+ }
- /*
- * Check for URG data
- * This will soread as well, so no need to
- * test for G_IO_IN below if this succeeds
- */
- if (revents & G_IO_PRI) {
- sorecvoob(so);
- }
- /*
- * Check sockets for reading
- */
- else if (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) {
- /*
- * Check for incoming connections
- */
- if (so->so_state & SS_FACCEPTCONN) {
- tcp_connect(so);
+ /*
+ * Check sockets for writing
+ */
+ if (!(so->so_state & SS_NOFDREF) &&
+ (revents & (G_IO_OUT | G_IO_ERR))) {
+ /*
+ * Check for non-blocking, still-connecting sockets
+ */
+ if (so->so_state & SS_ISFCONNECTING) {
+ /* Connected */
+ so->so_state &= ~SS_ISFCONNECTING;
+
+ ret = send(so->s, (const void *) &ret, 0, 0);
+ if (ret < 0) {
+ /* XXXXX Must fix, zero bytes is a NOP */
+ if (errno == EAGAIN || errno == EWOULDBLOCK ||
+ errno == EINPROGRESS || errno == ENOTCONN) {
continue;
- } /* else */
- ret = soread(so);
-
- /* Output it if we read something */
- if (ret > 0) {
- tcp_output(sototcpcb(so));
}
- }
- /*
- * Check sockets for writing
- */
- if (!(so->so_state & SS_NOFDREF) &&
- (revents & (G_IO_OUT | G_IO_ERR))) {
- /*
- * Check for non-blocking, still-connecting sockets
- */
- if (so->so_state & SS_ISFCONNECTING) {
- /* Connected */
- so->so_state &= ~SS_ISFCONNECTING;
-
- ret = send(so->s, (const void *) &ret, 0, 0);
- if (ret < 0) {
- /* XXXXX Must fix, zero bytes is a NOP */
- if (errno == EAGAIN || errno == EWOULDBLOCK ||
- errno == EINPROGRESS || errno == ENOTCONN) {
- continue;
- }
-
- /* else failed */
- so->so_state &= SS_PERSISTENT_MASK;
- so->so_state |= SS_NOFDREF;
- }
- /* else so->so_state &= ~SS_ISFCONNECTING; */
-
- /*
- * Continue tcp_input
- */
- tcp_input((struct mbuf *)NULL, sizeof(struct ip), so);
- /* continue; */
- } else {
- ret = sowrite(so);
- }
- /*
- * XXXXX If we wrote something (a lot), there
- * could be a need for a window update.
- * In the worst case, the remote will send
- * a window probe to get things going again
- */
+ /* else failed */
+ so->so_state &= SS_PERSISTENT_MASK;
+ so->so_state |= SS_NOFDREF;
}
+ /* else so->so_state &= ~SS_ISFCONNECTING; */
/*
- * Probe a still-connecting, non-blocking socket
- * to check if it's still alive
+ * Continue tcp_input
*/
-#ifdef PROBE_CONN
- if (so->so_state & SS_ISFCONNECTING) {
- ret = qemu_recv(so->s, &ret, 0, 0);
-
- if (ret < 0) {
- /* XXX */
- if (errno == EAGAIN || errno == EWOULDBLOCK ||
- errno == EINPROGRESS || errno == ENOTCONN) {
- continue; /* Still connecting, continue */
- }
-
- /* else failed */
- so->so_state &= SS_PERSISTENT_MASK;
- so->so_state |= SS_NOFDREF;
-
- /* tcp_input will take care of it */
- } else {
- ret = send(so->s, &ret, 0, 0);
- if (ret < 0) {
- /* XXX */
- if (errno == EAGAIN || errno == EWOULDBLOCK ||
- errno == EINPROGRESS || errno == ENOTCONN) {
- continue;
- }
- /* else failed */
- so->so_state &= SS_PERSISTENT_MASK;
- so->so_state |= SS_NOFDREF;
- } else {
- so->so_state &= ~SS_ISFCONNECTING;
- }
-
- }
- tcp_input((struct mbuf *)NULL, sizeof(struct ip), so);
- } /* SS_ISFCONNECTING */
-#endif
+ tcp_input((struct mbuf *)NULL, sizeof(struct ip), so);
+ /* continue; */
+ } else {
+ ret = sowrite(so);
}
/*
- * Now UDP sockets.
- * Incoming packets are sent straight away, they're not buffered.
- * Incoming UDP data isn't buffered either.
+ * XXXXX If we wrote something (a lot), there
+ * could be a need for a window update.
+ * In the worst case, the remote will send
+ * a window probe to get things going again
*/
- for (so = slirp->udb.so_next; so != &slirp->udb;
- so = so_next) {
- int revents;
-
- so_next = so->so_next;
+ }
- revents = 0;
- if (so->pollfds_idx != -1) {
- revents = g_array_index(pollfds, GPollFD,
- so->pollfds_idx).revents;
+ /*
+ * Probe a still-connecting, non-blocking socket
+ * to check if it's still alive
+ */
+#ifdef PROBE_CONN
+ if (so->so_state & SS_ISFCONNECTING) {
+ ret = qemu_recv(so->s, &ret, 0, 0);
+
+ if (ret < 0) {
+ /* XXX */
+ if (errno == EAGAIN || errno == EWOULDBLOCK ||
+ errno == EINPROGRESS || errno == ENOTCONN) {
+ continue; /* Still connecting, continue */
}
- if (so->s != -1 &&
- (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR))) {
- sorecvfrom(so);
+ /* else failed */
+ so->so_state &= SS_PERSISTENT_MASK;
+ so->so_state |= SS_NOFDREF;
+
+ /* tcp_input will take care of it */
+ } else {
+ ret = send(so->s, &ret, 0, 0);
+ if (ret < 0) {
+ /* XXX */
+ if (errno == EAGAIN || errno == EWOULDBLOCK ||
+ errno == EINPROGRESS || errno == ENOTCONN) {
+ continue;
+ }
+ /* else failed */
+ so->so_state &= SS_PERSISTENT_MASK;
+ so->so_state |= SS_NOFDREF;
+ } else {
+ so->so_state &= ~SS_ISFCONNECTING;
}
+
}
+ tcp_input((struct mbuf *)NULL, sizeof(struct ip), so);
+ } /* SS_ISFCONNECTING */
+#endif
+ }
- /*
- * Check incoming ICMP relies.
- */
- for (so = slirp->icmp.so_next; so != &slirp->icmp;
- so = so_next) {
- int revents;
+ /*
+ * Now UDP sockets.
+ * Incoming packets are sent straight away, they're not buffered.
+ * Incoming UDP data isn't buffered either.
+ */
+ for (so = slirp->udb.so_next; so != &slirp->udb;
+ so = so_next) {
+ int revents;
- so_next = so->so_next;
+ so_next = so->so_next;
- revents = 0;
- if (so->pollfds_idx != -1) {
- revents = g_array_index(pollfds, GPollFD,
- so->pollfds_idx).revents;
- }
+ revents = 0;
+ if (so->pollfd) {
+ revents = so->pollfd->revents;
+ }
- if (so->s != -1 &&
- (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR))) {
- icmp_receive(so);
- }
- }
+ if (so->s != -1 &&
+ (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR))) {
+ sorecvfrom(so);
}
+ }
+
+ /*
+ * Check incoming ICMP relies.
+ */
+ for (so = slirp->icmp.so_next; so != &slirp->icmp;
+ so = so_next) {
+ int revents;
+
+ so_next = so->so_next;
- if_start(slirp);
+ revents = 0;
+ if (so->pollfd) {
+ revents = so->pollfd->revents;
+ }
+
+ if (so->s != -1 &&
+ (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR))) {
+ icmp_receive(so);
+ }
}
+
+ if_start(slirp);
+ return true;
}
static void arp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
diff --git a/slirp/socket.c b/slirp/socket.c
index bb639ae..058d2e3 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -52,6 +52,7 @@ socreate(Slirp *slirp)
so->s = -1;
so->slirp = slirp;
so->pollfds_idx = -1;
+ so->pollfd = slirp_gsource_get_gfd(slirp->opaque, so->s);
}
return(so);
}
@@ -64,6 +65,7 @@ sofree(struct socket *so)
{
Slirp *slirp = so->slirp;
+ slirp_gsource_close_gfd(slirp->opaque, so->pollfd);
if (so->so_emu==EMU_RSH && so->extra) {
sofree(so->extra);
so->extra=NULL;
diff --git a/slirp/socket.h b/slirp/socket.h
index 57e0407..522c5f0 100644
--- a/slirp/socket.h
+++ b/slirp/socket.h
@@ -21,6 +21,7 @@ struct socket {
int s; /* The actual socket */
int pollfds_idx; /* GPollFD GArray index */
+ GPollFD *pollfd;
Slirp *slirp; /* managing slirp instance */
diff --git a/stubs/slirp.c b/stubs/slirp.c
index f1fc833..c343364 100644
--- a/stubs/slirp.c
+++ b/stubs/slirp.c
@@ -5,11 +5,3 @@ void slirp_update_timeout(uint32_t *timeout)
{
}
-void slirp_pollfds_fill(GArray *pollfds)
-{
-}
-
-void slirp_pollfds_poll(GArray *pollfds, int select_error)
-{
-}
-
--
1.7.4.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [RFC PATCH v5 13/14] slirp: handle race condition
2013-04-26 2:47 [Qemu-devel] [RFC PATCH v5 00/14] port network layer onto glib Liu Ping Fan
` (11 preceding siblings ...)
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 12/14] slirp: make slirp event dispatch based on slirp instance, not global Liu Ping Fan
@ 2013-04-26 2:47 ` Liu Ping Fan
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 14/14] slirp: use lock to protect the slirp_instances Liu Ping Fan
13 siblings, 0 replies; 22+ messages in thread
From: Liu Ping Fan @ 2013-04-26 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: mdroth, Jan Kiszka, Stefan Hajnoczi, Anthony Liguori,
Paolo Bonzini
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
Slirp and its peer can run on different context at the same time.
Using lock to protect. Lock rule: no extra lock can be hold after
slirp->lock. This will protect us from deadlock when calling to peer.
As to coding style, they accord to the nearby code's style.
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
slirp/if.c | 57 ++++++++++++++++++++++++++++++++--------
slirp/main.h | 3 +-
slirp/mbuf.h | 2 +
slirp/slirp.c | 81 ++++++++++++++++++++++++++++++++++++++++++---------------
slirp/slirp.h | 6 +++-
5 files changed, 115 insertions(+), 34 deletions(-)
diff --git a/slirp/if.c b/slirp/if.c
index dcd5faf..b6a30a8 100644
--- a/slirp/if.c
+++ b/slirp/if.c
@@ -132,12 +132,21 @@ diddit:
}
}
-#ifndef FULL_BOLT
- /*
- * This prevents us from malloc()ing too many mbufs
- */
- if_start(ifm->slirp);
-#endif
+}
+
+static void mbuf_free(gpointer data, gpointer user_data)
+{
+ struct mbuf *ifm = data;
+ m_free(ifm);
+}
+
+static void if_send_free(gpointer data, gpointer user_data)
+{
+ struct mbuf *ifm = data;
+ Slirp *slirp = user_data;
+
+ if_encap(slirp, ifm);
+ m_free(ifm);
}
/*
@@ -156,7 +165,10 @@ void if_start(Slirp *slirp)
{
uint64_t now = qemu_get_clock_ns(rt_clock);
bool from_batchq, next_from_batchq;
- struct mbuf *ifm, *ifm_next, *ifqt;
+ struct mbuf *ifm, *ifm_next, *ifqt, *mclone;
+ GList *drop_list, *send_list;
+ drop_list = send_list = NULL;
+ int ret;
DEBUG_CALL("if_start");
@@ -192,9 +204,27 @@ void if_start(Slirp *slirp)
}
/* Try to send packet unless it already expired */
- if (ifm->expiration_date >= now && !if_encap(slirp, ifm)) {
- /* Packet is delayed due to pending ARP resolution */
- continue;
+ if (ifm->expiration_date < now) {
+ drop_list = g_list_append(drop_list, ifm);
+ } else {
+ ret = if_query(slirp, ifm);
+ switch (ret) {
+ case 2:
+ send_list = g_list_append(send_list, ifm);
+ break;
+ case 1:
+ mclone = m_get(slirp);
+ m_copy(mclone, ifm, 0, ifm->m_len);
+ mclone->arp_requested = true;
+ send_list = g_list_append(send_list, mclone);
+ /* Packet is delayed due to pending ARP resolution */
+ continue;
+ case 0:
+ continue;
+ case -1:
+ drop_list = g_list_append(drop_list, ifm);
+ break;
+ }
}
if (ifm == slirp->next_m) {
@@ -230,8 +260,13 @@ void if_start(Slirp *slirp)
ifm->ifq_so->so_nqueued = 0;
}
- m_free(ifm);
}
slirp->if_start_busy = false;
+ qemu_mutex_unlock(&slirp->lock);
+
+ g_list_foreach(drop_list, mbuf_free, NULL);
+ g_list_free(drop_list);
+ g_list_foreach(send_list, if_send_free, slirp);
+ g_list_free(send_list);
}
diff --git a/slirp/main.h b/slirp/main.h
index f2e58cf..c0b7881 100644
--- a/slirp/main.h
+++ b/slirp/main.h
@@ -44,7 +44,8 @@ extern int tcp_keepintvl;
#define PROTO_PPP 0x2
#endif
-int if_encap(Slirp *slirp, struct mbuf *ifm);
+int if_query(Slirp *slirp, struct mbuf *ifm);
+void if_encap(Slirp *slirp, struct mbuf *ifm);
ssize_t slirp_send(struct socket *so, const void *buf, size_t len, int flags);
#endif
diff --git a/slirp/mbuf.h b/slirp/mbuf.h
index 3f3ab09..a61ab94 100644
--- a/slirp/mbuf.h
+++ b/slirp/mbuf.h
@@ -34,6 +34,7 @@
#define _MBUF_H_
#define MINCSIZE 4096 /* Amount to increase mbuf if too small */
+#define ETH_ALEN 6
/*
* Macros for type conversion
@@ -82,6 +83,7 @@ struct m_hdr {
struct mbuf {
struct m_hdr m_hdr;
Slirp *slirp;
+ uint8_t ethaddr[ETH_ALEN];
bool arp_requested;
uint64_t expiration_date;
/* start of dynamic buffer area, must be last element */
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 691f82f..8f5cbe0 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -206,6 +206,7 @@ Slirp *slirp_init(int restricted, struct in_addr vnetwork,
slirp_init_once();
+ qemu_mutex_init(&slirp->lock);
slirp->restricted = restricted;
if_init(slirp);
@@ -248,6 +249,7 @@ void slirp_cleanup(Slirp *slirp)
ip_cleanup(slirp);
m_cleanup(slirp);
+ qemu_mutex_destroy(&slirp->lock);
g_free(slirp->vdnssearch);
g_free(slirp->tftp_prefix);
@@ -410,6 +412,7 @@ gboolean slirp_handler(gpointer data)
struct socket *so, *so_next;
int ret;
+ qemu_mutex_lock(&slirp->lock);
/*
* See if anything has timed out
*/
@@ -593,6 +596,7 @@ gboolean slirp_handler(gpointer data)
}
}
+ /* drop the slirp->lock inside it */
if_start(slirp);
return true;
}
@@ -612,6 +616,7 @@ static void arp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
if (ah->ar_tip == ah->ar_sip) {
/* Gratuitous ARP */
arp_table_add(slirp, ah->ar_sip, ah->ar_sha);
+ qemu_mutex_unlock(&slirp->lock);
return;
}
@@ -624,6 +629,7 @@ static void arp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
if (ex_ptr->ex_addr.s_addr == ah->ar_tip)
goto arp_ok;
}
+ qemu_mutex_unlock(&slirp->lock);
return;
arp_ok:
memset(arp_reply, 0, sizeof(arp_reply));
@@ -645,13 +651,19 @@ static void arp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
rah->ar_sip = ah->ar_tip;
memcpy(rah->ar_tha, ah->ar_sha, ETH_ALEN);
rah->ar_tip = ah->ar_sip;
+ qemu_mutex_unlock(&slirp->lock);
+ /* lock should be dropped before calling peer */
slirp_output(slirp->opaque, arp_reply, sizeof(arp_reply));
+ } else {
+ qemu_mutex_unlock(&slirp->lock);
}
break;
case ARPOP_REPLY:
arp_table_add(slirp, ah->ar_sip, ah->ar_sha);
+ qemu_mutex_unlock(&slirp->lock);
break;
default:
+ qemu_mutex_unlock(&slirp->lock);
break;
}
}
@@ -665,14 +677,18 @@ void slirp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
return;
proto = ntohs(*(uint16_t *)(pkt + 12));
+
+ qemu_mutex_lock(&slirp->lock);
switch(proto) {
case ETH_P_ARP:
+ /* drop slirp->lock inside */
arp_input(slirp, pkt, pkt_len);
- break;
+ return;
case ETH_P_IP:
m = m_get(slirp);
- if (!m)
- return;
+ if (!m) {
+ break;
+ }
/* Note: we add to align the IP header */
if (M_FREEROOM(m) < pkt_len + 2) {
m_inc(m, pkt_len + 2);
@@ -682,34 +698,51 @@ void slirp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
m->m_data += 2 + ETH_HLEN;
m->m_len -= 2 + ETH_HLEN;
-
+ /* It just append packet, does not send immediately,
+ * so no need to drop slirp->lock inside.
+ */
ip_input(m);
- break;
+ /* drop slirp->lock inside */
+ if_start(slirp);
+ return;
default:
break;
}
+ qemu_mutex_unlock(&slirp->lock);
}
-/* Output the IP packet to the ethernet device. Returns 0 if the packet must be
- * re-queued.
- */
-int if_encap(Slirp *slirp, struct mbuf *ifm)
+/* -1 silent drop, 0 sllent, 1 need send out arp, 2 normal */
+int if_query(Slirp *slirp, struct mbuf *ifm)
{
uint8_t buf[1600];
- struct ethhdr *eh = (struct ethhdr *)buf;
- uint8_t ethaddr[ETH_ALEN];
const struct ip *iph = (const struct ip *)ifm->m_data;
if (ifm->m_len + ETH_HLEN > sizeof(buf)) {
+ return -1;
+ }
+ if (ifm->arp_requested) {
+ return 0;
+ }
+ if (!arp_table_search(slirp, iph->ip_dst.s_addr, ifm->ethaddr)) {
+ ifm->arp_requested = true;
return 1;
}
+ return 2;
+}
- if (!arp_table_search(slirp, iph->ip_dst.s_addr, ethaddr)) {
+/* Output the IP packet to the ethernet device.
+ */
+void if_encap(Slirp *slirp, struct mbuf *ifm)
+{
+ uint8_t buf[1600];
+ struct ethhdr *eh = (struct ethhdr *)buf;
+ const struct ip *iph = (const struct ip *)ifm->m_data;
+
+ if (ifm->arp_requested) {
uint8_t arp_req[ETH_HLEN + sizeof(struct arphdr)];
struct ethhdr *reh = (struct ethhdr *)arp_req;
struct arphdr *rah = (struct arphdr *)(arp_req + ETH_HLEN);
- if (!ifm->arp_requested) {
/* If the client addr is not known, send an ARP request */
memset(reh->h_dest, 0xff, ETH_ALEN);
memcpy(reh->h_source, special_ethaddr, ETH_ALEN - 4);
@@ -735,21 +768,17 @@ int if_encap(Slirp *slirp, struct mbuf *ifm)
rah->ar_tip = iph->ip_dst.s_addr;
slirp->client_ipaddr = iph->ip_dst;
slirp_output(slirp->opaque, arp_req, sizeof(arp_req));
- ifm->arp_requested = true;
/* Expire request and drop outgoing packet after 1 second */
ifm->expiration_date = qemu_get_clock_ns(rt_clock) + 1000000000ULL;
- }
- return 0;
} else {
- memcpy(eh->h_dest, ethaddr, ETH_ALEN);
+ memcpy(eh->h_dest, ifm->ethaddr, ETH_ALEN);
memcpy(eh->h_source, special_ethaddr, ETH_ALEN - 4);
/* XXX: not correct */
memcpy(&eh->h_source[2], &slirp->vhost_addr, 4);
eh->h_proto = htons(ETH_P_IP);
memcpy(buf + sizeof(struct ethhdr), ifm->m_data, ifm->m_len);
slirp_output(slirp->opaque, buf, ifm->m_len + ETH_HLEN);
- return 1;
}
}
@@ -860,15 +889,25 @@ void slirp_socket_recv(Slirp *slirp, struct in_addr guest_addr, int guest_port,
const uint8_t *buf, int size)
{
int ret;
- struct socket *so = slirp_find_ctl_socket(slirp, guest_addr, guest_port);
+ struct socket *so;
+
+ qemu_mutex_lock(&slirp->lock);
+ so = slirp_find_ctl_socket(slirp, guest_addr, guest_port);
- if (!so)
+ if (!so) {
+ qemu_mutex_unlock(&slirp->lock);
return;
+ }
ret = soreadbuf(so, (const char *)buf, size);
- if (ret > 0)
+ if (ret > 0) {
tcp_output(sototcpcb(so));
+ /* release lock inside */
+ if_start(slirp);
+ return;
+ }
+ qemu_mutex_unlock(&slirp->lock);
}
static void slirp_tcp_save(QEMUFile *f, struct tcpcb *tp)
diff --git a/slirp/slirp.h b/slirp/slirp.h
index 008360e..8ec0888 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -135,6 +135,7 @@ void free(void *ptr);
#include "qemu/queue.h"
#include "qemu/sockets.h"
+#include "qemu/thread.h"
#include "libslirp.h"
#include "ip.h"
@@ -158,7 +159,6 @@ void free(void *ptr);
#include "bootp.h"
#include "tftp.h"
-#define ETH_ALEN 6
#define ETH_HLEN 14
#define ETH_P_IP 0x0800 /* Internet Protocol packet */
@@ -207,6 +207,10 @@ struct Slirp {
u_int last_slowtimo;
int do_slowtimo;
+ /* lock to protect slirp running both on frontend or SlirpState context.
+ * Lock rule: biglock ->lock. Should be dropped before calling peer.
+ */
+ QemuMutex lock;
/* virtual network configuration */
struct in_addr vnetwork_addr;
struct in_addr vnetwork_mask;
--
1.7.4.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [RFC PATCH v5 14/14] slirp: use lock to protect the slirp_instances
2013-04-26 2:47 [Qemu-devel] [RFC PATCH v5 00/14] port network layer onto glib Liu Ping Fan
` (12 preceding siblings ...)
2013-04-26 2:47 ` [Qemu-devel] [RFC PATCH v5 13/14] slirp: handle race condition Liu Ping Fan
@ 2013-04-26 2:47 ` Liu Ping Fan
13 siblings, 0 replies; 22+ messages in thread
From: Liu Ping Fan @ 2013-04-26 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: mdroth, Jan Kiszka, Stefan Hajnoczi, Anthony Liguori,
Paolo Bonzini
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
slirps will run on dedicated thread, and dynamically join or disjoin
this list, so need lock to protect the global list.
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
include/qemu/module.h | 2 ++
slirp/slirp.c | 20 ++++++++++++++++++++
2 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/include/qemu/module.h b/include/qemu/module.h
index c4ccd57..2720943 100644
--- a/include/qemu/module.h
+++ b/include/qemu/module.h
@@ -22,6 +22,7 @@ static void __attribute__((constructor)) do_qemu_init_ ## function(void) { \
typedef enum {
MODULE_INIT_BLOCK,
+ MODULE_INIT_SLIRP,
MODULE_INIT_MACHINE,
MODULE_INIT_QAPI,
MODULE_INIT_QOM,
@@ -29,6 +30,7 @@ typedef enum {
} module_init_type;
#define block_init(function) module_init(function, MODULE_INIT_BLOCK)
+#define slirplayer_init(function) module_init(function, MODULE_INIT_SLIRP)
#define machine_init(function) module_init(function, MODULE_INIT_MACHINE)
#define qapi_init(function) module_init(function, MODULE_INIT_QAPI)
#define type_init(function) module_init(function, MODULE_INIT_QOM)
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 8f5cbe0..3008c7b 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -42,6 +42,7 @@ static const uint8_t zero_ethaddr[ETH_ALEN] = { 0, 0, 0, 0, 0, 0 };
u_int curtime;
+static QemuMutex slirp_instances_lock;
static QTAILQ_HEAD(slirp_instances, Slirp) slirp_instances =
QTAILQ_HEAD_INITIALIZER(slirp_instances);
@@ -236,14 +237,18 @@ Slirp *slirp_init(int restricted, struct in_addr vnetwork,
register_savevm(NULL, "slirp", 0, 3,
slirp_state_save, slirp_state_load, slirp);
+ qemu_mutex_lock(&slirp_instances_lock);
QTAILQ_INSERT_TAIL(&slirp_instances, slirp, entry);
+ qemu_mutex_unlock(&slirp_instances_lock);
return slirp;
}
void slirp_cleanup(Slirp *slirp)
{
+ qemu_mutex_lock(&slirp_instances_lock);
QTAILQ_REMOVE(&slirp_instances, slirp, entry);
+ qemu_mutex_unlock(&slirp_instances_lock);
unregister_savevm(NULL, "slirp", slirp);
@@ -262,9 +267,12 @@ void slirp_cleanup(Slirp *slirp)
void slirp_update_timeout(uint32_t *timeout)
{
+ qemu_mutex_lock(&slirp_instances_lock);
if (!QTAILQ_EMPTY(&slirp_instances)) {
*timeout = MIN(1000, *timeout);
}
+ qemu_mutex_unlock(&slirp_instances_lock);
+
curtime = qemu_get_clock_ms(rt_clock);
}
@@ -1167,3 +1175,15 @@ static int slirp_state_load(QEMUFile *f, void *opaque, int version_id)
return 0;
}
+
+static void slirplayer_cleanup(void)
+{
+ qemu_mutex_destroy(&slirp_instances_lock);
+}
+
+static void slirplayer_bootup(void)
+{
+ qemu_mutex_init(&slirp_instances_lock);
+ atexit(&slirplayer_cleanup);
+}
+slirplayer_init(slirplayer_bootup)
--
1.7.4.4
^ permalink raw reply related [flat|nested] 22+ messages in thread