Re: [PATCH 3/4] Ethtool: convert get_tso/set_tso calls to hw_features flags
From: Matt Carlson <hidden>
Date: 2010-11-02 02:50:05
On Sat, Oct 30, 2010 at 01:44:17AM -0700, Micha?? Miros??aw wrote:
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c index b07e2d1..c08172d 100644 --- a/drivers/net/tg3.c +++ b/drivers/net/tg3.c@@ -6142,7 +6142,7 @@ static inline void tg3_set_mtu(struct net_device *dev, struct tg3 *tp, if (new_mtu > ETH_DATA_LEN) { if (tp->tg3_flags2 & TG3_FLG2_5780_CLASS) { tp->tg3_flags2 &= ~TG3_FLG2_TSO_CAPABLE; - ethtool_op_set_tso(dev, 0); + dev->features &= ~NETIF_F_ALL_TSO; } else { tp->tg3_flags |= TG3_FLAG_JUMBO_RING_ENABLE; }@@ -9977,27 +9977,28 @@ static int tg3_set_tso(struct net_device *dev, u32 value) { struct tg3 *tp = netdev_priv(dev); - if (!(tp->tg3_flags2 & TG3_FLG2_TSO_CAPABLE)) { - if (value) - return -EINVAL; + if (!(tp->tg3_flags2 & TG3_FLG2_TSO_CAPABLE) && value) + return -EINVAL; + + if (!value) return 0; - } + + dev->features |= NETIF_F_TSO; + if ((dev->features & NETIF_F_IPV6_CSUM) && ((tp->tg3_flags2 & TG3_FLG2_HW_TSO_2) || (tp->tg3_flags2 & TG3_FLG2_HW_TSO_3))) { - if (value) { - dev->features |= NETIF_F_TSO6; - if ((tp->tg3_flags2 & TG3_FLG2_HW_TSO_3) || - GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5761 || - (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5784 && - GET_CHIP_REV(tp->pci_chip_rev_id) != CHIPREV_5784_AX) || - GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5785 || - GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_57780) - dev->features |= NETIF_F_TSO_ECN; - } else - dev->features &= ~(NETIF_F_TSO6 | NETIF_F_TSO_ECN); + dev->features |= NETIF_F_TSO6; + if ((tp->tg3_flags2 & TG3_FLG2_HW_TSO_3) || + GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5761 || + (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5784 && + GET_CHIP_REV(tp->pci_chip_rev_id) != CHIPREV_5784_AX) || + GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5785 || + GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_57780) + dev->features |= NETIF_F_TSO_ECN; } - return ethtool_op_set_tso(dev, value); + + return 1;
dev->hw_features looks to me like it should function as a set of flags indicating what the hardware is currently capable of doing. It would clean the above code a lot if we could do: dev->features |= (dev->hw_features & NETIF_F_ALL_TSO); In fact, the new member might serve as a replacement for the TG3_FLG2_TSO_CAPABLE flag. Let's not do that right now though, because it requires some careful attention in other code paths which would be a distraction. I'll follow-up with a patch to do this.
quoted hunk ↗ jump to hunk
} static int tg3_nway_reset(struct net_device *dev)@@ -11306,7 +11307,7 @@ static const struct ethtool_ops tg3_ethtool_ops = { .get_rx_csum = tg3_get_rx_csum, .set_rx_csum = tg3_set_rx_csum, .set_tx_csum = tg3_set_tx_csum, - .set_tso = tg3_set_tso, + .hw_set_tso = tg3_set_tso, .self_test = tg3_self_test, .get_strings = tg3_get_strings, .phys_id = tg3_phys_id,@@ -14681,6 +14682,7 @@ static int __devinit tg3_init_one(struct pci_dev *pdev, tp->rx_jumbo_pending = TG3_DEF_RX_JUMBO_RING_PENDING; dev->hw_features |= NETIF_F_SG; + dev->hw_features |= NETIF_F_TSO|NETIF_F_TSO6|NETIF_F_TSO_ECN;
Not all devices are TSO capable either. There is code later in this function that determines which TSO flags to set. I would probably reuse that code.
dev->ethtool_ops = &tg3_ethtool_ops;
dev->watchdog_timeo = TG3_TX_TIMEOUT;
dev->irq = pdev->irq;