Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
I've parked some working code, and I wanted to leave a pointer to it to end this thread. That is, the sense was that usbredir was not appropriate for the linux kernel, because Intel was working on a driver implementing the Media Agnostic USB standard, and having a proliferation of drivers didn't make sense. Stephanie and Sean of Intel tell me that they continue to progress on the ma-usb driver, so that all seems reasonable to me. However, due to time and customer pressure, I made the usbredir kernel module, and a set of associated utilities, work reasonably well for a small set of use cases. I've submitted that as an out of tree module build to the spice-devel mailing list. [1] It works reasonably well, and may benefit someone else that cannot wait for ma-usb. I felt it would be a courtesy to end this thread with this pointer; forgive me if it's unwanted noise. Cheers, Jeremy [1] http://lists.freedesktop.org/archives/spice-devel/2015-December/024779.html ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On Wed, Jul 22, 2015 at 11:55:32AM -0500, Jeremy White wrote: > I privately wrote to the Intel authors of that patch a week ago; I've > publicly included them in this thread as well. As far as I can tell, > they've been silent on this front since November; I fear that they may > have moved on, or that Intel is not actively working on this. None of > the Intel authors listed on the MA-USB specification are kernel > contributors, so I did not have a way to reach out to them. If you have > the means to engage others, I would appreciate that. Sorry for the delay. The short answer is: Yes, we have been actively working on this driver. Per Greg KH's request, we have been cleaning up the driver internally. There was a lot to clean up, which is why we have been silent on LKML. -Sean ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On 07/22/2015 12:59 PM, Sean O. Stalley wrote: On Wed, Jul 22, 2015 at 11:55:32AM -0500, Jeremy White wrote: I privately wrote to the Intel authors of that patch a week ago; I've publicly included them in this thread as well. As far as I can tell, they've been silent on this front since November; I fear that they may have moved on, or that Intel is not actively working on this. None of the Intel authors listed on the MA-USB specification are kernel contributors, so I did not have a way to reach out to them. If you have the means to engage others, I would appreciate that. Sorry for the delay. The short answer is: Yes, we have been actively working on this driver. Per Greg KH's request, we have been cleaning up the driver internally. There was a lot to clean up, which is why we have been silent on LKML. Great, thanks. I would appreciate a chance to work with you to make sure it will work well for XSpice. I'm happy to help as much as I can, and please don't feel the need to wait for a final version before reaching out to me. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On 07/22/2015 09:34 AM, Greg KH wrote: > On Wed, Jul 22, 2015 at 09:03:53AM -0500, Jeremy White wrote: >> On 07/09/2015 05:06 AM, Alex Elsayed wrote: >>> Alan Stern wrote: >>> On Mon, 6 Jul 2015, Jeremy White wrote: > Anything else fundamental to usbip that should inform the design of a > usbredir driver? usbip appears to be based off a 2004 vintage of > dummy_hcd. I'll look thoughtfully at the current dummy_hcd; please let > me know if there is anything else I should consider. One thing that springs to mind is USB-3 streams. When dummy-hcd was expanded to include USB-3, that was the major new ingredient. >>> >>> Another thing that comes to mind is that the USB-IF has its own official >>> standard for this kind of thing now, called Media-Agnostic USB[1]. In >>> November of 2014 a driver[2] was posted, followed by a second version[3], >>> and it is apparently being refined inside Intel[4]. >>> >>> [1] >>> http://www.usb.org/developers/docs/devclass_docs/Media_Agnostic_USB_v1.0.zip >>> [2] http://thread.gmane.org/gmane.linux.kernel/1820297 >>> [3] http://thread.gmane.org/gmane.linux.drivers.driver-project.devel/60498 >>> [4] http://article.gmane.org/gmane.linux.drivers.driver-project.devel/60757 >> >> Thanks for the pointer, Alex. >> >> I spent some time with the spec and their proposed code. It does seem >> plausible that XSpice could use a mausb/usbredir protocol converter. So if >> there was a mausb kernel module, I could potentially implement support in >> XSpice in user space and not need a usbredir module. >> >> I sent an email to the two developers at Intel to ask if there had been any >> further progress and if I could collaborate with them. I have not heard >> back. >> >> The MA spec is substantial and seems well thought out. But the usbredir >> protocol has the virtue of being relatively mature - it's 5 years old, with >> code in daily use. >> >> At this point it seems the best path forward is to continue work on the >> usbredir kernel module, which I will do unless I get some new information. > > Please work with the existing people, or with the existing spec, I don't > want to be adding multiple versions of this type of protocol to the > kernel. As it is, I really don't even want to take your code, given > that usbip is already there. Ignoring it isn't ok. The usbredir spec predates MA-USB by 4 years; it has greater claim to the title 'existing' than does MA-USB. I recognize that does not make it better, and I recognize the value of a spec from a standards body. But I also respect community standards in production use. And I did not and am not ignoring the MA-USB patch and spec. I privately wrote to the Intel authors of that patch a week ago; I've publicly included them in this thread as well. As far as I can tell, they've been silent on this front since November; I fear that they may have moved on, or that Intel is not actively working on this. None of the Intel authors listed on the MA-USB specification are kernel contributors, so I did not have a way to reach out to them. If you have the means to engage others, I would appreciate that. With no other input, my analysis was that it is better to proceed with the existing spec. It has a body of useful code, active users and developers, and I am certain it will solve my problem. Also, as for usbip, I'll point out that the existence of MA-USB corroborates Hans rationale for the need to supplant usbip. As I said, I will respond to any new information I receive. It would be great to have a kernel module developed (or at least approved) by seasoned hands at Intel. But how long should I wait? Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On Wed, Jul 22, 2015 at 09:03:53AM -0500, Jeremy White wrote: > On 07/09/2015 05:06 AM, Alex Elsayed wrote: > >Alan Stern wrote: > > > >>On Mon, 6 Jul 2015, Jeremy White wrote: > >> > >>>Anything else fundamental to usbip that should inform the design of a > >>>usbredir driver? usbip appears to be based off a 2004 vintage of > >>>dummy_hcd. I'll look thoughtfully at the current dummy_hcd; please let > >>>me know if there is anything else I should consider. > >> > >>One thing that springs to mind is USB-3 streams. When dummy-hcd was > >>expanded to include USB-3, that was the major new ingredient. > > > >Another thing that comes to mind is that the USB-IF has its own official > >standard for this kind of thing now, called Media-Agnostic USB[1]. In > >November of 2014 a driver[2] was posted, followed by a second version[3], > >and it is apparently being refined inside Intel[4]. > > > >[1] > >http://www.usb.org/developers/docs/devclass_docs/Media_Agnostic_USB_v1.0.zip > >[2] http://thread.gmane.org/gmane.linux.kernel/1820297 > >[3] http://thread.gmane.org/gmane.linux.drivers.driver-project.devel/60498 > >[4] http://article.gmane.org/gmane.linux.drivers.driver-project.devel/60757 > > Thanks for the pointer, Alex. > > I spent some time with the spec and their proposed code. It does seem > plausible that XSpice could use a mausb/usbredir protocol converter. So if > there was a mausb kernel module, I could potentially implement support in > XSpice in user space and not need a usbredir module. > > I sent an email to the two developers at Intel to ask if there had been any > further progress and if I could collaborate with them. I have not heard > back. > > The MA spec is substantial and seems well thought out. But the usbredir > protocol has the virtue of being relatively mature - it's 5 years old, with > code in daily use. > > At this point it seems the best path forward is to continue work on the > usbredir kernel module, which I will do unless I get some new information. Please work with the existing people, or with the existing spec, I don't want to be adding multiple versions of this type of protocol to the kernel. As it is, I really don't even want to take your code, given that usbip is already there. Ignoring it isn't ok. thanks, greg k-h ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On 07/09/2015 05:06 AM, Alex Elsayed wrote: Alan Stern wrote: On Mon, 6 Jul 2015, Jeremy White wrote: Anything else fundamental to usbip that should inform the design of a usbredir driver? usbip appears to be based off a 2004 vintage of dummy_hcd. I'll look thoughtfully at the current dummy_hcd; please let me know if there is anything else I should consider. One thing that springs to mind is USB-3 streams. When dummy-hcd was expanded to include USB-3, that was the major new ingredient. Another thing that comes to mind is that the USB-IF has its own official standard for this kind of thing now, called Media-Agnostic USB[1]. In November of 2014 a driver[2] was posted, followed by a second version[3], and it is apparently being refined inside Intel[4]. [1] http://www.usb.org/developers/docs/devclass_docs/Media_Agnostic_USB_v1.0.zip [2] http://thread.gmane.org/gmane.linux.kernel/1820297 [3] http://thread.gmane.org/gmane.linux.drivers.driver-project.devel/60498 [4] http://article.gmane.org/gmane.linux.drivers.driver-project.devel/60757 Thanks for the pointer, Alex. I spent some time with the spec and their proposed code. It does seem plausible that XSpice could use a mausb/usbredir protocol converter. So if there was a mausb kernel module, I could potentially implement support in XSpice in user space and not need a usbredir module. I sent an email to the two developers at Intel to ask if there had been any further progress and if I could collaborate with them. I have not heard back. The MA spec is substantial and seems well thought out. But the usbredir protocol has the virtue of being relatively mature - it's 5 years old, with code in daily use. At this point it seems the best path forward is to continue work on the usbredir kernel module, which I will do unless I get some new information. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
Alan Stern wrote: > On Mon, 6 Jul 2015, Jeremy White wrote: > >> Anything else fundamental to usbip that should inform the design of a >> usbredir driver? usbip appears to be based off a 2004 vintage of >> dummy_hcd. I'll look thoughtfully at the current dummy_hcd; please let >> me know if there is anything else I should consider. > > One thing that springs to mind is USB-3 streams. When dummy-hcd was > expanded to include USB-3, that was the major new ingredient. Another thing that comes to mind is that the USB-IF has its own official standard for this kind of thing now, called Media-Agnostic USB[1]. In November of 2014 a driver[2] was posted, followed by a second version[3], and it is apparently being refined inside Intel[4]. [1] http://www.usb.org/developers/docs/devclass_docs/Media_Agnostic_USB_v1.0.zip [2] http://thread.gmane.org/gmane.linux.kernel/1820297 [3] http://thread.gmane.org/gmane.linux.drivers.driver-project.devel/60498 [4] http://article.gmane.org/gmane.linux.drivers.driver-project.devel/60757 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On 07/08/2015 02:11 AM, Hans de Goede wrote: Hi, On 07-07-15 18:47, Jeremy White wrote: 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). style matters, as it's a thing with your brain. You learn patterns and if the patterns change, you have to do more work and don't see the real issues involved. So by ignoring our style you are saying you don't want anyone else in the kernel community to ever review or work on the code, which isn't ok. Looks like I can't side step this unless Hans is willing to shift the usbredir project entirely to using kernel style :-/. I'm fine with moving the usbredir project to the kernel style, the question is how to do this without causing any hidden breakage. Can you create a gnu-indent invocation which will do most of the work? And then a hopefully managable sized patch on top to fix the remaining style errors in usbredirparser ? Got it; that's a helpful surprise. I'll work on patches for all but the whitespace, for only the files needed. I'm hoping whitespace changes can be limited to a gnu-indent invocation at patch transfer time. I'd hate to destroy the history like that :-/. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
Hi, On 07-07-15 18:47, Jeremy White wrote: 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). style matters, as it's a thing with your brain. You learn patterns and if the patterns change, you have to do more work and don't see the real issues involved. So by ignoring our style you are saying you don't want anyone else in the kernel community to ever review or work on the code, which isn't ok. Looks like I can't side step this unless Hans is willing to shift the usbredir project entirely to using kernel style :-/. I'm fine with moving the usbredir project to the kernel style, the question is how to do this without causing any hidden breakage. Can you create a gnu-indent invocation which will do most of the work? And then a hopefully managable sized patch on top to fix the remaining style errors in usbredirparser ? I will plan to make changes so that checkpatch runs clean; I lay out my concerns and my plan below to make sure I'm taking the best path. My main concern with changing the ~2,500 lines of code from the upstream usbredir project is that it will increase the odds that I will introduce errors, both initially, and again later as I review and attempt to relay patches from the upstream. To summarize the checkpatch reports: the biggest issue is whitespace, which shouldn't be a problem; I should be able to automate that without error. There are also a fair number of one offs; FSF address, space after '!', etc. I hope to persuade Hans to take a few style only patches upstream to address those. That leaves a pack of about 60 brace placement and line length issues. I will plan to manually change those prior to submission. Any upstream changes that affect the same code will be manually corrected as well, prior to submission. Make sense? Sounds good, note that as said I'm fine with moving over the usbredir(parser) code to the kernel style, as long as the changes are reviewable. I think it may be best to only convert the usbredirdparser files, as those are the only ones you need for the kernel. Having a mixed style in usbredir is not ideal, but something I can live with. Regards, Hans ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
>> >> 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). > > style matters, as it's a thing with your brain. You learn patterns and > if the patterns change, you have to do more work and don't see the real > issues involved. So by ignoring our style you are saying you don't want > anyone else in the kernel community to ever review or work on the code, > which isn't ok. Looks like I can't side step this unless Hans is willing to shift the usbredir project entirely to using kernel style :-/. I will plan to make changes so that checkpatch runs clean; I lay out my concerns and my plan below to make sure I'm taking the best path. My main concern with changing the ~2,500 lines of code from the upstream usbredir project is that it will increase the odds that I will introduce errors, both initially, and again later as I review and attempt to relay patches from the upstream. To summarize the checkpatch reports: the biggest issue is whitespace, which shouldn't be a problem; I should be able to automate that without error. There are also a fair number of one offs; FSF address, space after '!', etc. I hope to persuade Hans to take a few style only patches upstream to address those. That leaves a pack of about 60 brace placement and line length issues. I will plan to manually change those prior to submission. Any upstream changes that affect the same code will be manually corrected as well, prior to submission. Make sense? Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On Mon, 6 Jul 2015, Jeremy White wrote: > Anything else fundamental to usbip that should inform the design of a > usbredir driver? usbip appears to be based off a 2004 vintage of > dummy_hcd. I'll look thoughtfully at the current dummy_hcd; please let > me know if there is anything else I should consider. One thing that springs to mind is USB-3 streams. When dummy-hcd was expanded to include USB-3, that was the major new ingredient. Alan Stern ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On 07/06/2015 03:20 AM, Oliver Neukum wrote: > On Fri, 2015-07-03 at 10:51 +0200, Krzysztof Opasiak wrote: >> Doesn't we have the same problem with functionfs/gadgetfs and >> dummy_hcd? >> Or with fuse? >> >> It's a very generic problem for all "virtualized devices" and it is >> known for quite a long time. This is why many tutorials about swap >> warns >> that swap should be set up only on real block devices which are fully >> served in kernel. > > Indeed. But the point is that it isn't limited to swap. As you as > a page needs to be laundered the problem exists. > Mmapping a file for write (non private) is enough. I'm persuaded to avoid user space in the core design. Anything else fundamental to usbip that should inform the design of a usbredir driver? usbip appears to be based off a 2004 vintage of dummy_hcd. I'll look thoughtfully at the current dummy_hcd; please let me know if there is anything else I should consider. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On Fri, 3 Jul 2015, Krzysztof Opasiak wrote: > > The point is that a device driver like usbip _cannot_ isolate the > > running kernel from the vagaries of the network transport if part of > > that transport occurs in userspace. > > > > If any part of the transport passes through userspace, you can end up > > in a situation like what I outlined above, where a message can't be > > transported until after its reply has been received. There's no way > > for a device driver to prevent a deadlock when this occurs, no matter > > what it virtualizes. > > > > Doesn't we have the same problem with functionfs/gadgetfs and dummy_hcd? > Or with fuse? Yes indeed. > It's a very generic problem for all "virtualized devices" and it is > known for quite a long time. This is why many tutorials about swap warns > that swap should be set up only on real block devices which are fully > served in kernel. That is good advice. :-) Alan Stern ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On 07/02/2015 10:20 PM, Alan Stern wrote: On Thu, 2 Jul 2015, Jeremy White wrote: Oliver is talking about the danger of having part of the communication path for a block device run through userspace. Imagine a situation where the client uses a USB storage device provided by the server as a swap device. And suppose a userspace daemon on the client has to process USB packets as they pass between the client and the server. If the daemon is idle for some time, parts of its address space may get stored in the swap area on the server and paged out. Now consider what happens when those parts of memory need to be paged back in. The client submits a request to read from the swap area. The request is transformed into USB packets and sent through the userspace daemon for transmission to the server. But the daemon can't process the packets because it is waiting for its missing parts to be paged back! Result: deadlock. Right. I followed that. Oliver also asserted that he believed that the current usbip implementation has this flaw; I do not follow that. The concept is that the usbip device driver virtualizes the device behavior; isolating the running kernel from the vagaries of the network transport. All proposed usbredir implementations, even if they move the network transport to user space, would retain that behavior. The point is that a device driver like usbip _cannot_ isolate the running kernel from the vagaries of the network transport if part of that transport occurs in userspace. If any part of the transport passes through userspace, you can end up in a situation like what I outlined above, where a message can't be transported until after its reply has been received. There's no way for a device driver to prevent a deadlock when this occurs, no matter what it virtualizes. Doesn't we have the same problem with functionfs/gadgetfs and dummy_hcd? Or with fuse? It's a very generic problem for all "virtualized devices" and it is known for quite a long time. This is why many tutorials about swap warns that swap should be set up only on real block devices which are fully served in kernel. -- Krzysztof Opasiak Samsung R&D Institute Poland Samsung Electronics ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On Thu, 2 Jul 2015, Jeremy White wrote: > > Oliver is talking about the danger of having part of the communication > > path for a block device run through userspace. > > > > Imagine a situation where the client uses a USB storage device provided > > by the server as a swap device. And suppose a userspace daemon on the > > client has to process USB packets as they pass between the client and > > the server. If the daemon is idle for some time, parts of its address > > space may get stored in the swap area on the server and paged out. > > > > Now consider what happens when those parts of memory need to be paged > > back in. The client submits a request to read from the swap area. > > The request is transformed into USB packets and sent through the > > userspace daemon for transmission to the server. But the daemon can't > > process the packets because it is waiting for its missing parts to be > > paged back! Result: deadlock. > > Right. I followed that. Oliver also asserted that he believed that the > current usbip implementation has this flaw; I do not follow that. The > concept is that the usbip device driver virtualizes the device behavior; > isolating the running kernel from the vagaries of the network transport. > All proposed usbredir implementations, even if they move the network > transport to user space, would retain that behavior. The point is that a device driver like usbip _cannot_ isolate the running kernel from the vagaries of the network transport if part of that transport occurs in userspace. If any part of the transport passes through userspace, you can end up in a situation like what I outlined above, where a message can't be transported until after its reply has been received. There's no way for a device driver to prevent a deadlock when this occurs, no matter what it virtualizes. Alan Stern ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On Thu, 2 Jul 2015, Jeremy White wrote: > >> I don't follow that analysis. The usbip interactions with the usb stack > >> all seem to be atomic, and never trigger a syscall, as far as I can > >> tell. A port reset will flip a few bits and return. A urb enqueue > >> queues and wakes a different thread, and returns. The alternate thread > >> performs the sendmsg. > >> > >> I'm not suggesting that running a storage device over usbip is > >> especially safe, but I don't see the limit on the design. > > > > Are you referring to the current code or the proposed user space pipe? > > I'm referring to current usbip code. But the proposed driver would have > the same behavior. > > To be clear, I think the only tangible new proposal is the one Hans put > forth, which would modify the driver I originally posted to use a > netlink socket instead of a passing a file descriptor in via sysfs. > That would allow the user space application responsible for initiating > the request to provide TLS as desired. It comes with the expense of an > extra memcpy, but I suspect Hans is right in saying the network > latencies make that an irrelevant cost. Oliver is talking about the danger of having part of the communication path for a block device run through userspace. Imagine a situation where the client uses a USB storage device provided by the server as a swap device. And suppose a userspace daemon on the client has to process USB packets as they pass between the client and the server. If the daemon is idle for some time, parts of its address space may get stored in the swap area on the server and paged out. Now consider what happens when those parts of memory need to be paged back in. The client submits a request to read from the swap area. The request is transformed into USB packets and sent through the userspace daemon for transmission to the server. But the daemon can't process the packets because it is waiting for its missing parts to be paged back! Result: deadlock. Alan Stern ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On 07/02/2015 02:59 PM, Alan Stern wrote: > On Thu, 2 Jul 2015, Jeremy White wrote: > I don't follow that analysis. The usbip interactions with the usb stack all seem to be atomic, and never trigger a syscall, as far as I can tell. A port reset will flip a few bits and return. A urb enqueue queues and wakes a different thread, and returns. The alternate thread performs the sendmsg. I'm not suggesting that running a storage device over usbip is especially safe, but I don't see the limit on the design. >>> >>> Are you referring to the current code or the proposed user space pipe? >> >> I'm referring to current usbip code. But the proposed driver would have >> the same behavior. >> >> To be clear, I think the only tangible new proposal is the one Hans put >> forth, which would modify the driver I originally posted to use a >> netlink socket instead of a passing a file descriptor in via sysfs. >> That would allow the user space application responsible for initiating >> the request to provide TLS as desired. It comes with the expense of an >> extra memcpy, but I suspect Hans is right in saying the network >> latencies make that an irrelevant cost. > > Oliver is talking about the danger of having part of the communication > path for a block device run through userspace. > > Imagine a situation where the client uses a USB storage device provided > by the server as a swap device. And suppose a userspace daemon on the > client has to process USB packets as they pass between the client and > the server. If the daemon is idle for some time, parts of its address > space may get stored in the swap area on the server and paged out. > > Now consider what happens when those parts of memory need to be paged > back in. The client submits a request to read from the swap area. > The request is transformed into USB packets and sent through the > userspace daemon for transmission to the server. But the daemon can't > process the packets because it is waiting for its missing parts to be > paged back! Result: deadlock. Right. I followed that. Oliver also asserted that he believed that the current usbip implementation has this flaw; I do not follow that. The concept is that the usbip device driver virtualizes the device behavior; isolating the running kernel from the vagaries of the network transport. All proposed usbredir implementations, even if they move the network transport to user space, would retain that behavior. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On 07/02/2015 01:46 PM, Oliver Neukum wrote: > On Thu, 2015-07-02 at 10:57 -0500, Jeremy White wrote: >> On 07/02/2015 07:10 AM, Oliver Neukum wrote: >>> On Thu, 2015-07-02 at 13:35 +0200, Hans de Goede wrote: Hi, On 02-07-15 10:45, Oliver Neukum wrote: > On Wed, 2015-07-01 at 10:06 +0100, Daniel P. Berrange wrote: > >> I don't really think it is sensible to be defining & implementing new >> network services which can't support strong encryption and >> authentication. >> Rather than passing the file descriptor to the kernel and having it do >> the I/O directly, I think it would be better to dissassociate the kernel >> from the network transport, and thus leave all sockets layer data I/O >> to userspace daemons so they can layer in TLS or SASL or whatever else >> is appropriate for the security need. > > Hi, > > this hits a fundamental limit. Block IO must be done entirely in kernel > space or the system will deadlock. The USB stack is part of the block > layer and the SCSI error handling. Thus if you involve user space you > cannot honor memory allocation with GFP_NOFS and you break all APIs > where we pass GFP_NOIO in the USB stack. > > Supposed you need to reset a storage device for error handling. > Your user space programm does a syscall, which allocates memory > and needs to launder pages. It proceeds to write to the storage device > you wish to reset. > > It is the same problem FUSE has with writable mmap. You cannot do > block devices in user space sanely. So how is this dealt with for usbip ? >>> >>> As far as I can tell, it isn't. Running a storage device over usbip >>> is a bit dangerous. >> >> I don't follow that analysis. The usbip interactions with the usb stack >> all seem to be atomic, and never trigger a syscall, as far as I can >> tell. A port reset will flip a few bits and return. A urb enqueue >> queues and wakes a different thread, and returns. The alternate thread >> performs the sendmsg. >> >> I'm not suggesting that running a storage device over usbip is >> especially safe, but I don't see the limit on the design. > > Are you referring to the current code or the proposed user space pipe? I'm referring to current usbip code. But the proposed driver would have the same behavior. To be clear, I think the only tangible new proposal is the one Hans put forth, which would modify the driver I originally posted to use a netlink socket instead of a passing a file descriptor in via sysfs. That would allow the user space application responsible for initiating the request to provide TLS as desired. It comes with the expense of an extra memcpy, but I suspect Hans is right in saying the network latencies make that an irrelevant cost. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On 07/02/2015 07:10 AM, Oliver Neukum wrote: > On Thu, 2015-07-02 at 13:35 +0200, Hans de Goede wrote: >> Hi, >> >> On 02-07-15 10:45, Oliver Neukum wrote: >>> On Wed, 2015-07-01 at 10:06 +0100, Daniel P. Berrange wrote: >>> I don't really think it is sensible to be defining & implementing new network services which can't support strong encryption and authentication. Rather than passing the file descriptor to the kernel and having it do the I/O directly, I think it would be better to dissassociate the kernel from the network transport, and thus leave all sockets layer data I/O to userspace daemons so they can layer in TLS or SASL or whatever else is appropriate for the security need. >>> >>> Hi, >>> >>> this hits a fundamental limit. Block IO must be done entirely in kernel >>> space or the system will deadlock. The USB stack is part of the block >>> layer and the SCSI error handling. Thus if you involve user space you >>> cannot honor memory allocation with GFP_NOFS and you break all APIs >>> where we pass GFP_NOIO in the USB stack. >>> >>> Supposed you need to reset a storage device for error handling. >>> Your user space programm does a syscall, which allocates memory >>> and needs to launder pages. It proceeds to write to the storage device >>> you wish to reset. >>> >>> It is the same problem FUSE has with writable mmap. You cannot do >>> block devices in user space sanely. >> >> So how is this dealt with for usbip ? > > As far as I can tell, it isn't. Running a storage device over usbip > is a bit dangerous. I don't follow that analysis. The usbip interactions with the usb stack all seem to be atomic, and never trigger a syscall, as far as I can tell. A port reset will flip a few bits and return. A urb enqueue queues and wakes a different thread, and returns. The alternate thread performs the sendmsg. I'm not suggesting that running a storage device over usbip is especially safe, but I don't see the limit on the design. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
Hi, On 02-07-15 10:45, Oliver Neukum wrote: On Wed, 2015-07-01 at 10:06 +0100, Daniel P. Berrange wrote: I don't really think it is sensible to be defining & implementing new network services which can't support strong encryption and authentication. Rather than passing the file descriptor to the kernel and having it do the I/O directly, I think it would be better to dissassociate the kernel from the network transport, and thus leave all sockets layer data I/O to userspace daemons so they can layer in TLS or SASL or whatever else is appropriate for the security need. Hi, this hits a fundamental limit. Block IO must be done entirely in kernel space or the system will deadlock. The USB stack is part of the block layer and the SCSI error handling. Thus if you involve user space you cannot honor memory allocation with GFP_NOFS and you break all APIs where we pass GFP_NOIO in the USB stack. Supposed you need to reset a storage device for error handling. Your user space programm does a syscall, which allocates memory and needs to launder pages. It proceeds to write to the storage device you wish to reset. It is the same problem FUSE has with writable mmap. You cannot do block devices in user space sanely. So how is this dealt with for usbip ? Regards, Hans ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On Wed, 2015-07-01 at 10:06 +0100, Daniel P. Berrange wrote: > I don't really think it is sensible to be defining & implementing new > network services which can't support strong encryption and authentication. > Rather than passing the file descriptor to the kernel and having it do > the I/O directly, I think it would be better to dissassociate the kernel > from the network transport, and thus leave all sockets layer data I/O > to userspace daemons so they can layer in TLS or SASL or whatever else > is appropriate for the security need. Hi, this hits a fundamental limit. Block IO must be done entirely in kernel space or the system will deadlock. The USB stack is part of the block layer and the SCSI error handling. Thus if you involve user space you cannot honor memory allocation with GFP_NOFS and you break all APIs where we pass GFP_NOIO in the USB stack. Supposed you need to reset a storage device for error handling. Your user space programm does a syscall, which allocates memory and needs to launder pages. It proceeds to write to the storage device you wish to reset. It is the same problem FUSE has with writable mmap. You cannot do block devices in user space sanely. Sorry Oliver ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
Hi, On 01-07-15 20:31, Jeremy White wrote: Assuming that's correct, then this seems to imply that the socket has raw plain text data being sent/received, and thus precludes the possibility of running any security protocol like TLS unless the kernel wants to have an impl of the TLS protocol. Good point. For completeness, I'll note that, in a Spice use case, the data would be encrypted by the normal Spice mechanisms. And it would be fairly straight forward to write a user space daemon that would accept TLS and then relay to the unencrypted socket (of course, it would rewrite everything, which would be inefficient). I don't really think it is sensible to be defining & implementing new network services which can't support strong encryption and authentication. Rather than passing the file descriptor to the kernel and having it do the I/O directly, I think it would be better to dissassociate the kernel from the network transport, and thus leave all sockets layer data I/O to userspace daemons so they can layer in TLS or SASL or whatever else is appropriate for the security need. And that would also eliminate the need to copy the parsing code, which would be a nice improvement. I considered this approach, but discarded it, perhaps wrongly, when my google fu suggested that netlink sockets were the best way to connect user space and a kernel module. (Because I perceived netlink sockets to be functionally equivalent to the relay daemon described, above). From the user space perspective, the usbredir parser has an interface that exposes about 20 callback functions, which are invoked with pointers to a variety of structures. The ideal would be to have a mechanism to 'call into' kernel space with those varying interfaces. Would using ioctls be a reasonable way to achieve this? Is there a better way? In the other direction, the usbredir hc provides a range of functions; I think most interesting are the urb en/dequeue, hub control, and hub status calls. Some of that can be handled in the driver; some would need to be passed on to user space. My google fu did not lead me to an obvious way to pass this information to user space. The approach that comes to mind is to use a signal, or woken socket, to instruct user space to poll. I'd appreciate further comments and advice. I think it makes sense to have the actual usbredir protocol parsing in the kernel, and use a netlink interface, this will make it much easier to deal with protocol extensions (although we have not had any extensions to the usbredir proto in a while), and will be much cleaner then an ioctl interface. I think that Daniel's concern can easily be fixed by rather then passing the fd of a socket into the kernel to simply forwarding the data back and forth from a socket opened by userspace into the netlink socket. This way SSL, SASL or whatever can be put in between, and you can even built a nice test-suite this way :) The downside of this is introducing an extra memcpy of all the data, but an ioctl interface has the same problem and is going to be unwieldy, so I advice against that. As for the extra memcpy I would not worry about that, in all the performance testing I've done it has almost always been all about latency not throughput. Regards, Hans ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
Hi, On 01-07-15 18:13, Greg KH wrote: On Wed, Jul 01, 2015 at 10:55:49AM -0500, Jeremy White wrote: 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. Because no one has cared to. Now it seems you care, so I'd prefer to see someone fix this up instead of adding another protocol that does much the same thing. I understand where you are coming from, but usbip is unfixable as it has no concept of capability negotiation, protocol versioning or some such. What we need is an usbip v2, and usbredir was written as that, and has been used in production for years now for redirection of usb devices from virtual-machine-viewers into qemu based virtual-machines. I understand that having 2 protocols for one thing is undesirable in general, but think of this as usb-mass-storage bulk transport vs uas, or ipv4 vs ipv6 in some cases it just is necessary to do a new better protocol. When I designed and implemented usbredir usbip was pretty much dead, but it got ressurected later. I've never spoken up on this and never attempted to block usbip's promotion out of staging, as that seemed unfair since no-one was working on a virtual-hcd driver for the usbredir protocol. Likewise I think it is unfair if my not speaking up back then now blocks an usbredir virtual-hcd driver from entering the kernel. Regards, Hans ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
> Assuming that's correct, then this seems to imply that the socket has raw > plain text data being sent/received, and thus precludes the possibility > of running any security protocol like TLS unless the kernel wants to have > an impl of the TLS protocol. Good point. For completeness, I'll note that, in a Spice use case, the data would be encrypted by the normal Spice mechanisms. And it would be fairly straight forward to write a user space daemon that would accept TLS and then relay to the unencrypted socket (of course, it would rewrite everything, which would be inefficient). > > I don't really think it is sensible to be defining & implementing new > network services which can't support strong encryption and authentication. > Rather than passing the file descriptor to the kernel and having it do > the I/O directly, I think it would be better to dissassociate the kernel > from the network transport, and thus leave all sockets layer data I/O > to userspace daemons so they can layer in TLS or SASL or whatever else > is appropriate for the security need. And that would also eliminate the need to copy the parsing code, which would be a nice improvement. I considered this approach, but discarded it, perhaps wrongly, when my google fu suggested that netlink sockets were the best way to connect user space and a kernel module. (Because I perceived netlink sockets to be functionally equivalent to the relay daemon described, above). From the user space perspective, the usbredir parser has an interface that exposes about 20 callback functions, which are invoked with pointers to a variety of structures. The ideal would be to have a mechanism to 'call into' kernel space with those varying interfaces. Would using ioctls be a reasonable way to achieve this? Is there a better way? In the other direction, the usbredir hc provides a range of functions; I think most interesting are the urb en/dequeue, hub control, and hub status calls. Some of that can be handled in the driver; some would need to be passed on to user space. My google fu did not lead me to an obvious way to pass this information to user space. The approach that comes to mind is to use a signal, or woken socket, to instruct user space to poll. I'd appreciate further comments and advice. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On Wed, Jul 01, 2015 at 10:55:49AM -0500, Jeremy White wrote: > 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. Because no one has cared to. Now it seems you care, so I'd prefer to see someone fix this up instead of adding another protocol that does much the same thing. > >> 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). style matters, as it's a thing with your brain. You learn patterns and if the patterns change, you have to do more work and don't see the real issues involved. So by ignoring our style you are saying you don't want anyone else in the kernel community to ever review or work on the code, which isn't ok. > >>> 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. I personally ignore RFC patches unless they are tiny and in an area that I happened to be worried about / working on at the moment as it feels that the submitter doesn't think they are good enough to be merged as-is, so I'll wait until they feel it is worth
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
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/msg8.html ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On Tue, Jun 30, 2015 at 10:34:25PM -0500, Jeremy White wrote: > > > >It's pointless to post a patch that you know has problems with it (i.e. > >it's not even in proper kernel coding style), as it will never be > >reviewed or even looked at. > > Thanks for the reply, and I'm sorry for the clumsy ask. > > I would still appreciate feedback on two points: > > 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? > 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. > >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? > I appreciate your patience (and clue bats) as I try to learn. > > > > >And really, all in one patch? That too is pretty hard to review... > > Yeah. I see the point of pain. I did not see a solution as I formed the > patch, but I'll try harder before resending. 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. thanks, greg k-h ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On Tue, Jun 30, 2015 at 04:44:10PM -0500, Jeremy White wrote: > This module uses the usbredir protocol and user space tools, > which are used by the SPICE project. > > Signed-off-by: Jeremy White > --- > MAINTAINERS |6 + > drivers/usb/Kconfig |2 + > drivers/usb/Makefile|1 + > drivers/usb/usbredir/Kconfig| 25 + > drivers/usb/usbredir/Makefile |4 + > drivers/usb/usbredir/README | 20 + > drivers/usb/usbredir/TODO | 12 + > drivers/usb/usbredir/device.c | 327 + > drivers/usb/usbredir/hub.c | 489 > drivers/usb/usbredir/main.c | 100 ++ > drivers/usb/usbredir/redir.c| 535 > drivers/usb/usbredir/rx.c | 40 + > drivers/usb/usbredir/strtok_r.c | 69 + > drivers/usb/usbredir/strtok_r.h | 26 + > drivers/usb/usbredir/sysfs.c| 145 +++ > drivers/usb/usbredir/tx.c | 151 +++ > drivers/usb/usbredir/urb.c | 266 > drivers/usb/usbredir/usbredir.h | 225 > drivers/usb/usbredir/usbredirfilter.c | 294 + > drivers/usb/usbredir/usbredirfilter.h | 144 +++ > drivers/usb/usbredir/usbredirparser.c | 1795 > +++ > drivers/usb/usbredir/usbredirparser.h | 386 ++ > drivers/usb/usbredir/usbredirproto-compat.h | 88 ++ > drivers/usb/usbredir/usbredirproto.h| 309 + > 24 files changed, 5459 insertions(+) > create mode 100644 drivers/usb/usbredir/Kconfig > create mode 100644 drivers/usb/usbredir/Makefile > create mode 100644 drivers/usb/usbredir/README > create mode 100644 drivers/usb/usbredir/TODO > create mode 100644 drivers/usb/usbredir/device.c > create mode 100644 drivers/usb/usbredir/hub.c > create mode 100644 drivers/usb/usbredir/main.c > create mode 100644 drivers/usb/usbredir/redir.c > create mode 100644 drivers/usb/usbredir/rx.c > create mode 100644 drivers/usb/usbredir/strtok_r.c > create mode 100644 drivers/usb/usbredir/strtok_r.h > create mode 100644 drivers/usb/usbredir/sysfs.c > create mode 100644 drivers/usb/usbredir/tx.c > create mode 100644 drivers/usb/usbredir/urb.c > create mode 100644 drivers/usb/usbredir/usbredir.h > create mode 100644 drivers/usb/usbredir/usbredirfilter.c > create mode 100644 drivers/usb/usbredir/usbredirfilter.h > create mode 100644 drivers/usb/usbredir/usbredirparser.c > create mode 100644 drivers/usb/usbredir/usbredirparser.h > create mode 100644 drivers/usb/usbredir/usbredirproto-compat.h > create mode 100644 drivers/usb/usbredir/usbredirproto.h It's pointless to post a patch that you know has problems with it (i.e. it's not even in proper kernel coding style), as it will never be reviewed or even looked at. Please do the most basic of polite things and fix this up before posting things. And really, all in one patch? That too is pretty hard to review... thanks, greg k-h ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On Tue, Jun 30, 2015 at 04:44:10PM -0500, Jeremy White wrote: > This module uses the usbredir protocol and user space tools, > which are used by the SPICE project. > > Signed-off-by: Jeremy White [snip] > diff --git a/drivers/usb/usbredir/Kconfig b/drivers/usb/usbredir/Kconfig > new file mode 100644 > index 000..284fd02 > --- /dev/null > +++ b/drivers/usb/usbredir/Kconfig > @@ -0,0 +1,25 @@ > +config USBREDIR > + tristate "USBREDIR support" > + depends on USB && NET > + ---help--- > + This enables connecting a remote USB device over IP using > + the USBREDIR protocol. This module provides a sysfs attach > + interface which, if given a socket connected to a remote > + usbredirserver, will enable the remote device to behave as > + though it were connected to the system running this module. > + > + For more information and user space tools, refer to the > + USBREDIR project, which can be found at > + http://www.spice-space.org/page/UsbRedir. [snip] > new file mode 100644 > index 000..217a2e4 > --- /dev/null > +++ b/drivers/usb/usbredir/README > @@ -0,0 +1,20 @@ > +USB Redirection Kernel Module > + > +This module allows a Linux system to instatiate USB devices > +that are located on a remote device. The USB data is transferred > +over a socket using the USBREDIR protocol, which is generally > +used in conjunction with the SPICE project. > + > +You will need the USBREDIR user space tools. They can > +be found at http://www.spice-space.org/page/UsbRedir. > + > +To use, start the usbredirserver on a remote system. > +For example, > + ./usbredirserver --port 4000 125f:db8a > +will export my ADATA thumb drive on the remote system. > + > +Next, on the local system, connect a socket and relay that to > +the kernel module. The connectkernel utility will do this as follows: > + ./connectkernel adata4000 my.remote.device.com 4000 > + > +The device should attach and be usable on the local system. What is the security story here ? If I am understanding correctly, you have a userspace helper which opens a socket, and does a connect() to establish the connection to the remote system, and then tells the kernel to use the file descriptor associated with the socket. Assuming that's correct, then this seems to imply that the socket has raw plain text data being sent/received, and thus precludes the possibility of running any security protocol like TLS unless the kernel wants to have an impl of the TLS protocol. I don't really think it is sensible to be defining & implementing new network services which can't support strong encryption and authentication. Rather than passing the file descriptor to the kernel and having it do the I/O directly, I think it would be better to dissassociate the kernel from the network transport, and thus leave all sockets layer data I/O to userspace daemons so they can layer in TLS or SASL or whatever else is appropriate for the security need. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
It's pointless to post a patch that you know has problems with it (i.e. it's not even in proper kernel coding style), as it will never be reviewed or even looked at. Thanks for the reply, and I'm sorry for the clumsy ask. I would still appreciate feedback on two points: 1. Is the basic premise reasonable? Is Hans correct in asserting that an alternate USB over IP module will be considered? 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? 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. I appreciate your patience (and clue bats) as I try to learn. And really, all in one patch? That too is pretty hard to review... Yeah. I see the point of pain. I did not see a solution as I formed the patch, but I'll try harder before resending. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
