Patchwork [01/16] net: Add a hub net client

login
register
mail settings
Submitter Stefan Hajnoczi
Date July 20, 2012, 12:01 p.m.
Message ID <1342785709-3152-2-git-send-email-stefanha@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/172230/
State New
Headers show

Comments

Stefan Hajnoczi - July 20, 2012, 12:01 p.m.
The vlan feature can be implemented in terms of hubs.  By introducing a
hub net client it becomes possible to remove the special case vlan code
from net.c and push the vlan feature out of generic networking code.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
---
 net.c             |   17 ++--
 net/Makefile.objs |    2 +-
 net/hub.c         |  223 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 net/hub.h         |   26 +++++++
 qapi-schema.json  |   30 +++++--
 5 files changed, 282 insertions(+), 16 deletions(-)
 create mode 100644 net/hub.c
 create mode 100644 net/hub.h
Laszlo Ersek - July 23, 2012, 12:45 p.m.
Two hairs to split:

On 07/20/12 14:01, Stefan Hajnoczi wrote:

> +static NetHubPort *net_hub_port_new(NetHub *hub, const char *name)
> +{
> +    VLANClientState *nc;
> +    NetHubPort *port;
> +    unsigned int id = hub->num_ports++;

There are projects that don't like to put logic or externally visible
side-effects into initializers. I don't know about qemu.

> diff --git a/qapi-schema.json b/qapi-schema.json
> index bc55ed2..6618eb5 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2094,6 +2094,19 @@
>      '*helper': 'str' } }
>  
>  ##
> +# @NetdevHubPortOptions
> +#
> +# Connect two or more net clients through a software hub.
> +#
> +# @hubid: hub identifier number
> +#
> +# Since 1.2
> +##
> +{ 'type': 'NetdevHubPortOptions',
> +  'data': {
> +    'hubid':     'int' } }

I think this should say 'uint32'.

Thanks,
Laszlo
Stefan Hajnoczi - July 23, 2012, 1:49 p.m.
On Mon, Jul 23, 2012 at 1:45 PM, Laszlo Ersek <lersek@redhat.com> wrote:
> Two hairs to split:
>
> On 07/20/12 14:01, Stefan Hajnoczi wrote:
>
>> +static NetHubPort *net_hub_port_new(NetHub *hub, const char *name)
>> +{
>> +    VLANClientState *nc;
>> +    NetHubPort *port;
>> +    unsigned int id = hub->num_ports++;
>
> There are projects that don't like to put logic or externally visible
> side-effects into initializers. I don't know about qemu.

I see what you're saying, we also add it to the hub's port list
further down.  It could be split into _new() and hub_add_port(hub,
port) but then autogenerating a unique name becomes harder.  Since
this function is static, the scope is limited and we can assume
callers understand what is going on here.

I'd like to leave it this way or do you see a concrete change that
improves the code?

>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index bc55ed2..6618eb5 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -2094,6 +2094,19 @@
>>      '*helper': 'str' } }
>>
>>  ##
>> +# @NetdevHubPortOptions
>> +#
>> +# Connect two or more net clients through a software hub.
>> +#
>> +# @hubid: hub identifier number
>> +#
>> +# Since 1.2
>> +##
>> +{ 'type': 'NetdevHubPortOptions',
>> +  'data': {
>> +    'hubid':     'int' } }
>
> I think this should say 'uint32'.

Okay.

Stefan
Laszlo Ersek - July 23, 2012, 2:05 p.m.
On 07/23/12 15:49, Stefan Hajnoczi wrote:
> On Mon, Jul 23, 2012 at 1:45 PM, Laszlo Ersek <lersek@redhat.com> wrote:
>> Two hairs to split:
>>
>> On 07/20/12 14:01, Stefan Hajnoczi wrote:
>>
>>> +static NetHubPort *net_hub_port_new(NetHub *hub, const char *name)
>>> +{
>>> +    VLANClientState *nc;
>>> +    NetHubPort *port;
>>> +    unsigned int id = hub->num_ports++;
>>
>> There are projects that don't like to put logic or externally visible
>> side-effects into initializers. I don't know about qemu.
> 
> I see what you're saying, we also add it to the hub's port list
> further down.  It could be split into _new() and hub_add_port(hub,
> port) but then autogenerating a unique name becomes harder.  Since
> this function is static, the scope is limited and we can assume
> callers understand what is going on here.
> 
> I'd like to leave it this way or do you see a concrete change that
> improves the code?

Oh no, that's not what I meant -- the function is perfectly fine, it's
just that the post-increment of object A shouldn't be in the definition
/ initializer list of object B. (Keeping the increment and the
construcion in one place is actually a good thing IMHO.)

The idea is, rather than

    unsigned int id = hub->num_ports++;

it should say

    unsigned int id;
    /* ... */
    id = hub->num_ports++;

... Hm, yes, I remember it from
<http://www.manpages.info/freebsd/style.9.html>. But again, I'm not sure
how qemu treats this.

Laszlo
Stefan Hajnoczi - July 23, 2012, 2:52 p.m.
On Mon, Jul 23, 2012 at 3:05 PM, Laszlo Ersek <lersek@redhat.com> wrote:
> On 07/23/12 15:49, Stefan Hajnoczi wrote:
>> On Mon, Jul 23, 2012 at 1:45 PM, Laszlo Ersek <lersek@redhat.com> wrote:
>>> Two hairs to split:
>>>
>>> On 07/20/12 14:01, Stefan Hajnoczi wrote:
>>>
>>>> +static NetHubPort *net_hub_port_new(NetHub *hub, const char *name)
>>>> +{
>>>> +    VLANClientState *nc;
>>>> +    NetHubPort *port;
>>>> +    unsigned int id = hub->num_ports++;
>>>
>>> There are projects that don't like to put logic or externally visible
>>> side-effects into initializers. I don't know about qemu.
>>
>> I see what you're saying, we also add it to the hub's port list
>> further down.  It could be split into _new() and hub_add_port(hub,
>> port) but then autogenerating a unique name becomes harder.  Since
>> this function is static, the scope is limited and we can assume
>> callers understand what is going on here.
>>
>> I'd like to leave it this way or do you see a concrete change that
>> improves the code?
>
> Oh no, that's not what I meant -- the function is perfectly fine, it's
> just that the post-increment of object A shouldn't be in the definition
> / initializer list of object B. (Keeping the increment and the
> construcion in one place is actually a good thing IMHO.)
>
> The idea is, rather than
>
>     unsigned int id = hub->num_ports++;
>
> it should say
>
>     unsigned int id;
>     /* ... */
>     id = hub->num_ports++;
>
> ... Hm, yes, I remember it from
> <http://www.manpages.info/freebsd/style.9.html>. But again, I'm not sure
> how qemu treats this.

"Be careful to not obfuscate the code by initializing variables in the
declarations.  Use this feature only thoughtfully.  DO NOT use
function calls in initializers."

This?

Do you know the rationale?  It's not clear to me how this "obfuscates" the code.

Messing around with side-effects in a series of variable declarations
with intializers could be bug-prone.  But here there is only one
initializer so it's not a problem.

Regarding QEMU, there's no coding style rule against initializers.
Personally I think that's a good thing because it keeps the code
concise - people reading it don't have to keep the declaration in
their mind until they hit the initializer later on.

Stefan
Laszlo Ersek - July 23, 2012, 3:16 p.m.
On 07/23/12 16:52, Stefan Hajnoczi wrote:
> On Mon, Jul 23, 2012 at 3:05 PM, Laszlo Ersek <lersek@redhat.com> wrote:

>> The idea is, rather than
>>
>>     unsigned int id = hub->num_ports++;
>>
>> it should say
>>
>>     unsigned int id;
>>     /* ... */
>>     id = hub->num_ports++;
>>
>> ... Hm, yes, I remember it from
>> <http://www.manpages.info/freebsd/style.9.html>. But again, I'm not sure
>> how qemu treats this.
> 
> "Be careful to not obfuscate the code by initializing variables in the
> declarations.  Use this feature only thoughtfully.  DO NOT use
> function calls in initializers."
> 
> This?
> 
> Do you know the rationale?  It's not clear to me how this "obfuscates" the code.

I think the rationale is that (a) people tend to reorganize definitions,
and the expressions in the initializer lists may depend on the original
order, (b) even worse with removal of variables, (c) many people have a
"conceptual divide" between the definition block and the logic below it,
and the first should set constant defaults at most. (Naturally this
prevents C99/C++ style mixing of definitions and code as well, at least
without explicit braces.)

I'm one of those people, but again I'm not sure if qemu has any
guideline on this.


> Messing around with side-effects in a series of variable declarations
> with intializers could be bug-prone.  But here there is only one
> initializer so it's not a problem.
> 
> Regarding QEMU, there's no coding style rule against initializers.
> Personally I think that's a good thing because it keeps the code
> concise - people reading it don't have to keep the declaration in
> their mind until they hit the initializer later on.

Well I keep the definitions at the top of the block so I can very easily
return to them visually (and be sure they do nothing else than define
variables / declare externs), but it's getting philosophical :)

I have nothing against this initializer as-is, I just wanted to voice
*if* there's such a guideline in qemu *then* it should be followed :)

Thanks!
Laszlo
Stefan Hajnoczi - July 23, 2012, 3:23 p.m.
On Mon, Jul 23, 2012 at 4:16 PM, Laszlo Ersek <lersek@redhat.com> wrote:
> On 07/23/12 16:52, Stefan Hajnoczi wrote:
>> On Mon, Jul 23, 2012 at 3:05 PM, Laszlo Ersek <lersek@redhat.com> wrote:
>
>>> The idea is, rather than
>>>
>>>     unsigned int id = hub->num_ports++;
>>>
>>> it should say
>>>
>>>     unsigned int id;
>>>     /* ... */
>>>     id = hub->num_ports++;
>>>
>>> ... Hm, yes, I remember it from
>>> <http://www.manpages.info/freebsd/style.9.html>. But again, I'm not sure
>>> how qemu treats this.
>>
>> "Be careful to not obfuscate the code by initializing variables in the
>> declarations.  Use this feature only thoughtfully.  DO NOT use
>> function calls in initializers."
>>
>> This?
>>
>> Do you know the rationale?  It's not clear to me how this "obfuscates" the code.
>
> I think the rationale is that (a) people tend to reorganize definitions,
> and the expressions in the initializer lists may depend on the original
> order, (b) even worse with removal of variables, (c) many people have a
> "conceptual divide" between the definition block and the logic below it,
> and the first should set constant defaults at most. (Naturally this
> prevents C99/C++ style mixing of definitions and code as well, at least
> without explicit braces.)
>
> I'm one of those people, but again I'm not sure if qemu has any
> guideline on this.
>
>
>> Messing around with side-effects in a series of variable declarations
>> with intializers could be bug-prone.  But here there is only one
>> initializer so it's not a problem.
>>
>> Regarding QEMU, there's no coding style rule against initializers.
>> Personally I think that's a good thing because it keeps the code
>> concise - people reading it don't have to keep the declaration in
>> their mind until they hit the initializer later on.
>
> Well I keep the definitions at the top of the block so I can very easily
> return to them visually (and be sure they do nothing else than define
> variables / declare externs), but it's getting philosophical :)
>
> I have nothing against this initializer as-is, I just wanted to voice
> *if* there's such a guideline in qemu *then* it should be followed :)

Yes, I understand - it's a question of style or flamewar fodder :).  I
double-checked that there is no guideline in ./CODING_STYLE and
./HACKING.

Stefan
Andreas Färber - July 23, 2012, 4:33 p.m.
Am 23.07.2012 17:23, schrieb Stefan Hajnoczi:
> On Mon, Jul 23, 2012 at 4:16 PM, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 07/23/12 16:52, Stefan Hajnoczi wrote:
>>> On Mon, Jul 23, 2012 at 3:05 PM, Laszlo Ersek <lersek@redhat.com> wrote:
>>
>>>> The idea is, rather than
>>>>
>>>>     unsigned int id = hub->num_ports++;
>>>>
>>>> it should say
>>>>
>>>>     unsigned int id;
>>>>     /* ... */
>>>>     id = hub->num_ports++;
>>>
>>> "Be careful to not obfuscate the code by initializing variables in the
>>> declarations.  Use this feature only thoughtfully.  DO NOT use
>>> function calls in initializers."
>>>
>>> This?
>>>
>>> Do you know the rationale?  It's not clear to me how this "obfuscates" the code.
>>
>> I think the rationale is that (a) people tend to reorganize definitions,
>> and the expressions in the initializer lists may depend on the original
>> order, (b) even worse with removal of variables, (c) many people have a
>> "conceptual divide" between the definition block and the logic below it,
>> and the first should set constant defaults at most. (Naturally this
>> prevents C99/C++ style mixing of definitions and code as well, at least
>> without explicit braces.)
>>
>> I'm one of those people, but again I'm not sure if qemu has any
>> guideline on this.
>>
>>
>>> Messing around with side-effects in a series of variable declarations
>>> with intializers could be bug-prone.  But here there is only one
>>> initializer so it's not a problem.
>>>
>>> Regarding QEMU, there's no coding style rule against initializers.
>>> Personally I think that's a good thing because it keeps the code
>>> concise - people reading it don't have to keep the declaration in
>>> their mind until they hit the initializer later on.
>>
>> Well I keep the definitions at the top of the block so I can very easily
>> return to them visually (and be sure they do nothing else than define
>> variables / declare externs), but it's getting philosophical :)
>>
>> I have nothing against this initializer as-is, I just wanted to voice
>> *if* there's such a guideline in qemu *then* it should be followed :)
> 
> Yes, I understand - it's a question of style or flamewar fodder :).  I
> double-checked that there is no guideline in ./CODING_STYLE and
> ./HACKING.

Hm, I'm not so much into those documents [cc'ing Blue], but we used to
be stricter about ANSI C some years back (which iirc forbids
non-constant expressions in initializers?). FWIW we have since switched
to C99 struct initializers and use QOM cast macros (that translate to a
function call) in initializers. -ansi -pedantic is unlikely to get far.

Cheers,
Andreas
Markus Armbruster - July 23, 2012, 5:11 p.m.
Andreas Färber <afaerber@suse.de> writes:

> Hm, I'm not so much into those documents [cc'ing Blue], but we used to
> be stricter about ANSI C some years back (which iirc forbids
> non-constant expressions in initializers?). FWIW we have since switched

It doesn't, except when the variable has static storage duration.

> to C99 struct initializers and use QOM cast macros (that translate to a
> function call) in initializers. -ansi -pedantic is unlikely to get far.
Blue Swirl - July 23, 2012, 7:01 p.m.
On Mon, Jul 23, 2012 at 4:33 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 23.07.2012 17:23, schrieb Stefan Hajnoczi:
>> On Mon, Jul 23, 2012 at 4:16 PM, Laszlo Ersek <lersek@redhat.com> wrote:
>>> On 07/23/12 16:52, Stefan Hajnoczi wrote:
>>>> On Mon, Jul 23, 2012 at 3:05 PM, Laszlo Ersek <lersek@redhat.com> wrote:
>>>
>>>>> The idea is, rather than
>>>>>
>>>>>     unsigned int id = hub->num_ports++;
>>>>>
>>>>> it should say
>>>>>
>>>>>     unsigned int id;
>>>>>     /* ... */
>>>>>     id = hub->num_ports++;
>>>>
>>>> "Be careful to not obfuscate the code by initializing variables in the
>>>> declarations.  Use this feature only thoughtfully.  DO NOT use
>>>> function calls in initializers."
>>>>
>>>> This?
>>>>
>>>> Do you know the rationale?  It's not clear to me how this "obfuscates" the code.
>>>
>>> I think the rationale is that (a) people tend to reorganize definitions,
>>> and the expressions in the initializer lists may depend on the original
>>> order, (b) even worse with removal of variables, (c) many people have a
>>> "conceptual divide" between the definition block and the logic below it,
>>> and the first should set constant defaults at most. (Naturally this
>>> prevents C99/C++ style mixing of definitions and code as well, at least
>>> without explicit braces.)
>>>
>>> I'm one of those people, but again I'm not sure if qemu has any
>>> guideline on this.
>>>
>>>
>>>> Messing around with side-effects in a series of variable declarations
>>>> with intializers could be bug-prone.  But here there is only one
>>>> initializer so it's not a problem.
>>>>
>>>> Regarding QEMU, there's no coding style rule against initializers.
>>>> Personally I think that's a good thing because it keeps the code
>>>> concise - people reading it don't have to keep the declaration in
>>>> their mind until they hit the initializer later on.
>>>
>>> Well I keep the definitions at the top of the block so I can very easily
>>> return to them visually (and be sure they do nothing else than define
>>> variables / declare externs), but it's getting philosophical :)
>>>
>>> I have nothing against this initializer as-is, I just wanted to voice
>>> *if* there's such a guideline in qemu *then* it should be followed :)
>>
>> Yes, I understand - it's a question of style or flamewar fodder :).  I
>> double-checked that there is no guideline in ./CODING_STYLE and
>> ./HACKING.
>
> Hm, I'm not so much into those documents [cc'ing Blue], but we used to
> be stricter about ANSI C some years back (which iirc forbids
> non-constant expressions in initializers?). FWIW we have since switched
> to C99 struct initializers and use QOM cast macros (that translate to a
> function call) in initializers. -ansi -pedantic is unlikely to get far.

For example in block/vvfat.c, various initializers and nonstandard
stuff like variable length arrays have been used since day one. It's
not the finest example of code in QEMU though.

>
> Cheers,
> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>
>

Patch

diff --git a/net.c b/net.c
index dbca77b..e7a8d81 100644
--- a/net.c
+++ b/net.c
@@ -30,6 +30,7 @@ 
 #include "net/dump.h"
 #include "net/slirp.h"
 #include "net/vde.h"
+#include "net/hub.h"
 #include "net/util.h"
 #include "monitor.h"
 #include "qemu-common.h"
@@ -816,19 +817,20 @@  static int (* const net_client_init_fun[NET_CLIENT_OPTIONS_KIND_MAX])(
     const NetClientOptions *opts,
     const char *name,
     VLANState *vlan) = {
-        [NET_CLIENT_OPTIONS_KIND_NIC]    = net_init_nic,
+        [NET_CLIENT_OPTIONS_KIND_NIC]       = net_init_nic,
 #ifdef CONFIG_SLIRP
-        [NET_CLIENT_OPTIONS_KIND_USER]   = net_init_slirp,
+        [NET_CLIENT_OPTIONS_KIND_USER]      = net_init_slirp,
 #endif
-        [NET_CLIENT_OPTIONS_KIND_TAP]    = net_init_tap,
-        [NET_CLIENT_OPTIONS_KIND_SOCKET] = net_init_socket,
+        [NET_CLIENT_OPTIONS_KIND_TAP]       = net_init_tap,
+        [NET_CLIENT_OPTIONS_KIND_SOCKET]    = net_init_socket,
 #ifdef CONFIG_VDE
-        [NET_CLIENT_OPTIONS_KIND_VDE]    = net_init_vde,
+        [NET_CLIENT_OPTIONS_KIND_VDE]       = net_init_vde,
 #endif
-        [NET_CLIENT_OPTIONS_KIND_DUMP]   = net_init_dump,
+        [NET_CLIENT_OPTIONS_KIND_DUMP]      = net_init_dump,
 #ifdef CONFIG_NET_BRIDGE
-        [NET_CLIENT_OPTIONS_KIND_BRIDGE] = net_init_bridge,
+        [NET_CLIENT_OPTIONS_KIND_BRIDGE]    = net_init_bridge,
 #endif
+        [NET_CLIENT_OPTIONS_KIND_HUBPORT]   = net_init_hubport,
 };
 
 
@@ -858,6 +860,7 @@  static int net_client_init1(const void *object, int is_netdev, Error **errp)
 #ifdef CONFIG_NET_BRIDGE
         case NET_CLIENT_OPTIONS_KIND_BRIDGE:
 #endif
+        case NET_CLIENT_OPTIONS_KIND_HUBPORT:
             break;
 
         default:
diff --git a/net/Makefile.objs b/net/Makefile.objs
index 72f50bc..cf04187 100644
--- a/net/Makefile.objs
+++ b/net/Makefile.objs
@@ -1,4 +1,4 @@ 
-common-obj-y = queue.o checksum.o util.o
+common-obj-y = queue.o checksum.o util.o hub.o
 common-obj-y += socket.o
 common-obj-y += dump.o
 common-obj-$(CONFIG_POSIX) += tap.o
diff --git a/net/hub.c b/net/hub.c
new file mode 100644
index 0000000..cc58d41
--- /dev/null
+++ b/net/hub.c
@@ -0,0 +1,223 @@ 
+/*
+ * Hub net client
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Stefan Hajnoczi   <stefanha@linux.vnet.ibm.com>
+ *  Zhi Yong Wu       <wuzhy@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "monitor.h"
+#include "net.h"
+#include "hub.h"
+
+/*
+ * A hub broadcasts incoming packets to all its ports except the source port.
+ * Hubs can be used to provide independent network segments, also confusingly
+ * named the QEMU 'vlan' feature.
+ */
+
+typedef struct NetHub NetHub;
+
+typedef struct NetHubPort {
+    VLANClientState nc;
+    QLIST_ENTRY(NetHubPort) next;
+    NetHub *hub;
+    unsigned int id;
+} NetHubPort;
+
+struct NetHub {
+    unsigned int id;
+    QLIST_ENTRY(NetHub) next;
+    unsigned int num_ports;
+    QLIST_HEAD(, NetHubPort) ports;
+};
+
+static QLIST_HEAD(, NetHub) hubs = QLIST_HEAD_INITIALIZER(&hubs);
+
+static ssize_t net_hub_receive(NetHub *hub, NetHubPort *source_port,
+                               const uint8_t *buf, size_t len)
+{
+    NetHubPort *port;
+
+    QLIST_FOREACH(port, &hub->ports, next) {
+        if (port == source_port) {
+            continue;
+        }
+
+        qemu_send_packet(&port->nc, buf, len);
+    }
+    return len;
+}
+
+static ssize_t net_hub_receive_iov(NetHub *hub, NetHubPort *source_port,
+                                   const struct iovec *iov, int iovcnt)
+{
+    NetHubPort *port;
+    ssize_t ret = 0;
+
+    QLIST_FOREACH(port, &hub->ports, next) {
+        if (port == source_port) {
+            continue;
+        }
+
+        ret = qemu_sendv_packet(&port->nc, iov, iovcnt);
+    }
+    return ret;
+}
+
+static NetHub *net_hub_new(unsigned int id)
+{
+    NetHub *hub;
+
+    hub = g_malloc(sizeof(*hub));
+    hub->id = id;
+    hub->num_ports = 0;
+    QLIST_INIT(&hub->ports);
+
+    QLIST_INSERT_HEAD(&hubs, hub, next);
+
+    return hub;
+}
+
+static ssize_t net_hub_port_receive(VLANClientState *nc,
+                                    const uint8_t *buf, size_t len)
+{
+    NetHubPort *port = DO_UPCAST(NetHubPort, nc, nc);
+
+    return net_hub_receive(port->hub, port, buf, len);
+}
+
+static ssize_t net_hub_port_receive_iov(VLANClientState *nc,
+                                        const struct iovec *iov, int iovcnt)
+{
+    NetHubPort *port = DO_UPCAST(NetHubPort, nc, nc);
+
+    return net_hub_receive_iov(port->hub, port, iov, iovcnt);
+}
+
+static void net_hub_port_cleanup(VLANClientState *nc)
+{
+    NetHubPort *port = DO_UPCAST(NetHubPort, nc, nc);
+
+    QLIST_REMOVE(port, next);
+}
+
+static NetClientInfo net_hub_port_info = {
+    .type = NET_CLIENT_OPTIONS_KIND_HUBPORT,
+    .size = sizeof(NetHubPort),
+    .receive = net_hub_port_receive,
+    .receive_iov = net_hub_port_receive_iov,
+    .cleanup = net_hub_port_cleanup,
+};
+
+static NetHubPort *net_hub_port_new(NetHub *hub, const char *name)
+{
+    VLANClientState *nc;
+    NetHubPort *port;
+    unsigned int id = hub->num_ports++;
+    char default_name[128];
+
+    if (!name) {
+        snprintf(default_name, sizeof(default_name),
+                 "hub%uport%u", hub->id, id);
+        name = default_name;
+    }
+
+    nc = qemu_new_net_client(&net_hub_port_info, NULL, NULL, "hub", name);
+    port = DO_UPCAST(NetHubPort, nc, nc);
+    port->id = id;
+    port->hub = hub;
+
+    QLIST_INSERT_HEAD(&hub->ports, port, next);
+
+    return port;
+}
+
+/**
+ * Create a port on a given hub
+ * @name: Net client name or NULL for default name.
+ *
+ * If there is no existing hub with the given id then a new hub is created.
+ */
+VLANClientState *net_hub_add_port(unsigned int hub_id, const char *name)
+{
+    NetHub *hub;
+    NetHubPort *port;
+
+    QLIST_FOREACH(hub, &hubs, next) {
+        if (hub->id == hub_id) {
+            break;
+        }
+    }
+
+    if (!hub) {
+        hub = net_hub_new(hub_id);
+    }
+
+    port = net_hub_port_new(hub, name);
+    return &port->nc;
+}
+
+/**
+ * Print hub configuration
+ */
+void net_hub_info(Monitor *mon)
+{
+    NetHub *hub;
+    NetHubPort *port;
+
+    QLIST_FOREACH(hub, &hubs, next) {
+        monitor_printf(mon, "hub %u\n", hub->id);
+        QLIST_FOREACH(port, &hub->ports, next) {
+            monitor_printf(mon, "    port %u peer %s\n", port->id,
+                           port->nc.peer ? port->nc.peer->name : "<none>");
+        }
+    }
+}
+
+/**
+ * Get the hub id that a client is connected to
+ *
+ * @id              Pointer for hub id output, may be NULL
+ */
+int net_hub_id_for_client(VLANClientState *nc, unsigned int *id)
+{
+    NetHub *hub;
+    NetHubPort *port;
+
+    QLIST_FOREACH(hub, &hubs, next) {
+        QLIST_FOREACH(port, &hub->ports, next) {
+            if (&port->nc == nc ||
+                (port->nc.peer && port->nc.peer == nc)) {
+                if (id) {
+                    *id = hub->id;
+                }
+                return 0;
+            }
+        }
+    }
+    return -ENOENT;
+}
+
+int net_init_hubport(const NetClientOptions *opts, const char *name,
+                     VLANState *vlan)
+{
+    const NetdevHubPortOptions *hubport;
+
+    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_HUBPORT);
+    hubport = opts->hubport;
+
+    /* The hub is a "vlan" so this option makes no sense. */
+    if (vlan) {
+        return -EINVAL;
+    }
+
+    net_hub_add_port(hubport->hubid, name);
+    return 0;
+}
diff --git a/net/hub.h b/net/hub.h
new file mode 100644
index 0000000..06939b3
--- /dev/null
+++ b/net/hub.h
@@ -0,0 +1,26 @@ 
+/*
+ * Hub net client
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Stefan Hajnoczi   <stefanha@linux.vnet.ibm.com>
+ *  Zhi Yong Wu       <wuzhy@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef NET_HUB_H
+#define NET_HUB_H
+
+#include "qemu-common.h"
+
+int net_init_hubport(const NetClientOptions *opts, const char *name,
+                     VLANState *vlan);
+VLANClientState *net_hub_add_port(unsigned int hub_id, const char *name);
+void net_hub_info(Monitor *mon);
+int net_hub_id_for_client(VLANClientState *nc, unsigned int *id);
+
+#endif /* NET_HUB_H */
diff --git a/qapi-schema.json b/qapi-schema.json
index bc55ed2..6618eb5 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2094,6 +2094,19 @@ 
     '*helper': 'str' } }
 
 ##
+# @NetdevHubPortOptions
+#
+# Connect two or more net clients through a software hub.
+#
+# @hubid: hub identifier number
+#
+# Since 1.2
+##
+{ 'type': 'NetdevHubPortOptions',
+  'data': {
+    'hubid':     'int' } }
+
+##
 # @NetClientOptions
 #
 # A discriminated record of network device traits.
@@ -2102,14 +2115,15 @@ 
 ##
 { 'union': 'NetClientOptions',
   'data': {
-    'none':   'NetdevNoneOptions',
-    'nic':    'NetLegacyNicOptions',
-    'user':   'NetdevUserOptions',
-    'tap':    'NetdevTapOptions',
-    'socket': 'NetdevSocketOptions',
-    'vde':    'NetdevVdeOptions',
-    'dump':   'NetdevDumpOptions',
-    'bridge': 'NetdevBridgeOptions' } }
+    'none':     'NetdevNoneOptions',
+    'nic':      'NetLegacyNicOptions',
+    'user':     'NetdevUserOptions',
+    'tap':      'NetdevTapOptions',
+    'socket':   'NetdevSocketOptions',
+    'vde':      'NetdevVdeOptions',
+    'dump':     'NetdevDumpOptions',
+    'bridge':   'NetdevBridgeOptions',
+    'hubport':  'NetdevHubPortOptions' } }
 
 ##
 # @NetLegacy