configure: add an option to disable vlans
PATCH 1/1 configure: add an option to disable vlans
With -netdev, there now seems to be little need to support vlans, enabling them leads to user confusion and bad performance. Disable support for vlans by default, add config option to enable.
--- configure | 12 ++++++++++++ net.c | 7 +++++++ 2 files changed, 19 insertions(+), 0 deletions(-)
configure +12 -0
@@ -299,6 +299,7 @@ pkgversion="" check_utests="no" user_pie="no" zero_malloc="" +vlans="no" # OS specific if check_define __linux__ ; then
@@ -660,6 +661,10 @@ for opt do ;; --enable-vhost-net) vhost_net="yes" ;; + --disable-vlans) vlans="no" + ;; + --enable-vlans) vlans="yes" + ;; *) echo "ERROR: unknown option $opt"; show_help="yes" ;; esac
@@ -826,6 +831,8 @@ echo " --enable-docs enable documentation build" echo " --disable-docs disable documentation build" echo " --disable-vhost-net disable vhost-net acceleration support" echo " --enable-vhost-net enable vhost-net acceleration support" +echo " --disable-vlans disable legacy qemu vlan support" +echo " --enable-vlans enable legacy qemu vlan support" echo "" echo "NOTE: The object files are built at the place where configure is launched" exit 1
@@ -2041,6 +2048,7 @@ echo "preadv support $preadv" echo "fdatasync $fdatasync" echo "uuid support $uuid" echo "vhost-net support $vhost_net" +echo "vlans support $vlans" if test $sdl_too_old = "yes"; then echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -2284,6 +2292,10 @@ bsd) ;; esac +if test $vlans = "yes" ; then + echo "CONFIG_VLANS=y" >> $config_target_mak +fi + tools= if test `expr "$target_list" : ".*softmmu.*"` != 0 ; then tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) $tools"
net.c +7 -0
@@ -1070,6 +1070,13 @@ int net_client_init(Monitor *mon, QemuOpts *opts, int is_netdev)return -1 ; } +#ifndef CONFIG_VLANS +if (!is_netdev) { +qerror_report (QERR_INVALID_PARAMETER_VALUE ,"netdev" ,"netdev id" ); +return -1 ; + } +#endif +if (is_netdev) { if (strcmp(type, "tap") != 0 &&#ifdef CONFIG_SLIRP
Signature · 1 line
1.7.1.12.g42b7f
Quoted 3 lines
With -netdev, there now seems to be little need to support vlans, enabling them leads to user confusion and bad performance. Disable support for vlans by default, add config option to enable.
No. If you want to remove vlans, then actually do that.
As I've said before if you want a point-point network model then you should implement that (and remove the vlan code, probably replacing with equivalent functionality). We should not have both point-point and broadcast interfaces.
Paul
Quoted 5 lines · Paul Brook
Quoted 3 lines
With -netdev, there now seems to be little need to support vlans, enabling them leads to user confusion and bad performance. Disable support for vlans by default, add config option to enable.
No. If you want to remove vlans, then actually do that.
How is this not what this patch does? You mean kill the code completely, not just --contigure option?
Quoted 5 lines
As I've said before if you want a point-point network model then you should implement that (and remove the vlan code, probably replacing with equivalent functionality). We should not have both point-point and broadcast interfaces.
Paul
This is what netdev does: replaces vlan. What is left is remove vlan.
Signature · 1 line
MST
Quoted 9 lines · Paul Brook
Quoted 5 lines
Quoted 3 lines
With -netdev, there now seems to be little need to support vlans, enabling them leads to user confusion and bad performance. Disable support for vlans by default, add config option to enable.
No. If you want to remove vlans, then actually do that.
How is this not what this patch does? You mean kill the code completely, not just --contigure option?
Yes. Configure options are bad. If code isn't worth enabling by default then you've got to have a very good reason why it exists at all.
Paul
Quoted 6 lines · Paul Brook
Quoted 6 lines · Paul Brook
Quoted 5 lines
Quoted 5 lines
With -netdev, there now seems to be little need to support vlans, enabling them leads to user confusion and bad performance. Disable support for vlans by default, add config option to enable.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
No. If you want to remove vlans, then actually do that.
How is this not what this patch does? You mean kill the code completely, not just --contigure option?
Yes. Configure options are bad. If code isn't worth enabling by default then you've got to have a very good reason why it exists at all.
Configure options are bad except when they are good.
Distributions don't want to support every possible bell and whistle that qemu supports. By having configuration options upstream, we ensure that everyone is consistently disabling thing in the same fashion and that the interfaces presented to the users are consistent.
I certainly believe that we should not disable features by default. But I think it's important that we support disabling features from a downstream supportability perspective.
Regards,
Anthony Liguori
Quoted 3 lines
Paul
Quoted 23 lines · Anthony Liguori
Quoted 6 lines · Paul Brook
Quoted 6 lines · Paul Brook
Quoted 5 lines
Quoted 5 lines
With -netdev, there now seems to be little need to support vlans, enabling them leads to user confusion and bad performance. Disable support for vlans by default, add config option to enable.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
No. If you want to remove vlans, then actually do that.
How is this not what this patch does? You mean kill the code completely, not just --contigure option?
Yes. Configure options are bad. If code isn't worth enabling by default then you've got to have a very good reason why it exists at all.
Configure options are bad except when they are good.
Distributions don't want to support every possible bell and whistle that qemu supports. By having configuration options upstream, we ensure that everyone is consistently disabling thing in the same fashion and that the interfaces presented to the users are consistent.
Quoted 7 lines
I certainly believe that we should not disable features by default. But I think it's important that we support disabling features from a downstream supportability perspective.
Regards,
Anthony Liguori
So I see two ways to go forward: switch default value in my patch, or disable vlans unconditionally.
Which will it be?
Quoted 3 lines
Quoted 3 lines
Paul
Quoted 4 lines · Michael S. Tsirkin
So I see two ways to go forward: switch default value in my patch, or disable vlans unconditionally.
The problem with disabling vlans unconditionally is that you break -net socket and -net dump.
If we can come up with an alternative way to do these things, I'm all for removing it.
Regards,
Anthony Liguori
Quoted 7 lines
Which will it be?
Quoted 4 lines
Quoted 4 lines
Paul
Quoted 11 lines · Anthony Liguori
Quoted 4 lines · Michael S. Tsirkin
So I see two ways to go forward: switch default value in my patch, or disable vlans unconditionally.
The problem with disabling vlans unconditionally is that you break -net socket and -net dump.
If we can come up with an alternative way to do these things, I'm all for removing it.
Hmm, I'll try to look at supporting -net socket in netdev. Does -net dump do anything that can't be done with tap+tcpdump?
Quoted 11 lines
Regards,
Anthony Liguori
Quoted 7 lines
Which will it be?
Quoted 4 lines
Quoted 4 lines
Paul
Quoted 8 lines · Michael S. Tsirkin
Quoted 6 lines · Anthony Liguori
Quoted 4 lines · Michael S. Tsirkin
So I see two ways to go forward: switch default value in my patch, or disable vlans unconditionally.
Quoted 9 lines
Quoted 6 lines
The problem with disabling vlans unconditionally is that you break -net socket and -net dump.
If we can come up with an alternative way to do these things, I'm all for removing it.
Hmm, I'll try to look at supporting -net socket in netdev. Does -net dump do anything that can't be done with tap+tcpdump?
tap+tcpdump requires root privileges (even if you have a tap helper).
Plus tcpdump doesn't help with slirp and -net dump is very useful for debugging slirp.
Regards,
Anthony Liguroi
Quoted 14 lines
Quoted 14 lines
Regards,
Anthony Liguori
Quoted 9 lines
Which will it be?
Quoted 5 lines
Quoted 5 lines
Paul
Quoted 9 lines · Anthony Liguori
Quoted 8 lines · Michael S. Tsirkin
Quoted 7 lines · Anthony Liguori
Quoted 3 lines · Michael S. Tsirkin
So I see two ways to go forward: switch default value in my patch, or disable vlans unconditionally.
The problem with disabling vlans unconditionally is that you break -net socket and -net dump.
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
If we can come up with an alternative way to do these things, I'm all for removing it.
Hmm, I'll try to look at supporting -net socket in netdev. Does -net dump do anything that can't be done with tap+tcpdump?
tap+tcpdump requires root privileges (even if you have a tap helper).
Plus tcpdump doesn't help with slirp and -net dump is very useful for debugging slirp.
Of course, you could add this functionality to netdev. It's arguably better there too because then you can debug virtio-net+tap with full offload enabled (which you cannot do today).
Regards,
Anthony Liguori
Quoted 12 lines · Anthony Liguori
Quoted 11 lines · Anthony Liguori
Quoted 10 lines · Michael S. Tsirkin
Quoted 9 lines · Anthony Liguori
Quoted 3 lines · Michael S. Tsirkin
So I see two ways to go forward: switch default value in my patch, or disable vlans unconditionally.
The problem with disabling vlans unconditionally is that you break -net socket and -net dump.
If we can come up with an alternative way to do these things, I'm all for removing it.
Quoted 7 lines
Quoted 7 lines
Quoted 2 lines
Hmm, I'll try to look at supporting -net socket in netdev. Does -net dump do anything that can't be done with tap+tcpdump?
tap+tcpdump requires root privileges (even if you have a tap helper).
Plus tcpdump doesn't help with slirp and -net dump is very useful for debugging slirp.
Developer's need for root access for debugging seems a reasonable price to pay to prevent user confusion and complexity that we have now.
Quoted 8 lines
Of course, you could add this functionality to netdev. It's arguably better there too because then you can debug virtio-net+tap with full offload enabled (which you cannot do today).
Regards,
Anthony Liguori
Care taking on it? I never even heard about -net dump before today.
Signature · 1 line
MST
Quoted 23 lines · Michael S. Tsirkin
Quoted 21 lines · Anthony Liguori
Quoted 19 lines · Anthony Liguori
Quoted 17 lines · Michael S. Tsirkin
Quoted 6 lines · Anthony Liguori
Quoted 4 lines · Michael S. Tsirkin
So I see two ways to go forward: switch default value in my patch, or disable vlans unconditionally.
Quoted 6 lines
The problem with disabling vlans unconditionally is that you break -net socket and -net dump.
If we can come up with an alternative way to do these things, I'm all for removing it.
Hmm, I'll try to look at supporting -net socket in netdev. Does -net dump do anything that can't be done with tap+tcpdump?
Quoted 8 lines
Quoted 5 lines
Quoted 5 lines
tap+tcpdump requires root privileges (even if you have a tap helper).
Plus tcpdump doesn't help with slirp and -net dump is very useful for debugging slirp.
Developer's need for root access for debugging seems a reasonable price to pay to prevent user confusion and complexity that we have now.
Removing vlans has the potential to break existing deployments. That's something we need to do very cautiously.
What do we gain from removing vlan support today?
Regards,
Anthony Liguori
Quoted 11 lines
Quoted 8 lines
Of course, you could add this functionality to netdev. It's arguably better there too because then you can debug virtio-net+tap with full offload enabled (which you cannot do today).
Regards,
Anthony Liguori
Care taking on it? I never even heard about -net dump before today.
Quoted 29 lines · Anthony Liguori
Quoted 28 lines · Michael S. Tsirkin
Quoted 26 lines · Anthony Liguori
Quoted 24 lines · Anthony Liguori
Quoted 8 lines · Michael S. Tsirkin
Quoted 6 lines · Anthony Liguori
Quoted 4 lines · Michael S. Tsirkin
So I see two ways to go forward: switch default value in my patch, or disable vlans unconditionally.
Quoted 9 lines
Quoted 6 lines
The problem with disabling vlans unconditionally is that you break -net socket and -net dump.
If we can come up with an alternative way to do these things, I'm all for removing it.
Hmm, I'll try to look at supporting -net socket in netdev. Does -net dump do anything that can't be done with tap+tcpdump?
tap+tcpdump requires root privileges (even if you have a tap helper).
Plus tcpdump doesn't help with slirp and -net dump is very useful for debugging slirp.
Quoted 6 lines
Quoted 3 lines
Developer's need for root access for debugging seems a reasonable price to pay to prevent user confusion and complexity that we have now.
Removing vlans has the potential to break existing deployments. That's something we need to do very cautiously.
You mean you want to support both peer to peer and broadcast indefinitely?
Quoted 5 lines
What do we gain from removing vlan support today?
Regards,
Anthony Liguori
This would address the following issues:
- people configure vlans and get bad performance
- people run --help and see info on vlans instead of peer
- migration issues betwen vlan/non-vlan
- hotplug is broken for vlan nics
Quoted 11 lines
Quoted 11 lines
Quoted 8 lines
Of course, you could add this functionality to netdev. It's arguably better there too because then you can debug virtio-net+tap with full offload enabled (which you cannot do today).
Regards,
Anthony Liguori
Care taking on it? I never even heard about -net dump before today.
Quoted 8 lines · Anthony Liguori
Quoted 4 lines · Michael S. Tsirkin
So I see two ways to go forward: switch default value in my patch, or disable vlans unconditionally.
The problem with disabling vlans unconditionally is that you break -net socket and -net dump.
-netdev socket seems to be supported. No?
Quoted 14 lines
If we can come up with an alternative way to do these things, I'm all for removing it.
Regards,
Anthony Liguori
Quoted 7 lines
Which will it be?
Quoted 4 lines
Quoted 4 lines
Paul
Quoted 4 lines · Michael S. Tsirkin
Quoted 2 lines · Anthony Liguori
On 06/07/2010 02:21 PM, Michael S. Tsirkin wrote:
Quoted 9 lines
Quoted 7 lines
Quoted 4 lines
So I see two ways to go forward: switch default value in my patch, or disable vlans unconditionally.
The problem with disabling vlans unconditionally is that you break -net socket and -net dump.
-netdev socket seems to be supported. No?
Sure, but it's of limited utility in the absence of vlans. A typical thing to do with -net socket would be to launch one instance of qemu with -net user, -net socket, and -net nic. Another qemu would be launched with -net socket and -net nic connected to the previous instance. Now you've got a working virtual network with external access.
-netdev socket alone won't get you this.
Regards,
Anthony Liguori
Quoted 12 lines · Anthony Liguori
Quoted 11 lines · Michael S. Tsirkin
Quoted 9 lines · Anthony Liguori
Quoted 4 lines · Michael S. Tsirkin
So I see two ways to go forward: switch default value in my patch, or disable vlans unconditionally.
The problem with disabling vlans unconditionally is that you break -net socket and -net dump.
Quoted 8 lines
Quoted 2 lines
-netdev socket seems to be supported. No?
Sure, but it's of limited utility in the absence of vlans. A typical thing to do with -net socket would be to launch one instance of qemu with -net user, -net socket, and -net nic. Another qemu would be launched with -net socket and -net nic connected to the previous instance. Now you've got a working virtual network with external access.
Only terribly slow, esp when tunneling tcp over tcp.
Quoted 1 line
-netdev socket alone won't get you this.
-socket should really support udp. Then you could get what you wanted with halfway decent speed using multicast.
Quoted 3 lines
Regards,
Anthony Liguori
Quoted 10 lines · Anthony Liguori
Quoted 8 lines · Michael S. Tsirkin
Quoted 6 lines · Anthony Liguori
Quoted 3 lines · Michael S. Tsirkin
So I see two ways to go forward: switch default value in my patch, or disable vlans unconditionally.
The problem with disabling vlans unconditionally is that you break -net socket and -net dump.
-netdev socket seems to be supported. No?
Quoted 7 lines
Sure, but it's of limited utility in the absence of vlans. A typical thing to do with -net socket would be to launch one instance of qemu with -net user, -net socket, and -net nic. Another qemu would be launched with -net socket and -net nic connected to the previous instance. Now you've got a working virtual network with external access.
-netdev socket alone won't get you this.
I see three possible options to handle this.
(1) Write a hub (or morph the current vlan code into this). Then
you can do something like:
qemu -netdev socket,id=p1 \
-netdev user,id=p2 \
-netdev dump,id=p3 \
-switch peer=p1,peer=p2,monitor=p3,port=p4 \
-device $nic,netdev=p4(2) Implement the same as external daemon which can be combined with -netdev socket.
(3) Just point people who need this to the various virtual switch projects (vde, ...) they can use and drop vlan.
cheers,
Gerd
Quoted 11 lines
I see three possible options to handle this.
(1) Write a hub (or morph the current vlan code into this). Then
you can do something like: qemu -netdev socket,id=p1 \ -netdev user,id=p2 \ -netdev dump,id=p3 \ -switch peer=p1,peer=p2,monitor=p3,port=p4 \ -device $nic,netdev=p4
Quoted 5 lines
(2) Implement the same as external daemon which can be combined with -netdev socket.
(3) Just point people who need this to the various virtual switch projects (vde, ...) they can use and drop vlan.
(2) is just a special case of (3), where we decide that the existing implementations suck and go write our own.
Paul
Quoted 11 lines · Paul Brook
Quoted 11 lines
I see three possible options to handle this.
(1) Write a hub (or morph the current vlan code into this). Then you can do something like: qemu -netdev socket,id=p1 \ -netdev user,id=p2 \ -netdev dump,id=p3 \ -switch peer=p1,peer=p2,monitor=p3,port=p4 \ -device $nic,netdev=p4
Quoted 9 lines
Quoted 6 lines
(2) Implement the same as external daemon which can be combined with -netdev socket.
(3) Just point people who need this to the various virtual switch projects (vde, ...) they can use and drop vlan.
(2) is just a special case of (3), where we decide that the existing implementations suck and go write our own.
To the extent that (1) is valuable, I think it's the best approach. I'd vote for officially deprecating vlans for 0.13 and then seeing how much people complain. If no one complains too much, then let's not bother introducing -switch.
Regards,
Anthony Liguori
Quoted 2 lines
Paul
Quoted 7 lines · Paul Brook
Quoted 7 lines · Paul Brook
Quoted 5 lines
Quoted 3 lines
With -netdev, there now seems to be little need to support vlans, enabling them leads to user confusion and bad performance. Disable support for vlans by default, add config option to enable.
No. If you want to remove vlans, then actually do that.
Quoted 7 lines
Quoted 2 lines
How is this not what this patch does? You mean kill the code completely, not just --contigure option?
Yes. Configure options are bad. If code isn't worth enabling by default then you've got to have a very good reason why it exists at all.
Paul
Everyone ok with disabling vlans with no config option?
Signature · 1 line
MST
Quoted 11 lines · Michael S. Tsirkin
Quoted 10 lines · Paul Brook
Quoted 9 lines · Paul Brook
Quoted 5 lines
Quoted 3 lines
With -netdev, there now seems to be little need to support vlans, enabling them leads to user confusion and bad performance. Disable support for vlans by default, add config option to enable.
No. If you want to remove vlans, then actually do that.
How is this not what this patch does? You mean kill the code completely, not just --contigure option?
Quoted 6 lines
Quoted 4 lines
Yes. Configure options are bad. If code isn't worth enabling by default then you've got to have a very good reason why it exists at all.
Paul
Everyone ok with disabling vlans with no config option?
Wrong question. You got to ask "anyone *not* ok with disabling vlans with no config option?"
Quoted 6 lines · Markus Armbruster
Quoted 1 line · Michael S. Tsirkin
Everyone ok with disabling vlans with no config option?
Wrong question. You got to ask "anyone *not* ok with disabling vlans with no config option?"
We do use socket devices in the form
-net nic,model=e1000,vlan=X,mac=MMM -net socket,vlan=X,mcast=Y:Z
but presumably this can just be rewritten as
-netdev socket,id=netX,mcast=Y:Z -device e1000,netdev=netX,mac=MMM
It's only the case of a nic connected to multiple backends by a VLAN that's being deprecated here, not any of the previously supported backends?
Cheers,
Chris.
Quoted 15 lines · Chris Webb
Quoted 6 lines · Markus Armbruster
Quoted 1 line · Michael S. Tsirkin
Everyone ok with disabling vlans with no config option?
Wrong question. You got to ask "anyone *not* ok with disabling vlans with no config option?"
We do use socket devices in the form
-net nic,model=e1000,vlan=X,mac=MMM -net socket,vlan=X,mcast=Y:Z
but presumably this can just be rewritten as
Quoted 8 lines
-netdev socket,id=netX,mcast=Y:Z -device e1000,netdev=netX,mac=MMM
It's only the case of a nic connected to multiple backends by a VLAN that's being deprecated here, not any of the previously supported backends?
Cheers,
Chris.
Exactly.
Quoted 10 lines · Paul Brook
Quoted 3 lines
With -netdev, there now seems to be little need to support vlans, enabling them leads to user confusion and bad performance. Disable support for vlans by default, add config option to enable.
No. If you want to remove vlans, then actually do that.
As I've said before if you want a point-point network model then you should implement that (and remove the vlan code, probably replacing with equivalent functionality). We should not have both point-point and broadcast interfaces.Paul
By the way I agree with this. It would have been better to simply prohibit more than 2 devices on a vlan instead of two competing ways to set up networking that we have now.
And the conclusion I think should be to remove broadcast support ASAP.
Signature · 1 line
MST