| Message ID | 1294360199-9860-9-git-send-email-jeffrey.t.kirsher@intel.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Thu, Jan 6, 2011 at 7:29 PM, <jeffrey.t.kirsher@intel.com> wrote: > +static int ixgb_set_flags(struct net_device *netdev, u32 data) > +{ > + struct ixgb_adapter *adapter = netdev_priv(netdev); > + bool need_reset; > + int rc; > + > + /* > + * TX vlan insertion does not work per HW design when Rx stripping is > + * disabled. Disable txvlan when rxvlan is off. > + */ > + if ((data & ETH_FLAG_RXVLAN) != (netdev->features & NETIF_F_HW_VLAN_RX)) > + data ^= ETH_FLAG_TXVLAN; Does this really do the right thing? If the RX vlan setting is changed, it will do the opposite of what the user requested for TX vlan? So if I start with both on (the default) and turn them both off in one command (a valid setting), I will get RX off and TX on (an invalid setting). Why not: if (!(data & ETH_FLAG_RXVLAN)) data &= ~ETH_FLAG_TXVLAN; -- 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
Jesse Gross wrote: > On Thu, Jan 6, 2011 at 7:29 PM, <jeffrey.t.kirsher@intel.com> wrote: >> +static int ixgb_set_flags(struct net_device *netdev, u32 data) +{ >> + struct ixgb_adapter *adapter = netdev_priv(netdev); + >> bool need_reset; + int rc; >> + >> + /* >> + * TX vlan insertion does not work per HW design when Rx >> stripping is + * disabled. Disable txvlan when rxvlan is >> off. + */ + if ((data & ETH_FLAG_RXVLAN) != >> (netdev->features & NETIF_F_HW_VLAN_RX)) + data ^= >> ETH_FLAG_TXVLAN; > > Does this really do the right thing? If the RX vlan setting is > changed, it will do the opposite of what the user requested for TX > vlan? > > So if I start with both on (the default) and turn them both off in one > command (a valid setting), I will get RX off and TX on (an invalid > setting). > > Why not: > > if (!(data & ETH_FLAG_RXVLAN)) > data &= ~ETH_FLAG_TXVLAN; Ah, you're right. We missed this in testing. I will spin another patch. Thanks for all your help. Emil-- 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
Jesse Gross wrote: > On Thu, Jan 6, 2011 at 7:29 PM, <jeffrey.t.kirsher@intel.com> wrote: >> +static int ixgb_set_flags(struct net_device *netdev, u32 data) +{ >> + struct ixgb_adapter *adapter = netdev_priv(netdev); + >> bool need_reset; + int rc; >> + >> + /* >> + * TX vlan insertion does not work per HW design when Rx >> stripping is + * disabled. Disable txvlan when rxvlan is >> off. + */ + if ((data & ETH_FLAG_RXVLAN) != >> (netdev->features & NETIF_F_HW_VLAN_RX)) + data ^= >> ETH_FLAG_TXVLAN; > > Does this really do the right thing? If the RX vlan setting is > changed, it will do the opposite of what the user requested for TX > vlan? > > So if I start with both on (the default) and turn them both off in one > command (a valid setting), I will get RX off and TX on (an invalid > setting). > > Why not: > > if (!(data & ETH_FLAG_RXVLAN)) > data &= ~ETH_FLAG_TXVLAN; Yeah that works for disabling rxvlan, but what if rxvlan is disabled, and the user attempts to enable txvlan? At least our validation argued that we should make it work both ways. Perhaps something like the following? if (!(data & ETH_FLAG_RXVLAN) && (netdev->features & NETIF_F_HW_VLAN_TX)) data &= ~ETH_FLAG_TXVLAN; else if (data & ETH_FLAG_TXVLAN) data |= ETH_FLAG_RXVLAN; Thanks, Emil-- 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 Sun, Jan 23, 2011 at 4:25 PM, Tantilov, Emil S <emil.s.tantilov@intel.com> wrote: > Jesse Gross wrote: >> On Thu, Jan 6, 2011 at 7:29 PM, <jeffrey.t.kirsher@intel.com> wrote: >>> +static int ixgb_set_flags(struct net_device *netdev, u32 data) +{ >>> + struct ixgb_adapter *adapter = netdev_priv(netdev); + >>> bool need_reset; + int rc; >>> + >>> + /* >>> + * TX vlan insertion does not work per HW design when Rx >>> stripping is + * disabled. Disable txvlan when rxvlan is >>> off. + */ + if ((data & ETH_FLAG_RXVLAN) != >>> (netdev->features & NETIF_F_HW_VLAN_RX)) + data ^= >>> ETH_FLAG_TXVLAN; >> >> Does this really do the right thing? If the RX vlan setting is >> changed, it will do the opposite of what the user requested for TX >> vlan? >> >> So if I start with both on (the default) and turn them both off in one >> command (a valid setting), I will get RX off and TX on (an invalid >> setting). >> >> Why not: >> >> if (!(data & ETH_FLAG_RXVLAN)) >> data &= ~ETH_FLAG_TXVLAN; > > Yeah that works for disabling rxvlan, but what if rxvlan is disabled, and the user attempts to enable txvlan? At least our validation argued that we should make it work both ways. Perhaps something like the following? > > if (!(data & ETH_FLAG_RXVLAN) && > (netdev->features & NETIF_F_HW_VLAN_TX)) > data &= ~ETH_FLAG_TXVLAN; > else if (data & ETH_FLAG_TXVLAN) > data |= ETH_FLAG_RXVLAN; I think the logic above does what you describe and will always result in a consistent state. Turning dependent features on when needed is a little bit inconsistent with the rest of Ethtool (for example, turning on TSO when checksum offloading is off will not enable checksum offloading, it will produce an error). However, I know that drivers aren't completely consistent here and the most important part is that it enforces valid states, so I don't have a strong opinion. Ben's previous suggestion of Ethtool querying again after the operation and reporting any flags that were automatically changed would help a lot here. -- 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
>-----Original Message----- >From: Jesse Gross [mailto:jesse@nicira.com] >Sent: Tuesday, January 25, 2011 9:23 AM >To: Tantilov, Emil S >Cc: Kirsher, Jeffrey T; davem@davemloft.net; netdev@vger.kernel.org; >bphilips@novell.com; Pieper, Jeffrey E >Subject: Re: [net-next 08/12] ixgb: convert to new VLAN model > >On Sun, Jan 23, 2011 at 4:25 PM, Tantilov, Emil S ><emil.s.tantilov@intel.com> wrote: >> Jesse Gross wrote: >>> On Thu, Jan 6, 2011 at 7:29 PM, <jeffrey.t.kirsher@intel.com> wrote: >>>> +static int ixgb_set_flags(struct net_device *netdev, u32 data) +{ >>>> + struct ixgb_adapter *adapter = netdev_priv(netdev); + >>>> bool need_reset; + int rc; >>>> + >>>> + /* >>>> + * TX vlan insertion does not work per HW design when Rx >>>> stripping is + * disabled. Disable txvlan when rxvlan is >>>> off. + */ + if ((data & ETH_FLAG_RXVLAN) != >>>> (netdev->features & NETIF_F_HW_VLAN_RX)) + data ^= >>>> ETH_FLAG_TXVLAN; >>> >>> Does this really do the right thing? If the RX vlan setting is >>> changed, it will do the opposite of what the user requested for TX >>> vlan? >>> >>> So if I start with both on (the default) and turn them both off in one >>> command (a valid setting), I will get RX off and TX on (an invalid >>> setting). >>> >>> Why not: >>> >>> if (!(data & ETH_FLAG_RXVLAN)) >>> data &= ~ETH_FLAG_TXVLAN; >> >> Yeah that works for disabling rxvlan, but what if rxvlan is disabled, and >the user attempts to enable txvlan? At least our validation argued that we >should make it work both ways. Perhaps something like the following? >> >> if (!(data & ETH_FLAG_RXVLAN) && >> (netdev->features & NETIF_F_HW_VLAN_TX)) >> data &= ~ETH_FLAG_TXVLAN; >> else if (data & ETH_FLAG_TXVLAN) >> data |= ETH_FLAG_RXVLAN; > >I think the logic above does what you describe and will always result >in a consistent state. Turning dependent features on when needed is a >little bit inconsistent with the rest of Ethtool (for example, turning >on TSO when checksum offloading is off will not enable checksum >offloading, it will produce an error). However, I know that drivers That is the reason I asked, as I don't want to keep bouncing the patch back and forth. Personally I like the idea of helping the user and adjusting the flags to something that works rather than a generic error message. >aren't completely consistent here and the most important part is that >it enforces valid states, so I don't have a strong opinion. Ben's >previous suggestion of Ethtool querying again after the operation and >reporting any flags that were automatically changed would help a lot >here. Sure, but I think a savvy user would always check the result of an ethtool command (ie. `ethtool -K` followed with `ethtool -k`, -A/-a, etc). ` Added Ben in case he has comments. Thanks, Emil -- 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 Tue, Jan 25, 2011 at 10:20 AM, Tantilov, Emil S <emil.s.tantilov@intel.com> wrote: >>-----Original Message----- >>From: Jesse Gross [mailto:jesse@nicira.com] >>Sent: Tuesday, January 25, 2011 9:23 AM >>To: Tantilov, Emil S >>Cc: Kirsher, Jeffrey T; davem@davemloft.net; netdev@vger.kernel.org; >>bphilips@novell.com; Pieper, Jeffrey E >>Subject: Re: [net-next 08/12] ixgb: convert to new VLAN model >> >>On Sun, Jan 23, 2011 at 4:25 PM, Tantilov, Emil S >><emil.s.tantilov@intel.com> wrote: >>> Jesse Gross wrote: >>>> On Thu, Jan 6, 2011 at 7:29 PM, <jeffrey.t.kirsher@intel.com> wrote: >>>>> +static int ixgb_set_flags(struct net_device *netdev, u32 data) +{ >>>>> + struct ixgb_adapter *adapter = netdev_priv(netdev); + >>>>> bool need_reset; + int rc; >>>>> + >>>>> + /* >>>>> + * TX vlan insertion does not work per HW design when Rx >>>>> stripping is + * disabled. Disable txvlan when rxvlan is >>>>> off. + */ + if ((data & ETH_FLAG_RXVLAN) != >>>>> (netdev->features & NETIF_F_HW_VLAN_RX)) + data ^= >>>>> ETH_FLAG_TXVLAN; >>>> >>>> Does this really do the right thing? If the RX vlan setting is >>>> changed, it will do the opposite of what the user requested for TX >>>> vlan? >>>> >>>> So if I start with both on (the default) and turn them both off in one >>>> command (a valid setting), I will get RX off and TX on (an invalid >>>> setting). >>>> >>>> Why not: >>>> >>>> if (!(data & ETH_FLAG_RXVLAN)) >>>> data &= ~ETH_FLAG_TXVLAN; >>> >>> Yeah that works for disabling rxvlan, but what if rxvlan is disabled, and >>the user attempts to enable txvlan? At least our validation argued that we >>should make it work both ways. Perhaps something like the following? >>> >>> if (!(data & ETH_FLAG_RXVLAN) && >>> (netdev->features & NETIF_F_HW_VLAN_TX)) >>> data &= ~ETH_FLAG_TXVLAN; >>> else if (data & ETH_FLAG_TXVLAN) >>> data |= ETH_FLAG_RXVLAN; >> >>I think the logic above does what you describe and will always result >>in a consistent state. Turning dependent features on when needed is a >>little bit inconsistent with the rest of Ethtool (for example, turning >>on TSO when checksum offloading is off will not enable checksum >>offloading, it will produce an error). However, I know that drivers > > That is the reason I asked, as I don't want to keep bouncing the patch back and forth. Personally I like the idea of helping the user and adjusting the flags to something that works rather than a generic error message. I think it is fine to adjust things, especially where the restrictions are hardware specific and the user is less likely to know what settings are related. As long as it works, it doesn't matter too much to me either way, so please do what you think is the most appropriate. > >>aren't completely consistent here and the most important part is that >>it enforces valid states, so I don't have a strong opinion. Ben's >>previous suggestion of Ethtool querying again after the operation and >>reporting any flags that were automatically changed would help a lot >>here. > > Sure, but I think a savvy user would always check the result of an ethtool command (ie. `ethtool -K` followed with `ethtool -k`, -A/-a, etc). Probably, but it seems the less savviness required from the user the better. Regardless, it doesn't affect anything here, it would just be a change to the userspace tool. -- 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, 2011-01-26 at 19:53 -0800, Jesse Gross wrote: > On Tue, Jan 25, 2011 at 10:20 AM, Tantilov, Emil S > <emil.s.tantilov@intel.com> wrote: [...] > > Sure, but I think a savvy user would always check the result of an > > ethtool command (ie. `ethtool -K` followed with `ethtool -k`, -A/-a, > > etc). > > Probably, but it seems the less savviness required from the user the > better. Regardless, it doesn't affect anything here, it would just be > a change to the userspace tool. I am intending to modify ethtool so that it will report any other offload settings that were changed automatically. Ben.
On Tue, Jan 25, 2011 at 10:20 AM, Tantilov, Emil S <emil.s.tantilov@intel.com> wrote: >>-----Original Message----- >>From: Jesse Gross [mailto:jesse@nicira.com] >>Sent: Tuesday, January 25, 2011 9:23 AM >>To: Tantilov, Emil S >>Cc: Kirsher, Jeffrey T; davem@davemloft.net; netdev@vger.kernel.org; >>bphilips@novell.com; Pieper, Jeffrey E >>Subject: Re: [net-next 08/12] ixgb: convert to new VLAN model >> >>On Sun, Jan 23, 2011 at 4:25 PM, Tantilov, Emil S >><emil.s.tantilov@intel.com> wrote: >>> Jesse Gross wrote: >>>> On Thu, Jan 6, 2011 at 7:29 PM, <jeffrey.t.kirsher@intel.com> wrote: >>>>> +static int ixgb_set_flags(struct net_device *netdev, u32 data) +{ >>>>> + struct ixgb_adapter *adapter = netdev_priv(netdev); + >>>>> bool need_reset; + int rc; >>>>> + >>>>> + /* >>>>> + * TX vlan insertion does not work per HW design when Rx >>>>> stripping is + * disabled. Disable txvlan when rxvlan is >>>>> off. + */ + if ((data & ETH_FLAG_RXVLAN) != >>>>> (netdev->features & NETIF_F_HW_VLAN_RX)) + data ^= >>>>> ETH_FLAG_TXVLAN; >>>> >>>> Does this really do the right thing? If the RX vlan setting is >>>> changed, it will do the opposite of what the user requested for TX >>>> vlan? >>>> >>>> So if I start with both on (the default) and turn them both off in one >>>> command (a valid setting), I will get RX off and TX on (an invalid >>>> setting). >>>> >>>> Why not: >>>> >>>> if (!(data & ETH_FLAG_RXVLAN)) >>>> data &= ~ETH_FLAG_TXVLAN; >>> >>> Yeah that works for disabling rxvlan, but what if rxvlan is disabled, and >>the user attempts to enable txvlan? At least our validation argued that we >>should make it work both ways. Perhaps something like the following? >>> >>> if (!(data & ETH_FLAG_RXVLAN) && >>> (netdev->features & NETIF_F_HW_VLAN_TX)) >>> data &= ~ETH_FLAG_TXVLAN; >>> else if (data & ETH_FLAG_TXVLAN) >>> data |= ETH_FLAG_RXVLAN; >> >>I think the logic above does what you describe and will always result >>in a consistent state. Turning dependent features on when needed is a >>little bit inconsistent with the rest of Ethtool (for example, turning >>on TSO when checksum offloading is off will not enable checksum >>offloading, it will produce an error). However, I know that drivers > > That is the reason I asked, as I don't want to keep bouncing the patch back and forth. Personally I like the idea of helping the user and adjusting the flags to something that works rather than a generic error message. Were you able to take a look at this? I think that we have something that is pretty close, so it would be nice to wrap it up. -- 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 Mon, 2011-03-07 at 17:31 -0800, Jesse Gross wrote: > On Tue, Jan 25, 2011 at 10:20 AM, Tantilov, Emil S > <emil.s.tantilov@intel.com> wrote: > >>-----Original Message----- > >>From: Jesse Gross [mailto:jesse@nicira.com] > >>Sent: Tuesday, January 25, 2011 9:23 AM > >>To: Tantilov, Emil S > >>Cc: Kirsher, Jeffrey T; davem@davemloft.net; netdev@vger.kernel.org; > >>bphilips@novell.com; Pieper, Jeffrey E > >>Subject: Re: [net-next 08/12] ixgb: convert to new VLAN model > >> > >>On Sun, Jan 23, 2011 at 4:25 PM, Tantilov, Emil S > >><emil.s.tantilov@intel.com> wrote: > >>> Jesse Gross wrote: > >>>> On Thu, Jan 6, 2011 at 7:29 PM, <jeffrey.t.kirsher@intel.com> wrote: > >>>>> +static int ixgb_set_flags(struct net_device *netdev, u32 data) +{ > >>>>> + struct ixgb_adapter *adapter = netdev_priv(netdev); + > >>>>> bool need_reset; + int rc; > >>>>> + > >>>>> + /* > >>>>> + * TX vlan insertion does not work per HW design when Rx > >>>>> stripping is + * disabled. Disable txvlan when rxvlan is > >>>>> off. + */ + if ((data & ETH_FLAG_RXVLAN) != > >>>>> (netdev->features & NETIF_F_HW_VLAN_RX)) + data ^= > >>>>> ETH_FLAG_TXVLAN; > >>>> > >>>> Does this really do the right thing? If the RX vlan setting is > >>>> changed, it will do the opposite of what the user requested for TX > >>>> vlan? > >>>> > >>>> So if I start with both on (the default) and turn them both off in one > >>>> command (a valid setting), I will get RX off and TX on (an invalid > >>>> setting). > >>>> > >>>> Why not: > >>>> > >>>> if (!(data & ETH_FLAG_RXVLAN)) > >>>> data &= ~ETH_FLAG_TXVLAN; > >>> > >>> Yeah that works for disabling rxvlan, but what if rxvlan is disabled, and > >>the user attempts to enable txvlan? At least our validation argued that we > >>should make it work both ways. Perhaps something like the following? > >>> > >>> if (!(data & ETH_FLAG_RXVLAN) && > >>> (netdev->features & NETIF_F_HW_VLAN_TX)) > >>> data &= ~ETH_FLAG_TXVLAN; > >>> else if (data & ETH_FLAG_TXVLAN) > >>> data |= ETH_FLAG_RXVLAN; > >> > >>I think the logic above does what you describe and will always result > >>in a consistent state. Turning dependent features on when needed is a > >>little bit inconsistent with the rest of Ethtool (for example, turning > >>on TSO when checksum offloading is off will not enable checksum > >>offloading, it will produce an error). However, I know that drivers > > > > That is the reason I asked, as I don't want to keep bouncing the patch back and forth. Personally I like the idea of helping the user and adjusting the flags to something that works rather than a generic error message. > > Were you able to take a look at this? I think that we have something > that is pretty close, so it would be nice to wrap it up. I have a patch which has passed testing from Emil, I just need to check with Emil that the current patch I have ready to push is the "latest" version and does not need any more tweaks. Cheers, Jeff
diff --git a/drivers/net/ixgb/ixgb.h b/drivers/net/ixgb/ixgb.h index 521c0c7..8f3df04 100644 --- a/drivers/net/ixgb/ixgb.h +++ b/drivers/net/ixgb/ixgb.h @@ -149,7 +149,7 @@ struct ixgb_desc_ring { struct ixgb_adapter { struct timer_list watchdog_timer; - struct vlan_group *vlgrp; + unsigned long active_vlans[BITS_TO_LONGS(VLAN_N_VID)]; u32 bd_number; u32 rx_buffer_len; u32 part_num; diff --git a/drivers/net/ixgb/ixgb_ethtool.c b/drivers/net/ixgb/ixgb_ethtool.c index 43994c1..1294161 100644 --- a/drivers/net/ixgb/ixgb_ethtool.c +++ b/drivers/net/ixgb/ixgb_ethtool.c @@ -706,6 +706,39 @@ ixgb_get_strings(struct net_device *netdev, u32 stringset, u8 *data) } } +static int ixgb_set_flags(struct net_device *netdev, u32 data) +{ + struct ixgb_adapter *adapter = netdev_priv(netdev); + bool need_reset; + int rc; + + /* + * TX vlan insertion does not work per HW design when Rx stripping is + * disabled. Disable txvlan when rxvlan is off. + */ + if ((data & ETH_FLAG_RXVLAN) != (netdev->features & NETIF_F_HW_VLAN_RX)) + data ^= ETH_FLAG_TXVLAN; + + need_reset = (data & ETH_FLAG_RXVLAN) != + (netdev->features & NETIF_F_HW_VLAN_RX); + + rc = ethtool_op_set_flags(netdev, data, ETH_FLAG_RXVLAN | + ETH_FLAG_TXVLAN); + if (rc) + return rc; + + if (need_reset) { + if (netif_running(netdev)) { + ixgb_down(adapter, true); + ixgb_up(adapter); + ixgb_set_speed_duplex(netdev); + } else + ixgb_reset(adapter); + } + + return 0; +} + static const struct ethtool_ops ixgb_ethtool_ops = { .get_settings = ixgb_get_settings, .set_settings = ixgb_set_settings, @@ -732,6 +765,8 @@ static const struct ethtool_ops ixgb_ethtool_ops = { .phys_id = ixgb_phys_id, .get_sset_count = ixgb_get_sset_count, .get_ethtool_stats = ixgb_get_ethtool_stats, + .get_flags = ethtool_op_get_flags, + .set_flags = ixgb_set_flags, }; void ixgb_set_ethtool_ops(struct net_device *netdev) diff --git a/drivers/net/ixgb/ixgb_main.c b/drivers/net/ixgb/ixgb_main.c index 5639ccc..0f681ac 100644 --- a/drivers/net/ixgb/ixgb_main.c +++ b/drivers/net/ixgb/ixgb_main.c @@ -100,8 +100,6 @@ static void ixgb_tx_timeout_task(struct work_struct *work); static void ixgb_vlan_strip_enable(struct ixgb_adapter *adapter); static void ixgb_vlan_strip_disable(struct ixgb_adapter *adapter); -static void ixgb_vlan_rx_register(struct net_device *netdev, - struct vlan_group *grp); static void ixgb_vlan_rx_add_vid(struct net_device *netdev, u16 vid); static void ixgb_vlan_rx_kill_vid(struct net_device *netdev, u16 vid); static void ixgb_restore_vlan(struct ixgb_adapter *adapter); @@ -336,7 +334,6 @@ static const struct net_device_ops ixgb_netdev_ops = { .ndo_set_mac_address = ixgb_set_mac, .ndo_change_mtu = ixgb_change_mtu, .ndo_tx_timeout = ixgb_tx_timeout, - .ndo_vlan_rx_register = ixgb_vlan_rx_register, .ndo_vlan_rx_add_vid = ixgb_vlan_rx_add_vid, .ndo_vlan_rx_kill_vid = ixgb_vlan_rx_kill_vid, #ifdef CONFIG_NET_POLL_CONTROLLER @@ -1508,7 +1505,7 @@ ixgb_xmit_frame(struct sk_buff *skb, struct net_device *netdev) DESC_NEEDED))) return NETDEV_TX_BUSY; - if (adapter->vlgrp && vlan_tx_tag_present(skb)) { + if (vlan_tx_tag_present(skb)) { tx_flags |= IXGB_TX_FLAGS_VLAN; vlan_id = vlan_tx_tag_get(skb); } @@ -2049,12 +2046,11 @@ ixgb_clean_rx_irq(struct ixgb_adapter *adapter, int *work_done, int work_to_do) ixgb_rx_checksum(adapter, rx_desc, skb); skb->protocol = eth_type_trans(skb, netdev); - if (adapter->vlgrp && (status & IXGB_RX_DESC_STATUS_VP)) { - vlan_hwaccel_receive_skb(skb, adapter->vlgrp, - le16_to_cpu(rx_desc->special)); - } else { - netif_receive_skb(skb); - } + if (status & IXGB_RX_DESC_STATUS_VP) + __vlan_hwaccel_put_tag(skb, + le16_to_cpu(rx_desc->special)); + + netif_receive_skb(skb); rxdesc_done: /* clean up descriptor, might be written over by hw */ @@ -2152,20 +2148,6 @@ map_skb: } } -/** - * ixgb_vlan_rx_register - enables or disables vlan tagging/stripping. - * - * @param netdev network interface device structure - * @param grp indicates to enable or disable tagging/stripping - **/ -static void -ixgb_vlan_rx_register(struct net_device *netdev, struct vlan_group *grp) -{ - struct ixgb_adapter *adapter = netdev_priv(netdev); - - adapter->vlgrp = grp; -} - static void ixgb_vlan_strip_enable(struct ixgb_adapter *adapter) { @@ -2200,6 +2182,7 @@ ixgb_vlan_rx_add_vid(struct net_device *netdev, u16 vid) vfta = IXGB_READ_REG_ARRAY(&adapter->hw, VFTA, index); vfta |= (1 << (vid & 0x1F)); ixgb_write_vfta(&adapter->hw, index, vfta); + set_bit(vid, adapter->active_vlans); } static void @@ -2208,35 +2191,22 @@ ixgb_vlan_rx_kill_vid(struct net_device *netdev, u16 vid) struct ixgb_adapter *adapter = netdev_priv(netdev); u32 vfta, index; - ixgb_irq_disable(adapter); - - vlan_group_set_device(adapter->vlgrp, vid, NULL); - - /* don't enable interrupts unless we are UP */ - if (adapter->netdev->flags & IFF_UP) - ixgb_irq_enable(adapter); - /* remove VID from filter table */ index = (vid >> 5) & 0x7F; vfta = IXGB_READ_REG_ARRAY(&adapter->hw, VFTA, index); vfta &= ~(1 << (vid & 0x1F)); ixgb_write_vfta(&adapter->hw, index, vfta); + clear_bit(vid, adapter->active_vlans); } static void ixgb_restore_vlan(struct ixgb_adapter *adapter) { - ixgb_vlan_rx_register(adapter->netdev, adapter->vlgrp); - - if (adapter->vlgrp) { - u16 vid; - for (vid = 0; vid < VLAN_N_VID; vid++) { - if (!vlan_group_get_device(adapter->vlgrp, vid)) - continue; - ixgb_vlan_rx_add_vid(adapter->netdev, vid); - } - } + u16 vid; + + for_each_set_bit(vid, adapter->active_vlans, VLAN_N_VID) + ixgb_vlan_rx_add_vid(adapter->netdev, vid); } #ifdef CONFIG_NET_POLL_CONTROLLER