| Message ID | 4A131364.5050402@myri.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Brice Goglin <brice@myri.com> Date: Tue, 19 May 2009 22:15:32 +0200 > Allow myri10ge LRO to be enabled/disabled via ethtool > (and by the stack for packet forwarding). > > Signed-off-by: Brice Goglin <brice@myri.com> Please also get rid of the module option. It being there sets a bad precedent, and others are going to point at your driver when they try to add such a turd to their's. Thanks. -- 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: Brice Goglin <brice@myri.com> Date: Tue, 19 May 2009 22:15:32 +0200 > Allow myri10ge LRO to be enabled/disabled via ethtool > (and by the stack for packet forwarding). > > Signed-off-by: Brice Goglin <brice@myri.com> So I applied these two patches for now to net-next-2.6 but in the long term I think it's probably better to convert this driver to GRO as we're trying to get rid of LRO and GRO handles forwarding and all of that stuff transparently. And I also still want the myri10ge_lro module option removed regardless of whether you do a GRO conversion or not. If LRO/GRO causes a performance regression in some situation, we should fix it instead of papering over this junk with module options. Thanks. -- 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
--- net-next-2.6/drivers/net/myri10ge/myri10ge.c 2009-04-18 12:25:35.000000000 +0200 +++ linux-tmp/drivers/net/myri10ge/myri10ge.c 2009-05-19 22:02:12.000000000 +0200 @@ -1300,7 +1300,7 @@ remainder -= MYRI10GE_ALLOC_SIZE; } - if (mgp->csum_flag && myri10ge_lro) { + if (dev->features & NETIF_F_LRO) { rx_frags[0].page_offset += MXGEFW_PAD; rx_frags[0].size -= MXGEFW_PAD; len -= MXGEFW_PAD; @@ -1716,12 +1716,17 @@ static int myri10ge_set_rx_csum(struct net_device *netdev, u32 csum_enabled) { struct myri10ge_priv *mgp = netdev_priv(netdev); + int err = 0; if (csum_enabled) mgp->csum_flag = MXGEFW_FLAGS_CKSUM; - else + else { + u32 flags = ethtool_op_get_flags(netdev); + err = ethtool_op_set_flags(netdev, (flags & ~ETH_FLAG_LRO)); mgp->csum_flag = 0; - return 0; + + } + return err; } static int myri10ge_set_tso(struct net_device *netdev, u32 tso_enabled) @@ -1904,7 +1909,9 @@ .get_sset_count = myri10ge_get_sset_count, .get_ethtool_stats = myri10ge_get_ethtool_stats, .set_msglevel = myri10ge_set_msglevel, - .get_msglevel = myri10ge_get_msglevel + .get_msglevel = myri10ge_get_msglevel, + .get_flags = ethtool_op_get_flags, + .set_flags = ethtool_op_set_flags }; static int myri10ge_allocate_rings(struct myri10ge_slice_state *ss) @@ -3910,6 +3917,8 @@ if (dac_enabled) netdev->features |= NETIF_F_HIGHDMA; + if (myri10ge_lro) + netdev->features |= NETIF_F_LRO; /* make sure we can get an irq, and that MSI can be * setup (if available). Also ensure netdev->irq
Allow myri10ge LRO to be enabled/disabled via ethtool (and by the stack for packet forwarding). Signed-off-by: Brice Goglin <brice@myri.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