David Laight <David.Laight@xxxxxxxxxx> writes: > From: Bjørn Mork >> It doesn't matter whether the buffer size goes up or down. We have to >> keep usbnet and device syncronized to be able to split transfers at the >> correct boundaries. The spec allow skipping short packets when using >> max sized transfers. If we don't tell usbnet about our new expected rx >> buffer size, then it will merge and/or split NTBs. The driver does not >> support this, and the result will be lots of framing errors. >> >> Fix by always reallocating usbnet rx buffers when the rx_max value >> changes. > > I'm guessing that the rx_max value is the maximum size of the USB bulk > data 'message' that the device generates? > > As such the URB only need to be longer that it. So did I think too at first. That's how I added the bug fixed by this commit :-) The problem with NCM is that it explicitly allows (and encourage) using transfers which are multiples of the USB packet size, *without* any terminating short packet (0 or 1 byte). This means that the USB core won't know or care about the end of one transfer and the beginning of the next. Which is fine. But the cdc_ncm driver has to know, because it must split the transfers into frames it can decode. Now, the current cdc_ncm implementation has a built-in assumption that the size of the URB == rx_max. This lets it simplify the splitting into frames to nearly nothing: Any received URB contains exactly one frame. Therefore we need to keep the rx URB size strictly syncronized the rx_max. > (Or multiples of the USB packet size, and the driver then merge URB > when generating skbs.) > > Since you are now copying the data out of the URB's skb before > passing the ethernet packet upstream, is there ever any real > requirement to use a small rx_max? or ever change rx_max? Yes. usbnet doesn't currently recycle skbs. Continuously allocating 32 kB skbs (or even 64 kB truesize, which is the current cdc_ncm worst case) on memory constrained hosts is bound to fail eventually. Reducing rx_max will reduce the order of the allocations and thereby the probability of failing. I've had reports from OpenWRT users with Huawei modems (which tend to use extreme buffers), constantly hitting the usbnet allocation failure event 'EVENT_RX_MEMORY'. The result is of course terrible performance, where usbnet gets really busy trying to allocate buffers which just cannot be allocated... Reducing rx_max by simply rebuilding the old driver with a lower hard limit has pretty much eliminated these issues, but is not generally a solution because it makes some modem firmwares fail. True, copying instead of cloning is probably going to help this problem too, but I believe there are still reasons why you do not want to allow the 64 kB skb abuse. 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
