[PATCH net] vmxnet3: Don't enable vlan filters in promiscuous mode.

[PATCH net] vmxnet3: Don't enable vlan filters in promiscuous mode.

[Jesse Gross]

The vmxnet3 driver enables vlan filters if filtering is enabled for
any vlan.  In promiscuous mode the filter table is cleared to in
order to disable filtering.  However, if a vlan device is subsequently
created that vlan will be added to the filter, re-engaging it.  As a
result, not only do we not see all the vlans in promiscuous mode, we
don't even see vlans for which a filter was previously created.

CC: Scott J. Goldman <scottjg@vmware.com>
CC: Shreyas Bhatewara <sbhatewara@vmware.com>
CC: VMware PV-Drivers <pv-drivers@vmware.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
---
 drivers/net/vmxnet3/vmxnet3_drv.c |   34 ++++++++++++++++++++--------------
 1 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 1cbacb3..0959583 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -1929,14 +1929,17 @@ static void
 vmxnet3_vlan_rx_add_vid(struct net_device *netdev, u16 vid)
 {
 	struct vmxnet3_adapter *adapter = netdev_priv(netdev);
-	u32 *vfTable = adapter->shared->devRead.rxFilterConf.vfTable;
-	unsigned long flags;
 
-	VMXNET3_SET_VFTABLE_ENTRY(vfTable, vid);
-	spin_lock_irqsave(&adapter->cmd_lock, flags);
-	VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
-			       VMXNET3_CMD_UPDATE_VLAN_FILTERS);
-	spin_unlock_irqrestore(&adapter->cmd_lock, flags);
+	if (!(netdev->flags & IFF_PROMISC)) {
+		u32 *vfTable = adapter->shared->devRead.rxFilterConf.vfTable;
+		unsigned long flags;
+
+		VMXNET3_SET_VFTABLE_ENTRY(vfTable, vid);
+		spin_lock_irqsave(&adapter->cmd_lock, flags);
+		VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
+				       VMXNET3_CMD_UPDATE_VLAN_FILTERS);
+		spin_unlock_irqrestore(&adapter->cmd_lock, flags);
+	}
 
 	set_bit(vid, adapter->active_vlans);
 }
@@ -1946,14 +1949,17 @@ static void
 vmxnet3_vlan_rx_kill_vid(struct net_device *netdev, u16 vid)
 {
 	struct vmxnet3_adapter *adapter = netdev_priv(netdev);
-	u32 *vfTable = adapter->shared->devRead.rxFilterConf.vfTable;
-	unsigned long flags;
 
-	VMXNET3_CLEAR_VFTABLE_ENTRY(vfTable, vid);
-	spin_lock_irqsave(&adapter->cmd_lock, flags);
-	VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
-			       VMXNET3_CMD_UPDATE_VLAN_FILTERS);
-	spin_unlock_irqrestore(&adapter->cmd_lock, flags);
+	if (!(netdev->flags & IFF_PROMISC)) {
+		u32 *vfTable = adapter->shared->devRead.rxFilterConf.vfTable;
+		unsigned long flags;
+
+		VMXNET3_CLEAR_VFTABLE_ENTRY(vfTable, vid);
+		spin_lock_irqsave(&adapter->cmd_lock, flags);
+		VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
+				       VMXNET3_CMD_UPDATE_VLAN_FILTERS);
+		spin_unlock_irqrestore(&adapter->cmd_lock, flags);
+	}
 
 	clear_bit(vid, adapter->active_vlans);
 }
-- 
1.7.4.1

Re: [PATCH net] vmxnet3: Don't enable vlan filters in promiscuous mode.

[Shreyas Bhatewara]

On Mon, 8 Aug 2011, Jesse Gross wrote:

> @@ -1929,14 +1929,17 @@ static void
>  vmxnet3_vlan_rx_add_vid(struct net_device *netdev, u16 vid)
>  {
>  	struct vmxnet3_adapter *adapter = netdev_priv(netdev);
> -	u32 *vfTable = adapter->shared->devRead.rxFilterConf.vfTable;
> -	unsigned long flags;
>  
> -	VMXNET3_SET_VFTABLE_ENTRY(vfTable, vid);
> -	spin_lock_irqsave(&adapter->cmd_lock, flags);
> -	VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
> -			       VMXNET3_CMD_UPDATE_VLAN_FILTERS);
> -	spin_unlock_irqrestore(&adapter->cmd_lock, flags);
> +	if (!(netdev->flags & IFF_PROMISC)) {
> +		u32 *vfTable = adapter->shared->devRead.rxFilterConf.vfTable;
> +		unsigned long flags;
> +
> +		VMXNET3_SET_VFTABLE_ENTRY(vfTable, vid);
> +		spin_lock_irqsave(&adapter->cmd_lock, flags);
> +		VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
> +				       VMXNET3_CMD_UPDATE_VLAN_FILTERS);
> +		spin_unlock_irqrestore(&adapter->cmd_lock, flags);
> +	}
>  

If this is done, the driver will ignore all vlan tag registrations (and 
deletions) while the interface is in promiscuous mode. Better solution
would be to send UPDATE_VLAN_FILTERS command alone inside the promiscuous 
condition. vfTable can be set/unset unconditionally as before. By doing 
this, when the interface comes out of promiscuous mode, the restored vlan 
state will have all the added/removed vlan tags into effect.

Re: [PATCH net] vmxnet3: Don't enable vlan filters in promiscuous mode.

[Jesse Gross]

On Wed, Aug 10, 2011 at 5:32 AM, Shreyas Bhatewara
<sbhatewara@vmware.com> wrote:
>
>
> On Mon, 8 Aug 2011, Jesse Gross wrote:
>
>> @@ -1929,14 +1929,17 @@ static void
>>  vmxnet3_vlan_rx_add_vid(struct net_device *netdev, u16 vid)
>>  {
>>       struct vmxnet3_adapter *adapter = netdev_priv(netdev);
>> -     u32 *vfTable = adapter->shared->devRead.rxFilterConf.vfTable;
>> -     unsigned long flags;
>>
>> -     VMXNET3_SET_VFTABLE_ENTRY(vfTable, vid);
>> -     spin_lock_irqsave(&adapter->cmd_lock, flags);
>> -     VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
>> -                            VMXNET3_CMD_UPDATE_VLAN_FILTERS);
>> -     spin_unlock_irqrestore(&adapter->cmd_lock, flags);
>> +     if (!(netdev->flags & IFF_PROMISC)) {
>> +             u32 *vfTable = adapter->shared->devRead.rxFilterConf.vfTable;
>> +             unsigned long flags;
>> +
>> +             VMXNET3_SET_VFTABLE_ENTRY(vfTable, vid);
>> +             spin_lock_irqsave(&adapter->cmd_lock, flags);
>> +             VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
>> +                                    VMXNET3_CMD_UPDATE_VLAN_FILTERS);
>> +             spin_unlock_irqrestore(&adapter->cmd_lock, flags);
>> +     }
>>
>
> If this is done, the driver will ignore all vlan tag registrations (and
> deletions) while the interface is in promiscuous mode. Better solution
> would be to send UPDATE_VLAN_FILTERS command alone inside the promiscuous
> condition. vfTable can be set/unset unconditionally as before. By doing
> this, when the interface comes out of promiscuous mode, the restored vlan
> state will have all the added/removed vlan tags into effect.

Adds and removes are not ignored when in promiscuous mode because the
active_vlans bitfield is still being updated (this is in the context
that you clipped out).  When we come out of promiscuous mode,
vmxnet3_restore_vlan() is called which will update the hardware with
the vlans that have been registered.  This is much safer than directly
updating vfTable because it will get overwritten if vmxnet3_set_mc()
is called again and is potentially very error prone to have it not
reflect what we actually want programmed in the hardware.

Re: [PATCH net] vmxnet3: Don't enable vlan filters in promiscuous mode.

[Shreyas Bhatewara]

> 
> Adds and removes are not ignored when in promiscuous mode because the
> active_vlans bitfield is still being updated (this is in the context
> that you clipped out).  When we come out of promiscuous mode,
> vmxnet3_restore_vlan() is called which will update the hardware with
> the vlans that have been registered.  This is much safer than directly
> updating vfTable because it will get overwritten if vmxnet3_set_mc()
> is called again and is potentially very error prone to have it not
> reflect what we actually want programmed in the hardware.
> 

Thats right.

Signed-off-by: Shreyas N Bhatewara <sbhatewara@vmware.com>

Re: [PATCH net] vmxnet3: Don't enable vlan filters in promiscuous mode.

[David Miller]

From: Jesse Gross <jesse@nicira.com>
Date: Mon,  8 Aug 2011 02:15:47 -0700

> The vmxnet3 driver enables vlan filters if filtering is enabled for
> any vlan.  In promiscuous mode the filter table is cleared to in
> order to disable filtering.  However, if a vlan device is subsequently
> created that vlan will be added to the filter, re-engaging it.  As a
> result, not only do we not see all the vlans in promiscuous mode, we
> don't even see vlans for which a filter was previously created.
> 
> CC: Scott J. Goldman <scottjg@vmware.com>
> CC: Shreyas Bhatewara <sbhatewara@vmware.com>
> CC: VMware PV-Drivers <pv-drivers@vmware.com>
> Signed-off-by: Jesse Gross <jesse@nicira.com>

Applied.