[Qemu-devel] [PATCH] configure: add an option to disable vlans

[Qemu-devel] [PATCH] configure: add an option to disable vlans

[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.

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

Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans

[Paul Brook]

> 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

Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans

[Michael S. Tsirkin]

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

Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans

[Paul Brook]

> 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

Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans

[Anthony Liguori]

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
>
>    

Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans

[Michael S. Tsirkin]

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
>>
>>    

Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans

[Anthony Liguori]

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
>>>
>>>
>>>        

Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans

[Michael S. Tsirkin]

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
>>>>
>>>>
>>>>        

Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans

[Anthony Liguori]

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
>>>>>
>>>>>
>>>>>
>>>>>            

Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans

[Anthony Liguori]

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

Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans

[Michael S. Tsirkin]

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

Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans

[Anthony Liguori]

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.
>
>    

Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans

[Michael S. Tsirkin]

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.
>>
>>    

Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans

[Michael S. Tsirkin]

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
>>>>
>>>>
>>>>        

Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans

[Anthony Liguori]

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

Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans

[Michael S. Tsirkin]

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

Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans

[Gerd Hoffmann]

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

Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans

[Paul Brook]

> 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

Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans

[Anthony Liguori]

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
>    

Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans

[Michael S. Tsirkin]

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

Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans

[Markus Armbruster]

"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?"

Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans

[Chris Webb]

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.

Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans

[Michael S. Tsirkin]

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.

Re: [Qemu-devel] [PATCH] configure: add an option to disable vlans

[Michael S. Tsirkin]

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