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

On Wed, Aug 10, 2011 at 5:32 AM, Shreyas Bhatewara
[off-list ref] wrote:

On Mon, 8 Aug 2011, Jesse Gross wrote:

quoted

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