| Message ID | 20090415100937.4b08ce28@dhcp-lab-109.englab.brq.redhat.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Stanislaw Gruszka <sgruszka@redhat.com> Date: Wed, 15 Apr 2009 10:09:37 +0200 > Add LRO alignment initially committed in 621544eb8c3beaa859c75850f816dd9b056a00a3 > and removed in 0dcffac1a329be69bab0ac604bf7283737108e68 during conversion to > multi-slice. > > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com> Applied, thanks. Please, in the future, add the header string of the commit message when referencing GIT commits. When this patch is added to the -stable kernel or similar the GIT commit ideas might be different and it will be impossible for someone reading your commit message to find the referenced commit using only the SHA ID. I fixed that up for you this time. Also, it would great to get this driver converted over to GRO, such bugs like this one aren't even possible with GRO :-) -- 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
David Miller wrote: > From: Stanislaw Gruszka <sgruszka@redhat.com> > Date: Wed, 15 Apr 2009 10:09:37 +0200 > > >> Add LRO alignment initially committed in 621544eb8c3beaa859c75850f816dd9b056a00a3 >> and removed in 0dcffac1a329be69bab0ac604bf7283737108e68 during conversion to >> multi-slice. >> >> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com> >> > > Applied, thanks. > > Please, in the future, add the header string of the commit message > when referencing GIT commits. When this patch is added to the -stable > kernel or similar the GIT commit ideas might be different and it > will be impossible for someone reading your commit message to find > the referenced commit using only the SHA ID. > I guess we need to send this patch to the stable maintainers since it should affect 2.6.27, .28 and .29. > Also, it would great to get this driver converted over to GRO, > such bugs like this one aren't even possible with GRO :-) > It looks like nobody complains about GRO bugs or performance-problems anymore so we might indeed look at converting myri10ge for 2.6.31. Is there a good summary somewhere of why GRO is better, and how to actually convert drivers? Brice -- 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: Wed, 15 Apr 2009 11:48:06 +0200 > David Miller wrote: >> From: Stanislaw Gruszka <sgruszka@redhat.com> >> Date: Wed, 15 Apr 2009 10:09:37 +0200 >> >> >>> Add LRO alignment initially committed in 621544eb8c3beaa859c75850f816dd9b056a00a3 >>> and removed in 0dcffac1a329be69bab0ac604bf7283737108e68 during conversion to >>> multi-slice. >>> >>> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com> >>> >> >> Applied, thanks. >> >> Please, in the future, add the header string of the commit message >> when referencing GIT commits. When this patch is added to the -stable >> kernel or similar the GIT commit ideas might be different and it >> will be impossible for someone reading your commit message to find >> the referenced commit using only the SHA ID. >> > > I guess we need to send this patch to the stable maintainers since it > should affect 2.6.27, .28 and .29. I will queue it up for -stable, you just have to ask me to do that. > Is there a good summary somewhere of why GRO is better, Transparent forwarding/bridging support, easier driver port. > and how to > actually convert drivers? Step 1: Remove all of your LRO support code, every last line Step 2: netif_receive_skb() --> napi_gro_receive() vlan_hwaccel_rx() --> vlan_gro_receive() It couldn't be any easier. And it would also behoove you to look at the commits that converted or added GRO support to other drivers. That's how I learned it :-) -- 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
David Miller wrote: > From: Brice Goglin <brice@myri.com> >> Is there a good summary somewhere of why GRO is better, > > Transparent forwarding/bridging support, easier driver port. > >> and how to >> actually convert drivers? > > Step 1: Remove all of your LRO support code, every last line > Step 2: netif_receive_skb() --> napi_gro_receive() > vlan_hwaccel_rx() --> vlan_gro_receive() > > It couldn't be any easier. > > And it would also behoove you to look at the commits that converted or > added GRO support to other drivers. That's how I learned it :-) Unfortunately, it doesn't appear that GRO is able to handle frags (like lro_receive_frags()), so I anticipate its overhead would be much higher than LRO for us, due to extra memory allocation and freeing overheads. I'll try to find the time to convert the driver and run some quick tests to confirm. However, since LRO is optional, it would make sense to convert the non-LRO code path at the very least. Drew -- 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
Andrew Gallatin wrote: > David Miller wrote: >> From: Brice Goglin <brice@myri.com> >>> Is there a good summary somewhere of why GRO is better, >> >> Transparent forwarding/bridging support, easier driver port. >> >>> and how to >>> actually convert drivers? >> >> Step 1: Remove all of your LRO support code, every last line >> Step 2: netif_receive_skb() --> napi_gro_receive() >> vlan_hwaccel_rx() --> vlan_gro_receive() >> >> It couldn't be any easier. >> >> And it would also behoove you to look at the commits that converted or >> added GRO support to other drivers. That's how I learned it :-) > > Unfortunately, it doesn't appear that GRO is able to handle frags > (like lro_receive_frags()), so I anticipate its overhead would Ah, I missed napi_gro_frags()! I've got quick and dirty test patch which uses that, but I need to fix a few things. I also need to figure out why it seems to be a bit slower than LRO (varies from 8.5 to 9.2 Gb/s, rather than always 9.4Gb/s) on my old, weak 2.0GHz athlon64. Thanks for the pointer, Drew -- 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: Andrew Gallatin <gallatin@myri.com> Date: Wed, 15 Apr 2009 17:04:36 -0400 > Andrew Gallatin wrote: >> Unfortunately, it doesn't appear that GRO is able to handle frags >> (like lro_receive_frags()), so I anticipate its overhead would > > Ah, I missed napi_gro_frags()! I've got quick and dirty test > patch which uses that, but I need to fix a few things. I also need > to figure out why it seems to be a bit slower than LRO > (varies from 8.5 to 9.2 Gb/s, rather than always 9.4Gb/s) > on my old, weak 2.0GHz athlon64. Herbert has been working on various optimizations to get cxgb3 GRO performance on par with LRO. Perhaps he has some things for you to try :-) -- 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/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c index 9eed126..f2c4a66 100644 --- a/drivers/net/myri10ge/myri10ge.c +++ b/drivers/net/myri10ge/myri10ge.c @@ -2447,6 +2447,7 @@ static int myri10ge_open(struct net_device *dev) lro_mgr->lro_arr = ss->rx_done.lro_desc; lro_mgr->get_frag_header = myri10ge_get_frag_header; lro_mgr->max_aggr = myri10ge_lro_max_pkts; + lro_mgr->frag_align_pad = 2; if (lro_mgr->max_aggr > MAX_SKB_FRAGS) lro_mgr->max_aggr = MAX_SKB_FRAGS;
Add LRO alignment initially committed in 621544eb8c3beaa859c75850f816dd9b056a00a3 and removed in 0dcffac1a329be69bab0ac604bf7283737108e68 during conversion to multi-slice. Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com> --- drivers/net/myri10ge/myri10ge.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)