| Message ID | 1312794947-3163-1-git-send-email-jesse@nicira.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > 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> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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); }
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(-)