RE: [PATCH 04/13] USB/IP: kernel module for userspace URBs transmission

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dear Greg,

Thank you for your comment.

1) About wrapping

> You do this on all of your patches, please wrap your changelog lines at
> 72 columns so they show up correctly in the log.

Very sorry. Could I send them as V2?

Before that, I posted graphical version to my blog instead of the ASCII diagram.
http://linux-usbip-additions.blogspot.jp/2015/04/figs-user-space-urbs-transmission.html

2) Why not use usbfs

> This is really interesting, but how does this tie into usbfs where you can
> send and receive USB urbs today from userspace to USB devices?  Why do you
> need another user/kernel interface here?

There are 2 reasons.

2-1) Application(vhci_hcd) side is also needed

In my understanding, usbfs provides functions to control USB devices. So device(usbip_host) side can be done by usbfs but application(vhci_hcd) side cannot. My patch covers both device(usbip_host) and application(vhci_hcd) side. And it is quite the same in both side and works symmetrically.

2-2) Maintainability

Usbfs provides similar interfaces which USB core provides to USB host drives. To use the interfaces in user space, implementation which are included in some portions of usbip_host.ko, vhci_hcd.ko and usbip_core should be moved to userspace. At the same time, utilities must be modified. Interfaces used between the utilities and the kernel modules (sysfs) will be changed to function calls in userspace.

For example, I'd like to break down usbip_host.ko down as below.
(a) submits and cancels URBs to USB core
(b) core part: receives and handle URBs, manage submitted URBs, etc.
(c) provides functions to utilities via sysfs
(d) calls usbip_common's functions
(e) calls kernel functions

To move it to user space,
(a) replace with usbfs calls
(b) copy the core part
(c) modify to interface inside userspace or use usbfs directly
(d) port some portion of usbip_core
(e) replace with systemcalls and libraries.

Then, to use usbfs (a), another usbip_host like program which has same for (b) and different in (c), (d) and (e). By (c), utilities should be changed unless sysfs emulation is not provided.

I think it's better to use the kernel modules as-is. Strictly, it's almost as-is because I put a small code to make replaceable kernel_sendmsg() and kernel_recvmsg().

As a reference, I stored my prototype including userspace usbip_host with libusb(not sysfs but a portable wrapper of sysfs) in staging/usbip of linux 3.14.2. It still needs refactoring.
https://drive.google.com/drive/folders/0BxnuWBW_tB9NflFDY1hlcVBRNEd4ZzB2VFJ3OTI0REFGelNBV2xXTHRNc0lReGJlaTRGdDQ/0BxnuWBW_tB9NfjhabzVMSnZ6VGxrTXBwNEE1dFJYdENvaC1IMUg1ZG1kTU9iOUN1OEpGUlU
In the prototype, libsrc/stub_main.c, stub_dev.c, stub_rx.c and stub_tx.c are portings of usbip_host. stub_common.c and stub_event.c is usbip_core. Macro USE_LIBUSB in utilities denotes portions to be modified in utilities.

Getting off the point, I'm trying to make cross-platform usbip_host with libusb. It may has less relationship to kernel.

3) About SSL patch

> And there have been previous patches to add openssl support for usbip, but
> do so in a much different way from this patch set.  Is that the primary
> reason you did this work, or do you have some other goal / requirement with
> usbip that caused you to create this design?

I'm sorry. I just didn't know the previous patch because I'm new to the mailing list.
For my requirement, my SSL patches(08/13 and 09/13) are not needed. I need Secure WebSocket (wss:) but it's covered by 13/13.

Could I remove them from V2?

And, please, let me know the reference of the previous SSL patch because I couldn't search in the linux-usb mail archive. I will study it and send my SSL patch later if it has some other effect.

Thanks,

n.iwata
//
> -----Original Message-----
> From: Greg KH [mailto:greg@xxxxxxxxx]
> Sent: Friday, April 03, 2015 5:07 PM
> To: fx IWATA NOBUO
> Cc: valentina.manea.m@xxxxxxxxx; shuah.kh@xxxxxxxxxxx;
> linux-usb@xxxxxxxxxxxxxxx; fx MICHIMURA TADAO
> Subject: Re: [PATCH 04/13] USB/IP: kernel module for userspace URBs
> transmission
> 
> On Fri, Apr 03, 2015 at 09:33:32AM +0900, Nobuo Iwata wrote:
> > Originally, USB/IP transmits requests and response PDUs for preparation
> to transfer URBs in user space, after the preparation, URBs are transmitted
> in kernel space.
> >
> > To make easy to introduce application network protocols like SSL,
> WebSocket and so on, the driver, usbip_ux.ko, forwards URBs to USB/IP user
> space utilities. It's just like fuse driver for user space file system.
> 
> You do this on all of your patches, please wrap your changelog lines at
> 72 columns so they show up correctly in the log.
> 
> > Then, utilities transfer URBs in user space.
> >
> > To do so, usbip_trx_ops makes send/receive functions pluggable.
> kernel_sendmsg() and kernel_recvmsg() for kernel mode transfer can be
> substituted by read/write methods to user space utilities.
> >
> > In the absence of usbip_ux.ko, original kernel space transferring is valid.
> usbip_ux.ko replaces usbip_trx_ops in its init routine.
> >
> > A) Original - kernel space URBs transfer
> >
> > User            +-------+   1) import/export    +-------+
> > space           |uspipd,|<--------------------->|usbip, |
> >                 |usbip  |                       |usbipa |
> >                 +---+---+                       +---+---+
> >                     |                               |
> >         2)Set sockfd|                               |2)Set sockfd
> >           thru sysfs|                               |  thru sysfs
> >                     V                               V
> > Kernel          +-------+        4)URBs         +-------+
> > space           |usbip  |<--------------------->|vhci   |
> >                 |host.ko|                       |hcd.ko |
> >                 +-------+                       +-------+
> >                 3)link to kernel trx_ops        3)link to kernel
> trx_ops
> >
> > B) New - user space URBs transfer
> >
> > User            +-------+    1)import/export    +-------+
> > space           |uspipd,|<--------------------->|usbip, |
> >             +---|usbip  |<--------------------->|usbipa |---+
> > 2)Set sockfd|+--+-------+        6)URBs         +-------+--+|2)Set
> sockfd
> >   thru ioctl||      ^                              ^       ||  thru
> ioctl
> >   (right)   ||      |5)read/write      5)read/write|       ||
> (left)
> > 3)Set sockfd||      +-------+               +------+       ||3)Set
> Sockfd
> >   thru sysfs|+-----------+  | /dev/usbip-ux |  +-----------+|  thru
> sysfs
> >   (left)    V            V  V               V  V            V
> (right)
> > Kernel  +-------+       +-------+       +-------+       +-------+
> > space   |usbip  |<----->|usbip  |       |usbip  |<----->|vhci   |
> >         |host.ko|5)send |ux.ko  |       |ux.ko  |5)send |hcd.ko |
> >         +-------+  recv +-------+       +-------+  recv +-------+
> >         4)link to user trx_ops          4)link to user trx_ops
> 
> This is really interesting, but how does this tie into usbfs where you can
> send and receive USB urbs today from userspace to USB devices?  Why do you
> need another user/kernel interface here?
> 
> And there have been previous patches to add openssl support for usbip, but
> do so in a much different way from this patch set.  Is that the primary
> reason you did this work, or do you have some other goal / requirement with
> usbip that caused you to create this design?
> 
> thanks,
> 
> greg k-h
--
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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux