| Message ID | 87vbsw4z5x.fsf@nemi.mork.no |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, 2014-05-23 at 11:33 +0200, Bjørn Mork wrote: > Jim Baxter <jim_baxter@mentor.com> writes: > > >> I'll create and test a patch for the cdc_ncm host driver unless someone > >> else wants to do that. I haven't really played with the gadget driver > >> before, so I'd prefer if someone knowing it (Jim maybe?) could take care > >> of it. If not, then I can always make an attempt using dummy_hcd to > >> test it. > > I can create a patch for the host driver, I will issue the gadget patch > > first to resolve any issues, the fix would be similar. > > Well, I couldn't help myself. I just had to test it. The attached > patch works for me, briefly tested with an Ericsson H5321gw NCM device. > I have no ideas about the performance impact as that modem is limited to > 21 Mbps HSDPA. Ideally, the skb_in should not be freed/reallocated, but recycled, especially if using 32KB. But thats a minor detail, this patch is already a huge win for a 21Mbps device. Acked-by: Eric Dumazet <edumazet@google.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
On 05/23/2014 02:33 AM, Bjørn Mork wrote: > Jim Baxter <jim_baxter@mentor.com> writes: > >>> I'll create and test a patch for the cdc_ncm host driver unless someone >>> else wants to do that. I haven't really played with the gadget driver >>> before, so I'd prefer if someone knowing it (Jim maybe?) could take care >>> of it. If not, then I can always make an attempt using dummy_hcd to >>> test it. >> I can create a patch for the host driver, I will issue the gadget patch >> first to resolve any issues, the fix would be similar. > > Well, I couldn't help myself. I just had to test it. The attached > patch works for me, briefly tested with an Ericsson H5321gw NCM device. > I have no ideas about the performance impact as that modem is limited to > 21 Mbps HSDPA. If you are measuring performance with the likes of netperf, you should be able to get an idea of the performance effect from the change in service demand (CPU consumed per unit of work) even if the maximum throughput remains capped. You can run a netperf TCP_STREAM test along the lines of: netperf -H <otherguy> -c -C -t TCP_STREAM and also netperf -H <otherguy> -c -C -t TCP_RR For extra added credit you can consider either multiple runs and post-processing, or adding a -i 30,3 to the command line to tell netperf to run at least three iterations, no more than thirty and it will try to achieve a 99% confidence that the reported means for throughput, local and remote CPU utilization are within +/- 2.5% of the actual mean. You can narrow or widen that with a -I 99,<width>. A width of 5% is what gives the +/- 2.5% (and/or demonstrates my lack of accurate statistics knowledge :) ) happy benchmarking, rick jones -- 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, 2014-05-23 at 08:44 -0700, Rick Jones wrote: > If you are measuring performance with the likes of netperf, you should > be able to get an idea of the performance effect from the change in > service demand (CPU consumed per unit of work) even if the maximum > throughput remains capped. This wont be enough to truly get an idea of the gains this patch brings. Add some random drops in the equation, and instead of dropping packets, we gently add them in the out of order queue, gently coalesce them, gently allow SACK enabled flows to recover thanks to fast retransmits, with no latency effect (No expensive collapse/prunes) # nstat -az|egrep "TCPRcvCoalesce|TCPOFOQueue|TCPRcvCollapsed|Prune" TcpExtPruneCalled 118 0.0 TcpExtRcvPruned 0 0.0 TcpExtOfoPruned 0 0.0 TcpExtTCPRcvCollapsed 1009 0.0 TcpExtTCPRcvCoalesce 857806 0.0 TcpExtTCPOFOQueue 201246 0.0 Yep, we _might_ consume some cpu cycles in the perfect case where no packet was dropped, but this is kind of an utopia. -- 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 4c7431cd046a6972e153e23249ad490a3692f9a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Mork?= <bjorn@mork.no> Date: Fri, 23 May 2014 09:40:51 +0200 Subject: [RFC] net: cdc_ncm: reduce skb truesize in rx path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cloning the big skbs we use for USB buffering chokes up TCP and SCTP because the socket memory limits are hitting earlier than they should. It is better to unconditionally copy the unwrapped packets to freshly allocated skbs. Reported-by: Jim Baxter <jim_baxter@mentor.com> Signed-off-by: Bjørn Mork <bjorn@mork.no> --- drivers/net/usb/cdc_ncm.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index 93c9ca9924eb..2bbbd65591c7 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -1289,12 +1289,11 @@ next_ndp: break; } else { - skb = skb_clone(skb_in, GFP_ATOMIC); + /* create a fresh copy to reduce truesize */ + skb = netdev_alloc_skb_ip_align(dev->net, len); if (!skb) goto error; - skb->len = len; - skb->data = ((u8 *)skb_in->data) + offset; - skb_set_tail_pointer(skb, len); + memcpy(skb_put(skb, len), skb_in->data + offset, len); usbnet_skb_return(dev, skb); payload += len; /* count payload bytes in this NTB */ } -- 2.0.0.rc4