Re: USBIP protocol

[Greg KH <greg@kroah.com>]

On Fri, Aug 29, 2008 at 08:43:54AM -0600, Matthew Wilcox wrote:
> On Fri, Aug 29, 2008 at 07:30:17AM -0700, Greg KH wrote:
> > On Fri, Aug 29, 2008 at 08:02:24AM -0600, Matthew Wilcox wrote:
> > > 
> > > I'm in the middle of implementing a userspace client for usbip and I
> > > strongly feel that the protocol needs to be changed before it is merged.
> > > 
> > >  - I'm unconvinced that TCP is the correct protocol to be running this over.
> > >    I understand the reluctance to use UDP, but the protocol is fundamentally
> > >    packet-based.  If TCP is used, the delimitation of packets within the
> > >    stream needs to be much more robust.  I've managed to wedge the VHCI driver
> > >    a number of times in ways that just wouldn't be possible if we were using
> > >    a packet protocol instead of a stream protocol.
> > 
> > USB is fundamentally packet-based, so it kind of fits very well.
> 
> Erm, did you not read what I wrote?  USB is packet based.  TCP isn't.
> We shouldn't be using TCP here.

Sorry, early morning, no coffee yet...

I think in the end, we should still use TCP otherwise you just end up
reinventing it with UDP :)

> > >  - Endianness.  This is a mess.  The usbip protocol is big-endian, but the
> > >    encapsulated usb protocol is little-endian.  This doesn't matter to the
> > >    people who are just tunnelling usb from one computer to another, but for
> > >    someone implementing a usbip client, it's very confusing.
> > 
> > Then just document it, no big deal.
> > Yeah, the current code isn't the cleanest here (sparse throws up some
> > warnings), but it's not that much work to fix it up, it's on my todo
> > list.
> 
> I'm not talking about the code.  I'm talking about the protocol.  It's a
> mess to have two different endiannesses within the same packet.

Ok, switch it all to be little endian, not a bit deal.

> > >  - There are actually two completely different protocols in use.  First,
> > >    the usbipd daemon listens on port 3240, and handles device discovery.
> > >    When usbip successfully attaches to usbipd, both sides of the connection
> > >    pass the socket fd into the kernel and the protocol changes.
> > >  - The protocol sends a 48-byte packet header for every command (and every
> > >    response).  It's cunningly hidden as a union.
> > 
> > Is that a real problem?
> 
> Yes, it really is.  It complicates the protocol, complicates the
> implementation, introduces unnecessary state, and makes it impossible to
> renegotiate on the same connection.

Fair enough, patches welcome :)

> > Windows has had this for years, no need for a RFC there, and if we just
> > document this well, no need for one here either.
> 
> Yes, and as a result we can't interoperate with Windows.

That is because (see below)

> By the way, is this actually built into Windows or just available as
> several mutually incompatible and pay-for products?  I did some
> searching a few months ago and didn't come up with anything official
> from Microsoft.

There is nothing official, there are various incompatible and pay-for
products in this area.

> Even if we don't go through the RFC process, just writing down the
> on-wire protocol should be mandatory for taking this kind of thing into
> the kernel.

Why, isn't the actual implementation better than a document?  :)

thanks,

greg k-h