| Message ID | 20110707164000.6f3f58f4@nehalam.ftrdhcpuser.net |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
2011/7/8 Stephen Hemminger <shemminger@vyatta.com>: > Found when reviewing the vendor driver. Apparently some chip versions > require receive checksumming to be enabled in order for RSS to work. > > Also, if fix_features has to change some settings; put in message > in log in similar manner to netdev_fix_features. > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> > > --- > v2 - enforce the requirement in fix_features rather than set features > > --- a/drivers/net/sky2.c 2011-07-07 13:56:03.575420273 -0700 > +++ b/drivers/net/sky2.c 2011-07-07 14:03:17.775403938 -0700 [...] > @@ -4176,8 +4180,18 @@ static u32 sky2_fix_features(struct net_ > /* In order to do Jumbo packets on these chips, need to turn off the > * transmit store/forward. Therefore checksum offload won't work. > */ > - if (dev->mtu > ETH_DATA_LEN && hw->chip_id == CHIP_ID_YUKON_EC_U) > + if (dev->mtu > ETH_DATA_LEN && hw->chip_id == CHIP_ID_YUKON_EC_U) { > + netdev_warn(dev, "checksum offload not possible with jumbo frames\n"); > features &= ~(NETIF_F_TSO|NETIF_F_SG|NETIF_F_ALL_CSUM); > + } > + > + /* Some hardware requires receive checksum for RSS to work. */ > + if ( (features & NETIF_F_RXHASH) && > + !(features & NETIF_F_RXCSUM) && > + (sky2->hw->flags & SKY2_HW_RSS_CHKSUM)) { > + netdev_warn(dev, "receive hashing forces receive checksum\n"); > + features |= NETIF_F_RXCSUM; > + } > > return features; > } I suggest using netdev_dbg() instead of netdev_warn() so it won't spam dmesg on every features update. Otherwise, looks good. 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 Fri, 8 Jul 2011 10:45:33 +0200 Michał Mirosław <mirqus@gmail.com> wrote: > 2011/7/8 Stephen Hemminger <shemminger@vyatta.com>: > > Found when reviewing the vendor driver. Apparently some chip versions > > require receive checksumming to be enabled in order for RSS to work. > > > > Also, if fix_features has to change some settings; put in message > > in log in similar manner to netdev_fix_features. > > > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> > > > > --- > > v2 - enforce the requirement in fix_features rather than set features > > > > --- a/drivers/net/sky2.c 2011-07-07 13:56:03.575420273 -0700 > > +++ b/drivers/net/sky2.c 2011-07-07 14:03:17.775403938 -0700 > [...] > > @@ -4176,8 +4180,18 @@ static u32 sky2_fix_features(struct net_ > > /* In order to do Jumbo packets on these chips, need to turn off the > > * transmit store/forward. Therefore checksum offload won't work. > > */ > > - if (dev->mtu > ETH_DATA_LEN && hw->chip_id == CHIP_ID_YUKON_EC_U) > > + if (dev->mtu > ETH_DATA_LEN && hw->chip_id == CHIP_ID_YUKON_EC_U) { > > + netdev_warn(dev, "checksum offload not possible with jumbo frames\n"); > > features &= ~(NETIF_F_TSO|NETIF_F_SG|NETIF_F_ALL_CSUM); > > + } > > + > > + /* Some hardware requires receive checksum for RSS to work. */ > > + if ( (features & NETIF_F_RXHASH) && > > + !(features & NETIF_F_RXCSUM) && > > + (sky2->hw->flags & SKY2_HW_RSS_CHKSUM)) { > > + netdev_warn(dev, "receive hashing forces receive checksum\n"); > > + features |= NETIF_F_RXCSUM; > > + } > > > > return features; > > } > > I suggest using netdev_dbg() instead of netdev_warn() so it won't spam > dmesg on every features update. I was just copying existing policy in netdev_fix_features. netdev_dbg() is useless because most user turn off debug messages. Maybe notice or info is more appropriate level 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
From: Stephen Hemminger <shemminger@vyatta.com> Date: Thu, 7 Jul 2011 16:40:00 -0700 > Found when reviewing the vendor driver. Apparently some chip versions > require receive checksumming to be enabled in order for RSS to work. > > Also, if fix_features has to change some settings; put in message > in log in similar manner to netdev_fix_features. > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> > > --- > v2 - enforce the requirement in fix_features rather than set features All 5 patches applied, I changed this patch #1 to use netdev_info() as per the discussion. -- 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
--- a/drivers/net/sky2.c 2011-07-07 13:56:03.575420273 -0700 +++ b/drivers/net/sky2.c 2011-07-07 14:03:17.775403938 -0700 @@ -2996,7 +2996,8 @@ static int __devinit sky2_init(struct sk hw->flags = SKY2_HW_GIGABIT | SKY2_HW_NEWER_PHY | SKY2_HW_NEW_LE - | SKY2_HW_ADV_POWER_CTL; + | SKY2_HW_ADV_POWER_CTL + | SKY2_HW_RSS_CHKSUM; /* New transmit checksum */ if (hw->chip_rev != CHIP_REV_YU_EX_B0) @@ -3024,7 +3025,7 @@ static int __devinit sky2_init(struct sk /* The workaround for status conflicts VLAN tag detection. */ if (hw->chip_rev == CHIP_REV_YU_FE2_A0) - hw->flags |= SKY2_HW_VLAN_BROKEN; + hw->flags |= SKY2_HW_VLAN_BROKEN | SKY2_HW_RSS_CHKSUM; break; case CHIP_ID_YUKON_SUPR: @@ -3033,6 +3034,9 @@ static int __devinit sky2_init(struct sk | SKY2_HW_NEW_LE | SKY2_HW_AUTO_TX_SUM | SKY2_HW_ADV_POWER_CTL; + + if (hw->chip_rev == CHIP_REV_YU_SU_A0) + hw->flags |= SKY2_HW_RSS_CHKSUM; break; case CHIP_ID_YUKON_UL_2: @@ -4176,8 +4180,18 @@ static u32 sky2_fix_features(struct net_ /* In order to do Jumbo packets on these chips, need to turn off the * transmit store/forward. Therefore checksum offload won't work. */ - if (dev->mtu > ETH_DATA_LEN && hw->chip_id == CHIP_ID_YUKON_EC_U) + if (dev->mtu > ETH_DATA_LEN && hw->chip_id == CHIP_ID_YUKON_EC_U) { + netdev_warn(dev, "checksum offload not possible with jumbo frames\n"); features &= ~(NETIF_F_TSO|NETIF_F_SG|NETIF_F_ALL_CSUM); + } + + /* Some hardware requires receive checksum for RSS to work. */ + if ( (features & NETIF_F_RXHASH) && + !(features & NETIF_F_RXCSUM) && + (sky2->hw->flags & SKY2_HW_RSS_CHKSUM)) { + netdev_warn(dev, "receive hashing forces receive checksum\n"); + features |= NETIF_F_RXCSUM; + } return features; } --- a/drivers/net/sky2.h 2011-07-07 13:56:03.563420272 -0700 +++ b/drivers/net/sky2.h 2011-07-07 13:56:07.719420117 -0700 @@ -2281,6 +2281,7 @@ struct sky2_hw { #define SKY2_HW_ADV_POWER_CTL 0x00000080 /* additional PHY power regs */ #define SKY2_HW_RSS_BROKEN 0x00000100 #define SKY2_HW_VLAN_BROKEN 0x00000200 +#define SKY2_HW_RSS_CHKSUM 0x00000400 /* RSS requires chksum */ u8 chip_id; u8 chip_rev;
Found when reviewing the vendor driver. Apparently some chip versions require receive checksumming to be enabled in order for RSS to work. Also, if fix_features has to change some settings; put in message in log in similar manner to netdev_fix_features. Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> --- v2 - enforce the requirement in fix_features rather than set features -- 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