Here's 3 patches that adds USB-IP functionality code to the kernel tree.

It is against 2.6.27-rc4, and has been added to the -staging tree.  If
there are no major objections, I'll also queue it up in the usb tree for
submission into 2.6.28.

The code is based on Takahiro's great work that has been sitting outside
of the kernel for many many years now.  Brian Merrell took the most
recent version of the code, cleaned it up and sent it to me.  I did more
cleanup and split the patch into 3 pieces, making it a bit easier to
work with.

There are still a number of checkpatch.pl warnings/errors emitted, and
Brian said he would work on them in the upcoming weeks, but they don't
distract from the basic functionality and a good review would be nice to
have.

We do need some more documentation for how to use this code (you need
the tools at usbip.sf.net), and how the sysfs files are exporting data
to userspace.  Brian, can you write up the sysfs interface in the format
that Documentation/ABI/ details and also provide some kind of basic
HOWTO to include in the kernel source tree?

thanks,

greg k-h
--

[ message continues ]

From: Takahiro Hirofuchi <hirofuchi@users.sourceforge.net>

This adds the common functions needed by both the host and client side
of the USB/IP code.

Brian Merrell cleaned up a lot of this code and submitted it for
inclusion.  Greg also did a lot of cleanup.

Signed-off-by: Brian G. Merrell <bgmerrell@novell.com>
Cc: Takahiro Hirofuchi <hirofuchi@users.sourceforge.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/usb/Kconfig           |    2 
 drivers/usb/Makefile          |    2 
 drivers/usb/ip/Kconfig        |   14 
 drivers/usb/ip/Makefile       |    6 
 drivers/usb/ip/usbip_common.c |  997 ++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/ip/usbip_common.h |  406 +++++++++++++++++
 drivers/usb/ip/usbip_event.c  |  141 +++++
 7 files changed, 1568 insertions(+)

--- /dev/null
+++ b/drivers/usb/ip/Kconfig
@@ -0,0 +1,14 @@
+config USB_IP_COMMON
+	tristate "USB IP support (EXPERIMENTAL)"
+	depends on USB && EXPERIMENTAL
+	default N
+	---help---
+	  This enables pushing USB packets over IP to allow remote
+	  machines access to USB devices directly.  For more details,
+	  and links to the userspace utility programs to let this work
+	  properly, see http://usbip.naist.jp/
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called usbip_common_mod.
+
+	  If unsure, say N.
--- /dev/null
+++ b/drivers/usb/ip/Makefile
@@ -0,0 +1,6 @@
+obj-$(CONFIG_USB_IP_COMMON) += usbip_common_mod.o
+usbip_common_mod-objs := usbip_common.o usbip_event.o
+
+ifeq ($(CONFIG_USB_DEBUG),y)
+	EXTRA_CFLAGS += -DDEBUG
+endif
--- /dev/null
+++ b/drivers/usb/ip/usbip_common.c
@@ -0,0 +1,997 @@
+/*
+ * Copyright (C) 2003-2008 Takahiro Hirofuchi
+ *
+ * This is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This is distributed in the ...
[ message continues ]

Such functions better print into a buffer, handed over in one go.
Otherwise you can get results
-second message-


Are you sure you cannot badly recourse here?

	Regards
		Oliver
--

[ message continues ]

On Thu, 28 Aug 2008 16:00:31 -0700

Local variable `flag' is unneeded.

It's a bit sloppy that a user can do

	echo wibble > /sys/whatever-file-this-is

and that will set usbip_debug_flag to zero.  We should have a library
function somewhere which can be used for this very common pattern.

Something like

/*
 * Convert an optionally NULL-terminated digit string into a long.  Returns
 * a -ve errno if an invalid character was detected.  Otherwise returns the 
 * number of characters whcih were consumed from the input.
 */
int sysfs_parse_long(const char *buf, long *ret)
{
	...
}

then the above becomes

static ssize_t store_flag(struct device *dev, struct device_attribute *attr,
		const char *buf, size_t count)
{
	return sysfs_parse_long(buf, &usbip_debug_flag);
}




There's a heck of a lot of debugging code there.  It'd be good to have














Is it appropriate that this be under CONFIG_USB_DEBUG?  Or should there


This header adds a lot of symbols which have fairly generic-sounding
names and which don't have "usbip" within them.  There is a risk that


I assume the below data structures are known about by userspace?

If so, the comment should make that very very clear.  This is part of
the kernel ABI.

Is this interface versioned?  If not, should it be?

This header doesn't appear to have __KERNEL__ guards.  Should we be
aiming to be able to use this header directly within userspace


hm.  So we sit there waiting for a wakeup and each time one occurs we
will assume that there is a single event waiting at ut->event.

I don't see handshaking in event_handler().  It looks like this will be



It all does look rather racy.  In what context does usbip_recv_iso()

--

[ message continues ]

From: Takahiro Hirofuchi <hirofuchi@users.sourceforge.net>

This adds the USB IP client driver

Brian Merrell cleaned up a lot of this code and submitted it for
inclusion.  Greg also did a lot of cleanup.

Signed-off-by: Brian G. Merrell <bgmerrell@novell.com>
Cc: Takahiro Hirofuchi <hirofuchi@users.sourceforge.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/usb/ip/Kconfig      |   10 
 drivers/usb/ip/Makefile     |    3 
 drivers/usb/ip/vhci.h       |  142 ++++
 drivers/usb/ip/vhci_hcd.c   | 1282 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/ip/vhci_rx.c    |  251 ++++++++
 drivers/usb/ip/vhci_sysfs.c |  250 ++++++++
 drivers/usb/ip/vhci_tx.c    |  239 ++++++++
 7 files changed, 2177 insertions(+)

--- a/drivers/usb/ip/Kconfig
+++ b/drivers/usb/ip/Kconfig
@@ -12,3 +12,13 @@ config USB_IP_COMMON
 	  module will be called usbip_common_mod.
 
 	  If unsure, say N.
+
+config USB_IP_VHCI_HCD
+	tristate "USB IP client driver"
+	depends on USB_IP_COMMON
+	---help---
+	 This enables the USB IP host controller driver which will
+	 run on the client machine.
+
+	 To compile this driver as a module, choose M here: the
+	 module will be called vhci_hcd.
--- a/drivers/usb/ip/Makefile
+++ b/drivers/usb/ip/Makefile
@@ -1,6 +1,9 @@
 obj-$(CONFIG_USB_IP_COMMON) += usbip_common_mod.o
 usbip_common_mod-objs := usbip_common.o usbip_event.o
 
+obj-$(CONFIG_USB_IP_VHCI_HCD) += vhci-hcd.o
+vhci-hcd-objs := vhci_sysfs.o vhci_tx.o vhci_rx.o vhci_hcd.o
+
 ifeq ($(CONFIG_USB_DEBUG),y)
 	EXTRA_CFLAGS += -DDEBUG
 endif
--- /dev/null
+++ b/drivers/usb/ip/vhci.h
@@ -0,0 +1,142 @@
+/*
+ * Copyright (C) 2003-2008 Takahiro Hirofuchi
+ *
+ * This is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This is distributed in the hope that it will be useful,
+ * ...
[ message continues ]

I realise it is commented out, but still.

The resume method may sleep. It is documented as such. Thus it can

There's no reason to hold the lock longer than necessary by
allocating memory in the lock section.



This is called in the context of usb_submit_urb()



The branch would be taken


This calls urb->complete. If the completion handler takes a spinlock



	Regards
		Oliver
--

[ message continues ]

Neither of these two tests is necessary.  They are both carried out by

This is wrong.  The driver should simply return ret; it should not


Rather than deal with simulating initialization commands in difficult
contexts, it would be better to change usbcore.  We should recognize
the existence of "virtual" USB devices which don't need address
assignment or re-enumeration following reset.  Then much of the

While I don't fully grasp these comments, they don't seem entirely
right.  Maybe my understanding is just weak, though.

For example, there's never any need to notify that an unlink has 
completed.  In fact, unlinks don't complete at all -- URBs complete but 
unlinks don't.

Perhaps the trickiest part is what happens when you try to unlink 
before the submit request has been accepted by the server.  This could 
be handled in a couple of ways: The server could refuse the submit 
request, or the client could send the unlink message after the submit 
response is received.

But I don't see why the client would ever want to call 
usb_hcd_giveback_urb() from within the dequeue method.  Unless perhaps 

This test shouldn't be needed; the spinlock should prevent it from ever 


Why is this test repeated?  Couldn't the code be moved up into the 

WHy is there special code for unlinking URBs after the TCP connection 
has gone down?  Shouldn't all outstanding URBs immediately complete 
with -ESHUTDOWN status?

Alan Stern

--

[ message continues ]

From: Takahiro Hirofuchi <hirofuchi@users.sourceforge.net>

This adds the USB IP client driver

Brian Merrell cleaned up a lot of this code and submitted it for
inclusion.  Greg also did a lot of cleanup.

Signed-off-by: Brian G. Merrell <bgmerrell@novell.com>
Cc: Takahiro Hirofuchi <hirofuchi@users.sourceforge.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/usb/ip/Kconfig     |   10 
 drivers/usb/ip/Makefile    |    3 
 drivers/usb/ip/stub.h      |   95 ++++++
 drivers/usb/ip/stub_dev.c  |  483 +++++++++++++++++++++++++++++++++++
 drivers/usb/ip/stub_main.c |  300 +++++++++++++++++++++
 drivers/usb/ip/stub_rx.c   |  616 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/ip/stub_tx.c   |  371 +++++++++++++++++++++++++++
 7 files changed, 1878 insertions(+)

--- a/drivers/usb/ip/Kconfig
+++ b/drivers/usb/ip/Kconfig
@@ -22,3 +22,13 @@ config USB_IP_VHCI_HCD
 
 	 To compile this driver as a module, choose M here: the
 	 module will be called vhci_hcd.
+
+config USB_IP_HOST
+	tristate "USB IP host driver"
+	depends on USB_IP_COMMON
+	---help---
+	 This enables the USB IP device driver which will run on the
+	 host machine.
+
+	 To compile this driver as a module, choose M here: the
+	 module will be called usbip.
--- a/drivers/usb/ip/Makefile
+++ b/drivers/usb/ip/Makefile
@@ -4,6 +4,9 @@ usbip_common_mod-objs := usbip_common.o 
 obj-$(CONFIG_USB_IP_VHCI_HCD) += vhci-hcd.o
 vhci-hcd-objs := vhci_sysfs.o vhci_tx.o vhci_rx.o vhci_hcd.o
 
+obj-$(CONFIG_USB_IP_HOST) += usbip.o
+usbip-objs := stub_dev.o stub_main.o stub_rx.o stub_tx.o
+
 ifeq ($(CONFIG_USB_DEBUG),y)
 	EXTRA_CFLAGS += -DDEBUG
 endif
--- /dev/null
+++ b/drivers/usb/ip/stub_dev.c
@@ -0,0 +1,483 @@
+/*
+ * Copyright (C) 2003-2008 Takahiro Hirofuchi
+ *
+ * This is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your ...
[ message continues ]

Very cool stuff anyway !

regards,
Willy

--

[ message continues ]

Great stuff, I almost worked on udp_hcd or something along these lines,
will definetely take a look! :-)

- Arnaldo
--

[ message continues ]

I will indeed continue to work on getting rid of the checkpatch.pl
warnings/errors during the upcoming weeks.  I already eliminated the
ones I could figure out without too much effort;  I am sure the
remaining ones are trivial to many of you out there, so I welcome


Thanks,
Brian G. Merrell
--

[ message continues ]

The kthread one looks like you will have to change a bit of the code,
all of the other remaining ones should be self-explanatory.  If not,
please let us know and we'll be glad to help out.

Can you also work to address all of the review comments made on the code
so far as well?  I don't know how much time you have to work on this,
but for now I'll let you send me changes and not do anything on the code
myself.  Sound good?

thanks,

greg k-h
--

[ message continues ]

Well, now that school is back in session I haven't had a lot of time
to look at this.  I was really hoping to make time but it just
hasn't happened.  I finish my degree in December, but until then it
doesn't look like I will have a lot of time to look at this.  I
still want to contribute what I can, but I don't want to hold things
up.  So please don't not make changes on my account.  Sorry.

Thanks,
Brian G. Merrell
--

[ message continues ]