Re: [net-next 10/10] ixgbe: convert to ndo_fix_features
flat view
From: Michal Miroslaw <mirq-linux@rere.qmqm.pl>
Date: 2011-07-22 08:02:58
On Thu, Jul 21, 2011 at 11:09:11PM -0700, Jeff Kirsher wrote:
From: Don Skidmore <redacted> 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 [off-list ref]. I also removed the special case not requiring a reset for X540 hardware. It is needed just as it is in 82599 hardware.
Looks mostly good now. Minor hints below. [...]
+static u32 ixgbe_fix_features(struct net_device *netdev, u32 data)
+{[...]
quoted hunk ↗ jump to hunk
+ /* Turn off LRO if not RSC capable or invalid ITR settings */ + 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; + e_info(probe, "rx-usecs set too low, not enabling RSC\n"); + }
Better:
... else if (data & NETIF_F_LRO && adapter->rx_itr_setting != 1 && adapter->rx_itr_setting > IXGBE_MAX_RSC_INT_RATE) {
e_info(...)
data &= ~NETIF_F_LRO;
}
quoted hunk ↗ jump to hunk
+ + return data; +} + +static int ixgbe_set_features(struct net_device *netdev, u32 data) +{ + struct ixgbe_adapter *adapter = netdev_priv(netdev); + bool need_reset = false; + + /* If Rx checksum is disabled, then RSC/LRO should also be disabled */ + if (!(data & NETIF_F_RXCSUM)) + adapter->flags &= ~IXGBE_FLAG_RX_CSUM_ENABLED; + else + adapter->flags |= IXGBE_FLAG_RX_CSUM_ENABLED;
This exactly mirrors NETIF_F_RXCSUM. Waiting for later cleanup? [...]
quoted hunk ↗ jump to hunk
+ /* + * 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; + }
You could make this more readable:
old = adapter->flags;
if (data & NETIF_F_NTUPLE) {
adapter->flags |= IXGBE_FLAG_FDIR_PERFECT_CAPABLE;
adapter->flags &= ~IXGBE_FLAG_FDIR_HASH_CAPABLE;
} else {
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;
}
if (old != adapter->flags)
need_reset = true;
quoted hunk ↗ jump to hunk
+ + 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,@@ -7153,6 +7228,8 @@ 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, + .ndo_fix_features = ixgbe_fix_features, }; static void __devinit ixgbe_probe_vf(struct ixgbe_adapter *adapter,@@ -7420,20 +7497,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;
Drop NETIF_F_GRO here, as its always set by network core now.
quoted hunk ↗ jump to hunk
- 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;
NTUPLE disabled by default. That's the idea? Best Regards, Michał Mirosław