| Message ID | 1310212240-24623-6-git-send-email-jeffrey.t.kirsher@intel.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Sat, Jul 09, 2011 at 04:50:40AM -0700, Jeff Kirsher wrote: > From: Don Skidmore <donald.c.skidmore@intel.com> > > Private rx_csum flags are now duplicate of netdev->features & > NETIF_F_RXCSUM. We remove those duplicates and now use the net_device_ops > ndo_set_features. This was based on the original patch submitted by > Michal Miroslaw <mirq-linux@rere.qmqm.pl>. I also removed the special > case not requiring a reset for X540 hardware. It is needed just as it is > in 82599 hardware. [...] > --- a/drivers/net/ixgbe/ixgbe_main.c > +++ b/drivers/net/ixgbe/ixgbe_main.c > @@ -7188,6 +7188,88 @@ int ixgbe_setup_tc(struct net_device *dev, u8 tc) [...] > +static int ixgbe_set_features(struct net_device *netdev, u32 data) > +{ > + struct ixgbe_adapter *adapter = netdev_priv(netdev); > + bool need_reset = false; > + > +#ifdef CONFIG_DCB > + if ((adapter->flags & IXGBE_FLAG_DCB_ENABLED) && > + !(data & NETIF_F_HW_VLAN_RX)) > + return -EINVAL; > +#endif > + /* return error if RXHASH is being enabled when RSS is not supported */ > + if (!(adapter->flags & IXGBE_FLAG_RSS_ENABLED) && > + (data & NETIF_F_RXHASH)) > + return -EINVAL; > + > + /* If Rx checksum is disabled, then RSC/LRO should also be disabled */ > + if (!(data & NETIF_F_RXCSUM)) { > + data &= ~NETIF_F_LRO; > + adapter->flags &= ~IXGBE_FLAG_RX_CSUM_ENABLED; > + } else { > + adapter->flags |= IXGBE_FLAG_RX_CSUM_ENABLED; > + } The checks above should be done in ndo_fix_features callback. The check for RSS_ENABLED for RXHASH is redundant, as the feature is removed in probe() in this case (see [MARK] below). > + > + /* if state changes we need to update adapter->flags and reset */ > + if ((adapter->flags2 & IXGBE_FLAG2_RSC_CAPABLE) && > + (!!(data & NETIF_F_LRO) != > + !!(adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED))) { > + if ((data & NETIF_F_LRO) && > + adapter->rx_itr_setting != 1 && > + adapter->rx_itr_setting > IXGBE_MAX_RSC_INT_RATE) { > + e_info(probe, "rx-usecs set too low, " > + "not enabling RSC\n"); > + } else { > + adapter->flags2 ^= IXGBE_FLAG2_RSC_ENABLED; > + switch (adapter->hw.mac.type) { > + case ixgbe_mac_X540: > + case ixgbe_mac_82599EB: > + need_reset = true; > + break; > + default: > + break; > + } > + } > + } > + > + /* > + * Check if Flow Director n-tuple support was enabled or disabled. If > + * the state changed, we need to reset. > + */ > + if (!(adapter->flags & IXGBE_FLAG_FDIR_PERFECT_CAPABLE)) { > + /* turn off ATR, enable perfect filters and reset */ > + if (data & NETIF_F_NTUPLE) { > + adapter->flags &= ~IXGBE_FLAG_FDIR_HASH_CAPABLE; > + adapter->flags |= IXGBE_FLAG_FDIR_PERFECT_CAPABLE; > + need_reset = true; > + } > + } else if (!(data & NETIF_F_NTUPLE)) { > + /* turn off Flow Director, set ATR and reset */ > + adapter->flags &= ~IXGBE_FLAG_FDIR_PERFECT_CAPABLE; > + if ((adapter->flags & IXGBE_FLAG_RSS_ENABLED) && > + !(adapter->flags & IXGBE_FLAG_DCB_ENABLED)) > + adapter->flags |= IXGBE_FLAG_FDIR_HASH_CAPABLE; > + need_reset = true; > + } Part of the checks above should be in ndo_fix_features, too. ndo_set_features should just enable what it has been passed. What ndo_fix_features callback returns is further limited by generic checks, and then (if the resulting set is different than current dev->features) ndo_set_features is called. > + > + if (need_reset) > + ixgbe_do_reset(netdev); > + > + return 0; > + > +} > + > static const struct net_device_ops ixgbe_netdev_ops = { > .ndo_open = ixgbe_open, > .ndo_stop = ixgbe_close, > @@ -7219,6 +7301,7 @@ static const struct net_device_ops ixgbe_netdev_ops = { > .ndo_fcoe_disable = ixgbe_fcoe_disable, > .ndo_fcoe_get_wwn = ixgbe_fcoe_get_wwn, > #endif /* IXGBE_FCOE */ > + .ndo_set_features = ixgbe_set_features, > }; > > static void __devinit ixgbe_probe_vf(struct ixgbe_adapter *adapter, > @@ -7486,20 +7569,24 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev, > > netdev->features = NETIF_F_SG | > NETIF_F_IP_CSUM | > + NETIF_F_IPV6_CSUM | > NETIF_F_HW_VLAN_TX | > NETIF_F_HW_VLAN_RX | > - NETIF_F_HW_VLAN_FILTER; > + NETIF_F_HW_VLAN_FILTER | > + NETIF_F_TSO | > + NETIF_F_TSO6 | > + NETIF_F_GRO | > + NETIF_F_RXHASH | > + NETIF_F_RXCSUM; > > - netdev->features |= NETIF_F_IPV6_CSUM; > - netdev->features |= NETIF_F_TSO; > - netdev->features |= NETIF_F_TSO6; > - netdev->features |= NETIF_F_GRO; > - netdev->features |= NETIF_F_RXHASH; > + netdev->hw_features = netdev->features; > > switch (adapter->hw.mac.type) { > case ixgbe_mac_82599EB: > case ixgbe_mac_X540: > netdev->features |= NETIF_F_SCTP_CSUM; > + netdev->hw_features |= NETIF_F_SCTP_CSUM | > + NETIF_F_NTUPLE; > break; > default: > break; > @@ -7538,6 +7625,8 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev, > netdev->vlan_features |= NETIF_F_HIGHDMA; > } > > + if (adapter->flags2 & IXGBE_FLAG2_RSC_CAPABLE) > + netdev->hw_features |= NETIF_F_LRO; > if (adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED) > netdev->features |= NETIF_F_LRO; > > @@ -7574,8 +7663,10 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev, > if (err) > goto err_sw_init; > > - if (!(adapter->flags & IXGBE_FLAG_RSS_ENABLED)) > + if (!(adapter->flags & IXGBE_FLAG_RSS_ENABLED)) { > + netdev->hw_features &= ~NETIF_F_RXHASH; > netdev->features &= ~NETIF_F_RXHASH; > + } > [MARK here] Best Regards, Michał Mirosław -- 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: Michal Miroslaw [mailto:mirq-linux@rere.qmqm.pl] >Sent: Saturday, July 09, 2011 5:11 AM >To: Kirsher, Jeffrey T >Cc: davem@davemloft.net; Skidmore, Donald C; netdev@vger.kernel.org; >gospo@redhat.com >Subject: Re: [net-next 5/5] ixgbe: convert to ndo_fix_features > >On Sat, Jul 09, 2011 at 04:50:40AM -0700, Jeff Kirsher wrote: >> From: Don Skidmore <donald.c.skidmore@intel.com> >> >> Private rx_csum flags are now duplicate of netdev->features & >> NETIF_F_RXCSUM. We remove those duplicates and now use the >net_device_ops >> ndo_set_features. This was based on the original patch submitted by >> Michal Miroslaw <mirq-linux@rere.qmqm.pl>. I also removed the special >> case not requiring a reset for X540 hardware. It is needed just as it >is >> in 82599 hardware. >[...] >> --- a/drivers/net/ixgbe/ixgbe_main.c >> +++ b/drivers/net/ixgbe/ixgbe_main.c >> @@ -7188,6 +7188,88 @@ int ixgbe_setup_tc(struct net_device *dev, u8 >tc) >[...] >> +static int ixgbe_set_features(struct net_device *netdev, u32 data) >> +{ >> + struct ixgbe_adapter *adapter = netdev_priv(netdev); >> + bool need_reset = false; >> + >> +#ifdef CONFIG_DCB >> + if ((adapter->flags & IXGBE_FLAG_DCB_ENABLED) && >> + !(data & NETIF_F_HW_VLAN_RX)) >> + return -EINVAL; >> +#endif >> + /* return error if RXHASH is being enabled when RSS is not >supported */ >> + if (!(adapter->flags & IXGBE_FLAG_RSS_ENABLED) && >> + (data & NETIF_F_RXHASH)) >> + return -EINVAL; >> + >> + /* If Rx checksum is disabled, then RSC/LRO should also be >disabled */ >> + if (!(data & NETIF_F_RXCSUM)) { >> + data &= ~NETIF_F_LRO; >> + adapter->flags &= ~IXGBE_FLAG_RX_CSUM_ENABLED; >> + } else { >> + adapter->flags |= IXGBE_FLAG_RX_CSUM_ENABLED; >> + } > >The checks above should be done in ndo_fix_features callback. The check >for >RSS_ENABLED for RXHASH is redundant, as the feature is removed in >probe() >in this case (see [MARK] below). Thanks for the comments Michal, it clears up a lot in my mind. :) So in ndo_fix_features we would just turn off feature flags rather than return an error? For example: if (!(data & NETIF_F_RXCSUM)) data &= ~NETIF_F_LRO; As for RSS_ENABLED/RXHASH check it was to cover the cases where RSS_ENABLED might have changed since probe. This could happen with a resume were we don't get enough MSI-X vectors. There are also paths in FCoE and DCB that get into code that could clear IXGBE_FLAG_RSS_ENABLED. > >> + >> + /* if state changes we need to update adapter->flags and reset */ >> + if ((adapter->flags2 & IXGBE_FLAG2_RSC_CAPABLE) && >> + (!!(data & NETIF_F_LRO) != >> + !!(adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED))) { >> + if ((data & NETIF_F_LRO) && >> + adapter->rx_itr_setting != 1 && >> + adapter->rx_itr_setting > IXGBE_MAX_RSC_INT_RATE) { >> + e_info(probe, "rx-usecs set too low, " >> + "not enabling RSC\n"); >> + } else { >> + adapter->flags2 ^= IXGBE_FLAG2_RSC_ENABLED; >> + switch (adapter->hw.mac.type) { >> + case ixgbe_mac_X540: >> + case ixgbe_mac_82599EB: >> + need_reset = true; >> + break; >> + default: >> + break; >> + } >> + } >> + } >> + >> + /* >> + * Check if Flow Director n-tuple support was enabled or disabled. >If >> + * the state changed, we need to reset. >> + */ >> + if (!(adapter->flags & IXGBE_FLAG_FDIR_PERFECT_CAPABLE)) { >> + /* turn off ATR, enable perfect filters and reset */ >> + if (data & NETIF_F_NTUPLE) { >> + adapter->flags &= ~IXGBE_FLAG_FDIR_HASH_CAPABLE; >> + adapter->flags |= IXGBE_FLAG_FDIR_PERFECT_CAPABLE; >> + need_reset = true; >> + } >> + } else if (!(data & NETIF_F_NTUPLE)) { >> + /* turn off Flow Director, set ATR and reset */ >> + adapter->flags &= ~IXGBE_FLAG_FDIR_PERFECT_CAPABLE; >> + if ((adapter->flags & IXGBE_FLAG_RSS_ENABLED) && >> + !(adapter->flags & IXGBE_FLAG_DCB_ENABLED)) >> + adapter->flags |= IXGBE_FLAG_FDIR_HASH_CAPABLE; >> + need_reset = true; >> + } > >Part of the checks above should be in ndo_fix_features, too. >ndo_set_features >should just enable what it has been passed. What ndo_fix_features >callback >returns is further limited by generic checks, and then (if the resulting >set >is different than current dev->features) ndo_set_features is called. I'm a little confused here. From your comments I get the idea that ndo_fix_features() just modifies and error checks our feature set. The result of this would then be just a change to the feature set (data variable in my case above). Is that a correct assumption? If so I'm confused as none of the two checks above change the feature set. They do change driver flags to indicate the new state and mark whether we need a reset. I don't believe we would want to do the reset until ndo_set_feature is called and if we broke up the setting of the driver flags into ndo_fix_features we would lose some state (i.e. if the IXGBE_FLAG2_RSC_ENABLED changed) that we need to decide if a reset is needed in ndo_set_features. Am I just missing something here? Thanks -Don > >> + >> + if (need_reset) >> + ixgbe_do_reset(netdev); >> + >> + return 0; >> + >> +} >> + >> static const struct net_device_ops ixgbe_netdev_ops = { >> .ndo_open = ixgbe_open, >> .ndo_stop = ixgbe_close, >> @@ -7219,6 +7301,7 @@ static const struct net_device_ops >ixgbe_netdev_ops = { >> .ndo_fcoe_disable = ixgbe_fcoe_disable, >> .ndo_fcoe_get_wwn = ixgbe_fcoe_get_wwn, >> #endif /* IXGBE_FCOE */ >> + .ndo_set_features = ixgbe_set_features, >> }; >> >> static void __devinit ixgbe_probe_vf(struct ixgbe_adapter *adapter, >> @@ -7486,20 +7569,24 @@ static int __devinit ixgbe_probe(struct >pci_dev *pdev, >> >> netdev->features = NETIF_F_SG | >> NETIF_F_IP_CSUM | >> + NETIF_F_IPV6_CSUM | >> NETIF_F_HW_VLAN_TX | >> NETIF_F_HW_VLAN_RX | >> - NETIF_F_HW_VLAN_FILTER; >> + NETIF_F_HW_VLAN_FILTER | >> + NETIF_F_TSO | >> + NETIF_F_TSO6 | >> + NETIF_F_GRO | >> + NETIF_F_RXHASH | >> + NETIF_F_RXCSUM; >> >> - netdev->features |= NETIF_F_IPV6_CSUM; >> - netdev->features |= NETIF_F_TSO; >> - netdev->features |= NETIF_F_TSO6; >> - netdev->features |= NETIF_F_GRO; >> - netdev->features |= NETIF_F_RXHASH; >> + netdev->hw_features = netdev->features; >> >> switch (adapter->hw.mac.type) { >> case ixgbe_mac_82599EB: >> case ixgbe_mac_X540: >> netdev->features |= NETIF_F_SCTP_CSUM; >> + netdev->hw_features |= NETIF_F_SCTP_CSUM | >> + NETIF_F_NTUPLE; >> break; >> default: >> break; >> @@ -7538,6 +7625,8 @@ static int __devinit ixgbe_probe(struct pci_dev >*pdev, >> netdev->vlan_features |= NETIF_F_HIGHDMA; >> } >> >> + if (adapter->flags2 & IXGBE_FLAG2_RSC_CAPABLE) >> + netdev->hw_features |= NETIF_F_LRO; >> if (adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED) >> netdev->features |= NETIF_F_LRO; >> >> @@ -7574,8 +7663,10 @@ static int __devinit ixgbe_probe(struct pci_dev >*pdev, >> if (err) >> goto err_sw_init; >> >> - if (!(adapter->flags & IXGBE_FLAG_RSS_ENABLED)) >> + if (!(adapter->flags & IXGBE_FLAG_RSS_ENABLED)) { >> + netdev->hw_features &= ~NETIF_F_RXHASH; >> netdev->features &= ~NETIF_F_RXHASH; >> + } >> > >[MARK here] > >Best Regards, >Michał Mirosław -- 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, Jul 11, 2011 at 04:53:57PM -0700, Skidmore, Donald C wrote: > >-----Original Message----- > >From: Michal Miroslaw [mailto:mirq-linux@rere.qmqm.pl] > >Sent: Saturday, July 09, 2011 5:11 AM > >To: Kirsher, Jeffrey T > >Cc: davem@davemloft.net; Skidmore, Donald C; netdev@vger.kernel.org; > >gospo@redhat.com > >Subject: Re: [net-next 5/5] ixgbe: convert to ndo_fix_features > > > >On Sat, Jul 09, 2011 at 04:50:40AM -0700, Jeff Kirsher wrote: > >> From: Don Skidmore <donald.c.skidmore@intel.com> > >> > >> Private rx_csum flags are now duplicate of netdev->features & > >> NETIF_F_RXCSUM. We remove those duplicates and now use the > >net_device_ops > >> ndo_set_features. This was based on the original patch submitted by > >> Michal Miroslaw <mirq-linux@rere.qmqm.pl>. I also removed the special > >> case not requiring a reset for X540 hardware. It is needed just as it > >is > >> in 82599 hardware. > >[...] > >> --- a/drivers/net/ixgbe/ixgbe_main.c > >> +++ b/drivers/net/ixgbe/ixgbe_main.c > >> @@ -7188,6 +7188,88 @@ int ixgbe_setup_tc(struct net_device *dev, u8 > >tc) > >[...] > >> +static int ixgbe_set_features(struct net_device *netdev, u32 data) > >> +{ > >> + struct ixgbe_adapter *adapter = netdev_priv(netdev); > >> + bool need_reset = false; > >> + > >> +#ifdef CONFIG_DCB > >> + if ((adapter->flags & IXGBE_FLAG_DCB_ENABLED) && > >> + !(data & NETIF_F_HW_VLAN_RX)) > >> + return -EINVAL; > >> +#endif > >> + /* return error if RXHASH is being enabled when RSS is not > >supported */ > >> + if (!(adapter->flags & IXGBE_FLAG_RSS_ENABLED) && > >> + (data & NETIF_F_RXHASH)) > >> + return -EINVAL; > >> + > >> + /* If Rx checksum is disabled, then RSC/LRO should also be > >disabled */ > >> + if (!(data & NETIF_F_RXCSUM)) { > >> + data &= ~NETIF_F_LRO; > >> + adapter->flags &= ~IXGBE_FLAG_RX_CSUM_ENABLED; > >> + } else { > >> + adapter->flags |= IXGBE_FLAG_RX_CSUM_ENABLED; > >> + } > > > >The checks above should be done in ndo_fix_features callback. The check > >for > >RSS_ENABLED for RXHASH is redundant, as the feature is removed in > >probe() > >in this case (see [MARK] below). > Thanks for the comments Michal, it clears up a lot in my mind. :) > > So in ndo_fix_features we would just turn off feature flags rather than return an error? For example: > > if (!(data & NETIF_F_RXCSUM)) > data &= ~NETIF_F_LRO; Exactly. > As for RSS_ENABLED/RXHASH check it was to cover the cases where RSS_ENABLED might have changed since probe. This could happen with a resume were we don't get enough MSI-X vectors. There are also paths in FCoE and DCB that get into code that could clear IXGBE_FLAG_RSS_ENABLED. That makes sense now. The checks should be in ndo_fix_features and clear features whenever they are unavailable. > >> + > >> + /* if state changes we need to update adapter->flags and reset */ > >> + if ((adapter->flags2 & IXGBE_FLAG2_RSC_CAPABLE) && > >> + (!!(data & NETIF_F_LRO) != > >> + !!(adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED))) { ndo_fix_features() should prevent the change if !(adapter->flags2 & IXGBE_FLAG2_RSC_CAPABLE) is true. > >> + if ((data & NETIF_F_LRO) && > >> + adapter->rx_itr_setting != 1 && > >> + adapter->rx_itr_setting > IXGBE_MAX_RSC_INT_RATE) { > >> + e_info(probe, "rx-usecs set too low, " > >> + "not enabling RSC\n"); And should clear NETIF_F_LRO when above condition is met. > >> + } else { > >> + adapter->flags2 ^= IXGBE_FLAG2_RSC_ENABLED; > >> + switch (adapter->hw.mac.type) { > >> + case ixgbe_mac_X540: > >> + case ixgbe_mac_82599EB: > >> + need_reset = true; > >> + break; > >> + default: > >> + break; > >> + } > >> + } > >> + } > >> + > >> + /* > >> + * Check if Flow Director n-tuple support was enabled or disabled. > >If > >> + * the state changed, we need to reset. > >> + */ > >> + if (!(adapter->flags & IXGBE_FLAG_FDIR_PERFECT_CAPABLE)) { > >> + /* turn off ATR, enable perfect filters and reset */ > >> + if (data & NETIF_F_NTUPLE) { > >> + adapter->flags &= ~IXGBE_FLAG_FDIR_HASH_CAPABLE; > >> + adapter->flags |= IXGBE_FLAG_FDIR_PERFECT_CAPABLE; > >> + need_reset = true; > >> + } > >> + } else if (!(data & NETIF_F_NTUPLE)) { > >> + /* turn off Flow Director, set ATR and reset */ > >> + adapter->flags &= ~IXGBE_FLAG_FDIR_PERFECT_CAPABLE; > >> + if ((adapter->flags & IXGBE_FLAG_RSS_ENABLED) && > >> + !(adapter->flags & IXGBE_FLAG_DCB_ENABLED)) > >> + adapter->flags |= IXGBE_FLAG_FDIR_HASH_CAPABLE; > >> + need_reset = true; > >> + } > > > >Part of the checks above should be in ndo_fix_features, too. > >ndo_set_features > >should just enable what it has been passed. What ndo_fix_features > >callback > >returns is further limited by generic checks, and then (if the resulting > >set > >is different than current dev->features) ndo_set_features is called. > > I'm a little confused here. From your comments I get the idea that ndo_fix_features() just modifies and error checks our feature set. The result of this would then be just a change to the feature set (data variable in my case above). Is that a correct assumption? If so I'm confused as none of the two checks above change the feature set. They do change driver flags to indicate the new state and mark whether we need a reset. I don't believe we would want to do the reset until ndo_set_feature is called and if we broke up the setting of the driver flags into ndo_fix_features we would lose some state (i.e. if the IXGBE_FLAG2_RSC_ENABLED changed) that we need to decide if a reset is needed in ndo_set_features. > > Am I just missing something here? I see that this changes only driver internal state, so your right in putting this in ndo_set_features callback. Can you draw a map of the relevant bits and what state should result from them (a truth table if you will)? I have a hard time understanding the idea. This changes _CAPABLE bits depending on _ENABLED which adds to confusion. I would expect that: _CAPABLE are set whenever a config is possible, disregarding dependencies _ENABLED are set whenever a config is requested and possible But it looks like what I described is reversed or just wrong. Best Regards, Michał Mirosław -- 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: Michal Miroslaw [mailto:mirq-linux@rere.qmqm.pl] >Sent: Tuesday, July 12, 2011 3:10 AM >To: Skidmore, Donald C >Cc: Kirsher, Jeffrey T; davem@davemloft.net; netdev@vger.kernel.org; >gospo@redhat.com >Subject: Re: [net-next 5/5] ixgbe: convert to ndo_fix_features > >On Mon, Jul 11, 2011 at 04:53:57PM -0700, Skidmore, Donald C wrote: >> >-----Original Message----- >> >From: Michal Miroslaw [mailto:mirq-linux@rere.qmqm.pl] >> >Sent: Saturday, July 09, 2011 5:11 AM >> >To: Kirsher, Jeffrey T >> >Cc: davem@davemloft.net; Skidmore, Donald C; netdev@vger.kernel.org; >> >gospo@redhat.com >> >Subject: Re: [net-next 5/5] ixgbe: convert to ndo_fix_features >> > >> >On Sat, Jul 09, 2011 at 04:50:40AM -0700, Jeff Kirsher wrote: >> >> From: Don Skidmore <donald.c.skidmore@intel.com> >> >> >> >> Private rx_csum flags are now duplicate of netdev->features & >> >> NETIF_F_RXCSUM. We remove those duplicates and now use the >> >net_device_ops >> >> ndo_set_features. This was based on the original patch submitted >by >> >> Michal Miroslaw <mirq-linux@rere.qmqm.pl>. I also removed the >special >> >> case not requiring a reset for X540 hardware. It is needed just as >it >> >is >> >> in 82599 hardware. >> >[...] >> >> --- a/drivers/net/ixgbe/ixgbe_main.c >> >> +++ b/drivers/net/ixgbe/ixgbe_main.c >> >> @@ -7188,6 +7188,88 @@ int ixgbe_setup_tc(struct net_device *dev, >u8 >> >tc) >> >[...] >> >> +static int ixgbe_set_features(struct net_device *netdev, u32 data) >> >> +{ >> >> + struct ixgbe_adapter *adapter = netdev_priv(netdev); >> >> + bool need_reset = false; >> >> + >> >> +#ifdef CONFIG_DCB >> >> + if ((adapter->flags & IXGBE_FLAG_DCB_ENABLED) && >> >> + !(data & NETIF_F_HW_VLAN_RX)) >> >> + return -EINVAL; >> >> +#endif >> >> + /* return error if RXHASH is being enabled when RSS is not >> >supported */ >> >> + if (!(adapter->flags & IXGBE_FLAG_RSS_ENABLED) && >> >> + (data & NETIF_F_RXHASH)) >> >> + return -EINVAL; >> >> + >> >> + /* If Rx checksum is disabled, then RSC/LRO should also be >> >disabled */ >> >> + if (!(data & NETIF_F_RXCSUM)) { >> >> + data &= ~NETIF_F_LRO; >> >> + adapter->flags &= ~IXGBE_FLAG_RX_CSUM_ENABLED; >> >> + } else { >> >> + adapter->flags |= IXGBE_FLAG_RX_CSUM_ENABLED; >> >> + } >> > >> >The checks above should be done in ndo_fix_features callback. The >check >> >for >> >RSS_ENABLED for RXHASH is redundant, as the feature is removed in >> >probe() >> >in this case (see [MARK] below). >> Thanks for the comments Michal, it clears up a lot in my mind. :) >> >> So in ndo_fix_features we would just turn off feature flags rather >than return an error? For example: >> >> if (!(data & NETIF_F_RXCSUM)) >> data &= ~NETIF_F_LRO; > >Exactly. > >> As for RSS_ENABLED/RXHASH check it was to cover the cases where >RSS_ENABLED might have changed since probe. This could happen with a >resume were we don't get enough MSI-X vectors. There are also paths in >FCoE and DCB that get into code that could clear IXGBE_FLAG_RSS_ENABLED. > >That makes sense now. The checks should be in ndo_fix_features and clear >features whenever they are unavailable. > I'm seeing how this would work; most of my earlier checks should fall pretty naturally in to ndo_fix_feature. :) >> >> + >> >> + /* if state changes we need to update adapter->flags and >reset */ >> >> + if ((adapter->flags2 & IXGBE_FLAG2_RSC_CAPABLE) && >> >> + (!!(data & NETIF_F_LRO) != >> >> + !!(adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED))) { > >ndo_fix_features() should prevent the change if >!(adapter->flags2 & IXGBE_FLAG2_RSC_CAPABLE) is true. > The logic in this conditional is a little complex. I believe it would end up looking something like the following: in fix_features: /* Prevent change if not RSC capable or invalid ITR setting */ if (data & NETIF_F_LRO) if (!(adapter->flags2 & IXGBE_FLAG2_RSC_CAPABLE) { data &= ~NETIF_F_LRO; else if (!(adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED) && (adapter->rx_itr_setting != 1 && adapter->rx_itr_setting > IXGBE_MAX_RSC_INT_RATE)) { data &= ~NETIF_F_LRO; } } in set_feature: /* Make sure RSC matches LRO, reset if change */ if (!!(data & NETIF_F_LRO) != !!(adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED)) { adapter->flags2 ^= IXGBE_FLAG2_RSC_ENABLED; switch (adapter->hw.mac.type) { case ixgbe_mac_X540: case ixgbe_mac_82599EB: need_reset = true; break; default: break; } } >> >> + if ((data & NETIF_F_LRO) && >> >> + adapter->rx_itr_setting != 1 && >> >> + adapter->rx_itr_setting > IXGBE_MAX_RSC_INT_RATE) >{ >> >> + e_info(probe, "rx-usecs set too low, " >> >> + "not enabling RSC\n"); > >And should clear NETIF_F_LRO when above condition is met. > >> >> + } else { >> >> + adapter->flags2 ^= IXGBE_FLAG2_RSC_ENABLED; >> >> + switch (adapter->hw.mac.type) { >> >> + case ixgbe_mac_X540: >> >> + case ixgbe_mac_82599EB: >> >> + need_reset = true; >> >> + break; >> >> + default: >> >> + break; >> >> + } >> >> + } >> >> + } >> >> + >> >> + /* >> >> + * Check if Flow Director n-tuple support was enabled or >disabled. >> >If >> >> + * the state changed, we need to reset. >> >> + */ >> >> + if (!(adapter->flags & IXGBE_FLAG_FDIR_PERFECT_CAPABLE)) { >> >> + /* turn off ATR, enable perfect filters and reset */ >> >> + if (data & NETIF_F_NTUPLE) { >> >> + adapter->flags &= ~IXGBE_FLAG_FDIR_HASH_CAPABLE; >> >> + adapter->flags |= >IXGBE_FLAG_FDIR_PERFECT_CAPABLE; >> >> + need_reset = true; >> >> + } >> >> + } else if (!(data & NETIF_F_NTUPLE)) { >> >> + /* turn off Flow Director, set ATR and reset */ >> >> + adapter->flags &= ~IXGBE_FLAG_FDIR_PERFECT_CAPABLE; >> >> + if ((adapter->flags & IXGBE_FLAG_RSS_ENABLED) && >> >> + !(adapter->flags & IXGBE_FLAG_DCB_ENABLED)) >> >> + adapter->flags |= IXGBE_FLAG_FDIR_HASH_CAPABLE; >> >> + need_reset = true; >> >> + } >> > >> >Part of the checks above should be in ndo_fix_features, too. >> >ndo_set_features >> >should just enable what it has been passed. What ndo_fix_features >> >callback >> >returns is further limited by generic checks, and then (if the >resulting >> >set >> >is different than current dev->features) ndo_set_features is called. >> >> I'm a little confused here. From your comments I get the idea that >ndo_fix_features() just modifies and error checks our feature set. The >result of this would then be just a change to the feature set (data >variable in my case above). Is that a correct assumption? If so I'm >confused as none of the two checks above change the feature set. They >do change driver flags to indicate the new state and mark whether we >need a reset. I don't believe we would want to do the reset until >ndo_set_feature is called and if we broke up the setting of the driver >flags into ndo_fix_features we would lose some state (i.e. if the >IXGBE_FLAG2_RSC_ENABLED changed) that we need to decide if a reset is >needed in ndo_set_features. >> >> Am I just missing something here? > >I see that this changes only driver internal state, so your right in >putting >this in ndo_set_features callback. > >Can you draw a map of the relevant bits and what state should result >from >them (a truth table if you will)? I have a hard time understanding the >idea. >This changes _CAPABLE bits depending on _ENABLED which adds to >confusion. > >I would expect that: > _CAPABLE are set whenever a config is possible, disregarding >dependencies > _ENABLED are set whenever a config is requested and possible >But it looks like what I described is reversed or just wrong. > >Best Regards, >Michał Mirosław Hopefully the code example above answers your questions. I can see why you were having a hard time understanding it; I was making the FALSE assumption and not clearing the feature flags for the more complex conditionals. Thanks again for the clarifications, the netdev-features.txt helped a lot too. Assuming I'm on the right track now I'll code up a new patch and send it out after our validation runs. Thanks, -Don Skidmore <donald.c.skidmore@intel.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
Good to see the idea converging. :) More hints below. On Tue, Jul 12, 2011 at 02:28:15PM -0700, Skidmore, Donald C wrote: [...] > The logic in this conditional is a little complex. I believe it would > end up looking something like the following: > > in fix_features: > > /* Prevent change if not RSC capable or invalid ITR setting */ > if (data & NETIF_F_LRO) That's not a fast path - you can drop (data & NETIF_F_LRO) check. > if (!(adapter->flags2 & IXGBE_FLAG2_RSC_CAPABLE) { > data &= ~NETIF_F_LRO; > else if (!(adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED) && > (adapter->rx_itr_setting != 1 && > adapter->rx_itr_setting > IXGBE_MAX_RSC_INT_RATE)) { > data &= ~NETIF_F_LRO; > } > } Same for !(adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED). After this change, you might add a call to netdev_update_features() whenever the second part of the condition is changed by user and you get correctness all the time 'for free'. > in set_feature: > > /* Make sure RSC matches LRO, reset if change */ > if (!!(data & NETIF_F_LRO) != > !!(adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED)) { > adapter->flags2 ^= IXGBE_FLAG2_RSC_ENABLED; > switch (adapter->hw.mac.type) { > case ixgbe_mac_X540: > case ixgbe_mac_82599EB: > need_reset = true; > break; > default: > break; > } > } Correct. Why would you need IXGBE_FLAG2_RSC_ENABLED now, when it just mirrors NETIF_F_LRO? [...] > Thanks again for the clarifications, the netdev-features.txt helped a lot too. Nice to hear (read) that! Best Regards, Michał Mirosław -- 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: Michal Miroslaw [mailto:mirq-linux@rere.qmqm.pl] >Sent: Tuesday, July 12, 2011 2:45 PM >To: Skidmore, Donald C >Cc: Kirsher, Jeffrey T; davem@davemloft.net; netdev@vger.kernel.org; >gospo@redhat.com >Subject: Re: [net-next 5/5] ixgbe: convert to ndo_fix_features > >Good to see the idea converging. :) More hints below. > >On Tue, Jul 12, 2011 at 02:28:15PM -0700, Skidmore, Donald C wrote: >[...] >> The logic in this conditional is a little complex. I believe it would >> end up looking something like the following: >> >> in fix_features: >> >> /* Prevent change if not RSC capable or invalid ITR setting */ >> if (data & NETIF_F_LRO) > >That's not a fast path - you can drop (data & NETIF_F_LRO) check. Nice - I was spending too much time looking at the old code didn't notice this. > >> if (!(adapter->flags2 & IXGBE_FLAG2_RSC_CAPABLE) { >> data &= ~NETIF_F_LRO; >> else if (!(adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED) && >> (adapter->rx_itr_setting != 1 && >> adapter->rx_itr_setting > IXGBE_MAX_RSC_INT_RATE)) >{ >> data &= ~NETIF_F_LRO; >> } >> } > >Same for !(adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED). After this >change, >you might add a call to netdev_update_features() whenever the second >part >of the condition is changed by user and you get correctness all the time >'for free'. I like this too and makes much more sense to me. This was a reflection of the original code but really didn't matter since anytime RSC_ENABLED was set the ITR values would have been correct anyway. > >> in set_feature: >> >> /* Make sure RSC matches LRO, reset if change */ >> if (!!(data & NETIF_F_LRO) != >> !!(adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED)) { >> adapter->flags2 ^= IXGBE_FLAG2_RSC_ENABLED; >> switch (adapter->hw.mac.type) { >> case ixgbe_mac_X540: >> case ixgbe_mac_82599EB: >> need_reset = true; >> break; >> default: >> break; >> } >> } > >Correct. Why would you need IXGBE_FLAG2_RSC_ENABLED now, when it just >mirrors NETIF_F_LRO? > We currently have a lot of checks for the IXGBE_FLAG2_RSC_ENABLED flag all over the code. I bet your next question is why not just using NETIF_F_LRO. :) Well this same code base is used in our source forge driver that has to be backward compatible with older kernels. It might be possible to do the kcompat magic to make it all work out but it won't be trivial and I didn't want to include it in this patch. I had planned on looking at it in the future to see if it was possible/worth doing. >[...] >> Thanks again for the clarifications, the netdev-features.txt helped a >lot too. > >Nice to hear (read) that! > >Best Regards, >Michał Mirosław I'll try to get a patch out soon. Thanks again, -Don Skidmore <donald.c.skidmore@intel.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
diff --git a/drivers/net/ixgbe/ixgbe.h b/drivers/net/ixgbe/ixgbe.h index 744b641..2b96d8d 100644 --- a/drivers/net/ixgbe/ixgbe.h +++ b/drivers/net/ixgbe/ixgbe.h @@ -588,6 +588,7 @@ extern void ixgbe_clear_rscctl(struct ixgbe_adapter *adapter, extern void ixgbe_set_rx_mode(struct net_device *netdev); extern int ixgbe_setup_tc(struct net_device *dev, u8 tc); extern void ixgbe_tx_ctxtdesc(struct ixgbe_ring *, u32, u32, u32, u32); +extern void ixgbe_do_reset(struct net_device *netdev); #ifdef IXGBE_FCOE extern void ixgbe_configure_fcoe(struct ixgbe_adapter *adapter); extern int ixgbe_fso(struct ixgbe_ring *tx_ring, struct sk_buff *skb, diff --git a/drivers/net/ixgbe/ixgbe_ethtool.c b/drivers/net/ixgbe/ixgbe_ethtool.c index 074e9ba..06b7f88 100644 --- a/drivers/net/ixgbe/ixgbe_ethtool.c +++ b/drivers/net/ixgbe/ixgbe_ethtool.c @@ -442,109 +442,6 @@ static int ixgbe_set_pauseparam(struct net_device *netdev, return 0; } -static void ixgbe_do_reset(struct net_device *netdev) -{ - struct ixgbe_adapter *adapter = netdev_priv(netdev); - - if (netif_running(netdev)) - ixgbe_reinit_locked(adapter); - else - ixgbe_reset(adapter); -} - -static u32 ixgbe_get_rx_csum(struct net_device *netdev) -{ - struct ixgbe_adapter *adapter = netdev_priv(netdev); - return adapter->flags & IXGBE_FLAG_RX_CSUM_ENABLED; -} - -static void ixgbe_set_rsc(struct ixgbe_adapter *adapter) -{ - int i; - - for (i = 0; i < adapter->num_rx_queues; i++) { - struct ixgbe_ring *ring = adapter->rx_ring[i]; - if (adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED) { - set_ring_rsc_enabled(ring); - ixgbe_configure_rscctl(adapter, ring); - } else { - ixgbe_clear_rscctl(adapter, ring); - } - } -} - -static int ixgbe_set_rx_csum(struct net_device *netdev, u32 data) -{ - struct ixgbe_adapter *adapter = netdev_priv(netdev); - bool need_reset = false; - - if (data) { - adapter->flags |= IXGBE_FLAG_RX_CSUM_ENABLED; - } else { - adapter->flags &= ~IXGBE_FLAG_RX_CSUM_ENABLED; - - if (adapter->flags2 & IXGBE_FLAG2_RSC_CAPABLE) { - adapter->flags2 &= ~IXGBE_FLAG2_RSC_ENABLED; - netdev->features &= ~NETIF_F_LRO; - } - - switch (adapter->hw.mac.type) { - case ixgbe_mac_X540: - ixgbe_set_rsc(adapter); - break; - case ixgbe_mac_82599EB: - need_reset = true; - break; - default: - break; - } - } - - if (need_reset) - ixgbe_do_reset(netdev); - - return 0; -} - -static u32 ixgbe_get_tx_csum(struct net_device *netdev) -{ - return (netdev->features & NETIF_F_IP_CSUM) != 0; -} - -static int ixgbe_set_tx_csum(struct net_device *netdev, u32 data) -{ - struct ixgbe_adapter *adapter = netdev_priv(netdev); - u32 feature_list; - - feature_list = (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM); - switch (adapter->hw.mac.type) { - case ixgbe_mac_82599EB: - case ixgbe_mac_X540: - feature_list |= NETIF_F_SCTP_CSUM; - break; - default: - break; - } - if (data) - netdev->features |= feature_list; - else - netdev->features &= ~feature_list; - - return 0; -} - -static int ixgbe_set_tso(struct net_device *netdev, u32 data) -{ - if (data) { - netdev->features |= NETIF_F_TSO; - netdev->features |= NETIF_F_TSO6; - } else { - netdev->features &= ~NETIF_F_TSO; - netdev->features &= ~NETIF_F_TSO6; - } - return 0; -} - static u32 ixgbe_get_msglevel(struct net_device *netdev) { struct ixgbe_adapter *adapter = netdev_priv(netdev); @@ -2287,81 +2184,6 @@ static int ixgbe_set_coalesce(struct net_device *netdev, return 0; } -static int ixgbe_set_flags(struct net_device *netdev, u32 data) -{ - struct ixgbe_adapter *adapter = netdev_priv(netdev); - bool need_reset = false; - int rc; - -#ifdef CONFIG_IXGBE_DCB - if ((adapter->flags & IXGBE_FLAG_DCB_ENABLED) && - !(data & ETH_FLAG_RXVLAN)) - return -EINVAL; -#endif - - need_reset = (data & ETH_FLAG_RXVLAN) != - (netdev->features & NETIF_F_HW_VLAN_RX); - - if ((data & ETH_FLAG_RXHASH) && - !(adapter->flags & IXGBE_FLAG_RSS_ENABLED)) - return -EOPNOTSUPP; - - rc = ethtool_op_set_flags(netdev, data, ETH_FLAG_LRO | ETH_FLAG_NTUPLE | - ETH_FLAG_RXVLAN | ETH_FLAG_TXVLAN | - ETH_FLAG_RXHASH); - if (rc) - return rc; - - /* if state changes we need to update adapter->flags and reset */ - if ((adapter->flags2 & IXGBE_FLAG2_RSC_CAPABLE) && - (!!(data & ETH_FLAG_LRO) != - !!(adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED))) { - if ((data & ETH_FLAG_LRO) && - (!adapter->rx_itr_setting || - (adapter->rx_itr_setting > IXGBE_MAX_RSC_INT_RATE))) { - e_info(probe, "rx-usecs set too low, " - "not enabling RSC.\n"); - } else { - adapter->flags2 ^= IXGBE_FLAG2_RSC_ENABLED; - switch (adapter->hw.mac.type) { - case ixgbe_mac_X540: - ixgbe_set_rsc(adapter); - break; - case ixgbe_mac_82599EB: - need_reset = true; - break; - default: - break; - } - } - } - - /* - * Check if Flow Director n-tuple support was enabled or disabled. If - * the state changed, we need to reset. - */ - if (!(adapter->flags & IXGBE_FLAG_FDIR_PERFECT_CAPABLE)) { - /* turn off ATR, enable perfect filters and reset */ - if (data & ETH_FLAG_NTUPLE) { - adapter->flags &= ~IXGBE_FLAG_FDIR_HASH_CAPABLE; - adapter->flags |= IXGBE_FLAG_FDIR_PERFECT_CAPABLE; - need_reset = true; - } - } else if (!(data & ETH_FLAG_NTUPLE)) { - /* turn off Flow Director, set ATR and reset */ - adapter->flags &= ~IXGBE_FLAG_FDIR_PERFECT_CAPABLE; - if ((adapter->flags & IXGBE_FLAG_RSS_ENABLED) && - !(adapter->flags & IXGBE_FLAG_DCB_ENABLED)) - adapter->flags |= IXGBE_FLAG_FDIR_HASH_CAPABLE; - need_reset = true; - } - - if (need_reset) - ixgbe_do_reset(netdev); - - return 0; -} - static int ixgbe_get_ethtool_fdir_entry(struct ixgbe_adapter *adapter, struct ethtool_rxnfc *cmd) { @@ -2744,16 +2566,8 @@ static const struct ethtool_ops ixgbe_ethtool_ops = { .set_ringparam = ixgbe_set_ringparam, .get_pauseparam = ixgbe_get_pauseparam, .set_pauseparam = ixgbe_set_pauseparam, - .get_rx_csum = ixgbe_get_rx_csum, - .set_rx_csum = ixgbe_set_rx_csum, - .get_tx_csum = ixgbe_get_tx_csum, - .set_tx_csum = ixgbe_set_tx_csum, - .get_sg = ethtool_op_get_sg, - .set_sg = ethtool_op_set_sg, .get_msglevel = ixgbe_get_msglevel, .set_msglevel = ixgbe_set_msglevel, - .get_tso = ethtool_op_get_tso, - .set_tso = ixgbe_set_tso, .self_test = ixgbe_diag_test, .get_strings = ixgbe_get_strings, .set_phys_id = ixgbe_set_phys_id, @@ -2761,8 +2575,6 @@ static const struct ethtool_ops ixgbe_ethtool_ops = { .get_ethtool_stats = ixgbe_get_ethtool_stats, .get_coalesce = ixgbe_get_coalesce, .set_coalesce = ixgbe_set_coalesce, - .get_flags = ethtool_op_get_flags, - .set_flags = ixgbe_set_flags, .get_rxnfc = ixgbe_get_rxnfc, .set_rxnfc = ixgbe_set_rxnfc, }; diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c index de30796..206c069 100644 --- a/drivers/net/ixgbe/ixgbe_main.c +++ b/drivers/net/ixgbe/ixgbe_main.c @@ -7188,6 +7188,88 @@ int ixgbe_setup_tc(struct net_device *dev, u8 tc) return 0; } +void ixgbe_do_reset(struct net_device *netdev) +{ + struct ixgbe_adapter *adapter = netdev_priv(netdev); + + if (netif_running(netdev)) + ixgbe_reinit_locked(adapter); + else + ixgbe_reset(adapter); +} + +static int ixgbe_set_features(struct net_device *netdev, u32 data) +{ + struct ixgbe_adapter *adapter = netdev_priv(netdev); + bool need_reset = false; + +#ifdef CONFIG_DCB + if ((adapter->flags & IXGBE_FLAG_DCB_ENABLED) && + !(data & NETIF_F_HW_VLAN_RX)) + return -EINVAL; +#endif + /* return error if RXHASH is being enabled when RSS is not supported */ + if (!(adapter->flags & IXGBE_FLAG_RSS_ENABLED) && + (data & NETIF_F_RXHASH)) + return -EINVAL; + + /* If Rx checksum is disabled, then RSC/LRO should also be disabled */ + if (!(data & NETIF_F_RXCSUM)) { + data &= ~NETIF_F_LRO; + adapter->flags &= ~IXGBE_FLAG_RX_CSUM_ENABLED; + } else { + adapter->flags |= IXGBE_FLAG_RX_CSUM_ENABLED; + } + + /* if state changes we need to update adapter->flags and reset */ + if ((adapter->flags2 & IXGBE_FLAG2_RSC_CAPABLE) && + (!!(data & NETIF_F_LRO) != + !!(adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED))) { + if ((data & NETIF_F_LRO) && + adapter->rx_itr_setting != 1 && + adapter->rx_itr_setting > IXGBE_MAX_RSC_INT_RATE) { + e_info(probe, "rx-usecs set too low, " + "not enabling RSC\n"); + } else { + adapter->flags2 ^= IXGBE_FLAG2_RSC_ENABLED; + switch (adapter->hw.mac.type) { + case ixgbe_mac_X540: + case ixgbe_mac_82599EB: + need_reset = true; + break; + default: + break; + } + } + } + + /* + * Check if Flow Director n-tuple support was enabled or disabled. If + * the state changed, we need to reset. + */ + if (!(adapter->flags & IXGBE_FLAG_FDIR_PERFECT_CAPABLE)) { + /* turn off ATR, enable perfect filters and reset */ + if (data & NETIF_F_NTUPLE) { + adapter->flags &= ~IXGBE_FLAG_FDIR_HASH_CAPABLE; + adapter->flags |= IXGBE_FLAG_FDIR_PERFECT_CAPABLE; + need_reset = true; + } + } else if (!(data & NETIF_F_NTUPLE)) { + /* turn off Flow Director, set ATR and reset */ + adapter->flags &= ~IXGBE_FLAG_FDIR_PERFECT_CAPABLE; + if ((adapter->flags & IXGBE_FLAG_RSS_ENABLED) && + !(adapter->flags & IXGBE_FLAG_DCB_ENABLED)) + adapter->flags |= IXGBE_FLAG_FDIR_HASH_CAPABLE; + need_reset = true; + } + + if (need_reset) + ixgbe_do_reset(netdev); + + return 0; + +} + static const struct net_device_ops ixgbe_netdev_ops = { .ndo_open = ixgbe_open, .ndo_stop = ixgbe_close, @@ -7219,6 +7301,7 @@ static const struct net_device_ops ixgbe_netdev_ops = { .ndo_fcoe_disable = ixgbe_fcoe_disable, .ndo_fcoe_get_wwn = ixgbe_fcoe_get_wwn, #endif /* IXGBE_FCOE */ + .ndo_set_features = ixgbe_set_features, }; static void __devinit ixgbe_probe_vf(struct ixgbe_adapter *adapter, @@ -7486,20 +7569,24 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev, netdev->features = NETIF_F_SG | NETIF_F_IP_CSUM | + NETIF_F_IPV6_CSUM | NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX | - NETIF_F_HW_VLAN_FILTER; + NETIF_F_HW_VLAN_FILTER | + NETIF_F_TSO | + NETIF_F_TSO6 | + NETIF_F_GRO | + NETIF_F_RXHASH | + NETIF_F_RXCSUM; - netdev->features |= NETIF_F_IPV6_CSUM; - netdev->features |= NETIF_F_TSO; - netdev->features |= NETIF_F_TSO6; - netdev->features |= NETIF_F_GRO; - netdev->features |= NETIF_F_RXHASH; + netdev->hw_features = netdev->features; switch (adapter->hw.mac.type) { case ixgbe_mac_82599EB: case ixgbe_mac_X540: netdev->features |= NETIF_F_SCTP_CSUM; + netdev->hw_features |= NETIF_F_SCTP_CSUM | + NETIF_F_NTUPLE; break; default: break; @@ -7538,6 +7625,8 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev, netdev->vlan_features |= NETIF_F_HIGHDMA; } + if (adapter->flags2 & IXGBE_FLAG2_RSC_CAPABLE) + netdev->hw_features |= NETIF_F_LRO; if (adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED) netdev->features |= NETIF_F_LRO; @@ -7574,8 +7663,10 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev, if (err) goto err_sw_init; - if (!(adapter->flags & IXGBE_FLAG_RSS_ENABLED)) + if (!(adapter->flags & IXGBE_FLAG_RSS_ENABLED)) { + netdev->hw_features &= ~NETIF_F_RXHASH; netdev->features &= ~NETIF_F_RXHASH; + } switch (pdev->device) { case IXGBE_DEV_ID_82599_SFP: