David Miller <davem@xxxxxxxxxxxxx> writes: > From: Eric Dumazet <eric.dumazet@xxxxxxxxx> > Date: Thu, 22 May 2014 14:03:21 -0700 > >> On Thu, 2014-05-22 at 13:58 -0700, Eric Dumazet wrote: >> >>> I would set rx_max (rx_urb_size) to SKB_MAX_HEAD(0) so that you do not >>> use high order allocations. >> >> Correction, that would need SKB_MAX_HEAD(NET_SKB_PAD + NET_IP_ALIGN), >> because drivers/net/usb/usbnet.c calls __netdev_alloc_skb_ip_align(). > > I seem to recall that this driver has a lot of strange buffering > restrictions and that Bjorn Mork was talking a lot about this > recently. Yes, I have been following this discussion with great interest, learning a lot. But although the problem is the same, I believe the driver in question isn't the one I have been looking at recently. The posted code snippet was from the NCM gadget driver (drivers/usb/gadget/f_ncm.c), isn't that right Jim? That driver implements the same protocol with the same inherent issues, but for the device side. The implementations are independent for historical reasons. This happened before I started looking at these things so I don't know exactly why. In an ideal world they should have shared pretty much everything except setup. > Bjorn people are getting really burnt because the driver ends up > having a skb->truesize of 32K for the buffers it allocates on receive > and this chokes up TCP and SCTP because the socket memory limits > are hitting earlier than they should. > > We've just in the past few postings been discussing whether the just > copy every packet into a more appropriately sized buffer, and it isn't > clear if that's a good idea of the data rates handled here. Yes, judging by this discussion I guess we should unconditionally copy instead of cloning in these drivers. We'll always have bad payload/truesize ratio for cloned skbs, often less than 1/10 even for max size payload. Actually, I thought we already did copy in the host cdc_ncm driver. But I was wrong. I was thinking of the cdc_mbim driver (which is different enough to have its own implementation of this part of the rx code). The cdc_ncm driver is cloning and the cdc_mbim driver is copying. So we're not even consistent... 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. BTW, wrt the data rates: These drivers are USB class drivers and we should really think of *all* possible rates, even future ones. This is not limited to 480 Mbps USB2. AFAICS, there isn't anything preventing the gadget driver from being used with e.g. a USB3380 controller to create a 5 Gbps NCM device. I'm sure the future will bring us even faster USB devices. The drivers will be the same. Which is sort of beautiful and scaring at the same time :-) But I assume the bad payload/truesize ratio is the most important factor here, so we should still copy? Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
