2008-08-29 14:02:49

by Matthew Wilcox

[permalink] [raw]
Subject: USBIP protocol


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.
- 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.
- The protocol needs an officially assigned port number. Port 3240 is
already assigned to Tony Matthews <tmatthews&triomotion.com> February
2002 (see http://www.iana.org/assignments/port-numbers)
- 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.

I think the protocol would be immeasurably improved by going through the
IETF RFC process and getting feedback from networking experts. Failing
that, I have some suggestions about how to improve it. I was hoping to
get my client finished before I started mucking with the protocol though.

(I have some other comments on the implementation, but they're a separate
issue).

--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."


2008-08-29 14:06:31

by Andi Kleen

[permalink] [raw]
Subject: Re: USBIP protocol

Matthew Wilcox <[email protected]> writes:

> 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.

If you want reliable transport with record boundaries an alternative
would be also SCTP. Main drawback is that firewalls often don't support it
though (but presumably that wouldn't be a big issue for this)

-Andi
--
[email protected]

2008-08-29 14:31:18

by Greg KH

[permalink] [raw]
Subject: Re: USBIP protocol

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.

> - 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.

> - The protocol needs an officially assigned port number. Port 3240 is
> already assigned to Tony Matthews <tmatthews&triomotion.com> February
> 2002 (see http://www.iana.org/assignments/port-numbers)

Great, let's get a real number.

> - 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?

> I think the protocol would be immeasurably improved by going through the
> IETF RFC process and getting feedback from networking experts. Failing
> that, I have some suggestions about how to improve it. I was hoping to
> get my client finished before I started mucking with the protocol though.

Why mess with the RFC process, is that really necessary for something
like this?

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.

thanks,

greg k-h

2008-08-29 14:44:22

by Matthew Wilcox

[permalink] [raw]
Subject: Re: USBIP protocol

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.

> > - 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.

> > - 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.

> > I think the protocol would be immeasurably improved by going through the
> > IETF RFC process and getting feedback from networking experts. Failing
> > that, I have some suggestions about how to improve it. I was hoping to
> > get my client finished before I started mucking with the protocol though.
>
> Why mess with the RFC process, is that really necessary for something
> like this?

It helps clarify the odd corners of any protocol. I don't have the
impression that it's a terribly heavy-weight process -- though we can
ask the netlink guys how it went for them.

> 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.

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.

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.

--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."

2008-08-29 14:54:52

by Greg KH

[permalink] [raw]
Subject: Re: USBIP protocol

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

2008-08-29 15:36:52

by Matthew Wilcox

[permalink] [raw]
Subject: Re: USBIP protocol

On Fri, Aug 29, 2008 at 07:54:07AM -0700, Greg KH wrote:
> 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 :)

Which brings us to the alternate -- that we need better framing in the
protocol.

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

No, but it does need agreement ;-)

> > > > - 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 :)

Patches don't seem appropriate for a design discussion. I'm more than
happy to make suggestions about how to unify the two protocols. I'll
send a followup to this with some ideas.

> > 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? :)

Surely you know that writing things down forces you to understand it
better?

--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."

2008-08-29 16:03:34

by Dave Higton

[permalink] [raw]
Subject: RE: USBIP protocol

Matthew Wilcox wrote:

> On Fri, Aug 29, 2008 at 07:54:07AM -0700, Greg KH wrote:
> >
> > Why, isn't the actual implementation better than a document? :)
>
> Surely you know that writing things down forces you to understand it
> better?

It's an enormous help for anyone working on the code, either now or
in the future. Reverse engineering a protocol from source code is
difficult.

Dave



*************************************************************************************************************************************************************************************************************************************************

NICE CTI Systems UK Limited ("NICE") is registered in England under company number, 3403044. The registered office of NICE is at Tollbar Way, Hedge End, Southampton, Hampshire SO30 2ZP.

Confidentiality: This communication and any attachments are intended for the above-named persons only and may be confidential and/or legally privileged. Any opinions expressed in this communication are not necessarily those of NICE. If this communication has come to you in error you must take no action based on it, nor must you copy or show it to anyone; please delete/destroy and inform the sender by e-mail immediately.

Monitoring: NICE may monitor incoming and outgoing e-mails.

Viruses: Although we have taken steps toward ensuring that this e-mail and attachments are free from any virus, we advise that in keeping with good computing practice the recipient should ensure they are actually virus free.

****************************************************************************************************************************************************************************************************************************************************


2008-08-29 20:31:58

by Marcel Holtmann

[permalink] [raw]
Subject: Re: USBIP protocol

Hi Matthew,

> > 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.
>
> If you want reliable transport with record boundaries an alternative
> would be also SCTP. Main drawback is that firewalls often don't support it
> though (but presumably that wouldn't be a big issue for this)

I would also have proposed SCTP. The telco carries seems be very happy
with it.

Regards

Marcel

2008-08-29 20:46:41

by Matthew Wilcox

[permalink] [raw]
Subject: Re: USBIP protocol

On Sat, Aug 30, 2008 at 12:31:43AM +0200, Marcel Holtmann wrote:
> Hi Matthew,
>
> > > 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.
> >
> > If you want reliable transport with record boundaries an alternative
> > would be also SCTP. Main drawback is that firewalls often don't support it
> > though (but presumably that wouldn't be a big issue for this)
>
> I would also have proposed SCTP. The telco carries seems be very happy
> with it.

I'm actually looking into the sunrpc protocol. That has the advantages:
- Already has an in-kernel implementation
- Widely understood and properly documented
- Can be run over TCP or UDP or even RDMA

--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."

2008-08-29 20:52:46

by Willy Tarreau

[permalink] [raw]
Subject: Re: USBIP protocol

On Fri, Aug 29, 2008 at 02:46:27PM -0600, Matthew Wilcox wrote:
> On Sat, Aug 30, 2008 at 12:31:43AM +0200, Marcel Holtmann wrote:
(...)
> > I would also have proposed SCTP. The telco carries seems be very happy
> > with it.
>
> I'm actually looking into the sunrpc protocol. That has the advantages:
> - Already has an in-kernel implementation
> - Widely understood and properly documented
> - Can be run over TCP or UDP or even RDMA

and :
- already supported by most firewalls.

Willy