On 07/01/2015 12:44 AM, Greg KH wrote: > On Tue, Jun 30, 2015 at 10:34:25PM -0500, Jeremy White wrote: >> 1. Is the basic premise reasonable? Is Hans correct in asserting that an >> alternate USB over IP module will be considered? > > I have no idea, if it fully replaces the usbip functionality, I don't > see why that would be rejected. But why can't you just fix up usbip for > the issues you find lacking? This is what Hans said 5 years ago: [1] > > 3) The protocol itself is far from ideal. > > Number 3 is the big deal breaker for me. I've looked at the > (undocumented) protocol by sifting through the source. And it is > very low level, all it does is shove usb packets back and forth > over the network. It has no concept of configuration > setting (the docs say make sure the device is in the right > configuration before sharing it). No concept of caching things > like descriptors, active configuration, per interface alt setting, > etc. > > Besides missing a lot of useful smarts the whole one packet at a > time approach does not really fly when it comes to isoc endpoints. > As there paper states, the vm-host / guest os drivers need to > make sure enough packets are submitted / queued at all time > to gap the network delay / fill the network pipe. > > For iso endpoints it makes much more sense to have a start / stop > stream model, where the usb-host "pumpes" the urb ringbuffer and > sends out data received from the usb device to the vm-host > (isoc input endp case), or sends data received from the vm-host > (through a buffer to deal with network jitter) to the isoc output > endpoint. > > This also halves the number of packets which need to be > send over the network, as their is no need for the vm-host to send > a request for each packet once an input stream has started / for > the usb-host to send an ack for each delivered packet for an ouput > stream. It would still send an error when an error occurs, but their > is no reason to ack all delivered packets. Given the delay > caused by buffering, etc. not being able to match up the error to > an exact packet is not important, as from the vm-host emulated usb > hc (host controller), the packet has long been delivered already. > > Instead we will simply report the error to the guest os for the > next packet enqueued by the guest after receiving notification of > the error from the usb-host. The protocol is now documented, so that part is out of date. I don't see any evidence that the bulk of his other concerns have been addressed, however. > >> 2. Do I correctly understand that there are no circumstances where copied >> code can be left unmodified? Even in the case where the copied code is >> working, production code, and the changes would be just for style? > > I doubt the changes would just be for "style" if you are craming it into > the kernel tree, as it's a totally different environment from any other > place this code might have been running in before. Well, the checkpatch.pl reports were all style (and mostly whitespace); roughly 3000 of them against 3000 lines of code :-/. I did review the code, looking for areas where I thought it would badly cram into the kernel, and I adjusted the few I found (and sent changes upstream). The ideal, of course, is to not want to copy this code at all. Daniel makes an alternate point that might lead to that; I'll reply to that thread. > >>> Please do the most basic of polite things and fix this up before posting >>> things. >> >> It is often difficult for a newcomer to know what the polite thing is, even >> after studying FAQs and documentation. > > Did you read Documentation/SubmittingPatches? Yes, and SubmittingDrivers, SubmitChecklist, every link listed on #kernelnewbies, and the entire lkml FAQ as well. In hindsight, I think it's mostly a failure of common sense on my part. The one constructive suggestion I would offer is that the 'RFC PATCH' is used heavily by the linux kernel community, but I didn't find much discussion of it in the documentation or FAQs. I think I jumped to some erroneous conclusions about it's use. I'm willing to try to add a note on that, if that would be helpful. > > Remember you need to make this trivial to review in order to get it > accepted. You have to do extra work because of this because our limited > resource is reviewers and maintainers, not developers. Yes, understood. Cheers, Jeremy [1] https://lists.gnu.org/archive/html/qemu-devel/2010-12/msg00008.html -- 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
