* [Qemu-devel] [PATCH] configure: add an option to disable vlans
@ 2010-06-07 15:03 Michael S. Tsirkin
2010-06-07 16:16 ` Paul Brook
0 siblings, 1 reply; 24+ messages in thread
From: Michael S. Tsirkin @ 2010-06-07 15:03 UTC (permalink / raw)
To: qemu-devel
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.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
configure | 12 ++++++++++++
net.c | 7 +++++++
2 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/configure b/configure
index 3cd2c5f..728d9a1 100755
--- a/configure
+++ b/configure
@@ -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"
diff --git a/net.c b/net.c
index 378edfc..a6af5f7 100644
--- a/net.c
+++ b/net.c
@@ -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
--
1.7.1.12.g42b7f
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans
2010-06-07 15:03 [Qemu-devel] [PATCH] configure: add an option to disable vlans Michael S. Tsirkin
@ 2010-06-07 16:16 ` Paul Brook
2010-06-07 16:17 ` Michael S. Tsirkin
2010-06-08 12:17 ` Michael S. Tsirkin
0 siblings, 2 replies; 24+ messages in thread
From: Paul Brook @ 2010-06-07 16:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael S. Tsirkin
> 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans
2010-06-07 16:16 ` Paul Brook
@ 2010-06-07 16:17 ` Michael S. Tsirkin
2010-06-07 16:42 ` Paul Brook
2010-06-08 12:17 ` Michael S. Tsirkin
1 sibling, 1 reply; 24+ messages in thread
From: Michael S. Tsirkin @ 2010-06-07 16:17 UTC (permalink / raw)
To: Paul Brook; +Cc: qemu-devel
On Mon, Jun 07, 2010 at 05:16:30PM +0100, Paul Brook wrote:
> > 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?
> 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.
--
MST
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans
2010-06-07 16:17 ` Michael S. Tsirkin
@ 2010-06-07 16:42 ` Paul Brook
2010-06-07 16:52 ` Anthony Liguori
2010-06-07 17:41 ` Michael S. Tsirkin
0 siblings, 2 replies; 24+ messages in thread
From: Paul Brook @ 2010-06-07 16:42 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel
> On Mon, Jun 07, 2010 at 05:16:30PM +0100, Paul Brook wrote:
> > > 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans
2010-06-07 16:42 ` Paul Brook
@ 2010-06-07 16:52 ` Anthony Liguori
2010-06-07 19:21 ` Michael S. Tsirkin
2010-06-07 17:41 ` Michael S. Tsirkin
1 sibling, 1 reply; 24+ messages in thread
From: Anthony Liguori @ 2010-06-07 16:52 UTC (permalink / raw)
To: Paul Brook; +Cc: qemu-devel, Michael S. Tsirkin
On 06/07/2010 11:42 AM, Paul Brook wrote:
>> On Mon, Jun 07, 2010 at 05:16:30PM +0100, Paul Brook wrote:
>>
>>>> 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.
>
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
> Paul
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans
2010-06-07 16:42 ` Paul Brook
2010-06-07 16:52 ` Anthony Liguori
@ 2010-06-07 17:41 ` Michael S. Tsirkin
2010-06-09 7:44 ` Markus Armbruster
1 sibling, 1 reply; 24+ messages in thread
From: Michael S. Tsirkin @ 2010-06-07 17:41 UTC (permalink / raw)
To: Paul Brook; +Cc: qemu-devel
On Mon, Jun 07, 2010 at 05:42:55PM +0100, Paul Brook wrote:
> > On Mon, Jun 07, 2010 at 05:16:30PM +0100, Paul Brook wrote:
> > > > 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
Everyone ok with disabling vlans with no config option?
--
MST
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans
2010-06-07 16:52 ` Anthony Liguori
@ 2010-06-07 19:21 ` Michael S. Tsirkin
2010-06-07 20:40 ` Anthony Liguori
0 siblings, 1 reply; 24+ messages in thread
From: Michael S. Tsirkin @ 2010-06-07 19:21 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Paul Brook, qemu-devel
On Mon, Jun 07, 2010 at 11:52:05AM -0500, Anthony Liguori wrote:
> On 06/07/2010 11:42 AM, Paul Brook wrote:
>>> On Mon, Jun 07, 2010 at 05:16:30PM +0100, Paul Brook wrote:
>>>
>>>>> 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.
>>
>
> 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
So I see two ways to go forward: switch default value in my patch,
or disable vlans unconditionally.
Which will it be?
>> Paul
>>
>>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans
2010-06-07 19:21 ` Michael S. Tsirkin
@ 2010-06-07 20:40 ` Anthony Liguori
2010-06-07 20:58 ` Michael S. Tsirkin
2010-06-08 11:09 ` Michael S. Tsirkin
0 siblings, 2 replies; 24+ messages in thread
From: Anthony Liguori @ 2010-06-07 20:40 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Paul Brook, qemu-devel
On 06/07/2010 02:21 PM, Michael S. Tsirkin wrote:
>
> 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
> Which will it be?
>
>
>>> Paul
>>>
>>>
>>>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans
2010-06-07 20:40 ` Anthony Liguori
@ 2010-06-07 20:58 ` Michael S. Tsirkin
2010-06-07 21:37 ` Anthony Liguori
2010-06-08 11:09 ` Michael S. Tsirkin
1 sibling, 1 reply; 24+ messages in thread
From: Michael S. Tsirkin @ 2010-06-07 20:58 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Paul Brook, qemu-devel
On Mon, Jun 07, 2010 at 03:40:57PM -0500, Anthony Liguori wrote:
> On 06/07/2010 02:21 PM, Michael S. Tsirkin wrote:
>>
>> 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?
> Regards,
>
> Anthony Liguori
>
>> Which will it be?
>>
>>
>>>> Paul
>>>>
>>>>
>>>>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans
2010-06-07 20:58 ` Michael S. Tsirkin
@ 2010-06-07 21:37 ` Anthony Liguori
2010-06-07 21:57 ` Anthony Liguori
0 siblings, 1 reply; 24+ messages in thread
From: Anthony Liguori @ 2010-06-07 21:37 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Paul Brook, qemu-devel
On 06/07/2010 03:58 PM, Michael S. Tsirkin wrote:
> On Mon, Jun 07, 2010 at 03:40:57PM -0500, Anthony Liguori wrote:
>
>> On 06/07/2010 02:21 PM, Michael S. Tsirkin wrote:
>>
>>> 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?
>
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
>> Regards,
>>
>> Anthony Liguori
>>
>>
>>> Which will it be?
>>>
>>>
>>>
>>>>> Paul
>>>>>
>>>>>
>>>>>
>>>>>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans
2010-06-07 21:37 ` Anthony Liguori
@ 2010-06-07 21:57 ` Anthony Liguori
2010-06-08 12:13 ` Michael S. Tsirkin
0 siblings, 1 reply; 24+ messages in thread
From: Anthony Liguori @ 2010-06-07 21:57 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Paul Brook, qemu-devel
On 06/07/2010 04:37 PM, Anthony Liguori wrote:
> On 06/07/2010 03:58 PM, Michael S. Tsirkin wrote:
>> On Mon, Jun 07, 2010 at 03:40:57PM -0500, Anthony Liguori wrote:
>>> On 06/07/2010 02:21 PM, Michael S. Tsirkin wrote:
>>>> 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?
>
> 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans
2010-06-07 20:40 ` Anthony Liguori
2010-06-07 20:58 ` Michael S. Tsirkin
@ 2010-06-08 11:09 ` Michael S. Tsirkin
2010-06-08 13:02 ` Anthony Liguori
1 sibling, 1 reply; 24+ messages in thread
From: Michael S. Tsirkin @ 2010-06-08 11:09 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Paul Brook, qemu-devel
On Mon, Jun 07, 2010 at 03:40:57PM -0500, Anthony Liguori wrote:
> On 06/07/2010 02:21 PM, Michael S. Tsirkin wrote:
>>
>> 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?
> If we can come up with an alternative way to do these things, I'm all
> for removing it.
>
> Regards,
>
> Anthony Liguori
>
>> Which will it be?
>>
>>
>>>> Paul
>>>>
>>>>
>>>>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans
2010-06-07 21:57 ` Anthony Liguori
@ 2010-06-08 12:13 ` Michael S. Tsirkin
2010-06-08 13:03 ` Anthony Liguori
0 siblings, 1 reply; 24+ messages in thread
From: Michael S. Tsirkin @ 2010-06-08 12:13 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Paul Brook, qemu-devel
On Mon, Jun 07, 2010 at 04:57:09PM -0500, Anthony Liguori wrote:
> On 06/07/2010 04:37 PM, Anthony Liguori wrote:
>> On 06/07/2010 03:58 PM, Michael S. Tsirkin wrote:
>>> On Mon, Jun 07, 2010 at 03:40:57PM -0500, Anthony Liguori wrote:
>>>> On 06/07/2010 02:21 PM, Michael S. Tsirkin wrote:
>>>>> 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?
>>
>> 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.
>
> 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.
--
MST
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans
2010-06-07 16:16 ` Paul Brook
2010-06-07 16:17 ` Michael S. Tsirkin
@ 2010-06-08 12:17 ` Michael S. Tsirkin
1 sibling, 0 replies; 24+ messages in thread
From: Michael S. Tsirkin @ 2010-06-08 12:17 UTC (permalink / raw)
To: Paul Brook; +Cc: qemu-devel
On Mon, Jun 07, 2010 at 05:16:30PM +0100, Paul Brook wrote:
> > 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.
--
MST
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans
2010-06-08 11:09 ` Michael S. Tsirkin
@ 2010-06-08 13:02 ` Anthony Liguori
2010-06-08 13:14 ` Michael S. Tsirkin
2010-06-08 14:25 ` Gerd Hoffmann
0 siblings, 2 replies; 24+ messages in thread
From: Anthony Liguori @ 2010-06-08 13:02 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Paul Brook, qemu-devel
On 06/08/2010 06:09 AM, Michael S. Tsirkin wrote:
> On Mon, Jun 07, 2010 at 03:40:57PM -0500, Anthony Liguori wrote:
>
>> On 06/07/2010 02:21 PM, Michael S. Tsirkin wrote:
>>
>>> 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans
2010-06-08 12:13 ` Michael S. Tsirkin
@ 2010-06-08 13:03 ` Anthony Liguori
2010-06-08 13:07 ` Michael S. Tsirkin
0 siblings, 1 reply; 24+ messages in thread
From: Anthony Liguori @ 2010-06-08 13:03 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Paul Brook, qemu-devel
On 06/08/2010 07:13 AM, Michael S. Tsirkin wrote:
> On Mon, Jun 07, 2010 at 04:57:09PM -0500, Anthony Liguori wrote:
>
>> On 06/07/2010 04:37 PM, Anthony Liguori wrote:
>>
>>> On 06/07/2010 03:58 PM, Michael S. Tsirkin wrote:
>>>
>>>> On Mon, Jun 07, 2010 at 03:40:57PM -0500, Anthony Liguori wrote:
>>>>
>>>>> On 06/07/2010 02:21 PM, Michael S. Tsirkin wrote:
>>>>>
>>>>>> 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?
>>>>
>>> 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
>> 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.
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans
2010-06-08 13:03 ` Anthony Liguori
@ 2010-06-08 13:07 ` Michael S. Tsirkin
0 siblings, 0 replies; 24+ messages in thread
From: Michael S. Tsirkin @ 2010-06-08 13:07 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Paul Brook, qemu-devel
On Tue, Jun 08, 2010 at 08:03:33AM -0500, Anthony Liguori wrote:
> On 06/08/2010 07:13 AM, Michael S. Tsirkin wrote:
>> On Mon, Jun 07, 2010 at 04:57:09PM -0500, Anthony Liguori wrote:
>>
>>> On 06/07/2010 04:37 PM, Anthony Liguori wrote:
>>>
>>>> On 06/07/2010 03:58 PM, Michael S. Tsirkin wrote:
>>>>
>>>>> On Mon, Jun 07, 2010 at 03:40:57PM -0500, Anthony Liguori wrote:
>>>>>
>>>>>> On 06/07/2010 02:21 PM, Michael S. Tsirkin wrote:
>>>>>>
>>>>>>> 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?
>>>>>
>>>> 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.
You mean you want to support both peer to peer and broadcast
indefinitely?
> 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
>>> 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.
>>
>>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans
2010-06-08 13:02 ` Anthony Liguori
@ 2010-06-08 13:14 ` Michael S. Tsirkin
2010-06-08 14:25 ` Gerd Hoffmann
1 sibling, 0 replies; 24+ messages in thread
From: Michael S. Tsirkin @ 2010-06-08 13:14 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Paul Brook, qemu-devel
On Tue, Jun 08, 2010 at 08:02:27AM -0500, Anthony Liguori wrote:
> On 06/08/2010 06:09 AM, Michael S. Tsirkin wrote:
>> On Mon, Jun 07, 2010 at 03:40:57PM -0500, Anthony Liguori wrote:
>>
>>> On 06/07/2010 02:21 PM, Michael S. Tsirkin wrote:
>>>
>>>> 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.
Only terribly slow, esp when tunneling tcp over tcp.
> -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.
> Regards,
>
> Anthony Liguori
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans
2010-06-08 13:02 ` Anthony Liguori
2010-06-08 13:14 ` Michael S. Tsirkin
@ 2010-06-08 14:25 ` Gerd Hoffmann
2010-06-08 14:37 ` Paul Brook
1 sibling, 1 reply; 24+ messages in thread
From: Gerd Hoffmann @ 2010-06-08 14:25 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel, Paul Brook, Michael S. Tsirkin
On 06/08/10 15:02, Anthony Liguori wrote:
> On 06/08/2010 06:09 AM, Michael S. Tsirkin wrote:
>> On Mon, Jun 07, 2010 at 03:40:57PM -0500, Anthony Liguori wrote:
>>> On 06/07/2010 02:21 PM, Michael S. Tsirkin wrote:
>>>> 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.
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans
2010-06-08 14:25 ` Gerd Hoffmann
@ 2010-06-08 14:37 ` Paul Brook
2010-06-08 18:01 ` Anthony Liguori
0 siblings, 1 reply; 24+ messages in thread
From: Paul Brook @ 2010-06-08 14:37 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel, Michael S. Tsirkin
> 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.
(2) is just a special case of (3), where we decide that the existing
implementations suck and go write our own.
Paul
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans
2010-06-08 14:37 ` Paul Brook
@ 2010-06-08 18:01 ` Anthony Liguori
0 siblings, 0 replies; 24+ messages in thread
From: Anthony Liguori @ 2010-06-08 18:01 UTC (permalink / raw)
To: Paul Brook; +Cc: qemu-devel, Gerd Hoffmann, Michael S. Tsirkin
On 06/08/2010 09:37 AM, Paul Brook wrote:
>> 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.
>>
> (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
> Paul
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans
2010-06-07 17:41 ` Michael S. Tsirkin
@ 2010-06-09 7:44 ` Markus Armbruster
2010-06-10 7:20 ` Chris Webb
0 siblings, 1 reply; 24+ messages in thread
From: Markus Armbruster @ 2010-06-09 7:44 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Paul Brook, qemu-devel
"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Mon, Jun 07, 2010 at 05:42:55PM +0100, Paul Brook wrote:
>> > On Mon, Jun 07, 2010 at 05:16:30PM +0100, Paul Brook wrote:
>> > > > 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
>
> 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?"
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans
2010-06-09 7:44 ` Markus Armbruster
@ 2010-06-10 7:20 ` Chris Webb
2010-06-10 8:43 ` Michael S. Tsirkin
0 siblings, 1 reply; 24+ messages in thread
From: Chris Webb @ 2010-06-10 7:20 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, Paul Brook, Michael S. Tsirkin
Markus Armbruster <armbru@redhat.com> writes:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
>
> > 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.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans
2010-06-10 7:20 ` Chris Webb
@ 2010-06-10 8:43 ` Michael S. Tsirkin
0 siblings, 0 replies; 24+ messages in thread
From: Michael S. Tsirkin @ 2010-06-10 8:43 UTC (permalink / raw)
To: Chris Webb; +Cc: qemu-devel, Markus Armbruster, Paul Brook
On Thu, Jun 10, 2010 at 08:20:56AM +0100, Chris Webb wrote:
> Markus Armbruster <armbru@redhat.com> writes:
>
> > "Michael S. Tsirkin" <mst@redhat.com> writes:
> >
> > > 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.
Exactly.
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2010-06-10 8:48 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-07 15:03 [Qemu-devel] [PATCH] configure: add an option to disable vlans Michael S. Tsirkin
2010-06-07 16:16 ` Paul Brook
2010-06-07 16:17 ` Michael S. Tsirkin
2010-06-07 16:42 ` Paul Brook
2010-06-07 16:52 ` Anthony Liguori
2010-06-07 19:21 ` Michael S. Tsirkin
2010-06-07 20:40 ` Anthony Liguori
2010-06-07 20:58 ` Michael S. Tsirkin
2010-06-07 21:37 ` Anthony Liguori
2010-06-07 21:57 ` Anthony Liguori
2010-06-08 12:13 ` Michael S. Tsirkin
2010-06-08 13:03 ` Anthony Liguori
2010-06-08 13:07 ` Michael S. Tsirkin
2010-06-08 11:09 ` Michael S. Tsirkin
2010-06-08 13:02 ` Anthony Liguori
2010-06-08 13:14 ` Michael S. Tsirkin
2010-06-08 14:25 ` Gerd Hoffmann
2010-06-08 14:37 ` Paul Brook
2010-06-08 18:01 ` Anthony Liguori
2010-06-07 17:41 ` Michael S. Tsirkin
2010-06-09 7:44 ` Markus Armbruster
2010-06-10 7:20 ` Chris Webb
2010-06-10 8:43 ` Michael S. Tsirkin
2010-06-08 12:17 ` Michael S. Tsirkin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.