[Qemu-devel] [PATCH] net: Allow hubports to connect to other netdevs

[Qemu-devel] [PATCH] net: Allow hubports to connect to other netdevs

[Thomas Huth]

QEMU can emulate hubs to connect NICs and netdevs. This is currently
primarily used for the mis-named 'vlan' feature of the networking
subsystem. Now the 'vlan' feature has been marked as deprecated, since
its name is rather confusing and the users often rather mis-configure
their network when trying to use it. But while the 'vlan' parameter
should be removed at one point in time, the basic idea of emulating
a hub in QEMU is still good: It's useful for bundling up the output of
multiple NICs into one single l2tp netdev for example.

Now to be able to use the hubport feature without 'vlan's, there is one
missing piece: The possibility to connect a hubport to a netdev, too.
This patch adds this possibility by introducing a new "netdev=..."
parameter to the hubports.

To bundle up the output of multiple NICs into one socket netdev, you can
now run QEMU with these parameters for example:

qemu-system-ppc64 ... -netdev socket,id=s1,connect=:11122 \
    -netdev hubport,hubid=1,id=h1,netdev=s1 \
    -netdev hubport,hubid=1,id=h2 -device e1000,netdev=h2 \
    -netdev hubport,hubid=1,id=h3 -device virtio-net-pci,netdev=h3

For using the socket netdev, you have got to start another QEMU as the
receiving side first, for example with network dumping enabled:

qemu-system-x86_64 -M isapc -netdev socket,id=s0,listen=:11122 \
    -device ne2k_isa,netdev=s0 \
    -object filter-dump,id=f1,netdev=s0,file=/tmp/dump.dat

After the ppc64 guest tried to boot from both NICs, you can see in the
dump file (using Wireshark, for example), that the output of both NICs
(the e1000 and the virtio-net-pci) has been successfully transfered
via the socket netdev in this case.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 See also the original discussion here for some more information:
 https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg05650.html

 net/hub.c       | 23 ++++++++++++++++++++++-
 qapi/net.json   |  4 +++-
 qemu-options.hx |  8 +++++---
 3 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/net/hub.c b/net/hub.c
index 14b4eec..0638729 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -13,6 +13,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/error.h"
 #include "monitor/monitor.h"
 #include "net/net.h"
 #include "clients.h"
@@ -286,12 +287,32 @@ int net_init_hubport(const Netdev *netdev, const char *name,
                      NetClientState *peer, Error **errp)
 {
     const NetdevHubPortOptions *hubport;
+    NetClientState *hubncs;
 
     assert(netdev->type == NET_CLIENT_DRIVER_HUBPORT);
     assert(!peer);
     hubport = &netdev->u.hubport;
 
-    net_hub_add_port(hubport->hubid, name);
+    hubncs = net_hub_add_port(hubport->hubid, name);
+    if (!hubncs) {
+        error_setg(errp, "failed to add port to hub %i with id '%s'",
+                   hubport->hubid, name);
+        return -1;
+    }
+
+    if (hubport->has_netdev) {
+        NetClientState *hubpeer;
+
+        hubpeer = qemu_find_netdev(hubport->netdev);
+        if (!hubpeer) {
+            error_setg(errp, "netdev '%s' not found", hubport->netdev);
+            return -1;
+        }
+        assert(!hubncs->peer && !hubpeer->peer);
+        hubncs->peer = hubpeer;
+        hubpeer->peer = hubncs;
+    }
+
     return 0;
 }
 
diff --git a/qapi/net.json b/qapi/net.json
index 4beff5d..e41e046 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -410,12 +410,14 @@
 # Connect two or more net clients through a software hub.
 #
 # @hubid: hub identifier number
+# @netdev: used to connect hub to a netdev instead of a device (since 2.12)
 #
 # Since: 1.2
 ##
 { 'struct': 'NetdevHubPortOptions',
   'data': {
-    'hubid':     'int32' } }
+    'hubid':     'int32',
+    '*netdev':    'str' } }
 
 ##
 # @NetdevNetmapOptions:
diff --git a/qemu-options.hx b/qemu-options.hx
index 678181c..9ec4af7 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2017,7 +2017,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
 #endif
     "-netdev vhost-user,id=str,chardev=dev[,vhostforce=on|off]\n"
     "                configure a vhost-user network, backed by a chardev 'dev'\n"
-    "-netdev hubport,id=str,hubid=n\n"
+    "-netdev hubport,id=str,hubid=n[,netdev=nd]\n"
     "                configure a hub port on QEMU VLAN 'n'\n", QEMU_ARCH_ALL)
 DEF("net", HAS_ARG, QEMU_OPTION_net,
     "-net nic[,vlan=n][,netdev=nd][,macaddr=mac][,model=type][,name=str][,addr=str][,vectors=v]\n"
@@ -2445,13 +2445,15 @@ vde_switch -F -sock /tmp/myswitch
 qemu-system-i386 linux.img -net nic -net vde,sock=/tmp/myswitch
 @end example
 
-@item -netdev hubport,id=@var{id},hubid=@var{hubid}
+@item -netdev hubport,id=@var{id},hubid=@var{hubid}[,netdev=@var{nd}]
 
 Create a hub port on QEMU "vlan" @var{hubid}.
 
 The hubport netdev lets you connect a NIC to a QEMU "vlan" instead of a single
 netdev.  @code{-net} and @code{-device} with parameter @option{vlan} create the
-required hub automatically.
+required hub automatically. Alternatively, you can also connect the hubport
+to another netdev with ID @var{nd} by using the @option{netdev=@var{nd}}
+option.
 
 @item -netdev vhost-user,chardev=@var{id}[,vhostforce=on|off][,queues=n]
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] net: Allow hubports to connect to other netdevs

[Paolo Bonzini]

On 10/01/2018 15:32, Thomas Huth wrote:
> QEMU can emulate hubs to connect NICs and netdevs. This is currently
> primarily used for the mis-named 'vlan' feature of the networking
> subsystem. Now the 'vlan' feature has been marked as deprecated, since
> its name is rather confusing and the users often rather mis-configure
> their network when trying to use it. But while the 'vlan' parameter
> should be removed at one point in time, the basic idea of emulating
> a hub in QEMU is still good: It's useful for bundling up the output of
> multiple NICs into one single l2tp netdev for example.
> 
> Now to be able to use the hubport feature without 'vlan's, there is one
> missing piece: The possibility to connect a hubport to a netdev, too.
> This patch adds this possibility by introducing a new "netdev=..."
> parameter to the hubports.
> 
> To bundle up the output of multiple NICs into one socket netdev, you can
> now run QEMU with these parameters for example:
> 
> qemu-system-ppc64 ... -netdev socket,id=s1,connect=:11122 \
>     -netdev hubport,hubid=1,id=h1,netdev=s1 \
>     -netdev hubport,hubid=1,id=h2 -device e1000,netdev=h2 \
>     -netdev hubport,hubid=1,id=h3 -device virtio-net-pci,netdev=h3
> 
> For using the socket netdev, you have got to start another QEMU as the
> receiving side first, for example with network dumping enabled:
> 
> qemu-system-x86_64 -M isapc -netdev socket,id=s0,listen=:11122 \
>     -device ne2k_isa,netdev=s0 \
>     -object filter-dump,id=f1,netdev=s0,file=/tmp/dump.dat
> 
> After the ppc64 guest tried to boot from both NICs, you can see in the
> dump file (using Wireshark, for example), that the output of both NICs
> (the e1000 and the virtio-net-pci) has been successfully transfered
> via the socket netdev in this case.
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  See also the original discussion here for some more information:
>  https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg05650.html
> 
>  net/hub.c       | 23 ++++++++++++++++++++++-
>  qapi/net.json   |  4 +++-
>  qemu-options.hx |  8 +++++---
>  3 files changed, 30 insertions(+), 5 deletions(-)
> 
> diff --git a/net/hub.c b/net/hub.c
> index 14b4eec..0638729 100644
> --- a/net/hub.c
> +++ b/net/hub.c
> @@ -13,6 +13,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "qapi/error.h"
>  #include "monitor/monitor.h"
>  #include "net/net.h"
>  #include "clients.h"
> @@ -286,12 +287,32 @@ int net_init_hubport(const Netdev *netdev, const char *name,
>                       NetClientState *peer, Error **errp)
>  {
>      const NetdevHubPortOptions *hubport;
> +    NetClientState *hubncs;
>  
>      assert(netdev->type == NET_CLIENT_DRIVER_HUBPORT);
>      assert(!peer);
>      hubport = &netdev->u.hubport;
>  
> -    net_hub_add_port(hubport->hubid, name);
> +    hubncs = net_hub_add_port(hubport->hubid, name);
> +    if (!hubncs) {
> +        error_setg(errp, "failed to add port to hub %i with id '%s'",
> +                   hubport->hubid, name);
> +        return -1;
> +    }
> +
> +    if (hubport->has_netdev) {
> +        NetClientState *hubpeer;
> +
> +        hubpeer = qemu_find_netdev(hubport->netdev);
> +        if (!hubpeer) {
> +            error_setg(errp, "netdev '%s' not found", hubport->netdev);
> +            return -1;
> +        }
> +        assert(!hubncs->peer && !hubpeer->peer);
> +        hubncs->peer = hubpeer;
> +        hubpeer->peer = hubncs;
> +    }
> +
>      return 0;
>  }
>  
> diff --git a/qapi/net.json b/qapi/net.json
> index 4beff5d..e41e046 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -410,12 +410,14 @@
>  # Connect two or more net clients through a software hub.
>  #
>  # @hubid: hub identifier number
> +# @netdev: used to connect hub to a netdev instead of a device (since 2.12)
>  #
>  # Since: 1.2
>  ##
>  { 'struct': 'NetdevHubPortOptions',
>    'data': {
> -    'hubid':     'int32' } }
> +    'hubid':     'int32',
> +    '*netdev':    'str' } }
>  
>  ##
>  # @NetdevNetmapOptions:
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 678181c..9ec4af7 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2017,7 +2017,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
>  #endif
>      "-netdev vhost-user,id=str,chardev=dev[,vhostforce=on|off]\n"
>      "                configure a vhost-user network, backed by a chardev 'dev'\n"
> -    "-netdev hubport,id=str,hubid=n\n"
> +    "-netdev hubport,id=str,hubid=n[,netdev=nd]\n"
>      "                configure a hub port on QEMU VLAN 'n'\n", QEMU_ARCH_ALL)
>  DEF("net", HAS_ARG, QEMU_OPTION_net,
>      "-net nic[,vlan=n][,netdev=nd][,macaddr=mac][,model=type][,name=str][,addr=str][,vectors=v]\n"
> @@ -2445,13 +2445,15 @@ vde_switch -F -sock /tmp/myswitch
>  qemu-system-i386 linux.img -net nic -net vde,sock=/tmp/myswitch
>  @end example
>  
> -@item -netdev hubport,id=@var{id},hubid=@var{hubid}
> +@item -netdev hubport,id=@var{id},hubid=@var{hubid}[,netdev=@var{nd}]
>  
>  Create a hub port on QEMU "vlan" @var{hubid}.
>  
>  The hubport netdev lets you connect a NIC to a QEMU "vlan" instead of a single
>  netdev.  @code{-net} and @code{-device} with parameter @option{vlan} create the
> -required hub automatically.
> +required hub automatically. Alternatively, you can also connect the hubport
> +to another netdev with ID @var{nd} by using the @option{netdev=@var{nd}}
> +option.
>  
>  @item -netdev vhost-user,chardev=@var{id}[,vhostforce=on|off][,queues=n]
>  
> 

Thank you very much!

What is the next step for deprecating vlans?  I guess

1) removing the vlan property from DEFINE_NIC_PROPERTIES

2) removing the vlan property from -net, defaulting to vlan 0

3) only allow exactly two -net options, one for the backend (creating a
netdev) and one from the front-end (filling in nd_table)

?

Thanks,

Paolo

Re: [Qemu-devel] [PATCH] net: Allow hubports to connect to other netdevs

[Thomas Huth]

On 10.01.2018 15:47, Paolo Bonzini wrote:
> On 10/01/2018 15:32, Thomas Huth wrote:
>> QEMU can emulate hubs to connect NICs and netdevs. This is currently
>> primarily used for the mis-named 'vlan' feature of the networking
>> subsystem. Now the 'vlan' feature has been marked as deprecated, since
>> its name is rather confusing and the users often rather mis-configure
>> their network when trying to use it. But while the 'vlan' parameter
>> should be removed at one point in time, the basic idea of emulating
>> a hub in QEMU is still good: It's useful for bundling up the output of
>> multiple NICs into one single l2tp netdev for example.
>>
>> Now to be able to use the hubport feature without 'vlan's, there is one
>> missing piece: The possibility to connect a hubport to a netdev, too.
>> This patch adds this possibility by introducing a new "netdev=..."
>> parameter to the hubports.
>>
>> To bundle up the output of multiple NICs into one socket netdev, you can
>> now run QEMU with these parameters for example:
>>
>> qemu-system-ppc64 ... -netdev socket,id=s1,connect=:11122 \
>>     -netdev hubport,hubid=1,id=h1,netdev=s1 \
>>     -netdev hubport,hubid=1,id=h2 -device e1000,netdev=h2 \
>>     -netdev hubport,hubid=1,id=h3 -device virtio-net-pci,netdev=h3
>>
>> For using the socket netdev, you have got to start another QEMU as the
>> receiving side first, for example with network dumping enabled:
>>
>> qemu-system-x86_64 -M isapc -netdev socket,id=s0,listen=:11122 \
>>     -device ne2k_isa,netdev=s0 \
>>     -object filter-dump,id=f1,netdev=s0,file=/tmp/dump.dat
>>
>> After the ppc64 guest tried to boot from both NICs, you can see in the
>> dump file (using Wireshark, for example), that the output of both NICs
>> (the e1000 and the virtio-net-pci) has been successfully transfered
>> via the socket netdev in this case.
>>
>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  See also the original discussion here for some more information:
>>  https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg05650.html
>>
>>  net/hub.c       | 23 ++++++++++++++++++++++-
>>  qapi/net.json   |  4 +++-
>>  qemu-options.hx |  8 +++++---
>>  3 files changed, 30 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/hub.c b/net/hub.c
>> index 14b4eec..0638729 100644
>> --- a/net/hub.c
>> +++ b/net/hub.c
>> @@ -13,6 +13,7 @@
>>   */
>>  
>>  #include "qemu/osdep.h"
>> +#include "qapi/error.h"
>>  #include "monitor/monitor.h"
>>  #include "net/net.h"
>>  #include "clients.h"
>> @@ -286,12 +287,32 @@ int net_init_hubport(const Netdev *netdev, const char *name,
>>                       NetClientState *peer, Error **errp)
>>  {
>>      const NetdevHubPortOptions *hubport;
>> +    NetClientState *hubncs;
>>  
>>      assert(netdev->type == NET_CLIENT_DRIVER_HUBPORT);
>>      assert(!peer);
>>      hubport = &netdev->u.hubport;
>>  
>> -    net_hub_add_port(hubport->hubid, name);
>> +    hubncs = net_hub_add_port(hubport->hubid, name);
>> +    if (!hubncs) {
>> +        error_setg(errp, "failed to add port to hub %i with id '%s'",
>> +                   hubport->hubid, name);
>> +        return -1;
>> +    }
>> +
>> +    if (hubport->has_netdev) {
>> +        NetClientState *hubpeer;
>> +
>> +        hubpeer = qemu_find_netdev(hubport->netdev);
>> +        if (!hubpeer) {
>> +            error_setg(errp, "netdev '%s' not found", hubport->netdev);
>> +            return -1;
>> +        }
>> +        assert(!hubncs->peer && !hubpeer->peer);
>> +        hubncs->peer = hubpeer;
>> +        hubpeer->peer = hubncs;
>> +    }
>> +
>>      return 0;
>>  }
>>  
>> diff --git a/qapi/net.json b/qapi/net.json
>> index 4beff5d..e41e046 100644
>> --- a/qapi/net.json
>> +++ b/qapi/net.json
>> @@ -410,12 +410,14 @@
>>  # Connect two or more net clients through a software hub.
>>  #
>>  # @hubid: hub identifier number
>> +# @netdev: used to connect hub to a netdev instead of a device (since 2.12)
>>  #
>>  # Since: 1.2
>>  ##
>>  { 'struct': 'NetdevHubPortOptions',
>>    'data': {
>> -    'hubid':     'int32' } }
>> +    'hubid':     'int32',
>> +    '*netdev':    'str' } }
>>  
>>  ##
>>  # @NetdevNetmapOptions:
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 678181c..9ec4af7 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -2017,7 +2017,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
>>  #endif
>>      "-netdev vhost-user,id=str,chardev=dev[,vhostforce=on|off]\n"
>>      "                configure a vhost-user network, backed by a chardev 'dev'\n"
>> -    "-netdev hubport,id=str,hubid=n\n"
>> +    "-netdev hubport,id=str,hubid=n[,netdev=nd]\n"
>>      "                configure a hub port on QEMU VLAN 'n'\n", QEMU_ARCH_ALL)
>>  DEF("net", HAS_ARG, QEMU_OPTION_net,
>>      "-net nic[,vlan=n][,netdev=nd][,macaddr=mac][,model=type][,name=str][,addr=str][,vectors=v]\n"
>> @@ -2445,13 +2445,15 @@ vde_switch -F -sock /tmp/myswitch
>>  qemu-system-i386 linux.img -net nic -net vde,sock=/tmp/myswitch
>>  @end example
>>  
>> -@item -netdev hubport,id=@var{id},hubid=@var{hubid}
>> +@item -netdev hubport,id=@var{id},hubid=@var{hubid}[,netdev=@var{nd}]
>>  
>>  Create a hub port on QEMU "vlan" @var{hubid}.
>>  
>>  The hubport netdev lets you connect a NIC to a QEMU "vlan" instead of a single
>>  netdev.  @code{-net} and @code{-device} with parameter @option{vlan} create the
>> -required hub automatically.
>> +required hub automatically. Alternatively, you can also connect the hubport
>> +to another netdev with ID @var{nd} by using the @option{netdev=@var{nd}}
>> +option.
>>  
>>  @item -netdev vhost-user,chardev=@var{id}[,vhostforce=on|off][,queues=n]
> 
> Thank you very much!
> 
> What is the next step for deprecating vlans?  I guess
> 
> 1) removing the vlan property from DEFINE_NIC_PROPERTIES
> 
> 2) removing the vlan property from -net, defaulting to vlan 0

Yes.

> 3) only allow exactly two -net options, one for the backend (creating a
> netdev) and one from the front-end (filling in nd_table)

I see multiple options here:

a) Only allow one pair of -net options. That's likely bad if there is a
board with more than one embedded NIC, since "-net" is the only way to
configure them right now.

b) Keep the current way (just without the 'vlan' parameter), so that all
-net front- and back-ends are always connected to "vlan 0". We'd stay as
compatible as possible this way, but actually this is also the way that
regularly confuses the users - they do not expect that everything ends
up on one hub!

c) Introduce a slightly new behaviour, so that if you specify -net
nic,id=a1 -net user,id=a2 -net nic,id=b1 -net user,id=b2 then QEMU
connects a1 with a2, and b1 with b2 directly, without a hub in between.
That's likely the behaviour that most users expect, but it's also a
slight change in behavior compared to the previous QEMU versions.

d) Finally get rid of the ugly "-net" parameter (or maybe rather limit
it to option a) and introduce a new "-n" convenience parameter instead
which can be used to configure the default/embedded NICs. With that new
convenience parameter, you don't have to specify it twice anymore like
in "-net nic -net tap", but you could simply specify "-n tap" and QEMU
then configures a tap interface for the default NIC of the current
machine for you. (I'd drop the possibility to change the NIC model here,
but in case it is still required, I'd go for something like "-n
tap,model=e1000" or so)

Opinions?

 Thomas

Re: [Qemu-devel] [PATCH] net: Allow hubports to connect to other netdevs

[Jason Wang]

On 2018年01月10日 22:32, Thomas Huth wrote:
> QEMU can emulate hubs to connect NICs and netdevs. This is currently
> primarily used for the mis-named 'vlan' feature of the networking
> subsystem. Now the 'vlan' feature has been marked as deprecated, since
> its name is rather confusing and the users often rather mis-configure
> their network when trying to use it. But while the 'vlan' parameter
> should be removed at one point in time, the basic idea of emulating
> a hub in QEMU is still good: It's useful for bundling up the output of
> multiple NICs into one single l2tp netdev for example.
>
> Now to be able to use the hubport feature without 'vlan's, there is one
> missing piece: The possibility to connect a hubport to a netdev, too.
> This patch adds this possibility by introducing a new "netdev=..."
> parameter to the hubports.
>
> To bundle up the output of multiple NICs into one socket netdev, you can
> now run QEMU with these parameters for example:
>
> qemu-system-ppc64 ... -netdev socket,id=s1,connect=:11122 \
>      -netdev hubport,hubid=1,id=h1,netdev=s1 \
>      -netdev hubport,hubid=1,id=h2 -device e1000,netdev=h2 \
>      -netdev hubport,hubid=1,id=h3 -device virtio-net-pci,netdev=h3
>
> For using the socket netdev, you have got to start another QEMU as the
> receiving side first, for example with network dumping enabled:
>
> qemu-system-x86_64 -M isapc -netdev socket,id=s0,listen=:11122 \
>      -device ne2k_isa,netdev=s0 \
>      -object filter-dump,id=f1,netdev=s0,file=/tmp/dump.dat
>
> After the ppc64 guest tried to boot from both NICs, you can see in the
> dump file (using Wireshark, for example), that the output of both NICs
> (the e1000 and the virtio-net-pci) has been successfully transfered
> via the socket netdev in this case.
>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   See also the original discussion here for some more information:
>   https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg05650.html
>
>   net/hub.c       | 23 ++++++++++++++++++++++-
>   qapi/net.json   |  4 +++-
>   qemu-options.hx |  8 +++++---
>   3 files changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/net/hub.c b/net/hub.c
> index 14b4eec..0638729 100644
> --- a/net/hub.c
> +++ b/net/hub.c
> @@ -13,6 +13,7 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qapi/error.h"
>   #include "monitor/monitor.h"
>   #include "net/net.h"
>   #include "clients.h"
> @@ -286,12 +287,32 @@ int net_init_hubport(const Netdev *netdev, const char *name,
>                        NetClientState *peer, Error **errp)
>   {
>       const NetdevHubPortOptions *hubport;
> +    NetClientState *hubncs;
>   
>       assert(netdev->type == NET_CLIENT_DRIVER_HUBPORT);
>       assert(!peer);
>       hubport = &netdev->u.hubport;
>   
> -    net_hub_add_port(hubport->hubid, name);
> +    hubncs = net_hub_add_port(hubport->hubid, name);
> +    if (!hubncs) {
> +        error_setg(errp, "failed to add port to hub %i with id '%s'",
> +                   hubport->hubid, name);
> +        return -1;
> +    }
> +
> +    if (hubport->has_netdev) {
> +        NetClientState *hubpeer;
> +
> +        hubpeer = qemu_find_netdev(hubport->netdev);
> +        if (!hubpeer) {
> +            error_setg(errp, "netdev '%s' not found", hubport->netdev);
> +            return -1;
> +        }
> +        assert(!hubncs->peer && !hubpeer->peer);
> +        hubncs->peer = hubpeer;
> +        hubpeer->peer = hubncs;
> +    }
> +

Instead of open coding here, maybe you can pass peer to 
net_hub_port_new() and let qemu_new_net_client() do this for you.

And since it was a hub, do we need to send to its netdev too inside 
net_hub_receive()?

Thanks

>       return 0;
>   }
>   
> diff --git a/qapi/net.json b/qapi/net.json
> index 4beff5d..e41e046 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -410,12 +410,14 @@
>   # Connect two or more net clients through a software hub.
>   #
>   # @hubid: hub identifier number
> +# @netdev: used to connect hub to a netdev instead of a device (since 2.12)
>   #
>   # Since: 1.2
>   ##
>   { 'struct': 'NetdevHubPortOptions',
>     'data': {
> -    'hubid':     'int32' } }
> +    'hubid':     'int32',
> +    '*netdev':    'str' } }
>   
>   ##
>   # @NetdevNetmapOptions:
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 678181c..9ec4af7 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2017,7 +2017,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
>   #endif
>       "-netdev vhost-user,id=str,chardev=dev[,vhostforce=on|off]\n"
>       "                configure a vhost-user network, backed by a chardev 'dev'\n"
> -    "-netdev hubport,id=str,hubid=n\n"
> +    "-netdev hubport,id=str,hubid=n[,netdev=nd]\n"
>       "                configure a hub port on QEMU VLAN 'n'\n", QEMU_ARCH_ALL)
>   DEF("net", HAS_ARG, QEMU_OPTION_net,
>       "-net nic[,vlan=n][,netdev=nd][,macaddr=mac][,model=type][,name=str][,addr=str][,vectors=v]\n"
> @@ -2445,13 +2445,15 @@ vde_switch -F -sock /tmp/myswitch
>   qemu-system-i386 linux.img -net nic -net vde,sock=/tmp/myswitch
>   @end example
>   
> -@item -netdev hubport,id=@var{id},hubid=@var{hubid}
> +@item -netdev hubport,id=@var{id},hubid=@var{hubid}[,netdev=@var{nd}]
>   
>   Create a hub port on QEMU "vlan" @var{hubid}.
>   
>   The hubport netdev lets you connect a NIC to a QEMU "vlan" instead of a single
>   netdev.  @code{-net} and @code{-device} with parameter @option{vlan} create the
> -required hub automatically.
> +required hub automatically. Alternatively, you can also connect the hubport
> +to another netdev with ID @var{nd} by using the @option{netdev=@var{nd}}
> +option.
>   
>   @item -netdev vhost-user,chardev=@var{id}[,vhostforce=on|off][,queues=n]
>   

Re: [Qemu-devel] [PATCH] net: Allow hubports to connect to other netdevs

[Thomas Huth]

On 15.01.2018 08:40, Jason Wang wrote:
> 
> On 2018年01月10日 22:32, Thomas Huth wrote:
>> QEMU can emulate hubs to connect NICs and netdevs. This is currently
>> primarily used for the mis-named 'vlan' feature of the networking
>> subsystem. Now the 'vlan' feature has been marked as deprecated, since
>> its name is rather confusing and the users often rather mis-configure
>> their network when trying to use it. But while the 'vlan' parameter
>> should be removed at one point in time, the basic idea of emulating
>> a hub in QEMU is still good: It's useful for bundling up the output of
>> multiple NICs into one single l2tp netdev for example.
>>
>> Now to be able to use the hubport feature without 'vlan's, there is one
>> missing piece: The possibility to connect a hubport to a netdev, too.
>> This patch adds this possibility by introducing a new "netdev=..."
>> parameter to the hubports.
[...]
>> @@ -286,12 +287,32 @@ int net_init_hubport(const Netdev *netdev, const
>> char *name,
>>                        NetClientState *peer, Error **errp)
>>   {
>>       const NetdevHubPortOptions *hubport;
>> +    NetClientState *hubncs;
>>         assert(netdev->type == NET_CLIENT_DRIVER_HUBPORT);
>>       assert(!peer);
>>       hubport = &netdev->u.hubport;
>>   -    net_hub_add_port(hubport->hubid, name);
>> +    hubncs = net_hub_add_port(hubport->hubid, name);
>> +    if (!hubncs) {
>> +        error_setg(errp, "failed to add port to hub %i with id '%s'",
>> +                   hubport->hubid, name);
>> +        return -1;
>> +    }
>> +
>> +    if (hubport->has_netdev) {
>> +        NetClientState *hubpeer;
>> +
>> +        hubpeer = qemu_find_netdev(hubport->netdev);
>> +        if (!hubpeer) {
>> +            error_setg(errp, "netdev '%s' not found", hubport->netdev);
>> +            return -1;
>> +        }
>> +        assert(!hubncs->peer && !hubpeer->peer);
>> +        hubncs->peer = hubpeer;
>> +        hubpeer->peer = hubncs;
>> +    }
>> +
> 
> Instead of open coding here, maybe you can pass peer to
> net_hub_port_new() and let qemu_new_net_client() do this for you.

Sure. I'll send a v2.

> And since it was a hub, do we need to send to its netdev too inside
> net_hub_receive()?

I currently don't think so, but I'll check again...

 Thomas

Re: [Qemu-devel] [PATCH] net: Allow hubports to connect to other netdevs

[Thomas Huth]

On 15.01.2018 18:36, Thomas Huth wrote:
> On 15.01.2018 08:40, Jason Wang wrote:
[...]
>> And since it was a hub, do we need to send to its netdev too inside
>> net_hub_receive()?
> 
> I currently don't think so, but I'll check again...

OK, I now think we're definitely fine here. The check is really just
there to make sure that we do not send the packet back to the same
sender. And I've checked with a command line like this that network
traffic (TFTP booting in this case) works as expected:

qemu-system-ppc64 -vga none -nographic \
 -netdev user,id=s1,tftp=/path/to/tftpdir,bootfile=ppc64.img \
 -netdev hubport,hubid=1,id=h1,netdev=s1  \
 -netdev hubport,hubid=1,id=h3 -device virtio-net-pci,netdev=h3 \
 -boot n -object filter-dump,id=f1,netdev=s1,file=/tmp/dump.dat

Both, the behaviour of the TFTP boot in the guest and the wireshark dump
looked fine, there were no missing packets here.

 Thomas

Re: [Qemu-devel] [PATCH] net: Allow hubports to connect to other netdevs

[Jason Wang]

On 2018年01月16日 02:06, Thomas Huth wrote:
> On 15.01.2018 18:36, Thomas Huth wrote:
>> On 15.01.2018 08:40, Jason Wang wrote:
> [...]
>>> And since it was a hub, do we need to send to its netdev too inside
>>> net_hub_receive()?
>> I currently don't think so, but I'll check again...
> OK, I now think we're definitely fine here. The check is really just
> there to make sure that we do not send the packet back to the same
> sender. And I've checked with a command line like this that network
> traffic (TFTP booting in this case) works as expected:
>
> qemu-system-ppc64 -vga none -nographic \
>   -netdev user,id=s1,tftp=/path/to/tftpdir,bootfile=ppc64.img \
>   -netdev hubport,hubid=1,id=h1,netdev=s1  \
>   -netdev hubport,hubid=1,id=h3 -device virtio-net-pci,netdev=h3 \
>   -boot n -object filter-dump,id=f1,netdev=s1,file=/tmp/dump.dat
>
> Both, the behaviour of the TFTP boot in the guest and the wireshark dump
> looked fine, there were no missing packets here.
>
>   Thomas

Right, I think I misread the command parameters.

Thanks